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

[Greg Kurz]

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;

Why make it pir & 0x7f here ? If it should actually be 0x3f, maybe also use
the helper in pnv_xive_get_indirect_tctx().

> +}
> +
>  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);
>  
>              /*