Re: [Xen-devel] [PATCH v3] IOMMU: keep disabled until iommu_setup() is called

["Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx>]

Looks fine, Thanks!
Acked-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Wednesday, October 24, 2012 11:50 PM
> To: xen-devel
> Cc: Wei Wang; Zhang, Xiantao; Ronny Hegewald
> Subject: [PATCH v3] IOMMU: keep 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 as introduced with the
> patch "x86-64: detect processors subject to AMD erratum #121 and refuse to
> boot" since xen 4.1.3, resulting in
> find_iommu_for_device() to be called in the context of
> disable_IO_APIC() / __stop_this_cpu().
> 
> This patch fixes this by keeping the iommu disabled until iommu_setup() is
> entered.
> 
> Originally-by: Ronny Hegewald <ronny.hegewald@xxxxxxxxx>
> 
> In order for iommu_enable to be off initially, iommu_supports_eim() must
> not depend on it anymore, nor must acpi_parse_dmar(). The former in turn
> requires that iommu_intremap gets uncoupled from iommu_enabled (in
> particular, failure during IOMMU setup should no longer result in
> iommu_intremap getting cleared by generic code; IOMMU specific code can
> still do so provided in can live with the consequences).
> 
> This could have the nice side effect of allowing to use "iommu=off"
> even when x2APIC was pre-enabled by the BIOS (in which case interrupt
> remapping is a requirement, but DMA translation [obviously] isn't), but that
> doesn't currently work (and hence x2apic_bsp_setup() forces the IOMMU
> on rather than just interrupt remapping).
> 
> For consistency with VT-d, make the AMD IOMMU code also skip all ACPI
> table parsing when neither iommu_enable nor iommu_intremap are set.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -975,6 +975,8 @@ void __init x2apic_bsp_setup(void)
>          goto restore_out;
>      }
> 
> +    force_iommu = 1;
> +
>      genapic = apic_x2apic_probe();
>      printk("Switched to APIC driver %s.\n", genapic->name);
> 
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -164,9 +164,13 @@ int __init amd_iov_detect(void)  {
>      INIT_LIST_HEAD(&amd_iommu_head);
> 
> +    if ( !iommu_enable && !iommu_intremap )
> +        return 0;
> +
>      if ( (amd_iommu_detect_acpi() !=0) || (iommu_found() == 0) )
>      {
>          printk("AMD-Vi: IOMMU not found!\n");
> +        iommu_intremap = 0;
>          return -ENODEV;
>      }
> 
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -41,7 +41,8 @@ static void iommu_dump_p2m_table(unsigne
>   *   no-intremap                Disable VT-d Interrupt Remapping
>   */
>  custom_param("iommu", parse_iommu_param); -bool_t __read_mostly
> iommu_enabled = 1;
> +bool_t __initdata iommu_enable = 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;
> @@ -77,7 +78,7 @@ static void __init parse_iommu_param(cha
>              *ss = '\0';
> 
>          if ( !parse_bool(s) )
> -            iommu_enabled = 0;
> +            iommu_enable = 0;
>          else if ( !strcmp(s, "force") || !strcmp(s, "required") )
>              force_iommu = val;
>          else if ( !strcmp(s, "workaround_bios_bug") ) @@ -395,7 +396,7 @@ int
> __init iommu_setup(void)
>      if ( iommu_dom0_strict )
>          iommu_passthrough = 0;
> 
> -    if ( iommu_enabled )
> +    if ( iommu_enable )
>      {
>          rc = iommu_hardware_setup();
>          iommu_enabled = (rc == 0);
> @@ -409,8 +410,6 @@ int __init iommu_setup(void)
>      if ( !iommu_enabled )
>      {
>          iommu_snoop = 0;
> -        iommu_qinval = 0;
> -        iommu_intremap = 0;
>          iommu_passthrough = 0;
>          iommu_dom0_strict = 0;
>      }
> @@ -419,6 +418,8 @@ int __init iommu_setup(void)
>          printk(" - Dom0 mode: %s\n",
>                 iommu_passthrough ? "Passthrough" :
>                 iommu_dom0_strict ? "Strict" : "Relaxed");
> +    else if ( iommu_intremap )
> +        printk("Interrupt remapping enabled\n");
> 
>      return rc;
>  }
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -744,7 +744,7 @@ static int __init acpi_parse_dmar(struct
>      dmar = (struct acpi_table_dmar *)table;
>      dmar_flags = dmar->flags;
> 
> -    if ( !iommu_enabled )
> +    if ( !iommu_enable && !iommu_intremap )
>      {
>          ret = -EINVAL;
>          goto out;
> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -149,8 +149,7 @@ int iommu_supports_eim(void)
>      struct acpi_drhd_unit *drhd;
>      int apic;
> 
> -    if ( !iommu_enabled || !iommu_qinval || !iommu_intremap ||
> -         list_empty(&acpi_drhd_units) )
> +    if ( !iommu_qinval || !iommu_intremap ||
> + list_empty(&acpi_drhd_units) )
>          return 0;
> 
>      /* We MUST have a DRHD unit for each IOAPIC. */
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2086,7 +2086,10 @@ int __init intel_vtd_setup(void)
>      int ret;
> 
>      if ( list_empty(&acpi_drhd_units) )
> -        return -ENODEV;
> +    {
> +        ret = -ENODEV;
> +        goto error;
> +    }
> 
>      platform_quirks_init();
> 
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -26,7 +26,7 @@
>  #include <public/hvm/ioreq.h>
>  #include <public/domctl.h>
> 
> -extern bool_t iommu_enabled;
> +extern bool_t iommu_enable, iommu_enabled;
>  extern bool_t force_iommu, iommu_verbose;  extern bool_t
> iommu_workaround_bios_bug, iommu_passthrough;  extern bool_t
> iommu_snoop, iommu_qinval, iommu_intremap;
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel