diff mbox series

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

Message ID e109a4092d80a825256d26a8e56dbb5a2ae6d04e.1705066642.git.mykyta_poturai@epam.com (mailing list archive)
State New
Headers show
Series Add support for MSI injection on Arm | expand

Commit Message

Mykyta Poturai Jan. 14, 2024, 10:01 a.m. UTC
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@epam.com>
---
 xen/arch/arm/include/asm/vgic.h | 11 +++++++++++
 xen/arch/arm/vgic-v3-its.c      | 35 +++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

Comments

Stefano Stabellini Jan. 24, 2024, 1:17 a.m. UTC | #1
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@epam.com>
> ---
>  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 ?

Other than that, it looks OK


> +    vgic_vcpu_inject_lpi(its->d, vlpi);
> +
> +    return 0;
> +}
> +
>  unsigned int vgic_v3_its_count(const struct domain *d)
>  {
>      struct host_its *hw_its;
> -- 
> 2.34.1
>
Julien Grall Jan. 24, 2024, 11:21 a.m. UTC | #2
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@epam.com>
>> ---
>>   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 Jan. 24, 2024, 11:41 a.m. UTC | #3
Hi,

On 14/01/2024 10:01, 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@epam.com>
> ---
>   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;

Reading the code, I think you want to use get_event_pending_irq(). This 
will return the associated pending_irq. Then you can...

> +
> +    vgic_vcpu_inject_lpi(its->d, vlpi);

... open-code vgic_vcpu_inject_lpi(). This would avoid access the guest 
memory (done by read_itte) and reduce to just one lookup (today you are 
doing two: read_itte() and irq_to_pending()).

Cheers,
diff mbox series

Patch

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;
+
+    vgic_vcpu_inject_lpi(its->d, vlpi);
+
+    return 0;
+}
+
 unsigned int vgic_v3_its_count(const struct domain *d)
 {
     struct host_its *hw_its;