From: Manish Honap <[email protected]> vfio-cxl passes a CXL Type-2 device to a guest and must run CXL Reset at the vfio reset points. The cxl_reset PCI reset method does not fit that caller:
- It requests every HPA range the device decodes. vfio-cxl holds that range with request_mem_region_exclusive() while it exposes the memory, so the request fails with -EBUSY. - When the reset cannot run, the method returns -ENOTTY and the PCI core moves on to the next method. That is FLR, which leaves device memory intact for the next user. - The snapshot and restore helpers that vfio-cxl needs after a bus reset and on return from low power are not exported. Let cxl_hdm_quiesce_prepare() take the resource the caller already holds and skip the request for ranges inside it. pci_cxl_reset_owned() accepts only a resource marked busy and exclusive, as request_mem_region_exclusive() leaves it. The CPU caches are still written back and invalidated for every range before and after the reset, as the reset method does. Add: - pci_cxl_reset_owned(): runs the reset method's sequence for a caller that holds the device lock and returns any failure to that caller - pci_cxl_reset_capable(): checks the cached HDM state and the CPU cache invalidation that the reset-method probe skips. - pci_cxl_hdm_snapshot(): copies the HDM state cached at enumeration without requiring the device lock. A driver needs this copy in probe, which the PCI core may run in a worker on the device's NUMA node, where device_lock_assert() would warn although the binding task holds the lock. pci_cxl_reset_prepare() becomes that copy plus the lock assertion. Export: - pci_cxl_reset_owned() - pci_cxl_reset_capable() - pci_cxl_hdm_snapshot() - cxl_restore_state_after_pci_reset() to the vfio-cxl module only, and declare them in include/cxl/pci.h. CXL Reset always clears memory, so there is no caller Memory Clear choice. Assisted-by: LLM Signed-off-by: Manish Honap <[email protected]> --- drivers/cxl/core/hdm_state.c | 22 ++++-- drivers/pci/cxl.c | 136 ++++++++++++++++++++++++++++------- drivers/pci/pci.h | 18 +---- include/cxl/hdm.h | 3 +- include/cxl/pci.h | 39 ++++++++++ 5 files changed, 171 insertions(+), 47 deletions(-) diff --git a/drivers/cxl/core/hdm_state.c b/drivers/cxl/core/hdm_state.c index 95daeda0ef80..9452047e3b97 100644 --- a/drivers/cxl/core/hdm_state.c +++ b/drivers/cxl/core/hdm_state.c @@ -267,7 +267,8 @@ static int cxl_hdm_ranges_collect(struct cxl_hdm_quiesce *q, struct device *dev, return 0; } -static int cxl_hdm_ranges_request(struct cxl_hdm_quiesce *q) +static int cxl_hdm_ranges_request(struct cxl_hdm_quiesce *q, + const struct resource *owned) { struct cxl_hdm_range *range; @@ -275,6 +276,12 @@ static int cxl_hdm_ranges_request(struct cxl_hdm_quiesce *q) list_for_each_entry(range, &q->ranges, list) { const struct range *hpa_range = &range->hpa_range; + struct resource res = DEFINE_RES_MEM(hpa_range->start, + range->len); + + /* The caller already holds this range exclusively. */ + if (owned && resource_contains(owned, &res)) + continue; range->res = request_mem_region(hpa_range->start, range->len, "cxl_reset"); @@ -316,15 +323,20 @@ static int cxl_hdm_ranges_invalidate(struct cxl_hdm_quiesce *q) * @dev: device for error messages * @q: quiesce state to initialize * @snapshot: HDM state from cxl_hdm_cache_snapshot() + * @owned: resource the caller holds exclusively over the ranges, or NULL * * Reserve every enabled system-physical range in @snapshot exclusively, then - * write back and invalidate CPU caches for them. Call - * cxl_hdm_quiesce_destroy() afterwards whether or not this succeeds. + * write back and invalidate CPU caches for them. A range inside @owned is + * not requested again; the caller's reservation already excludes other + * users. + * + * Call cxl_hdm_quiesce_destroy() afterwards whether or not this succeeds. * * Return: 0 on success, or a negative errno. */ int cxl_hdm_quiesce_prepare(struct device *dev, struct cxl_hdm_quiesce *q, - const struct cxl_hdm_info *snapshot) + const struct cxl_hdm_info *snapshot, + const struct resource *owned) { int rc; @@ -340,7 +352,7 @@ int cxl_hdm_quiesce_prepare(struct device *dev, struct cxl_hdm_quiesce *q, if (rc) return rc; - rc = cxl_hdm_ranges_request(q); + rc = cxl_hdm_ranges_request(q, owned); if (rc) return rc; diff --git a/drivers/pci/cxl.c b/drivers/pci/cxl.c index ed0b6f619c70..b4d467506c54 100644 --- a/drivers/pci/cxl.c +++ b/drivers/pci/cxl.c @@ -372,9 +372,32 @@ static int cxl_reset_save_restored_state(struct pci_dev *pdev, u16 command) return rc; } +/** + * pci_cxl_hdm_snapshot() - copy the HDM state cached at enumeration + * @pdev: CXL device + * + * The cache is read under cxl_rwsem.dpa, so the caller needs no device + * lock. A driver can call this from probe, which the PCI core may run in a + * worker on the device's NUMA node while the binding task holds the lock. + * + * Return: a copy to free with kfree(); NULL when @pdev has no cached HDM + * state; or an ERR_PTR() on failure. + */ +struct cxl_hdm_info *pci_cxl_hdm_snapshot(struct pci_dev *pdev) +{ + struct cxl_hdm_info *snapshot; + + snapshot = cxl_hdm_cache_snapshot(&pdev->hdm); + if (IS_ERR(snapshot) && PTR_ERR(snapshot) == -ENXIO) + return NULL; + + return snapshot; +} +EXPORT_SYMBOL_FOR_MODULES(pci_cxl_hdm_snapshot, "vfio-cxl"); + /** * pci_cxl_reset_prepare() - allocate HDM state storage ahead of a bus reset - * @pdev: device about to be reset + * @pdev: device about to be reset, with its device lock held * * Copy the cached HDM state before the reset, so that restoring cannot fail * for lack of memory. The decoder settings are refreshed when the state is @@ -386,14 +409,8 @@ static int cxl_reset_save_restored_state(struct pci_dev *pdev, u16 command) */ struct cxl_hdm_info *pci_cxl_reset_prepare(struct pci_dev *pdev) { - struct cxl_hdm_info *snapshot; - device_lock_assert(&pdev->dev); - snapshot = cxl_hdm_cache_snapshot(&pdev->hdm); - if (IS_ERR(snapshot) && PTR_ERR(snapshot) == -ENXIO) - return NULL; - - return snapshot; + return pci_cxl_hdm_snapshot(pdev); } /** @@ -433,6 +450,7 @@ int cxl_restore_state_after_pci_reset(struct pci_dev *pdev, cxl_region_quiesce_unlock(); return rc; } +EXPORT_SYMBOL_FOR_MODULES(cxl_restore_state_after_pci_reset, "vfio-cxl"); /* * CXL r4.0 sec 9.7.2 defines the reset completion timeout encodings. @@ -642,7 +660,8 @@ static int __cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap, return rc; } -static int cxl_reset_quiesced(struct pci_dev *pdev, int dvsec, u16 cap) +static int cxl_reset_quiesced(struct pci_dev *pdev, int dvsec, u16 cap, + const struct resource *owned) __must_hold(cxl_region_quiesce) { struct cxl_hdm_info *snapshot __free(kfree) = NULL; @@ -663,7 +682,7 @@ static int cxl_reset_quiesced(struct pci_dev *pdev, int dvsec, u16 cap) return PTR_ERR(snapshot); } - rc = cxl_hdm_quiesce_prepare(&pdev->dev, &q, snapshot); + rc = cxl_hdm_quiesce_prepare(&pdev->dev, &q, snapshot, owned); if (rc) { cxl_hdm_quiesce_destroy(&q); return rc; @@ -686,32 +705,40 @@ static int cxl_reset_quiesced(struct pci_dev *pdev, int dvsec, u16 cap) return cxl_reset_save_restored_state(pdev, command); } -static int cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap) +static int cxl_reset_execute(struct pci_dev *pdev, int dvsec, u16 cap, + const struct resource *owned) { int rc; + /* Raw reset callers may not have saved the PCI state needed for restore. */ + if (!pdev->state_saved) { + pci_err(pdev, "CXL reset requires saved PCI state\n"); + return -EINVAL; + } + cxl_region_quiesce_lock(); - rc = cxl_reset_quiesced(pdev, dvsec, cap); + rc = cxl_reset_quiesced(pdev, dvsec, cap, owned); cxl_region_quiesce_unlock(); return rc; } -int cxl_reset_function(struct pci_dev *pdev, bool probe) +static int cxl_reset_check(struct pci_dev *pdev, bool probe, u16 *dvsec, + u16 *cap) { - u16 dvsec, cap, ctrl; + u16 ctrl; int rc; - rc = cxl_pci_device_dvsec(pdev, &dvsec, &cap); + rc = cxl_pci_device_dvsec(pdev, dvsec, cap); if (rc) return probe ? -ENOTTY : rc; - if (!(cap & PCI_DVSEC_CXL_CACHE_CAPABLE) || - !(cap & PCI_DVSEC_CXL_MEM_CAPABLE) || - !(cap & PCI_DVSEC_CXL_RST_CAPABLE) || - !(cap & PCI_DVSEC_CXL_RST_MEM_CLR_CAPABLE)) + if (!(*cap & PCI_DVSEC_CXL_CACHE_CAPABLE) || + !(*cap & PCI_DVSEC_CXL_MEM_CAPABLE) || + !(*cap & PCI_DVSEC_CXL_RST_CAPABLE) || + !(*cap & PCI_DVSEC_CXL_RST_MEM_CLR_CAPABLE)) return -ENOTTY; - rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_CTRL, &ctrl); + rc = pci_read_config_word(pdev, *dvsec + PCI_DVSEC_CXL_CTRL, &ctrl); if (rc) return probe ? -ENOTTY : pcibios_err_to_errno(rc); if (PCI_POSSIBLE_ERROR(ctrl)) @@ -737,11 +764,70 @@ int cxl_reset_function(struct pci_dev *pdev, bool probe) if (!cpu_cache_has_invalidate_memregion()) return -ENOTTY; - /* Raw reset callers may not have saved the PCI state needed for restore. */ - if (!pdev->state_saved) { - pci_err(pdev, "CXL reset requires saved PCI state\n"); + return 0; +} + +int cxl_reset_function(struct pci_dev *pdev, bool probe) +{ + u16 dvsec, cap; + int rc; + + rc = cxl_reset_check(pdev, probe, &dvsec, &cap); + if (rc || probe) + return rc; + + return cxl_reset_execute(pdev, dvsec, cap, NULL); +} + +/** + * pci_cxl_reset_capable() - check whether CXL Reset can run on a device now + * @pdev: CXL device + * + * Unlike the reset-method probe, this also checks the cached HDM state and + * the CPU cache invalidation that the reset needs. + * + * Return: true when pci_cxl_reset_owned() can attempt the reset. + */ +bool pci_cxl_reset_capable(struct pci_dev *pdev) +{ + u16 dvsec, cap; + + return !cxl_reset_check(pdev, false, &dvsec, &cap); +} +EXPORT_SYMBOL_FOR_MODULES(pci_cxl_reset_capable, "vfio-cxl"); + +/** + * pci_cxl_reset_owned() - CXL Reset for the driver that owns the HDM range + * @pdev: CXL device, with its device lock held and PCI state saved + * @owned: resource the caller reserved over the device's HPA range with + * request_mem_region_exclusive(), or NULL + * + * Run the same sequence as the cxl_reset method. Ranges inside @owned are not + * requested again, so a driver that keeps the HPA range reserved while it + * exposes the memory can reset the device. The CPU caches are still + * invalidated for every range before and after the reset. + * + * Return: 0 on success, or a negative errno. Unlike the reset method, a + * device that cannot be reset returns an error to the caller instead of + * falling through to another reset method. -EINVAL if @owned is not a busy, + * exclusive reservation. + */ +int pci_cxl_reset_owned(struct pci_dev *pdev, const struct resource *owned) +{ + const unsigned long held = IORESOURCE_BUSY | IORESOURCE_EXCLUSIVE; + u16 dvsec, cap; + int rc; + + device_lock_assert(&pdev->dev); + + /* Only a busy, exclusive reservation keeps other users off the range. */ + if (owned && WARN_ON_ONCE((owned->flags & held) != held)) return -EINVAL; - } - return cxl_reset_execute(pdev, dvsec, cap); + rc = cxl_reset_check(pdev, false, &dvsec, &cap); + if (rc) + return rc; + + return cxl_reset_execute(pdev, dvsec, cap, owned); } +EXPORT_SYMBOL_FOR_MODULES(pci_cxl_reset_owned, "vfio-cxl"); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 476e8eefafb2..b1fa4ec7ae75 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -730,14 +730,12 @@ static inline void pci_doe_destroy(struct pci_dev *pdev) { } static inline void pci_doe_disconnected(struct pci_dev *pdev) { } #endif -struct cxl_hdm_info; +#include <cxl/pci.h> + #ifdef CONFIG_CXL_RESET void pci_cxl_hdm_cache_init(struct pci_dev *pdev); void pci_cxl_hdm_cache_release(struct pci_dev *pdev); int cxl_reset_function(struct pci_dev *pdev, bool probe); -struct cxl_hdm_info *pci_cxl_reset_prepare(struct pci_dev *pdev); -int cxl_restore_state_after_pci_reset(struct pci_dev *pdev, - struct cxl_hdm_info *snapshot); #else static inline void pci_cxl_hdm_cache_init(struct pci_dev *pdev) { } static inline void pci_cxl_hdm_cache_release(struct pci_dev *pdev) { } @@ -745,18 +743,6 @@ static inline int cxl_reset_function(struct pci_dev *pdev, bool probe) { return -ENOTTY; } - -static inline struct cxl_hdm_info *pci_cxl_reset_prepare(struct pci_dev *pdev) -{ - return NULL; -} - -static inline int -cxl_restore_state_after_pci_reset(struct pci_dev *pdev, - struct cxl_hdm_info *snapshot) -{ - return 0; -} #endif #ifdef CONFIG_PCI_NPEM diff --git a/include/cxl/hdm.h b/include/cxl/hdm.h index 02cad2d74329..fa3b5845a33c 100644 --- a/include/cxl/hdm.h +++ b/include/cxl/hdm.h @@ -96,7 +96,8 @@ struct cxl_hdm_quiesce { void cxl_region_quiesce_lock(void); void cxl_region_quiesce_unlock(void); int cxl_hdm_quiesce_prepare(struct device *dev, struct cxl_hdm_quiesce *q, - const struct cxl_hdm_info *snapshot) + const struct cxl_hdm_info *snapshot, + const struct resource *owned) __must_hold(cxl_region_quiesce); int cxl_hdm_quiesce_flush(struct cxl_hdm_quiesce *q) __must_hold(cxl_region_quiesce); diff --git a/include/cxl/pci.h b/include/cxl/pci.h index 0b74819fea6d..64efecb44c1f 100644 --- a/include/cxl/pci.h +++ b/include/cxl/pci.h @@ -4,6 +4,7 @@ #ifndef __CXL_CXL_PCI_H__ #define __CXL_CXL_PCI_H__ +#include <linux/errno.h> #include <linux/types.h> /* Register Block Identifier (RBI) */ @@ -27,6 +28,44 @@ int cxl_reg_map_set_owned_resource(struct cxl_register_map *map, int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type, struct cxl_register_map *map); int cxl_setup_regs(struct cxl_register_map *map); + +struct cxl_hdm_info; +#ifdef CONFIG_CXL_RESET +struct cxl_hdm_info *pci_cxl_hdm_snapshot(struct pci_dev *pdev); +struct cxl_hdm_info *pci_cxl_reset_prepare(struct pci_dev *pdev); +int cxl_restore_state_after_pci_reset(struct pci_dev *pdev, + struct cxl_hdm_info *snapshot); +bool pci_cxl_reset_capable(struct pci_dev *pdev); +int pci_cxl_reset_owned(struct pci_dev *pdev, const struct resource *owned); +#else +static inline struct cxl_hdm_info *pci_cxl_hdm_snapshot(struct pci_dev *pdev) +{ + return NULL; +} + +static inline struct cxl_hdm_info *pci_cxl_reset_prepare(struct pci_dev *pdev) +{ + return NULL; +} + +static inline int +cxl_restore_state_after_pci_reset(struct pci_dev *pdev, + struct cxl_hdm_info *snapshot) +{ + return 0; +} + +static inline bool pci_cxl_reset_capable(struct pci_dev *pdev) +{ + return false; +} + +static inline int pci_cxl_reset_owned(struct pci_dev *pdev, + const struct resource *owned) +{ + return -ENOTTY; +} +#endif /* * CXL r4.0 sec 8.1.3.1: a CXL.mem capable device reports one or two HDM * ranges. 00b is illegal when Mem_Capable is set and 11b is reserved. -- 2.53.0
