[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Rusty Bird <rustybird@xxxxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxx>
  • From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
  • Date: Mon, 31 Jul 2017 07:11:33 +0000
  • Accept-language: en-US
  • Delivery-date: Mon, 31 Jul 2017 07:12:19 +0000
  • Dlp-product: dlpe-windows
  • Dlp-reaction: no-action
  • Dlp-version: 10.0.102.7
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>
  • Thread-index: AQHTBs9KJYM3N8Nk2EaymT+56ymJMqJtimHQ
  • Thread-topic: [PATCH] VT-d: don't panic/warn on iommu=no-igfx

> From: Rusty Bird [mailto:rustybird@xxxxxxxxxxxxxxx]
> 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 <rustybird@xxxxxxxxxxxxxxx>
> ---
> 
> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.