Re: [Qemu-devel] [PATCH 3/5] hw/block/pflash_cfi01: Add the DeviceReset() handler

[Laszlo Ersek]

On 05/05/19 22:06, Philippe Mathieu-Daudé wrote:
> The pflash device is a child of TYPE_DEVICE, so it can implement
> the DeviceReset handler. Actually it has to implement it, else
> on machine reset it might stay in an incoherent state, as it has
> been reported in the buglink listed below.
> 
> Add the DeviceReset handler and remove its call from the realize()
> function.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1678713
> Reported-by: Laszlo Ersek <address@hidden>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  hw/block/pflash_cfi01.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 073cd14978f..639b05bc4d5 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -762,7 +762,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
> **errp)
>          pfl->max_device_width = pfl->device_width;
>      }
>  
> -    pflash_reset(pfl);
>      /* Hardcoded CFI table */
>      /* Standard "QRY" string */
>      pfl->cfi_table[0x10] = 'Q';
> @@ -850,6 +849,11 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
> **errp)
>      pfl->cfi_table[0x3f] = 0x01; /* Number of protection fields */
>  }
>  
> +static void pflash_cfi01_reset(DeviceState *dev)
> +{
> +    pflash_reset(PFLASH_CFI01(dev));
> +}
> +
>  static Property pflash_cfi01_properties[] = {
>      DEFINE_PROP_DRIVE("drive", PFlashCFI01, blk),
>      /* num-blocks is the number of blocks actually visible to the guest,
> @@ -894,6 +898,7 @@ static void pflash_cfi01_class_init(ObjectClass *klass, 
> void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> +    dc->reset = pflash_cfi01_reset;
>      dc->realize = pflash_cfi01_realize;
>      dc->props = pflash_cfi01_properties;
>      dc->vmsd = &vmstate_pflash;
> 

(Hopefully I'm not missing anything obvious, while writing the below:)

Sometimes the object model is really nice.

Reviewed-by: Laszlo Ersek <address@hidden>

Thanks
Laszlo