Re: [PATCH 3/3] EDAC, ghes: Make it a proper module

From: Kani, Toshimitsu
Date: Fri Jul 28 2017 - 14:51:58 EST


On Wed, 2017-07-26 at 10:48 +0200, Borislav Petkov wrote:
> From: Borislav Petkov <bp@xxxxxxx>
>
> Register with the GHES notifier chain so that there's no need to call
> into the module with ghes_edac_report_mem_error().
>
> Signed-off-by: Borislav Petkov <bp@xxxxxxx>
:
> +static int report_mem_error(struct notifier_block *nb, unsigned long
> sev, void *data)
> Â{
> + struct cper_sec_mem_err *mem_err = data;
> Â enum hw_event_mc_err_type type;
> Â struct edac_raw_error_desc *e;
> Â struct mem_ctl_info *mci;
> - struct ghes_edac_pvt *pvt = NULL;
> - char *p;
> + struct ghes_edac_pvt *pvt = ghes_pvt;
> Â u8 grain_bits;
> + char *p;
> Â
> - list_for_each_entry(pvt, &ghes_reglist, list) {
> - if (ghes == pvt->ghes)
> - break;
> - }
> Â if (!pvt) {

I think it always hits this error condition. See below.

> - pr_err("Internal error: Can't find EDAC
> structure\n");
> - return;
> + edac_pr_err("Internal error: Can't find EDAC
> structure\n");
> + return NOTIFY_DONE;
> Â }
> +
:
> +static int __init ghes_edac_register(void)
> Â{
> + struct ghes_edac_pvt *pvt = ghes_pvt;

This simply sets NULL to pvt, and does not initialize ghes_pvt.

> Â bool fake = false;
> Â int rc, num_dimm = 0;
> Â struct mem_ctl_info *mci;
> Â struct edac_mc_layer layers[1];
> - struct ghes_edac_pvt *pvt;
> Â struct ghes_edac_dimm_fill dimm_fill;
> Â
:
> -EXPORT_SYMBOL_GPL(ghes_edac_register);
> +module_init(ghes_edac_register);

Since this patch has removed the GHES-presence check and ordering hack
for ghes_edac, it now registers itself per the module_init() ordering
regardless of the presence of GHES. As Mauro pointed out, some type of
GHES check needs to be in place before making this change.

Thanks,
-Toshi