Re: [Qemu-devel] [PATCHv2 4/8] Store IDE bus id in IDEBus structure for easy access.

[Gleb Natapov]

On Thu, Nov 04, 2010 at 03:22:50PM +0100, Markus Armbruster wrote:
> Gleb Natapov <address@hidden> writes:
> 
> > On Thu, Nov 04, 2010 at 09:46:57AM +0100, Markus Armbruster wrote:
> >> > But why order of device creation is important? It shouldn't be if we
> >> > want to move HW description into config file. We even may allow creating
> >> > piix3-ide with only second IDE bus, but not first.
> >> 
> >> That's not how buses work in qdev.
> >> 
> >> Configuration file, command line and monitor configure *devices*.
> >> Devices need to be created after the device providing their parent bus,
> >> obviously.  Other than that, order should not matter.
> >> 
> > Correct, but it will if we use your function for looking for IDEBus id.
> > See below.
> 
> Where I'll explain why device order creation can't affect bus numbers.
> 
Bus order creation will affect bus number thought.

> >> Buses are always created by device model code.  Thus, creation order is
> >> fixed in code.  Thus defining bus number in terms of creation order is
> >> fine.
> > So I can't create piix3-ide with only one bus. Looks like arbitrary
> > limitation for me.
> 
> If it has just one bus, it's simply not piix3-ide anymore.  The real
> PIIX3 PCI device has an IDE function that provides two channels, no
> more, no less.
> 
We are doing mental exercise to see if our design is flexible enough

> >> If we create piix3-ide with only the second bus (for the sake of
> >> argument), then its bus number is zero by definition, as clarified by
> >> you below.  Unless you want to amend your definition :)
> >> 
> > As clarified by me below in case of piix3-ide bus number is actually
> > used to figure out how to talk to device on that bus. So if (for
> > the sake of argument of course) we will create piix3-ide device with
> > only secondary bus (the one accessible through BAR2,3 of piix3-ide),
> > device sitting on it will be named address@hidden since there will be only 
> > one
> > bus on piix3-ide. Given name address@hidden guest will try to use BAR0,1 and
> > fail. I understand that such config is not possible today (at least
> > with piix3-ide) but it is important to understand that this is not
> > "just a number showing in which order bus was created"
> 
> Why would a hypothetical piix3-ide with just one IDE channel
> nevertheless expose BARs for *two* IDE channels?
> 
> To me it looks like the need for "holes" in the bus number sequence is
> purely theoretical.
> 
And to me it looks like relying on implicit bus ordering may limit us in
the situation too.

> >> >> >> >                                          Are you against adding 
> >> >> >> > bus_id
> >> >> >> > to IDEBus?
> >> >> >> 
> >> >> >> "Against" is too hard a word.  If it's a general question, I'd 
> >> >> >> prefer a
> >> >> >> general answer.
> >> >> > It is as general as "what pci slot/func of a pci device". We store 
> >> >> > those
> >> >> > in PCIDevice.
> >> >> 
> >> >> It's actually more general than that :)
> >> >> 
> >> >> PCI slot.function is the address of a PCI device on its parent bus.
> >> >> It's specific to PCI buses.
> >> >> 
> >> >> The bus number is the "address" of a bus on its parent device.  It's the
> >> >> same regardless of the device.
> >> >> 
> >> > In case of IDE bus siting on piix3-ide it is not just arbitrary number
> >> > like you suggest here. It actually tells how to talk to a device. IDE
> >> > bus 0 is reachable through BAR0,1 of piix3-ide and IDE bus 1 is reachable
> >> > through BAR2,3. So this number is part of the device address as much as
> >> > pci slot/func is.
> >> 
> >> What I mean to say: regardless of what the device is, or what kind of
> >> buses it provides, the buses can *always* be identified the same way:
> >> define an order, identify bus by ordinal number.
> > Only if you always create them all and in the correct order. Then, just by
> > coincidence (not by design), your assertion above will be true.
> 
> By definition of bus number, not by coincidence.
> 
> It's trivial to create buses in order.  It also ensures the numbers are
> sane: start with 0, no holes.
In case of piix3-ide I agree it is indeed trivial to create them in
order, but are you sure about all other, not yet implemented, cases?
> 
> Passing bus numbers explicitly isn't hard either.  Programmer is free to
> pass nonsensical numbers.  For the most common case of one bus, the bus
> number parameter is just noise.
If programmer makes an error this is a bug that should be fixed.

> 
> There are two disputed issues here:
> 
> 1. Whether to pass bus numbers explicitly or not.  I'd prefer not.  But
>    I won't insist on it.
> 
> 2. Whether bus numbers are IDE specific or general.  In my opinion, they
>    are general.
> 
Do we have other cases of device providing two buses in qemu tree now?
I prefer to do it only for piix3-ide for now and change it later if
this is a common pattern.

> >> Because of that, I believe the concept "bus number" should be a generic
> >> qdev concept, not special to IDE buses.
> > If you think that other devices may benefit from "bus number" I can
> > move it into BusState. Important thing is that "bus number" should
> > be determined by bus creator and should be independent from order of
> > creation. The thing is other devices may use other means to address
> > different buses. For instance system bus may have two PCI domains
> > one is addressable via 0x0cf8 IO port another is addressable through
> > MMIO address 0xf8000000. "bus number" is meaningless for those two
> > buses. Instead one of them will be named address@hidden and another one is
> > address@hidden
> 
> The system bus doesn't "have two PCI domains".  There are *devices*
> providing PCI buses on the system bus.
> 
Correct. Not a good example indeed. I can't think of other device with
two buses except piix3_ide.

> In your example, there are two such devices on the system bus.  One with
> configuration I/O port 0x0cf8, and one with memory-mapped configuration
> at 0xf8000000.  Bus number is indeed meaningless, because a bus number
> numbers a device's buses, not the system bus's devices!
> 
> Devices are identified by their address on the parent bus.  The
> addressing scheme is specific to the parent bus type.
> 
> Buses are identified by their bus number within their parent device.
> The addressing scheme is always the same: ordinal number.
> 
> >> > I agree it is conceptually wrong. It is not even needed for unique device
> >> > path generation. It is only needed to make both IDE buses configurable
> >> > from command line in ISA machine. I'll drop it in favor of other 
> >> > solution,
> >> > but qdev has to rethink how devices should it addressable from command
> >> > line. Current way is broken.
> >> 
> >> I agree it's broken and needs fixing.  I appreciate you trying to fix
> >> it, but it indeed looks like it's better to fix it separately.
> >> 
> > OK.
> >
> >> >> 
> >> >> Perhaps we can treat the non-addressability of -M isapc's second IDE bus
> >> >> as a separate problem.
> >> >> 
> >> > Agree, hence I will not use this patch and will get back to just
> >> > recording bus_id. But -M isapc is just a symptom of much more serious
> >> > problem in qdev. The way devices addressable there is not well defined.
> >> > Two devices may have the same device path. Ultimately I think qdev
> >> > should use device addresses similar to what I am trying to achieve here.
> >> > For ISA machine it should automatically generate ids like address@hidden
> >> > for first isa IDE controller and address@hidden for second one. And get
> >> > rid of user assigned ids. They are good for nothing and exist only
> >> > because qdev developer didn't agree on how to name devices.
> >> 
> >> Yes, ambigous paths are a well-known problem.  For user-defined devices,
> >> users can define the (unique) ID, which provides an unambigous path.
> > This is just dropping problem onto a user. Qemu is capable to
> > create unique ids by itself. Advantage is that it will work for
> > internally create devices and user-defined devices.
> 
> IDs are a user feature.  The user has full control over them.  If we
> created IDs automatically, they could clash with the user's.
> 
That is why I am saying that it is misfeature. It should have been
automatically created for all devices.

> The problem is that our rules what to do when the user didn't assign ID
> are broken.  Yes, we should make sure every device has an unambigous
> name even then.  More so for devices created automatically.
> 
> >> But default devices don't have one.  When we have two of identical
> >> devices without ID on the same bus, their paths become ambigous.
> >> 
> >> There has been quite some discussion on "canonical path" on the list,
> >> but no consensus.  Ironically, one of the places where we got stuck was
> >> ISA.  You cut right through that, so that's progress.  Maybe people
> >> aren't looking ;)
> > That is funny since the problem was already solved looong time ago. Just
> > look at Open Firmware device path. They are capable of addressing all
> > devices just fine, ISA devices included. What specific problem you had
> > with ISA bus? 
> 
> Lack of consensus.  I was in favour of using I/O base, just like you do.
> There were worries about ISA devices not using any I/O ports.
There is a solution for that problem for almost 15 years and we are
still looking for consensus on qemu list?! Here is ISA device binding
spec for Open Firmware: http://playground.sun.com/1275/bindings/isa/isa0_4d.ps 
If ISA device have no IO ports MMIO is used.

--
                        Gleb.