| Message ID | 1488970371-8865-2-git-send-email-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Mar 08, 2017 at 11:52:44AM +0100, Cédric Le Goater wrote: > This helper will be used to translate the server number of the XIVE > (which is a PIR) into an ICPState index number (which is a cpu index). > > Signed-off-by: Cédric Le Goater <clg@kaod.org> This seems a slightly roundabout way of doing things. Why not just have the vcpu_by_pir() interface, then have the XICSFabric implementor go directly from PIR to xics server state. > --- > hw/intc/xics.c | 11 +++++++++++ > hw/ppc/ppc.c | 16 ++++++++++++++++ > include/hw/ppc/xics.h | 1 + > target/ppc/cpu.h | 10 ++++++++++ > 4 files changed, 38 insertions(+) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index e740989a1162..209e1a75ecb9 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -49,6 +49,17 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id) > return -1; > } > > +int xics_get_cpu_index_by_pir(int pir) > +{ > + PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir); > + > + if (cpu) { > + return cpu->parent_obj.cpu_index; > + } > + > + return -1; > +} > + > void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu) > { > CPUState *cs = CPU(cpu); > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c > index 5f93083d4a16..94bbe382a73a 100644 > --- a/hw/ppc/ppc.c > +++ b/hw/ppc/ppc.c > @@ -1379,6 +1379,22 @@ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id) > return NULL; > } > > +PowerPCCPU *ppc_get_vcpu_by_pir(int pir) > +{ > + CPUState *cs; > + > + CPU_FOREACH(cs) { > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + > + if (env->spr_cb[SPR_PIR].default_value == pir) { > + return cpu; > + } > + } > + > + return NULL; > +} > + > void ppc_cpu_parse_features(const char *cpu_model) > { > CPUClass *cc; > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index 9a5e715fe553..42bd24e975cb 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -173,6 +173,7 @@ void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu); > > /* Internal XICS interfaces */ > int xics_get_cpu_index_by_dt_id(int cpu_dt_id); > +int xics_get_cpu_index_by_pir(int pir); > > void icp_set_cppr(ICPState *icp, uint8_t cppr); > void icp_set_mfrr(ICPState *icp, uint8_t mfrr); > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index 7c4a1f50b38b..24a5af95cb45 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -2518,5 +2518,15 @@ int ppc_get_vcpu_dt_id(PowerPCCPU *cpu); > */ > PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id); > > +/** > + * ppc_get_vcpu_by_pir_id: > + * @pir: Processor Identifier Register (SPR_PIR) > + * > + * Searches for a CPU by @pir. > + * > + * Returns: a PowerPCCPU struct > + */ > +PowerPCCPU *ppc_get_vcpu_by_pir(int pir); > + > void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len); > #endif /* PPC_CPU_H */
On 03/14/2017 06:38 AM, David Gibson wrote: > On Wed, Mar 08, 2017 at 11:52:44AM +0100, Cédric Le Goater wrote: >> This helper will be used to translate the server number of the XIVE >> (which is a PIR) into an ICPState index number (which is a cpu index). >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > This seems a slightly roundabout way of doing things. Why not just > have the vcpu_by_pir() interface, then have the XICSFabric implementor > go directly from PIR to xics server state. yes. This is only used to find ICPs and if we introduce a new ICP type object, as you suggest, we might not need anymore this interface. I will take a look. Thanks, C. >> --- >> hw/intc/xics.c | 11 +++++++++++ >> hw/ppc/ppc.c | 16 ++++++++++++++++ >> include/hw/ppc/xics.h | 1 + >> target/ppc/cpu.h | 10 ++++++++++ >> 4 files changed, 38 insertions(+) >> >> diff --git a/hw/intc/xics.c b/hw/intc/xics.c >> index e740989a1162..209e1a75ecb9 100644 >> --- a/hw/intc/xics.c >> +++ b/hw/intc/xics.c >> @@ -49,6 +49,17 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id) >> return -1; >> } >> >> +int xics_get_cpu_index_by_pir(int pir) >> +{ >> + PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir); >> + >> + if (cpu) { >> + return cpu->parent_obj.cpu_index; >> + } >> + >> + return -1; >> +} >> + >> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu) >> { >> CPUState *cs = CPU(cpu); >> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c >> index 5f93083d4a16..94bbe382a73a 100644 >> --- a/hw/ppc/ppc.c >> +++ b/hw/ppc/ppc.c >> @@ -1379,6 +1379,22 @@ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id) >> return NULL; >> } >> >> +PowerPCCPU *ppc_get_vcpu_by_pir(int pir) >> +{ >> + CPUState *cs; >> + >> + CPU_FOREACH(cs) { >> + PowerPCCPU *cpu = POWERPC_CPU(cs); >> + CPUPPCState *env = &cpu->env; >> + >> + if (env->spr_cb[SPR_PIR].default_value == pir) { >> + return cpu; >> + } >> + } >> + >> + return NULL; >> +} >> + >> void ppc_cpu_parse_features(const char *cpu_model) >> { >> CPUClass *cc; >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h >> index 9a5e715fe553..42bd24e975cb 100644 >> --- a/include/hw/ppc/xics.h >> +++ b/include/hw/ppc/xics.h >> @@ -173,6 +173,7 @@ void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu); >> >> /* Internal XICS interfaces */ >> int xics_get_cpu_index_by_dt_id(int cpu_dt_id); >> +int xics_get_cpu_index_by_pir(int pir); >> >> void icp_set_cppr(ICPState *icp, uint8_t cppr); >> void icp_set_mfrr(ICPState *icp, uint8_t mfrr); >> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h >> index 7c4a1f50b38b..24a5af95cb45 100644 >> --- a/target/ppc/cpu.h >> +++ b/target/ppc/cpu.h >> @@ -2518,5 +2518,15 @@ int ppc_get_vcpu_dt_id(PowerPCCPU *cpu); >> */ >> PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id); >> >> +/** >> + * ppc_get_vcpu_by_pir_id: >> + * @pir: Processor Identifier Register (SPR_PIR) >> + * >> + * Searches for a CPU by @pir. >> + * >> + * Returns: a PowerPCCPU struct >> + */ >> +PowerPCCPU *ppc_get_vcpu_by_pir(int pir); >> + >> void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len); >> #endif /* PPC_CPU_H */ >
On 03/14/2017 06:38 AM, David Gibson wrote: > On Wed, Mar 08, 2017 at 11:52:44AM +0100, Cédric Le Goater wrote: >> This helper will be used to translate the server number of the XIVE >> (which is a PIR) into an ICPState index number (which is a cpu index). >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > This seems a slightly roundabout way of doing things. Why not just > have the vcpu_by_pir() interface, then have the XICSFabric implementor > go directly from PIR to xics server state. So what you are saying is that we should try to move the "nature" of the 'server' parameter of the xics framework in the icp_get() handler of the XICSFabric. Correct ? Because at the end, it all boils down to use a 'server' to look for an ICPState. Each machine would do its conversion : xics_get_cpu_index_by_dt_id() for spapr xics_get_cpu_index_by_pir() for powernv C.
On Tue, Mar 14, 2017 at 06:00:43PM +0100, Cédric Le Goater wrote: > On 03/14/2017 06:38 AM, David Gibson wrote: > > On Wed, Mar 08, 2017 at 11:52:44AM +0100, Cédric Le Goater wrote: > >> This helper will be used to translate the server number of the XIVE > >> (which is a PIR) into an ICPState index number (which is a cpu index). > >> > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > > > This seems a slightly roundabout way of doing things. Why not just > > have the vcpu_by_pir() interface, then have the XICSFabric implementor > > go directly from PIR to xics server state. > > So what you are saying is that we should try to move the "nature" > of the 'server' parameter of the xics framework in the icp_get() > handler of the XICSFabric. Correct ? Because at the end, it all > boils down to use a 'server' to look for an ICPState. > > Each machine would do its conversion : > > xics_get_cpu_index_by_dt_id() for spapr > xics_get_cpu_index_by_pir() for powernv Yes, that's exactly right. I think it makes sense for the XICSFabric to be the thing that defines the mapping from XICS server numbers to whatever other ID is relevant.
diff --git a/hw/intc/xics.c b/hw/intc/xics.c index e740989a1162..209e1a75ecb9 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -49,6 +49,17 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id) return -1; } +int xics_get_cpu_index_by_pir(int pir) +{ + PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir); + + if (cpu) { + return cpu->parent_obj.cpu_index; + } + + return -1; +} + void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu) { CPUState *cs = CPU(cpu); diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c index 5f93083d4a16..94bbe382a73a 100644 --- a/hw/ppc/ppc.c +++ b/hw/ppc/ppc.c @@ -1379,6 +1379,22 @@ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id) return NULL; } +PowerPCCPU *ppc_get_vcpu_by_pir(int pir) +{ + CPUState *cs; + + CPU_FOREACH(cs) { + PowerPCCPU *cpu = POWERPC_CPU(cs); + CPUPPCState *env = &cpu->env; + + if (env->spr_cb[SPR_PIR].default_value == pir) { + return cpu; + } + } + + return NULL; +} + void ppc_cpu_parse_features(const char *cpu_model) { CPUClass *cc; diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index 9a5e715fe553..42bd24e975cb 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -173,6 +173,7 @@ void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu); /* Internal XICS interfaces */ int xics_get_cpu_index_by_dt_id(int cpu_dt_id); +int xics_get_cpu_index_by_pir(int pir); void icp_set_cppr(ICPState *icp, uint8_t cppr); void icp_set_mfrr(ICPState *icp, uint8_t mfrr); diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 7c4a1f50b38b..24a5af95cb45 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -2518,5 +2518,15 @@ int ppc_get_vcpu_dt_id(PowerPCCPU *cpu); */ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id); +/** + * ppc_get_vcpu_by_pir_id: + * @pir: Processor Identifier Register (SPR_PIR) + * + * Searches for a CPU by @pir. + * + * Returns: a PowerPCCPU struct + */ +PowerPCCPU *ppc_get_vcpu_by_pir(int pir); + void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len); #endif /* PPC_CPU_H */
This helper will be used to translate the server number of the XIVE (which is a PIR) into an ICPState index number (which is a cpu index). Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/intc/xics.c | 11 +++++++++++ hw/ppc/ppc.c | 16 ++++++++++++++++ include/hw/ppc/xics.h | 1 + target/ppc/cpu.h | 10 ++++++++++ 4 files changed, 38 insertions(+)