KVM vs. PCI-E Function Level Reset (FLR) ...

I've ground my way through most of the Linux kernel code supporting KVM and
"Device Assignment" but I haven't been able to untangle the path of when/where PCI-E Function Level Resets (FLRs) are applied and what happens if the Virtual Machine OS/Driver attempts to perform an FLR on the PCI Pass Through Device.

It looks like the Linux KVM kernel support code issues an FLR against an
Assigned Device when the device is assigned and when it's freed but it's not clear when those actions are taken. For instance, if a device is assigned to a VM and then the VM reboots itself, is that counted as another assignment point? I.e. will KVM issue a new pci_reset_function() on the device so it shows up reset in the newly rebooted VM?

And what happens if the VM OS/Driver attempts to write the PCI Pass Through
Device's PCI-E FLR bit? I assume that that write (and the following polling reads) are trapped by the KVM code but I can't find the code which implements the PCI Configuration Space emulation to see if the FLR is implemented there. For instance, if I run Linux 2.6.30 in the VM and my Device Driver calls pci_reset_function() in its "probe()" function will that result in a Device FLR? it doesn't appear to be the case ...

Note that it's impossible for a Device Driver to call pci_reset_function() under Linux 2.6.31 and later because a call to device_lock() was added to pci_dev_reset() in chageset 8e9394ce on Feb 17, 2010 by Greg Kroah-Hartman. This means that a call to pci_reset_function() in a device driver's "probe()" routine will result in an immediate deadlock.

Thanks for any help and/or insight into these questions!

Casey

The assignment/deassignment happened when guest was created/destroyed. Currently it wouldn't issue a FLR when guest reset.

The PCI configuration space emulated is in QEmu rather than KVM. You can check qemu-kvm/hw/device-assignment.c. We didn't emulate FLR capability now. (OK, some other device specific reset method may involved, you can check pci_dev_reset())

What I saw the code is like this:

static int pci_dev_reset(struct pci_dev *dev, int probe)

{                                                         
        int rc;                                           
                                                          
        might_sleep();                                    
                                                          
        if (!probe) {                                     
                pci_block_user_cfg_access(dev);           
                /* block PM suspend, driver probe, etc. */
                device_lock(&dev->dev);                   
        }                                                

[...]

So seems it's fine with _probe_ set, to use with probe() routine.

--
regards
Yang, Sheng

| From: Sheng Yang <[email protected]>
| Date: Tuesday, July 13, 2010 05:53 pm
| 
| On Wednesday 14 July 2010 04:41:01 Casey Leedom wrote:
| > 
| >   It looks like the Linux KVM kernel support code issues an FLR against
| > an Assigned Device when the device is assigned and when it's freed
| > but it's not clear when those actions are taken.  For instance,
| > if a device is assigned to a VM and then the VM reboots itself, is
| > that counted as another assignment point? I.e. will KVM issue a
| > new pci_reset_function() on the device so it shows up reset in
| > the newly rebooted VM?
| 
| The assignment/deassignment happened when guest was created/destroyed.
| Currently it wouldn't issue a FLR when guest reset.

Hhrrmmm, this seems like a semantic error. "Resetting" the Vm should be
morally equivalent to resetting a real physical machine. And when a real physical machine is reset, all of its buses, etc. get reset which propagates to Device Resets on those buses ... I think that Assigned Devices should be reset for reboots and power off/on ...

| >   And what happens if the VM OS/Driver attempts to write the PCI Pass
| > Through Device's PCI-E FLR bit?  I assume that that write (and the
| > following polling reads) are trapped by the KVM code but I can't find the
| > code which implements the PCI Configuration Space emulation to see if the
| > FLR is implemented there.  For instance, if I run Linux 2.6.30 in the VM
| > and my Device Driver calls pci_reset_function() in its "probe()" function
| > will that result in a Device FLR? it doesn't appear to be the case ...
| 
| The PCI configuration space emulated is in QEmu rather than KVM. You can
| check qemu-kvm/hw/device-assignment.c. We didn't emulate FLR capability
| now. (OK, some other device specific reset method may involved, you can
| check pci_dev_reset())

Okay, I think that this is also going to be an issue for supporting Assigned
Devices. For PCI-E SR-IOV Virtual Functions which are assigned to a VM, they need to be reset at reboot/power off/power on and the Configuration Space emulation needs to support the Guest OS/VM Device Driver issuing an FLR ...

| >       Note that it's impossible for a Device Driver to call 
| >       pci_reset_function() under Linux 2.6.31 and later
| >       because a call to device_lock() was added to
| >       pci_dev_reset() in chageset 8e9394ce on Feb
| >       17, 2010 by Greg Kroah-Hartman.  This means
| >       that a call to pci_reset_function() in a device
| >       driver's "probe()" routine will result in an
| >       immediate deadlock.
| 
| What I saw the code is like this:
| 
| static int pci_dev_reset(struct pci_dev *dev, int probe)
| {
|         int rc;
| 
|         might_sleep();
| 
|         if (!probe) {
|                 pci_block_user_cfg_access(dev);
|                 /* block PM suspend, driver probe, etc. */
|                 device_lock(&dev->dev);
|         }
| [...]
| 
| So seems it's fine with _probe_ set, to use with probe() routine.

You're looking at a local routine in drivers/pci/pci.c. That routine is
called twice in pci_reset_function(). The "probe" parameter is used to indicate whether the caller wants to "probe" for the ability to perform a PCI Function Reset or to actually _do_ the reset. pci_reset_function() first calls pci_dev_reset() is probe=1 and, if that returns an error code, it returns immediately with the error. Otherwise it saves the PCI State of the device, makes another call to pci_dev_reset() with probe=0, and then restores the device's PCI State. Thus, this "probe" in pci_dev_reset() doesn't have anything to do with the possibility that a device's own (pci_dev *)->driver->probe() routine happens to be calling pci_reset_function(). Since, apparently, the device's own ...->probe() routine is called with the device's (pci_dev *)->lock held, a call to pci_reset_function() on itself will result in an immediate deadlock from Linux 2.6.31 on ...

Casey

(Please use reply to all next time.)

Yes, it should be done when reset. And power on/off should be there, because that's means the create and destroy the guest.

You can add the FLR support for your device if you need to call it in the guest. But I don't quite understand why you need to call it in the guest. KVM should has already done that, and it's not necessary for native device.

So you want to issue FLR(rather than probing the feature) when driver->probe() called? Current seems KVM is the only user of pci_reset_function(), so I think we can update it after your driver posted to the mailing list. Also I am not sure why you want to reset function when probing. KVM should cover them all I think.

--
regards
Yang, Sheng

[[Sorry for any duplicates -- something is going wrong with my local mail relay and my outbound mail is being rejected. I'm heading over to our IT guy's office to have a "Serious Talk" with him ... -- Casey]]

| From: Sheng Yang <[email protected]>
| Date: Wednesday, July 14, 2010 06:31 pm
| 
| On Thursday 15 July 2010 02:01:29 Casey Leedom wrote:
| > | From: Sheng Yang <[email protected]>
| > | Date: Tuesday, July 13, 2010 05:53 pm
| 
| (Please use reply to all next time.)

Sorry, old habit.

| > | On Wednesday 14 July 2010 04:41:01 Casey Leedom wrote:
|
| >   Hhrrmmm, this seems like a semantic error.  "Resetting" the Vm should
| > be morally equivalent to resetting a real physical machine.  And when
| > a real physical machine is reset, all of its buses, etc. get reset which
| > propagates to Device Resets on those buses ...  I think that Assigned
| > Devices should be reset for reboots and power off/on ...
| 
| Yes, it should be done when reset. And power on/off should be there,
| because that's means the create and destroy the guest.

Okay, cool. I've looked through the kernel code and I don't see any likely
places for putting such a VM Reboot/Reset feature in so I assume that this is also controlled by QEmu and it would need to be responsible for either doing a KVM_DEASSIGN_PCI_DEVICE/KVM_ASSIGN_PCI_DEVICE ioctl() pair for each assigned device or issuing a new KVM_RESET_ASSIGNED_PCI_DEVICE ioctl() for Reboot/Reset
...

| > Assigned Devices.  For PCI-E SR-IOV Virtual Functions which are assigned
| > to a VM, they need to be reset at reboot/power off/power on and the
| > Configuration Space emulation needs to support the Guest OS/VM Device
| > Driver issuing an FLR ...
| 
| You can add the FLR support for your device if you need to call it in the
| guest. But I don't quite understand why you need to call it in the guest.
| KVM should has already done that, and it's not necessary for native
| device.

This was mostly for device driver load/unload support. I.e. being able to
issue a Function Level Reset to a PCI Device Function (regardless of it being an SR-IOV Virtual Function or not) is a nice way to zap the device back to a canonical state.

| So you want to issue FLR(rather than probing the feature) when
| driver->probe() called? Current seems KVM is the only user of
| pci_reset_function(), so I think we can update it after your driver posted
| to the mailing list. Also I am not sure why you want to reset function
| when probing. KVM should cover them all I think.

Remember, the "probe" argument in pci_dev_reset() has nothing to do with
whether it's being called from a device driver's probe() routine. The "probe" argument in pci_dev_reset() is used for the two-stage effort in pci_reset_function() which does:

1. "try to see if it's possible to reset this function by itself"

    Call pci_dev_reset(dev, probe=1);

2. "go ahead and do the function reset"

    Call pci_dev_reset(dev, probe=0);

And yes, I'd noticed that KVM was the only current subscriber of the kernel
interface but, as I noted above, it is useful for resetting the device function into a known state -- or at least it was useful till the deadlock got introduced in 2.6.31 ... :-( And again, the use of this actually has no dependency on KVM: I would want to be able to call pci_reset_function() in any device driver's probe() routine ... It just happens that I ran into this need (and unfortunate 2.6.31 deadlock) with our PCI-E SR-IOV Virtual Function Driver.

Oh, and the driver (cxgb4vf) has been posted to net-next. I'm guessing that
it should get merged into 2.6.35 ... or maybe 2.6.36 ... I'm really not sure of how the merge schedule between net-next and the core kernel runs ...

Casey

Yeah, the detection of reset is not that straightforward... Maybe we need an ioctl for reset event in qemu-kvm kvm_reset_vcpu().

We don't need assign/deassign device when reboot/reset, we only need to notify KVM that the reset is happening, then KVM know what's to do.

OK.

What I meant was, before your driver, there is no such requirement in the code. And no one can predict every usage of the code in the future, so it's quite reasonable you called the "deadlock" is there. And I can't say it's a "deadlock" because there is no code in the current tree make it "deadlock" IIUR.

So now you have this requirement, you can modify it to fit your need. That's quite straightforward...

That's good. So you can modify the function to provide a lockless version. That make sense now. :)

--
regards
Yang, Sheng

| From: Sheng Yang <[email protected]>
| Date: Thursday, July 15, 2010 05:56 pm
| 
| Yeah, the detection of reset is not that straightforward... Maybe we need
| an ioctl for reset event in qemu-kvm kvm_reset_vcpu().
| 
| We don't need assign/deassign device when reboot/reset, we only need to
| notify KVM that the reset is happening, then KVM know what's to do.

I'm not familiar enough with the KVM/QEmu to know where how to make these
changes easily. I'd be happy to test such changes or even take a whack at making the changes if someone could point me at the relevant repositories/files. Alternatively, I could file a bug if KVM has its own bug database ...

| >   [Calling pci_reset_function() in our driver->probe() routine] was
| > mostly for device driver load/unload support.  I.e. being able
| > to issue a Function Level Reset to a PCI Device Function (regardless of
| > it being an SR-IOV Virtual Function or not) is a nice way to zap the
| > device back to a canonical state.
| 
| OK.
| 
| What I meant was, before your driver, there is no such requirement in the
| code. And no one can predict every usage of the code in the future, so
| it's quite reasonable you called the "deadlock" is there. And I can't say
| it's a "deadlock" because there is no code in the current tree make it
| "deadlock" IIUR.
| 
| So now you have this requirement, you can modify it to fit your need.
| That's quite straightforward...
| 
| >   Oh, and the driver has been posted to net-next.  I'm guessing that it
| > 
| > _should_ get merged into 2.6.35 ... or maybe 2.6.36 ... I'm really not
| > sure of how the merge schedule between net-next and the core kernel runs
| > ...
| 
| That's good. So you can modify the function to provide a lockless version.
| That make sense now. :)

Okay, I'll ask Yu Zhao why he added the lock in pci_dev_reset() (changeset
8c1c699fec9e9021bf6ff0285dee086bb27aec90) and if he feels that adding a non- locking path would make sense -- this is an area of the kernel which I've only perused so I don't want to propose random changes ... :-)

Casey