Re: [Qemu-devel] [PATCH] initialize unit id of IDE bus

[Gleb Natapov]

On Tue, Oct 26, 2010 at 01:59:45PM +0200, Markus Armbruster wrote:
> Gleb Natapov <address@hidden> writes:
> 
> > On Tue, Oct 26, 2010 at 11:29:49AM +0200, Markus Armbruster wrote:
> >> Gleb Natapov <address@hidden> writes:
> >> 
> >> > On Mon, Oct 25, 2010 at 08:06:13PM +0200, Markus Armbruster wrote:
> >> >> Gleb Natapov <address@hidden> writes:
> >> >> 
> >> >> > On Mon, Oct 25, 2010 at 06:22:12PM +0200, Markus Armbruster wrote:
> >> >> >> Gleb Natapov <address@hidden> writes:
> >> >> >> 
> >> >> >> > On Mon, Oct 25, 2010 at 04:29:35PM +0200, Markus Armbruster wrote:
> >> >> >> >> Gleb Natapov <address@hidden> writes:
> >> >> >> >> 
> >> >> >> >> > Without this patch both buses on PIIX3_IDE device have the same 
> >> >> >> >> > unit id.
> >> >> >> >> 
> >> >> >> >> Are you sure that's wrong?
> >> >> >> >> 
> >> >> >> > So how do I know which bus is it on PIIX3_IDE?
> >> >> >> [...]
> >> >> >> 
> >> >> >> Let me try to explain the IDE pointer thicket.
> >> >> >> 
> >> >> >> piix3-ide provides two IDE buses.  pci_piix_ide_initfn() stores them 
> >> >> >> in
> >> >> >> PCIIDEState member IDEBus bus[2].  Technically redundant, because 
> >> >> >> qdev
> >> >> >> stores child buses in dev.qdev.child_bus.
> >> >> >> 
> >> >> >> IDEBus points back: qbus.parent.
> >> >> >> 
> >> >> >> Up to two IDE devices can sit on each IDE bus.  The first one uses
> >> >> >> IDEBus members master and ifs[0], the second one uses slave and 
> >> >> >> ifs[1].
> >> >> >> 
> >> >> >> ifs[i].bus points back to the IDE bus.
> >> >> >> 
> >> >> >> {master,slave}.qdev.parent_bus point back to the IDE bus.
> >> >> >> 
> >> >> >> Say you got an IDEDevice and want to know which to which of the two
> >> >> >> buses it's connected.  Let's call it d.
> >> >> >> 
> >> >> >> d->qdev.parent_bus is the BusState.
> >> >> >> 
> >> >> >> Upcast to IDEBus: b = DO_UPCAST(IDEBus, qbus, d->qdev.parent_bus).
> >> >> >> 
> >> >> >> b->qbus.parent is the IDE controller.
> >> >> >> 
> >> >> >> Upcast to PCIIDEState: c = DO_UPCAST(PCIIDEState, dev, 
> >> >> >> n->qbus.parent);
> >> >> >> 
> >> >> >> If c->bus[0] == b, it's on the first bus.
> >> >> >> 
> >> >> >> Else it must be on the second bus, i.e. c->bus[1] == b.
> >> >> >> 
> >> >> >> Hope I didn't screw this up too badly.
> >> >> > Will check tomorrow if this works, but why not have simple property on
> >> >> > IDEBus that says which one is it?
> >> >> 
> >> >> Because nobody has needed it so far?
> >> >> 
> >> >> Does the bus care whether it's first or second?
> >> >> 
> >> > User that wants to address particular disk cares.
> >> 
> >> Define "user".
> >> 
> > User is a guest OS.
> 
> How do you know how the guest OS numbers its IDE buses?  More below.
> 
It enumerates all IDE controllers using PCI bus enumeration. And each
controller has two IDE buses 0 and 1. 0 uses bar 0,1 1 uses bar 2,3, no
need to enumerate any further.

> >> For users of qemu, the way to address a particular disk (or any qdev
> >> device) is by user-specified ID.
> >> 
> >> For users within qemu, there are other ways.
> >> 
> > Which ways?
> 
> Walk the device tree.
What device tree? And how qemu tells to a guest what device to search
for?

> 
> >> I'm asking whether the bus cares, because IDEBus holds the state of the
> >> bus.  If the bus itself doesn't care whether it's primary or secondary,
> >> then this state should not carry that information.
> >> 
> > Bus cares because devices on the bus are addressed differently depending
> > on which bus it resides on.
> 
> I'm afraid we're talking past each other.
> 
My impression too.

> The bus is a part of the controller device.  The IDE controller device
> has no idea which other IDE controller devices are in the system.  It
> can't care for a global bus number, because it doesn't know it.
But qdev creases it anyway "ide.1" "ide.0" (It only does it in a
guest unusable way). And it has to do it since you want to give user
ability to attach disk image to specific device, so user have to be able
to specify exact device path you claim does not exit internally in qemu!

> 
> The guest OS enumerates the IDE controller devices, and names or numbers
> them and their IDE buses how it sees fit.  It's well advised to follow
> the system architecture's conventions, where such conventions exist.
> They need not exist.
Now we have a situation where qemu needs to communicate whereabouts of
some device it created to a guest OS. How do you propose to do that?

> 
> Consider some non-PC architecture that knows nothing about IDE, but
> still has a PCI bus.  A user plugs in two PCI cards, each one has two
> IDE buses.  How do you number these four buses globally?
> 
You don't. You number only IDE buses under one PCI device, so you'll
have paths like:
address@hidden:00:05.0/address@hidden:0
address@hidden:00:06.0/address@hidden:0

And they point to two different disks in a way perfectly understandable
by any guest.

> The sane way to address such a bus is by (device-address-of-controller,
> bus#-on-this-controller).  And the general way to do a device address is
> (bus, address-on-bus).  Recurses up the device tree.
That is what I did above. The question is "what is bus# of IDE device on
IDE bus"? In my case it is 0/1,

> 
> > >> >                                   BTW what -device magic should I use 
> > >> > to
> >> >> > create secondary disk on second IDE bus?
> >> >> 
> >> >> Try -device ide-drive,bus=ide.1,unit=1,drive=...
> >> >                                                                  Can I
> >> 
> >> In qdev, we address buses by ID, just like devices.  Unlike device IDs,
> >> which are specified by the user, the bus IDs are auto-generated.  The
> >> secondary bus of piix3-ide gets the bus ID "ide.1".
> > Unfortunately those ids are meaningless from outside of qemu.
> >
> >> 
> >> We do have usability problems there.  There is no way for the user to
> >> specify an ID for a default device, as far as I know.  The bus IDs are
> >> defined by the device providing the bus.  If it provides none, qdev core
> >> makes one up.  This breaks down in corner cases.  For instance, with -M
> >> isapc, we get two isa-ide.  Each provides an IDE bus without specifying
> >> its ID.  qdev core assigns the same name "ide.0" to both.  bus=ide.0
> >> picks the first one (in not-really-specified qtree traversal order).
> >> As far as I can tell, there's no way to address the second one.
> >> 
> > This looks like fundamental problem in qdev design.
> 
> It's not fundamental, it's just screwed up.
> 
> >                                                     In my case bus names
> > assigned by qdev are useless even when done right though :(
> 
> IDs are a qdev artifact, yes.
> 
> [...]

--
                        Gleb.