Malcolm Crossley
2013-Jan-15 23:27 UTC
[PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
http://www.intel.com/content/www/us/en/chipsets/5520-and-5500-chipset-ioh-specification-update.html Stepping B-3 has two errata (#47 and #53) related to Interrupt remapping, to which the workaround is for the BIOS to completely disable interrupt remapping. These errata are fixed in stepping C-2. Unfortunately this chipset is very common and many BIOSes are not disabling remapping. We can detect this in Xen and prevent turning on remapping in the first place. However, this will turn VT-d off on many systems by default. Users who still wish to use VT-d can use iommu=force if they are happy exposing the associated security risk. Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> diff -r 35a0556a7f76 -r ee475f0e6aeb xen/drivers/passthrough/vtd/quirks.c --- a/xen/drivers/passthrough/vtd/quirks.c +++ b/xen/drivers/passthrough/vtd/quirks.c @@ -244,6 +244,29 @@ void vtd_ops_postamble_quirk(struct iomm } } +/* 5500/5520/X58 Chipset Interrupt remapping errata, for stepping B-3. + * Fixed in stepping C-2. */ +void __init tylersburg_intremap_quirk(void) +{ + uint32_t bus, device; + uint8_t rev; + + for ( bus = 0; bus < 0x100; bus++ ) + { + /* Match on System Management Registers on Device 20 Function 0 */ + device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID); + rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID); + + if ( rev == 0x13 && device == 0x342e8086 ) + { + dprintk(XENLOG_INFO VTDPREFIX, + "Disabling Interrupt Remapping due to Intel 5500/5520/X58 Chipset errata #47, #53\n"); + iommu_intremap = 0; + break; + } + } +} + /* initialize platform identification flags */ void __init platform_quirks_init(void) { @@ -264,6 +287,9 @@ void __init platform_quirks_init(void) /* ioremap IGD MMIO+0x2000 page */ map_igd_reg(); + + /* Tylersburg interrupt remap quirk */ + tylersburg_intremap_quirk(); } /*
Zhang, Xiantao
2013-Jan-16 08:00 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
Thanks! Acked-by: Xiantao Zhang <xiantao.zhang@intel.com> Xiantao> -----Original Message----- > From: xen-devel-bounces@lists.xen.org [mailto:xen-devel- > bounces@lists.xen.org] On Behalf Of Malcolm Crossley > Sent: Wednesday, January 16, 2013 7:27 AM > To: xen-devel@lists.xensource.com > Cc: andrew.cooper3@citrix.com; Zhang, Xiantao > Subject: [Xen-devel] [PATCH] VTD/Intremap: Disable Intremap on Chipset > 5500/5520/X58 due to errata > > http://www.intel.com/content/www/us/en/chipsets/5520-and-5500- > chipset-ioh-specification-update.html > > Stepping B-3 has two errata (#47 and #53) related to Interrupt > remapping, to which the workaround is for the BIOS to completely disable > interrupt remapping. These errata are fixed in stepping C-2. > > Unfortunately this chipset is very common and many BIOSes are not > disabling remapping. We can detect this in Xen and prevent turning on > remapping in the first place. However, this will turn VT-d off on many > systems by default. > > Users who still wish to use VT-d can use iommu=force if they are happy > exposing the associated security risk. > > Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > diff -r 35a0556a7f76 -r ee475f0e6aeb xen/drivers/passthrough/vtd/quirks.c > --- a/xen/drivers/passthrough/vtd/quirks.c > +++ b/xen/drivers/passthrough/vtd/quirks.c > @@ -244,6 +244,29 @@ void vtd_ops_postamble_quirk(struct iomm > } > } > > +/* 5500/5520/X58 Chipset Interrupt remapping errata, for stepping B-3. > + * Fixed in stepping C-2. */ > +void __init tylersburg_intremap_quirk(void) > +{ > + uint32_t bus, device; > + uint8_t rev; > + > + for ( bus = 0; bus < 0x100; bus++ ) > + { > + /* Match on System Management Registers on Device 20 Function 0 */ > + device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID); > + rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID); > + > + if ( rev == 0x13 && device == 0x342e8086 ) > + { > + dprintk(XENLOG_INFO VTDPREFIX, > + "Disabling Interrupt Remapping due to Intel 5500/5520/X58 > Chipset errata #47, #53\n"); > + iommu_intremap = 0; > + break; > + } > + } > +} > + > /* initialize platform identification flags */ > void __init platform_quirks_init(void) > { > @@ -264,6 +287,9 @@ void __init platform_quirks_init(void) > > /* ioremap IGD MMIO+0x2000 page */ > map_igd_reg(); > + > + /* Tylersburg interrupt remap quirk */ > + tylersburg_intremap_quirk(); > } > > /* > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Jan Beulich
2013-Jan-16 13:57 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
>>> On 16.01.13 at 00:27, Malcolm Crossley <malcolm.crossley@citrix.com> wrote: > http://www.intel.com/content/www/us/en/chipsets/5520-and-5500-chipset-ioh-specific > ation-update.html > > Stepping B-3 has two errata (#47 and #53) related to Interrupt > remapping, to which the workaround is for the BIOS to completely disable > interrupt remapping. These errata are fixed in stepping C-2. > > Unfortunately this chipset is very common and many BIOSes are not > disabling remapping. We can detect this in Xen and prevent turning on > remapping in the first place. However, this will turn VT-d off on many > systems by default. > > Users who still wish to use VT-d can use iommu=force if they are happy > exposing the associated security risk. > > Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > diff -r 35a0556a7f76 -r ee475f0e6aeb xen/drivers/passthrough/vtd/quirks.c > --- a/xen/drivers/passthrough/vtd/quirks.c > +++ b/xen/drivers/passthrough/vtd/quirks.c > @@ -244,6 +244,29 @@ void vtd_ops_postamble_quirk(struct iomm > } > } > > +/* 5500/5520/X58 Chipset Interrupt remapping errata, for stepping B-3. > + * Fixed in stepping C-2. */ > +void __init tylersburg_intremap_quirk(void) > +{ > + uint32_t bus, device; > + uint8_t rev; > + > + for ( bus = 0; bus < 0x100; bus++ ) > + { > + /* Match on System Management Registers on Device 20 Function 0 */ > + device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID); > + rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID); > + > + if ( rev == 0x13 && device == 0x342e8086 ) > + { > + dprintk(XENLOG_INFO VTDPREFIX, > + "Disabling Interrupt Remapping due to Intel 5500/5520/X58 Chipset errata #47, #53\n"); > + iommu_intremap = 0;Unless it is guaranteed that no system with this chipset can have x2APIC, I don''t think this is right. For one, this happens way too late (namely after x2apic_bsp_setup()). And second, if you move this earlier, such systems with x2APIC pre-enabled won''t boot anymore. Jan> + break; > + } > + } > +} > + > /* initialize platform identification flags */ > void __init platform_quirks_init(void) > { > @@ -264,6 +287,9 @@ void __init platform_quirks_init(void) > > /* ioremap IGD MMIO+0x2000 page */ > map_igd_reg(); > + > + /* Tylersburg interrupt remap quirk */ > + tylersburg_intremap_quirk(); > } > > /* > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Jan Beulich
2013-Jan-16 14:14 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
>>> On 16.01.13 at 00:27, Malcolm Crossley <malcolm.crossley@citrix.com> wrote: > Unfortunately this chipset is very common and many BIOSes are not > disabling remapping. We can detect this in Xen and prevent turning on > remapping in the first place. However, this will turn VT-d off on many > systems by default.Why would the IOMMU be turned off in that case as a whole? Interrupt remapping is not a prerequisite iirc.> Users who still wish to use VT-d can use iommu=force if they are happy > exposing the associated security risk."iommu=force" doesn''t enable interrupt remapping if it was forcibly turned off. Jan
Malcolm Crossley
2013-Jan-16 14:33 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
On 16/01/13 14:14, Jan Beulich wrote:>>>> On 16.01.13 at 00:27, Malcolm Crossley <malcolm.crossley@citrix.com> wrote: >> Unfortunately this chipset is very common and many BIOSes are not >> disabling remapping. We can detect this in Xen and prevent turning on >> remapping in the first place. However, this will turn VT-d off on many >> systems by default. > Why would the IOMMU be turned off in that case as a whole? > Interrupt remapping is not a prerequisite iirc.You are correct. I read the description of XSA-3 but didn''t the check the current xen-unstable code correctly.>> Users who still wish to use VT-d can use iommu=force if they are happy >> exposing the associated security risk. > "iommu=force" doesn''t enable interrupt remapping if it was > forcibly turned off.This was based upon the faulty code reading above and therefore the comment is wrong . I will change the patch description.> Jan >
Malcolm Crossley
2013-Jan-16 21:09 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
On 16/01/13 13:57, Jan Beulich wrote:>>>> On 16.01.13 at 00:27, Malcolm Crossley <malcolm.crossley@citrix.com> wrote: >> http://www.intel.com/content/www/us/en/chipsets/5520-and-5500-chipset-ioh-specific >> ation-update.html >> >> Stepping B-3 has two errata (#47 and #53) related to Interrupt >> remapping, to which the workaround is for the BIOS to completely disable >> interrupt remapping. These errata are fixed in stepping C-2. >> >> Unfortunately this chipset is very common and many BIOSes are not >> disabling remapping. We can detect this in Xen and prevent turning on >> remapping in the first place. However, this will turn VT-d off on many >> systems by default. >> >> Users who still wish to use VT-d can use iommu=force if they are happy >> exposing the associated security risk. >> >> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> >> diff -r 35a0556a7f76 -r ee475f0e6aeb xen/drivers/passthrough/vtd/quirks.c >> --- a/xen/drivers/passthrough/vtd/quirks.c >> +++ b/xen/drivers/passthrough/vtd/quirks.c >> @@ -244,6 +244,29 @@ void vtd_ops_postamble_quirk(struct iomm >> } >> } >> >> +/* 5500/5520/X58 Chipset Interrupt remapping errata, for stepping B-3. >> + * Fixed in stepping C-2. */ >> +void __init tylersburg_intremap_quirk(void) >> +{ >> + uint32_t bus, device; >> + uint8_t rev; >> + >> + for ( bus = 0; bus < 0x100; bus++ ) >> + { >> + /* Match on System Management Registers on Device 20 Function 0 */ >> + device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID); >> + rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID); >> + >> + if ( rev == 0x13 && device == 0x342e8086 ) >> + { >> + dprintk(XENLOG_INFO VTDPREFIX, >> + "Disabling Interrupt Remapping due to Intel 5500/5520/X58 Chipset errata #47, #53\n"); >> + iommu_intremap = 0; > Unless it is guaranteed that no system with this chipset can have > x2APIC, I don''t think this is right. For one, this happens way too > late (namely after x2apic_bsp_setup()). And second, if you move > this earlier, such systems with x2APIC pre-enabled won''t boot > anymore.After some digging, I discovered that the errata affects chipsets (5520,5500,X58) which don''t have IOMMU EIM( Extended Interrupt Mode) support. EIM is required to support x2APIC mode and so this means the chipset can''t support x2APIC mode and so we should never see a system with x2APIC enabled. For reference, the chipset of this processor generation which does support EIM is the 7500 and it does not suffer from this errata. Currently Xen is relying on the ACPI_DMAR_X2APIC_OPT_OUT bit in the DMAR table to detect x2apic support in the IOMMU. In theory it would be better to read the EIM bit in the IOMMU device itself instead of relying on the BIOS but this may be difficult to do at that early stage of initialisation of Xen. Malcolm> >> + break; >> + } >> + } >> +} >> + >> /* initialize platform identification flags */ >> void __init platform_quirks_init(void) >> { >> @@ -264,6 +287,9 @@ void __init platform_quirks_init(void) >> >> /* ioremap IGD MMIO+0x2000 page */ >> map_igd_reg(); >> + >> + /* Tylersburg interrupt remap quirk */ >> + tylersburg_intremap_quirk(); >> } >> >> /* >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel > >
Jan Beulich
2013-Jan-17 08:49 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
>>> On 16.01.13 at 22:09, Malcolm Crossley <malcolm.crossley@citrix.com> wrote: > On 16/01/13 13:57, Jan Beulich wrote: >>>>> On 16.01.13 at 00:27, Malcolm Crossley <malcolm.crossley@citrix.com> wrote: >>> +/* 5500/5520/X58 Chipset Interrupt remapping errata, for stepping B-3. >>> + * Fixed in stepping C-2. */ >>> +void __init tylersburg_intremap_quirk(void) >>> +{ >>> + uint32_t bus, device; >>> + uint8_t rev; >>> + >>> + for ( bus = 0; bus < 0x100; bus++ ) >>> + { >>> + /* Match on System Management Registers on Device 20 Function 0 */ >>> + device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID); >>> + rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID); >>> + >>> + if ( rev == 0x13 && device == 0x342e8086 ) >>> + { >>> + dprintk(XENLOG_INFO VTDPREFIX, >>> + "Disabling Interrupt Remapping due to Intel 5500/5520/X58 Chipset errata #47, #53\n"); >>> + iommu_intremap = 0; >> Unless it is guaranteed that no system with this chipset can have >> x2APIC, I don''t think this is right. For one, this happens way too >> late (namely after x2apic_bsp_setup()). And second, if you move >> this earlier, such systems with x2APIC pre-enabled won''t boot >> anymore. > After some digging, I discovered that the errata affects chipsets > (5520,5500,X58) which don''t have IOMMU EIM( Extended Interrupt Mode) > support. EIM is required to support x2APIC mode and so this means the > chipset can''t support x2APIC mode and so we should never see a system > with x2APIC enabled.Xiantao, while I realize that you already ack-ed that patch, I''d like this to be confirmed, as well as the fact that there indeed is on better workaround known than disabling interrupt remapping. Malcolm, I still don''t feel well with this making a small security hole bigger (the erratum to me mainly means that with lost interrupts we might observe systems hangs in the worst case, or maybe data corruption if recovery code in the OSes doesn''t work well). Whereas with interrupt remapping disabled passthrough becomes inherently insecure (and the host vulnerable to guest attacks). So the question really is whether, rather than disabling interrupt remapping, disabling the IOMMU as a whole wouldn''t be the better (read: more secure) workaround. Jan
Ian Campbell
2013-Jan-17 09:01 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
On Thu, 2013-01-17 at 08:49 +0000, Jan Beulich wrote:> So the question really is whether, rather than disabling interrupt > remapping, disabling the IOMMU as a whole wouldn''t be the > better (read: more secure) workaround.That, plus providing a command line "I accept the risks" option, is the approach we''ve taken in the past I think. Ian.
Jan Beulich
2013-Jan-17 09:09 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
>>> On 17.01.13 at 10:01, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Thu, 2013-01-17 at 08:49 +0000, Jan Beulich wrote: >> So the question really is whether, rather than disabling interrupt >> remapping, disabling the IOMMU as a whole wouldn''t be the >> better (read: more secure) workaround. > > That, plus providing a command line "I accept the risks" option, is the > approach we''ve taken in the past I think.An option for this we already have - "iommu=force". Jan
Ian Campbell
2013-Jan-17 09:15 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
On Thu, 2013-01-17 at 09:09 +0000, Jan Beulich wrote:> >>> On 17.01.13 at 10:01, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Thu, 2013-01-17 at 08:49 +0000, Jan Beulich wrote: > >> So the question really is whether, rather than disabling interrupt > >> remapping, disabling the IOMMU as a whole wouldn''t be the > >> better (read: more secure) workaround. > > > > That, plus providing a command line "I accept the risks" option, is the > > approach we''ve taken in the past I think. > > An option for this we already have - "iommu=force".I thought that but isn''t it sort of the other way found? iommu=force will fail to boot if the iommu cannot be enabled, including intremap support. But perhaps iommu=no-intremap does what we want already? Ian.
Jan Beulich
2013-Jan-17 09:33 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
>>> On 17.01.13 at 10:15, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Thu, 2013-01-17 at 09:09 +0000, Jan Beulich wrote: >> >>> On 17.01.13 at 10:01, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> > On Thu, 2013-01-17 at 08:49 +0000, Jan Beulich wrote: >> >> So the question really is whether, rather than disabling interrupt >> >> remapping, disabling the IOMMU as a whole wouldn''t be the >> >> better (read: more secure) workaround. >> > >> > That, plus providing a command line "I accept the risks" option, is the >> > approach we''ve taken in the past I think. >> >> An option for this we already have - "iommu=force". > > I thought that but isn''t it sort of the other way found? iommu=force > will fail to boot if the iommu cannot be enabled, including intremap > support.Of course using "iommu=force" when enabling the IOMMU fails is a bad idea - that will indeed panic. But we''re talking about the situation where enabling the IOMMU so far worked fine, and the option is only to be used as an override to us disabling it as workaround.> But perhaps iommu=no-intremap does what we want already?Not really - we want to do the disabling automatically (turning off both iommu and interrupt remapping), so that to override this one would need "iommu=force" (enabling the IOMMU, but not interrupt remapping) or "iommu=force,intremap" (enabling both - that case needs some code adjustment to work as described). Jan
Ian Campbell
2013-Jan-17 09:51 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
On Thu, 2013-01-17 at 09:33 +0000, Jan Beulich wrote:> >>> On 17.01.13 at 10:15, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Thu, 2013-01-17 at 09:09 +0000, Jan Beulich wrote: > >> >>> On 17.01.13 at 10:01, Ian Campbell <Ian.Campbell@citrix.com> wrote: > >> > On Thu, 2013-01-17 at 08:49 +0000, Jan Beulich wrote: > >> >> So the question really is whether, rather than disabling interrupt > >> >> remapping, disabling the IOMMU as a whole wouldn''t be the > >> >> better (read: more secure) workaround. > >> > > >> > That, plus providing a command line "I accept the risks" option, is the > >> > approach we''ve taken in the past I think. > >> > >> An option for this we already have - "iommu=force". > > > > I thought that but isn''t it sort of the other way found? iommu=force > > will fail to boot if the iommu cannot be enabled, including intremap > > support. > > Of course using "iommu=force" when enabling the IOMMU fails > is a bad idea - that will indeed panic. But we''re talking about the > situation where enabling the IOMMU so far worked fine, and the > option is only to be used as an override to us disabling it as > workaround.I wasn''t aware that =force had dual meaning like that. It seems dangerous to me, if I build a (security) product which wants to ensure that an iommu is always available and therefore add "iommu=force" to the command line this will have the side effect of forcibly enabling the iommu in an unsafe mode on certain systems. I hope I''ve just misunderstood how this works!> > But perhaps iommu=no-intremap does what we want already? > > Not really - we want to do the disabling automatically (turning > off both iommu and interrupt remapping), so that to override > this one would need "iommu=force" (enabling the IOMMU, but > not interrupt remapping) or "iommu=force,intremap" (enabling > both - that case needs some code adjustment to work as > described).I agree with the premise here, even if I''m confused by the exact names of the options needed to override. Ian.
Jan Beulich
2013-Jan-17 10:01 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
>>> On 17.01.13 at 10:51, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Thu, 2013-01-17 at 09:33 +0000, Jan Beulich wrote: >> >>> On 17.01.13 at 10:15, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> > On Thu, 2013-01-17 at 09:09 +0000, Jan Beulich wrote: >> >> >>> On 17.01.13 at 10:01, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> >> > On Thu, 2013-01-17 at 08:49 +0000, Jan Beulich wrote: >> >> >> So the question really is whether, rather than disabling interrupt >> >> >> remapping, disabling the IOMMU as a whole wouldn''t be the >> >> >> better (read: more secure) workaround. >> >> > >> >> > That, plus providing a command line "I accept the risks" option, is the >> >> > approach we''ve taken in the past I think. >> >> >> >> An option for this we already have - "iommu=force". >> > >> > I thought that but isn''t it sort of the other way found? iommu=force >> > will fail to boot if the iommu cannot be enabled, including intremap >> > support. >> >> Of course using "iommu=force" when enabling the IOMMU fails >> is a bad idea - that will indeed panic. But we''re talking about the >> situation where enabling the IOMMU so far worked fine, and the >> option is only to be used as an override to us disabling it as >> workaround. > > I wasn''t aware that =force had dual meaning like that.It''s not that way currently, just how I envisioned Malcolm''s patch to be changed (don''t set iommu [and iommu_intremap] to 0 when iommu_force).> It seems > dangerous to me, if I build a (security) product which wants to ensure > that an iommu is always available and therefore add "iommu=force" to the > command line this will have the side effect of forcibly enabling the > iommu in an unsafe mode on certain systems.In that case we need a new sub-option instead. Malcolm - are you going to follow up on this? Jan
Malcolm Crossley
2013-Jan-17 10:02 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
On 17/01/13 08:49, Jan Beulich wrote:>>>> On 16.01.13 at 22:09, Malcolm Crossley <malcolm.crossley@citrix.com> wrote: >> On 16/01/13 13:57, Jan Beulich wrote: >>>>>> On 16.01.13 at 00:27, Malcolm Crossley <malcolm.crossley@citrix.com> wrote: >>>> +/* 5500/5520/X58 Chipset Interrupt remapping errata, for stepping B-3. >>>> + * Fixed in stepping C-2. */ >>>> +void __init tylersburg_intremap_quirk(void) >>>> +{ >>>> + uint32_t bus, device; >>>> + uint8_t rev; >>>> + >>>> + for ( bus = 0; bus < 0x100; bus++ ) >>>> + { >>>> + /* Match on System Management Registers on Device 20 Function 0 */ >>>> + device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID); >>>> + rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID); >>>> + >>>> + if ( rev == 0x13 && device == 0x342e8086 ) >>>> + { >>>> + dprintk(XENLOG_INFO VTDPREFIX, >>>> + "Disabling Interrupt Remapping due to Intel 5500/5520/X58 Chipset errata #47, #53\n"); >>>> + iommu_intremap = 0; >>> Unless it is guaranteed that no system with this chipset can have >>> x2APIC, I don''t think this is right. For one, this happens way too >>> late (namely after x2apic_bsp_setup()). And second, if you move >>> this earlier, such systems with x2APIC pre-enabled won''t boot >>> anymore. >> After some digging, I discovered that the errata affects chipsets >> (5520,5500,X58) which don''t have IOMMU EIM( Extended Interrupt Mode) >> support. EIM is required to support x2APIC mode and so this means the >> chipset can''t support x2APIC mode and so we should never see a system >> with x2APIC enabled. > Malcolm, > > I still don''t feel well with this making a small security hole bigger > (the erratum to me mainly means that with lost interrupts we > might observe systems hangs in the worst case, or maybe > data corruption if recovery code in the OSes doesn''t work well). > Whereas with interrupt remapping disabled passthrough > becomes inherently insecure (and the host vulnerable to guest > attacks). > > So the question really is whether, rather than disabling interrupt > remapping, disabling the IOMMU as a whole wouldn''t be the > better (read: more secure) workaround.The patch is just achieving the same effect in Xen as the BIOS workaround recommended in the Intel errata document. I also don''t like the making the security hole bigger but I think we are making an Xen IOMMU support policy change if we disable the IOMMU support when interrupt remapping is not supported. Additionally, I believe that if we make this policy change then we will disable Xen IOMMU support for the Intel 3100/3200 chipset series.>
Jan Beulich
2013-Jan-17 10:41 UTC
Re: [PATCH] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata
>>> On 17.01.13 at 11:02, Malcolm Crossley <malcolm.crossley@citrix.com> wrote: > On 17/01/13 08:49, Jan Beulich wrote: >> I still don''t feel well with this making a small security hole bigger >> (the erratum to me mainly means that with lost interrupts we >> might observe systems hangs in the worst case, or maybe >> data corruption if recovery code in the OSes doesn''t work well). >> Whereas with interrupt remapping disabled passthrough >> becomes inherently insecure (and the host vulnerable to guest >> attacks). >> >> So the question really is whether, rather than disabling interrupt >> remapping, disabling the IOMMU as a whole wouldn''t be the >> better (read: more secure) workaround. > > The patch is just achieving the same effect in Xen as the BIOS > workaround recommended in the Intel errata document.But there''s a difference nevertheless. If firmware disables something, it''s identical to hardware not having the capability. Whereas if software disables functionality and opens/widens a security hole by doing so, that''s a flaw that would subsequently require an advisory+fix on its own.> I also don''t like the making the security hole bigger but I think we are > making an Xen IOMMU support policy change if we disable the IOMMU > support when interrupt remapping is not supported.Why do you think so? The behavior we currently have causes the system to not boot if "iommu=force", including the case where interrupt remapping didn''t get enabled but was intended to be (i.e. no "iommu=no-intremap"). And without "iommu=force" we make it impossible to assign devices to HVM guests (i.e. preventing security issues). Between functionality and security, I think security has to win by default (but an override ought to be possible).> Additionally, I believe that if we make this policy change then we will > disable Xen IOMMU support for the Intel 3100/3200 chipset series.Because of what? Jan