Re: [Qemu-ppc] [PATCH 12/12] pseries: Generate unique LIOBNs for PCI host bridges

[David Gibson]

On Thu, Nov 22, 2012 at 09:23:03AM +0200, Michael S. Tsirkin wrote:
> On Thu, Nov 22, 2012 at 01:27:18PM +1100, David Gibson wrote:
> > On Wed, Nov 21, 2012 at 05:27:37PM +0200, Michael S. Tsirkin wrote:
> > > On Wed, Nov 21, 2012 at 02:27:08PM +0100, Alexander Graf wrote:
> > > > On 11/21/2012 02:21 PM, David Gibson wrote:
> > > > >On Wed, Nov 21, 2012 at 03:13:39PM +0200, Michael S. Tsirkin wrote:
> > > > >>On Wed, Nov 21, 2012 at 11:36:00PM +1100, David Gibson wrote:
> > > > >>>On Wed, Nov 21, 2012 at 01:34:48PM +0200, Michael S. Tsirkin wrote:
> > > > >>>>On Wed, Nov 21, 2012 at 11:57:05AM +1100, David Gibson wrote:
> > > > >>>>>On Tue, Nov 20, 2012 at 02:26:09PM +0200, Michael S. Tsirkin wrote:
> > > > >>>>>>On Tue, Nov 20, 2012 at 10:27:11AM +0100, Alexander Graf wrote:
> > > > >>>>>>>On 19.11.2012, at 23:51, David Gibson wrote:
> > > > >>>>>>>
> > > > >>>>>>>>On Mon, Nov 19, 2012 at 05:34:12PM +0100, Alexander Graf wrote:
> > > > >>>>>>>>>On 13.11.2012, at 03:47, David Gibson wrote:
> > > > >>>>>>>>>
> > > > >>>>>>>>>>From: Alexey Kardashevskiy<address@hidden>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>In future (with VFIO) we will have multiple PCI host bridges 
> > > > >>>>>>>>>>on
> > > > >>>>>>>>>>pseries.  Each one needs a unique LIOBN (IOMMU id).  At the 
> > > > >>>>>>>>>>moment we
> > > > >>>>>>>>>>derive these from the pci domain number, but the whole notion 
> > > > >>>>>>>>>>of
> > > > >>>>>>>>>>domain numbers on the qemu side is bogus and in any case 
> > > > >>>>>>>>>>they're not
> > > > >>>>>>>>>>actually uniquely allocated at this point.
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>This patch, therefore uses a simple sequence counter to 
> > > > >>>>>>>>>>generate
> > > > >>>>>>>>>>unique LIOBNs for PCI host bridges.
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>Signed-off-by: Alexey Kardashevskiy<address@hidden>
> > > > >>>>>>>>>>Signed-off-by: David Gibson<address@hidden>
> > > > >>>>>>>>>I don't really like the idea of having a global variable just
> > > > >>>>>>>>>because our domain ID generation seems to not work as
> > > > >>>>>>>>>expected. Michael, any comments here?
> > > > >>>>>>>>Well, the patch I sent which changed domain id generation was
> > > > >>>>>>>>ignored.  In any case, as I said, the whole concept of domain 
> > > > >>>>>>>>numbers
> > > > >>>>>>>Michael?
> > > > >>>>>>This is user visible, right?
> > > > >>>>>>So IMHO we should have the user specify LIOBN through a property,
> > > > >>>>>>rather than assign what's essentially a random value.
> > > > >>>>>Well, I can implement an override through a property, which could 
> > > > >>>>>be
> > > > >>>>>useful in some circumstances.  But we still need to have qemu 
> > > > >>>>>generate
> > > > >>>>>unique defaults, rather than forcing it to be specified in every 
> > > > >>>>>case.
> > > > >>>>I don't see why.
> > > > >>>>And if you want automatic defaults then they need to be generated 
> > > > >>>>in a
> > > > >>>>way that does not depend on implementation detail such as order of
> > > > >>>>device initialization.
> > > > >>>Because requiring explicit unique liobns to be supplied whenever 
> > > > >>>there
> > > > >>>is more than one PHB is horrible for usability.
> > > > >>We should make simple things simple and complex things possible.
> > > > >>More than one PHB seems like an advanced feature
> > > > >Not for pseries.  On real hardware of this type, dozens of PHBs is
> > > > >routine.  Plus, vfio passthrough is coming, we need at minimum one PHB
> > > > >for emulated devices and one for passthrough devices.
> > > > 
> > > > Yeah, I second Davids opinion here. We need to make this easy for users.
> > > 
> > > I think users don't normally create PHBs. They request a disk, a network
> > > device, a pass-through device ... In this case if this requires
> > > more PHBs create them internally but I imagine this doesn't
> > > require any allocation scheme - simply set some address
> > > for virtual devices, some other one for assigned devices ... no?
> > 
> > No.  One PHB for passthrough and one for emulated is the minimum.
> > Since we don't emulated p2p bridges,
> 
> Actually qemu does emulate p2p bridges.
> 
> > each PHB can only support a small
> > number of PCI devices, so if enough PCI devices are requested, we will
> > still need to create - and assign numbers to - additional PHBs.
> 
> Each PHB can support up to 32 slots right? This seems ample for
> a typical use. If you want many tens of devices you need to
> supply addresses manually, this looks reasonable to me.
> 
> Allocating PHBs on the fly seems unencessarily tricky.

I still think you're thinking far too much in terms of x86-like
guests.  But, I guess, emulated PCI devices will be pretty rare, since
they're so slow anyway.

But, what Alex was getting at is another factor I forgot.  Because of
the way the PAPR PCI paravirtualized interfaces work, host devices
which are in different "partitionable endpoints" (roughly the minimum
granularity for safe isolation) must appear in different virtual PHBs
in the guest.  So if we pass through a lot of PCI devices, we will
almost certainly need a lot of guest PHBs.

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