On Mon, Dec 03, 2018 at 06:05:54PM +0000, James Morse wrote:
> When CPER records are found the address of the records is stashed
> in the struct ghes. Once the records have been processed, this
> address is overwritten with zero so that it won't be processed
> again without being re-populated by firmware.
>
> This goes wrong if a struct ghes can be processed concurrently,
> as can happen at probe time when an NMI occurs. If the NMI arrives
> on another CPU, the probing CPU may call ghes_clear_estatus() on the
> records before the handler had finished with them.
> Even on the same CPU, once the interrupted handler is resumed, it
> will call ghes_clear_estatus() on the NMIs records, this memory may
> have already been re-used by firmware.
>
> Avoid this stashing by letting the caller hold the address. A
> later patch will do away with the use of ghes->flags in the
> read/clear code too.
>
> Signed-off-by: James Morse <james.morse@xxxxxxx>
>
> ---
> Changes since v6:
> * Moved earlier in the series
> * Added buf_adder = 0 on all the error paths, and test for it in
> ghes_estatus_clear() for extra sanity.
> ---
> drivers/acpi/apei/ghes.c | 40 +++++++++++++++++++++++-----------------
> include/acpi/ghes.h | 1 -
> 2 files changed, 23 insertions(+), 18 deletions(-)
...
> @@ -349,17 +350,20 @@ static int ghes_read_estatus(struct ghes *ghes)
> if (rc)
> pr_warn_ratelimited(FW_WARN GHES_PFX
> "Failed to read error status block!\n");
> +
> return rc;
> }
>
> -static void ghes_clear_estatus(struct ghes *ghes)
> +static void ghes_clear_estatus(struct ghes *ghes, u64 buf_paddr)
> {
> ghes->estatus->block_status = 0;
> if (!(ghes->flags & GHES_TO_CLEAR))
> return;
> - ghes_copy_tofrom_phys(ghes->estatus, ghes->buffer_paddr,
> - sizeof(ghes->estatus->block_status), 0);
> - ghes->flags &= ~GHES_TO_CLEAR;
<---- newline here.
> + if (buf_paddr) {
Also, you can save yourself an indendation level:
if (!buf_paddr)
return;
ghes_copy...
> + ghes_copy_tofrom_phys(ghes->estatus, buf_paddr,
> + sizeof(ghes->estatus->block_status), 0);
> + ghes->flags &= ~GHES_TO_CLEAR;
> + }
> }
With that addressed:
Reviewed-by: Borislav Petkov <bp@xxxxxxx>
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
