Re: [Qemu-devel] [PATCH for-2.4 1/2] core: reset handler for bus-less devices

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Mon, 13 Jul 2015 16:20:09 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 13.07.2015 um 16:11 schrieb Cornelia Huck:
> > On Mon, 13 Jul 2015 14:22:05 +0200
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > 
> >> Am 09.07.2015 um 18:51 schrieb Cornelia Huck:
> >>> Devices that don't live on a bus aren't caught by the normal device
> >>> reset logic. Let's register a reset handler for those devices during
> >>> device realization that calls the reset handler for the associated
> >>> device class.
> >>>
> >>> Suggested-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> >> reboot (from within guest) and external reset (system_reset in monitor)
> >> now work fine with the s390 watchdog.
> >>
> >> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>
> >>> ---
> >>>  hw/core/qdev.c | 15 +++++++++++++++
> >>>  1 file changed, 15 insertions(+)
> > 
> > Thanks.
> > 
> > Any objections against taking this through s390-next? I'd like to fix
> > diag288 reset (+ that annoying migration regession) for 2.4-rc1 and
> > send a pull request soon.
> 
> Which device does this fix (only this diag88?), and is it really not
> possible to register a reset handler where it's being created?

The original patch did this
(<1436259202-20509-2-git-send-email-cornelia.huck@de.ibm.com>). Peter
C. suspected NAND may also be affected
(<CAEgOgz6Fa5TSApDrj9iHA+2joDt1yBg6amCAp-634Bbe5bgNvA@mail.gmail.com>).

> 
> Peter C.'s theory does not match practice for x86, and this patch will
> lead to bus-less devices that are properly being reset by their parent
> getting reset twice, potentially causing issues due to qemu_irqs. I'd
> rather avoid that.

Introducing new bugs is not something I want to do. Are double resets a
problem in practice, though?

> 
> One workaround would be to amend this patch with a DeviceClass flag for
> whether to enable this new behavior, defaulting to no and getting
> overridden by your affected device.

That is still something that can be easily missed. Providing a reset
handler that isn't called until you do some magical incantations is
somewhat surprising.

But in the end, what I care about is that resetting diag288 works with
2.4 :)