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

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

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