diff mbox

intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE

Message ID 1474474337-45073-1-git-send-email-feng.wu@intel.com
State New
Headers show

Commit Message

Wu, Feng Sept. 21, 2016, 4:12 p.m. UTC 
The Trigger Mode field of IOAPIC must match the Trigger Mode in
the IRTE according to VT-d Spec 5.1.5.1.

Signed-off-by: Feng Wu <feng.wu@intel.com>
---
 hw/i386/intel_iommu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Peter Xu Sept. 21, 2016, 5:45 a.m. UTC | #1 
On Thu, Sep 22, 2016 at 12:12:17AM +0800, Feng Wu wrote:
> The Trigger Mode field of IOAPIC must match the Trigger Mode in
> the IRTE according to VT-d Spec 5.1.5.1.
> 
> Signed-off-by: Feng Wu <feng.wu@intel.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

Could I ask why we want this now? I know that both vector and trigger
mode should not be aligned in current kernel. However haven't go
deeper yet.

Thanks,

-- peterx
Wu, Feng Sept. 21, 2016, 5:54 a.m. UTC | #2 
> -----Original Message-----

> From: Peter Xu [mailto:peterx@redhat.com]

> Sent: Wednesday, September 21, 2016 1:45 PM

> To: Wu, Feng <feng.wu@intel.com>

> Cc: qemu-devel@nongnu.org; pbonzini@redhat.com

> Subject: Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode

> against the one in IRTE

> 

> On Thu, Sep 22, 2016 at 12:12:17AM +0800, Feng Wu wrote:

> > The Trigger Mode field of IOAPIC must match the Trigger Mode in

> > the IRTE according to VT-d Spec 5.1.5.1.

> >

> > Signed-off-by: Feng Wu <feng.wu@intel.com>

> 

> Reviewed-by: Peter Xu <peterx@redhat.com>

> 

> Could I ask why we want this now? I know that both vector and trigger

> mode should not be aligned in current kernel. 


Oh, I don't aware of this. I was just looking at the code and found seems
the Spec says we need to check it. So I added the check. :)

Thanks,
Feng

> However haven't go

> deeper yet.

> 

> Thanks,

> 

> -- peterx
Peter Xu Sept. 21, 2016, 6:12 a.m. UTC | #3 
On Wed, Sep 21, 2016 at 05:54:40AM +0000, Wu, Feng wrote:
> 
> 
> > -----Original Message-----
> > From: Peter Xu [mailto:peterx@redhat.com]
> > Sent: Wednesday, September 21, 2016 1:45 PM
> > To: Wu, Feng <feng.wu@intel.com>
> > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com
> > Subject: Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode
> > against the one in IRTE
> > 
> > On Thu, Sep 22, 2016 at 12:12:17AM +0800, Feng Wu wrote:
> > > The Trigger Mode field of IOAPIC must match the Trigger Mode in
> > > the IRTE according to VT-d Spec 5.1.5.1.
> > >
> > > Signed-off-by: Feng Wu <feng.wu@intel.com>
> > 
> > Reviewed-by: Peter Xu <peterx@redhat.com>
> > 
> > Could I ask why we want this now? I know that both vector and trigger
> > mode should not be aligned in current kernel. 
> 
> Oh, I don't aware of this. I was just looking at the code and found seems
> the Spec says we need to check it. So I added the check. :)

Yeah, that's good enough a reason. :-)

-- peterx
Wu, Feng Sept. 21, 2016, 6:34 a.m. UTC | #4 
> -----Original Message-----

> From: Peter Xu [mailto:peterx@redhat.com]

> Sent: Wednesday, September 21, 2016 2:12 PM

> To: Wu, Feng <feng.wu@intel.com>

> Cc: qemu-devel@nongnu.org; pbonzini@redhat.com

> Subject: Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode

> against the one in IRTE

> 

> On Wed, Sep 21, 2016 at 05:54:40AM +0000, Wu, Feng wrote:

> >

> >

> > > -----Original Message-----

> > > From: Peter Xu [mailto:peterx@redhat.com]

> > > Sent: Wednesday, September 21, 2016 1:45 PM

> > > To: Wu, Feng <feng.wu@intel.com>

> > > Cc: qemu-devel@nongnu.org; pbonzini@redhat.com

> > > Subject: Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger

> Mode

> > > against the one in IRTE

> > >

> > > On Thu, Sep 22, 2016 at 12:12:17AM +0800, Feng Wu wrote:

> > > > The Trigger Mode field of IOAPIC must match the Trigger Mode in

> > > > the IRTE according to VT-d Spec 5.1.5.1.

> > > >

> > > > Signed-off-by: Feng Wu <feng.wu@intel.com>

> > >

> > > Reviewed-by: Peter Xu <peterx@redhat.com>

> > >

> > > Could I ask why we want this now? I know that both vector and trigger

> > > mode should not be aligned in current kernel.

> >

> > Oh, I don't aware of this. I was just looking at the code and found seems

> > the Spec says we need to check it. So I added the check. :)

> 

> Yeah, that's good enough a reason. :-)


Thanks for your review :)

Thanks,
Feng

> 

> -- peterx
Wu, Feng Sept. 28, 2016, 10:05 a.m. UTC | #5 
Hi Paolo,


> -----Original Message-----
> From: Wu, Feng
> Sent: Thursday, September 22, 2016 12:12 AM
> To: qemu-devel@nongnu.org
> Cc: pbonzini@redhat.com; Wu, Feng <feng.wu@intel.com>
> Subject: [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one in
> IRTE
> 
> The Trigger Mode field of IOAPIC must match the Trigger Mode in
> the IRTE according to VT-d Spec 5.1.5.1.

Any comments about this patch? Thanks a lot!

Thanks,
Feng

> 
> Signed-off-by: Feng Wu <feng.wu@intel.com>
> ---
>  hw/i386/intel_iommu.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 28c31a2..f32ad5c 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -27,6 +27,7 @@
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pci_bus.h"
>  #include "hw/i386/pc.h"
> +#include "hw/i386/apic-msidef.h"
>  #include "hw/boards.h"
>  #include "hw/i386/x86-iommu.h"
>  #include "hw/pci-host/q35.h"
> @@ -2203,6 +2204,8 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState
> *iommu,
>          }
>      } else {
>          uint8_t vector = origin->data & 0xff;
> +        uint8_t trigger_mode = (origin->data >> MSI_DATA_TRIGGER_SHIFT) & 0x1;
> +
>          VTD_DPRINTF(IR, "received IOAPIC interrupt");
>          /* IOAPIC entry vector should be aligned with IRTE vector
>           * (see vt-d spec 5.1.5.1). */
> @@ -2211,6 +2214,15 @@ static int
> vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
>                          "entry: %d, IRTE: %d, index: %d",
>                          vector, irq.vector, index);
>          }
> +
> +        /* The Trigger Mode field must match the Trigger Mode in the IRTE.
> +         * (see vt-d spec 5.1.5.1). */
> +        if (trigger_mode != irq.trigger_mode) {
> +            VTD_DPRINTF(GENERAL, "IOAPIC trigger mode inconsistent: "
> +                        "entry: %u, IRTE: %u, index: %d",
> +                        trigger_mode, irq.trigger_mode, index);
> +        }
> +
>      }
> 
>      /*
> --
> 1.9.1
Paolo Bonzini Sept. 28, 2016, 10:21 a.m. UTC | #6 
On 28/09/2016 12:05, Wu, Feng wrote:
> Hi Paolo,
> 
> 
>> -----Original Message-----
>> From: Wu, Feng
>> Sent: Thursday, September 22, 2016 12:12 AM
>> To: qemu-devel@nongnu.org
>> Cc: pbonzini@redhat.com; Wu, Feng <feng.wu@intel.com>
>> Subject: [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one in
>> IRTE
>>
>> The Trigger Mode field of IOAPIC must match the Trigger Mode in
>> the IRTE according to VT-d Spec 5.1.5.1.
> 
> Any comments about this patch? Thanks a lot!

Hi, I am not the maintainer of the IOMMU emulation code.  CCing mst and
Peter.

Thanks,

Paolo

>> Signed-off-by: Feng Wu <feng.wu@intel.com>
>> ---
>>  hw/i386/intel_iommu.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 28c31a2..f32ad5c 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -27,6 +27,7 @@
>>  #include "hw/pci/pci.h"
>>  #include "hw/pci/pci_bus.h"
>>  #include "hw/i386/pc.h"
>> +#include "hw/i386/apic-msidef.h"
>>  #include "hw/boards.h"
>>  #include "hw/i386/x86-iommu.h"
>>  #include "hw/pci-host/q35.h"
>> @@ -2203,6 +2204,8 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState
>> *iommu,
>>          }
>>      } else {
>>          uint8_t vector = origin->data & 0xff;
>> +        uint8_t trigger_mode = (origin->data >> MSI_DATA_TRIGGER_SHIFT) & 0x1;
>> +
>>          VTD_DPRINTF(IR, "received IOAPIC interrupt");
>>          /* IOAPIC entry vector should be aligned with IRTE vector
>>           * (see vt-d spec 5.1.5.1). */
>> @@ -2211,6 +2214,15 @@ static int
>> vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
>>                          "entry: %d, IRTE: %d, index: %d",
>>                          vector, irq.vector, index);
>>          }
>> +
>> +        /* The Trigger Mode field must match the Trigger Mode in the IRTE.
>> +         * (see vt-d spec 5.1.5.1). */
>> +        if (trigger_mode != irq.trigger_mode) {
>> +            VTD_DPRINTF(GENERAL, "IOAPIC trigger mode inconsistent: "
>> +                        "entry: %u, IRTE: %u, index: %d",
>> +                        trigger_mode, irq.trigger_mode, index);
>> +        }
>> +
>>      }
>>
>>      /*
>> --
>> 1.9.1
>
Peter Xu Sept. 28, 2016, 10:34 a.m. UTC | #7 
On Wed, Sep 28, 2016 at 12:21:37PM +0200, Paolo Bonzini wrote:
> 
> 
> On 28/09/2016 12:05, Wu, Feng wrote:
> > Hi Paolo,
> > 
> > 
> >> -----Original Message-----
> >> From: Wu, Feng
> >> Sent: Thursday, September 22, 2016 12:12 AM
> >> To: qemu-devel@nongnu.org
> >> Cc: pbonzini@redhat.com; Wu, Feng <feng.wu@intel.com>
> >> Subject: [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one in
> >> IRTE
> >>
> >> The Trigger Mode field of IOAPIC must match the Trigger Mode in
> >> the IRTE according to VT-d Spec 5.1.5.1.
> > 
> > Any comments about this patch? Thanks a lot!
> 
> Hi, I am not the maintainer of the IOMMU emulation code.  CCing mst and
> Peter.

I've provided my r-b already. :)

-- peterx
Wu, Feng Sept. 28, 2016, 10:56 a.m. UTC | #8 
> -----Original Message-----

> From: Peter Xu [mailto:peterx@redhat.com]

> Sent: Wednesday, September 28, 2016 6:35 PM

> To: Paolo Bonzini <pbonzini@redhat.com>

> Cc: Wu, Feng <feng.wu@intel.com>; qemu-devel@nongnu.org; Michael S.

> Tsirkin <mst@redhat.com>

> Subject: Re: [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one

> in IRTE

> 

> On Wed, Sep 28, 2016 at 12:21:37PM +0200, Paolo Bonzini wrote:

> >

> >

> > On 28/09/2016 12:05, Wu, Feng wrote:

> > > Hi Paolo,

> > >

> > >

> > >> -----Original Message-----

> > >> From: Wu, Feng

> > >> Sent: Thursday, September 22, 2016 12:12 AM

> > >> To: qemu-devel@nongnu.org

> > >> Cc: pbonzini@redhat.com; Wu, Feng <feng.wu@intel.com>

> > >> Subject: [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the

> one in

> > >> IRTE

> > >>

> > >> The Trigger Mode field of IOAPIC must match the Trigger Mode in

> > >> the IRTE according to VT-d Spec 5.1.5.1.

> > >

> > > Any comments about this patch? Thanks a lot!

> >

> > Hi, I am not the maintainer of the IOMMU emulation code.  CCing mst and

> > Peter.

> 

> I've provided my r-b already. :)


Okay, thanks to you guys!

Thanks,
Feng

> 

> -- peterx
diff mbox

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 28c31a2..f32ad5c 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -27,6 +27,7 @@ 
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/i386/pc.h"
+#include "hw/i386/apic-msidef.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
 #include "hw/pci-host/q35.h"
@@ -2203,6 +2204,8 @@  static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
         }
     } else {
         uint8_t vector = origin->data & 0xff;
+        uint8_t trigger_mode = (origin->data >> MSI_DATA_TRIGGER_SHIFT) & 0x1;
+
         VTD_DPRINTF(IR, "received IOAPIC interrupt");
         /* IOAPIC entry vector should be aligned with IRTE vector
          * (see vt-d spec 5.1.5.1). */
@@ -2211,6 +2214,15 @@  static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
                         "entry: %d, IRTE: %d, index: %d",
                         vector, irq.vector, index);
         }
+
+        /* The Trigger Mode field must match the Trigger Mode in the IRTE.
+         * (see vt-d spec 5.1.5.1). */
+        if (trigger_mode != irq.trigger_mode) {
+            VTD_DPRINTF(GENERAL, "IOAPIC trigger mode inconsistent: "
+                        "entry: %u, IRTE: %u, index: %d",
+                        trigger_mode, irq.trigger_mode, index);
+        }
+
     }
 
     /*