Re: [Qemu-devel] About hotplug multifunction

[Marcelo Tosatti]

On Fri, Sep 09, 2011 at 10:05:01AM -0700, Alex Williamson wrote:
> On Fri, 2011-09-09 at 10:32 +0300, Michael S. Tsirkin wrote:
> > On Fri, Sep 09, 2011 at 03:08:21AM -0400, Amos Kong wrote:
> > > Hello all,
> > > 
> > > I'm working on hotplug pci multifunction. 
> > > 
> > > 1. qemu cmdline: 
> > > ./x86_64-softmmu/qemu-system-x86_64 -snapshot -m 2000 
> > > /home/kvm_autotest_root/images/rhel61-64-virtio.qcow2 -vnc :0 -monitor 
> > > unix:/tmp/a,server,nowait --enable-kvm -net none
> > > 
> > > 2. script to add virtio-blk devices:
> > > for i in `seq 1 7` 0;do
> > > qemu-img create /tmp/resize$i.qcow2 1G -f qcow2
> > > echo drive_add 0x6.$i id=drv$i,if=none,file=/tmp/resize$i.qcow2 | nc -U 
> > > /tmp/a
> > > echo device_add 
> > > virtio-blk-pci,id=dev$i,drive=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> > > /tmp/a
> > > done
> 
> I don't think it should work this way, there shouldn't be special
> intrinsic meaning that hotplugging func 0 makes the whole device appear.

Function 0 is mandatory. Thats what the guest (at least the Linux
driver) searches when a notification for the slot is received.

> Perhaps we need a notify= device option so we can add devices without
> notifying the guest, then maybe a different command to cause the pci
> slot notification.
>
> > > 3. script to add virio-nic devices:
> > > for i in `seq 1 7` 0;do
> > > echo netdev_add tap,id=drv$i | nc -U /tmp/a
> > > echo device_add 
> > > virtio-net-pci,id=dev$i,netdev=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> > > /tmp/a
> > > done
> > > 
> > > 4. current qemu behaviors
> > > 4.1. add func 1~7 one by one, then add func 0
> > > virtio-nic : success, all funcs are added
> > > virtio-blk : success
> > > 
> > > 4.2. add func 0~7 one by one
> > > virtio-nic : failed, only func 0 is added
> > > virtio-blk : success
> > > 
> > > 4.3. removing any single func in monitor
> > > virtio-nic: func 0 are not found in 'lspci', func 1~7 also exist. 
> > > eth1~eth7 also exist.
> > > virtio-blk: func 0 are not found in 'lspci', func 1~7 also exist. the 
> > > device. /dev/vda disappears,
> > >               vdb,vdc,vde,vdf,vdg,vdh,vdi,vdj also exist. If I re-add 8 
> > > funcs to guest, they all works.
> > >               # lspci (00:06.1 ~ 00:06.7 exist, 00:06.0 doesn't exit)
> > >               00:06.1 SCSI storage controller: Red Hat, Inc Virtio block 
> > > device (rev ff)
> 
> We shouldn't be able to remove single funcs of a multifunction device,
> imho.
> 
> > something I noted when readin our acpi code:
> > we currently pass eject request for function 0 only:
> >                Name (_ADR, nr##0000)
> > We either need a device per function there (acpi 1.0),
> > send eject request for them all, or use ffff
> > as function number (newer acpi, not sure which version).
> > Need to see which guests (windows,linux) can handle which form.
> 
> I'd guess we need to change that to ffff.

No need, only make sure function 0 is there and all other functions
should be removed automatically by the guest on eject notification.

> > > 
> > > qemu sends an acpi event to guest, then guest will remove all funcs in 
> > > the slot.
> > > linux-2.6/drivers/pci/hotplug/acpiphp_glue.c:
> > > static int disable_device(struct acpiphp_slot *slot) {
> > >     list_for_each_entry(func, &slot->funcs, sibling) {
> > >         ...
> > > 
> > > Questions:
> > > 1. why func1~7 still can be found after hot-remove? is it same as real 
> > > hardware?
> 
> I think we want to behave the same as adding and removing a complete
> physical devices, which means all the functions get added and removed
> together.  Probably the only time individual functions disappear on real
> hardware is poking chipset registers to hide and expose sub devices.  It
> may not be necessary to make them atomically [in]accessible, but we
> should treat them as a set.

ACPI PCI hotplug is based on slots, not on functions. It does not
support addition/removal of individual functions.

> > > 2. why the func 1~7 could not be added to guest (addingfunc 0~7 one by 
> > > one)?
> > > 3. how about this interface to hotplug/hot-unplug multifunction:
> > >    1) Add func 1-7 by monitor, add func 0, then send an acpi event to 
> > > notice guest
> > >    2) Remove func0, send an acpi event to guest. (all funcs can be 
> > > removed)
> 
> I think I'd prefer an explicit interface.  Thanks,
> 
> Alex

Function 0 must be present for the guest to detect the device. I do
not see the problem of specifying (and documenting) that the insert
notification is sent for function 0 only.

An explicit interface is going to break the current scheme where a
single "device_add" command also does the notification.