Re: [PATCH v2 09/10] esp: include the current PDMA callback in the migration stream

[Peter Maydell]

On Wed, 2 Mar 2022 at 21:38, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This involves (re)adding a PDMA-specific subsection to hold the reference to 
> the
> current PDMA callback.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/scsi/esp.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index a818b2b07a..32926834bc 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -1209,6 +1209,25 @@ static int esp_post_load(void *opaque, int version_id)
>      return 0;
>  }
>
> +static bool esp_pdma_needed(void *opaque)
> +{
> +    ESPState *s = ESP(opaque);
> +
> +    return s->dma_memory_read == NULL && s->dma_memory_write == NULL &&
> +           s->dma_enabled;

A comment about why this is the correct condition would be helpful.
If I understand it correctly, something like this ?

 /*
  * pdma_cb is used only by the sysbus ESP device, not the PCI ESP
  * device. The PCI device sets the s->dma_memory_read and
  * s->dma_memory_write function pointers at realize.
  */

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

> +}
> +
> +static const VMStateDescription vmstate_esp_pdma = {
> +    .name = "esp/pdma",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .needed = esp_pdma_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(pdma_cb, ESPState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  const VMStateDescription vmstate_esp = {
>      .name = "esp",
>      .version_id = 6,
> @@ -1243,6 +1262,10 @@ const VMStateDescription vmstate_esp = {
>          VMSTATE_UINT8_TEST(lun, ESPState, esp_is_version_6),
>          VMSTATE_END_OF_LIST()
>      },
> +    .subsections = (const VMStateDescription * []) {
> +        &vmstate_esp_pdma,
> +        NULL
> +    }
>  };

Do we need to do something similar to handle s->dma_cb ?

thanks
-- PMM