Re: [Qemu-devel] Re: [PATCH 09/10] pci: set PCI multi-function bit appropriately.

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Jun 18, 2010 at 10:38:20PM +0900, Isaku Yamahata wrote:
> On Fri, Jun 18, 2010 at 03:44:04PM +0300, Michael S. Tsirkin wrote:
> > > > If I understand correctly what this does, it goes over
> > > > other functions of the same device, and sets the MULTI_FUNCTION bit
> > > > for them if there's more than one function.
> > > > Instead, why don't we just set PCI_HEADER_TYPE_MULTI_FUNCTION
> > > > in relevant devices?
> > > 
> > > pci address, devfn ,is exported to users as addr property,
> > > so users can populate pci function(PCIDevice in qemu)
> > > at arbitrary devfn.
> > > It means each function(PCIDevice) don't know whether pci device
> > > (PCIDevice[8]) is multi function or not.
> > > So I chose to handle it in pci generic layer.
> > > 
> > > It can be argued that it's user operation fault and that
> > > the missing part is validation checks to catch such user errors.
> > 
> > Exactly. Another part that is missing is a way to hotplug
> > a multifunction device.
> 
> Yes, multi function hot plug is also on my wish list.
> 
> 
> > OTOH I think that hotplug of separate functions has no chance to work,
> > so users are better off getting an error.
> > 
> > > But I prefer more flexible and more user friendly way.
> > 
> > I think that most users would only add many functions
> > to a device as a result of an error.
> >
> > If we really want the ability to put unrelated devices
> > as functions in a single one, let's just add
> > a 'multifunction' qdev property, and validate that
> > it is set appropriately.
> 
> I think "unrelated" is policy. There is no obvious way to determine
> which functions can be in a same device.
> For example, popular chipset contains isa bridge, ide controller,
> usb controller, sound and modem in a single device as functions.
> It's up to hardware designer policy which functions are grouped into
> a device.
> 
> So qemu should be able to populate any function in a device,
> and leave such policy check to higher level tool like libvirt/virt-manager.

Fine. But tweaking one device's config when another one is added does not
lead to robust code.  Let's just require that users declare devices as
multifunction, and verify this.

-- 
MST