Cc: QOM/qdev maintainers Cédric Le Goater <[email protected]> writes:
> Hello, > > On 1/19/26 15:25, Markus Armbruster wrote: >> Cédric Le Goater <[email protected]> writes: >> >>> On 1/15/26 20:47, Philippe Mathieu-Daudé wrote: >>>> Hi, >>>> Cc'ing Markus. >>>> On 12/1/26 09:30, Kane Chen via qemu development wrote: >>>>> From: Kane-Chen-AS <[email protected]> >>>>> >>>>> Currently, the Aspeed I2C controller uses a static naming convention >>>>> for its buses (e.g., "aspeed.i2c.bus.0"). This approach leads to >>>>> object name conflicts in machine models that incorporate multiple I2C >>>>> controllers, such as an Aspeed SoC paired with an external IO expander >>>>> or a co-processor like the AST1700. >>>>> >>>> Is this a side-effect of Problem 4: 'The /machine/unattached/ orphanage' >>>> described here? >>>> https://lore.kernel.org/qemu-devel/[email protected]/ No, but ... >>>> This problem isn't specific to I2C nor Aspeed. ... yes, indeed. Details below. >>> See the discussion here : >>> >>> >>> https://lore.kernel.org/qemu-devel/[email protected]/ >>> >>> The Aspeed SoC has 3*16 I2C buses attached on 3 different I2C >>> controllers plus the I2C/I3C buses. We need to find a way to >>> distinguish these groups at the QEMU machine level to be able >>> to add devices on the right bus when using the command line. >>> >>> Suggestions welcome ! >> >> Please show me how to start a QEMU with the 48 I2C mentioned above, >> complete with output of "info qtree". > > Clone > > https://github.com/legoater/qemu/commits/aspeed-11.0 > > Download > > > https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.08/ast2700-default-obmc.tar.gz > > And run : > > qemu-system-aarch64 -M ast2700a1-evb -m 8G -smp 4 -net > nic,macaddr=,netdev=net0 -netdev user,id=net0,hostfwd=::2207-:22 -drive > file=path/to/ast2700-default/image-bmc,format=raw,if=mtd -nographic -serial > mon:stdio -snapshot > > Today, to attach an I2C device on one of the Aspeed SoC I2C buses : > > -device tmp105,bus=aspeed.i2c.bus.1,address=0x4d,id=tmp-test > > The Aspeed SoC I2C bus names follow the "aspeed.i2c.bus.X" format. > This is the model typename. The 2 new IO expander models attached > to the Aspeed SoC have an extra 16 I2C buses each. These buses use > an "ioexpX.Y" name, as proposed in the aspeed-next branch. > > Attaching a device to one of the IO expanders I2C buses would be : > > -device tmp105,bus=ioexp0.1,address=0x4d,id=tmp-test > > See the qtree below. Thank you! Machine ast2700a1-evb has QOM objects /machine/soc/i2c/ (aspeed.i2c-ast2700) /bus[N] (aspeed.i2c.bus) for N in 0..15 /aspeed.i2c.bus.N (i2c-bus) in both master and aspeed-11.0. Object aspeed.i2c-ast2700 is a sysbus device that contains 16 aspeed.i2c.bus objects as children bus[N] for N in 0..15. Each object has a property "bus-id" with value N. Object aspeed.i2c.bus is a sysbus device that contains an i2c-bus object as child aspeed.i2c.bus.N. Aside: why parent TYPE_SYS_BUS_DEVICE, and not TYPE_DEVICE? It doesn't actually plug into a sysbus... Object i2c-bus is a qbus. In master, object aspeed.i2c.bus computes the child name by appending .N to its own type name TYPE_ASPEED_I2C_BUS. In aspeed-11.0, it takes it from its property "bus-name". The property is set by its QOM parent aspeed.i2c-ast2700, and computed by appending .N to the value of its property "bus-label". It defaults the property to TYPE_ASPEED_I2C_BUS. Since nothing sets this property in this case, we get the same child name. aspeed-11.0 additionally has QOM objects /machine/soc/ioexp[M] (aspeed.ast1700) for M in 0..1 /ioexp-i2c[0] (aspeed.i2c-ast2700) /bus[N] (aspeed.i2c.bus) for N in 0..15 /ioexpM.N (i2c-bus) Object aspeed.ast1700 is a sysbus device that contains an aspeed.i2c-ast2700 as child "ioexp-i2c[0]". It has a property "board-idx" with value M. Aside: we only ever create one aspeed.i2c-ast2700 child. Why [0]? Aside^2: I tried to strangle the "[*]" feature in the crib, but failed. Has been a minor thorn in my side ever since. aspeed.ast1700 set its child's (aspeed.i2c-ast2700) property "bus-label" to "ioexpM". This makes the child set the grandchild's (aspeed.i2c.bus) property "bus-name" to "ioexpM.N", which makes the grandchild name the great-grandchild (i2c-bus) "ioexpM.N". This naming business is complicated, and I had a hard time ferreting it out. As far as I can tell, it's all in service of -device bus=... Let's examine how that works. We want to be able to plug i2c devices into any of these 48 i2c buses with -device / device_add. To do that, we need to select a bus with the "bus" argument. In a world saner than the one we live in, the value of "bus" would be a QOM path or qdev ID, where qdev ID is shorthand for the QOM path /machine/peripheral/ID. For instance, the first i2c bus could then be selected with absolute QOM path "/machine/soc/i2c/bus[0]" or partial QOM paths "soc/i2c/bus[0]" or "i2c/bus[0]". Partial QOM paths are a convenience feature that is virtually unknown (and possibly ill-advised): you can omit leading path components as long as there's no ambiguity. However, in the world we live in, the value of bus is not a QOM path, it's a path in the qtree. Why? qdev and -device / device_add predate QOM. If the path starts with "/", it's anchored at the main system bus. Else, it's anchored at a bus whose name is the first path component. If there's more than one such bus, we pick the first one we find. This is a misfeature. Remaining path components, if any, pick a path in the qtree from that anchor towards leaves. Note that the child of a qbus is always a qdev, and the child of a qdev always a qbus. This must ultimately resolve to a qbus of the appropriate type. Picking a qdev child of a qbus works like this: * If a child with a (user-specified) qdev ID equal to the path component exists, pick it. Since qdev IDs are unique, there can only be one. * Else, if children whose QOM type name equals the path component exists, pick the first one. * Else, if children whose qdev alias equals the path component exists, pick the first one. Picking the first one is again a misfeature. Picking a qbus child is simpler: we pick the first child whose bus name equals the path component. Bus names are defined as follows: * Whatever creates the bus may set its name. * Else, if the qbus's parent qdev has an ID, the bus name is ID.N, where N counts up from 0 within that qdev. * Else, the bus name is TYPE.N, where TYPE is the parent qdev's QOM type name, and N counts up from 0 within that bus class. The only case where this is actually works is picking the N-th bus child provided by a qdev with an ID: use bus=ID.N (a partial tree path of just one component). Anything else is unfit for purpose, except in special cases, e.g. when the machine can have just one device of a certain type. This mess is harmless for user-created devices: just specify the ID. It's awful for onboard devices, which cannot have an ID. This is a qdev design flaw. It's not specific to I2C or Aspeed, as Philippe suspected. To illustrate it further, let's have a look at the qtree of machine ast2700a1-evb. Output of "info qtree" in master: bus: main-system-bus type System [...] dev: aspeed.i2c.bus, id "" for N in 0..15 [...] bus: aspeed.i2c.bus.N type i2c-bus [...] In aspeed-11.0, we additionally have dev: aspeed.i2c.bus, id "" for M in 0..1, N in 0..15 [...] bus: ioexpM.N type i2c-bus [...] The i2c-buses all have unique names: aspeed.i2c-bus.N and ioexpM.N. We can point to any of them with a partial qtree path of just one component: bus=NAME where NAME is one of these unique names works, and there is no ambiguity. The buses have unique names only because device code takes pains to make them configurable by parent devices, and the parent devices cooperate to configure them so the resulting bus names are unique. This is a lot of complexity to work around this qdev design flaw for just one special instance. Can we instead remedy the design flaw once and for all? Here't the obvious stupid idea: give -device / device_add the means to pick a bus by QOM path. Thoughts?
