I implemented a patch and attached.
patch description:
In order to make Xen more defensive to VT-d related BIOS issue, this
patch ignores a DRHD if all devices under its scope are not pci
discoverable, and regards a DRHD as invalid and then disable whole VT-d
if some devices under its scope are not pci discoverable. But if
iommu=force is set, it will enable all DRHDs reported by BIOS, to avoid
any security vulnerability with malicious s/s re-enabling "supposed
disabled" devices. Pls note that we don't know the devices under the
"Include_all" DRHD are existent or not, because the scope of
"Include_all" DRHD won't enumerate common pci device, it only
enumerates I/OxAPIC and HPET devices.
Signed-off-by: Noboru Iwamatsu <n_iwamatsu@xxxxxxxxxxxxxx>
Signed-off-by: Weidong Han <weidong.han@xxxxxxxxx>
Noboru, pls test the patch on your machine?
Joe, could you review the patch? and pls ACK it if it's fine for you.
Regards,
Weidong
Noboru Iwamatsu wrote:
Thanks,
I understood.
Noboru Iwamatsu wrote:
Hi Weidong,
I'm not sure why the security problem is caused by ignoring
the DRHD that has only non-existent devices.
Could you explain details or where to read the spec?
It's requested from security experts. The device that is not pci
discoverable may be re-enabled by malicious software. If its DRHD is not
enabled, the re-enabled device is not protected by VT-d. It will cause
security issue.
As you saying, security is the top-priority.
However, when iommu=force is specified, we should enable vt-d
if there are some potential issues.
Because users want to "force" anyway.
iommu=force was introduced to enable VT-d anyway for security purpose. I
plan to still enable those DRHDs that includes non-existed device when
iommu=force, otherwise ignore them.
Regards,
Weidong
Regards,
Noboru.
Keir Fraser wrote:
If we want to keep iommu=1 as default, then it is unacceptable to
fail to
boot on a fairly wide range of modern systems. We have to
warn-and-disable,
partially or completely, unless iommu=force is specified. Or we need to
revert to iommu=0 as the default.
What do you think, Weidong?
Yes. I agree to warn-and-disable for these BIOS issues, and consider
security more when iommu=force. Therefore I will implement a patch based
on Nororu's patch.
Regards,
Weidong
-- Keir
On 21/01/2010 14:17, "Sander Eikelenboom" <linux@xxxxxxxxxxxxxx> wrote:
Hello Weidong,
The problem is most vendor's just don't fix it and ignore the problem
completely.
Most often hiding them selves behind: come back when it's a problem
with
Microsoft Windows, that the only single thing we support (and no other
software, so no vmware, no xen, no linux, perhaps even no hypervisor)
Well I don't know if the virtual pc in windows 7 supports an iommu
now, but it
didn't in the past as far as i know, so any complain bounces off, and
there it
all seems to end for them.
Besides that i don't know if they do know what the problems with there
implementation in BIOS is when someone reports it.
I think some behind the scenes pressure from Intel to vendors might
help to
solve some of them.
(my Q35 chipset, "Intel V-PRO" marketed motherboard (so much for
that) also
suffers RMRR problem when another graphics card is inserted which
switches off
the IGD).
Although i think in my case your patch will work around that for me.
Perhaps a
third option is needed, which does all the workarounds possible and
warns
about potential security problem when requested ?
--
Sander
Thursday, January 21, 2010, 1:46:39 PM, you wrote:
Noboru Iwamatsu wrote:
Hi Weidong,
I re-send the DRHD-fix patch.
If DRHD does not have existent devices, ignore it.
If DRHD has both existent and non-existent devices, consider it
invalid
and not register.
Although you patch workarounds your buggy BIOS, but we still need to
enable it for security purpose as I mentioned in previous mail. We
needn't workaround / fix all BIOS issues in software. I think
security
is more important for this specific BIOS issue. Did you report the
BIOS
issue to your OEM vendor? maybe it's better to get it fixed in BIOS.
Regards,
Weidong
According to this patch and yours, my machine successfully booted
with vt-d enabled.
Signed-off-by: Noboru Iwamatsu <n_iwamatsu@xxxxxxxxxxxxxx>
Keir Fraser wrote:
On 21/01/2010 10:19, "Weidong Han" <weidong.han@xxxxxxxxx> wrote:
Sorry this is typo.
I mean:
So, I think RMRR that has no-existent device is "invalid"
and whole RMRR should be ignored.
looks reasonable.
Keir, I Acks Noboru's rmrr patch. Or do you want us to merge
them to one
patch?
Merge them up, re-send with both sign-off and acked-by all in one
email.
Thanks,
Keir
Sorry, I disagree with Noboru after thinking it again. If the RMRR
has
both no-existent device and also has existent devices in its
scope, we
should not ignore it because the existent devices under its scope
will
be impacted without the RMRR. so I suggest to print a warning
instead of
ignore it. Attached a patch for it.
Signed-off-by: Weidong Han <weidong.han@xxxxxxxxx>
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;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|