Re: ensuring a machine's buses have unique names

[Markus Armbruster]

BALATON Zoltan <balaton@eik.bme.hu> writes:

> On Tue, 21 Sep 2021, Peter Maydell wrote:
>> On Wed, 15 Sept 2021 at 05:28, Markus Armbruster <armbru@redhat.com> wrote:
>>>
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>> I'm not sure how best to sort this tangle out. We could:
>>>>  * make controller devices pass in NULL as bus name; this
>>>>    means that some bus names will change, which is an annoying
>>>>    breakage but for these minor bus types we can probably
>>>>    get away with it. This brings these buses into line with
>>>>    how we've been handling uniqueness for ide and scsi.
>>>
>>> To gauge the breakage, we need a list of the affected bus names.
>>
>> Looking through, there are a few single-use or special
>> purpose buses I'm going to ignore for now (eg vmbus, or
>> the s390 ones). The four big bus types where controllers
>> often specify a bus name and override the 'autogenerate
>> unique name' handling are pci, ssi, sd, and i2c. (pci mostly
>> gets away with it I expect by machines only having one pci
>> bus.) Of those, I've gone through i2c. These are all the
>> places where we create a specifically-named i2c bus (via
>> i2c_init_bus()), together with the affected boards:
>>
>>   hw/arm/pxa2xx.c
>>    - the PXA SoC code creates both the intended-for-use
>>      i2c buses (which get auto-names) and also several i2c
>>      buses intended for internal board-code use only which
>>      are all given the same name "dummy".
>>      Boards: connex, verdex, tosa, mainstone, akita, spitz,
>>      borzoi, terrier, z2
>>   hw/arm/stellaris.c
>>    - The i2c controller names its bus "i2c". There is only one i2c
>>      controller on these boards, so no name conflicts.
>>      Boards: lm3s811evb, lm3s6965evb
>>   hw/display/ati.c
>>    - The ATI VGA device has an on-board i2c controller which it
>>      connects the DDC that holds the EDID information. The bus is
>>      always named "ati-vga.ddc", so if you have multiple of this
>>      PCI device in the system the buses have the same names.
>>   hw/display/sm501.c
>>    - Same as ATI, but the bus name is "sm501.i2c"
>>   hw/i2c/aspeed_i2c.c
>>    - This I2C controller has either 14 or 16 (!) different i2c
>>      buses, and it assigns them names "aspeed.i2c.N" for N = 0,1,2,...
>>      The board code mostly seems to use these to wire up various
>>      on-board i2c devices.
>>      Boards: palmetto-bmc, supermicrox11-bmc, ast2500-evb, romulus-bmc,
>>      swift-bmc, sonorapass-bmc, witherspoon-bmc, ast2600-evb,
>>      tacoma-bmc, g220a-bmc, quanta-q71l-bmc, rainier-bmc
>>   hw/i2c/bitbang_i2c.c
>>    - the "GPIO to I2C bridge" device always names its bus "i2c".
>>      Used only on musicpal, which only creates one of these buses.
>>      Boards: musicpal
>>   hw/i2c/exynos4210_i2c.c
>>    - This i2c controller always names its bus "i2c". There are 9
>>      of these controllers on the board, so they all have clashing
>>      names.
>>      Boards: nuri, smdkc210
>>   hw/i2c/i2c_mux_pca954x.c
>>    - This is an i2c multiplexer. All the child buses are named
>>      "i2c-bus". The multiplexer is used by the aspeed and npcm7xx
>>      boards. (There's a programmable way to get at individual
>>      downstream i2c buses despite the name clash; none of the boards
>>      using this multiplexer actually connect any devices downstream of
>>      it yet.)
>>      Boards: palmetto-bmc, supermicrox11-bmc, ast2500-evb, romulus-bmc,
>>      swift-bmc, sonorapass-bmc, witherspoon-bmc, ast2600-evb,
>>      tacoma-bmc, g220a-bmc, quanta-q71l-bmc, rainier-bmc,
>>      npcm750-evb, quanta-gsj, quanta-gbs-bmc, kudo-bmc
>>   hw/i2c/mpc_i2c.c
>>    - This controller always names its bus "i2c". There is only one
>>      of these controllers in the machine.
>>      Boards: ppce500, mpc8544ds
>>   hw/i2c/npcm7xx_smbus.c
>>    - This controller always names its bus "i2c-bus". There are multiple
>>      controllers on the boards. The name also clashes with the one used
>>      by the pca954x muxes on these boards (see above).
>>      Boards: npcm750-evb, quanta-gsj, quanta-gbs-bmc, kudo-bmc
>>   hw/i2c/pm_smbus.c
>>    - This is the PC SMBUS implementation (it is not a QOM device...)
>>      The bus is always called "i2c".
>>      Boards: haven't worked through; at least all the x86 PC-like
>>      boards, I guess
>>   hw/i2c/ppc4xx_i2c.c
>>    - This controller always names its bus "i2c". The taihu and
>>      ref405ep have only one controller, but sam460ex has two which
>>      will have non-unique names.
>>      Boards: taihu, ref405ep, sam460ex
>>   hw/i2c/versatile_i2c.c
>>    - This controller always names its bus "i2c". The MPS boards all
>>      have multiples of this controller with clashing names; the others
>>      have only one controller.
>>      Boards: mps2-an385, mps2-an386, mps2-an500, mps2-an511,
>>      mps2-an505, mps2-an521, mps3-an524, mps3-an547,
>>      realview-eb, realview-eb-mpcore, realview-pb-a8, realview-pbx-a9,
>>      versatileab, versatilepb, vexpress-a9, vexpress-a15
>>
>> In a lot of these cases I suspect the i2c controllers are
>> provided either to allow connection of various internal-to-the-board
>> devices, or simply so that guest OS bootup code that initializes
>> the i2c controller doesn't fall over. However since there's
>> nothing stopping users from creating i2c devices themselves
>> on the commandline, some people might be doing that.
>>
>> In some of these cases (eg the i2c bus on the ATI VGA driver)
>> I suspect the desired behaviour is "unique bus name based on
>> a standard template, eg 'ati-vga.ddc.0/1/...'. It sounds like
>> we can't do that, though. (Also they probably don't want to
>> permit users to command-line plug i2c devices into it...)
>
> To me it looks like device code can't really set a globally unique
> name on creating the bus without getting some help from upper
> levels. So maybe naming busses should be done by qdev (or whatever is
> handling this) instead of passing the name as an argument to
> qbus_create or only use that name as a unique component within the
> device and append it to a unique name for the device.

Have you read the whole thread?  qdev does come up with a name when
passed a null pointer.  The problem is that we often don't.  Another
problem is that qdev can come up with non-unique names unless the user
is careful with device IDs (the values of -device / device_add parameter
id).

>                                                       Thus we would
> get names like sys.pci-0.ati-vga-0.ddc or so but not sure we want that
> as it's hard to use on command line. Alternatively we can accept non
> unique names but use another unique property such as device id to
> identify devices which could be generated as an integer incremented
> after every device add or some hash which would result in shorter
> unique ids. Such id is already used by -drive and -net options where
> used supplies a unique id and then can use that to reference the
> created object by that id in other options. This could be extended to
> devices and buses if it had a unique id, then it's not a problem to
> have non-unique names.
>
> Regards,
> BALATON Zoltan