On Wed, Jul 08, 2026 at 02:59:05PM +0100, Ahmed Tiba wrote:
> Move the CXL CPER handling paths out of ghes.c and into ghes_cper.c so the
> helpers can be reused. The code is moved as-is, with the public
> prototypes updated so GHES keeps calling into the new translation unit.
> 
> While moving this code, also add CXL CPER section length checks and use
> spinlock_irqsave() in CXL register/unregister paths for locking
> consistency.

NAK on moving and changing in the same patch, and esp in a patch
whose subject only says MOVE.


> 
> Reviewed-by: Jonathan Cameron <[email protected]>
> Signed-off-by: Ahmed Tiba <[email protected]>
> ---
>  drivers/acpi/apei/ghes.c      | 180 
> +++++++++++-------------------------------
>  drivers/acpi/apei/ghes_cper.c | 135 +++++++++++++++++++++++++++++++
>  include/acpi/ghes_cper.h      |  11 +++
>  3 files changed, 194 insertions(+), 132 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 07e4001ea8d7..2a83d326e692 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -383,138 +383,6 @@ static void ghes_handle_aer(struct 
> acpi_hest_generic_data *gdata)
>  #endif
>  }
>  
> -/* Room for 8 entries */
> -#define CXL_CPER_PROT_ERR_FIFO_DEPTH 8
> -static DEFINE_KFIFO(cxl_cper_prot_err_fifo, struct 
> cxl_cper_prot_err_work_data,
> -                 CXL_CPER_PROT_ERR_FIFO_DEPTH);
> -
> -/* Synchronize schedule_work() with cxl_cper_prot_err_work changes */
> -static DEFINE_SPINLOCK(cxl_cper_prot_err_work_lock);
> -struct work_struct *cxl_cper_prot_err_work;
> -
> -static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> -                                int severity)
> -{
> -#ifdef CONFIG_ACPI_APEI_PCIEAER
> -     struct cxl_cper_prot_err_work_data wd;
> -
> -     if (cxl_cper_sec_prot_err_valid(prot_err))
> -             return;
> -
> -     guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
> -
> -     if (!cxl_cper_prot_err_work)
> -             return;
> -
> -     if (cxl_cper_setup_prot_err_work_data(&wd, prot_err, severity))
> -             return;
> -
> -     if (!kfifo_put(&cxl_cper_prot_err_fifo, wd)) {
> -             pr_err_ratelimited("CXL CPER kfifo overflow\n");
> -             return;
> -     }
> -
> -     schedule_work(cxl_cper_prot_err_work);
> -#endif
> -}
> -
> -int cxl_cper_register_prot_err_work(struct work_struct *work)
> -{
> -     if (cxl_cper_prot_err_work)
> -             return -EINVAL;
> -
> -     guard(spinlock)(&cxl_cper_prot_err_work_lock);
> -     cxl_cper_prot_err_work = work;
> -     return 0;
> -}
> -EXPORT_SYMBOL_NS_GPL(cxl_cper_register_prot_err_work, "CXL");
> -
> -int cxl_cper_unregister_prot_err_work(struct work_struct *work)
> -{
> -     if (cxl_cper_prot_err_work != work)
> -             return -EINVAL;
> -
> -     guard(spinlock)(&cxl_cper_prot_err_work_lock);
> -     cxl_cper_prot_err_work = NULL;
> -     return 0;
> -}
> -EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_prot_err_work, "CXL");
> -
> -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");
> -
> -/* Room for 8 entries for each of the 4 event log queues */
> -#define CXL_CPER_FIFO_DEPTH 32
> -DEFINE_KFIFO(cxl_cper_fifo, struct cxl_cper_work_data, CXL_CPER_FIFO_DEPTH);
> -
> -/* Synchronize schedule_work() with cxl_cper_work changes */
> -static DEFINE_SPINLOCK(cxl_cper_work_lock);
> -struct work_struct *cxl_cper_work;
> -
> -static void cxl_cper_post_event(enum cxl_event_type event_type,
> -                             struct cxl_cper_event_rec *rec)
> -{
> -     struct cxl_cper_work_data wd;
> -
> -     if (rec->hdr.length <= sizeof(rec->hdr) ||
> -         rec->hdr.length > sizeof(*rec)) {
> -             pr_err(FW_WARN "CXL CPER Invalid section length (%u)\n",
> -                    rec->hdr.length);
> -             return;
> -     }
> -
> -     if (!(rec->hdr.validation_bits & CPER_CXL_COMP_EVENT_LOG_VALID)) {
> -             pr_err(FW_WARN "CXL CPER invalid event\n");
> -             return;
> -     }
> -
> -     guard(spinlock_irqsave)(&cxl_cper_work_lock);
> -
> -     if (!cxl_cper_work)
> -             return;
> -
> -     wd.event_type = event_type;
> -     memcpy(&wd.rec, rec, sizeof(wd.rec));
> -
> -     if (!kfifo_put(&cxl_cper_fifo, wd)) {
> -             pr_err_ratelimited("CXL CPER kfifo overflow\n");
> -             return;
> -     }
> -
> -     schedule_work(cxl_cper_work);
> -}
> -
> -int cxl_cper_register_work(struct work_struct *work)
> -{
> -     if (cxl_cper_work)
> -             return -EINVAL;
> -
> -     guard(spinlock)(&cxl_cper_work_lock);
> -     cxl_cper_work = work;
> -     return 0;
> -}
> -EXPORT_SYMBOL_NS_GPL(cxl_cper_register_work, "CXL");
> -
> -int cxl_cper_unregister_work(struct work_struct *work)
> -{
> -     if (cxl_cper_work != work)
> -             return -EINVAL;
> -
> -     guard(spinlock)(&cxl_cper_work_lock);
> -     cxl_cper_work = NULL;
> -     return 0;
> -}
> -EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_work, "CXL");
> -
> -int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd)
> -{
> -     return kfifo_get(&cxl_cper_fifo, wd);
> -}
> -EXPORT_SYMBOL_NS_GPL(cxl_cper_kfifo_get, "CXL");
> -
>  static void ghes_log_hwerr(int sev, guid_t *sec_type)
>  {
>       if (sev != CPER_SEV_RECOVERABLE)
> @@ -549,6 +417,42 @@ static void ghes_log_hwerr(int sev, guid_t *sec_type)
>       hwerr_log_error_type(HWERR_RECOV_OTHERS);
>  }
>  
> +static bool ghes_cxl_event_len_valid(struct acpi_hest_generic_data *gdata,
> +                                  struct cxl_cper_event_rec *rec)
> +{
> +     if (gdata->error_data_length < sizeof(*rec) ||
> +         rec->hdr.length <= sizeof(rec->hdr) ||
> +         rec->hdr.length > gdata->error_data_length ||
> +         rec->hdr.length > sizeof(*rec)) {
> +             pr_err(FW_WARN "CXL CPER Invalid section length (%u)\n",
> +                    rec->hdr.length);
> +             return false;
> +     }
> +
> +     return true;
> +}
> +
> +static bool ghes_cxl_prot_err_len_valid(struct acpi_hest_generic_data *gdata,
> +                                     struct cxl_cper_sec_prot_err *prot_err)
> +{
> +     if (gdata->error_data_length < sizeof(*prot_err) +
> +         sizeof(struct cxl_ras_capability_regs)) {
> +             pr_err(FW_WARN "CXL CPER Invalid protocol error length (%u)\n",
> +                    gdata->error_data_length);
> +             return false;
> +     }
> +
> +     if (prot_err->dvsec_len >
> +         gdata->error_data_length - sizeof(*prot_err) -
> +         sizeof(struct cxl_ras_capability_regs)) {
> +             pr_err(FW_WARN "CXL CPER invalid DVSEC length (%u)\n",
> +                    prot_err->dvsec_len);
> +             return false;
> +     }
> +
> +     return true;
> +}
> +
>  static void ghes_do_proc(struct ghes *ghes,
>                        const struct acpi_hest_generic_status *estatus)
>  {
> @@ -585,18 +489,30 @@ static void ghes_do_proc(struct ghes *ghes,
>               } else if (guid_equal(sec_type, &CPER_SEC_CXL_PROT_ERR)) {
>                       struct cxl_cper_sec_prot_err *prot_err = 
> acpi_hest_get_payload(gdata);
>  
> +                     if (!ghes_cxl_prot_err_len_valid(gdata, prot_err))
> +                             continue;
> +
>                       cxl_cper_post_prot_err(prot_err, gdata->error_severity);
>               } else if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID)) {
>                       struct cxl_cper_event_rec *rec = 
> acpi_hest_get_payload(gdata);
>  
> +                     if (!ghes_cxl_event_len_valid(gdata, rec))
> +                             continue;
> +
>                       cxl_cper_post_event(CXL_CPER_EVENT_GEN_MEDIA, rec);
>               } else if (guid_equal(sec_type, &CPER_SEC_CXL_DRAM_GUID)) {
>                       struct cxl_cper_event_rec *rec = 
> acpi_hest_get_payload(gdata);
>  
> +                     if (!ghes_cxl_event_len_valid(gdata, rec))
> +                             continue;
> +
>                       cxl_cper_post_event(CXL_CPER_EVENT_DRAM, rec);
>               } else if (guid_equal(sec_type, &CPER_SEC_CXL_MEM_MODULE_GUID)) 
> {
>                       struct cxl_cper_event_rec *rec = 
> acpi_hest_get_payload(gdata);
>  
> +                     if (!ghes_cxl_event_len_valid(gdata, rec))
> +                             continue;
> +
>                       cxl_cper_post_event(CXL_CPER_EVENT_MEM_MODULE, rec);
>               } else {
>                       void *err = acpi_hest_get_payload(gdata);
> diff --git a/drivers/acpi/apei/ghes_cper.c b/drivers/acpi/apei/ghes_cper.c
> index 7e4a66b788b8..b59e3ed3eab3 100644
> --- a/drivers/acpi/apei/ghes_cper.c
> +++ b/drivers/acpi/apei/ghes_cper.c
> @@ -12,9 +12,12 @@
>   *   Author: Huang Ying <[email protected]>
>   */
>  
> +#include <linux/aer.h>
> +#include <linux/cleanup.h>
>  #include <linux/err.h>
>  #include <linux/genalloc.h>
>  #include <linux/io.h>
> +#include <linux/kfifo.h>
>  #include <linux/kernel.h>
>  #include <linux/math64.h>
>  #include <linux/mm.h>
> @@ -350,6 +353,138 @@ void ghes_defer_non_standard_event(struct 
> acpi_hest_generic_data *gdata,
>       schedule_work(&entry->work);
>  }
>  
> +/* Room for 8 entries */
> +#define CXL_CPER_PROT_ERR_FIFO_DEPTH 8
> +static DEFINE_KFIFO(cxl_cper_prot_err_fifo, struct 
> cxl_cper_prot_err_work_data,
> +                 CXL_CPER_PROT_ERR_FIFO_DEPTH);
> +
> +/* Synchronize schedule_work() with cxl_cper_prot_err_work changes */
> +static DEFINE_SPINLOCK(cxl_cper_prot_err_work_lock);
> +struct work_struct *cxl_cper_prot_err_work;
> +
> +void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> +                         int severity)
> +{
> +#ifdef CONFIG_ACPI_APEI_PCIEAER
> +     struct cxl_cper_prot_err_work_data wd;
> +
> +     if (cxl_cper_sec_prot_err_valid(prot_err))
> +             return;
> +
> +     guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
> +
> +     if (!cxl_cper_prot_err_work)
> +             return;
> +
> +     if (cxl_cper_setup_prot_err_work_data(&wd, prot_err, severity))
> +             return;
> +
> +     if (!kfifo_put(&cxl_cper_prot_err_fifo, wd)) {
> +             pr_err_ratelimited("CXL CPER kfifo overflow\n");
> +             return;
> +     }
> +
> +     schedule_work(cxl_cper_prot_err_work);
> +#endif
> +}
> +
> +int cxl_cper_register_prot_err_work(struct work_struct *work)
> +{
> +     if (cxl_cper_prot_err_work)
> +             return -EINVAL;
> +
> +     guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
> +     cxl_cper_prot_err_work = work;
> +     return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_cper_register_prot_err_work, "CXL");
> +
> +int cxl_cper_unregister_prot_err_work(struct work_struct *work)
> +{
> +     if (cxl_cper_prot_err_work != work)
> +             return -EINVAL;
> +
> +     guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
> +     cxl_cper_prot_err_work = NULL;
> +     return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_prot_err_work, "CXL");
> +
> +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");
> +
> +/* Room for 8 entries for each of the 4 event log queues */
> +#define CXL_CPER_FIFO_DEPTH 32
> +static DEFINE_KFIFO(cxl_cper_fifo, struct cxl_cper_work_data, 
> CXL_CPER_FIFO_DEPTH);
> +
> +/* Synchronize schedule_work() with cxl_cper_work changes */
> +static DEFINE_SPINLOCK(cxl_cper_work_lock);
> +struct work_struct *cxl_cper_work;
> +
> +void cxl_cper_post_event(enum cxl_event_type event_type,
> +                      struct cxl_cper_event_rec *rec)
> +{
> +     struct cxl_cper_work_data wd;
> +
> +     if (rec->hdr.length <= sizeof(rec->hdr) ||
> +         rec->hdr.length > sizeof(*rec)) {
> +             pr_err(FW_WARN "CXL CPER Invalid section length (%u)\n",
> +                    rec->hdr.length);
> +             return;
> +     }
> +
> +     if (!(rec->hdr.validation_bits & CPER_CXL_COMP_EVENT_LOG_VALID)) {
> +             pr_err(FW_WARN "CXL CPER invalid event\n");
> +             return;
> +     }
> +
> +     guard(spinlock_irqsave)(&cxl_cper_work_lock);
> +
> +     if (!cxl_cper_work)
> +             return;
> +
> +     wd.event_type = event_type;
> +     memcpy(&wd.rec, rec, sizeof(wd.rec));
> +
> +     if (!kfifo_put(&cxl_cper_fifo, wd)) {
> +             pr_err_ratelimited("CXL CPER kfifo overflow\n");
> +             return;
> +     }
> +
> +     schedule_work(cxl_cper_work);
> +}
> +
> +int cxl_cper_register_work(struct work_struct *work)
> +{
> +     if (cxl_cper_work)
> +             return -EINVAL;
> +
> +     guard(spinlock_irqsave)(&cxl_cper_work_lock);
> +     cxl_cper_work = work;
> +     return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_cper_register_work, "CXL");
> +
> +int cxl_cper_unregister_work(struct work_struct *work)
> +{
> +     if (cxl_cper_work != work)
> +             return -EINVAL;
> +
> +     guard(spinlock_irqsave)(&cxl_cper_work_lock);
> +     cxl_cper_work = NULL;
> +     return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_work, "CXL");
> +
> +int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd)
> +{
> +     return kfifo_get(&cxl_cper_fifo, wd);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_cper_kfifo_get, "CXL");
> +
>  /*
>   * GHES error status reporting throttle, to report more kinds of
>   * errors, instead of just most frequently occurred errors.
> diff --git a/include/acpi/ghes_cper.h b/include/acpi/ghes_cper.h
> index d9f9253d8de9..a853a5996cdf 100644
> --- a/include/acpi/ghes_cper.h
> +++ b/include/acpi/ghes_cper.h
> @@ -20,6 +20,7 @@
>  
>  #include <acpi/ghes.h>
>  #include <asm/fixmap.h>
> +#include <cxl/event.h>
>  
>  #define GHES_PFX     "GHES: "
>  
> @@ -106,5 +107,15 @@ void ghes_estatus_cache_add(struct acpi_hest_generic 
> *generic,
>                           struct acpi_hest_generic_status *estatus);
>  void ghes_defer_non_standard_event(struct acpi_hest_generic_data *gdata,
>                                  int sev);
> +void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> +                         int severity);
> +int cxl_cper_register_prot_err_work(struct work_struct *work);
> +int cxl_cper_unregister_prot_err_work(struct work_struct *work);
> +int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd);
> +void cxl_cper_post_event(enum cxl_event_type event_type,
> +                      struct cxl_cper_event_rec *rec);
> +int cxl_cper_register_work(struct work_struct *work);
> +int cxl_cper_unregister_work(struct work_struct *work);
> +int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd);
>  
>  #endif /* ACPI_APEI_GHES_CPER_H */
> 
> -- 
> 2.43.0
> 

Reply via email to