[Qemu-devel] [PATCH v4 03/25] ppc/pnv: Introduce a PNV_CHIP_CPU_FOREACH() helper

Cédric Le Goater posted 25 patches 6 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v4 03/25] ppc/pnv: Introduce a PNV_CHIP_CPU_FOREACH() helper
Posted by Cédric Le Goater 6 years, 4 months ago
As there is now easy way to loop on the CPUs belonging to a chip, add
a helper to filter out external CPUs.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/pnv_xive.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index ae449aa1119b..e1c15b6b5b71 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -392,15 +392,36 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
     return pnv_xive_vst_read(xive, VST_TSEL_IVT, blk, idx, eas);
 }
 
+static int cpu_pir(PowerPCCPU *cpu)
+{
+    CPUPPCState *env = &cpu->env;
+    return env->spr_cb[SPR_PIR].default_value;
+}
+
+static int cpu_chip_id(PowerPCCPU *cpu)
+{
+    int pir = cpu_pir(cpu);
+    return (pir >> 8) & 0x7f;
+}
+
+#define PNV_CHIP_CPU_FOREACH(chip, cs)                                  \
+    CPU_FOREACH(cs)                                                     \
+        if (chip->chip_id != cpu_chip_id(POWERPC_CPU(cs))) {} else
+
 static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
                               uint8_t nvt_blk, uint32_t nvt_idx,
                               bool cam_ignore, uint8_t priority,
                               uint32_t logic_serv, XiveTCTXMatch *match)
 {
+    PnvXive *xive = PNV_XIVE(xptr);
     CPUState *cs;
     int count = 0;
 
-    CPU_FOREACH(cs) {
+    /*
+     * Loop on all CPUs of the machine and filter out the CPUs
+     * belonging to another chip.
+     */
+    PNV_CHIP_CPU_FOREACH(xive->chip, cs) {
         PowerPCCPU *cpu = POWERPC_CPU(cs);
         XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
         int ring;
-- 
2.21.0


Re: [PATCH v4 03/25] ppc/pnv: Introduce a PNV_CHIP_CPU_FOREACH() helper
Posted by David Gibson 6 years, 4 months ago
On Wed, Sep 18, 2019 at 06:06:23PM +0200, Cédric Le Goater wrote:
> As there is now easy way to loop on the CPUs belonging to a chip, add
> a helper to filter out external CPUs.

This seems a somewhat odd way to go about it, given that the chip does
have a cores array and the cores then have a threads array.  What's
the difficulty with using that rather than looping through all vcpus
and filtering?

> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/intc/pnv_xive.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index ae449aa1119b..e1c15b6b5b71 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -392,15 +392,36 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>      return pnv_xive_vst_read(xive, VST_TSEL_IVT, blk, idx, eas);
>  }
>  
> +static int cpu_pir(PowerPCCPU *cpu)
> +{
> +    CPUPPCState *env = &cpu->env;
> +    return env->spr_cb[SPR_PIR].default_value;
> +}
> +
> +static int cpu_chip_id(PowerPCCPU *cpu)
> +{
> +    int pir = cpu_pir(cpu);
> +    return (pir >> 8) & 0x7f;
> +}
> +
> +#define PNV_CHIP_CPU_FOREACH(chip, cs)                                  \
> +    CPU_FOREACH(cs)                                                     \
> +        if (chip->chip_id != cpu_chip_id(POWERPC_CPU(cs))) {} else
> +
>  static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>                                uint8_t nvt_blk, uint32_t nvt_idx,
>                                bool cam_ignore, uint8_t priority,
>                                uint32_t logic_serv, XiveTCTXMatch *match)
>  {
> +    PnvXive *xive = PNV_XIVE(xptr);
>      CPUState *cs;
>      int count = 0;
>  
> -    CPU_FOREACH(cs) {
> +    /*
> +     * Loop on all CPUs of the machine and filter out the CPUs
> +     * belonging to another chip.
> +     */
> +    PNV_CHIP_CPU_FOREACH(xive->chip, cs) {
>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>          XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>          int ring;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [PATCH v4 03/25] ppc/pnv: Introduce a PNV_CHIP_CPU_FOREACH() helper
Posted by Cédric Le Goater 6 years, 4 months ago
On 03/10/2019 03:50, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:23PM +0200, Cédric Le Goater wrote:
>> As there is now easy way to loop on the CPUs belonging to a chip, add
>> a helper to filter out external CPUs.
> 
> This seems a somewhat odd way to go about it, given that the chip does
> have a cores array and the cores then have a threads array.  What's
> the difficulty with using that rather than looping through all vcpus
> and filtering?

because I find some ugliness in using the chip->cores array I guess. 
See pnv_chip_core_realize(), pnv_chip_quad_realize() etc.

Anyway, I agree this is rather hacky and with Greg's ideas to move 
the XiveTCTX list under the XiveRouter, we should get rid of it.

C. 


>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/intc/pnv_xive.c | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>> index ae449aa1119b..e1c15b6b5b71 100644
>> --- a/hw/intc/pnv_xive.c
>> +++ b/hw/intc/pnv_xive.c
>> @@ -392,15 +392,36 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>>      return pnv_xive_vst_read(xive, VST_TSEL_IVT, blk, idx, eas);
>>  }
>>  
>> +static int cpu_pir(PowerPCCPU *cpu)
>> +{
>> +    CPUPPCState *env = &cpu->env;
>> +    return env->spr_cb[SPR_PIR].default_value;
>> +}
>> +
>> +static int cpu_chip_id(PowerPCCPU *cpu)
>> +{
>> +    int pir = cpu_pir(cpu);
>> +    return (pir >> 8) & 0x7f;
>> +}
>> +
>> +#define PNV_CHIP_CPU_FOREACH(chip, cs)                                  \
>> +    CPU_FOREACH(cs)                                                     \
>> +        if (chip->chip_id != cpu_chip_id(POWERPC_CPU(cs))) {} else
>> +
>>  static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>>                                uint8_t nvt_blk, uint32_t nvt_idx,
>>                                bool cam_ignore, uint8_t priority,
>>                                uint32_t logic_serv, XiveTCTXMatch *match)
>>  {
>> +    PnvXive *xive = PNV_XIVE(xptr);
>>      CPUState *cs;
>>      int count = 0;
>>  
>> -    CPU_FOREACH(cs) {
>> +    /*
>> +     * Loop on all CPUs of the machine and filter out the CPUs
>> +     * belonging to another chip.
>> +     */
>> +    PNV_CHIP_CPU_FOREACH(xive->chip, cs) {
>>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>>          XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>>          int ring;
>