From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey Kardashevskiy Subject: Re: [RFC PATCH] vfio: add fixup for broken PCI devices Date: Fri, 22 Jun 2012 18:16:08 +1000 Message-ID: <4FE429C8.5030308@ozlabs.ru> References: <4FBF3627.3030504@ozlabs.ru> <1339024663.23475.338.camel@bling.home> Mime-Version: 1.0 Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Cc: Benjamin Herrenschmidt , David Gibson , Alex Graf , kvm@vger.kernel.org, qemu-devel@nongnu.org To: Alex Williamson Return-path: Received: from mail-pb0-f46.google.com ([209.85.160.46]:41203 "EHLO mail-pb0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761611Ab2FVIQP (ORCPT ); Fri, 22 Jun 2012 04:16:15 -0400 Received: by pbbrp8 with SMTP id rp8so3248260pbb.19 for ; Fri, 22 Jun 2012 01:16:14 -0700 (PDT) In-Reply-To: <1339024663.23475.338.camel@bling.home> Sender: kvm-owner@vger.kernel.org List-ID: On 07/06/12 09:17, Alex Williamson wrote: > On Fri, 2012-05-25 at 17:35 +1000, Alexey Kardashevskiy wrote: >> Some adapters (like NEC PCI USB controller) do not flush their config >> on a sioftware reset and remember DMA config, etc. >> >> If we use such an adapter with QEMU, then crash QEMU (stop it with >> ctrl-A ctrl-X), and try to use it in QEMU again, it may start working >> immediately with previous config when pci_enable_device() is called >> on that PCI function. >> >> To eliminate such effect, some quirk should be called. The proposed >> pci_fixup_final does its job well for mentioned NEC PCI USB but not >> sure if it is 100% correct. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> drivers/vfio/pci/vfio_pci.c | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c >> index 1e5315c..6e7c12d 100644 >> --- a/drivers/vfio/pci/vfio_pci.c >> +++ b/drivers/vfio/pci/vfio_pci.c >> @@ -88,6 +88,8 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev) >> { >> int bar; >> >> + pci_fixup_device(pci_fixup_final, vdev->pdev); >> + >> pci_disable_device(vdev->pdev); >> >> vfio_pci_set_irqs_ioctl(vdev, VFIO_IRQ_SET_DATA_NONE | > > Sorry, just taking a look at this again. Do you have any idea what > fixup it is that makes it work? Calling a fixup at this point seems > rather odd. I suspect the problem is that vfio is only calling > pci_load_and_free_saved_state if pci_reset_function reports that it > worked. kvm device assignment doesn't do that and I'm not sure why I > did that. If you unconditionally call pci_load_and_free_saved_state a > bit further down in this function, does it solve the problem? Thanks, Checked again. Seems to be a false alarm, cannot reproduce the bad behavior anymore, looks like it was caused by another issue which Alex fixed. So although the problem may arise again, there is nothing urgent to do at the moment. -- Alexey From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45471) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Shz2W-0002RP-OB for qemu-devel@nongnu.org; Fri, 22 Jun 2012 04:16:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Shz2Q-0001jK-EU for qemu-devel@nongnu.org; Fri, 22 Jun 2012 04:16:24 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:45614) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Shz2Q-0001j2-7q for qemu-devel@nongnu.org; Fri, 22 Jun 2012 04:16:18 -0400 Received: by pbbro12 with SMTP id ro12so3336613pbb.4 for ; Fri, 22 Jun 2012 01:16:14 -0700 (PDT) Message-ID: <4FE429C8.5030308@ozlabs.ru> Date: Fri, 22 Jun 2012 18:16:08 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <4FBF3627.3030504@ozlabs.ru> <1339024663.23475.338.camel@bling.home> In-Reply-To: <1339024663.23475.338.camel@bling.home> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH] vfio: add fixup for broken PCI devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-devel@nongnu.org, Alex Graf , kvm@vger.kernel.org, David Gibson On 07/06/12 09:17, Alex Williamson wrote: > On Fri, 2012-05-25 at 17:35 +1000, Alexey Kardashevskiy wrote: >> Some adapters (like NEC PCI USB controller) do not flush their config >> on a sioftware reset and remember DMA config, etc. >> >> If we use such an adapter with QEMU, then crash QEMU (stop it with >> ctrl-A ctrl-X), and try to use it in QEMU again, it may start working >> immediately with previous config when pci_enable_device() is called >> on that PCI function. >> >> To eliminate such effect, some quirk should be called. The proposed >> pci_fixup_final does its job well for mentioned NEC PCI USB but not >> sure if it is 100% correct. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> drivers/vfio/pci/vfio_pci.c | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c >> index 1e5315c..6e7c12d 100644 >> --- a/drivers/vfio/pci/vfio_pci.c >> +++ b/drivers/vfio/pci/vfio_pci.c >> @@ -88,6 +88,8 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev) >> { >> int bar; >> >> + pci_fixup_device(pci_fixup_final, vdev->pdev); >> + >> pci_disable_device(vdev->pdev); >> >> vfio_pci_set_irqs_ioctl(vdev, VFIO_IRQ_SET_DATA_NONE | > > Sorry, just taking a look at this again. Do you have any idea what > fixup it is that makes it work? Calling a fixup at this point seems > rather odd. I suspect the problem is that vfio is only calling > pci_load_and_free_saved_state if pci_reset_function reports that it > worked. kvm device assignment doesn't do that and I'm not sure why I > did that. If you unconditionally call pci_load_and_free_saved_state a > bit further down in this function, does it solve the problem? Thanks, Checked again. Seems to be a false alarm, cannot reproduce the bad behavior anymore, looks like it was caused by another issue which Alex fixed. So although the problem may arise again, there is nothing urgent to do at the moment. -- Alexey