From: Dan Williams <[email protected]> Tighten the CPER protocol error kfifo registration API and symbol visibility.
Use EXPORT_SYMBOL_FOR_MODULES() instead of EXPORT_SYMBOL_NS_GPL() for the CPER kfifo registration symbols. This names the consuming module explicitly and gives compile-time enforcement. Drop the work_struct argument from the unregister path. Change the WARN_ONCE condition to a NULL check since there is no caller pointer to compare against anymore. Change register/unregister return types to void. Flag double registration with WARN_ONCE() inside the lock instead of returning an error. Change cxl_ras_init() to void because there is one consumer and one producer so the error return was unnecessary. Remove the now-dead error check in cxl_core_init(). Add a diagnostic log when the driver is not bound in cxl_cper_handle_prot_err(). Co-developed-by: Terry Bowman <[email protected]> Signed-off-by: Terry Bowman <[email protected]> Signed-off-by: Dan Williams <[email protected]> --- Changes in v17 -> v18: - New patch. --- drivers/acpi/apei/ghes.c | 32 +++++++++++++++----------------- drivers/cxl/core/core.h | 7 ++----- drivers/cxl/core/port.c | 6 +----- drivers/cxl/core/ras.c | 12 +++++++----- include/cxl/event.h | 17 ++++++----------- 5 files changed, 31 insertions(+), 43 deletions(-) diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index ca7a138c1ff2e..187f54e31c33e 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -778,41 +778,41 @@ static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err, #endif } -int cxl_cper_register_prot_err_work(struct work_struct *work) +void cxl_cper_register_prot_err_work(struct work_struct *work) { guard(raw_spinlock_irqsave)(&cxl_cper_prot_err_work_lock); if (WARN_ONCE(cxl_cper_prot_err_work, "CPER-CXL kfifo consumer already registered\n")) - return -EINVAL; + return; cxl_cper_prot_err_work = work; - return 0; } -EXPORT_SYMBOL_NS_GPL(cxl_cper_register_prot_err_work, "CXL"); +EXPORT_SYMBOL_FOR_MODULES(cxl_cper_register_prot_err_work, "cxl_core"); -int cxl_cper_unregister_prot_err_work(struct work_struct *work) +void cxl_cper_unregister_prot_err_work(void) { + struct work_struct *old; + scoped_guard(raw_spinlock_irqsave, &cxl_cper_prot_err_work_lock) { - if (WARN_ONCE(cxl_cper_prot_err_work != work, - "CPER-CXL kfifo consumer mismatch on unregister\n")) - return -EINVAL; + WARN_ONCE(!cxl_cper_prot_err_work, + "CPER-CXL kfifo consumer not registered on unregister\n"); + old = cxl_cper_prot_err_work; cxl_cper_prot_err_work = NULL; } - cancel_work_sync(work); + if (old) + cancel_work_sync(old); /* Discard stale entries so they are not replayed on next module load */ kfifo_reset(&cxl_cper_prot_err_fifo); - - return 0; } -EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_prot_err_work, "CXL"); +EXPORT_SYMBOL_FOR_MODULES(cxl_cper_unregister_prot_err_work, "cxl_core"); int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd) { return kfifo_get(&cxl_cper_prot_err_fifo, wd); } -EXPORT_SYMBOL_NS_GPL(cxl_cper_prot_err_kfifo_get, "CXL"); +EXPORT_SYMBOL_FOR_MODULES(cxl_cper_prot_err_kfifo_get, "cxl_core"); /* Room for 8 entries for each of the 4 event log queues */ #define CXL_CPER_FIFO_DEPTH 32 @@ -867,12 +867,12 @@ int cxl_cper_register_work(struct work_struct *work) } EXPORT_SYMBOL_NS_GPL(cxl_cper_register_work, "CXL"); -int cxl_cper_unregister_work(struct work_struct *work) +void cxl_cper_unregister_work(struct work_struct *work) { scoped_guard(raw_spinlock_irqsave, &cxl_cper_work_lock) { if (WARN_ONCE(cxl_cper_work != work, "CXL CPER kfifo consumer mismatch on unregister\n")) - return -EINVAL; + return; cxl_cper_work = NULL; } @@ -880,8 +880,6 @@ int cxl_cper_unregister_work(struct work_struct *work) /* Discard stale entries so they are not replayed on next module load */ kfifo_reset(&cxl_cper_fifo); - - return 0; } EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_work, "CXL"); diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h index 07555ae638594..23fe40ddf4c6b 100644 --- a/drivers/cxl/core/core.h +++ b/drivers/cxl/core/core.h @@ -183,7 +183,7 @@ static inline struct device *dport_to_host(struct cxl_dport *dport) return &port->dev; } #ifdef CONFIG_CXL_RAS -int cxl_ras_init(void); +void cxl_ras_init(void); void cxl_ras_exit(void); bool cxl_handle_ras(struct device *dev, void __iomem *ras_base); void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base); @@ -192,10 +192,7 @@ void cxl_disable_rch_root_ints(struct cxl_dport *dport); void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds); void devm_cxl_dport_ras_setup(struct cxl_dport *dport); #else -static inline int cxl_ras_init(void) -{ - return 0; -} +static inline void cxl_ras_init(void) { } static inline void cxl_ras_exit(void) { } static inline bool cxl_handle_ras(struct device *dev, void __iomem *ras_base) { diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 1215ee4f40351..f90f899c31d07 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -2531,14 +2531,10 @@ static __init int cxl_core_init(void) if (rc) goto err_region; - rc = cxl_ras_init(); - if (rc) - goto err_ras; + cxl_ras_init(); return 0; -err_ras: - cxl_region_exit(); err_region: bus_unregister(&cxl_bus_type); err_bus: diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c index 99fb00949c2fa..135f1997e6f4f 100644 --- a/drivers/cxl/core/ras.c +++ b/drivers/cxl/core/ras.c @@ -104,8 +104,11 @@ void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data) } guard(device)(&pdev->dev); - if (!pdev->dev.driver) + if (!pdev->dev.driver) { + dev_warn_ratelimited(&pdev->dev, + "Device is unbound, abort CPER error handling\n"); return; + } struct device *mem_dev __free(put_device) = bus_find_device( &cxl_bus_type, NULL, pdev, match_memdev_by_parent); @@ -129,15 +132,14 @@ static void cxl_cper_prot_err_work_fn(struct work_struct *work) } static DECLARE_WORK(cxl_cper_prot_err_work, cxl_cper_prot_err_work_fn); -int cxl_ras_init(void) +void cxl_ras_init(void) { - return cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work); + cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work); } void cxl_ras_exit(void) { - cxl_cper_unregister_prot_err_work(&cxl_cper_prot_err_work); - cancel_work_sync(&cxl_cper_prot_err_work); + cxl_cper_unregister_prot_err_work(); } static void cxl_dport_map_ras(struct cxl_dport *dport) diff --git a/include/cxl/event.h b/include/cxl/event.h index ff97fea718d2c..3471d4f75c025 100644 --- a/include/cxl/event.h +++ b/include/cxl/event.h @@ -287,10 +287,10 @@ struct cxl_cper_prot_err_work_data { #ifdef CONFIG_ACPI_APEI_GHES int cxl_cper_register_work(struct work_struct *work); -int cxl_cper_unregister_work(struct work_struct *work); +void cxl_cper_unregister_work(struct work_struct *work); int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd); -int cxl_cper_register_prot_err_work(struct work_struct *work); -int cxl_cper_unregister_prot_err_work(struct work_struct *work); +void cxl_cper_register_prot_err_work(struct work_struct *work); +void cxl_cper_unregister_prot_err_work(void); int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd); #else static inline int cxl_cper_register_work(struct work_struct *work) @@ -298,21 +298,16 @@ static inline int cxl_cper_register_work(struct work_struct *work) return 0; } -static inline int cxl_cper_unregister_work(struct work_struct *work) -{ - return 0; -} +static inline void cxl_cper_unregister_work(struct work_struct *work) { } static inline int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd) { return 0; } -static inline int cxl_cper_register_prot_err_work(struct work_struct *work) +static inline void cxl_cper_register_prot_err_work(struct work_struct *work) { - return 0; } -static inline int cxl_cper_unregister_prot_err_work(struct work_struct *work) +static inline void cxl_cper_unregister_prot_err_work(void) { - return 0; } static inline int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd) { -- 2.34.1
