Re: [PATCH v1 0/6] iommu/vt-d: Reset DMAR_UNITS_SUPPORTED
On Sat, Jun 25, 2022 at 08:51:58PM +0800, Lu Baolu wrote: > Hi folks, > > This is a follow-up series of changes proposed by this patch: > > https://lore.kernel.org/linux-iommu/20220615183650.32075-1-steve.w...@hpe.com/ > > It removes several static arrays of size DMAR_UNITS_SUPPORTED and sets > the DMAR_UNITS_SUPPORTED to 1024. > After Kevin Tian's comments, for the whole series: Reviewed-by: Steve Wahl --> Steve -- Steve Wahl, Hewlett Packard Enterprise ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Wed, Jun 22, 2022 at 08:05:12AM -0700, Jerry Snitselaar wrote: > On Wed, Jun 22, 2022 at 7:52 AM Baolu Lu wrote: > > > > On 2022/6/16 02:36, Steve Wahl wrote: > > > To support up to 64 sockets with 10 DMAR units each (640), make the > > > value of DMAR_UNITS_SUPPORTED adjustable by a config variable, > > > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is > > > set. > > > > > > If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set > > > to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to > > > allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ > > > remapping doesn't support X2APIC mode x2apic disabled"; and the system > > > fails to boot properly. > > > > > > Signed-off-by: Steve Wahl > > > Reviewed-by: Kevin Tian > > > --- > > > > > > Note that we could not find a reason for connecting > > > DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps > > > it seemed like the two would continue to match on earlier processors. > > > There doesn't appear to be kernel code that assumes that the value of > > > one is related to the other. > > > > > > v2: Make this value a config option, rather than a fixed constant. The > > > default > > > values should match previous configuration except in the MAXSMP case. > > > Keeping the > > > value at a power of two was requested by Kevin Tian. > > > > > > v3: Make the config option dependent upon DMAR_TABLE, as it is not used > > > without this. > > > > > > drivers/iommu/intel/Kconfig | 7 +++ > > > include/linux/dmar.h| 6 +- > > > 2 files changed, 8 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > > > index 39a06d245f12..07aaebcb581d 100644 > > > --- a/drivers/iommu/intel/Kconfig > > > +++ b/drivers/iommu/intel/Kconfig > > > @@ -9,6 +9,13 @@ config DMAR_PERF > > > config DMAR_DEBUG > > > bool > > > > > > +config DMAR_UNITS_SUPPORTED > > > + int "Number of DMA Remapping Units supported" > > > + depends on DMAR_TABLE > > > + default 1024 if MAXSMP > > > + default 128 if X86_64 > > > + default 64 > > > > With this patch applied, the IOMMU configuration looks like: > > > > [*] AMD IOMMU support > > AMD IOMMU Version 2 driver > > [*] Enable AMD IOMMU internals in DebugFS > > (1024) Number of DMA Remapping Units supported <<<< NEW > > [*] Support for Intel IOMMU using DMA Remapping Devices > > [*] Export Intel IOMMU internals in Debugfs > > [*] Support for Shared Virtual Memory with Intel IOMMU > > [*] Enable Intel DMA Remapping Devices by default > > [*] Enable Intel IOMMU scalable mode by default > > [*] Support for Interrupt Remapping > > [*] OMAP IOMMU Support > > [*] Export OMAP IOMMU internals in DebugFS > > [*] Rockchip IOMMU Support > > > > The NEW item looks confusing. It looks to be a generic configurable > > value though it's actually Intel DMAR specific. Any thoughts? > > > > Best regards, > > baolu > > > > Would moving it under INTEL_IOMMU at least have it show up below > "Support for Intel IOMMU using DMA Remapping Devices"? I'm not sure it > can be better than that, because IRQ_REMAP selects DMAR_TABLE, so we > can't stick it in the if INTEL_IOMMU section. > > Regards, > Jerry I have no qualms with Jerry's suggestion. --> Steve Wahl -- Steve Wahl, Hewlett Packard Enterprise ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
To support up to 64 sockets with 10 DMAR units each (640), make the value of DMAR_UNITS_SUPPORTED adjustable by a config variable, CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is set. If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ remapping doesn't support X2APIC mode x2apic disabled"; and the system fails to boot properly. Signed-off-by: Steve Wahl Reviewed-by: Kevin Tian --- Note that we could not find a reason for connecting DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps it seemed like the two would continue to match on earlier processors. There doesn't appear to be kernel code that assumes that the value of one is related to the other. v2: Make this value a config option, rather than a fixed constant. The default values should match previous configuration except in the MAXSMP case. Keeping the value at a power of two was requested by Kevin Tian. v3: Make the config option dependent upon DMAR_TABLE, as it is not used without this. drivers/iommu/intel/Kconfig | 7 +++ include/linux/dmar.h| 6 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index 39a06d245f12..07aaebcb581d 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -9,6 +9,13 @@ config DMAR_PERF config DMAR_DEBUG bool +config DMAR_UNITS_SUPPORTED + int "Number of DMA Remapping Units supported" + depends on DMAR_TABLE + default 1024 if MAXSMP + default 128 if X86_64 + default 64 + config INTEL_IOMMU bool "Support for Intel IOMMU using DMA Remapping Devices" depends on PCI_MSI && ACPI && (X86 || IA64) diff --git a/include/linux/dmar.h b/include/linux/dmar.h index 45e903d84733..0c03c1845c23 100644 --- a/include/linux/dmar.h +++ b/include/linux/dmar.h @@ -18,11 +18,7 @@ struct acpi_dmar_header; -#ifdef CONFIG_X86 -# define DMAR_UNITS_SUPPORTEDMAX_IO_APICS -#else -# define DMAR_UNITS_SUPPORTED64 -#endif +#defineDMAR_UNITS_SUPPORTEDCONFIG_DMAR_UNITS_SUPPORTED /* DMAR Flags */ #define DMAR_INTR_REMAP0x1 -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Wed, Jun 15, 2022 at 09:38:35AM +0800, Baolu Lu wrote: > On 2022/6/15 05:12, Steve Wahl wrote: > > On Tue, Jun 14, 2022 at 12:01:45PM -0700, Jerry Snitselaar wrote: > > > On Tue, Jun 14, 2022 at 11:45:35AM -0500, Steve Wahl wrote: > > > > On Tue, Jun 14, 2022 at 10:21:29AM +0800, Baolu Lu wrote: > > > > > On 2022/6/14 09:54, Jerry Snitselaar wrote: > > > > > > On Mon, Jun 13, 2022 at 6:51 PM Baolu Lu > > > > > > wrote: > > > > > > > > > > > > > > On 2022/6/14 09:44, Jerry Snitselaar wrote: > > > > > > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu > > > > > > > > Lu wrote: > > > > > > > > > On 2022/6/14 04:57, Jerry Snitselaar wrote: > > > > > > > > > > On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: > > > > > > > > > > > To support up to 64 sockets with 10 DMAR units each > > > > > > > > > > > (640), make the > > > > > > > > > > > value of DMAR_UNITS_SUPPORTED adjustable by a config > > > > > > > > > > > variable, > > > > > > > > > > > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 > > > > > > > > > > > when MAXSMP is > > > > > > > > > > > set. > > > > > > > > > > > > > > > > > > > > > > If the available hardware exceeds DMAR_UNITS_SUPPORTED > > > > > > > > > > > (previously set > > > > > > > > > > > to MAX_IO_APICS, or 128), it causes these messages: > > > > > > > > > > > "DMAR: Failed to > > > > > > > > > > > allocate seq_id", "DMAR: Parse DMAR table failure.", and > > > > > > > > > > > "x2apic: IRQ > > > > > > > > > > > remapping doesn't support X2APIC mode x2apic disabled"; > > > > > > > > > > > and the system > > > > > > > > > > > fails to boot properly. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Steve Wahl > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > Note that we could not find a reason for connecting > > > > > > > > > > > DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done > > > > > > > > > > > previously. Perhaps > > > > > > > > > > > it seemed like the two would continue to match on earlier > > > > > > > > > > > processors. > > > > > > > > > > > There doesn't appear to be kernel code that assumes that > > > > > > > > > > > the value of > > > > > > > > > > > one is related to the other. > > > > > > > > > > > > > > > > > > > > > > v2: Make this value a config option, rather than a fixed > > > > > > > > > > > constant. The default > > > > > > > > > > > values should match previous configuration except in the > > > > > > > > > > > MAXSMP case. Keeping the > > > > > > > > > > > value at a power of two was requested by Kevin Tian. > > > > > > > > > > > > > > > > > > > > > > drivers/iommu/intel/Kconfig | 6 ++ > > > > > > > > > > > include/linux/dmar.h| 6 +- > > > > > > > > > > > 2 files changed, 7 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/iommu/intel/Kconfig > > > > > > > > > > > b/drivers/iommu/intel/Kconfig > > > > > > > > > > > index 247d0f2d5fdf..fdbda77ac21e 100644 > > > > > > > > > > > --- a/drivers/iommu/intel/Kconfig > > > > > > > > > > > +++ b/drivers/iommu/intel/Kconfig > > > > > > > > > > > @@ -9,6 +9,12 @@ config DMAR_PERF > > > > > > > > >
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Tue, Jun 14, 2022 at 12:01:45PM -0700, Jerry Snitselaar wrote: > On Tue, Jun 14, 2022 at 11:45:35AM -0500, Steve Wahl wrote: > > On Tue, Jun 14, 2022 at 10:21:29AM +0800, Baolu Lu wrote: > > > On 2022/6/14 09:54, Jerry Snitselaar wrote: > > > > On Mon, Jun 13, 2022 at 6:51 PM Baolu Lu > > > > wrote: > > > > > > > > > > On 2022/6/14 09:44, Jerry Snitselaar wrote: > > > > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu > > > > > > wrote: > > > > > > > On 2022/6/14 04:57, Jerry Snitselaar wrote: > > > > > > > > On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: > > > > > > > > > To support up to 64 sockets with 10 DMAR units each (640), > > > > > > > > > make the > > > > > > > > > value of DMAR_UNITS_SUPPORTED adjustable by a config variable, > > > > > > > > > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when > > > > > > > > > MAXSMP is > > > > > > > > > set. > > > > > > > > > > > > > > > > > > If the available hardware exceeds DMAR_UNITS_SUPPORTED > > > > > > > > > (previously set > > > > > > > > > to MAX_IO_APICS, or 128), it causes these messages: "DMAR: > > > > > > > > > Failed to > > > > > > > > > allocate seq_id", "DMAR: Parse DMAR table failure.", and > > > > > > > > > "x2apic: IRQ > > > > > > > > > remapping doesn't support X2APIC mode x2apic disabled"; and > > > > > > > > > the system > > > > > > > > > fails to boot properly. > > > > > > > > > > > > > > > > > > Signed-off-by: Steve Wahl > > > > > > > > > --- > > > > > > > > > > > > > > > > > > Note that we could not find a reason for connecting > > > > > > > > > DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. > > > > > > > > > Perhaps > > > > > > > > > it seemed like the two would continue to match on earlier > > > > > > > > > processors. > > > > > > > > > There doesn't appear to be kernel code that assumes that the > > > > > > > > > value of > > > > > > > > > one is related to the other. > > > > > > > > > > > > > > > > > > v2: Make this value a config option, rather than a fixed > > > > > > > > > constant. The default > > > > > > > > > values should match previous configuration except in the > > > > > > > > > MAXSMP case. Keeping the > > > > > > > > > value at a power of two was requested by Kevin Tian. > > > > > > > > > > > > > > > > > > drivers/iommu/intel/Kconfig | 6 ++ > > > > > > > > > include/linux/dmar.h| 6 +- > > > > > > > > > 2 files changed, 7 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/drivers/iommu/intel/Kconfig > > > > > > > > > b/drivers/iommu/intel/Kconfig > > > > > > > > > index 247d0f2d5fdf..fdbda77ac21e 100644 > > > > > > > > > --- a/drivers/iommu/intel/Kconfig > > > > > > > > > +++ b/drivers/iommu/intel/Kconfig > > > > > > > > > @@ -9,6 +9,12 @@ config DMAR_PERF > > > > > > > > > config DMAR_DEBUG > > > > > > > > >bool > > > > > > > > > > > > > > > > > > +config DMAR_UNITS_SUPPORTED > > > > > > > > > +int "Number of DMA Remapping Units supported" > > > > > > > > Also, should there be a "depends on (X86 || IA64)" here? > > > > > > > Do you have any compilation errors or warnings? > > > > > > > > > > > > > > Best regards, > > > > > > > baolu > > > > > > > > > > > > >
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Tue, Jun 14, 2022 at 10:21:29AM +0800, Baolu Lu wrote: > On 2022/6/14 09:54, Jerry Snitselaar wrote: > > On Mon, Jun 13, 2022 at 6:51 PM Baolu Lu wrote: > > > > > > On 2022/6/14 09:44, Jerry Snitselaar wrote: > > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu > > > > wrote: > > > > > On 2022/6/14 04:57, Jerry Snitselaar wrote: > > > > > > On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: > > > > > > > To support up to 64 sockets with 10 DMAR units each (640), make > > > > > > > the > > > > > > > value of DMAR_UNITS_SUPPORTED adjustable by a config variable, > > > > > > > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when > > > > > > > MAXSMP is > > > > > > > set. > > > > > > > > > > > > > > If the available hardware exceeds DMAR_UNITS_SUPPORTED > > > > > > > (previously set > > > > > > > to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed > > > > > > > to > > > > > > > allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: > > > > > > > IRQ > > > > > > > remapping doesn't support X2APIC mode x2apic disabled"; and the > > > > > > > system > > > > > > > fails to boot properly. > > > > > > > > > > > > > > Signed-off-by: Steve Wahl > > > > > > > --- > > > > > > > > > > > > > > Note that we could not find a reason for connecting > > > > > > > DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. > > > > > > > Perhaps > > > > > > > it seemed like the two would continue to match on earlier > > > > > > > processors. > > > > > > > There doesn't appear to be kernel code that assumes that the > > > > > > > value of > > > > > > > one is related to the other. > > > > > > > > > > > > > > v2: Make this value a config option, rather than a fixed > > > > > > > constant. The default > > > > > > > values should match previous configuration except in the MAXSMP > > > > > > > case. Keeping the > > > > > > > value at a power of two was requested by Kevin Tian. > > > > > > > > > > > > > > drivers/iommu/intel/Kconfig | 6 ++ > > > > > > > include/linux/dmar.h| 6 +- > > > > > > > 2 files changed, 7 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/iommu/intel/Kconfig > > > > > > > b/drivers/iommu/intel/Kconfig > > > > > > > index 247d0f2d5fdf..fdbda77ac21e 100644 > > > > > > > --- a/drivers/iommu/intel/Kconfig > > > > > > > +++ b/drivers/iommu/intel/Kconfig > > > > > > > @@ -9,6 +9,12 @@ config DMAR_PERF > > > > > > > config DMAR_DEBUG > > > > > > >bool > > > > > > > > > > > > > > +config DMAR_UNITS_SUPPORTED > > > > > > > +int "Number of DMA Remapping Units supported" > > > > > > Also, should there be a "depends on (X86 || IA64)" here? > > > > > Do you have any compilation errors or warnings? > > > > > > > > > > Best regards, > > > > > baolu > > > > > > > > > I think it is probably harmless since it doesn't get used elsewhere, > > > > but our tooling was complaining to me because DMAR_UNITS_SUPPORTED was > > > > being autogenerated into the configs for the non-x86 architectures we > > > > build (aarch64, s390x, ppcle64). > > > > We have files corresponding to the config options that it looks at, > > > > and I had one for x86 and not the others so it noticed the > > > > discrepancy. > > > > > > So with "depends on (X86 || IA64)", that tool doesn't complain anymore, > > > right? > > > > > > Best regards, > > > baolu > > > > > > > Yes, with the depends it no longer happens. > > The dmar code only exists on X86 and IA64 arch's. Adding this depending > makes sense to me. I will add it if no objections. I think that works after Baolu's patchset that makes intel-iommu.h private. I'm pretty sure it wouldn't have worked before that. No objections. --> Steve -- Steve Wahl, Hewlett Packard Enterprise ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Fri, May 13, 2022 at 10:09:46AM +0800, Baolu Lu wrote: > On 2022/5/13 07:12, Steve Wahl wrote: > > On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: > > > To support up to 64 sockets with 10 DMAR units each (640), make the > > > value of DMAR_UNITS_SUPPORTED adjustable by a config variable, > > > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is > > > set. > > > > > > If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set > > > to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to > > > allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ > > > remapping doesn't support X2APIC mode x2apic disabled"; and the system > > > fails to boot properly. > > > > > > Signed-off-by: Steve Wahl > > > > I've received a report from the kernel test robot , > > that this patch causes an error (shown below) when > > CONFIG_IOMMU_SUPPORT is not set. > > > > In my opinion, this is because include/linux/dmar.h and > > include/linux/intel-iommu are being #included when they are not really > > being used. > > > > I've tried placing the contents of intel-iommu.h within an #ifdef > > CONFIG_INTEL_IOMMU, and that fixes the problem. > > > > Two questions: > > > > A) Is this the desired approach to to the fix? > > Most part of include/linux/intel-iommu.h is private to Intel IOMMU > driver. They should be put in a header like drivers/iommu/intel > /iommu.h. Eventually, we should remove include/linux/intel-iommu.h > and device drivers interact with iommu subsystem through the IOMMU > kAPIs. > > Best regards, > baolu Baolu's recent patch to move intel-iommu.h private still allows my [PATCH v2] to apply with no changes, and gets rid of the compile errors when CONFIG_IOMMU_SUPPORT is not set, so the kernel test robot should be happy now. Is there another step I should do to bring this patch back into focus? Thanks. --> Steve -- Steve Wahl, Hewlett Packard Enterprise ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/7] iommu/vt-d: Make intel-iommu.h private
On Sat, May 14, 2022 at 09:43:15AM +0800, Lu Baolu wrote: > Hi folks, > > The include/linux/intel-iommu.h should be private to the Intel IOMMU > driver. Other drivers or components should interact with the IOMMU > drivers through the kAPIs provided by the iommu core. > > This series cleanups all includes of intel-iommu.h outside of the Intel > IOMMU driver and move this header from include/linux to > drivers/iommu/intel/. > > No functional changes intended. Please help to review and suggest. I went through and examined the changes as well. These changes make the robots complaint against my patch go away, which is great by me! Reviewed-by: Steve Wahl -- Steve Wahl, Hewlett Packard Enterprise ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: > To support up to 64 sockets with 10 DMAR units each (640), make the > value of DMAR_UNITS_SUPPORTED adjustable by a config variable, > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is > set. > > If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set > to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to > allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ > remapping doesn't support X2APIC mode x2apic disabled"; and the system > fails to boot properly. > > Signed-off-by: Steve Wahl I've received a report from the kernel test robot , that this patch causes an error (shown below) when CONFIG_IOMMU_SUPPORT is not set. In my opinion, this is because include/linux/dmar.h and include/linux/intel-iommu are being #included when they are not really being used. I've tried placing the contents of intel-iommu.h within an #ifdef CONFIG_INTEL_IOMMU, and that fixes the problem. Two questions: A) Is this the desired approach to to the fix? B) Should it be a separate patch, or added onto this patch as a v3? Error message: -- In file included from include/linux/intel-iommu.h:21, from arch/x86/kvm/x86.c:44: >> include/linux/dmar.h:21:33: error: 'CONFIG_DMAR_UNITS_SUPPORTED' undeclared >> here (not in a function); did you mean 'DMAR_UNITS_SUPPORTED'? 21 | #define DMAR_UNITS_SUPPORTEDCONFIG_DMAR_UNITS_SUPPORTED | ^~~ include/linux/intel-iommu.h:531:35: note: in expansion of macro 'DMAR_UNITS_SUPPORTED' 531 | unsigned int iommu_refcnt[DMAR_UNITS_SUPPORTED]; | ^~~~ vim +21 include/linux/dmar.h 20 > 21 #define DMAR_UNITS_SUPPORTEDCONFIG_DMAR_UNITS_SUPPORTED 22 Initial stab at fixing it: -- diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h index 2f9891cb3d00..916fd7b5bcb5 100644 --- a/include/linux/intel-iommu.h +++ b/include/linux/intel-iommu.h @@ -10,6 +10,8 @@ #ifndef _INTEL_IOMMU_H_ #define _INTEL_IOMMU_H_ +#ifdef CONFIG_INTEL_IOMMU + #include #include #include @@ -831,4 +833,6 @@ static inline const char *decode_prq_descriptor(char *str, size_t size, return str; } +#endif /* CONFIG_IOMMU_SUPPORT */ + #endif Thanks. --> Steve Wahl -- Steve Wahl, Hewlett Packard Enterprise ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
To support up to 64 sockets with 10 DMAR units each (640), make the value of DMAR_UNITS_SUPPORTED adjustable by a config variable, CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is set. If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ remapping doesn't support X2APIC mode x2apic disabled"; and the system fails to boot properly. Signed-off-by: Steve Wahl --- Note that we could not find a reason for connecting DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps it seemed like the two would continue to match on earlier processors. There doesn't appear to be kernel code that assumes that the value of one is related to the other. v2: Make this value a config option, rather than a fixed constant. The default values should match previous configuration except in the MAXSMP case. Keeping the value at a power of two was requested by Kevin Tian. drivers/iommu/intel/Kconfig | 6 ++ include/linux/dmar.h| 6 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index 247d0f2d5fdf..fdbda77ac21e 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -9,6 +9,12 @@ config DMAR_PERF config DMAR_DEBUG bool +config DMAR_UNITS_SUPPORTED + int "Number of DMA Remapping Units supported" + default 1024 if MAXSMP + default 128 if X86_64 + default 64 + config INTEL_IOMMU bool "Support for Intel IOMMU using DMA Remapping Devices" depends on PCI_MSI && ACPI && (X86 || IA64) diff --git a/include/linux/dmar.h b/include/linux/dmar.h index 45e903d84733..0c03c1845c23 100644 --- a/include/linux/dmar.h +++ b/include/linux/dmar.h @@ -18,11 +18,7 @@ struct acpi_dmar_header; -#ifdef CONFIG_X86 -# define DMAR_UNITS_SUPPORTEDMAX_IO_APICS -#else -# define DMAR_UNITS_SUPPORTED64 -#endif +#defineDMAR_UNITS_SUPPORTEDCONFIG_DMAR_UNITS_SUPPORTED /* DMAR Flags */ #define DMAR_INTR_REMAP0x1 -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED
On Tue, May 10, 2022 at 01:16:26AM +, Tian, Kevin wrote: > > From: Steve Wahl > > Sent: Friday, May 6, 2022 11:26 PM > > > > On Fri, May 06, 2022 at 08:12:11AM +, Tian, Kevin wrote: > > > > From: David Woodhouse > > > > Sent: Friday, May 6, 2022 3:17 PM > > > > > > > > On Fri, 2022-05-06 at 06:49 +, Tian, Kevin wrote: > > > > > > From: Baolu Lu > > > > > > > > > > > > > --- a/include/linux/dmar.h > > > > > > > +++ b/include/linux/dmar.h > > > > > > > @@ -19,7 +19,7 @@ > > > > > > > struct acpi_dmar_header; > > > > > > > > > > > > > > #ifdef CONFIG_X86 > > > > > > > -# define DMAR_UNITS_SUPPORTEDMAX_IO_APICS > > > > > > > +# define DMAR_UNITS_SUPPORTED640 > > > > > > > #else > > > > > > > # defineDMAR_UNITS_SUPPORTED64 > > > > > > > #endif > > > > > > > > > > ... is it necessary to permanently do 10x increase which wastes memory > > > > > on most platforms which won't have such need. > > > > > > > > I was just looking at that. It mostly adds about 3½ KiB to each struct > > > > dmar_domain. > > > > > > > > I think the only actual static array is the dmar_seq_ids bitmap which > > > > grows to 640 *bits* which is fairly negligible, and the main growth is > > > > that it adds about 3½ KiB to each struct dmar_domain for the > > > > iommu_refcnt[] and iommu_did[] arrays. > > > > > > Thanks for the quick experiment! though the added material is > > > negligible it's cleaner to me if having a way to configure it as > > > discussed below. > > > > > > > > > > > > Does it make more sense to have a configurable approach similar to > > > > > CONFIG_NR_CPUS? or even better can we just replace those static > > > > > arrays with dynamic allocation so removing this restriction > > > > > completely? > > > > > > > > Hotplug makes that fun, but I suppose you only need to grow the array > > > > in a given struct dmar_domain if you actually add a device to it that's > > > > behind a newly added IOMMU. I don't know if the complexity of making it > > > > fully dynamic is worth it though. We could make it a config option, > > > > and/or a command line option (perhaps automatically derived from > > > > CONFIG_NR_CPUS). > > > > > > either config option or command line option is OK to me. Probably > > > the former is simpler given no need to dynamically expand the > > > static array. btw though deriving from CONFIG_NR_CPUS could work > > > in this case it is unclear why tying the two together is necessary in > > > concept, e.g. is there guarantee that the number of IOMMUs must > > > be smaller than the number of CPUs in a platform? > > > > > > > > > > > If it wasn't for hotplug, I think we'd know the right number by the > > > > time we actually need it anyway, wouldn't we? Can we have a heuristic > > > > for how many DMAR units are likely to be hotplugged? Is it as simple as > > > > the ratio of present to not-yet-present CPUs in MADT? > > > > > > Probably. But I don't have enough knowledge on DMAR hotplug to > > > judge (e.g. whether it's strictly tied to CPU hotplug and if yes whether > > > there could be multiple IOMMUs hotplugged together with a CPU > > > socket)... > > > > > > Thanks > > > Kevin > > > > Would anyone be more comfortable if we only increase the limit where > > MAXSMP is set? > > > > i.e. > > > > #if defined(CONFIG_X86) && defined(CONFIG_MAXSMP) > > # defineDMAR_UNITS_SUPPORTED640 > > #elif defined(CONFIG_X86) > > # defineDMAR_UNITS_SUPPORTEDMAX_IO_APICS > > #else > > # defineDMAR_UNITS_SUPPORTED64 > > #endif > > > > Thank you all for your time looking at this. > > > > This works for your own configuration but it's unclear whether other > MAXSMP platforms have the exact same requirements (different > number of sockets, different ratio of #iommus/#sockets, etc.). In any > case since we are at it having a generic way to extend it makes more > sense to me. So, to be clear, what you would like to see would be Kconfig entries to create a config option, say "NR_DMARS", set up so the default is: MAXSMP? 640 X86_64? 128 X86_32? 64 other64 And DMAR_UNITS_SUPPORTED gets removed, and everywhere it was used we use CONFIG_NR_DMARS in its place? I can give that a shot but wanted to confirm this is what you'd want first. Thanks, --> Steve -- Steve Wahl, Hewlett Packard Enterprise ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED
On Fri, May 06, 2022 at 08:12:11AM +, Tian, Kevin wrote: > > From: David Woodhouse > > Sent: Friday, May 6, 2022 3:17 PM > > > > On Fri, 2022-05-06 at 06:49 +, Tian, Kevin wrote: > > > > From: Baolu Lu > > > > > > > > > --- a/include/linux/dmar.h > > > > > +++ b/include/linux/dmar.h > > > > > @@ -19,7 +19,7 @@ > > > > > struct acpi_dmar_header; > > > > > > > > > > #ifdef CONFIG_X86 > > > > > -# define DMAR_UNITS_SUPPORTEDMAX_IO_APICS > > > > > +# define DMAR_UNITS_SUPPORTED640 > > > > > #else > > > > > # defineDMAR_UNITS_SUPPORTED64 > > > > > #endif > > > > > > ... is it necessary to permanently do 10x increase which wastes memory > > > on most platforms which won't have such need. > > > > I was just looking at that. It mostly adds about 3½ KiB to each struct > > dmar_domain. > > > > I think the only actual static array is the dmar_seq_ids bitmap which > > grows to 640 *bits* which is fairly negligible, and the main growth is > > that it adds about 3½ KiB to each struct dmar_domain for the > > iommu_refcnt[] and iommu_did[] arrays. > > Thanks for the quick experiment! though the added material is > negligible it's cleaner to me if having a way to configure it as > discussed below. > > > > > > Does it make more sense to have a configurable approach similar to > > > CONFIG_NR_CPUS? or even better can we just replace those static > > > arrays with dynamic allocation so removing this restriction > > > completely? > > > > Hotplug makes that fun, but I suppose you only need to grow the array > > in a given struct dmar_domain if you actually add a device to it that's > > behind a newly added IOMMU. I don't know if the complexity of making it > > fully dynamic is worth it though. We could make it a config option, > > and/or a command line option (perhaps automatically derived from > > CONFIG_NR_CPUS). > > either config option or command line option is OK to me. Probably > the former is simpler given no need to dynamically expand the > static array. btw though deriving from CONFIG_NR_CPUS could work > in this case it is unclear why tying the two together is necessary in > concept, e.g. is there guarantee that the number of IOMMUs must > be smaller than the number of CPUs in a platform? > > > > > If it wasn't for hotplug, I think we'd know the right number by the > > time we actually need it anyway, wouldn't we? Can we have a heuristic > > for how many DMAR units are likely to be hotplugged? Is it as simple as > > the ratio of present to not-yet-present CPUs in MADT? > > Probably. But I don't have enough knowledge on DMAR hotplug to > judge (e.g. whether it's strictly tied to CPU hotplug and if yes whether > there could be multiple IOMMUs hotplugged together with a CPU > socket)... > > Thanks > Kevin Would anyone be more comfortable if we only increase the limit where MAXSMP is set? i.e. #if defined(CONFIG_X86) && defined(CONFIG_MAXSMP) # defineDMAR_UNITS_SUPPORTED640 #elif defined(CONFIG_X86) # defineDMAR_UNITS_SUPPORTEDMAX_IO_APICS #else # defineDMAR_UNITS_SUPPORTED64 #endif Thank you all for your time looking at this. --> Steve Wahl -- Steve Wahl, Hewlett Packard Enterprise ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED
Increase DMAR_UNITS_SUPPORTED to support 64 sockets with 10 DMAR units each, for a total of 640. If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ remapping doesn't support X2APIC mode x2apic disabled"; and the system fails to boot. Signed-off-by: Steve Wahl Reviewed-by: Mike Travis --- Note that we could not find a reason for connecting DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps it seemed like the two would continue to match on earlier processors. There doesn't appear to be kernel code that assumes that the value of one is related to the other. include/linux/dmar.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/dmar.h b/include/linux/dmar.h index 45e903d84733..9d4867b8f42e 100644 --- a/include/linux/dmar.h +++ b/include/linux/dmar.h @@ -19,7 +19,7 @@ struct acpi_dmar_header; #ifdef CONFIG_X86 -# define DMAR_UNITS_SUPPORTEDMAX_IO_APICS +# define DMAR_UNITS_SUPPORTED640 #else # define DMAR_UNITS_SUPPORTED64 #endif -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
