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

["Cédric Le Goater" <clg@kaod.org>]

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;
>