Re: [EXTERNAL] PROBLEM: commit f36a74b9345a leads to not booting system with AMD 2990WX

Re: [EXTERNAL] PROBLEM: commit f36a74b9345a leads to not booting system with AMD 2990WX

[David Woodhouse]

[-- Attachment #1.1: Type: text/plain, Size: 1523 bytes --]

On Tue, 2021-01-05 at 00:05 +0100, Johnathan Smithinovic wrote:
> commit f36a74b9345a leads to not booting system with AMD 2990WX
> 
> 
> When trying to boot 5.11-rc2 as usual the messages of the bootloader stay on my
> screen and not much appears to happen (fans run a bit slower than in GRUB,
> devices don't seem to get accessed). Without this commit everything seems to
> work as usual.
> 
> (In the hope that it is helpful I appended messages mentioning "IO APIC"
> from 5.11-rc2 with the mentioned commit reverted (and the kernel parameter
> apic=debug added).
> I'm sorry that I can't provide more details: I'm unsure how I can gather
> more information since my Motherboard (ASUS ROG ZENITH EXTREME) does not have
> any documented serial ports and USB devices don't seem to get turned on.)

No problem, that was enough to reproduce it in qemu just by simulating
the same BIOS bug which causes it to *start* enabling, then abort, the
IRQ remapping. Thanks for the report.

Does this fix it?

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 7e2c445a1fae..f0adbc48fd17 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3854,6 +3854,9 @@ static int irq_remapping_select(struct irq_domain *d, struct irq_fwspec *fwspec,
 	struct amd_iommu *iommu;
 	int devid = -1;
 
+	if (!amd_iommu_irq_remap)
+		return 0;
+
 	if (x86_fwspec_is_ioapic(fwspec))
 		devid = get_ioapic_devid(fwspec->param[0]);
 	else if (x86_fwspec_is_hpet(fwspec))


[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] iommu/amd: Stop irq_remapping_select() matching when remapping is disabled

[David Woodhouse]

[-- Attachment #1.1: Type: text/plain, Size: 1587 bytes --]

From: David Woodhouse <dwmw@amazon.co.uk>

The AMD IOMMU initialisation registers the IRQ remapping domain for
each IOMMU before doing the final sanity check that every I/OAPIC is
covered.

This means that the AMD irq_remapping_select() function gets invoked
even when IRQ remapping has been disabled, eventually leading to a NULL
pointer dereference in alloc_irq_table().

Unfortunately, the IVRS isn't fully parsed early enough that the sanity
check can be done in time to registering the IRQ domain altogether.
Doing that would be nice, but is a larger and more error-prone task. The
simple fix is just for irq_remapping_select() to refuse to report a
match when IRQ remapping has disabled.

Link: https://lore.kernel.org/lkml/ed4be9b4-24ac-7128-c522-7ef359e8185d@gmx.at
Fixes: a1a785b57242 ("iommu/amd: Implement select() method on remapping irqdomain")
Reported-by: Johnathan Smithinovic <johnathan.smithinovic@gmx.at>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 drivers/iommu/amd/iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 7e2c445a1fae..f0adbc48fd17 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3854,6 +3854,9 @@ static int irq_remapping_select(struct irq_domain *d, struct irq_fwspec *fwspec,
 	struct amd_iommu *iommu;
 	int devid = -1;
 
+	if (!amd_iommu_irq_remap)
+		return 0;
+
 	if (x86_fwspec_is_ioapic(fwspec))
 		devid = get_ioapic_devid(fwspec->param[0]);
 	else if (x86_fwspec_is_hpet(fwspec))
-- 
2.29.2


[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [EXTERNAL] PROBLEM: commit f36a74b9345a leads to not booting system with AMD 2990WX

[Johnathan Smithinovic]

On 05/01/2021 01:19, David Woodhouse wrote:
> On Tue, 2021-01-05 at 00:05 +0100, Johnathan Smithinovic wrote:
>> commit f36a74b9345a leads to not booting system with AMD 2990WX
>>
>>
>
> No problem, that was enough to reproduce it in qemu just by simulating
> the same BIOS bug which causes it to *start* enabling, then abort, the
> IRQ remapping. Thanks for the report.
>
> Does this fix it?
>

Yes, the kernel now starts as usual, thanks a lot!

Kind regards
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/amd: Stop irq_remapping_select() matching when remapping is disabled

[Will Deacon]

On Tue, 05 Jan 2021 01:36:13 +0000, David Woodhouse wrote:
> The AMD IOMMU initialisation registers the IRQ remapping domain for
> each IOMMU before doing the final sanity check that every I/OAPIC is
> covered.
> 
> This means that the AMD irq_remapping_select() function gets invoked
> even when IRQ remapping has been disabled, eventually leading to a NULL
> pointer dereference in alloc_irq_table().
> 
> [...]

Applied to arm64 (for-next/iommu/fixes), thanks!

[1/1] iommu/amd: Stop irq_remapping_select() matching when remapping is disabled
      https://git.kernel.org/arm64/c/b34f10c2dc59

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu