Re: [Qemu-devel] Re: [PATCH] PCI: Bus number from the bridge, not the device

[Gleb Natapov]

On Sun, Nov 21, 2010 at 11:50:18AM +0200, Michael S. Tsirkin wrote:
> On Sun, Nov 21, 2010 at 10:32:11AM +0200, Gleb Natapov wrote:
> > On Sat, Nov 20, 2010 at 10:17:09PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Nov 19, 2010 at 10:38:42PM +0200, Gleb Natapov wrote:
> > > > On Fri, Nov 19, 2010 at 06:02:58PM +0100, Markus Armbruster wrote:
> > > > > "Michael S. Tsirkin" <address@hidden> writes:
> > > > > 
> > > > > > On Tue, Nov 09, 2010 at 11:41:43AM +0900, Isaku Yamahata wrote:
> > > > > >> On Mon, Nov 08, 2010 at 06:26:33PM +0200, Michael S. Tsirkin wrote:
> > > > > >> > Replace bus number with slot numbers of parent bridges up to the 
> > > > > >> > root.
> > > > > >> > This works for root bridge in a compatible way because bus 
> > > > > >> > number there
> > > > > >> > is hard-coded to 0.
> > > > > >> > IMO nested bridges are broken anyway, no way to be compatible 
> > > > > >> > there.
> > > > > >> > 
> > > > > >> > 
> > > > > >> > Gleb, Markus, I think the following should be sufficient for 
> > > > > >> > PCI.  What
> > > > > >> > do you think?  Also - do we need to update QMP/monitor to teach 
> > > > > >> > them to
> > > > > >> > work with these paths?
> > > > > >> > 
> > > > > >> > This is on top of Alex's patch, completely untested.
> > > > > >> > 
> > > > > >> > 
> > > > > >> > pci: fix device path for devices behind nested bridges
> > > > > >> > 
> > > > > >> > We were using bus number in the device path, which is clearly
> > > > > >> > broken as this number is guest-assigned for all devices
> > > > > >> > except the root.
> > > > > >> > 
> > > > > >> > Fix by using hierarchical list of slots, walking the path
> > > > > >> > from root down to device, instead. Add :00 as bus number
> > > > > >> > so that if there are no nested bridges, this is compatible
> > > > > >> > with what we have now.
> > > > > >> 
> > > > > >> This format, Domain:00:Slot:Slot....:Slot.Function, doesn't work
> > > > > >> because pci-to-pci bridge is pci function.
> > > > > >> So the format should be
> > > > > >> Domain:00:Slot.Function:Slot.Function....:Slot.Function
> > > > > >> 
> > > > > >> thanks,
> > > > > >
> > > > > > Hmm, interesting. If we do this we aren't backwards compatible
> > > > > > though, so maybe we could try using openfirmware paths, just as 
> > > > > > well.
> > > > > 
> > > > > Whatever we do, we need to make it work for all (qdevified) devices 
> > > > > and
> > > > > buses.
> > > > > 
> > > > > It should also be possible to use canonical addressing with 
> > > > > device_add &
> > > > > friends.  I.e. permit naming a device by (a unique abbreviation of) 
> > > > > its
> > > > > canonical address in addition to naming it by its user-defined ID.  
> > > > > For
> > > > > instance, something like
> > > > > 
> > > > >    device_del /pci/@1,1
> > > > > 
> > > > FWIW openbios allows this kind of abbreviation.
> > > > 
> > > > > in addition to
> > > > > 
> > > > >    device_del ID
> > > > > 
> > > > > Open Firmware is a useful source of inspiration there, but should it
> > > > > come into conflict with usability, we should let usability win.
> > > > 
> > > > --
> > > >                         Gleb.
> > > 
> > > 
> > > I think that the domain (PCI segment group), bus, slot, function way to
> > > address pci devices is still the most familiar and the easiest to map to
> > Most familiar to whom?
> 
> The guests.
Which one? There are many guests. Your favorite?

> For CLI, we need an easy way to map a device in guest to the
> device in qemu and back.
Then use eth0, /dev/sdb, or even C:. Your way is not less broken since what
you are saying is "lets use name that guest assigned to a device". 

> 
> > It looks like you identify yourself with most of
> > qemu users, but if most qemu users are like you then qemu has not enough
> > users :) Most users that consider themselves to be "advanced" may know
> > what eth1 or /dev/sdb means. This doesn't mean we should provide
> > "device_del eth1" or "device_add /dev/sdb" command though. 
> > 
> > More important is that "domain" (encoded as number like you used to)
> > and "bus number" has no meaning from inside qemu.
> > So while I said many
> > times I don't care about exact CLI syntax to much it should make sense
> > at least. It can use id to specify PCI bus in CLI like this:
> > device_del pci.0:1.1. Or it can even use device id too like this:
> > device_del pci.0:ide.0. Or it can use HW topology like in FO device
> > path. But doing ah-hoc device enumeration inside qemu and then using it
> > for CLI is not it.
> > 
> > > functionality in the guests.  Qemu is buggy in the moment in that is
> > > uses the bus addresses assigned by guest and not the ones in ACPI,
> > > but that can be fixed.
> > It looks like you confused ACPI _SEG for something it isn't.
> 
> Maybe I did. This is what linux does:
> 
> struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root
> *root)
> {
>         struct acpi_device *device = root->device;
>         int domain = root->segment;
>         int busnum = root->secondary.start;
> 
> And I think this is consistent with the spec.
> 
It means that one domain may include several host bridges. At that level
domain is defined as something that have unique name for each device
inside it thus no two buses in one segment/domain can have same bus
number. This is what PCI spec tells you. 

And this further shows that using "domain" as defined by guest is very
bad idea. 

> > ACPI spec
> > says that PCI segment group is purely software concept managed by system
> > firmware. In fact one segment may include multiple PCI host bridges.
> 
> It can't I think:
Read _BBN definition:
 The _BBN object is located under a PCI host bridge and must be unique for
 every host bridge within a segment since it is the PCI bus number.

Clearly above speaks about multiple host bridge within a segment.

>       Multiple Host Bridges
> 
>       A platform may have multiple PCI Express or PCI-X host bridges. The base
>       address for the
>       MMCONFIG space for these host bridges may need to be allocated at
>       different locations. In such
>       cases, using MCFG table and _CBA method as defined in this section means
>       that each of these host
>       bridges must be in its own PCI Segment Group.
> 
This is not from ACPI spec, but without going to deep into it above
paragraph talks about some particular case when each host bridge must
be in its own PCI Segment Group with is a definite prove that in other
cases multiple host bridges can be in on segment group.

> 
> > _SEG
> > is not what OSPM uses to tie HW resource to ACPI resource. It used _CRS
> > (Current Resource Settings) for that just like OF. No surprise there.
> 
> OSPM uses both I think.
> 
> All I see linux do with CRS is get the bus number range.
So lets assume that HW has two PCI host bridges and ACPI has:
        Device(PCI0) {
            Name (_HID, EisaId ("PNP0A03"))
            Name (_SEG, 0x00)
        }
        Device(PCI1) {
            Name (_HID, EisaId ("PNP0A03"))
            Name (_SEG, 0x01)
        }
I.e no _CRS to describe resources. How do you think OSPM knows which of
two pci host bridges is PCI0 and which one is PCI1?


> And the spec says, e.g.:
> 
>         the memory mapped configuration base
>       address (always corresponds to bus number 0) for the PCI Segment Group
>       of the host bridge is provided by _CBA and the bus range covered by the
>       base address is indicated by the corresponding bus range specified in
>       _CRS.
> 
Don't see how it is relevant. And _CBA is defined only for PCI Express. Lets
solve the problem for PCI first and then move to PCI Express. Jumping from one
to another destruct us from main discussion.

> 
> > > 
> > > That should be enough for e.g. device_del. We do have the need to
> > > describe the topology when we interface with firmware, e.g. to describe
> > > the ACPI tables themselves to qemu (this is what Gleb's patches deal
> > > with), but that's probably the only case.
> > > 
> > Describing HW topology is the only way to unambiguously describe device to
> > something or someone outside qemu and have persistent device naming
> > between different HW configuration.
> 
> Not really, since ACPI is a binary blob programmed by qemu.
> 
APCI is part of the guest, not qemu. Just saying "not really" doesn't
prove much. I still haven't seen any proposition from you that actually
solve the problem. No, "lets use guest naming" is not it. There is no
such thing as "The Guest". 

--
                        Gleb.