| Message ID | 20240209123226.32576-1-philmd@linaro.org |
|---|---|
| Headers | show |
| Series | hw/qdev: Split 'wiring' phase from 'realize' | expand |
Hello, Instead of adding a new method to devices, why not move wiring to the code that created/realized the device? Either it's the job of the code that created the device and wants to use it or if it's an internal object the device itself is created then it should be OK to wire it in the device realize method. I probably don't understand the problem fully, what I want to say is that device creation is complex enough now so trying to avoid adding more complexity to it would be a good thing. Regards, BALATON Zoltan
Am 9. Februar 2024 12:32:18 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >Hi, > >Various issues related to implementing dynamic machines have >been documented in [1]. > >We are trying to understand what means "a qdev is realized". >One explanation was "the device is guest visible"; however >many devices are realized before being mapped, thus are not >"guest visible". Some devices map / wire their IRQs before >being realized (such ISA devices). There is a need for devices >to be "automatically" mapped/wired (see [2]) such CLI-created >devices, but this apply generically to dynamic machines. > >Currently the device creation steps are expected to roughly be: > > (external use) (QDev core) (Device Impl) > ~~~~~~~~~~~~ ~~~~~~~~~ ~~~~~~~~~~~ > > INIT enter > -----> > +----------------------+ > | Allocate state | > +----------------------+ > -----> > +---------------------+ > | INIT children | > | | > | Alias children properties > | | > | Expose properties | > INIT exit +---------------------+ > <----------------------------------- > +----------------+ > | set properties | > | | > | set ClkIn | > +----------------+ REALIZE enter > ----------------------------------> > +----------------------+ > | Use config properties| > | | > | Realize children | > | | > | Init GPIOs/IRQs | > | | > | Init MemoryRegions | > +----------------------+ > REALIZE exit > <----------------------------------- ---- "realized" / "guest visible" >+-----------------+ >| Explicit wiring:| >| IRQs | >| I/O / Mem | >| ClkOut | >+-----------------+ RESET enter > ---------------------------------> > +----------------------+ > | Reset default values | > +----------------------+ > >But as mentioned, various devices "wire" parts before they exit >the "realize" step. >In order to clarify, I'm trying to enforce what can be done >*before* and *after* realization. > >*after* a device is expected to be stable (no more configurable) >and fully usable. > >To be able to use internal/auto wiring (such ISA devices) and >keep the current external/explicit wiring, I propose to add an >extra "internal wiring" step, happening after the REALIZE step >as: > > (external use) (QDev core) (Device Impl) > ~~~~~~~~~~~~ ~~~~~~~~~ ~~~~~~~~~~~ > > INIT enter > -----> > +----------------------+ > | Allocate state | > +----------------------+ > -----> > +---------------------+ > | INIT children | > | | > | Alias children properties > | | > | Expose properties | > INIT exit +---------------------+ > <----------------------------------- > +----------------+ > | set properties | > | | > | set ClkIn | > +----------------+ REALIZE enter > ----------------------------------> > +----------------------+ > | Use config properties| > | | > | Realize children | > | | > | Init GPIOs/IRQs | > | | > | Init MemoryRegions | > +----------------------+ > REALIZE exit <--- > +----------------------+ > | Internal auto wiring | > | IRQs | (i.e. ISA bus) > | I/O / Mem | > | ClkOut | > +----------------------+ > <--- ---- "realized" >+-----------------+ >| External wiring:| >| IRQs | >| I/O / Mem | >| ClkOut | >+-----------------+ RESET enter ---- "guest visible" > ---------------------------------> > +----------------------+ > | Reset default values | > +----------------------+ > >The "realized" point is not changed. "guest visible" concept only >occurs *after* wiring, just before the reset phase. > >This series introduces the DeviceClass::wire handler within qdev >core realization code, and convert devices using the implicit >wiring to using that explicit handler. > >QDev API assertions patches will be posted later as another series. > >Thoughts? Hi Phil, What exactly are you trying to achieve? I assume that the goal is to make devices user-creatable which currently can't due to current limitations. For this I'm presenting an idea: I'd probably not force the new discipline onto devices which aren't user-creatable -- that seems like too much gymnastics to me for no gain. Instead, I'd act on the suggestion of having an interface TYPE_USER_CREATABLE and put the wire method there. This wire method would be responsible for performing *all* duties between the states "realized" and "guest-visible". Based on the parameters it is given by the user, it would find all resources it needs to wire up the device. It finds these resources by e.g. scanning the QOM tree. A good practice would be to pass the root node for the scanning as parameter, e.g. the current machine. It could happen that a user-crrated device depends on another user-created device. To make this work, the part of QEMU that deals with user-created devices would perform a fixed point iteration over all not yet finished user-created devices. When there is no progress within one iteration, QEMU aborts with an error message. This error message could come from the `wire` invocation of any -- or even better -- all not yet finished devices. To demonstrate the approach, one could probably pick just one device, add the interface and hack up something in the core handling of user-created devices. Does this idea sound reasonable? Is this what you're ultimately after? Best regards, Bernhatd > >Regards, > >Phil. > >[1] https://lore.kernel.org/all/87o7d1i7ky.fsf@pond.sub.org/ >[2] https://lore.kernel.org/qemu-devel/20231127052024.435743-1-gustavo.romero@linaro.org/ > >Cc: Eduardo Habkost <eduardo@habkost.net> >Cc: Markus Armbruster <armbru@redhat.com>' --cc ' >Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>' >Cc: Richard Henderson <richard.henderson@linaro.org> >Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >Cc: Thomas Huth <thuth@redhat.com> >Cc: Paolo Bonzini <pbonzini@redhat.com> >Cc: Alexander Graf <agraf@csgraf.de> >Cc: Bernhard Beschow <shentey@gmail.com> >Cc: Stefan Hajnoczi <stefanha@redhat.com> >Cc: Peter Maydell <peter.maydell@linaro.org> >Cc: Cédric Le Goater <clg@kaod.org> >Cc: Luc Michel <luc.michel@amd.com> >Cc: Zhao Liu <zhao1.liu@linux.intel.com> >Cc: Gustavo Romero <gustavo.romero@linaro.org> > >Philippe Mathieu-Daudé (7): > hw/qdev: Introduce DeviceClass::[un]wire() handlers > hw/input/pckbd: Connect i8042 GPIOs once mouse/keyboard are realized > hw/ide/cmd646: Configure IDE bus IRQs after realization > hw/ide/sii3112: Configure IDE bus IRQs after realization > hw/ide/via: Configure IDE bus IRQs after realization > hw/intc/mips_gic: Initialize IRQ array once device is realized > hw/misc/mac_via: Have VIA1 child access parent IRQ once realized > > include/hw/qdev-core.h | 8 +++++++- > hw/core/qdev.c | 21 ++++++++++++++++++++- > hw/ide/cmd646.c | 12 +++++++++++- > hw/ide/sii3112.c | 10 ++++++++++ > hw/ide/via.c | 10 ++++++++++ > hw/input/pckbd.c | 38 +++++++++++++++++++++++++++----------- > hw/intc/mips_gic.c | 11 +++++++++-- > hw/misc/mac_via.c | 9 ++++++++- > 8 files changed, 102 insertions(+), 17 deletions(-) >
Hi, Various issues related to implementing dynamic machines have been documented in [1]. We are trying to understand what means "a qdev is realized". One explanation was "the device is guest visible"; however many devices are realized before being mapped, thus are not "guest visible". Some devices map / wire their IRQs before being realized (such ISA devices). There is a need for devices to be "automatically" mapped/wired (see [2]) such CLI-created devices, but this apply generically to dynamic machines. Currently the device creation steps are expected to roughly be: (external use) (QDev core) (Device Impl) ~~~~~~~~~~~~ ~~~~~~~~~ ~~~~~~~~~~~ INIT enter -----> +----------------------+ | Allocate state | +----------------------+ -----> +---------------------+ | INIT children | | | | Alias children properties | | | Expose properties | INIT exit +---------------------+ <----------------------------------- +----------------+ | set properties | | | | set ClkIn | +----------------+ REALIZE enter ----------------------------------> +----------------------+ | Use config properties| | | | Realize children | | | | Init GPIOs/IRQs | | | | Init MemoryRegions | +----------------------+ REALIZE exit <----------------------------------- ---- "realized" / "guest visible" +-----------------+ | Explicit wiring:| | IRQs | | I/O / Mem | | ClkOut | +-----------------+ RESET enter ---------------------------------> +----------------------+ | Reset default values | +----------------------+ But as mentioned, various devices "wire" parts before they exit the "realize" step. In order to clarify, I'm trying to enforce what can be done *before* and *after* realization. *after* a device is expected to be stable (no more configurable) and fully usable. To be able to use internal/auto wiring (such ISA devices) and keep the current external/explicit wiring, I propose to add an extra "internal wiring" step, happening after the REALIZE step as: (external use) (QDev core) (Device Impl) ~~~~~~~~~~~~ ~~~~~~~~~ ~~~~~~~~~~~ INIT enter -----> +----------------------+ | Allocate state | +----------------------+ -----> +---------------------+ | INIT children | | | | Alias children properties | | | Expose properties | INIT exit +---------------------+ <----------------------------------- +----------------+ | set properties | | | | set ClkIn | +----------------+ REALIZE enter ----------------------------------> +----------------------+ | Use config properties| | | | Realize children | | | | Init GPIOs/IRQs | | | | Init MemoryRegions | +----------------------+ REALIZE exit <--- +----------------------+ | Internal auto wiring | | IRQs | (i.e. ISA bus) | I/O / Mem | | ClkOut | +----------------------+ <--- ---- "realized" +-----------------+ | External wiring:| | IRQs | | I/O / Mem | | ClkOut | +-----------------+ RESET enter ---- "guest visible" ---------------------------------> +----------------------+ | Reset default values | +----------------------+ The "realized" point is not changed. "guest visible" concept only occurs *after* wiring, just before the reset phase. This series introduces the DeviceClass::wire handler within qdev core realization code, and convert devices using the implicit wiring to using that explicit handler. QDev API assertions patches will be posted later as another series. Thoughts? Regards, Phil. [1] https://lore.kernel.org/all/87o7d1i7ky.fsf@pond.sub.org/ [2] https://lore.kernel.org/qemu-devel/20231127052024.435743-1-gustavo.romero@linaro.org/ Cc: Eduardo Habkost <eduardo@habkost.net> Cc: Markus Armbruster <armbru@redhat.com>' --cc ' Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>' Cc: Richard Henderson <richard.henderson@linaro.org> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Cc: Thomas Huth <thuth@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Alexander Graf <agraf@csgraf.de> Cc: Bernhard Beschow <shentey@gmail.com> Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Cédric Le Goater <clg@kaod.org> Cc: Luc Michel <luc.michel@amd.com> Cc: Zhao Liu <zhao1.liu@linux.intel.com> Cc: Gustavo Romero <gustavo.romero@linaro.org> Philippe Mathieu-Daudé (7): hw/qdev: Introduce DeviceClass::[un]wire() handlers hw/input/pckbd: Connect i8042 GPIOs once mouse/keyboard are realized hw/ide/cmd646: Configure IDE bus IRQs after realization hw/ide/sii3112: Configure IDE bus IRQs after realization hw/ide/via: Configure IDE bus IRQs after realization hw/intc/mips_gic: Initialize IRQ array once device is realized hw/misc/mac_via: Have VIA1 child access parent IRQ once realized include/hw/qdev-core.h | 8 +++++++- hw/core/qdev.c | 21 ++++++++++++++++++++- hw/ide/cmd646.c | 12 +++++++++++- hw/ide/sii3112.c | 10 ++++++++++ hw/ide/via.c | 10 ++++++++++ hw/input/pckbd.c | 38 +++++++++++++++++++++++++++----------- hw/intc/mips_gic.c | 11 +++++++++-- hw/misc/mac_via.c | 9 ++++++++- 8 files changed, 102 insertions(+), 17 deletions(-)