| Message ID | bed4b2da51e0c894cc255f712b67e2e57295d826.1726293808.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add ACPI CPER firmware first error injection on ARM emulation | expand |
On Sat, 14 Sep 2024 08:13:23 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > The GHES migration logic at GED should now support HEST table > location too. > > Increase migration version and change needed to check for both > ghes_addr_le and hest_addr_le But I don't think it will work like this (but I might be easily wrong) However I don't know enough to properly review this patch, CCing Peter & Fabiano > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > hw/acpi/generic_event_device.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index 15b4c3ebbf24..4e5e387ee2df 100644 > --- a/hw/acpi/generic_event_device.c > +++ b/hw/acpi/generic_event_device.c > @@ -343,10 +343,11 @@ static const VMStateDescription vmstate_ged_state = { > > static const VMStateDescription vmstate_ghes = { > .name = "acpi-ghes", > - .version_id = 1, > - .minimum_version_id = 1, > + .version_id = 2, > + .minimum_version_id = 2, > .fields = (const VMStateField[]) { > VMSTATE_UINT64(ghes_addr_le, AcpiGhesState), > + VMSTATE_UINT64(hest_addr_le, AcpiGhesState), > VMSTATE_END_OF_LIST() > }, > }; > @@ -354,13 +355,13 @@ static const VMStateDescription vmstate_ghes = { > static bool ghes_needed(void *opaque) > { > AcpiGedState *s = opaque; > - return s->ghes_state.ghes_addr_le; > + return s->ghes_state.ghes_addr_le && s->ghes_state.hest_addr_le; > } what I would do: add ghes_needed_v2(): return s->ghes_state.hest_addr_le; and then instead of reusing vmstate_ghes_state would add new vmstate_ghes_v2_state subsection that migrates only VMSTATE_UINT64(hest_addr_le, AcpiGhesState) field. btw: we probably don't need ghes_addr_le for new code that uses HEST to lookup relevant error status block. but we should still keep it for 9.1 and older machine types as they expect/use it. Separate subsections would work with this req just fine. > static const VMStateDescription vmstate_ghes_state = { > .name = "acpi-ged/ghes", > - .version_id = 1, > - .minimum_version_id = 1, > + .version_id = 2, > + .minimum_version_id = 2, > .needed = ghes_needed, > .fields = (const VMStateField[]) { > VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
On Sat, 14 Sep 2024 08:13:23 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > The GHES migration logic at GED should now support HEST table > location too. > > Increase migration version and change needed to check for both > ghes_addr_le and hest_addr_le. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > hw/acpi/generic_event_device.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index 15b4c3ebbf24..4e5e387ee2df 100644 > --- a/hw/acpi/generic_event_device.c > +++ b/hw/acpi/generic_event_device.c > @@ -343,10 +343,11 @@ static const VMStateDescription vmstate_ged_state = { > > static const VMStateDescription vmstate_ghes = { > .name = "acpi-ghes", > - .version_id = 1, > - .minimum_version_id = 1, > + .version_id = 2, > + .minimum_version_id = 2, > .fields = (const VMStateField[]) { > VMSTATE_UINT64(ghes_addr_le, AcpiGhesState), > + VMSTATE_UINT64(hest_addr_le, AcpiGhesState), > VMSTATE_END_OF_LIST() > }, > }; > @@ -354,13 +355,13 @@ static const VMStateDescription vmstate_ghes = { > static bool ghes_needed(void *opaque) > { > AcpiGedState *s = opaque; > - return s->ghes_state.ghes_addr_le; ^^^^^^^^^^^^ another thing, perhaps we should rename it to 'hardware_errors_addr' to make it less confusing > + return s->ghes_state.ghes_addr_le && s->ghes_state.hest_addr_le; > } > > static const VMStateDescription vmstate_ghes_state = { > .name = "acpi-ged/ghes", > - .version_id = 1, > - .minimum_version_id = 1, > + .version_id = 2, > + .minimum_version_id = 2, > .needed = ghes_needed, > .fields = (const VMStateField[]) { > VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
On Tue, Sep 17, 2024 at 11:19:21AM +0200, Igor Mammedov wrote: > On Sat, 14 Sep 2024 08:13:23 +0200 > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > The GHES migration logic at GED should now support HEST table > > location too. > > > > Increase migration version and change needed to check for both > > ghes_addr_le and hest_addr_le > But I don't think it will work like this (but I might be easily wrong) > However I don't know enough to properly review this patch, CCing Peter & Fabiano > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > --- > > hw/acpi/generic_event_device.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > > index 15b4c3ebbf24..4e5e387ee2df 100644 > > --- a/hw/acpi/generic_event_device.c > > +++ b/hw/acpi/generic_event_device.c > > @@ -343,10 +343,11 @@ static const VMStateDescription vmstate_ged_state = { > > > > static const VMStateDescription vmstate_ghes = { > > .name = "acpi-ghes", > > - .version_id = 1, > > - .minimum_version_id = 1, > > + .version_id = 2, > > + .minimum_version_id = 2, > > .fields = (const VMStateField[]) { > > VMSTATE_UINT64(ghes_addr_le, AcpiGhesState), > > + VMSTATE_UINT64(hest_addr_le, AcpiGhesState), > > VMSTATE_END_OF_LIST() > > }, > > }; > > @@ -354,13 +355,13 @@ static const VMStateDescription vmstate_ghes = { > > static bool ghes_needed(void *opaque) > > { > > AcpiGedState *s = opaque; > > - return s->ghes_state.ghes_addr_le; > > + return s->ghes_state.ghes_addr_le && s->ghes_state.hest_addr_le; > > } > > what I would do: > add ghes_needed_v2(): return s->ghes_state.hest_addr_le; > > and then instead of reusing vmstate_ghes_state would add new > vmstate_ghes_v2_state subsection that migrates only > VMSTATE_UINT64(hest_addr_le, AcpiGhesState) > field. > > btw: we probably don't need ghes_addr_le for new code that > uses HEST to lookup relevant error status block. > but we should still keep it for 9.1 and older machine types > as they expect/use it. Separate subsections would work with > this req just fine. Right, if we need bi-directional migration we need above and a compat property (or VMSTATE_UINT64_TEST() would work too, iiuc). OTOH VMSD versioning only works for forward migration, not backward. > > > static const VMStateDescription vmstate_ghes_state = { > > .name = "acpi-ged/ghes", > > - .version_id = 1, > > - .minimum_version_id = 1, > > + .version_id = 2, > > + .minimum_version_id = 2, (and IIUC if we set min ver=2, even forward migration should fail.. better test it with an old binary, migrating back and forth) > > .needed = ghes_needed, > > .fields = (const VMStateField[]) { > > VMSTATE_STRUCT(ghes_state, AcpiGedState, 1, > Thanks,
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c index 15b4c3ebbf24..4e5e387ee2df 100644 --- a/hw/acpi/generic_event_device.c +++ b/hw/acpi/generic_event_device.c @@ -343,10 +343,11 @@ static const VMStateDescription vmstate_ged_state = { static const VMStateDescription vmstate_ghes = { .name = "acpi-ghes", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (const VMStateField[]) { VMSTATE_UINT64(ghes_addr_le, AcpiGhesState), + VMSTATE_UINT64(hest_addr_le, AcpiGhesState), VMSTATE_END_OF_LIST() }, }; @@ -354,13 +355,13 @@ static const VMStateDescription vmstate_ghes = { static bool ghes_needed(void *opaque) { AcpiGedState *s = opaque; - return s->ghes_state.ghes_addr_le; + return s->ghes_state.ghes_addr_le && s->ghes_state.hest_addr_le; } static const VMStateDescription vmstate_ghes_state = { .name = "acpi-ged/ghes", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .needed = ghes_needed, .fields = (const VMStateField[]) { VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
The GHES migration logic at GED should now support HEST table location too. Increase migration version and change needed to check for both ghes_addr_le and hest_addr_le. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> --- hw/acpi/generic_event_device.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)