On 30/05/2022 15:05, Damien Hedde wrote:
On 5/30/22 12:25, Peter Maydell wrote:
On Mon, 30 May 2022 at 10:50, Damien Hedde <damien.he...@greensocs.com> wrote:
TYPE_SYS_BUS_DEVICE also comes with reset support.
If a device is on not on any bus it is not reached by the root sysbus
reset which propagates to every device (and other sub-buses).
Even if we move all the mmio/sysbus-irq logic into TYPE_DEVICE, we will
still miss that. The bus is needed to handle the reset.
For devices created in machine init code, we have the option to do it in
the machine reset handler. But for user created device, this is an issue.
Yes, the missing reset support in TYPE_DEVICE is a design
flaw that we really should try to address.
I think the easiest way to handle this would be just after calling dc->realize; if
the device has bus == NULL and dc->reset != NULL then manually call
qemu_register_reset() for dc->reset. In a qdev world dc->reset is intended to be a
bus-level reset, but I can't see an issue with manual registration for individual
devices in this way, particularly as there are no reset ordering guarantees for sysbus.
If we end up putting in TYPE_DEVICE support for mmios, interrupts and
some way to do the bus reset. What would be the difference between the
current TYPE_SYS_BUS_DEVICE ?
There would be none, and the idea would be to get rid of
TYPE_SYS_BUS_DEVICE entirely...
Do you expect the bus object to disappear in the process (bus-less system) or
transforming the sysbus into a ~TYPE_BUS thing ?
I'd probably lean towards removing sysbus completely since in real life devices can
exist outside of a bus. If a device needs a bus then it should already be modelled in
QEMU, and anything that requires a hierarchy can already be represented via QOM children.
Assuming we manage to sort out this does cold plugging using the following scenario
looks ok ? (regarding having to issue one command to create the device AND some
commands to handle memory-region and interrupt lines)
> device_add driver=ibex-uart id=uart chardev=serial0
> sysbus-mmio-map device=uart addr=1073741824
> qom-set path=uart property=sysbus-irq[0] value=plic/unnamed-gpio-in[1]
TYPE_DEVICE or TYPE_SYS_BUS_DEVICE, my goal is still to be able to cold-plug a
"ibex-uart" define its memory map and choose which irq I wire where.
Anyhow getting back on topic: my main objection here is that you're adding a command
"sysbus-mmio-map" when we don't want the concept of SysBusDevice to be exposed
outside of QEMU at all. Referring back to my last email I think we should extend the
device concept in the monitor to handle the additional functionality perhaps along
the lines of:
- A monitor command such as "device_map" which is effectively a wrapper around
memory_region_add_subregion(). Do we also want a "device_unmap"? We should
consider allow mapping to other memory regions other than the system root.
- A monitor command such as "device_connect" which can be used to simplify your
IRQ
wiring, perhaps also with a "device_disconnect"?
- An outline of the monitor commands showing the complete workflow from
introspection
of a device to mapping its memory region(s) and connecting its gpios
Does that give you enough information to come up with a more detailed proposal?
ATB,
Mark.
