From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [Qemu-devel] [PATCHv2 4/8] Store IDE bus id in IDEBus structure for easy access. Date: Thu, 4 Nov 2010 17:26:31 +0200 Message-ID: <20101104152631.GA14910@redhat.com> References: <1288525209-3303-5-git-send-email-gleb@redhat.com> <20101103134726.GI7881@redhat.com> <20101103164308.GJ7881@redhat.com> <20101104080730.GA6018@redhat.com> <20101104092348.GB6018@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: blauwirbel@gmail.com, alex.williamson@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org To: Markus Armbruster Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59842 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752257Ab0KDP0k (ORCPT ); Thu, 4 Nov 2010 11:26:40 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Nov 04, 2010 at 03:22:50PM +0100, Markus Armbruster wrote: > Gleb Natapov 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 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" > > 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 pci@i0cf8 and another one is > > pci@f8000000. > > 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 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. > > 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.