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

From: Baolu Lu
Date: Tue Jun 14 2022 - 21:38:53 EST


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@xxxxxxxxxxxxxxx> wrote:

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

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