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

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

[Casey Leedom]

  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

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

[Sheng Yang]

On Wednesday 14 July 2010 04:41:01 Casey Leedom wrote:
>   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?

The assignment/deassignment happened when guest was created/destroyed. Currently 
it wouldn't issue a FLR when guest reset. 
 
>   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())

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

--
regards
Yang, Sheng


> 
>   Thanks for any help and/or insight into these questions!
> 
> Casey
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Casey Leedom]

| From: Sheng Yang <sheng@linux.intel.com>
| 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

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

[Casey Leedom]

  We've done a bit of testing and determined that the KVM support code does 
call pci_reset_function() from kvm_free_assigned_device() when a Virtual Machine 
is powered down.  But pci_reset_function() is not called when a Virtual 
Machine is rebooted.  It really should be reset for a reboot to track what 
happens to a Physical Host when a reboot is performed ...

  We've also determined that writing the FLR bit in the PCI-E Express Capability 
in an Assigned Device from the Guest OS/Driver does not result in a call to 
pci_reset_function() in the Hypervisor.  This seems to be a problem ...

Casey

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

[Sheng Yang]

On Thursday 15 July 2010 02:01:29 Casey Leedom wrote:
> | From: Sheng Yang <sheng@linux.intel.com>
> | Date: Tuesday, July 13, 2010 05:53 pm

(Please use reply to all next time.)
> | 
> | 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 ...

Yes, it should be done when reset. And power on/off should be there, because that's 
means the create and destroy the guest.
> 
> | >   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 ...

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.

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

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

> 
> Casey
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Casey Leedom]

[[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 <sheng@linux.intel.com>
| Date: Wednesday, July 14, 2010 06:31 pm
| 
| On Thursday 15 July 2010 02:01:29 Casey Leedom wrote:
| > | From: Sheng Yang <sheng@linux.intel.com>
| > | 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

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

[Sheng Yang]

On Thursday 15 July 2010 23:39:36 Casey Leedom wrote:
> | From: Sheng Yang <sheng@linux.intel.com>
> | Date: Wednesday, July 14, 2010 06:31 pm
> | 
> | On Thursday 15 July 2010 02:01:29 Casey Leedom wrote:
> | > | From: Sheng Yang <sheng@linux.intel.com>
> | > | 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 ...

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

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

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. :)

--
regards
Yang, Sheng

> 
> Casey

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

[Casey Leedom]

| From: Sheng Yang <sheng@linux.intel.com>
| 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