[V3,25/29] x86/vmsi: Hook delivering remapping format msi to guest

Commit Message

lan,Tianyu Sept. 22, 2017, 3:02 a.m. UTC

From: Chao Gao <[e-mail]>

In two situations, hypervisor delivers a msi to a hvm guest. One is
when qemu sends a request to hypervisor through XEN_DMOP_inject_msi.
The other is when a physical interrupt arrives and it has been bound
to a guest msi.

For the former, the msi is routed to common vIOMMU layer if it is in
remapping format. For the latter, if the pt irq is bound to a guest
remapping msi, a new remapping msi is constructed based on the binding
information and routed to common vIOMMU layer.

Signed-off-by: Chao Gao <[e-mail]>
Signed-off-by: Lan Tianyu <[e-mail]>
---
 xen/arch/x86/hvm/irq.c       |  7 +++++++
 xen/arch/x86/hvm/vmsi.c      | 14 +++++++++++++-
 xen/drivers/passthrough/io.c | 21 ++++++++++-----------
 3 files changed, 30 insertions(+), 12 deletions(-)

Comments

Roger Pau Monné Oct. 19, 2017, 4:07 p.m. UTC | #1

On Thu, Sep 21, 2017 at 11:02:06PM -0400, Lan Tianyu wrote:
> diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
> index 6196334..349a8cf 100644
> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/io.c
> @@ -942,21 +942,20 @@ static void __msi_pirq_eoi(struct hvm_pirq_dpci *pirq_dpci)
>  static int _hvm_dpci_msi_eoi(struct domain *d,
>                               struct hvm_pirq_dpci *pirq_dpci, void *arg)
>  {
> -    int vector = (long)arg;
> +    uint8_t vector, dlm, vector_target = (long)arg;

Since you are changing this, please cast to (uint8_t) instead.

> +    uint32_t dest;
> +    bool dm;

Why are you moving dest, dm, dlm and vector here?

>  
> -    if ( (pirq_dpci->flags & HVM_IRQ_DPCI_MACH_MSI) &&
> -         (pirq_dpci->gmsi.legacy.gvec == vector) )
> +    if ( pirq_dpci->flags & HVM_IRQ_DPCI_MACH_MSI )
>      {
> -        unsigned int dest = MASK_EXTR(pirq_dpci->gmsi.legacy.gflags,
> -                                      XEN_DOMCTL_VMSI_X86_DEST_ID_MASK);
> -        bool dest_mode = pirq_dpci->gmsi.legacy.gflags &
> -                         XEN_DOMCTL_VMSI_X86_DM_MASK;

AFAICT their scope is limited to this if.

> +        if ( pirq_dpci_2_msi_attr(d, pirq_dpci, &vector, &dest, &dm, &dlm) )
> +            return 0;
>  
> -        if ( vlapic_match_dest(vcpu_vlapic(current), NULL, 0, dest,
> -                               dest_mode) )
> +        if ( vector == vector_target &&
> +             vlapic_match_dest(vcpu_vlapic(current), NULL, 0, dest, dm) )
>          {
> -            __msi_pirq_eoi(pirq_dpci);
> -            return 1;
> +                __msi_pirq_eoi(pirq_dpci);
> +                return 1;
>          }
>      }
>  
> -- 
> 1.8.3.1
>

Jan Beulich Oct. 20, 2017, 6:48 a.m. UTC | #2

>>> On 19.10.17 at 18:07, <[e-mail]> wrote:
> On Thu, Sep 21, 2017 at 11:02:06PM -0400, Lan Tianyu wrote:
>> diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
>> index 6196334..349a8cf 100644
>> --- a/xen/drivers/passthrough/io.c
>> +++ b/xen/drivers/passthrough/io.c
>> @@ -942,21 +942,20 @@ static void __msi_pirq_eoi(struct hvm_pirq_dpci *pirq_dpci)
>>  static int _hvm_dpci_msi_eoi(struct domain *d,
>>                               struct hvm_pirq_dpci *pirq_dpci, void *arg)
>>  {
>> -    int vector = (long)arg;
>> +    uint8_t vector, dlm, vector_target = (long)arg;
> 
> Since you are changing this, please cast to (uint8_t) instead.

That would cause a compiler warning - long (or unsigned long) is
the right type to use (on Xen and Linux at least) when wanting to
convert a pointer to an integer.

Jan

Patch

diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
index e425df9..e99ba7d 100644
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -23,9 +23,11 @@ 
 #include <xen/sched.h>
 #include <xen/irq.h>
 #include <xen/keyhandler.h>
+#include <xen/viommu.h>
 #include <asm/hvm/domain.h>
 #include <asm/hvm/support.h>
 #include <asm/msi.h>
+#include <asm/viommu.h>
 
 /* Must be called with hvm_domain->irq_lock hold */
 static void assert_gsi(struct domain *d, unsigned ioapic_gsi)
@@ -339,6 +341,11 @@  int hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data)
     uint8_t trig_mode = (data & MSI_DATA_TRIGGER_MASK)
         >> MSI_DATA_TRIGGER_SHIFT;
     uint8_t vector = data & MSI_DATA_VECTOR_MASK;
+    struct arch_irq_remapping_request request;
+
+    irq_request_msi_fill(&request, 0, addr, data);
+    if ( viommu_check_irq_remapping(d, &request) )
+        return viommu_handle_irq_request(d, &request);
 
     if ( !vector )
     {
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 7f21853..1244df1 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -31,6 +31,7 @@ 
 #include <xen/errno.h>
 #include <xen/sched.h>
 #include <xen/irq.h>
+#include <xen/viommu.h>
 #include <public/hvm/ioreq.h>
 #include <asm/hvm/io.h>
 #include <asm/hvm/vpic.h>
@@ -39,6 +40,7 @@ 
 #include <asm/current.h>
 #include <asm/event.h>
 #include <asm/io_apic.h>
+#include <asm/viommu.h>
 
 static void vmsi_inj_irq(
     struct vlapic *target,
@@ -115,7 +117,17 @@  void vmsi_deliver_pirq(struct domain *d, const struct hvm_pirq_dpci *pirq_dpci)
 
     ASSERT(pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_MSI);
 
-    vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode);
+    if ( pirq_dpci->flags & HVM_IRQ_DPCI_GUEST_REMAPPED )
+    {
+        struct arch_irq_remapping_request request;
+
+        irq_request_msi_fill(&request, pirq_dpci->gmsi.intremap.source_id,
+                             pirq_dpci->gmsi.intremap.addr,
+                             pirq_dpci->gmsi.intremap.data);
+        viommu_handle_irq_request(d, &request);
+    }
+    else
+        vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode);
 }
 
 /* Return value, -1 : multi-dests, non-negative value: dest_vcpu_id */
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index 6196334..349a8cf 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -942,21 +942,20 @@  static void __msi_pirq_eoi(struct hvm_pirq_dpci *pirq_dpci)
 static int _hvm_dpci_msi_eoi(struct domain *d,
                              struct hvm_pirq_dpci *pirq_dpci, void *arg)
 {
-    int vector = (long)arg;
+    uint8_t vector, dlm, vector_target = (long)arg;
+    uint32_t dest;
+    bool dm;
 
-    if ( (pirq_dpci->flags & HVM_IRQ_DPCI_MACH_MSI) &&
-         (pirq_dpci->gmsi.legacy.gvec == vector) )
+    if ( pirq_dpci->flags & HVM_IRQ_DPCI_MACH_MSI )
     {
-        unsigned int dest = MASK_EXTR(pirq_dpci->gmsi.legacy.gflags,
-                                      XEN_DOMCTL_VMSI_X86_DEST_ID_MASK);
-        bool dest_mode = pirq_dpci->gmsi.legacy.gflags &
-                         XEN_DOMCTL_VMSI_X86_DM_MASK;
+        if ( pirq_dpci_2_msi_attr(d, pirq_dpci, &vector, &dest, &dm, &dlm) )
+            return 0;
 
-        if ( vlapic_match_dest(vcpu_vlapic(current), NULL, 0, dest,
-                               dest_mode) )
+        if ( vector == vector_target &&
+             vlapic_match_dest(vcpu_vlapic(current), NULL, 0, dest, dm) )
         {
-            __msi_pirq_eoi(pirq_dpci);
-            return 1;
+                __msi_pirq_eoi(pirq_dpci);
+                return 1;
         }
     }