On 25/05/2022 12:45, Peter Maydell wrote:
On Wed, 25 May 2022 at 10:51, Damien Hedde
<damien.hedde@greensocs.com> wrote:
On 5/24/22 19:44, Mark Cave-Ayland wrote:
Sorry for coming late into this series, however one of the things I've
been thinking about a lot recently is that with the advent of QOM and
qdev, is there really any distinction between TYPE_DEVICE and
TYPE_SYS_BUS_DEVICE anymore, and whether it makes sense to keep
TYPE_SYS_BUS_DEVICE long term.
On QAPI/CLI level there is a huge difference since TYPE_SYS_BUS_DEVICE
is the only subtype of TYPE_DEVICE which is subject to special
treatment. It prevents to plug a sysbus device which has not be allowed
by code and that's what I want to get rid of (or workaround by allowing
all of them).
Yes, but the fact that TYPE_SYS_BUS_DEVICE is a special subclass
is an accident of history. At some point we really ought to tidy
this up so that any TYPE_DEVICE can have MMIO regions and IRQs,
and get rid of the subclass entirely. This isn't trivial, for
reasons including problems with reset handling, but I would
prefer it if we didn't bake "sysbus is special" into places like
the QMP commands.
Right, and in fact we can already do this today using QOM regardless of
whether something is a SysBusDevice or not. As an example here is the
output of qemu-system-sparc's "info qom-tree" for the slavio_misc device:
/device[20] (slavio_misc)
/configuration[0] (memory-region)
/diagnostic[0] (memory-region)
/leds[0] (memory-region)
/misc-system-functions[0] (memory-region)
/modem[0] (memory-region)
/software-powerdown-control[0] (memory-region)
/system-control[0] (memory-region)
/unnamed-gpio-in[0] (irq)
Now imagine that I instantiate a device with qdev_new():
DeviceState *dev = qdev_new("slavio_misc");
I can obtain a reference to the "configuration" memory-region using
something like:
MemoryRegion *config_mr = MEMORY_REGION(object_resolve_path_component(
OBJECT(dev), "configuration[0]"));
and for the IRQ I can do either:
qemu_irq *irq = IRQ(object_resolve_path_component(
OBJECT(dev), "unnamed-gpio-in[0]"));
or simply:
qemu_irq *irq = qdev_get_gpio_in(dev, 0);
Maybe for simplicity we could even add a qdev wrapper function to obtain
a reference for memory regions similar to qdev gpios i.e.
qdev_get_mmio(dev, "configuration", 0) based upon the above example?
Now from the monitor we can already enumerate this information using
qom-list if we have the QOM path:
(qemu) qom-list /machine/unattached/device[20]
type (string)
parent_bus (link<bus>)
hotplugged (bool)
hotpluggable (bool)
realized (bool)
diagnostic[0] (child<memory-region>)
unnamed-gpio-in[0] (child<irq>)
modem[0] (child<memory-region>)
leds[0] (child<memory-region>)
misc-system-functions[0] (child<memory-region>)
sysbus-irq[1] (link<irq>)
sysbus-irq[0] (link<irq>)
system-control[0] (child<memory-region>)
configuration[0] (child<memory-region>)
software-powerdown-control[0] (child<memory-region>)
From this I think we're missing just 2 things: i) a method to look up
properties from a device id which can be used to facilitate
introspection, and ii) a function to map a memory region from a device
(similar to Damien's patch). Those could be something like:
device_list <id>
- looks up the QOM path for device "id" and calls qom-list on the
result