On 01/06/2022 09:39, Damien Hedde wrote:
On 5/31/22 22:43, Mark Cave-Ayland wrote:
On 31/05/2022 10:22, Damien Hedde wrote:
On 5/31/22 10:00, Mark Cave-Ayland wrote:
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.
I'm a bit afraid calling qemu_register_reset() outside dc->realize might modify
the behavior for existing devices. Does any device end up having a non-NULL bus
right now when using "-device" CLI ?
If you take a look at "info qtree" then that will show you all devices that are
attached to a bus i.e. ones where bus is not NULL.
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
For me, a "memory bus" is a bus. But I understand in QEMU, this is modeled by a
memory region and we do not want to represent it anymore by a qdev/qbus hierarchy.
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?
Yes. Sorry for being not clear enough. I did not wanted to insist on specific
command names. I've no issues regarding the modifications you request about having
a device_connect or a device_map.
My question was more about the workflow which does not rely on issuing a single
'device_add' command handling mapping/connection using parameters. Note that since
we are talking supporting of map/connect for the base type TYPE_DEVICE, I don't
really see how we could have parameters for these without impacting subtypes.
I'm not sure I understand what you are saying here? Can you give an example?
There are 2 possible workflows:
1. several commands
> device_add ...
> device_map ...
> device_connect ...
2. single command
> device_add ... map={...} connect={...}
The 2nd one is more like how we connect devices with '-device': all is done at once.
But if this is supposed to apply to TYPE_DEVICE (versus TYPE_SYS_BUS_DEVICE), it
becomes IMHO hard to prevent using them on devices where it does not makes sense (for
example: a virtio or pci device for which everything is already handled).
My initial feeling is that 1) is the better approach, since you can report errors at
each stage. Once you have the id for a specific device you can them attempt to
device_map it, reporting an error if there is a region overlap. Similarly for
device_connect can you report an error if the input gpio is already connected.
ATB,
Mark.
