public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Casey Leedom <leedom@chelsio.com>
To: Sheng Yang <sheng@linux.intel.com>
Cc: kvm@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: KVM vs. PCI-E Function Level Reset (FLR) ...
Date: Thu, 15 Jul 2010 09:06:09 -0700	[thread overview]
Message-ID: <201007150906.09536.leedom@chelsio.com> (raw)
In-Reply-To: <201007150839.37130.leedom@chelsio.com>

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

  parent reply	other threads:[~2010-07-15 16:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-13 20:41 KVM vs. PCI-E Function Level Reset (FLR) Casey Leedom
2010-07-14  0:53 ` Sheng Yang
2010-07-14 18:01   ` Casey Leedom
2010-07-15  1:31     ` Sheng Yang
     [not found]       ` <201007150839.37130.leedom@chelsio.com>
2010-07-15 16:06         ` Casey Leedom [this message]
2010-07-16  0:56         ` Sheng Yang
2010-07-16 17:29           ` Casey Leedom
     [not found] <201007150033.o6F0XUBj024880@stargate.chelsio.com>
2010-07-15  0:55 ` Casey Leedom

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201007150906.09536.leedom@chelsio.com \
    --to=leedom@chelsio.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=sheng@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox