2022-05-09 07:30:26

by Steve Wahl

[permalink] [raw]
Subject: [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 <[email protected]>
Reviewed-by: Mike Travis <[email protected]>
---

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_SUPPORTED MAX_IO_APICS
+# define DMAR_UNITS_SUPPORTED 640
#else
# define DMAR_UNITS_SUPPORTED 64
#endif
--
2.26.2



2022-05-13 02:23:18

by Steve Wahl

[permalink] [raw]
Subject: [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 <[email protected]>
---

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_SUPPORTED MAX_IO_APICS
-#else
-# define DMAR_UNITS_SUPPORTED 64
-#endif
+#define DMAR_UNITS_SUPPORTED CONFIG_DMAR_UNITS_SUPPORTED

/* DMAR Flags */
#define DMAR_INTR_REMAP 0x1
--
2.26.2


2022-05-13 04:07:12

by Steve Wahl

[permalink] [raw]
Subject: 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 <[email protected]>

I've received a report from the kernel test robot <[email protected]>,
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_SUPPORTED CONFIG_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_SUPPORTED CONFIG_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 <linux/types.h>
#include <linux/iova.h>
#include <linux/io.h>
@@ -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

2022-05-14 00:45:20

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

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 <[email protected]>
>
> I've received a report from the kernel test robot <[email protected]>,
> 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

>
> 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_SUPPORTED CONFIG_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_SUPPORTED CONFIG_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 <linux/types.h>
> #include <linux/iova.h>
> #include <linux/io.h>
> @@ -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
>
>


2022-05-18 19:59:52

by Steve Wahl

[permalink] [raw]
Subject: 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 <[email protected]>
> >
> > I've received a report from the kernel test robot <[email protected]>,
> > 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

2022-05-23 08:23:47

by Tian, Kevin

[permalink] [raw]
Subject: RE: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

> From: Steve Wahl
> Sent: Thursday, May 19, 2022 3:58 AM
>
> 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 <[email protected]>
> > >
> > > I've received a report from the kernel test robot <[email protected]>,
> > > 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?
>

This looks good to me.

Reviewed-by: Kevin Tian <[email protected]>

2022-06-13 21:57:20

by Jerry Snitselaar

[permalink] [raw]
Subject: 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 <[email protected]>
> ---
>
> 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(-)
>

Baolu do you have this queued up for v5.20? Also do you have a public repo where
you keep the vt-d changes before sending Joerg the patches for a release?

Regards,
Jerry

2022-06-13 21:59:08

by Jerry Snitselaar

[permalink] [raw]
Subject: 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 <[email protected]>
> ---
>
> 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?

> + 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_SUPPORTED MAX_IO_APICS
> -#else
> -# define DMAR_UNITS_SUPPORTED 64
> -#endif
> +#define DMAR_UNITS_SUPPORTED CONFIG_DMAR_UNITS_SUPPORTED
>
> /* DMAR Flags */
> #define DMAR_INTR_REMAP 0x1
> --
> 2.26.2
>

2022-06-14 01:40:55

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On 2022/6/14 04:38, 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 <[email protected]>
>> ---
>>
>> 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(-)
>>
>
> Baolu do you have this queued up for v5.20? Also do you have a public repo where
> you keep the vt-d changes before sending Joerg the patches for a release?

Yes. I have started to queue patches for v5.20. They could be found on
github:

https://github.com/LuBaolu/intel-iommu/commits/vtd-next-for-v5.20

Best regards,
baolu

2022-06-14 01:46:55

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

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 <[email protected]>
>> ---
>>
>> 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

>
>> + 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_SUPPORTED MAX_IO_APICS
>> -#else
>> -# define DMAR_UNITS_SUPPORTED 64
>> -#endif
>> +#define DMAR_UNITS_SUPPORTED CONFIG_DMAR_UNITS_SUPPORTED
>>
>> /* DMAR Flags */
>> #define DMAR_INTR_REMAP 0x1
>> --
>> 2.26.2
>>
>

2022-06-14 02:05:48

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On Mon, Jun 13, 2022 at 6:51 PM Baolu Lu <[email protected]> wrote:
>
> On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<[email protected]> 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<[email protected]>
> >>>> ---
> >>>>
> >>>> 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.

Regards,
Jerry

2022-06-14 02:52:12

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu <[email protected]> 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 <[email protected]>
> >> ---
> >>
> >> 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.


> >
> >> + 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_SUPPORTED MAX_IO_APICS
> >> -#else
> >> -# define DMAR_UNITS_SUPPORTED 64
> >> -#endif
> >> +#define DMAR_UNITS_SUPPORTED CONFIG_DMAR_UNITS_SUPPORTED
> >>
> >> /* DMAR Flags */
> >> #define DMAR_INTR_REMAP 0x1
> >> --
> >> 2.26.2
> >>
> >
>

2022-06-14 03:01:56

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On 2022/6/14 09:44, Jerry Snitselaar wrote:
> On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<[email protected]> 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<[email protected]>
>>>> ---
>>>>
>>>> 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

2022-06-14 03:06:48

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On 2022/6/14 09:54, Jerry Snitselaar wrote:
> On Mon, Jun 13, 2022 at 6:51 PM Baolu Lu <[email protected]> wrote:
>>
>> On 2022/6/14 09:44, Jerry Snitselaar wrote:
>>> On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<[email protected]> 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<[email protected]>
>>>>>> ---
>>>>>>
>>>>>> 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.

Best regards,
baolu

2022-06-14 16:48:38

by Steve Wahl

[permalink] [raw]
Subject: 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 <[email protected]> wrote:
> > >
> > > On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<[email protected]> 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<[email protected]>
> > > > > > > ---
> > > > > > >
> > > > > > > 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

2022-06-14 19:16:36

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

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 <[email protected]> wrote:
> > > >
> > > > On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<[email protected]> 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<[email protected]>
> > > > > > > > ---
> > > > > > > >
> > > > > > > > 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.
>

Yes, I think applying it with the depends prior to Baolu's change would
still run into the issue from the KTR report if someone compiled without
INTEL_IOMMU enabled.

This was dealing with being able to do something like:

make allmodconfig ARCH=arm64 ; grep DMAR_UNITS .config

and finding CONFIG_DMAR_UNITS_SUPPORTED=64.

Thinking some more though, instead of the depends being on the arch
would depending on DMAR_TABLE or INTEL_IOMMU be more appropriate?

Regards,
Jerry

> --> Steve
>
> --
> Steve Wahl, Hewlett Packard Enterprise

2022-06-14 21:48:58

by Steve Wahl

[permalink] [raw]
Subject: 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 <[email protected]> wrote:
> > > > >
> > > > > On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > > > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<[email protected]> 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<[email protected]>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > 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.
> >
>
> Yes, I think applying it with the depends prior to Baolu's change would
> still run into the issue from the KTR report if someone compiled without
> INTEL_IOMMU enabled.
>
> This was dealing with being able to do something like:
>
> make allmodconfig ARCH=arm64 ; grep DMAR_UNITS .config
>
> and finding CONFIG_DMAR_UNITS_SUPPORTED=64.
>
> Thinking some more though, instead of the depends being on the arch
> would depending on DMAR_TABLE or INTEL_IOMMU be more appropriate?

At least in my limited exploration, depending on INTEL_IOMMU yields
compile errors, but depending upon DMAR_TABLE appears to work fine.

--> Steve

--
Steve Wahl, Hewlett Packard Enterprise

2022-06-15 01:40:30

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

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 <[email protected]> wrote:
>>>>>>
>>>>>> On 2022/6/14 09:44, Jerry Snitselaar wrote:
>>>>>>> On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<[email protected]> 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<[email protected]>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> 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.
>>>
>>
>> Yes, I think applying it with the depends prior to Baolu's change would
>> still run into the issue from the KTR report if someone compiled without
>> INTEL_IOMMU enabled.
>>
>> This was dealing with being able to do something like:
>>
>> make allmodconfig ARCH=arm64 ; grep DMAR_UNITS .config
>>
>> and finding CONFIG_DMAR_UNITS_SUPPORTED=64.
>>
>> Thinking some more though, instead of the depends being on the arch
>> would depending on DMAR_TABLE or INTEL_IOMMU be more appropriate?
>
> At least in my limited exploration, depending on INTEL_IOMMU yields
> compile errors, but depending upon DMAR_TABLE appears to work fine.

DMAR_TABLE is used beyond INTEL_IOMMU, so depending on DMAR_TABLE seems
better.

Steve, do you mind posting a v3 with this fixed?

Best regards,
baolu

2022-06-15 15:27:07

by Steve Wahl

[permalink] [raw]
Subject: 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 <[email protected]> wrote:
> > > > > > >
> > > > > > > On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > > > > > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<[email protected]> 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<[email protected]>
> > > > > > > > > > > ---
> > > > > > > > > > >
> > > > > > > > > > > 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.
> > > >
> > >
> > > Yes, I think applying it with the depends prior to Baolu's change would
> > > still run into the issue from the KTR report if someone compiled without
> > > INTEL_IOMMU enabled.
> > >
> > > This was dealing with being able to do something like:
> > >
> > > make allmodconfig ARCH=arm64 ; grep DMAR_UNITS .config
> > >
> > > and finding CONFIG_DMAR_UNITS_SUPPORTED=64.
> > >
> > > Thinking some more though, instead of the depends being on the arch
> > > would depending on DMAR_TABLE or INTEL_IOMMU be more appropriate?
> >
> > At least in my limited exploration, depending on INTEL_IOMMU yields
> > compile errors, but depending upon DMAR_TABLE appears to work fine.
>
> DMAR_TABLE is used beyond INTEL_IOMMU, so depending on DMAR_TABLE seems
> better.
>
> Steve, do you mind posting a v3 with this fixed?

I can do that. Expect it shortly.

--> Steve

--
Steve Wahl, Hewlett Packard Enterprise

2022-06-15 18:46:07

by Steve Wahl

[permalink] [raw]
Subject: [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 <[email protected]>
Reviewed-by: Kevin Tian <[email protected]>
---

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_SUPPORTED MAX_IO_APICS
-#else
-# define DMAR_UNITS_SUPPORTED 64
-#endif
+#define DMAR_UNITS_SUPPORTED CONFIG_DMAR_UNITS_SUPPORTED

/* DMAR Flags */
#define DMAR_INTR_REMAP 0x1
--
2.26.2

2022-06-15 19:13:08

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH v3] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On Wed, Jun 15, 2022 at 01:36:50PM -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 <[email protected]>
> Reviewed-by: Kevin Tian <[email protected]>

Reviewed-by: Jerry Snitselaar <[email protected]>

> ---
>
> 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_SUPPORTED MAX_IO_APICS
> -#else
> -# define DMAR_UNITS_SUPPORTED 64
> -#endif
> +#define DMAR_UNITS_SUPPORTED CONFIG_DMAR_UNITS_SUPPORTED
>
> /* DMAR Flags */
> #define DMAR_INTR_REMAP 0x1
> --
> 2.26.2
>

2022-06-22 15:13:42

by Steve Wahl

[permalink] [raw]
Subject: 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 <[email protected]> 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<[email protected]>
> > > Reviewed-by: Kevin Tian<[email protected]>
> > > ---
> > >
> > > 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
> > <M> 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

2022-06-22 15:27:32

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH v3] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On Wed, Jun 22, 2022 at 7:52 AM Baolu Lu <[email protected]> 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<[email protected]>
> > Reviewed-by: Kevin Tian<[email protected]>
> > ---
> >
> > 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
> <M> 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

2022-06-22 15:55:05

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v3] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

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<[email protected]>
> Reviewed-by: Kevin Tian<[email protected]>
> ---
>
> 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
<M> 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

2022-06-23 02:43:57

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v3] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On 2022/6/22 23:05, Jerry Snitselaar wrote:
> On Wed, Jun 22, 2022 at 7:52 AM Baolu Lu<[email protected]> 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<[email protected]>
>>> Reviewed-by: Kevin Tian<[email protected]>
>>> ---
>>>
>>> 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
>> <M> 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.

It's more reasonable to move it under INTEL_IOMMU, but the trouble is
that this also stands even if INTEL_IOMMU is not configured.

The real problem here is that the iommu sequence ID overflows if
DMAR_UNITS_SUPPORTED is not big enough. This is purely a software
implementation issue, I am not sure whether user opt-in when building a
kernel package could help a lot here.

If we can't find a better way, can we just step back?

Best regards,
baolu

2022-06-23 04:45:14

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v3] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On 2022/6/23 10:51, Jerry Snitselaar wrote:
>> The real problem here is that the iommu sequence ID overflows if
>> DMAR_UNITS_SUPPORTED is not big enough. This is purely a software
>> implementation issue, I am not sure whether user opt-in when building a
>> kernel package could help a lot here.
>>
> Is this something that could be figured out when parsing the dmar
> table? It looks like currently iommu_refcnt[], iommu_did[], and
> dmar_seq_ids[] depend on it.

That's definitely a better solution.

Best regards,
baolu

2022-06-23 05:05:25

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH v3] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

On Thu, Jun 23, 2022 at 10:29:35AM +0800, Baolu Lu wrote:
> On 2022/6/22 23:05, Jerry Snitselaar wrote:
> > On Wed, Jun 22, 2022 at 7:52 AM Baolu Lu<[email protected]> 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<[email protected]>
> > > > Reviewed-by: Kevin Tian<[email protected]>
> > > > ---
> > > >
> > > > 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
> > > <M> 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.
>
> It's more reasonable to move it under INTEL_IOMMU, but the trouble is
> that this also stands even if INTEL_IOMMU is not configured.

My thought only was with it after the 'config INTEL_IOMMU' block and before 'if INTEL_IOMMU'
it would show up like:

[*] Support for Intel IOMMU using DMA Remapping Devices
(1024) Number of DMA Remapping Units supported <<<< NEW

>
> The real problem here is that the iommu sequence ID overflows if
> DMAR_UNITS_SUPPORTED is not big enough. This is purely a software
> implementation issue, I am not sure whether user opt-in when building a
> kernel package could help a lot here.
>

Is this something that could be figured out when parsing the dmar
table? It looks like currently iommu_refcnt[], iommu_did[], and
dmar_seq_ids[] depend on it.

Regards,
Jerry


> If we can't find a better way, can we just step back?
>
> Best regards,
> baolu