linux-kernel - Re: [PATCH 10/11] qapi/acpi-hest: add an interface to do generic CPER error injection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250124134007.18b508bc@imammedo.users.ipa.redhat.com>
Date: Fri, 24 Jan 2025 13:40:07 +0100
From: Igor Mammedov <imammedo@...hat.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>, "Michael S . Tsirkin"
 <mst@...hat.com>, Shiju Jose <shiju.jose@...wei.com>,
 <qemu-arm@...gnu.org>, <qemu-devel@...gnu.org>, Ani Sinha
 <anisinha@...hat.com>, Dongjiu Geng <gengdongjiu1@...il.com>, Eric Blake
 <eblake@...hat.com>, Markus Armbruster <armbru@...hat.com>, Michael Roth
 <michael.roth@....com>, Paolo Bonzini <pbonzini@...hat.com>, Peter Maydell
 <peter.maydell@...aro.org>, "Shannon Zhao" <shannon.zhaosl@...il.com>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 10/11] qapi/acpi-hest: add an interface to do generic
 CPER error injection

On Thu, 23 Jan 2025 11:00:32 +0000
Jonathan Cameron <Jonathan.Cameron@...wei.com> wrote:

> On Wed, 22 Jan 2025 16:46:27 +0100
> Mauro Carvalho Chehab <mchehab+huawei@...nel.org> wrote:
> 
> > Creates a QMP command to be used for generic ACPI APEI hardware error
> > injection (HEST) via GHESv2, and add support for it for ARM guests.
> > 
> > Error injection uses ACPI_HEST_SRC_ID_QMP source ID to be platform
> > independent. This is mapped at arch virt bindings, depending on the
> > types supported by QEMU and by the BIOS. So, on ARM, this is supported
> > via ACPI_GHES_NOTIFY_GPIO notification type.
> > 
> > This patch is co-authored:
> >     - original ghes logic to inject a simple ARM record by Shiju Jose;
> >     - generic logic to handle block addresses by Jonathan Cameron;
> >     - generic GHESv2 error inject by Mauro Carvalho Chehab;
> > 
> > Co-authored-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> > Co-authored-by: Shiju Jose <shiju.jose@...wei.com>
> > Co-authored-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> > Signed-off-by: Shiju Jose <shiju.jose@...wei.com>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> > 
> > ---
> > 
> > Changes since v9:
> > - ARM source IDs renamed to reflect SYNC/ASYNC;
> > - command name changed to better reflect what it does;
> > - some improvements at JSON documentation;
> > - add a check for QMP source at the notification logic.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>  
> Another bonus.
> 
> Few trivial formatting comments, otherwise looks fine to me.
> 
> Jonathan
> 
> > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> > index 5d29db3918dd..cf83c959b5ef 100644
> > --- a/hw/acpi/ghes.c
> > +++ b/hw/acpi/ghes.c
> > @@ -547,7 +547,7 @@ void ghes_record_cper_errors(const void *cper, size_t len,
> >      /* Write the generic error data entry into guest memory */
> >      cpu_physical_memory_write(cper_addr, cper, len);
> >  
> > -    notifier_list_notify(&acpi_generic_error_notifiers, NULL);
> > +    notifier_list_notify(&acpi_generic_error_notifiers, &source_id);
> >  }
> >  
> >  int acpi_ghes_memory_errors(uint16_t source_id, uint64_t physical_address)
> > diff --git a/hw/acpi/ghes_cper.c b/hw/acpi/ghes_cper.c
> > new file mode 100644
> > index 000000000000..02c47b41b990
> > --- /dev/null
> > +++ b/hw/acpi/ghes_cper.c
> > @@ -0,0 +1,32 @@
> > +/*
> > + * CPER payload parser for error injection
> > + *
> > + * Copyright(C) 2024 Huawei LTD.

2025

> > + *
> > + * This code is licensed under the GPL version 2 or later. See the
> > + * COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "qemu/base64.h"
> > +#include "qemu/error-report.h"
> > +#include "qemu/uuid.h"
> > +#include "qapi/qapi-commands-acpi-hest.h"
> > +#include "hw/acpi/ghes.h"
> > +
> > +void qmp_inject_ghes_error(const char *qmp_cper, Error **errp)
> > +{
> > +  
> Odd blank line that can go.
> 
> > +    uint8_t *cper;
> > +    size_t  len;
> > +
> > +    cper = qbase64_decode(qmp_cper, -1, &len, errp);
> > +    if (!cper) {
> > +        error_setg(errp, "missing GHES CPER payload");
> > +        return;
> > +    }
> > +
> > +    ghes_record_cper_errors(cper, len, ACPI_HEST_SRC_ID_QMP, errp);
> > +}
> > diff --git a/hw/acpi/ghes_cper_stub.c b/hw/acpi/ghes_cper_stub.c
> > new file mode 100644
> > index 000000000000..8782e2c02fa8
> > --- /dev/null
> > +++ b/hw/acpi/ghes_cper_stub.c
> > @@ -0,0 +1,19 @@
> > +/*
> > + * Stub interface for CPER payload parser for error injection
> > + *
> > + * Copyright(C) 2024 Huawei LTD.

2025

> > + *
> > + * This code is licensed under the GPL version 2 or later. See the
> > + * COPYING file in the top-level directory.
> > + *  
> Trivial but I'd drop these trailing blank lines as they don't add
> anything other than making people scroll further.
> 
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "qapi/qapi-commands-acpi-hest.h"
> > +#include "hw/acpi/ghes.h"  
> 
> Trivial but doe we need ghes.h?
> 
> > +
> > +void qmp_inject_ghes_error(const char *cper, Error **errp)
> > +{
> > +    error_setg(errp, "GHES QMP error inject is not compiled in");
> > +}  
>