On Mon, Dec 03, 2018 at 06:06:06PM +0000, James Morse wrote:
> ghes_read_estatus() reads the record address, then the record's
> header, then performs some sanity checks before reading the
> records into the provided estatus buffer.
>
> To provide this estatus buffer the caller must know the size of the
> records in advance, or always provide a worst-case sized buffer as
> happens today for the non-NMI notifications.
>
> Add a function to peek at the record's header to find the size. This
> will let the NMI path allocate the right amount of memory before reading
> the records, instead of using the worst-case size, and having to copy
> the records.
>
> Split ghes_read_estatus() to create __ghes_peek_estatus() which
> returns the address and size of the CPER records.
>
> Signed-off-by: James Morse <james.morse@xxxxxxx>
>
> Changes since v6:
> * Additional buf_addr = 0 error handling
> * Moved checking out of peek-estatus
> * Reworded an error message so we can tell them apart
> ---
> drivers/acpi/apei/ghes.c | 59 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 47 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index b70f5fd962cc..07a12aac4c1a 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -277,12 +277,12 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
> }
> }
>
> -static int ghes_read_estatus(struct ghes *ghes,
> - struct acpi_hest_generic_status *estatus,
> - u64 *buf_paddr, int fixmap_idx)
> +/* Read the CPER block and returning its address, and header in estatus. */
s/and /,/
> +static int __ghes_peek_estatus(struct ghes *ghes, int fixmap_idx,
> + struct acpi_hest_generic_status *estatus,
Also, we probably should stick to some order of arguments of those
functions for easier code staring, i.e.
function_name(ghes, estatus, buf_paddr, fixmap_idx)
or so.
> + u64 *buf_paddr)
> {
> struct acpi_hest_generic *g = ghes->generic;
> - u32 len;
> int rc;
>
> rc = apei_read(buf_paddr, &g->error_status_address);
> @@ -303,29 +303,64 @@ static int ghes_read_estatus(struct ghes *ghes,
> return -ENOENT;
> }
>
> - rc = -EIO;
> - len = cper_estatus_len(estatus);
> + return 0;
> +}
> +
> +/* Check the top-level record header has an appropriate size. */
> +int __ghes_check_estatus(struct ghes *ghes,
> + struct acpi_hest_generic_status *estatus)
> +{
> + u32 len = cper_estatus_len(estatus);
> + int rc = -EIO;
> +
> if (len < sizeof(*estatus))
> goto err_read_block;
> if (len > ghes->generic->error_block_length)
> goto err_read_block;
> if (cper_estatus_check_header(estatus))
> goto err_read_block;
Please make this chunk more user-friendly, maybe in a separate patch ontop:
/* Check the top-level record header has an appropriate size. */
int __ghes_check_estatus(struct ghes *ghes,
struct acpi_hest_generic_status *estatus)
{
u32 len = cper_estatus_len(estatus);
if (len < sizeof(*estatus)) {
pr_warn_ratelimited(FW_WARN GHES_PFX "Truncated error status block!\n");
return -EIO;
}
if (len > ghes->generic->error_block_length) {
pr_warn_ratelimited(FW_WARN GHES_PFX "Invalid error status block length!\n");
return -EIO;
}
if (cper_estatus_check_header(estatus)) {
pr_warn_ratelimited(FW_WARN GHES_PFX "Invalid CPER header!\n");
return -EIO;
}
return 0;
}
Thx.
--
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
