Re: [libvirt] [Qemu-ppc] [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default

On Mon, Dec 05, 2016 at 03:54:49PM -0500, Laine Stump wrote: Actually, unlike the comments below, this assertion is neither particularly broad, nor critical of libvirt (it is obliquely critical of PAPR and PowerVM). The point is that the way PCI-E devices are presented under PAPR is different from the way they'd appear on a hardware bus or on virtualized PC or ARM. Specifically we don't present the pseudo-P2P bridges of the root complexes to the guest. Instead (unless you explicitly ask for a bridge) all the devices will appear directly on the root "bus" - obviously this bus is a fiction since PCI-E is point to point in hardware. In short, from the point of view of topology, PAPR PCI-E will look much more like vanilla PCI than PCI-E on an x86. So to the extent that libvirt address allocation differs between PCI and PCI-E, and I gather it does, the PCI-E variant will be wrong for PAPR. That's why for the medium term I advocate option (1) above of treating PAPR PCI-E as if it was a vanilla PCI bus, but with a flag indicating that PCI-E devices can be attached to it. My point is that the variety of constraints we can have in the back end is so broad that it's impossible to construct a representation to that level of detail which is truly backend independent. The problem here is "adequate details". Again, AFAICT the variety of possible constraints given all the vagaries of different platforms is so great that I don't think any format short of code is sufficient to describe it. So, here's a more specific point. You *don't* give PCI addresses to qemu - rather you give device/slot numbers, and which bus/bridge they're attached to. libvirt's use of DDDD:BB:SS.F addresses is a fundamental error, because such addresses aren't meaningful until *after* the bus is enumerated. The bus number is explicitly allocated during enumeration (by guest firmware or OS). You can kind of predict what they'll do, but it requires knowing about every bridge and device on the system, which means it risks breakage every time the backend changes its default setup - and the interpretation of the addresses could be changed by an insertion of a bridge somewhere apparently unrelated in the XML. The domain number is an entirely guest-local concept, except - sorta - on x86 where there's an obvious choice of the index that's used to control the multiplexed access to all the host bridges' config spaces. On platforms which have separate registers for each host bridge, it's entirely possible for libvirt's idea of domain numbers to differ from the guest's which is wonderfully confusing. I dispute the "any time". I'll grant you want to keep the virtual hardware stable for an extended period. But if you ever want to update your backend to newer behaviour (meaning new machine type version, not just new qemu version) then you should revisit address allocation. And, yes, that potentially breaks the guest, but so could the backend changes anyway, just like any hardware update for a physical machine. In most case - assuming it's been set up sensibly - the guest OS will cope with minor hw reshuffles. That is, after all, why RHEL locates filesystems by UUID and NICs by MAC in normal operation. Putting in the config seems like the logical place at first, but it's a mistake: First, the information we need to ensure stable guest hardware is inherently backend specific. On qemu we need to say which bridges and bus ids each device is connected to. Some other system could use PCI addresses (I think that would be a a design flaw in the backend, but if libvirt can't deal with imperfect backends it has failed its remit), for containers the whole question doesn't even make sense. A really simplistic hypervisor could take simply an (ordered) list of plugin devices and always put them into a fixed set of slots. Putting this into the general config forces us to attempt a solution to the insoluble: greating a neutral representation translatable to any backend specific representation. Second, this means that the config now contains both information that was originally entered by the user (or next level up) as what they want for the VM, and also information that libvirt decided once upon a time and is now sticking with. We can no longer separate which bits of information are which. That means when we want to update the backend, we can't tell what we should regenerate. [Meta rant, when it comes to design flaws that breed other design flaws the fact that the domain XML is both a read and write interface is just about the granddaddy of them all. It has two awful consequences: 1) After the VM is first run, we can no longer tell which config constraints were ones actually requested by the user, and which were added by some version of libvirt in the past and are now just there so we make the same decisions as before. That distinction matters. 2) Because the user could be relying on *seeing* something in the XML, not just that whatever it put in there will still be understood by new libvirt versions means we basically can't ever fix errors in the XML design.] CAN not? Why not? Fundamentally it seems like we need to get the information about what the user wants and what the virtual hardware can do into the same place. At least from the examples you've mentioned so far, pushing "needs to be hotpluggable" and so forth flags down into qemu seems a lot more feasible that pulling up the complex and platform dependent set of constraints into libvirt. And yes, qemu is very far from perfect in this area - the introspection interfaces need to be greatly improved. So both (a) and (b) are precisely examples of what I'm describing. libvirt is assuming (a) requires hotplug => must be on a root port (b) device bound to NUMA node => use expander bus But (a) is only true for guest platforms using the standard hotplug mechanism. pseries does not. (b) is essentially only true for x86 - the whole concept of the expander bus is a hack to workaround x86's historical inability to have truly independent PCI host bridge. On pseries we can create independent host bridges and assign them each a NUMA node (including the default one). qemu already knew that, libvirt needs to be taught. Just as it seems like libvirt needs to be taught about practically every platform quirk. Again, no, qemu does not take addresses at all. At least not in the "DDDD:BB:SS.F" sense. Even if it did, DDDD:BB:SS.F addresses have no place in a supposedly back-end independent config. Ok, so, yes, I see that a lot of this allocation mess originates because qemu didn't provide sufficient auto-allocation, or introspection, so libvirt had to work around that. The trouble is that those workarounds have now been baked into the libvirt interface - primarily by the fact that the data is integrated back into the primary config rather than stored as adjunct back end specific data. That means it's now insanely difficult to improve the interfaces to qemu, because libvirt XMLs now have all this address information. A few of them are because the user really did request that for some reason, but many are because that was how libvirt tried to implement some more abstract user request. We can now no longer tell the difference, which means we can't translate those abstract requests into a new saner qemu interface. Frankly, if the higher level management needs that much control over the PCI topology, going via an abstraction layer like libvirt seems not the right choice. In that case the management needs to understand the guest platform in great detail anyway, so what does libvirt buy you. Huh. Your patch makes perfect sense to me - a step in the right direction - and the reason for NACK seems completely bogus. Yeah, but "address which makes them hotpluggable" depends on guest platform (machine type). As does how you assign them to NUMA nodes. So, I'm not necessarily against allowing an explicit address override for special cases (although DDDD:BB:SS.F is the wrong way to encode it). If that breaks due to a backend update, it's reasonable to just punt the error back to the user and get them to fix it - if they didn't understand the guest platform well enough to fix this, they shouldn't have been overriding addresses. But it's not reasonable to punt back to the user when the only thing that got broken by the backend update was information libvirt (maybe not even the current version) decided some time in the past, based on faulty assumptions about the guest platform. But libvirt has no way to distinguish these cases because all the info is folded back into the one XML blob. I haven't had a chance to look in detail at Eduardo's latest stuff, but yes it should be an improvement. But I think you vastly underestimate how many different platform quirks we could have which won't be easy to encode. Right, there are certainly reasons for overrides. But at the moment *every* device is overridden, which means when there is something wrong in libvirt's assumptions about how to place things, every device can break, not just overridden ones. With the tools that qemu currently gives you. But now the assumption that address allocation belongs to libvirt means that it's insanely difficult to build those tools without breaking libvirt. Well, I did warn you it was a rant. That said, while there are understandable historical reasons for them design flaws in libvirt which make it really difficult to design other things properly are alas not a myth. Three examples: 1) Read/write XML interface. Makes it nigh-impossible to fix flaws by updating to a more coherent internal representation (with backwards compat glue to parse the old format into the new one). 2) Not separating original user config from libvirt additions / annotations. Discussed repeatedly above. 3) Use of DDDD:BB.SS.F addresses (rather than a reference to bus attachment point + SS.F). You can't use addresses to construct a machine in an address space which only exists once the machine is constructed. And that's just the first three I came up with that have at least a slight connection to the problem at hand. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson