Re: [PATCH 1/2] arm: vgic: Add the ability to trigger MSIs from the Hypervisor

[Julien Grall <julien@xxxxxxx>]

Hi,

On 24/01/2024 01:17, Stefano Stabellini wrote:
> On Sun, 14 Jan 2024, Mykyta Poturai wrote:
> > Add the vgic_its_trigger_msi() function to the vgic interface. This
> > function allows to inject MSIs from the Hypervisor to the guest.
> > Which is useful for userspace PCI backend drivers.
> >
> > Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>
> > ---
> >   xen/arch/arm/include/asm/vgic.h | 11 +++++++++++
> >   xen/arch/arm/vgic-v3-its.c      | 35 +++++++++++++++++++++++++++++++++
> >   2 files changed, 46 insertions(+)
> >
> > diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/asm/vgic.h
> > index 922779ce14..4695743848 100644
> > --- a/xen/arch/arm/include/asm/vgic.h
> > +++ b/xen/arch/arm/include/asm/vgic.h
> > @@ -317,6 +317,17 @@ extern bool vgic_migrate_irq(struct vcpu *old, struct vcpu 
> > *new, unsigned int ir
> >   extern void vgic_check_inflight_irqs_pending(struct domain *d, struct vcpu *v,
> >                                                unsigned int rank, uint32_t r);
> >   
> > +#ifdef CONFIG_HAS_ITS
> > +int vgic_its_trigger_msi(struct domain *d, paddr_t doorbell_address,
> > +                                u32 devid, u32 eventid);
> > +#else
> > +static inline int vgic_its_trigger_msi(struct domain *d, paddr_t 
> > doorbell_address,
> > +                                u32 devid, u32 eventid)
> > +{
> > +    return -EOPNOTSUPP;
> > +}
> > +#endif /* CONFIG_HAS_ITS */
> > +
> >   #endif /* !CONFIG_NEW_VGIC */
> >   
> >   /*** Common VGIC functions used by Xen arch code ****/
> > diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> > index 70b5aeb822..683a378f6e 100644
> > --- a/xen/arch/arm/vgic-v3-its.c
> > +++ b/xen/arch/arm/vgic-v3-its.c
> > @@ -1484,6 +1484,41 @@ static int vgic_v3_its_init_virtual(struct domain *d, 
> > paddr_t guest_addr,
> >       return 0;
> >   }
> >   
> > +int vgic_its_trigger_msi(struct domain *d, paddr_t doorbell_address,
> > +                                u32 devid, u32 eventid)
> > +{
> > +    struct vcpu *vcpu;
> > +    struct virt_its *pos, *temp;
> > +    struct virt_its *its = NULL;
> > +    uint32_t vlpi;
> > +    bool ret;
> > +
> > +    list_for_each_entry_safe( pos, temp, &d->arch.vgic.vits_list, vits_list )
> > +    {
> > +        if ( pos->doorbell_address == doorbell_address )
> > +        {
> > +            its = pos;
> > +            break;
> > +        }
> > +    }
> > +
> > +    if ( !its )
> > +        return -EINVAL;
> > +
> > +    spin_lock(&its->its_lock);
> > +    ret = read_itte(its, devid, eventid, &vcpu, &vlpi);
> > +    spin_unlock(&its->its_lock);
> > +    if ( !ret )
> > +        return -1;
> > +
> > +    if ( vlpi == INVALID_LPI )
> > +        return -1;
> We need a better error code, maybe EINVAL or ENOENT ?
EINVAL tends to be overloaded. I would prefer ENOENT.

Cheers,

--
Julien Grall