All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Markus Armbruster" <armbru@redhat.com>
Subject: Re: ensuring a machine's buses have unique names
Date: Thu, 26 Aug 2021 16:08:48 +0200	[thread overview]
Message-ID: <87czq0l2mn.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <CAFEAcA8Q2XEANtKfk_Ak2GgeM8b_=kf_qduLztCuL=E_k36FWg@mail.gmail.com> (Peter Maydell's message of "Thu, 12 Aug 2021 12:05:36 +0100")

Peter Maydell <peter.maydell@linaro.org> writes:

> What's the right way to ensure that when a machine has multiple
> buses of the same type (eg multiple i2c controllers, multiple
> sd card controllers) they all get assigned unique names so that
> the user can use '-device ...,bus=some-name' to put a device on a
> specific bus?
>
> For instance in hw/arm/xlnx-zynqmp.c, the SoC object creates
> a set of alias properties on the SoC for the sd-bus buses that
> its 4 SD card controllers create. The alias properties are named
> "sd-bus%d" so they are unique. This works, but it's kind of error-prone
> because you need each machine model to remember to create these
> aliases when necessary.
>
> mps3-an524 is an example of a machine that fails to do this
> for its i2c buses, and therefore the user can't usefully
> tell QEMU which bus to plug a command-line created i2c bus into.

Another one used to be isapc.  It's not anymore.  I believe it's due to

commit 61de36761b565a4138d8ad7ec75489ab28fe84b6
Author: Alexander Graf <agraf@csgraf.de>
Date:   Thu Feb 6 16:08:15 2014 +0100

    qdev: Keep global allocation counter per bus
    
    When we have 2 separate qdev devices that both create a qbus of the
    same type without specifying a bus name or device name, we end up
    with two buses of the same name, such as ide.0 on the Mac machines:
    
      dev: macio-ide, id ""
        bus: ide.0
          type IDE
      dev: macio-ide, id ""
        bus: ide.0
          type IDE
    
    If we now spawn a device that connects to a ide.0 the last created
    bus gets the device, with the first created bus inaccessible to the
    command line.
    
    After some discussion on IRC we concluded that the best quick fix way
    forward for this is to make automated bus-class type based allocation
    count a global counter. That's what this patch implements. With this
    we instead get
    
      dev: macio-ide, id ""
        bus: ide.1
          type IDE
      dev: macio-ide, id ""
        bus: ide.0
          type IDE
    
    on the example mentioned above.
    
    This also means that if you did -device ...,bus=ide.0 you got a device
    on the first bus (the last created one) before this patch and get that
    device on the second one (the first created one) now.  Breaks
    migration unless you change bus=ide.0 to bus=ide.1 on the destination.
    
    This is intended and makes the bus enumeration work as expected.
    
    As per review request follows a list of otherwise affected boards and
    the reasoning for the conclusion that they are ok:
    
       target      machine         bus id              times
       ------      -------         ------              -----
    
       aarch64     n800            i2c-bus.0           2
       aarch64     n810            i2c-bus.0           2
       arm         n800            i2c-bus.0           2
       arm         n810            i2c-bus.0           2
    
    -> Devices are only created explicitly on one of the two buses, using
       s->mpu->i2c[0], so no change to the guest.
    
       aarch64     vexpress-a15    virtio-mmio-bus.0   4
       aarch64     vexpress-a9     virtio-mmio-bus.0   4
       aarch64     virt            virtio-mmio-bus.0   32
       arm         vexpress-a15    virtio-mmio-bus.0   4
       arm         vexpress-a9     virtio-mmio-bus.0   4
       arm         virt            virtio-mmio-bus.0   32
    
    -> Makes -device bus= work for all virtio-mmio buses.  Breaks
       migration.  Workaround for migration from old to new: specify
       virtio-mmio-bus.4 or .32 respectively rather than .0 on the
       destination.
    
       aarch64     xilinx-zynq-a9  usb-bus.0           2
       arm         xilinx-zynq-a9  usb-bus.0           2
       mips64el    fulong2e        usb-bus.0           2
    
    -> Normal USB operation not affected. Migration driver needs command
       line to use the other bus.
    
       i386        isapc           ide.0               2
       x86_64      isapc           ide.0               2
       mips        mips            ide.0               2
       mips64      mips            ide.0               2
       mips64el    mips            ide.0               2
       mipsel      mips            ide.0               2
       ppc         g3beige         ide.0               2
       ppc         mac99           ide.0               2
       ppc         prep            ide.0               2
       ppc64       g3beige         ide.0               2
       ppc64       mac99           ide.0               2
       ppc64       prep            ide.0               2
    
    -> Makes -device bus= work for all IDE buses.  Breaks migration.
       Workaround for migration from old to new: specify ide.1 rather than
       ide.0 on the destination.
    
    Signed-off-by: Alexander Graf <agraf@suse.de>
    Signed-off-by: Markus Armbruster <armbru@redhat.com>
    Reviewed-by: Andreas Faerber <afaerber@suse.de>
    Signed-off-by: Alexander Graf <agraf@suse.de>

Note that the automatic bus numbers depend on the order in which board
code creates devices.  Too implicit and fragile for my taste.  But it's
been working well enough.

> Ideally we should make buses get unique names by default
> and also assert() at startup that there aren't any duplicated
> names, I think.
>
> Side note: is there a way to mark a bus as "do not consider
> this when plugging devices where the user did not specify
> the bus themselves" ? Some of the i2c buses on that machine
> are purely internal to the board (eg there's one that has
> the touchscreen controller hanging off it and nothing else),
> and some are "this i2c bus is connected to the expansion port",
> so ideally if no bus is specified we would want to prefer
> the expansion-port i2c bus, not the ones that are internal-only.

I think the only existing way is to make qbus_is_full() return true.



  parent reply	other threads:[~2021-08-26 14:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12 11:05 ensuring a machine's buses have unique names Peter Maydell
2021-08-26 13:00 ` Peter Maydell
2021-08-26 14:08 ` Markus Armbruster [this message]
2021-09-14 15:25   ` Peter Maydell
2021-09-15  4:28     ` Markus Armbruster
2021-09-21 13:20       ` Peter Maydell
2021-09-21 15:48         ` BALATON Zoltan
2021-09-22  4:37           ` Markus Armbruster
2021-09-22  9:58             ` BALATON Zoltan
2021-09-22 13:07               ` Markus Armbruster
2021-09-22  7:02         ` Markus Armbruster
2021-09-22  8:14           ` Cédric Le Goater
2021-09-22  4:46     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87czq0l2mn.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.