Skip to content

Commit 36e6afc

Browse files
ssuthiku-amdmehmetb0
authored andcommitted
iommu/amd: Simplify and Consolidate Virtual APIC (AVIC) Enablement
BugLink: https://bugs.launchpad.net/bugs/2080378 Currently, enabling AVIC requires individually detect and enable GAM and GALOG features on each IOMMU, which is difficult to keep track on multi-IOMMU system, where the features needs to be enabled system-wide. In addition, these features do not need to be enabled in early stage. It can be delayed until after amd_iommu_init_pci(). Therefore, consolidate logic for detecting and enabling IOMMU GAM and GALOG features into a helper function, enable_iommus_vapic(), which uses the check_feature_on_all_iommus() helper function to ensure system-wide support of the features before enabling them, and postpone until after amd_iommu_init_pci(). The new function also check and clean up feature enablement residue from previous boot (e.g. in case of booting into kdump kernel), which triggers a WARN_ON (shown below) introduced by the commit a8d4a37 ("iommu/amd: Restore GA log/tail pointer on host resume") in iommu_ga_log_enable(). [ 7.731955] ------------[ cut here ]------------ [ 7.736575] WARNING: CPU: 0 PID: 1 at drivers/iommu/amd/init.c:829 iommu_ga_log_enable.isra.0+0x16f/0x190 [ 7.746135] Modules linked in: [ 7.749193] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W -------- --- 5.19.0-0.rc7.53.eln120.x86_64 #1 [ 7.759706] Hardware name: Dell Inc. PowerEdge R7525/04D5GJ, BIOS 2.1.6 03/09/2021 [ 7.767274] RIP: 0010:iommu_ga_log_enable.isra.0+0x16f/0x190 [ 7.772931] Code: 20 20 00 00 8b 00 f6 c4 01 74 da 48 8b 44 24 08 65 48 2b 04 25 28 00 00 00 75 13 48 83 c4 10 5b 5d e9 f5 00 72 00 0f 0b eb e1 <0f> 0b eb dd e8 f8 66 42 00 48 8b 15 f1 85 53 01 e9 29 ff ff ff 48 [ 7.791679] RSP: 0018:ffffc90000107d20 EFLAGS: 00010206 [ 7.796905] RAX: ffffc90000780000 RBX: 0000000000000100 RCX: ffffc90000780000 [ 7.804038] RDX: 0000000000000001 RSI: ffffc90000780000 RDI: ffff8880451f9800 [ 7.811170] RBP: ffff8880451f9800 R08: ffffffffffffffff R09: 0000000000000000 [ 7.818303] R10: 0000000000000000 R11: 0000000000000000 R12: 0008000000000000 [ 7.825435] R13: ffff8880462ea900 R14: 0000000000000021 R15: 0000000000000000 [ 7.832572] FS: 0000000000000000(0000) GS:ffff888054a00000(0000) knlGS:0000000000000000 [ 7.840657] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 7.846400] CR2: ffff888054dff000 CR3: 0000000053210000 CR4: 0000000000350eb0 [ 7.853533] Call Trace: [ 7.855979] <TASK> [ 7.858085] amd_iommu_enable_interrupts+0x180/0x270 [ 7.863051] ? iommu_setup+0x271/0x271 [ 7.866803] state_next+0x197/0x2c0 [ 7.870295] ? iommu_setup+0x271/0x271 [ 7.874049] iommu_go_to_state+0x24/0x2c [ 7.877976] amd_iommu_init+0xf/0x29 [ 7.881554] pci_iommu_init+0xe/0x36 [ 7.885133] do_one_initcall+0x44/0x200 [ 7.888975] do_initcalls+0xc8/0xe1 [ 7.892466] kernel_init_freeable+0x14c/0x199 [ 7.896826] ? rest_init+0xd0/0xd0 [ 7.900231] kernel_init+0x16/0x130 [ 7.903723] ret_from_fork+0x22/0x30 [ 7.907306] </TASK> [ 7.909497] ---[ end trace 0000000000000000 ]--- Fixes: commit a8d4a37 ("iommu/amd: Restore GA log/tail pointer on host resume") Reported-by: Jerry Snitselaar <[email protected]> Cc: Joerg Roedel <[email protected]> Cc: Maxim Levitsky <[email protected]> Cc: Will Deacon <[email protected]> (maintainer:IOMMU DRIVERS) Signed-off-by: Suravee Suthikulpanit <[email protected]> Reviewed-by: Jerry Snitselaar <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Joerg Roedel <[email protected]> (backported from commit c5e1a1e) [ghadi-rahme: Had to adjust the context due to missing commits] Signed-off-by: Ghadi Elie Rahme <[email protected]> Acked-by: Guoqing Jiang <[email protected]> Acked-by: Ivan Hu <[email protected]> Signed-off-by: Roxana Nicolescu <[email protected]>
1 parent 29936d3 commit 36e6afc

File tree

1 file changed

+56
-30
lines changed

1 file changed

+56
-30
lines changed

drivers/iommu/amd/init.c

Lines changed: 56 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -825,11 +825,6 @@ static int iommu_ga_log_enable(struct amd_iommu *iommu)
825825
if (!iommu->ga_log)
826826
return -EINVAL;
827827

828-
/* Check if already running */
829-
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
830-
if (WARN_ON(status & (MMIO_STATUS_GALOG_RUN_MASK)))
831-
return 0;
832-
833828
entry = iommu_virt_to_phys(iommu->ga_log) | GA_LOG_SIZE_512;
834829
memcpy_toio(iommu->mmio_base + MMIO_GA_LOG_BASE_OFFSET,
835830
&entry, sizeof(entry));
@@ -1915,10 +1910,6 @@ static int __init iommu_init_pci(struct amd_iommu *iommu)
19151910
if (iommu_feature(iommu, FEATURE_PPR) && alloc_ppr_log(iommu))
19161911
return -ENOMEM;
19171912

1918-
ret = iommu_init_ga_log(iommu);
1919-
if (ret)
1920-
return ret;
1921-
19221913
if (iommu->cap & (1UL << IOMMU_CAP_NPCACHE)) {
19231914
pr_info("Using strict mode due to virtualization\n");
19241915
iommu_set_dma_strict();
@@ -1995,8 +1986,6 @@ static void print_iommu_info(void)
19951986
}
19961987
if (irq_remapping_enabled) {
19971988
pr_info("Interrupt remapping enabled\n");
1998-
if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
1999-
pr_info("Virtual APIC enabled\n");
20001989
if (amd_iommu_xt_mode == IRQ_REMAP_X2APIC_MODE)
20011990
pr_info("X2APIC enabled\n");
20021991
}
@@ -2277,9 +2266,6 @@ static int iommu_init_irq(struct amd_iommu *iommu)
22772266

22782267
if (iommu->ppr_log != NULL)
22792268
iommu_feature_enable(iommu, CONTROL_PPRINT_EN);
2280-
2281-
iommu_ga_log_enable(iommu);
2282-
22832269
return 0;
22842270
}
22852271

@@ -2485,8 +2471,6 @@ static void iommu_enable_ga(struct amd_iommu *iommu)
24852471
#ifdef CONFIG_IRQ_REMAP
24862472
switch (amd_iommu_guest_ir) {
24872473
case AMD_IOMMU_GUEST_IR_VAPIC:
2488-
iommu_feature_enable(iommu, CONTROL_GAM_EN);
2489-
fallthrough;
24902474
case AMD_IOMMU_GUEST_IR_LEGACY_GA:
24912475
iommu_feature_enable(iommu, CONTROL_GA_EN);
24922476
iommu->irte_ops = &irte_128_ops;
@@ -2557,19 +2541,6 @@ static void early_enable_iommus(void)
25572541
iommu_flush_all_caches(iommu);
25582542
}
25592543
}
2560-
2561-
#ifdef CONFIG_IRQ_REMAP
2562-
/*
2563-
* Note: We have already checked GASup from IVRS table.
2564-
* Now, we need to make sure that GAMSup is set.
2565-
*/
2566-
if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) &&
2567-
!check_feature_on_all_iommus(FEATURE_GAM_VAPIC))
2568-
amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;
2569-
2570-
if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
2571-
amd_iommu_irq_ops.capability |= (1 << IRQ_POSTING_CAP);
2572-
#endif
25732544
}
25742545

25752546
static void enable_iommus_v2(void)
@@ -2582,10 +2553,64 @@ static void enable_iommus_v2(void)
25822553
}
25832554
}
25842555

2556+
static void enable_iommus_vapic(void)
2557+
{
2558+
#ifdef CONFIG_IRQ_REMAP
2559+
u32 status, i;
2560+
struct amd_iommu *iommu;
2561+
2562+
for_each_iommu(iommu) {
2563+
/*
2564+
* Disable GALog if already running. It could have been enabled
2565+
* in the previous boot before kdump.
2566+
*/
2567+
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
2568+
if (!(status & MMIO_STATUS_GALOG_RUN_MASK))
2569+
continue;
2570+
2571+
iommu_feature_disable(iommu, CONTROL_GALOG_EN);
2572+
iommu_feature_disable(iommu, CONTROL_GAINT_EN);
2573+
2574+
/*
2575+
* Need to set and poll check the GALOGRun bit to zero before
2576+
* we can set/ modify GA Log registers safely.
2577+
*/
2578+
for (i = 0; i < LOOP_TIMEOUT; ++i) {
2579+
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
2580+
if (!(status & MMIO_STATUS_GALOG_RUN_MASK))
2581+
break;
2582+
udelay(10);
2583+
}
2584+
2585+
if (WARN_ON(i >= LOOP_TIMEOUT))
2586+
return;
2587+
}
2588+
2589+
if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) &&
2590+
!check_feature_on_all_iommus(FEATURE_GAM_VAPIC)) {
2591+
amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;
2592+
return;
2593+
}
2594+
2595+
/* Enabling GAM support */
2596+
for_each_iommu(iommu) {
2597+
if (iommu_init_ga_log(iommu) ||
2598+
iommu_ga_log_enable(iommu))
2599+
return;
2600+
2601+
iommu_feature_enable(iommu, CONTROL_GAM_EN);
2602+
}
2603+
2604+
amd_iommu_irq_ops.capability |= (1 << IRQ_POSTING_CAP);
2605+
pr_info("Virtual APIC enabled\n");
2606+
#endif
2607+
}
2608+
2609+
25852610
static void enable_iommus(void)
25862611
{
25872612
early_enable_iommus();
2588-
2613+
enable_iommus_vapic();
25892614
enable_iommus_v2();
25902615
}
25912616

@@ -2983,6 +3008,7 @@ static int __init state_next(void)
29833008
register_syscore_ops(&amd_iommu_syscore_ops);
29843009
ret = amd_iommu_init_pci();
29853010
init_state = ret ? IOMMU_INIT_ERROR : IOMMU_PCI_INIT;
3011+
enable_iommus_vapic();
29863012
enable_iommus_v2();
29873013
break;
29883014
case IOMMU_PCI_INIT:

0 commit comments

Comments
 (0)