From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:46919)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1ScSqA-00061Q-Sj
	for qemu-devel@nongnu.org; Wed, 06 Jun 2012 22:52:52 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1ScSq9-0001EO-4o
	for qemu-devel@nongnu.org; Wed, 06 Jun 2012 22:52:50 -0400
Received: from gate.crashing.org ([63.228.1.57]:59397)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1ScSq8-0001EE-RV
	for qemu-devel@nongnu.org; Wed, 06 Jun 2012 22:52:49 -0400
Message-ID: <1339037557.24838.2.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Thu, 07 Jun 2012 12:52:37 +1000
In-Reply-To: <1339024663.23475.338.camel@bling.home>
References: <4FBF3627.3030504@ozlabs.ru>
	<1339024663.23475.338.camel@bling.home>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Mime-Version: 1.0
Subject: Re: [Qemu-devel] [RFC PATCH] vfio: add fixup for broken PCI devices
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>, qemu-devel@nongnu.org, Alex Graf <agraf@suse.de>, kvm@vger.kernel.org, David Gibson <david@gibson.dropbear.id.au>

On Wed, 2012-06-06 at 17:17 -0600, Alex Williamson wrote:
> > 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?  

No it won't do, you need device-specific "reset" fixup code for devices
where the function reset doesn't do the right thing.

My suggestion is to add a new quirk category (in addition to
early,late,... add reset) and call that here.

Then we can do one for the NEC OHCI that properly stops the controller,
among others. I would be -very- surprised if that chip ends up being the
only one causing that sort of trouble.

Also, some chips will need some "tweaks" after the reset, for example if
we do a full link reset, I know of at least one device that might
randomly fail to properly train the PCIe link, such a quirk is a perfect
spot to add the right fixup.

Cheers,
Ben.