qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gleb Natapov <gleb@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: blauwirbel@gmail.com, alex.williamson@redhat.com,
	qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: Re: [Qemu-devel] [PATCHv2 4/8] Store IDE bus id in IDEBus structure for easy access.
Date: Thu, 4 Nov 2010 11:23:48 +0200	[thread overview]
Message-ID: <20101104092348.GB6018@redhat.com> (raw)
In-Reply-To: <m3k4ktscfy.fsf@blackfin.pond.sub.org>

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.

> 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 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 ide@0 since there will be only one
bus on piix3-ide. Given name ide@0 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"


> >> >> >                                          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.

> 
> 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 pci@i0cf8 and another one is
pci@f8000000.


> > 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 isa-ide@0x170
> > for first isa IDE controller and isa-ide@0x1f0 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.

> 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? 

--
			Gleb.

  reply	other threads:[~2010-11-04  9:23 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-31 11:40 [Qemu-devel] [PATCHv2 0/8 RFC] boot order specification Gleb Natapov
2010-10-31 11:40 ` [Qemu-devel] [PATCHv2 1/8] Introduce deriver_name field to DeviceInfo structure Gleb Natapov
2010-11-04  9:20   ` Markus Armbruster
2010-11-04  9:42     ` Gleb Natapov
2010-11-04 14:58       ` Markus Armbruster
2010-11-04 15:44         ` Gleb Natapov
2010-11-05 14:14           ` Markus Armbruster
2010-11-05 15:41             ` Gleb Natapov
2010-11-05 16:24               ` Markus Armbruster
2010-11-05 18:31                 ` Gleb Natapov
2010-11-06  9:01                   ` Markus Armbruster
2010-11-06 11:53                     ` Gleb Natapov
2010-11-06 12:55                       ` Markus Armbruster
2010-10-31 11:40 ` [Qemu-devel] [PATCHv2 2/8] Keep track of ISA ports ISA device is using in qdev Gleb Natapov
2010-10-31 11:40 ` [Qemu-devel] [PATCHv2 3/8] Add get_dev_path callback to ISA bus " Gleb Natapov
2010-10-31 11:40 ` [Qemu-devel] [PATCHv2 4/8] Store IDE bus id in IDEBus structure for easy access Gleb Natapov
2010-11-03 13:39   ` Markus Armbruster
2010-11-03 13:47     ` Gleb Natapov
2010-11-03 15:18       ` Markus Armbruster
2010-11-03 16:43         ` Gleb Natapov
2010-11-03 17:22           ` Markus Armbruster
2010-11-04  8:07             ` Gleb Natapov
2010-11-04  8:46               ` Markus Armbruster
2010-11-04  9:23                 ` Gleb Natapov [this message]
2010-11-04 14:22                   ` Markus Armbruster
2010-11-04 15:26                     ` Gleb Natapov
2010-11-05 14:04                       ` Markus Armbruster
2010-11-05 15:54                         ` Gleb Natapov
2010-11-05 16:31                           ` Markus Armbruster
2010-11-05 18:44                             ` Gleb Natapov
2010-11-06  9:25                               ` Markus Armbruster
2010-11-06 11:37                                 ` Gleb Natapov
2010-11-06 12:46                                   ` Markus Armbruster
2010-10-31 11:40 ` [Qemu-devel] [PATCHv2 5/8] Add get_dev_path callback to IDE bus Gleb Natapov
2010-10-31 11:40 ` [Qemu-devel] [PATCHv2 6/8] Add get_dev_path callback for system bus Gleb Natapov
2010-10-31 11:40 ` [Qemu-devel] [PATCHv2 7/8] Change pci bus get_dev_path callback to print only slot and func Gleb Natapov
2010-11-08 17:17   ` [Qemu-devel] " Michael S. Tsirkin
2010-11-08 17:29     ` Gleb Natapov
2010-11-08 18:12       ` Michael S. Tsirkin
2010-11-08 18:22         ` Gleb Natapov
2010-11-08 21:00       ` Michael S. Tsirkin
2010-11-08 21:12         ` Gleb Natapov
2010-11-08 17:26   ` Michael S. Tsirkin
2010-10-31 11:40 ` [Qemu-devel] [PATCHv2 8/8] Add bootindex parameter to net/block/fd device Gleb Natapov
2010-10-31 22:25 ` [Qemu-devel] Re: [PATCHv2 0/8 RFC] boot order specification Kevin O'Connor
2010-11-01  7:53   ` Gleb Natapov
2010-11-04  9:24     ` Markus Armbruster
2010-11-04  9:45       ` Gleb Natapov

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=20101104092348.GB6018@redhat.com \
    --to=gleb@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=armbru@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=kvm@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).