Re: [RFC PATCH] spapr/xive: Allocate vCPU IPIs from local context

[Cédric Le Goater]

On 10/1/21 12:38, Greg Kurz wrote:
> On Fri, 1 Oct 2021 11:59:45 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
>
> > > > > Maybe it would be simpler to call xive_source_is_initialized() instead of
> > > > > xive_eas_is_valid() in cases like this, e.g. hcall code since it is shared
> > > > > between emulation and KVM ?
> > > >
> > > > Yes but we need a better check than :
> > > >
> > > >        if (lisn < SPAPR_XIRQ_BASE) {
> > > >            return !!xive_get_field64(EAS_END_INDEX, xive->eat[lisn].w);
> > > >        }
> > > >
> > > > This is more an heuristic than a precise test on the "validity" of
> > > > a source at the KVM level.
> > >
> > > I guess we should be able to get kvmppc_xive_irq_state::valid from
> > > KVM by making the KVM_DEV_XIVE_GRP_SOURCE attribute readable. Would
> > > that be enough ?
> >
> > A bit slow may be. Or an extra bit in the XiveSource 'status' field
> > to reflect KVM initialization.
> >
> > C.
>
> Something like this ?

Yes.

we need to add is/set/clr wrappers for the ASSERTED bit first. It
would clarify the code. Then add the KVM bit.

C.

> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> index 42f72b68fc00..6eba58b0ff10 100644
> --- a/hw/intc/spapr_xive_kvm.c
> +++ b/hw/intc/spapr_xive_kvm.c
> @@ -232,6 +232,7 @@ int kvmppc_xive_source_reset_one(XiveSource *xsrc, int 
> srcno, Error **errp)
>   {
>       SpaprXive *xive = SPAPR_XIVE(xsrc->xive);
>       uint64_t state = 0;
> +    int ret;
>   
>       assert(xive->fd != -1);
>   
> @@ -242,8 +243,15 @@ int kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp)
>           }
>       }
>   
> -    return kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE, srcno, &state,
> -                             true, errp);
> +    ret = kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE, srcno, &state,
> +                            true, errp);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    xsrc->status[srcno] |= XIVE_STATUS_KVM;
> +
> +    return 0;
>   }
>   
>   static int kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp)
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 445eccfe6b73..d85520beb64a 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -252,6 +252,7 @@ static inline hwaddr xive_source_esb_mgmt(XiveSource *xsrc, 
> int srcno)
>    * When doing an EOI, the Q bit will indicate if the interrupt
>    * needs to be re-triggered.
>    */
> +#define XIVE_STATUS_KVM       0x8  /* Set when initialized in KVM */
>   #define XIVE_STATUS_ASSERTED  0x4  /* Extra bit for LSI */
>   #define XIVE_ESB_VAL_P        0x2
>   #define XIVE_ESB_VAL_Q        0x1