Re: [libvirt] [Qemu-devel] [RFC] qmp: query-device-slots command

On 12/13/2016 06:00 PM, Eduardo Habkost wrote: > On Tue, Dec 13, 2016 at 04:15:18PM +0200, Marcel Apfelbaum wrote: >> On 12/13/2016 02:42 PM, Eduardo Habkost wrote: >>> On Tue, Dec 13, 2016 at 12:04:17PM +0100, Markus Armbruster wrote: >>>> Quick interface review only: >>>> >>>> Eduardo Habkost <ehabkost(a)redhat.com&gt; writes: >>>> >>>>> This adds a new command to QMP: query-device-slots. It will allow >>>>> management software to query possible slots where devices can be >>>>> plugged. >>>>> >>>>> This implementation of the command will return: >>>>> >>>>> * Multiple PCI slots per bus, in the case of PCI buses; >>>>> * One slot per bus in the case of the other buses; >>>> >>>> Umm, that doesn't seem right. For instance, a SCSI bus has multiple >>>> slots. The slot address is the SCSI ID. An IDE bus may have one >>>> (SATA) >>>> or two (PATA). For more examples, see qdev-device-use.txt section >>>> "Specifying Bus and Address on Bus". >>> >>> Yes, I should have clarified that: this version changes only PCI >>> to expose multiple slots, but other buses also need be changed to >>> implement BusClass::enumerate_slots() properly, too. >>> >>>> >>>>> * One slot for each entry from query-hotpluggable-cpus. >>>>> In the example below, I am not sure if the PCIe ports are all >>>>> supposed to report all slots, but I didn't find any existing >>>>> field in PCIBus that would help me figure out the actual number >>>>> of slots in a given PCI bus. >>>>> >>>>> Git tree >>>>> -------- >>>>> >>>>> This patch needs the previous query-machines series I am working >>>>> on. The full tree can be found on the git tree at: >>>>> >>>>> git://github.com/ehabkost/qemu-hacks.git >>>>> work/query-machines-bus-info >>>>> >>>>> Example output >>>>> -------------- >>>>> >>>>> The following output was returned by QEMU when running it as: >>>>> >>>>> $ qemu-system-x86_64 -machine q35 \ >>>>> -readconfig docs/q35-chipset.cfg \ >>>>> -smp 4,maxcpus=8,sockets=2,cores=2,threads=2 >>>>> >>>>> { >>>>> "return": [ >>>> >>>> Are you sure >3000 lines of example output make sense here? >>> >>> I'm not. :) >>> >>> That's why I need feedback from the PCI experts. I believe most >>> of the PCI ports on q35 accept only one device,

but I see no code >>> implementing that restriction, and no obvious PCIBus or >>> PCIBusClass field indicating that. >> >> Hi, >> >> Sadly there is no such restriction today as far as I know. >> We trust the user that he knows what he is doing :) >> and we let libvirt implement the restriction. >> >> I do agree we should fix that. > > OK, this matches what I saw on the current code. > > But I would like to understand what exactly should be the rules > if we fix that. Should we report only 1 slot if > pci_bus_has_pcie_upstream_port(bus)? > We should report 1 slot for a bus exposed by a PCIe Root Port or PCIe Downstream Port. Be aware that 1 slot doesn't necessarily refer to only one device, we can have a multi-function device.

Thanks, Marcel > (Note that pci_bus_has_pcie_upstream_port() doesn't exist in the > current code, it is added by my patch) >