Skip to content

Commit

Permalink
iommu/amd: Simplify and Consolidate Virtual APIC (AVIC) Enablement
Browse files Browse the repository at this point in the history
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 <jsnitsel@redhat.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Maxim Levitsky <mlevitsk@redhat.com>
Cc: Will Deacon <will@kernel.org> (maintainer:IOMMU DRIVERS)
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Link: https://lore.kernel.org/r/20220726134348.6438-2-suravee.suthikulpanit@amd.com
Signed-off-by: Joerg Roedel <jroedel@suse.de>
(backported from commit c5e1a1e)
[ghadi-rahme: Had to adjust the context due to missing commits]
Signed-off-by: Ghadi Elie Rahme <ghadi.rahme@canonical.com>
Acked-by: Guoqing Jiang <guoqing.jiang@canonical.com>
Acked-by: Ivan Hu <ivan.hu@canonical.com>
Signed-off-by: Roxana Nicolescu <roxana.nicolescu@canonical.com>
  • Loading branch information
ssuthiku-amd authored and mehmetb0 committed Nov 8, 2024
1 parent 29936d3 commit 36e6afc
Showing 1 changed file with 56 additions and 30 deletions.
86 changes: 56 additions & 30 deletions drivers/iommu/amd/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -825,11 +825,6 @@ static int iommu_ga_log_enable(struct amd_iommu *iommu)
if (!iommu->ga_log)
return -EINVAL;

/* Check if already running */
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
if (WARN_ON(status & (MMIO_STATUS_GALOG_RUN_MASK)))
return 0;

entry = iommu_virt_to_phys(iommu->ga_log) | GA_LOG_SIZE_512;
memcpy_toio(iommu->mmio_base + MMIO_GA_LOG_BASE_OFFSET,
&entry, sizeof(entry));
Expand Down Expand Up @@ -1915,10 +1910,6 @@ static int __init iommu_init_pci(struct amd_iommu *iommu)
if (iommu_feature(iommu, FEATURE_PPR) && alloc_ppr_log(iommu))
return -ENOMEM;

ret = iommu_init_ga_log(iommu);
if (ret)
return ret;

if (iommu->cap & (1UL << IOMMU_CAP_NPCACHE)) {
pr_info("Using strict mode due to virtualization\n");
iommu_set_dma_strict();
Expand Down Expand Up @@ -1995,8 +1986,6 @@ static void print_iommu_info(void)
}
if (irq_remapping_enabled) {
pr_info("Interrupt remapping enabled\n");
if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
pr_info("Virtual APIC enabled\n");
if (amd_iommu_xt_mode == IRQ_REMAP_X2APIC_MODE)
pr_info("X2APIC enabled\n");
}
Expand Down Expand Up @@ -2277,9 +2266,6 @@ static int iommu_init_irq(struct amd_iommu *iommu)

if (iommu->ppr_log != NULL)
iommu_feature_enable(iommu, CONTROL_PPRINT_EN);

iommu_ga_log_enable(iommu);

return 0;
}

Expand Down Expand Up @@ -2485,8 +2471,6 @@ static void iommu_enable_ga(struct amd_iommu *iommu)
#ifdef CONFIG_IRQ_REMAP
switch (amd_iommu_guest_ir) {
case AMD_IOMMU_GUEST_IR_VAPIC:
iommu_feature_enable(iommu, CONTROL_GAM_EN);
fallthrough;
case AMD_IOMMU_GUEST_IR_LEGACY_GA:
iommu_feature_enable(iommu, CONTROL_GA_EN);
iommu->irte_ops = &irte_128_ops;
Expand Down Expand Up @@ -2557,19 +2541,6 @@ static void early_enable_iommus(void)
iommu_flush_all_caches(iommu);
}
}

#ifdef CONFIG_IRQ_REMAP
/*
* Note: We have already checked GASup from IVRS table.
* Now, we need to make sure that GAMSup is set.
*/
if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) &&
!check_feature_on_all_iommus(FEATURE_GAM_VAPIC))
amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;

if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
amd_iommu_irq_ops.capability |= (1 << IRQ_POSTING_CAP);
#endif
}

static void enable_iommus_v2(void)
Expand All @@ -2582,10 +2553,64 @@ static void enable_iommus_v2(void)
}
}

static void enable_iommus_vapic(void)
{
#ifdef CONFIG_IRQ_REMAP
u32 status, i;
struct amd_iommu *iommu;

for_each_iommu(iommu) {
/*
* Disable GALog if already running. It could have been enabled
* in the previous boot before kdump.
*/
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
if (!(status & MMIO_STATUS_GALOG_RUN_MASK))
continue;

iommu_feature_disable(iommu, CONTROL_GALOG_EN);
iommu_feature_disable(iommu, CONTROL_GAINT_EN);

/*
* Need to set and poll check the GALOGRun bit to zero before
* we can set/ modify GA Log registers safely.
*/
for (i = 0; i < LOOP_TIMEOUT; ++i) {
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
if (!(status & MMIO_STATUS_GALOG_RUN_MASK))
break;
udelay(10);
}

if (WARN_ON(i >= LOOP_TIMEOUT))
return;
}

if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) &&
!check_feature_on_all_iommus(FEATURE_GAM_VAPIC)) {
amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;
return;
}

/* Enabling GAM support */
for_each_iommu(iommu) {
if (iommu_init_ga_log(iommu) ||
iommu_ga_log_enable(iommu))
return;

iommu_feature_enable(iommu, CONTROL_GAM_EN);
}

amd_iommu_irq_ops.capability |= (1 << IRQ_POSTING_CAP);
pr_info("Virtual APIC enabled\n");
#endif
}


static void enable_iommus(void)
{
early_enable_iommus();

enable_iommus_vapic();
enable_iommus_v2();
}

Expand Down Expand Up @@ -2983,6 +3008,7 @@ static int __init state_next(void)
register_syscore_ops(&amd_iommu_syscore_ops);
ret = amd_iommu_init_pci();
init_state = ret ? IOMMU_INIT_ERROR : IOMMU_PCI_INIT;
enable_iommus_vapic();
enable_iommus_v2();
break;
case IOMMU_PCI_INIT:
Expand Down

0 comments on commit 36e6afc

Please sign in to comment.