Re: [PATCH v4 01/14] acpi/ghes: prepare to change the way HEST offsets are calculated

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Wed, 26 Feb 2025 15:37:14 +0100
Igor Mammedov <imammedo@redhat.com> escreveu:

> On Fri, 21 Feb 2025 15:35:10 +0100
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> 

> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 3ac8f8e17861..8ab8d11b6536 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -946,9 +946,18 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
> >      build_dbg2(tables_blob, tables->linker, vms);
> >  
> >      if (vms->ras) {
> > -        acpi_add_table(table_offsets, tables_blob);
> > -        acpi_build_hest(tables_blob, tables->hardware_errors, tables->linker,
> > -                        vms->oem_id, vms->oem_table_id);
> > +        AcpiGedState *acpi_ged_state;
> > +        AcpiGhesState *ags;
> > +
> > +        acpi_ged_state = ACPI_GED(object_resolve_path_type("", TYPE_ACPI_GED,  
>                             ^^^ will explode if object_resolve_path_type() returns NULL
> > +                                                       NULL));  
> 
> it's also expensive load-wise.
> You have access to vms with ged pointer here, use that
> (search for 'acpi_ged_state = ACPI_GED' example)

Ok, but the state binding on ghes were designed to use ACPI_GED. I moved
the code that it is using ACPI_GED() to the beginning of v5 series,
just after the HEST table test addition.

With that, ACPI_GED() is now used only on two places inside ghes:

- at virt_acpi_build(), during VM initialization;
- at acpi_ghes_get_state().

If you want to replace it by some other solution, IMO we should do
it on some separate series, as this is not related to neither error
injection nor with offset calculation to get read ack address. 

> > +        if (acpi_ged_state) {  
> 
>                 hence, this check is not really needed,
>                 we have to have GED at this point or abort
> 
>                 earlier code that instantiates GED should take care of
>                 cleanly exiting if it failed to create GED so we would never get
>                 to missing GED here

I dropped this check on v5.

Thanks,
Mauro