From mboxrd@z Thu Jan 1 00:00:00 1970 From: Weidong Han Subject: Re: [PATCH] VT-d: improve RMRR validity checking Date: Fri, 22 Jan 2010 20:15:11 +0800 Message-ID: <4B5996CF.9020409@intel.com> References: <4B59098B.6000108@intel.com> <4B590FA4.4000008@jp.fujitsu.com> <4B59132B.40607@intel.com> <4B59188C.50901@jp.fujitsu.com> <4B59660F.4000909@intel.com> <1098023846.20100122101901@eikelenboom.it> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1098023846.20100122101901@eikelenboom.it> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Sander Eikelenboom Cc: "xen-devel@lists.xensource.com" , "Kay, Allen M" , "Cihula, Joseph" , Noboru Iwamatsu , "keir.fraser@eu.citrix.com" List-Id: xen-devel@lists.xenproject.org Sander Eikelenboom wrote: > Hello Weidong, > > Wouldn't it be more clear to add an option to iommu= for this case ? > > if iommu=on,..,..,security > > With the security option specified: > -it would be most strict in it's checks, since enforcing security with the iommu requires that as you have pointed out. > -warn,fail or panic incase it can't enable all to enforce the security. > iommu=force is for security. It does as you described above. So I think "security" option is not necessary. > Without the security option specified (default) > - it tries to work as with the security option specified > - but incase of problems makes the assumption the iommu's main task is not security, but making as much of vt-d working to keep the passthrough functionality > - it will only warn, that you will lose the security part, that it would be wise to let your bios be fixed, and not making it panic > - and keep vt-d enabled > > the default iommu=1 works like iommu=force if BIOS is correct. But in fact we encountered some buggy BIOS, and then we added some workarounds to make VT-d still be enabled, or warn and disable VT-d if the issue is regarded as invalid and cannot be workarounded. These workarounds make Xen more defensive to VT-d BIOS issues. The panic only occurs when operating VT-d hardware fails, because it means the hardware is possibly malfunctional. In short, default iommu=1 can workaround known VT-d BIOS issues we observed till now, while iommu=force ensures best security provided by VT-d. Regards, Weidong > Regards, > > Sander > > > > Friday, January 22, 2010, 9:47:11 AM, you wrote: > > >> diff -r 207fba95a7d5 xen/drivers/passthrough/vtd/dmar.c >> --- a/xen/drivers/passthrough/vtd/dmar.c Fri Jan 22 13:12:45 2010 +0800 >> +++ b/xen/drivers/passthrough/vtd/dmar.c Fri Jan 22 22:32:10 2010 +0800 >> @@ -396,8 +396,49 @@ acpi_parse_one_drhd(struct acpi_dmar_ent >> >> if ( ret ) >> xfree(dmaru); >> + else if ( force_iommu || dmaru->include_all ) >> + acpi_register_drhd_unit(dmaru); >> else >> - acpi_register_drhd_unit(dmaru); >> + { >> + u8 b, d, f; >> + int i, invalid_cnt = 0; >> + >> + for ( i = 0; i < dmaru->scope.devices_cnt; i++ ) >> + { >> + b = PCI_BUS(dmaru->scope.devices[i]); >> + d = PCI_SLOT(dmaru->scope.devices[i]); >> + f = PCI_FUNC(dmaru->scope.devices[i]); >> + >> + if ( pci_device_detect(b, d, f) == 0 ) >> + { >> + dprintk(XENLOG_WARNING VTDPREFIX, >> + " Non-existent device (%x:%x.%x) is reported " >> + "in this DRHD's scope!\n", b, d, f); >> + invalid_cnt++; >> + } >> + } >> + >> + if ( invalid_cnt ) >> + { >> + xfree(dmaru); >> + if ( 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"); >> + } >> + else >> + { >> + dprintk(XENLOG_WARNING VTDPREFIX, >> + " The DRHD is invalid due to some devices under " >> + "its scope are not PCI discoverable!\n"); >> + ret = -EINVAL; >> + } >> + } >> + else >> + acpi_register_drhd_unit(dmaru); >> + } >> + >> return ret; >> } >> >> > > > >