Re: [Qemu-arm] [Qemu-devel] [PATCH for 4.1?] pl330: fix vmstate description

[Philippe Mathieu-Daudé]

On 7/24/19 4:35 PM, Damien Hedde wrote:
> Fix the pl330 main and queue vmstate description.
> There were missing POINTER flags causing crashes during
> incoming migration because:
> + PL330State chan field is a pointer to an array
> + PL330Queue queue field is a pointer to an array
> 
> Also bump corresponding vmsd version numbers.
> 
> Signed-off-by: Damien Hedde <address@hidden>
> ---
> 
> I found this while working on reset with xilinx-zynq machine.
> 
> I'm not sure what's the vmsd version policy in such cases (for
> backward compatibility). I've simply bumped them since migration
> was not working anyway (vmstate_load_state was erasing critical part
> of PL330State and causing segfaults while loading following fields).

I still not understand versioning and migration, so I can't say, but
then you use the correct macro, since we have:

    s->chan = g_new0(PL330Chan, s->num_chnls);

So:
Reviewed-by: Philippe Mathieu-Daude <address@hidden>

> 
> Tested doing migration with the xilinx-zynq-a9 machine.
> 
> ---
>  hw/dma/pl330.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
> index 58df965a46..a56a3e7771 100644
> --- a/hw/dma/pl330.c
> +++ b/hw/dma/pl330.c
> @@ -218,11 +218,12 @@ typedef struct PL330Queue {
>  
>  static const VMStateDescription vmstate_pl330_queue = {
>      .name = "pl330_queue",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .fields = (VMStateField[]) {
> -        VMSTATE_STRUCT_VARRAY_UINT32(queue, PL330Queue, queue_size, 1,
> -                                 vmstate_pl330_queue_entry, PL330QueueEntry),
> +        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(queue, PL330Queue, queue_size,
> +                                             vmstate_pl330_queue_entry,
> +                                             PL330QueueEntry),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -278,12 +279,12 @@ struct PL330State {
>  
>  static const VMStateDescription vmstate_pl330 = {
>      .name = "pl330",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .fields = (VMStateField[]) {
>          VMSTATE_STRUCT(manager, PL330State, 0, vmstate_pl330_chan, 
> PL330Chan),
> -        VMSTATE_STRUCT_VARRAY_UINT32(chan, PL330State, num_chnls, 0,
> -                                     vmstate_pl330_chan, PL330Chan),
> +        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(chan, PL330State, num_chnls,
> +                                             vmstate_pl330_chan, PL330Chan),
>          VMSTATE_VBUFFER_UINT32(lo_seqn, PL330State, 1, NULL, num_chnls),
>          VMSTATE_VBUFFER_UINT32(hi_seqn, PL330State, 1, NULL, num_chnls),
>          VMSTATE_STRUCT(fifo, PL330State, 0, vmstate_pl330_fifo, PL330Fifo),
>