[PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Steve Wahl]

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 <steve.wahl@hpe.com>
Reviewed-by: Mike Travis <mike.travis@hpe.com>
---

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

Re: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Baolu Lu]

On 2022/5/6 03:46, Steve Wahl wrote:
> 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 <steve.wahl@hpe.com>
> Reviewed-by: Mike Travis <mike.travis@hpe.com>
> ---
> 
> 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.

+Kevin

This maximum value was introduced by below commit. And I don't see any
hardware/software restrictions that we can't enlarge it after ten years.

commit 1b198bb04ad72669d4bd6575fc9945ed595bfee0
Author: Mike Travis <travis@sgi.com>
Date:   Mon Mar 5 15:05:16 2012 -0800

     x86/iommu/intel: Increase the number of iommus supported to 
MAX_IO_APICS

     The number of IOMMUs supported should be the same as the number
     of IO APICS.  This limit comes into play when the IOMMUs are
     identity mapped, thus the number of possible IOMMUs in the
     "static identity" (si) domain should be this same number.
[...]

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

Best regards,
baolu

RE: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Friday, May 6, 2022 1:57 PM
> 
> On 2022/5/6 03:46, Steve Wahl wrote:
> > 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 <steve.wahl@hpe.com>
> > Reviewed-by: Mike Travis <mike.travis@hpe.com>
> > ---
> >
> > 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.
> 
> +Kevin
> 
> This maximum value was introduced by below commit. And I don't see any
> hardware/software restrictions that we can't enlarge it after ten years.

I don't see a rationale either, but...

> 
> commit 1b198bb04ad72669d4bd6575fc9945ed595bfee0
> Author: Mike Travis <travis@sgi.com>
> Date:   Mon Mar 5 15:05:16 2012 -0800
> 
>      x86/iommu/intel: Increase the number of iommus supported to
> MAX_IO_APICS
> 
>      The number of IOMMUs supported should be the same as the number
>      of IO APICS.  This limit comes into play when the IOMMUs are
>      identity mapped, thus the number of possible IOMMUs in the
>      "static identity" (si) domain should be this same number.
> [...]
> 
> >
> >   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
> 

... is it necessary to permanently do 10x increase which wastes memory
on most platforms which won't have such need.

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?

another nit: dmar is intel specific thus CONFIG_X86 is always true.

Thanks
Kevin

Re: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Rodel, Jorg]

On Fri, May 06, 2022 at 06:49:43AM +0000, Tian, Kevin wrote:
> another nit: dmar is intel specific thus CONFIG_X86 is always true.

There are Itanium systems which have DMAR units. Is that no longer
supported?

Regards,

-- 
Jörg Rödel
jroedel@suse.de

SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
 
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Re: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1862 bytes --]

On Fri, 2022-05-06 at 06:49 +0000, Tian, Kevin wrote:
> > From: Baolu Lu <baolu.lu@linux.intel.com>
> > 
> > > --- 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
> 
> ... 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.

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

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?


> another nit: dmar is intel specific thus CONFIG_X86 is always true.

DMAR exists on IA64 too.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

RE: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Tian, Kevin]

> From: Rodel, Jorg
> Sent: Friday, May 6, 2022 3:11 PM
> 
> On Fri, May 06, 2022 at 06:49:43AM +0000, Tian, Kevin wrote:
> > another nit: dmar is intel specific thus CONFIG_X86 is always true.
> 
> There are Itanium systems which have DMAR units. Is that no longer
> supported?
> 

Sorry I just forgot that. haven't touched an Itanium system for
over ten years. 😊

RE: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Tian, Kevin]

> From: David Woodhouse <dwmw2@infradead.org>
> Sent: Friday, May 6, 2022 3:17 PM
> 
> On Fri, 2022-05-06 at 06:49 +0000, Tian, Kevin wrote:
> > > From: Baolu Lu <baolu.lu@linux.intel.com>
> > >
> > > > --- 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
> >
> > ... 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

Re: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Steve Wahl]

On Fri, May 06, 2022 at 08:12:11AM +0000, Tian, Kevin wrote:
> > From: David Woodhouse <dwmw2@infradead.org>
> > Sent: Friday, May 6, 2022 3:17 PM
> > 
> > On Fri, 2022-05-06 at 06:49 +0000, Tian, Kevin wrote:
> > > > From: Baolu Lu <baolu.lu@linux.intel.com>
> > > >
> > > > > --- 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
> > >
> > > ... 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)
# define	DMAR_UNITS_SUPPORTED	640
#elif defined(CONFIG_X86)
# define	DMAR_UNITS_SUPPORTED	MAX_IO_APICS
#else
# define	DMAR_UNITS_SUPPORTED	64
#endif

Thank you all for your time looking at this.

--> Steve Wahl

-- 
Steve Wahl, Hewlett Packard Enterprise

RE: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Tian, Kevin]

> From: Steve Wahl <steve.wahl@hpe.com>
> Sent: Friday, May 6, 2022 11:26 PM
> 
> On Fri, May 06, 2022 at 08:12:11AM +0000, Tian, Kevin wrote:
> > > From: David Woodhouse <dwmw2@infradead.org>
> > > Sent: Friday, May 6, 2022 3:17 PM
> > >
> > > On Fri, 2022-05-06 at 06:49 +0000, Tian, Kevin wrote:
> > > > > From: Baolu Lu <baolu.lu@linux.intel.com>
> > > > >
> > > > > > --- 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
> > > >
> > > > ... 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)
> # define	DMAR_UNITS_SUPPORTED	640
> #elif defined(CONFIG_X86)
> # define	DMAR_UNITS_SUPPORTED	MAX_IO_APICS
> #else
> # define	DMAR_UNITS_SUPPORTED	64
> #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.

Thanks
Kevin

Re: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Steve Wahl]

On Tue, May 10, 2022 at 01:16:26AM +0000, Tian, Kevin wrote:
> > From: Steve Wahl <steve.wahl@hpe.com>
> > Sent: Friday, May 6, 2022 11:26 PM
> > 
> > On Fri, May 06, 2022 at 08:12:11AM +0000, Tian, Kevin wrote:
> > > > From: David Woodhouse <dwmw2@infradead.org>
> > > > Sent: Friday, May 6, 2022 3:17 PM
> > > >
> > > > On Fri, 2022-05-06 at 06:49 +0000, Tian, Kevin wrote:
> > > > > > From: Baolu Lu <baolu.lu@linux.intel.com>
> > > > > >
> > > > > > > --- 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
> > > > >
> > > > > ... 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)
> > # define	DMAR_UNITS_SUPPORTED	640
> > #elif defined(CONFIG_X86)
> > # define	DMAR_UNITS_SUPPORTED	MAX_IO_APICS
> > #else
> > # define	DMAR_UNITS_SUPPORTED	64
> > #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
     other    64

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

RE: [PATCH] iommu/vt-d: Increase DMAR_UNITS_SUPPORTED

[Tian, Kevin]

> From: Steve Wahl <steve.wahl@hpe.com>
> Sent: Wednesday, May 11, 2022 3:07 AM
> 
> On Tue, May 10, 2022 at 01:16:26AM +0000, Tian, Kevin wrote:
> > > From: Steve Wahl <steve.wahl@hpe.com>
> > > Sent: Friday, May 6, 2022 11:26 PM
> > >
> > > On Fri, May 06, 2022 at 08:12:11AM +0000, Tian, Kevin wrote:
> > > > > From: David Woodhouse <dwmw2@infradead.org>
> > > > > Sent: Friday, May 6, 2022 3:17 PM
> > > > >
> > > > > On Fri, 2022-05-06 at 06:49 +0000, Tian, Kevin wrote:
> > > > > > > From: Baolu Lu <baolu.lu@linux.intel.com>
> > > > > > >
> > > > > > > > --- 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
> > > > > >
> > > > > > ... 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)
> > > # define	DMAR_UNITS_SUPPORTED	640
> > > #elif defined(CONFIG_X86)
> > > # define	DMAR_UNITS_SUPPORTED	MAX_IO_APICS
> > > #else
> > > # define	DMAR_UNITS_SUPPORTED	64
> > > #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

usually we do 2's power thus 1024 is more reasonable. If people do
care about the exact memory footprint they can always manually
change it.

>      X86_64?  128
>      X86_32?  64
>      other    64
> 
> And DMAR_UNITS_SUPPORTED gets removed, and everywhere it was used
> we
> use CONFIG_NR_DMARS in its place?

Let's keep DMAR_UNITS_SUPPORTED and just redefine it to be
CONFIG_NR_DMARS for less changes.

> 
> I can give that a shot but wanted to confirm this is what you'd want
> first.
> 
> Thanks,
> 
> --> Steve
> 
> --
> Steve Wahl, Hewlett Packard Enterprise

[PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

[Steve Wahl]

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 <steve.wahl@hpe.com>
---

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

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

[Steve Wahl]

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 <steve.wahl@hpe.com>

I've received a report from the kernel test robot <lkp@intel.com>,
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

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

[Baolu Lu]

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 <steve.wahl@hpe.com>
> 
> I've received a report from the kernel test robot <lkp@intel.com>,
> 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
> 
> 

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

[Steve Wahl]

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 <steve.wahl@hpe.com>
> > 
> > I've received a report from the kernel test robot <lkp@intel.com>,
> > 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

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

[Tian, Kevin]

> 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 <steve.wahl@hpe.com>
> > >
> > > I've received a report from the kernel test robot <lkp@intel.com>,
> > > 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 <kevin.tian@intel.com>

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

[Jerry Snitselaar]

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 <steve.wahl@hpe.com>
> ---
> 
> 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

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

[Jerry Snitselaar]

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 <steve.wahl@hpe.com>
> ---
> 
> 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
> 

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

[Baolu Lu]

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 <steve.wahl@hpe.com>
>> ---
>>
>> 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

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

[Baolu Lu]

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 <steve.wahl@hpe.com>
>> ---
>>
>> 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
>>
> 

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

[Jerry Snitselaar]

On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu <baolu.lu@linux.intel.com> 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 <steve.wahl@hpe.com>
> >> ---
> >>
> >> 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
> >>
> >
>

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

[Baolu Lu]

On 2022/6/14 09:44, Jerry Snitselaar wrote:
> On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
>>>> ---
>>>>
>>>> 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

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

[Jerry Snitselaar]

On Mon, Jun 13, 2022 at 6:51 PM Baolu Lu <baolu.lu@linux.intel.com> wrote:
>
> On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
> >>>> ---
> >>>>
> >>>> 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

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

[Baolu Lu]

On 2022/6/14 09:54, Jerry Snitselaar wrote:
> On Mon, Jun 13, 2022 at 6:51 PM Baolu Lu <baolu.lu@linux.intel.com> wrote:
>>
>> On 2022/6/14 09:44, Jerry Snitselaar wrote:
>>> On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
>>>>>> ---
>>>>>>
>>>>>> 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

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

[Steve Wahl]

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 <baolu.lu@linux.intel.com> wrote:
> > > 
> > > On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
> > > > > > > ---
> > > > > > > 
> > > > > > > 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

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

[Jerry Snitselaar]

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 <baolu.lu@linux.intel.com> wrote:
> > > > 
> > > > On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > 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

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

[Steve Wahl]

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 <baolu.lu@linux.intel.com> wrote:
> > > > > 
> > > > > On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > > > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
> > > > > > > > > ---
> > > > > > > > > 
> > > > > > > > > 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

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

[Baolu Lu]

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 <baolu.lu@linux.intel.com> wrote:
>>>>>>
>>>>>> On 2022/6/14 09:44, Jerry Snitselaar wrote:
>>>>>>> On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> 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

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

[Steve Wahl]

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 <baolu.lu@linux.intel.com> wrote:
> > > > > > > 
> > > > > > > On 2022/6/14 09:44, Jerry Snitselaar wrote:
> > > > > > > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
> > > > > > > > > > > ---
> > > > > > > > > > > 
> > > > > > > > > > > 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

[PATCH v3] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

[Steve Wahl]

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 <steve.wahl@hpe.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
---

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

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

[Jerry Snitselaar]

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 <steve.wahl@hpe.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

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

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

[Baolu Lu]

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<steve.wahl@hpe.com>
> Reviewed-by: Kevin Tian<kevin.tian@intel.com>
> ---
> 
> 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

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

[Jerry Snitselaar]

On Wed, Jun 22, 2022 at 7:52 AM Baolu Lu <baolu.lu@linux.intel.com> 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<steve.wahl@hpe.com>
> > Reviewed-by: Kevin Tian<kevin.tian@intel.com>
> > ---
> >
> > 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

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

[Steve Wahl]

On Wed, Jun 22, 2022 at 08:05:12AM -0700, Jerry Snitselaar wrote:
> On Wed, Jun 22, 2022 at 7:52 AM Baolu Lu <baolu.lu@linux.intel.com> 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<steve.wahl@hpe.com>
> > > Reviewed-by: Kevin Tian<kevin.tian@intel.com>
> > > ---
> > >
> > > 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

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

[Baolu Lu]

On 2022/6/22 23:05, Jerry Snitselaar wrote:
> On Wed, Jun 22, 2022 at 7:52 AM Baolu Lu<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
>>> Reviewed-by: Kevin Tian<kevin.tian@intel.com>
>>> ---
>>>
>>> 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

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

[Jerry Snitselaar]

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<baolu.lu@linux.intel.com>  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<steve.wahl@hpe.com>
> > > > Reviewed-by: Kevin Tian<kevin.tian@intel.com>
> > > > ---
> > > > 
> > > > 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

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

[Baolu Lu]

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