On Thu, 1 Aug 2024 14:56:37 +0200 Mauro Carvalho Chehab <[email protected]> wrote:
> Em Tue, 30 Jul 2024 10:40:28 +0200 > Igor Mammedov <[email protected]> escreveu: > > > > diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h > > > index 674f6958e905..4f1ab1a73a06 100644 > > > --- a/include/hw/acpi/ghes.h > > > +++ b/include/hw/acpi/ghes.h > > > @@ -58,6 +58,7 @@ enum AcpiGhesNotifyType { > > > > > > enum { > > > ACPI_HEST_SRC_ID_SEA = 0, > > > + ACPI_HEST_SRC_ID_GPIO = 1, > > is it defined by some spec, or just a made up number? > > I don't know. Maybe Jonathan or Shiju knows better, as the original patch > came from them, but I didn't find any parts of the ACPI spec defining the > values for source ID. > > Checking at build_ghes_v2() implementation, this is used on two places: > > 1. as GHESv2 source ID: > /* > * Type: > * Generic Hardware Error Source version 2(GHESv2 - Type 10) > */ > build_append_int_noprefix(table_data, ACPI_GHES_SOURCE_GENERIC_ERROR_V2, > 2); > /* Source Id */ > build_append_int_noprefix(table_data, source_id, 2); > /* Related Source Id */ > build_append_int_noprefix(table_data, 0xffff, 2); > > as an address offset: > > address_offset = table_data->len; > /* Error Status Address */ > build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 0x40, 0, > 4 /* QWord access */, 0); > bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > address_offset + GAS_ADDR_OFFSET, sizeof(uint64_t), > ACPI_GHES_ERRORS_FW_CFG_FILE, source_id * sizeof(uint64_t)); > > So, if I had to guess, I'd say that this was made up, in a way that > the size of the table will fit just two sources, starting from zero. > > So, I'll change the code to just: > > enum { > ACPI_HEST_SRC_ID_SEA = 0, > ACPI_HEST_SRC_ID_GPIO, LGTM. The naming is perhaps not ideal but the scheme predates my involvement so I'm not sure of the reasoning. Could change it to QEMU_ACPI... to make it really really clear these aren't an ACPI spec thing, but may not be worth it. J > /* future ids go here */ > ACPI_HEST_SRC_ID_RESERVED, > }; > > To remove the false impression that this could be originated from the > spec. > > Thanks, > Mauro >
