Re: [PATCH 0/3] iommu/amd: Fix booting with amd_iommu=off
On Wed, Mar 17, 2021 at 05:10:34PM +0800, Joerg Roedel wrote: > From: Joerg Roedel > > Hi, > > it turned out that booting a kernel with amd_iommu=off on a machine > that has an AMD IOMMU causes an early kernel crash. There are two > reasons for this, and fixing one of them is already sufficient, but > both reasons deserve fixing, which is done in this patch-set. > > Regards, > > Joerg > > Joerg Roedel (3): > iommu/amd: Move Stoney Ridge check to detect_ivrs() > iommu/amd: Don't call early_amd_iommu_init() when AMD IOMMU is > disabled > iommu/amd: Keep track of amd_iommu_irq_remap state Series are Acked-by: Huang Rui > > drivers/iommu/amd/init.c | 36 > 1 file changed, 20 insertions(+), 16 deletions(-) > > -- > 2.30.2 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix iommu remap panic while amd_iommu is set to disable
On Tue, Mar 16, 2021 at 11:00:26PM +0800, Joerg Roedel wrote: > On Tue, Mar 16, 2021 at 09:36:02PM +0800, Huang Rui wrote: > > Thanks for the comments. Could you please elaborate this? > > > > Do you mean while amd_iommu=off, we won't prepare the IVRS, and even > > needn't get all ACPI talbes. Because they are never be used and the next > > state will always goes into IOMMU_CMDLINE_DISABLED, am I right? > > The first problem was that amd_iommu_irq_remap is never set back to > false when irq-remapping initialization fails in amd_iommu_prepare(). > > But there are other problems, like that even when the IOMMU is set to > disabled on the command line with amd_iommu=off, the code still sets up > all IOMMUs and registers IRQ domains for them. > Yes, that was the problem I found in my side. No very clear about the orignal intention. But if you said this is a problem, that makes sense... > Later the code checks wheter the IOMMU should stay disabled and tears > everything down, except for the IRQ domains, which stay in the global > list. > > The APIs do not really support tearing down IRQ domains well, so its not > so easy to add this to the tear-down path. Now that the IRQ domains stay > in the list, the ACPI code will come along later and calls the > ->select() call-back for every IRQ domain, which gets execution to > irq_remapping_select(), depite IOMMU being disabled and > amd_iommu_rlookup_table already de-allocated. But since > amd_iommu_irq_remap is still true the NULL pointer is dereferenced, > causing the crash. OK. We found some memleaks here before as well. It looks cause by iommu data buffer initialization and not be cleared. And yes, if setup iommu here, it actually causes many issues. unreferenced object 0x888332047500 (size 224): comm "swapper/0", pid 0, jiffies 4294892296 (age 362.192s) hex dump (first 32 bytes): b0 22 03 00 00 00 00 00 00 00 00 40 00 00 00 00 .".@ 06 00 00 00 00 00 00 00 00 20 00 00 00 20 00 00 . ... .. backtrace: [<8f162024>] kmem_cache_alloc+0x145/0x440 [<420093ba>] kmem_cache_create_usercopy+0x127/0x2c0 [<f5ed7ff0>] kmem_cache_create+0x16/0x20 [<4c1e4f47>] iommu_go_to_state+0x8e4/0x165d [<a191b705>] amd_iommu_prepare+0x1a/0x2f [<afe5b97e>] irq_remapping_prepare+0x35/0x6d [<209f36b5>] enable_IR_x2apic+0x2b/0x1ae [<b64491b5>] default_setup_apic_routing+0x16/0x65 [<e89c64a1>] apic_intr_mode_init+0x81/0x10a [<eaa14bf6>] x86_late_time_init+0x24/0x35 [<e291fc71>] start_kernel+0x509/0x5c7 [<99930dfe>] x86_64_start_reservations+0x24/0x26 [<b369765d>] x86_64_start_kernel+0x75/0x79 [<4f411919>] secondary_startup_64_no_verify+0xb0/0xbb > > When the IRQ domains would not be around, this would also not happen. So > my patches also change the initializtion to not do all the setup work > when amd_iommu=off was passed on the command line. > Thanks for the fix and explaination. :-) Best Regards, Ray ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix iommu remap panic while amd_iommu is set to disable
On Tue, Mar 16, 2021 at 09:16:34PM +0800, Joerg Roedel wrote: > Hi Huang, > > On Thu, Mar 11, 2021 at 10:28:07PM +0800, Huang Rui wrote: > > diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c > > index f0adbc48fd17..a08e885403b7 100644 > > --- a/drivers/iommu/amd/iommu.c > > +++ b/drivers/iommu/amd/iommu.c > > @@ -3862,7 +3862,7 @@ static int irq_remapping_select(struct irq_domain *d, > > struct irq_fwspec *fwspec, > > else if (x86_fwspec_is_hpet(fwspec)) > > devid = get_hpet_devid(fwspec->param[0]); > > > > - if (devid < 0) > > + if (devid < 0 || !amd_iommu_rlookup_table) > > return 0; > > The problem is deeper than this fix suggests. I prepared other fixes for > this particular problem. Please find them here: > Thanks for the comments. Could you please elaborate this? Do you mean while amd_iommu=off, we won't prepare the IVRS, and even needn't get all ACPI talbes. Because they are never be used and the next state will always goes into IOMMU_CMDLINE_DISABLED, am I right? Thanks, Ray > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fjoro%2Flinux.git%2Flog%2F%3Fh%3Diommu-fixes&data=04%7C01%7Cray.huang%40amd.com%7Cce731dda3b444ac9a14108d8e87dbb3e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637514974013915073%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=NVahf1tIfgno%2BNWPu8hY4DygiGWdKXBJ8G6OsD%2BHC14%3D&reserved=0 > > Regards, > > Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: Fix iommu remap panic while amd_iommu is set to disable
While amd_iommu is set to disable in cmd line, it will free the iommu
resources. Then the pages of rlookup table is freed as well. If that, we
have to check rlookup table in irq_remapping_select(), otherwise, it
will trigger a kernel panic below.
[2.245855] BUG: kernel NULL pointer dereference, address: 0500
[2.252861] #PF: supervisor read access in kernel mode
[2.258053] #PF: error_code(0x) - not-present page
[2.263247] PGD 0 P4D 0
[2.265844] Oops: [#1] SMP NOPTI
[2.269570] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.11.0-custom #1
[2.276150] Hardware name: AMD Chachani-VN/Chachani-VN, BIOS VCH162755N.FD
03/04/2021
[2.284085] RIP: 0010:irq_remapping_select+0x5c/0xb0
[2.289107] Code: 4b 0c 48 3d 70 7c c8 8f 75 0d eb 35 48 8b 00 48 3d 70 7c
c8 8f 74 2a 0f b6 50 10 39 d1 75 ed 0f b7 40 12 48 8b 15 f4 8a 3b 02 <48> 8b 14
c2 48 85 d2 74 0e b8 01 00 00 00 4c 3b a2 90 04 00 00 74
[2.307999] RSP: :8f403d40 EFLAGS: 00010246
[2.313285] RAX: 00a0 RBX: 8f403d98 RCX: 0021
[2.320471] RDX: RSI: 000a RDI: 8cbb4006118a
[2.327658] RBP: 8f403d50 R08: 0021 R09: 0002
[2.334838] R10: 000a R11: f000 R12: 8cbb401bfe40
[2.342022] R13: R14: 8cbb401be900 R15:
[2.349210] FS: () GS:8cbd73e0()
knlGS:
[2.357399] CS: 0010 DS: ES: CR0: 80050033
[2.363198] CR2: 0500 CR3: 0002dae0a000 CR4: 000406b0
[2.370382] Call Trace:
[2.372897] irq_find_matching_fwspec+0x48/0xd0
[2.377489] mp_irqdomain_create+0x7c/0x180
[2.381736] ? __raw_callee_save___native_queued_spin_unlock+0x15/0x23
[2.388320] setup_IO_APIC+0x81/0x875
[2.392048] ? clear_IO_APIC_pin+0xd6/0x130
[2.396294] ? clear_IO_APIC+0x39/0x60
[2.400103] apic_intr_mode_init+0x107/0x10a
[2.404432] x86_late_time_init+0x24/0x35
[2.408507] start_kernel+0x509/0x5c7
[2.412230] x86_64_start_reservations+0x24/0x26
[2.416911] x86_64_start_kernel+0x75/0x79
[2.421068] secondary_startup_64_no_verify+0xb0/0xbb
Fixes: a1a785b572425 ("iommu/amd: Implement select() method on remapping
irqdomain")
Tested-by: Xiaojian Du
Signed-off-by: Huang Rui
Cc: Joerg Roedel
Cc: Suravee Suthikulpanit
Cc: Alex Deucher
Cc: sta...@vger.kernel.org
---
drivers/iommu/amd/iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index f0adbc48fd17..a08e885403b7 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3862,7 +3862,7 @@ static int irq_remapping_select(struct irq_domain *d,
struct irq_fwspec *fwspec,
else if (x86_fwspec_is_hpet(fwspec))
devid = get_hpet_devid(fwspec->param[0]);
- if (devid < 0)
+ if (devid < 0 || !amd_iommu_rlookup_table)
return 0;
iommu = amd_iommu_rlookup_table[devid];
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken
On Tue, Nov 24, 2020 at 06:51:11AM +0800, Kuehling, Felix wrote: > On 2020-11-23 5:33 p.m., Will Deacon wrote: > > On Mon, Nov 23, 2020 at 09:04:14PM +, Deucher, Alexander wrote: > >> [AMD Public Use] > >> > >>> -Original Message- > >>> From: Will Deacon > >>> Sent: Monday, November 23, 2020 8:44 AM > >>> To: linux-ker...@vger.kernel.org > >>> Cc: linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; Will > >>> Deacon ; Bjorn Helgaas ; > >>> Deucher, Alexander ; Edgar Merger > >>> ; Joerg Roedel > >>> Subject: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken > >>> > >>> Edgar Merger reports that the AMD Raven GPU does not work reliably on his > >>> system when the IOMMU is enabled: > >>> > >>>| [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, > >>> signaled seq=1, emitted seq=3 > >>>| [...] > >>>| amdgpu :0b:00.0: GPU reset begin! > >>>| AMD-Vi: Completion-Wait loop timed out > >>>| iommu ivhd0: AMD-Vi: Event logged [IOTLB_INV_TIMEOUT > >>> device=0b:00.0 address=0x38edc0970] > >>> > >>> This is indicative of a hardware/platform configuration issue so, since > >>> disabling ATS has been shown to resolve the problem, add a quirk to match > >>> this particular device while Edgar follows-up with AMD for more > >>> information. > >>> > >>> Cc: Bjorn Helgaas > >>> Cc: Alex Deucher > >>> Reported-by: Edgar Merger > >>> Suggested-by: Joerg Roedel > >>> Link: > >>> https://lore. > >>> kernel.org/linux- > >>> iommu/MWHPR10MB1310F042A30661D4158520B589FC0@MWHPR10M > >>> B1310.namprd10.prod.outlook.com > >>> her%40amd.com%7C1a883fe14d0c408e7d9508d88fb5df4e%7C3dd8961fe488 > >>> 4e608e11a82d994e183d%7C0%7C0%7C637417358593629699%7CUnknown%7 > >>> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi > >>> LCJXVCI6Mn0%3D%7C1000&sdata=TMgKldWzsX8XZ0l7q3%2BszDWXQJJ > >>> LOUfX5oGaoLN8n%2B8%3D&reserved=0 > >>> Signed-off-by: Will Deacon > >>> --- > >>> > >>> Hi all, > >>> > >>> Since Joerg is away at the moment, I'm posting this to try to make some > >>> progress with the thread in the Link: tag. > >> + Felix > >> > >> What system is this? Can you provide more details? Does a sbios update > >> fix this? Disabling ATS for all Ravens will break GPU compute for a lot > >> of people. I'd prefer to just black list this particular system (e.g., > >> just SSIDs or revision) if possible. > > +Ray > > There are already many systems where the IOMMU is disabled in the BIOS, > or the CRAT table reporting the APU compute capabilities is broken. Ray > has been working on a fallback to make APUs behave like dGPUs on such > systems. That should also cover this case where ATS is blacklisted. That > said, it affects the programming model, because we don't support the > unified and coherent memory model on dGPUs like we do on APUs with > IOMMUv2. So it would be good to make the conditions for this workaround > as narrow as possible. Yes, besides the comments from Alex and Felix, may we get your firmware version (SMC firmware which is from SBIOS) and device id? > >>>| [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, > >>> signaled seq=1, emitted seq=3 It looks only gfx ib test passed, and fails to lanuch desktop, am I right? We would like to see whether it is Raven, Raven kicker (new Raven), or Picasso. In our side, per the internal test result, we didn't see the similiar issue on Raven kicker and Picasso platform. Thanks, Ray > > These are the relevant changes in KFD and Thunk for reference: > > ### KFD ### > > commit 914913ab04dfbcd0226ecb6bc99d276832ea2908 > Author: Huang Rui > Date: Tue Aug 18 14:54:23 2020 +0800 > > drm/amdkfd: implement the dGPU fallback path for apu (v6) > > We still have a few iommu issues which need to address, so force raven > as "dgpu" path for the moment. > > This is to add the fallback path to bypass IOMMU if IOMMU v2 is > disabled > or ACPI CRAT table not correct. > > v2: Use ignore_crat parameter to decide whether it will go with > IOMMUv2. > v3: Align with existed thunk, don't change the way of raven, only > renoir > will use "dgpu" path
[PATCH] iommu/amd: fix the left value check of cmd buffer
The generic command buffer entry is 128 bits (16 bytes), so the offset
of tail and head pointer should be 16 bytes aligned and increased with
0x10 per command.
When cmd buf is full, head = (tail + 0x10) % CMD_BUFFER_SIZE.
So when left space of cmd buf should be able to store only two
command, we should be issued one COMPLETE_WAIT additionally to wait
all older commands completed. Then the left space should be increased
after IOMMU fetching from cmd buf.
So left check value should be left <= 0x20 (two commands).
Signed-off-by: Huang Rui
---
Hi Joerg,
The command entry is 128 bits (16 bytes aligned), so the left value
shouldn't be 2. I think here, you might check the case when the buf is
full, so modify the value as the two commands left. Please review.
Thanks,
Rui
---
drivers/iommu/amd_iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 019e027..3ef0f42 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1023,7 +1023,7 @@ static int __iommu_queue_command_sync(struct amd_iommu
*iommu,
next_tail = (tail + sizeof(*cmd)) % CMD_BUFFER_SIZE;
left = (head - next_tail) % CMD_BUFFER_SIZE;
- if (left <= 2) {
+ if (left <= 0x20) {
struct iommu_cmd sync_cmd;
int ret;
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/9] ACPI: Add support for AMBA bus type
On Fri, Dec 04, 2015 at 10:50:23AM +0200, Mika Westerberg wrote: > On Fri, Dec 04, 2015 at 11:24:18AM +0800, Wang Hongcheng wrote: > > From: Huang Rui > > > > Inspired by acpi platform bus type, to make driver "porting" more > > straightforward, this patch introduces ACPI support to the AMBA bus > > type. Instead of writing ACPI "glue" drivers for the exiting AMBA > > drivers. > > Hmm, isn't there already similar patch series bringing AMBA ACPI > support? > > https://lkml.org/lkml/2015/9/30/394 Err, I see the patch for the first time... But looks like it isn't accepted till now. Will he continue to work on it? Thanks, Rui ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/9] ACPI: Add support for AMBA bus type
On Fri, Dec 04, 2015 at 09:59:28AM +, G Gregory wrote: > On 4 December 2015 at 09:42, Hanjun Guo wrote: > > On 2015/12/4 17:17, Huang Rui wrote: > >> On Fri, Dec 04, 2015 at 10:50:23AM +0200, Mika Westerberg wrote: > >>> On Fri, Dec 04, 2015 at 11:24:18AM +0800, Wang Hongcheng wrote: > >>>> From: Huang Rui > >>>> > >>>> Inspired by acpi platform bus type, to make driver "porting" more > >>>> straightforward, this patch introduces ACPI support to the AMBA bus > >>>> type. Instead of writing ACPI "glue" drivers for the exiting AMBA > >>>> drivers. > >>> Hmm, isn't there already similar patch series bringing AMBA ACPI > >>> support? > >>> > >>> https://lkml.org/lkml/2015/9/30/394 > >> Err, I see the patch for the first time... > >> > >> But looks like it isn't accepted till now. Will he continue to work on > >> it? > > > > Sure, Graeme is working on the updated version now :) > > > Yes, as Hanjun said I am planning a v2. I've just been caught up in > other business this week and probably next week. > OK, thanks to clarify it. Could you please take care of AMD use case that we need a configurable fix_rate clk, periphid, and platform data? Thanks, Rui ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
