[PATCH][VTD] force boot to fail if interrupt remapping cannot be enabled when iommu=force

[PATCH][VTD] force boot to fail if interrupt remapping cannot be enabled when iommu=force

[Kay, Allen M]

[-- Attachment #1: Type: text/plain, Size: 374 bytes --]

Force Xen boot to fail if interrupt remapping fails to enable and the following are true: iommu=force is set as xen boot parameter, VT-d engine HW is interrupt remapping capable, DMAR_INTR_REMAP bit is set in DMAR flags.  This forces iommu=force boot instances has interrupt remapping enabled if HW and BIOS supports it.

Signed-off-by: Allen Kay <allen.m.kay@intel.com>

[-- Attachment #2: intremap0427.patch --]
[-- Type: application/octet-stream, Size: 1177 bytes --]

diff -r 2f08c89b767d xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Wed Apr 20 17:13:08 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Wed Apr 27 22:28:19 2011 -0700
@@ -1971,9 +1971,23 @@ static int init_vtd_hw(void)
             iommu = drhd->iommu;
             if ( enable_intremap(iommu, 0) != 0 )
             {
-                dprintk(XENLOG_WARNING VTDPREFIX,
-                        "Interrupt Remapping not enabled\n");
-                break;
+                int call_panic = 1;
+
+                if ( !force_iommu )
+                    call_panic = 0;
+                if ( !ecap_intr_remap(iommu->ecap) )
+                    call_panic = 0;
+                if ( !platform_supports_intremap() )
+                    call_panic = 0;
+
+                if ( !call_panic )
+                {
+                    dprintk(XENLOG_WARNING VTDPREFIX,
+                            "Interrupt Remapping not enabled\n");
+                    break;
+                }
+                else
+                    panic("enable_intremap() failed with iommu=force and platform is intremap capable\n");
             }
         }
     }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][VTD] force boot to fail if interrupt remapping cannot be enabled when iommu=force

[Keir Fraser]

On 28/04/2011 00:42, "Kay, Allen M" <allen.m.kay@intel.com> wrote:

> Force Xen boot to fail if interrupt remapping fails to enable and the
> following are true: iommu=force is set as xen boot parameter, VT-d engine HW
> is interrupt remapping capable, DMAR_INTR_REMAP bit is set in DMAR flags.
> This forces iommu=force boot instances has interrupt remapping enabled if HW
> and BIOS supports it.

If HW and BIOS support it, why would it fail to be enabled? This doesn't
look like a particularly useful panic() path. If interrupt remapping is so
important, perhaps iommu=force should unconditionally require it, and panic
in its absence regardless of platform features? As it is, this looks like a
panic that is never realistically going to trigger.

 -- Keir

> Signed-off-by: Allen Kay <allen.m.kay@intel.com>

Re: Re: [PATCH][VTD] force boot to fail if interrupt remapping cannot be enabled when iommu=force

[Joanna Rutkowska]

[-- Attachment #1.1: Type: text/plain, Size: 1122 bytes --]

On 04/28/11 19:05, Keir Fraser wrote:
> On 28/04/2011 00:42, "Kay, Allen M" <allen.m.kay@intel.com> wrote:
> 
>> Force Xen boot to fail if interrupt remapping fails to enable and the
>> following are true: iommu=force is set as xen boot parameter, VT-d engine HW
>> is interrupt remapping capable, DMAR_INTR_REMAP bit is set in DMAR flags.
>> This forces iommu=force boot instances has interrupt remapping enabled if HW
>> and BIOS supports it.
> 
> If HW and BIOS support it, why would it fail to be enabled? This doesn't
> look like a particularly useful panic() path. If interrupt remapping is so
> important, perhaps iommu=force should unconditionally require it, and panic
> in its absence regardless of platform features?

Indeed, seems to me like a desired behavior.

j.

> As it is, this looks like a
> panic that is never realistically going to trigger.
> 
>  -- Keir
> 
>> Signed-off-by: Allen Kay <allen.m.kay@intel.com>
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 518 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [PATCH][VTD] force boot to fail if interrupt remapping cannot be enabled when iommu=force

[Cihula, Joseph]

> From: Keir Fraser [mailto:keir.xen@gmail.com]
> Sent: Thursday, April 28, 2011 10:05 AM
> 
> On 28/04/2011 00:42, "Kay, Allen M" <allen.m.kay@intel.com> wrote:
> 
> > Force Xen boot to fail if interrupt remapping fails to enable and the
> > following are true: iommu=force is set as xen boot parameter, VT-d
> > engine HW is interrupt remapping capable, DMAR_INTR_REMAP bit is set in DMAR flags.
> > This forces iommu=force boot instances has interrupt remapping enabled
> > if HW and BIOS supports it.
> 
> If HW and BIOS support it, why would it fail to be enabled? This doesn't look like a particularly
> useful panic() path. If interrupt remapping is so important, perhaps iommu=force should
> unconditionally require it, and panic in its absence regardless of platform features? As it is,
> this looks like a panic that is never realistically going to trigger.

There are ways that malicious SW, running before the TXT launch of Xen, can alter the DMAR ACPI tables such that Xen will fail to enable interrupt remapping (IR).

Since not all platforms support IR, we certainly don't want to fail Xen on those platforms.  We simply want to ensure that if the HW does support IR that Xen can't be "tricked" into not using it.

Joe