Re: [Xen-devel] [PATCH] VT-d: improve RMRR validity checking

[Weidong Han <weidong.han@xxxxxxxxx>]

I implemented a patch for it. Noboru, pls have a try on your machine.
If you use default iommu=1, VT-d will be disabled with warning messages.
If you use iommu=workaround_bios_bug, it should enable VT-d and works 
for you.
If you use iommu=force, it panics.

patch title: VT-d: add "iommu=workaround_bios_bug" option
patch description:
   Add this option to workaround BIOS bugs. Currently it ignores DRHD 
if "all" devices under its scope are not pci discoverable. This 
workarounds a BIOS bug in some platforms to make VT-d work. But note 
that this option doesn't guarantee security, because it might ignore DRHD.
   So there are 3 options which handle BIOS bugs differently:
   iommu=1 (default): If detect non-existent device under a DRHD's 
scope, or find incorrect RMRR setting (base_address > end_address), 
disable VT-d completely in Xen with warning messages. This guarantees 
security when VT-d enabled, or just disable VT-d to let Xen work without 
VT-d.
   iommu=force: it enforces to enable VT-d in Xen. If VT-d cannot be 
enabled, it will crashes Xen. This is mainly for users who must need VT-d.
   iommu=workaround_bogus_bios: it workarounds some BIOS bugs to make 
VT-d still work.  This might be insecure because there might be a device 
not protected by any DRHD if the device is re-enabled by malicious s/w. 
This is for users who want to use VT-d regardless of security.
Signed-off-by: Weidong Han <weidong.han@xxxxxxxxx>

Regards,
Weidong

Noboru Iwamatsu wrote:
> Weidong, Keir,
>
> I agree your suggestions.
>
> Noboru.
>
>   
> > Keir Fraser wrote:
> >     
> > > On 25/01/2010 10:45, "Sander Eikelenboom" <linux@xxxxxxxxxxxxxx> wrote:
> > >
> > >       
> > > > a) Could be discussed if panic should be default instead of disabling
> > > > iommu or
> > > > not, although there seem to be a lot of broken bioses, so that would
> > > > lead to a
> > > > lot of machines not booting.
> > > >         
> > > Absolutely not acceptable. Warn and completely disable IOMMU is the
> > > correct
> > > default causing least pain to the most end users.
> > >
> > > -- Keir
> > >
> > >       
> > Agree. It should not crash Xen by default due to BIOS issues.
> > warn-and-disable is better. It won't impact common Xen users, and if a
> > user really wants to use VT-d, he can try iommu=workaround_bogus_bios,
> > or directly report to OEM vendor to get it fixed in BIOS. As VT-d is
> > used more and more widely, I think the BIOS issues will be found and
> > fixed more quickly than before, thus the situation should be better.
> >
> > Regards,
> > Weidong
> >
> >
> >
> >     
>
>   

diff -r 5dabbf2826c5 xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c   Mon Jan 25 09:58:53 2010 +0800
+++ b/xen/drivers/passthrough/iommu.c   Tue Jan 26 21:17:46 2010 +0800
@@ -30,6 +30,8 @@ static int iommu_populate_page_table(str
  *   pv                         Enable IOMMU for PV domains
  *   no-pv                      Disable IOMMU for PV domains (default)
  *   force|required             Don't boot unless IOMMU is enabled
+ *   workaround_bios_bug        Workaround some bios issue to still enable
+                                VT-d, don't guarantee security
  *   passthrough                Enable VT-d DMA passthrough (no DMA
  *                              translation for Dom0)
  *   no-snoop                   Disable VT-d Snoop Control
@@ -40,6 +42,7 @@ int iommu_enabled = 1;
 int iommu_enabled = 1;
 int iommu_pv_enabled;
 int force_iommu;
+int iommu_workaround_bios_bug;
 int iommu_passthrough;
 int iommu_snoop = 1;
 int iommu_qinval = 1;
@@ -65,6 +68,8 @@ static void __init parse_iommu_param(cha
             iommu_pv_enabled = 0;
         else if ( !strcmp(s, "force") || !strcmp(s, "required") )
             force_iommu = 1;
+        else if ( !strcmp(s, "workaround_bios_bug") )
+            iommu_workaround_bios_bug = 1;
         else if ( !strcmp(s, "passthrough") )
             iommu_passthrough = 1;
         else if ( !strcmp(s, "no-snoop") )
diff -r 5dabbf2826c5 xen/drivers/passthrough/vtd/dmar.c
--- a/xen/drivers/passthrough/vtd/dmar.c        Mon Jan 25 09:58:53 2010 +0800
+++ b/xen/drivers/passthrough/vtd/dmar.c        Tue Jan 26 21:16:49 2010 +0800
@@ -421,17 +421,21 @@ acpi_parse_one_drhd(struct acpi_dmar_ent
         if ( invalid_cnt )
         {
             xfree(dmaru);
-            if ( invalid_cnt == dmaru->scope.devices_cnt )
+
+            if ( iommu_workaround_bios_bug &&
+                 invalid_cnt == dmaru->scope.devices_cnt )
             {
                 dprintk(XENLOG_WARNING VTDPREFIX,
-                    "  Ignore the DRHD due to all devices under "
-                    "its scope are not PCI discoverable!\n");
+                    "  Workaround BIOS bug: ignore the DRHD due to all "
+                    "devices under its scope are not PCI discoverable!\n");
             }
             else
             {
                 dprintk(XENLOG_WARNING VTDPREFIX,
-                    "  The DRHD is invalid due to some devices under "
-                    "its scope are not PCI discoverable!\n");
+                    "  The DRHD is invalid due to there are devices under "
+                    "its scope are not PCI discoverable! Pls try option "
+                    "iommu=force or iommu=workaround_bios_bug if you "
+                    "really want VT-d\n");
                 ret = -EINVAL;
             }
         }
diff -r 5dabbf2826c5 xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h   Mon Jan 25 09:58:53 2010 +0800
+++ b/xen/include/xen/iommu.h   Tue Jan 26 21:17:08 2010 +0800
@@ -29,6 +29,7 @@ extern int iommu_enabled;
 extern int iommu_enabled;
 extern int iommu_pv_enabled;
 extern int force_iommu;
+extern int iommu_workaround_bios_bug;
 extern int iommu_passthrough;
 extern int iommu_snoop;
 extern int iommu_qinval;

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