Re: [PATCH v9 01/12] acpi/ghes: add a firmware file with HEST address

Mauro Carvalho Chehab
2024-09-13T05:44:40Z

Em Wed, 11 Sep 2024 15:51:08 +0200
Igor Mammedov <imammedo@redhat.com> escreveu:

> On Sun, 25 Aug 2024 05:45:56 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
>
> > Store HEST table address at GPA, placing its content at
> > hest_addr_le variable.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>
> This looks good to me.
>
> in addition to this, it needs a patch on top to make sure
> that we migrate hest_addr_le.
> See a08a64627b6b 'ACPI: Record the Generic Error Status Block address'
> and fixes on top of that for an example.

Hmm... If I understood such change well, vmstate_ghes_state() will
use this structure as basis to do migration:

/* ghes.h */
typedef struct AcpiGhesState {
uint64_t hest_addr_le;
uint64_t ghes_addr_le;
bool present; /* True if GHES is present at all on this board */
} AcpiGhesState;

/* generic_event_device.c */
static const VMStateDescription vmstate_ghes_state = {
.name = "acpi-ged/ghes",
.version_id = 1,
.minimum_version_id = 1,
.needed = ghes_needed,
.fields = (VMStateField[]) {
VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
vmstate_ghes_state, AcpiGhesState),
VMSTATE_END_OF_LIST()
}
};

/* hw/arm/virt-acpi-build.c */
void virt_acpi_setup(VirtMachineState *vms)
{
...
if (vms->ras) {
assert(vms->acpi_dev);
acpi_ged_state = ACPI_GED(vms->acpi_dev);
acpi_ghes_add_fw_cfg(&acpi_ged_state->ghes_state,
vms->fw_cfg, tables.hardware_errors);
}

Which relies on acpi_ghes_add_fw_cfg() function to setup callbacks for
the migration:

/* ghes.c */
void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState *s,
GArray *hardware_error)
{
/* Create a read-only fw_cfg file for GHES */
fw_cfg_add_file(s, ACPI_HW_ERROR_FW_CFG_FILE, hardware_error->data,
hardware_error->len);

/* Create a read-write fw_cfg file for Address */
fw_cfg_add_file_callback(s, ACPI_HW_ERROR_ADDR_FW_CFG_FILE, NULL, NULL,
NULL, &(ags->ghes_addr_le), sizeof(ags->ghes_addr_le), false);

fw_cfg_add_file_callback(s, ACPI_HEST_ADDR_FW_CFG_FILE, NULL, NULL,
NULL, &(ags->hest_addr_le), sizeof(ags->hest_addr_le), false);

ags->present = true;
}

It sounds to me that both ghes_addr_le and hest_addr_le will be migrated
altogether.

Did I miss something?

Thanks,
Mauro