PerfOpt is only a feature usable by integrated GPUs and only in identity mode. Instead of leaving a policy knob in amdgpu, just turn it on when an integrated GPU is in identity.
This drops quite a bit of compatibility glue. There was a refcounting system, exported symbols, and device attach/detach logic. By just setting it immediately it's a lot more straightforward. Suggested-by: Jason Gunthorpe <[email protected]> Signed-off-by: Mario Limonciello (AMD) <[email protected]> --- v2: * Move WARN_ON to amd_iommu_perfopt_clear() * Make amd_iommu_perfopt_clear() void * Drop unnecessary cleanup/create paths that unset feature * Enable the feature in IOMMU if supported (per guidance in IVRS spec) * Use existing check_feature() helper instead --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 50 ----- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 - drivers/iommu/amd/amd_iommu.h | 3 +- drivers/iommu/amd/amd_iommu_types.h | 3 - drivers/iommu/amd/init.c | 50 +---- drivers/iommu/amd/iommu.c | 243 +++------------------ include/linux/amd-iommu.h | 11 - 8 files changed, 40 insertions(+), 333 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 0e53a02ad1bab..a9c6f5d4a6397 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -156,7 +156,6 @@ struct amdgpu_watchdog_timer { * Modules parameters. */ extern int amdgpu_modeset; -extern int amdgpu_iommu_perfopt; extern unsigned int amdgpu_vram_limit; extern int amdgpu_vis_vram_limit; extern int amdgpu_gart_size; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index bfe4d90343c4e..9269e780feb73 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -33,7 +33,6 @@ #include <linux/console.h> #include <linux/slab.h> #include <linux/iommu.h> -#include <linux/amd-iommu.h> #include <linux/pci.h> #include <linux/pci-p2pdma.h> #include <linux/apple-gmux.h> @@ -3753,28 +3752,6 @@ amdgpu_device_should_register_switcheroo(struct amdgpu_device *adev, bool px) apple_gmux_detect(NULL, NULL))); } -static inline bool amdgpu_device_identity(struct amdgpu_device *adev) -{ - struct pci_dev *pdev = adev->pdev; - struct iommu_domain *domain = iommu_get_domain_for_dev(&pdev->dev); - - if (!domain) - return false; - - return domain->type == IOMMU_DOMAIN_IDENTITY; -} - -static bool amdgpu_device_use_perfopt(struct amdgpu_device *adev) -{ - if (amdgpu_iommu_perfopt == 0) - return false; - - if (!(adev->flags & AMD_IS_APU)) - return false; - - return amdgpu_device_identity(adev); -} - /** * amdgpu_device_init - initialize the driver * @@ -3982,16 +3959,6 @@ int amdgpu_device_init(struct amdgpu_device *adev, if (r) return r; - if (amdgpu_device_use_perfopt(adev)) { - int perfopt_ret = amd_iommu_enable_perfopt(pdev); - - /* Optional optimization; a failure to arm it must not abort probe. */ - if (perfopt_ret) - dev_warn(adev->dev, - "Failed to enable IOMMU PerfOpt (%d); continuing without it\n", - perfopt_ret); - } - /* * No need to remove conflicting FBs for non-display class devices. * This prevents the sysfb from being freed accidently. @@ -4361,9 +4328,6 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev) amdgpu_gart_dummy_page_fini(adev); - if (amdgpu_device_use_perfopt(adev)) - amd_iommu_disable_perfopt(adev->pdev); - if (pci_dev_is_disconnected(adev->pdev)) amdgpu_device_unmap_mmio(adev); @@ -4727,20 +4691,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool notify_clients) if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) return 0; - if (amdgpu_device_use_perfopt(adev)) { - int perfopt_ret = amd_iommu_enable_perfopt(adev->pdev); - - /* - * Must not return on failure: a bare return would leak the - * SR-IOV VF exclusive-mode acquisition taken above (released - * via the exit: path). - */ - if (perfopt_ret) - dev_warn(adev->dev, - "Failed to enable IOMMU PerfOpt (%d); continuing without it\n", - perfopt_ret); - } - if (adev->in_s0ix) amdgpu_dpm_gfx_state_change(adev, sGpuChangeState_D0Entry); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 3ca98091ce487..5c33c19fd9bc5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -185,7 +185,6 @@ char *amdgpu_disable_cu; char *amdgpu_virtual_display; int amdgpu_enforce_isolation = -1; int amdgpu_modeset = -1; -int amdgpu_iommu_perfopt = -1; /* Specifies the default granularity for SVM, used in buffer * migration and restoration of backing memory when handling @@ -393,17 +392,6 @@ module_param_named(fw_load_type, amdgpu_fw_load_type, int, 0444); MODULE_PARM_DESC(aspm, "ASPM support (1 = enable, 0 = disable, -1 = auto)"); module_param_named(aspm, amdgpu_aspm, int, 0444); -/** - * DOC: iommu_perfopt (int) - * Control the AMD IOMMU PerfOpt DMA-latency optimization - * (0 = disable; -1 = enable on supported devices). - * This arms the IOMMU PerfOpt control (IOMMU spec, MMIO Offset 016Ch, EFR PerfOptSup / PerfOptEn) - * Arming it disables ATS, PRI, PASID and SVA for the GPU and removes IOMMU DMA containment for it, - * trading isolation for lower DMA latency. - */ -MODULE_PARM_DESC(iommu_perfopt, "Control IOMMU PerfOpt DMA-latency optimization (-1 = enable on supported devices, 0 = disable)"); -module_param_named(iommu_perfopt, amdgpu_iommu_perfopt, int, 0444); - /** * DOC: runpm (int) * Override for runtime power management control for dGPUs. The amdgpu driver can dynamically power down diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h index 93845b3c3fbf7..29355810db7c3 100644 --- a/drivers/iommu/amd/amd_iommu.h +++ b/drivers/iommu/amd/amd_iommu.h @@ -48,8 +48,7 @@ extern u8 amd_iommu_hpt_vasize; extern unsigned long amd_iommu_pgsize_bitmap; extern bool amd_iommu_hatdis; -int amd_iommu_perfopt_clear(struct amd_iommu *iommu); -int amd_iommu_perfopt_restore(struct amd_iommu *iommu); +int amd_iommu_perfopt_set(struct amd_iommu *iommu, bool enable); /* Protection domain ops */ void amd_iommu_init_identity_domain(void); diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h index 1c1624b7c9a36..9d873819304fb 100644 --- a/drivers/iommu/amd/amd_iommu_types.h +++ b/drivers/iommu/amd/amd_iommu_types.h @@ -657,9 +657,6 @@ struct amd_iommu { /* Extended features 2 */ u64 features2; - /* Devices requesting PerfOpt; the shared PERF_OPT_EN bit is on while >0. Protected by @lock. */ - int perfopt_refcount; - /* PCI device id of the IOMMU device */ u16 devid; diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c index 8ec8a6fccaaf3..36e0bd4c713a5 100644 --- a/drivers/iommu/amd/init.c +++ b/drivers/iommu/amd/init.c @@ -451,6 +451,9 @@ static void iommu_disable(struct amd_iommu *iommu) /* Clear IRTE cache disabling bit */ iommu_feature_disable(iommu, CONTROL_IRTCACHEDIS); + + /* Clear PerfOpt feature */ + amd_iommu_perfopt_set(iommu, FALSE); } /* @@ -1943,9 +1946,6 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h, if (!iommu->mmio_base) return -ENOMEM; - if (amd_iommu_perfopt_clear(iommu)) - pr_err("IOMMU%d: failed to clear PerfOpt\n", iommu->index); - return init_iommu_from_acpi(iommu, h); } @@ -2314,6 +2314,9 @@ static void print_iommu_info(void) if (check_feature2(FEATURE_SEVSNPIO_SUP)) pr_cont(" SEV-TIO"); + if (check_feature(FEATURE_PERF_OPT)) + pr_cont(" PerfOpt"); + pr_cont("\n"); } @@ -2921,6 +2924,7 @@ static void early_enable_iommu(struct amd_iommu *iommu) iommu_enable_xt(iommu); iommu_enable_irtcachedis(iommu); iommu_enable_2k_int(iommu); + amd_iommu_perfopt_set(iommu, TRUE); iommu_enable(iommu); amd_iommu_flush_all_caches(iommu); } @@ -3050,46 +3054,10 @@ static void enable_iommus_vapic(void) #endif } -static int clear_perfopt_all(void) -{ - struct amd_iommu *iommu; - int err, ret = 0; - - for_each_iommu(iommu) { - err = amd_iommu_perfopt_clear(iommu); - if (err) - ret = err; - } - - return ret; -} - -static int restore_perfopt_all(void) -{ - struct amd_iommu *iommu; - int err, ret = 0; - - for_each_iommu(iommu) { - err = amd_iommu_perfopt_restore(iommu); - if (err) - ret = err; - } - - return ret; -} - static void disable_iommus(void) { struct amd_iommu *iommu; - /* - * PerfOpt is an optional performance bit, so a failure to clear it must - * not skip the mandatory disable below. This also runs from the void - * amd_iommu_disable() shutdown/kexec path, which cannot report an error. - */ - if (clear_perfopt_all()) - pr_err("Failed to clear PerfOpt while disabling IOMMUs\n"); - for_each_iommu(iommu) iommu_disable(iommu); @@ -3115,10 +3083,6 @@ static void amd_iommu_resume(void *data) for_each_iommu(iommu) early_enable_iommu(iommu); - /* early_enable_iommu() cleared PERF_OPT_EN; re-assert it from the refcount. */ - if (restore_perfopt_all()) - pr_err("Failed to restore PerfOpt after IOMMU resume\n"); - iommu_enable_event_buffer(); amd_iommu_enable_interrupts(); } diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c index c9b28e5e582ea..a1adb40d5b79d 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -2345,6 +2345,35 @@ static void pdom_detach_iommu(struct amd_iommu *iommu, spin_unlock_irqrestore(&pdom->lock, flags); } +/* + * Program or clear the per-IOMMU PerfOpt enable bit + * This allows a privileged internal GPU to be able to bypass IOMMU when + * in identity mode. + */ +int amd_iommu_perfopt_set(struct amd_iommu *iommu, bool enable) +{ + u32 old, val, readback; + + if (!check_feature(FEATURE_PERF_OPT)) + return enable ? -ENODEV : 0; + + old = readl(iommu->mmio_base + MMIO_PERF_OPT_OFFSET); + if (old == U32_MAX) + return -EIO; + + val = enable ? old | PERF_OPT_EN : old & ~PERF_OPT_EN; + if (val != old) + writel(val, iommu->mmio_base + MMIO_PERF_OPT_OFFSET); + readback = readl(iommu->mmio_base + MMIO_PERF_OPT_OFFSET); + if (readback == U32_MAX || + (readback & PERF_OPT_EN) != (val & PERF_OPT_EN)) { + dev_err(iommu->iommu.dev, "Failed to set PerfOpt to %d\n", enable); + return -EIO; + } + + return 0; +} + /* * If a device is not yet associated with a domain, this function makes the * device visible in the domain @@ -2406,7 +2435,6 @@ static int attach_device(struct device *dev, /* Update device table */ dev_update_dte(dev_data, true); - out: mutex_unlock(&dev_data->mutex); @@ -2512,192 +2540,6 @@ static int iommu_init_device_caps(struct iommu_dev_data *dev_data, return 0; } -/* Program the per-IOMMU PerfOpt enable bit. Caller must hold iommu->lock. */ -static int __perfopt_write(struct amd_iommu *iommu, bool enable) -{ - u32 old, val, readback; - - if (!(readq(iommu->mmio_base + MMIO_EXT_FEATURES) & FEATURE_PERF_OPT)) - return enable ? -ENODEV : 0; - - old = readl(iommu->mmio_base + MMIO_PERF_OPT_OFFSET); - if (old == U32_MAX) - return -EIO; - - val = enable ? old | PERF_OPT_EN : old & ~PERF_OPT_EN; - if (val != old) - writel(val, iommu->mmio_base + MMIO_PERF_OPT_OFFSET); - readback = readl(iommu->mmio_base + MMIO_PERF_OPT_OFFSET); - if (readback == U32_MAX || - (readback & PERF_OPT_EN) != (val & PERF_OPT_EN)) - return -EIO; - return 0; -} - -/* - * PERF_OPT_EN is a single bit shared by every device behind @iommu, so it is - * reference counted: armed on the first requesting device, cleared on the last. - */ -static int perfopt_get(struct amd_iommu *iommu) -{ - unsigned long flags; - int ret = 0; - - if (!iommu->mmio_base) - return 0; - - raw_spin_lock_irqsave(&iommu->lock, flags); - if (iommu->perfopt_refcount == 0) { - ret = __perfopt_write(iommu, true); - if (ret) - goto out; - } - iommu->perfopt_refcount++; -out: - raw_spin_unlock_irqrestore(&iommu->lock, flags); - return ret; -} - -static int perfopt_put(struct amd_iommu *iommu) -{ - unsigned long flags; - int ret = 0; - - if (!iommu->mmio_base) - return 0; - - raw_spin_lock_irqsave(&iommu->lock, flags); - if (iommu->perfopt_refcount > 0 && --iommu->perfopt_refcount == 0) - ret = __perfopt_write(iommu, false); - raw_spin_unlock_irqrestore(&iommu->lock, flags); - return ret; -} - -/* - * Force PERF_OPT_EN off without touching the refcount (used on init, shutdown, - * and suspend). The count is preserved so amd_iommu_perfopt_restore() can - * re-arm on resume. - */ -int amd_iommu_perfopt_clear(struct amd_iommu *iommu) -{ - unsigned long flags; - int ret; - - if (!iommu->mmio_base) - return 0; - - raw_spin_lock_irqsave(&iommu->lock, flags); - ret = __perfopt_write(iommu, false); - raw_spin_unlock_irqrestore(&iommu->lock, flags); - return ret; -} - -/* - * Re-assert PERF_OPT_EN from the refcount after the hardware was reprogrammed on - * resume, so devices armed before suspend keep the optimization without each - * consumer driver re-arming. - */ -int amd_iommu_perfopt_restore(struct amd_iommu *iommu) -{ - unsigned long flags; - int ret; - - if (!iommu->mmio_base) - return 0; - - raw_spin_lock_irqsave(&iommu->lock, flags); - ret = __perfopt_write(iommu, iommu->perfopt_refcount > 0); - raw_spin_unlock_irqrestore(&iommu->lock, flags); - return ret; -} - -int amd_iommu_enable_perfopt(struct pci_dev *pdev) -{ - struct iommu_dev_data *dev_data = dev_iommu_priv_get(&pdev->dev); - struct amd_iommu *iommu = rlookup_amd_iommu(&pdev->dev); - struct protection_domain *domain; - int ret; - - if (!iommu || !dev_data) - return -ENODEV; - - if (!(iommu->features & FEATURE_PERF_OPT)) - return -ENODEV; - - domain = dev_data->domain; - if (!domain) - return -ENODEV; - - /* Already armed for this device (e.g. re-entry on resume). */ - if (dev_data->perfopt) - return 0; - - /* - * The bit is only architecturally valid while the device is untranslated: - * identity domain with ATS/PRI/PASID off. The identity domain is - * SVA-capable so attach_device() enabled ATS/PRI/PASID and built a GCR3 - * table. Re-home the device onto the same identity domain with - * perfopt set, so the attach_device() skip_caps path leaves - * ATS/PRI/PASID off and no GCR3 table. This follows the detach/attach - * pattern used by amd_iommu_attach_device(). - * - * Locking: this and amd_iommu_disable_perfopt() run only from the - * consumer driver's bind/unbind path. group->mutex is not exposed to - * drivers, but a device bound to its native driver cannot have its domain - * changed concurrently by the core (VFIO ownership is mutually exclusive; - * sysfs domain changes require an unused group), so the detach/attach pair - * is serialized without it. - */ - dev_data->perfopt = true; - detach_device(&pdev->dev); - ret = attach_device(&pdev->dev, domain); - if (ret) - goto err_restore; - - ret = perfopt_get(iommu); - if (ret) - goto err_rearm; - - dev_info_once(&pdev->dev, "PerfOpt armed on IOMMU%d\n", iommu->index); - return 0; - -err_rearm: - detach_device(&pdev->dev); -err_restore: - dev_data->perfopt = false; - if (attach_device(&pdev->dev, domain)) - pci_err(pdev, "failed to restore state after PerfOpt setup; device left detached\n"); - dev_err_once(&pdev->dev, "PerfOpt failed to arm on IOMMU%d (%d)\n", - iommu->index, ret); - return ret; -} -EXPORT_SYMBOL_GPL(amd_iommu_enable_perfopt); - -void amd_iommu_disable_perfopt(struct pci_dev *pdev) -{ - struct iommu_dev_data *dev_data = dev_iommu_priv_get(&pdev->dev); - struct amd_iommu *iommu = rlookup_amd_iommu(&pdev->dev); - struct protection_domain *domain; - - if (!iommu || !dev_data || !dev_data->perfopt || !dev_data->domain) - return; - - if (WARN_ON(perfopt_put(iommu))) - pci_err(pdev, "failed to clear PerfOpt\n"); - - /* - * Restore ATS/PRI/PASID (and thus SVA) by re-homing the device onto its - * identity domain with the flag cleared, so a later bind without PerfOpt - * sees a normally-capable device. See the locking note in - * amd_iommu_enable_perfopt(). - */ - domain = dev_data->domain; - dev_data->perfopt = false; - detach_device(&pdev->dev); - if (attach_device(&pdev->dev, domain)) - pci_err(pdev, "failed to restore caps after PerfOpt disable\n"); -} -EXPORT_SYMBOL_GPL(amd_iommu_disable_perfopt); static struct iommu_device *amd_iommu_probe_device(struct device *dev) { struct iommu_device *iommu_dev; @@ -2741,14 +2583,6 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev) static void amd_iommu_release_device(struct device *dev) { struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev); - struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data); - - if (dev_data->perfopt) { - if (WARN_ON(perfopt_put(iommu))) - dev_err(dev, "IOMMU%d: failed to clear PerfOpt on release\n", - iommu->index); - dev_data->perfopt = false; - } WARN_ON(dev_data->domain); @@ -3123,19 +2957,6 @@ static int blocked_domain_attach_device(struct iommu_domain *domain, struct iommu_domain *old) { struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev); - struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data); - - /* - * blocked_domain is also the .release_domain, so this is the normal - * teardown path: drop the reference and clear the flag here too, and - * don't fail teardown if the WARN-guarded write doesn't stick. - */ - if (dev_data->perfopt) { - if (WARN_ON(perfopt_put(iommu))) - dev_err(dev, "IOMMU%d: failed to clear PerfOpt for blocked domain\n", - iommu->index); - dev_data->perfopt = false; - } if (dev_data->domain) detach_device(dev); @@ -3204,9 +3025,6 @@ static int amd_iommu_attach_device(struct iommu_domain *dom, struct device *dev, struct amd_iommu *iommu = get_amd_iommu_from_dev(dev); int ret; - if (dev_data->perfopt && !pdom_is_in_pt_mode(domain)) - return -EBUSY; - /* * Skip attach device to domain if new domain is same as * devices current domain @@ -3421,6 +3239,9 @@ static int amd_iommu_def_domain_type(struct device *dev) if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) return 0; + /* Enable PerfOpt feature if hardware supports it */ + dev_data->perfopt = check_feature(FEATURE_PERF_OPT); + return IOMMU_DOMAIN_IDENTITY; } diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h index 104817e8d8977..5c9dcf720f846 100644 --- a/include/linux/amd-iommu.h +++ b/include/linux/amd-iommu.h @@ -76,15 +76,4 @@ static inline int amd_iommu_snp_disable(void) { return 0; } static inline bool amd_iommu_sev_tio_supported(void) { return false; } #endif -#ifdef CONFIG_AMD_IOMMU -int amd_iommu_enable_perfopt(struct pci_dev *pdev); -void amd_iommu_disable_perfopt(struct pci_dev *pdev); -#else -static inline int amd_iommu_enable_perfopt(struct pci_dev *pdev) -{ - return 0; -} -static inline void amd_iommu_disable_perfopt(struct pci_dev *pdev) { } -#endif - #endif /* _ASM_X86_AMD_IOMMU_H */ -- 2.53.0
