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

Re: [PATCH v2 1/3] x86/IOMMU: mark IOMMU / intremap not in use when ACPI tables are missing


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 22 Oct 2021 07:59:20 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ooAiu42OjOtMgI5grSxQUzr+M00R2QwtZHSiOu261VM=; b=bCl6ncrZrggjoc/w+EHHf++IeX07tyf8n8JtpqjmZFfnSCNj4h0Cdnk9pfhhsSVBln4affM9oADC9MQTsJh1pT32tcXbpl6FZJh+hrfG13DgWCiNiV5J1Al0gvPaWS7XGOG92tzCbN6zxmjDTc3O0iu29itDpiT1/aaXmMkvM3luRquF6HL9LOJ5xb7UK090C+gNCQDrF/oW5eMKD8ZunvpQQX08zn8MvEIcM7hPFkX7ofaMSJJjnHQoFy4I0NAN1Uarp8fEDyjwahhSk4oINSYixdDkD8w5r369kP9sG3JTYF+UixCT7cUOumDRL5LZZ7u6IJ4m7l8HbfrJ+O1vNA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ez1WLYeOY5rZ48KDwU7oiXOi1Y0RMvvzY3irRzc8o/lHgCogUcwIXTSdMwMPl5c3DP/Pkl6hKPC/0RHX+Wcx7+XVbw1b27UywIOmPQ8jO2VZqYqVGydIjQ4upD5QTKtRCYJ44pmT5S8zV6L0DXt0gFpGzWwxXiBMtCHJ0JLba2PE+So2hXRGLk3G2L848oO0yqTcPEEXmOvJ/j7iVybCgm8uc7h3RFrLFuqgmqzo7Asrh+Iqvz18mP/Dgo460qGkS0PA/Iz90Kpb18nh53pNjuLIAiYU7KoQu4XxErdwVo4R5pcp1B1sQfEB6oUZ5aqpClyZHwJQZcSRBSLCYVY6NQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>
  • Delivery-date: Fri, 22 Oct 2021 05:59:45 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 21.10.2021 11:58, Jan Beulich wrote:
> x2apic_bsp_setup() gets called ahead of iommu_setup(), and since x2APIC
> mode (physical vs clustered) depends on iommu_intremap, that variable
> needs to be set to off as soon as we know we can't / won't enable
> interrupt remapping, i.e. in particular when parsing of the respective
> ACPI tables failed. Move the turning off of iommu_intremap from AMD
> specific code into acpi_iommu_init(), accompanying it by clearing of
> iommu_enable.
> 
> Take the opportunity and also fully skip ACPI table parsing logic on
> VT-d when both "iommu=off" and "iommu=no-intremap" are in effect anyway,
> like was already the case for AMD.
> 
> The tag below only references the commit uncovering a pre-existing
> anomaly.
> 
> Fixes: d8bd82327b0f ("AMD/IOMMU: obtain IVHD type to use earlier")
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Ouch, forgot to Cc Kevin; now added.

Jan

> ---
> While the change here deals with apic_x2apic_probe() as called from
> x2apic_bsp_setup(), the check_x2apic_preenabled() path looks to be
> similarly affected. That call occurs before acpi_boot_init(), which is
> what calls acpi_iommu_init(). The ordering in setup.c is in part
> relatively fragile, which is why for the moment I'm still hesitant to
> move the generic_apic_probe() call down. Plus I don't have easy access
> to a suitable system to test this case. Thoughts?
> ---
> v2: Treat iommu_enable and iommu_intremap as separate options.
> 
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -183,9 +183,6 @@ int __init acpi_ivrs_init(void)
>  {
>      int rc;
>  
> -    if ( !iommu_enable && !iommu_intremap )
> -        return 0;
> -
>      rc = amd_iommu_get_supported_ivhd_type();
>      if ( rc < 0 )
>          return rc;
> @@ -193,10 +190,7 @@ int __init acpi_ivrs_init(void)
>      ivhd_type = rc;
>  
>      if ( (amd_iommu_detect_acpi() !=0) || (iommu_found() == 0) )
> -    {
> -        iommu_intremap = iommu_intremap_off;
>          return -ENODEV;
> -    }
>  
>      iommu_init_ops = &_iommu_init_ops;
>  
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -777,11 +777,7 @@ static int __init acpi_parse_dmar(struct
>      dmar = (struct acpi_table_dmar *)table;
>      dmar_flags = dmar->flags;
>  
> -    if ( !iommu_enable && !iommu_intremap )
> -    {
> -        ret = -EINVAL;
> -        goto out;
> -    }
> +    ASSERT(iommu_enable || iommu_intremap);
>  
>      if ( !dmar->width )
>      {
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -41,6 +41,24 @@ enum iommu_intremap __read_mostly iommu_
>  bool __read_mostly iommu_intpost;
>  #endif
>  
> +void __init acpi_iommu_init(void)
> +{
> +    int ret;
> +
> +    if ( !iommu_enable && !iommu_intremap )
> +        return;
> +
> +    ret = acpi_dmar_init();
> +    if ( ret == -ENODEV )
> +        ret = acpi_ivrs_init();
> +
> +    if ( ret )
> +    {
> +        iommu_enable = false;
> +        iommu_intremap = iommu_intremap_off;
> +    }
> +}
> +
>  int __init iommu_hardware_setup(void)
>  {
>      struct IO_APIC_route_entry **ioapic_entries = NULL;
> --- a/xen/include/asm-x86/acpi.h
> +++ b/xen/include/asm-x86/acpi.h
> @@ -141,16 +141,10 @@ extern u32 x86_acpiid_to_apicid[];
>  extern u32 pmtmr_ioport;
>  extern unsigned int pmtmr_width;
>  
> +void acpi_iommu_init(void);
>  int acpi_dmar_init(void);
>  int acpi_ivrs_init(void);
>  
> -static inline int acpi_iommu_init(void)
> -{
> -    int ret = acpi_dmar_init();
> -
> -    return ret == -ENODEV ? acpi_ivrs_init() : ret;
> -}
> -
>  void acpi_mmcfg_init(void);
>  
>  /* Incremented whenever we transition through S3. Value is 1 during boot. */
> 
> 




 


Rackspace

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