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 <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 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;
-- 
2.20.1

On Sun, May 05, 2019 at 10:06:00PM +0200, 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 <lersek@redhat.com>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Wei Yang <richardw.yang@linux.intel.com>


-- 
Wei Yang
Help you, Help me

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 <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  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 <lersek@redhat.com>

Thanks
Laszlo