[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: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
  • Date: Fri, 29 Oct 2021 08:44:38 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.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=aSYA/iS4g4dvAdWlB/IxW3i1t49wb+LtWl8hPvbpU6o=; b=LHaee6BwwDG/ZMeH6Uyy5nOoEZ9jL0bk0ueqL7CSnwbSeCwEu8cgaYYGJkkXRR+7mr7H3BkooWIt1LSR2H8k7iftfpPeSsFp0hJeZTeIxSYXXB1+WNyizXd0JvYjYFUd19NFD9DJJt0/1L/fEXt5zmGDO7iIi8NwTpIVogTPnECUbQ4Ee+wLk6mbzfwX8H4+Wf8J4c3sp6RU1+30e2+xNq0DbBngN3moivhDzD38eFM4cNnT7RAOSqaVqj5Og6TLhjgHLlaNADUfqHd7IDnvRKGYdGOxacKKiOUadg1HNInvvmjmdtdDvY4+V+yVXxQwD+A3S4RpApQwfbJ3lcmdeg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LvWhFLgEs0h6qYxJQr52jKq4Axbom/nCT1S+i9JxqpAzuA0ul5rDiv5BSNeypl4seQU5oG9lhvIyNVq9sp65ILKm7Wdm16cpzSwTQ0UDY5nVGh5iMMcNnqN/SlMDPQOseD32qstifzfDqnhjb0kNZZrVo5fYeSaZLDH1TSD9WCGk2Y8lKf2CWEnv6MM7nCscLujA0USbBCt6o2Sxm0uVRHK8pAdLYyGlFXxCjf1VsbC4OM/O8gFJANvzDwSXtp94Fxx/MuXeM/KESbDMqAH3g7GU6GWegbktJLlHAN1sQHF87ehTqgGxR0w0N5BT6Vru5Lzf5y2AKiffICL8BUIMmg==
  • Authentication-results: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=intel.com;
  • Cc: "Cooper, Andrew" <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Fri, 29 Oct 2021 08:45:11 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHXxmJDj1yUJeGSDkOPNy18ywaldKvehyQAgAsuYGA=
  • Thread-topic: [PATCH v2 1/3] x86/IOMMU: mark IOMMU / intremap not in use when ACPI tables are missing

> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Friday, October 22, 2021 1:59 PM
> 
> 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.

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>

> 
> 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®.