Re: [Xen-devel] [PATCH] VT-d: don't panic/warn on iommu=no-igfx

[Rusty Bird]

Tian, Kevin:
> > From: Rusty Bird [mailto:rustyb...@openmailbox.org]
> > +if ( !iommu_igfx )
> > +return;
> 
> A message might be also helpful here so user can confirm its
> boot option takes effect...

Done, see v2 patch. Thanks for the feedback!

Rusty


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] VT-d: don't panic/warn on iommu=no-igfx

[Tian, Kevin]

> From: Rusty Bird [mailto:rustyb...@openmailbox.org]
> Sent: Thursday, July 27, 2017 7:54 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 _without_ the 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 
> ---
> 
> Notes:
> Best viewed with "git show --ignore-space-change --function-context"
> 
>  xen/drivers/passthrough/vtd/iommu.c  | 18 --
>  xen/drivers/passthrough/vtd/quirks.c |  3 ---
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 19328f6..2849ea1 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -747,14 +747,20 @@ 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 )
> +return;

A message might be also helpful here so user can confirm its
boot option takes effect...

> 
> -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;
> 
> --
> 2.9.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] VT-d: don't panic/warn on iommu=no-igfx

[Rusty Bird]

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 _without_ the 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 
---

Notes:
Best viewed with "git show --ignore-space-change --function-context"

 xen/drivers/passthrough/vtd/iommu.c  | 18 --
 xen/drivers/passthrough/vtd/quirks.c |  3 ---
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 19328f6..2849ea1 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -747,14 +747,20 @@ 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 )
+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;
 
-- 
2.9.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel