Commit c5e1a1eb authored by Suravee Suthikulpanit's avatar Suravee Suthikulpanit Committed by Joerg Roedel
Browse files

iommu/amd: Simplify and Consolidate Virtual APIC (AVIC) Enablement 



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 a8d4a37d ("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 a8d4a37d ("iommu/amd: Restore GA log/tail pointer on host resume")
Reported-by: default avatarJerry 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: default avatarSuravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Reviewed-by: default avatarJerry Snitselaar <jsnitsel@redhat.com>
Link: https://lore.kernel.org/r/20220726134348.6438-2-suravee.suthikulpanit@amd.com


Signed-off-by: default avatarJoerg Roedel <jroedel@suse.de>
parent 30315e71

drivers/iommu/amd/init.c

+55 −30
Original line number Diff line number Diff line
@@ -908,11 +908,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));
@@ -2068,10 +2063,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();
@@ -2155,8 +2146,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");

	}
@@ -2446,9 +2435,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;

}
@@ -2678,8 +2664,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;
@@ -2759,19 +2743,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)
@@ -2784,10 +2755,63 @@ 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();

}
@@ -3126,6 +3150,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: