Re: [PATCH for-5.0 v5 11/23] ppc/pnv: Introduce a pnv_xive_is_cpu_enabled() helper

[Cédric Le Goater]

On 20/11/2019 18:26, Greg Kurz wrote:
> On Fri, 15 Nov 2019 17:24:24 +0100
> Cédric Le Goater <address@hidden> wrote:
> 
>> and use this helper to exclude CPUs which are not enabled in the XIVE
>> controller.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>  hw/intc/pnv_xive.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>> index 71ca4961b6b1..4c8c6e51c20f 100644
>> --- a/hw/intc/pnv_xive.c
>> +++ b/hw/intc/pnv_xive.c
>> @@ -372,6 +372,20 @@ 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 bool pnv_xive_is_cpu_enabled(PnvXive *xive, PowerPCCPU *cpu)
>> +{
>> +    int pir = cpu_pir(cpu);
>> +    int thrd_id = pir & 0x7f;
>> +
>> +    return xive->regs[PC_THREAD_EN_REG0 >> 3] & PPC_BIT(thrd_id);
> 
> A similar check is open-coded in pnv_xive_get_indirect_tctx() :
> 
>     /* Check that HW thread is XIVE enabled */
>     if (!(xive->regs[PC_THREAD_EN_REG0 >> 3] & PPC_BIT(pir & 0x3f))) {
>         xive_error(xive, "IC: CPU %x is not enabled", pir);
>     }
> 
> The thread id is only the 6 lower bits of the PIR there, and so seems to
> indicate the skiboot sources:
> 
>         /* Get bit in register */
>         bit = c->pir & 0x3f;

skiboot uses 0x3f when enabling the TCTXT of a CPU because register
INT_TCTXT_EN0 covers cores 0-15 (normal) and 0-7 (fused) and 
register INT_TCTXT_EN1 covers cores 16-23 (normal) and 8-11 (fused). 
The encoding in the registers is a bit different.

> Why make it pir & 0x7f here ? 

See pnv_chip_core_pir_p9 comments for some details on the CPU ID 
layout.

> If it should actually be 0x3f, 
but yes, we should fix the mask in the register setting. 

> maybe also use the helper in pnv_xive_get_indirect_tctx().

This is getting changed later on. So I rather not.

C.

> 
>> +}
>> +
>>  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,
>> @@ -393,6 +407,10 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, 
>> uint8_t format,
>>              XiveTCTX *tctx;
>>              int ring;
>>  
>> +            if (!pnv_xive_is_cpu_enabled(xive, cpu)) {
>> +                continue;
>> +            }
>> +
>>              tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>>  
>>              /*
>