On Sun, 2011-05-22 at 19:15 +0100, Tim Deegan wrote:
> At 18:19 +0100 on 20 May (1305915548), Ian Jackson wrote:
> > Tim Deegan writes ("Re: [Xen-devel] Xen security advisory CVE-2011-1898 -
> > VT-d (PCI passthrough) MSI"):
> > > At 21:48 +0100 on 19 May (1305841716), Cihula, Joseph wrote:
> > > > So how would the user (or installation SW) specify to use the best
> > > > (IOMMU) security available on the platform?
> > >
> > > iommu=on. That pretty much lines up with the current meaining.
> > >
> > > Only iommu=force requires a fully secure IOMMU, and you can
> > > overide that with iommu=force,nointremap.
> >
> > I think this is the best behaviour. Do we have a patch that
> > implements it ? If I'm not confused, the patch further upthread
> > crashes on lack of intremap even with iommu=on.
>
> AIUI Ian Campbell's most recent patch does exactly this. Ian?
You mean the first one in
<1305708848.20907.109.camel@xxxxxxxxxxxxxxxxxxxxxx>? I believe it has
this effect, yes, although the iommu setup code has several places which
fallback to intremap = 0 if some condition is not met e.g. "!
ecap_intr_remap(iommu->ecap)" or if "Queue Invalidation" is not
enabled/supported, I expect they need a similar check. Rather than
sprinkling panic()s everywhere lets just pull the check for force && !
intremap out into the top level generic function.
There also seemed to be a missing "iommu_intremap = 0" in the case where
enable_intremap failed in init_vtd_hw.
Ian.
8<----------------------------------------------------
# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1306141240 -3600
# Node ID aafc09d804172e3169b64e0ae8f4fc56a1a70fbb
# Parent 3db330334e3512a5326bbed4881718a63eec171a
IOMMU: Fail if intremap is not available and iommu=required/force.
Rather than sprinkling panic()s throughout the setup code hoist the check up
into common code.
Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
diff -r 3db330334e35 -r aafc09d80417 xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c Fri May 13 14:41:18 2011 +0100
+++ b/xen/drivers/passthrough/iommu.c Mon May 23 10:00:40 2011 +0100
@@ -311,6 +311,7 @@ int deassign_device(struct domain *d, u8
int __init iommu_setup(void)
{
int rc = -ENODEV;
+ int need_intremap = iommu_intremap;
if ( iommu_dom0_strict )
iommu_passthrough = 0;
@@ -322,7 +323,9 @@ int __init iommu_setup(void)
}
if ( force_iommu && !iommu_enabled )
- panic("IOMMU setup failed, crash Xen for security purpose!\n");
+ panic("Unable to enable IOMMU and iommu=required/force\n");
+ if ( force_iommu && !iommu_intremap && need_intremap )
+ panic("Unable to enable Interrupt Remapping and
iommu=required/force\n");
if ( !iommu_enabled )
{
diff -r 3db330334e35 -r aafc09d80417 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c Fri May 13 14:41:18 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c Mon May 23 10:00:40 2011 +0100
@@ -1971,8 +1971,6 @@ static int init_vtd_hw(void)
"ioapic_to_iommu: ioapic 0x%x (id: 0x%x) is NULL! "
"Will not try to enable Interrupt Remapping.\n",
apic, IO_APIC_ID(apic));
- if ( force_iommu )
- panic("intremap remapping failed to enable with
iommu=required/force in grub\n");
break;
}
}
@@ -1984,11 +1982,10 @@ static int init_vtd_hw(void)
iommu = drhd->iommu;
if ( enable_intremap(iommu, 0) != 0 )
{
+ iommu_intremap = 0;
dprintk(XENLOG_WARNING VTDPREFIX,
"Interrupt Remapping not enabled\n");
- if ( force_iommu && platform_supports_intremap() )
- panic("intremap remapping failed to enable with
iommu=required/force in grub\n");
break;
}
}
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home