Re: [PATCH v3 03/14] acpi/ghes: Use HEST table offsets when preparing GHES records

[Mauro Carvalho Chehab]

Em Mon, 3 Feb 2025 15:34:23 +0100
Igor Mammedov <imammedo@xxxxxxxxxx> escreveu:

> On Fri, 31 Jan 2025 18:42:44 +0100
> Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote:
> 
> > There are two pointers that are needed during error injection:
> > 
> > 1. The start address of the CPER block to be stored;
> > 2. The address of the ack.
> > 
> > It is preferable to calculate them from the HEST table.  This allows
> > checking the source ID, the size of the table and the type of the
> > HEST error block structures.
> > 
> > Yet, keep the old code, as this is needed for migration purposes.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
> > ---
> >  hw/acpi/ghes.c         | 132 ++++++++++++++++++++++++++++++++++++-----
> >  include/hw/acpi/ghes.h |   1 +
> >  2 files changed, 119 insertions(+), 14 deletions(-)
> > 
> > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> > index 27478f2d5674..8f284fd191a6 100644
> > --- a/hw/acpi/ghes.c
> > +++ b/hw/acpi/ghes.c
> > @@ -41,6 +41,12 @@
> >  /* Address offset in Generic Address Structure(GAS) */
> >  #define GAS_ADDR_OFFSET 4
> >  
> > +/*
> > + * ACPI spec 1.0b
> > + * 5.2.3 System Description Table Header
> > + */
> > +#define ACPI_DESC_HEADER_OFFSET     36
> > +
> >  /*
> >   * The total size of Generic Error Data Entry
> >   * ACPI 6.1/6.2: 18.3.2.7.1 Generic Error Data,
> > @@ -61,6 +67,25 @@
> >   */
> >  #define ACPI_GHES_GESB_SIZE                 20
> >  
> > +/*
> > + * Offsets with regards to the start of the HEST table stored at
> > + * ags->hest_addr_le,  
> 
> If I read this literary, then offsets above are not what
> declared later in this patch.
> I'd really drop this comment altogether as it's confusing,
> and rather get variables/macro naming right
> 
> > according with the memory layout map at
> > + * docs/specs/acpi_hest_ghes.rst.
> > + */  
> 
> what we need is update to above doc, describing new and old ways.
> a separate patch.

I can't see anything that should be changed at
docs/specs/acpi_hest_ghes.rst, as this series doesn't change the
firmware layout: we're still using two firmware tables:

- etc/acpi/tables, with HEST on it;
- etc/hardware_errors, with:
	- error block addresses;
	- read_ack registers;
	- CPER records.

The only changes that this series introduce are related to how
the error generation logic navigates between HEST and hw_errors
firmware. This is not described at acpi_hest_ghes.rst, and both
ways follow ACPI specs to the letter.

The only difference is that the code which populates the CPER
record and the error/read offsets doesn't require to know how
the HEST table generation placed offsets, as it will basically
reproduce what OSPM firmware does when handling	HEST events.

Thanks,
Mauro