* Igor Mammedov (imamm...@redhat.com) wrote:
> On Mon, 19 Oct 2020 17:31:56 +0800
> Peng Liang <liangpen...@huawei.com> wrote:
>
> > There is a field with vmstate_ghes_state as vmsd in vmstate_ghes_state,
> > which will lead to infinite recursion in dump_vmstate_vmsd.
> >
> > Fixes: a08a64627b ("ACPI: Record the Generic Error Status Block address")
> > Reported-by: Euler Robot <euler.ro...@huawei.com>
> > Signed-off-by: Peng Liang <liangpen...@huawei.com>
> > ---
> > hw/acpi/generic_event_device.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > index 6df400e1ee16..4b6867300a55 100644
> > --- a/hw/acpi/generic_event_device.c
> > +++ b/hw/acpi/generic_event_device.c
> > @@ -334,8 +334,7 @@ static const VMStateDescription vmstate_ghes_state = {
> > .minimum_version_id = 1,
> > .needed = ghes_needed,
> > .fields = (VMStateField[]) {
> > - VMSTATE_STRUCT(ghes_state, AcpiGedState, 1,
> > - vmstate_ghes_state, AcpiGhesState),
> > + VMSTATE_UINT64(ghes_state.ghes_addr_le, AcpiGedState),
>
> not sure its' ok handle it this way,
>
> see how it is done with another structure:
>
> static const VMStateDescription vmstate_ged_state = {
>
> .name = "acpi-ged-state",
>
> .version_id = 1,
>
> .minimum_version_id = 1,
>
> .fields = (VMStateField[]) {
>
> VMSTATE_UINT32(sel, GEDState),
>
> VMSTATE_END_OF_LIST()
>
> }
>
> };
>
> ...
>
> VMSTATE_STRUCT(ged_state, AcpiGedState, 1, vmstate_ged_state, GEDState),
>
> i.e. it looks like we are missing structure definition for AcpiGhesState
>
> CCing David,
> to help with migration magic in case I'm wrong or missed something
Yeh that's confusing :-)
Given a:
VMSTATE_STRUCT(a, B, 1, vmstate_c, C)
We're saying there's a field 'a' in type B, and field 'a'
should be of type C and be serialised using vmstate_c.
That also means that in any vmstate_c, we're expecting it
to be passed a type C generally.
Having said that; you don't need a struct - you can get away
with that VMSTATE_UINT64, there's two problems:
a) That assumes that your ghes always stays that simple.
b) If you wanted to store a Ghes from a number of different
parent structures then you're stuck because your vmstate_ghes_state
is bound to being a strict field of AcpiGedState.
So yes, it's neatest to do it using a VMSD for AcpiGhesState
And congratulations on finding a loop; I don't think we've ever had one
before :-)
Dave
> > VMSTATE_END_OF_LIST()
> > }
> > };
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
