| Message ID | 20170731090404.6503-1-rustybird@openmailbox.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> From: Rusty Bird [mailto:rustybird@openmailbox.org] > Sent: Monday, July 31, 2017 5:04 PM > > When operating on an Intel graphics device, iommu_enable_translation() > panicked (force_iommu==1) or warned (force_iommu==0) about the BIOS if > is_igd_vt_enabled_quirk() returned 0. That's good if the actual BIOS > problem has been detected. But since commit 1463411, returning 0 could > also happen if the user simply passed "iommu=no-igfx", in which case > bailing out with an info message (instead of a panic/warning) would be > more appropriate. > > The panic broke the combination "iommu=force,no-igfx", and also the case > where "iommu=no-igfx" is passed but force_iommu=1 is set automatically > by x2apic_bsp_setup(). > > Move the iommu_igfx check from is_igd_vt_enabled_quirk() into its only > caller iommu_enable_translation(), and tweak the logic. > > Signed-off-by: Rusty Bird <rustybird@openmailbox.org> Acked-by: Kevin Tian <kevin.tian@intel.com>
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index 19328f6..daaed0a 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -747,14 +747,24 @@ static void iommu_enable_translation(struct acpi_drhd_unit *drhd) unsigned long flags; struct iommu *iommu = drhd->iommu; - if ( is_igd_drhd(drhd) && !is_igd_vt_enabled_quirk() ) + if ( is_igd_drhd(drhd) ) { - if ( force_iommu ) - panic("BIOS did not enable IGD for VT properly, crash Xen for security purpose"); + if ( !iommu_igfx ) + { + printk(XENLOG_INFO VTDPREFIX + "Passed iommu=no-igfx option. Disabling IGD VT-d engine.\n"); + return; + } - printk(XENLOG_WARNING VTDPREFIX - "BIOS did not enable IGD for VT properly. Disabling IGD VT-d engine.\n"); - return; + if ( !is_igd_vt_enabled_quirk() ) + { + if ( force_iommu ) + panic("BIOS did not enable IGD for VT properly, crash Xen for security purpose"); + + printk(XENLOG_WARNING VTDPREFIX + "BIOS did not enable IGD for VT properly. Disabling IGD VT-d engine.\n"); + return; + } } /* apply platform specific errata workarounds */ diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c index 91f96ac..5bbbd96 100644 --- a/xen/drivers/passthrough/vtd/quirks.c +++ b/xen/drivers/passthrough/vtd/quirks.c @@ -70,9 +70,6 @@ int is_igd_vt_enabled_quirk(void) { u16 ggc; - if ( !iommu_igfx ) - return 0; - if ( !IS_ILK(ioh_id) ) return 1;
When operating on an Intel graphics device, iommu_enable_translation() panicked (force_iommu==1) or warned (force_iommu==0) about the BIOS if is_igd_vt_enabled_quirk() returned 0. That's good if the actual BIOS problem has been detected. But since commit 1463411, returning 0 could also happen if the user simply passed "iommu=no-igfx", in which case bailing out with an info message (instead of a panic/warning) would be more appropriate. The panic broke the combination "iommu=force,no-igfx", and also the case where "iommu=no-igfx" is passed but force_iommu=1 is set automatically by x2apic_bsp_setup(). Move the iommu_igfx check from is_igd_vt_enabled_quirk() into its only caller iommu_enable_translation(), and tweak the logic. Signed-off-by: Rusty Bird <rustybird@openmailbox.org> --- Notes: Changed since v1: print info message when iommu_igfx==0 Best viewed with "git show --ignore-space-change --function-context" xen/drivers/passthrough/vtd/iommu.c | 22 ++++++++++++++++------ xen/drivers/passthrough/vtd/quirks.c | 3 --- 2 files changed, 16 insertions(+), 9 deletions(-)