Hello James, On Fri, Feb 28, 2020 at 12:49 PM James Morse <james.morse@xxxxxxx> wrote: > > -static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int sev) > +static bool ghes_handle_memory_failure(struct ghes *ghes, > + struct acpi_hest_generic_data *gdata, > + int sev) It doesn't look like ghes needs to be added as a parameter to this function, unless I'm missing something :) > { > -#ifdef CONFIG_ACPI_APEI_MEMORY_FAILURE > unsigned long pfn; > int flags = -1; > int sec_sev = ghes_severity(gdata->error_severity); > struct cper_sec_mem_err *mem_err = acpi_hest_get_payload(gdata); > > + if (!IS_ENABLED(CONFIG_ACPI_APEI_MEMORY_FAILURE)) > + return false; > + > if (!(mem_err->validation_bits & CPER_MEM_VALID_PA)) > - return; > + return false; > > pfn = mem_err->physical_addr >> PAGE_SHIFT; > if (!pfn_valid(pfn)) { > pr_warn_ratelimited(FW_WARN GHES_PFX > "Invalid address in generic error data: %#llx\n", > mem_err->physical_addr); > - return; > + return false; > } > > /* iff following two events can be handled properly by now */ > @@ -440,9 +465,12 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int > if (sev == GHES_SEV_RECOVERABLE && sec_sev == GHES_SEV_RECOVERABLE) > flags = 0; > > - if (flags != -1) > + if (flags != -1) { > memory_failure_queue(pfn, flags); > -#endif > + return true; > + } > + > + return false; > } > This series looks good to me overall. I'm going to pull it and give it a spin. Thanks, Tyler