Ronny Hegewald
2012-Oct-21 22:22 UTC
[PATCH V2] keep iommu disabled until iommu_setup() is called
The iommu is enabled by default when xen is booting and later disabled in
iommu_setup() when no iommu is present.
But under some circumstances iommu-code can be called before iommu_setup() is
processed. If there is no iommu available xen crashes.
This can happen for example when panic(...) is called that got introduced with
patch "x86-64: detect processors subject to AMD erratum #121 and refuse to
boot." since xen 4.1.3 and results in the following stacktrace:
find_iommu_for_device
amd_iommu_ioapic_update_ire
timer_interrupt
enable_8259_A_irq
do_IRQ
printk_start_of_line
acpi_os_printf
io_apic_write
__ioapic_write_entry
ioapic_write_entry
__clear_IO_APIC_pin
clear_IO_APIC
disable_IO_APIC
__stop_this_cpu
smp_send_stop
machine_restart
panic
tasklet_schedule_on_cpu
display_cacheinfo
init_amd
generic_identify
identify_cpu
_start_xen
_high_start
This patch fixes this by keeping the iommu disabled until iommu_setup() is
entered.
Signed-off-by: Ronny Hegewald@online.de
---
Changed since V1:
* simplify code as suggested by Jan Beulich
--- xen/drivers/passthrough/iommu.c.org 2012-10-05 03:38:33.000000000 +0000
+++ xen/drivers/passthrough/iommu.c 2012-10-21 21:07:37.000000000 +0000
@@ -38,7 +38,8 @@
* no-intremap Disable VT-d Interrupt Remapping
*/
custom_param("iommu", parse_iommu_param);
-bool_t __read_mostly iommu_enabled = 1;
+bool_t iommu_enable_default __initdata = 1;
+bool_t __read_mostly iommu_enabled;
bool_t __read_mostly force_iommu;
bool_t __initdata iommu_dom0_strict;
bool_t __read_mostly iommu_verbose;
@@ -61,7 +62,7 @@
*ss = ''\0'';
if ( !parse_bool(s) )
- iommu_enabled = 0;
+ iommu_enable_default = 0;
else if ( !strcmp(s, "force") || !strcmp(s,
"required") )
force_iommu = 1;
else if ( !strcmp(s, "workaround_bios_bug") )
@@ -316,7 +317,7 @@
if ( iommu_dom0_strict )
iommu_passthrough = 0;
- if ( iommu_enabled )
+ if ( iommu_enable_default )
{
rc = iommu_hardware_setup();
iommu_enabled = (rc == 0);
Jan Beulich
2012-Oct-24 12:51 UTC
Re: [PATCH V2] keep iommu disabled until iommu_setup() is called
>>> On 22.10.12 at 00:22, Ronny Hegewald <ronny.hegewald@online.de> wrote: > The iommu is enabled by default when xen is booting and later disabled in > iommu_setup() when no iommu is present. > > But under some circumstances iommu-code can be called before iommu_setup() is > > processed. If there is no iommu available xen crashes. > > This can happen for example when panic(...) is called that got introduced > with > patch "x86-64: detect processors subject to AMD erratum #121 and refuse to > boot." since xen 4.1.3 and results in the following stacktrace: > > find_iommu_for_device > amd_iommu_ioapic_update_ire > timer_interrupt > enable_8259_A_irq > do_IRQ > printk_start_of_line > acpi_os_printf > io_apic_write > __ioapic_write_entry > ioapic_write_entry > __clear_IO_APIC_pin > clear_IO_APIC > disable_IO_APIC > __stop_this_cpu > smp_send_stop > machine_restart > panic > tasklet_schedule_on_cpu > display_cacheinfo > init_amd > generic_identify > identify_cpu > _start_xen > _high_start > > > This patch fixes this by keeping the iommu disabled until iommu_setup() is > entered. > > Signed-off-by: Ronny Hegewald@online.de > > --- > Changed since V1: > * simplify code as suggested by Jan BeulichI''m completing that adjustment as originally requested. Plus, in case you happen to send patches again in the future, please prepare them against the -unstable tree, not some older version. Jan> --- xen/drivers/passthrough/iommu.c.org 2012-10-05 03:38:33.000000000 +0000 > +++ xen/drivers/passthrough/iommu.c 2012-10-21 21:07:37.000000000 +0000 > @@ -38,7 +38,8 @@ > * no-intremap Disable VT-d Interrupt Remapping > */ > custom_param("iommu", parse_iommu_param); > -bool_t __read_mostly iommu_enabled = 1; > +bool_t iommu_enable_default __initdata = 1; > +bool_t __read_mostly iommu_enabled; > bool_t __read_mostly force_iommu; > bool_t __initdata iommu_dom0_strict; > bool_t __read_mostly iommu_verbose; > @@ -61,7 +62,7 @@ > *ss = ''\0''; > > if ( !parse_bool(s) ) > - iommu_enabled = 0; > + iommu_enable_default = 0; > else if ( !strcmp(s, "force") || !strcmp(s, "required") ) > force_iommu = 1; > else if ( !strcmp(s, "workaround_bios_bug") ) > @@ -316,7 +317,7 @@ > if ( iommu_dom0_strict ) > iommu_passthrough = 0; > > - if ( iommu_enabled ) > + if ( iommu_enable_default ) > { > rc = iommu_hardware_setup(); > iommu_enabled = (rc == 0); > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Jan Beulich
2012-Oct-24 14:09 UTC
Re: [PATCH V2] keep iommu disabled until iommu_setup() is called
>>> On 22.10.12 at 00:22, Ronny Hegewald <ronny.hegewald@online.de> wrote: > The iommu is enabled by default when xen is booting and later disabled in > iommu_setup() when no iommu is present. > > But under some circumstances iommu-code can be called before iommu_setup() is > > processed. If there is no iommu available xen crashes. > > This can happen for example when panic(...) is called that got introduced > with > patch "x86-64: detect processors subject to AMD erratum #121 and refuse to > boot." since xen 4.1.3 and results in the following stacktrace: > > find_iommu_for_device > amd_iommu_ioapic_update_ire > timer_interrupt > enable_8259_A_irq > do_IRQ > printk_start_of_line > acpi_os_printf > io_apic_write > __ioapic_write_entry > ioapic_write_entry > __clear_IO_APIC_pin > clear_IO_APIC > disable_IO_APIC > __stop_this_cpu > smp_send_stop > machine_restart > panic > tasklet_schedule_on_cpu > display_cacheinfo > init_amd > generic_identify > identify_cpu > _start_xen > _high_start > > > This patch fixes this by keeping the iommu disabled until iommu_setup() is > entered. > > Signed-off-by: Ronny Hegewald@online.deAs suspected, this breaks on Intel systems: When x2APIC is pre- enabled by the BIOS, iommu_supports_eim() now fails, causing a panic(). I''m checking whether this can be fixed, but the interactions are rather subtle (and there appears to be more breakage there). Jan> --- xen/drivers/passthrough/iommu.c.org 2012-10-05 03:38:33.000000000 +0000 > +++ xen/drivers/passthrough/iommu.c 2012-10-21 21:07:37.000000000 +0000 > @@ -38,7 +38,8 @@ > * no-intremap Disable VT-d Interrupt Remapping > */ > custom_param("iommu", parse_iommu_param); > -bool_t __read_mostly iommu_enabled = 1; > +bool_t iommu_enable_default __initdata = 1; > +bool_t __read_mostly iommu_enabled; > bool_t __read_mostly force_iommu; > bool_t __initdata iommu_dom0_strict; > bool_t __read_mostly iommu_verbose; > @@ -61,7 +62,7 @@ > *ss = ''\0''; > > if ( !parse_bool(s) ) > - iommu_enabled = 0; > + iommu_enable_default = 0; > else if ( !strcmp(s, "force") || !strcmp(s, "required") ) > force_iommu = 1; > else if ( !strcmp(s, "workaround_bios_bug") ) > @@ -316,7 +317,7 @@ > if ( iommu_dom0_strict ) > iommu_passthrough = 0; > > - if ( iommu_enabled ) > + if ( iommu_enable_default ) > { > rc = iommu_hardware_setup(); > iommu_enabled = (rc == 0); > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel