[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240730073447-mutt-send-email-mst@kernel.org>
Date: Tue, 30 Jul 2024 07:36:32 -0400
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Igor Mammedov <imammedo@...hat.com>
Cc: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Shiju Jose <shiju.jose@...wei.com>, Ani Sinha <anisinha@...hat.com>,
Dongjiu Geng <gengdongjiu1@...il.com>, linux-kernel@...r.kernel.org,
qemu-arm@...gnu.org, qemu-devel@...gnu.org
Subject: Re: [PATCH v3 6/7] acpi/ghes: update comments to point to newer ACPI
specs
On Tue, Jul 30, 2024 at 01:24:30PM +0200, Igor Mammedov wrote:
> On Mon, 22 Jul 2024 08:45:58 +0200
> Mauro Carvalho Chehab <mchehab+huawei@...nel.org> wrote:
>
> > There is one reference to ACPI 4.0 and several references
> > to ACPI 6.x versions.
> >
> > Update them to point to ACPI 6.5 whenever possible.
>
> when it comes to APCI doc comments, they should point to
> the 1st (earliest) revision that provides given feature/value/field/table.
Yes. And the motivation is twofold.
First, guests are built against
old acpi versions. knowing in which version things appeared
helps us know which guests support a feature.
Second, acpi guys keep churning out new versions.
It makes no sense to try and update to latest one,
it will soon get out of date again.
>
> > There's one reference that was kept pointing to ACPI 6.4,
> > though, with HEST revision 1.
> >
> > ACPI 6.5 now defines HEST revision 2, and defined a new
> > way to handle source types starting from 12. According
> > with ACPI 6.5 revision history:
> >
> > 2312 Update to the HEST table and adding new error
> > source descriptor - Table 18.2.
> >
> > Yet, the spec doesn't define yet any new source
> > descriptors. It just defines a different behavior when
> > source type is above 11.
> >
> > I also double-checked GHES implementation on an open
> > source project (Linux Kernel). Currently upstream
> > doesn't currently handle HEST revision, ignoring such
> > field.
> >
> > In any case, revision 2 seems to be backward-compatible
> > with revison 1 when type <= 11 and just one error is
> > contained on a HEST record.
> >
> > So, while it is probably safe to update it, there's no
> > real need. So, let's keep the implementation using
> > an ACPI 6.4 compatible table, e. g. HEST revision 1.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> > ---
> > hw/acpi/ghes.c | 48 ++++++++++++++++++++++++++++--------------------
> > 1 file changed, 28 insertions(+), 20 deletions(-)
> >
> > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> > index 6075ef5893ce..ebf1b812aaaa 100644
> > --- a/hw/acpi/ghes.c
> > +++ b/hw/acpi/ghes.c
> > @@ -45,9 +45,9 @@
> > #define GAS_ADDR_OFFSET 4
> >
> > /*
> > - * The total size of Generic Error Data Entry
> > - * ACPI 6.1/6.2: 18.3.2.7.1 Generic Error Data,
> > - * Table 18-343 Generic Error Data Entry
> > + * The total size of Generic Error Data Entry before data field
> > + * ACPI 6.5: 18.3.2.7.1 Generic Error Data,
> > + * Table 18.12 Generic Error Data Entry
> > */
> > #define ACPI_GHES_DATA_LENGTH 72
> >
> > @@ -65,8 +65,8 @@
> >
> > /*
> > * Total size for Generic Error Status Block except Generic Error Data Entries
> > - * ACPI 6.2: 18.3.2.7.1 Generic Error Data,
> > - * Table 18-380 Generic Error Status Block
> > + * ACPI 6.5: 18.3.2.7.1 Generic Error Data,
> > + * Table 18.11 Generic Error Status Block
> > */
> > #define ACPI_GHES_GESB_SIZE 20
> >
> > @@ -82,7 +82,8 @@ enum AcpiGenericErrorSeverity {
> >
> > /*
> > * Hardware Error Notification
> > - * ACPI 4.0: 17.3.2.7 Hardware Error Notification
> > + * ACPI 6.5: 18.3.2.9 Hardware Error Notification,
> > + * Table 18.14 - Hardware Error Notification Structure
> > * Composes dummy Hardware Error Notification descriptor of specified type
> > */
> > static void build_ghes_hw_error_notification(GArray *table, const uint8_t type)
> > @@ -112,7 +113,8 @@ static void build_ghes_hw_error_notification(GArray *table, const uint8_t type)
> >
> > /*
> > * Generic Error Data Entry
> > - * ACPI 6.1: 18.3.2.7.1 Generic Error Data
> > + * ACPI 6.5: 18.3.2.7.1 Generic Error Data,
> > + * Table 18.12 - Generic Error Data Entry
> > */
> > static void acpi_ghes_generic_error_data(GArray *table,
> > const uint8_t *section_type, uint32_t error_severity,
> > @@ -148,7 +150,8 @@ static void acpi_ghes_generic_error_data(GArray *table,
> >
> > /*
> > * Generic Error Status Block
> > - * ACPI 6.1: 18.3.2.7.1 Generic Error Data
> > + * ACPI 6.5: 18.3.2.7.1 Generic Error Data,
> > + * Table 18.11 - Generic Hardware Error Source Structure
> > */
> > static void acpi_ghes_generic_error_status(GArray *table, uint32_t block_status,
> > uint32_t raw_data_offset, uint32_t raw_data_length,
> > @@ -429,15 +432,18 @@ void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker)
> > 0, sizeof(uint64_t), ACPI_GHES_ERRORS_FW_CFG_FILE, 0);
> > }
> >
> > -/* Build Generic Hardware Error Source version 2 (GHESv2) */
> > +/*
> > + * Build Generic Hardware Error Source version 2 (GHESv2)
> > + * ACPI 6.5: 18.3.2.8 Generic Hardware Error Source version 2 (GHESv2 - Type 10),
> > + * Table 18.13: Generic Hardware Error Source version 2 (GHESv2)
> > + */
> > static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker)
> > {
> > uint64_t address_offset;
> > - /*
> > - * Type:
> > - * Generic Hardware Error Source version 2(GHESv2 - Type 10)
> > - */
> > + /* Type: (GHESv2 - Type 10) */
> > build_append_int_noprefix(table_data, ACPI_GHES_SOURCE_GENERIC_ERROR_V2, 2);
> > +
> > + /* ACPI 6.5: Table 18.10 - Generic Hardware Error Source Structure */
> > /* Source Id */
> > build_append_int_noprefix(table_data, source_id, 2);
> > /* Related Source Id */
> > @@ -481,11 +487,8 @@ static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker)
> > /* Error Status Block Length */
> > build_append_int_noprefix(table_data, ACPI_GHES_MAX_RAW_DATA_LENGTH, 4);
> >
> > - /*
> > - * Read Ack Register
> > - * ACPI 6.1: 18.3.2.8 Generic Hardware Error Source
> > - * version 2 (GHESv2 - Type 10)
> > - */
> > + /* ACPI 6.5: fields defined at GHESv2 table */
> > + /* Read Ack Register */
> > address_offset = table_data->len;
> > build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 0x40, 0,
> > 4 /* QWord access */, 0);
> > @@ -504,11 +507,16 @@ static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker)
> > build_append_int_noprefix(table_data, 0x1, 8);
> > }
> >
> > -/* Build Hardware Error Source Table */
> > +/*
> > + * Build Hardware Error Source Table
> > + * ACPI 6.4: 18.3.2 ACPI Error Source
> > + * Table 18.2: Hardware Error Source Table (HEST)
> > + */
> > void acpi_build_hest(GArray *table_data, BIOSLinker *linker,
> > const char *oem_id, const char *oem_table_id)
> > {
> > - AcpiTable table = { .sig = "HEST", .rev = 1,
> > + AcpiTable table = { .sig = "HEST",
> > + .rev = 1, /* ACPI 4.0 to 6.4 */
> > .oem_id = oem_id, .oem_table_id = oem_table_id };
> >
> > acpi_table_begin(&table, table_data);
Powered by blists - more mailing lists