On Thu, 27 Feb 2025 12:45:38 +0100
Mauro Carvalho Chehab <mchehab+hua...@kernel.org> wrote:
> Em Wed, 26 Feb 2025 15:37:14 +0100
> Igor Mammedov <imamm...@redhat.com> escreveu:
>
> > On Fri, 21 Feb 2025 15:35:10 +0100
> > Mauro Carvalho Chehab <mchehab+hua...@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;
ACPI_GED() is not expensive, what I'm referring to is
object_resolve_path_type()
given it's a new code and virt_acpi_build() has direct access
to ged pointer, there is no excuse to use object_resolve_path_type().
all you have to do here is:
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index e6328af5d2..040d875d4e 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -949,8 +949,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables
*tables)
AcpiGedState *acpi_ged_state;
AcpiGhesState *ags;
- acpi_ged_state = ACPI_GED(object_resolve_path_type("", TYPE_ACPI_GED,
- NULL));
+ acpi_ged_state = ACPI_GED(vms->acpi_dev);
ags = &acpi_ged_state->ghes_state;
if (ags) {
acpi_add_table(table_offsets, tables_blob);
> - at acpi_ghes_get_state().
this one is different, it doesn't have access to ged so it
has to look up for it.
>
> 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
>
