From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] pci-assign: Do not reset the device unless the kernel supports it Date: Wed, 08 Jun 2011 09:52:12 +0200 Message-ID: <4DEF2A2C.4040801@web.de> References: <4DED470F.4020203@web.de> <1307396894.5901.5.camel@x201> <4DED4EDA.80803@web.de> <4DEDDC1D.7000905@redhat.com> <4DEDDDDE.4010209@web.de> <1307472373.5901.33.camel@x201> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigE774990F5E4E074219029881" Cc: Avi Kivity , Marcelo Tosatti , kvm To: Alex Williamson Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:36471 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752406Ab1FHHwR (ORCPT ); Wed, 8 Jun 2011 03:52:17 -0400 In-Reply-To: <1307472373.5901.33.camel@x201> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigE774990F5E4E074219029881 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2011-06-07 20:46, Alex Williamson wrote: > On Tue, 2011-06-07 at 10:14 +0200, Jan Kiszka wrote: >> On 2011-06-07 10:06, Avi Kivity wrote: >>> On 06/07/2011 01:04 AM, Jan Kiszka wrote: >>>> On 2011-06-06 23:48, Alex Williamson wrote: >>>>> On Mon, 2011-06-06 at 23:30 +0200, Jan Kiszka wrote: >>>>>> From: Jan Kiszka >>>>>> >>>>>> At least kernels 2.6.38 and 2.6.39 do not properly support issuin= g a >>>>>> reset on an assigned device and corrupt its config space. Prevent= >>>>>> this by checking for a host kernel with the required support, >>>> tagged by >>>>>> the to-be-introduced KVM_CAP_DEVICE_RESET. >>>>> >>>>> Wouldn't it be easier just to revert ed78661f in 2.6.39 stable? I= >>>> guess >>>>> we don't have an option to do that for .38 since stable is done th= ere, >>>>> but there are also some intel-iommu breakages that won't make >>>> stable for >>>>> that release. It seems like the userspace invoked reset resolves >>>> known, >>>>> demonstrable issues of devices continuing to DMA into guest memory= >>>> while >>>>> ed78661f is mostly a theoretical change. >>>> >>>> Easier would be this patch. But I don't mind reverting the problemat= ic >>>> commit in 39, whatever is preferred. We should just resolve the issu= e >>>> finally. >>> >>> Kernel problems should be solved in the kernel (with exceptions of >>> course, but don't see the need here). >> >> Then please file a revert for stable ASAP. >=20 > How's this? For stable only or course. Thanks, >=20 > Alex >=20 > Revert "KVM: Save/restore state of assigned PCI device" >=20 > From: Alex Williamson >=20 > This reverts ed78661f2614d3c9f69c23e280db3bafdabdf5bb as it assumes > the saved PCI state will remain valid for the entire length of time > that it is attached to a guest. This fails when userspace makes use > of the pci-sysfs reset interface, which invalidates the saved device > state, leaving nothing to be restored after the device is reset on > de-assignment. This leaves the device in an unusable state. >=20 > 3.0.0 will add an interface for KVM to save the PCI state in a [ It will be called "3.0". :) ] > buffer unaffected by other callers of pci_reset_function(), but the > most appropriate stable fix seems to be reverting this change since > the original assumption about the device saved state persisting is > incorrect. >=20 > Signed-off-by: Alex Williamson > --- >=20 > virt/kvm/assigned-dev.c | 5 +---- > 1 files changed, 1 insertions(+), 4 deletions(-) >=20 >=20 > diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c > index ae72ae6..e3f1235 100644 > --- a/virt/kvm/assigned-dev.c > +++ b/virt/kvm/assigned-dev.c > @@ -197,8 +197,7 @@ static void kvm_free_assigned_device(struct kvm *kv= m, > { > kvm_free_assigned_irq(kvm, assigned_dev); > =20 > - __pci_reset_function(assigned_dev->dev); > - pci_restore_state(assigned_dev->dev); > + pci_reset_function(assigned_dev->dev); > =20 > pci_release_regions(assigned_dev->dev); > pci_disable_device(assigned_dev->dev); > @@ -515,7 +514,6 @@ static int kvm_vm_ioctl_assign_device(struct kvm *k= vm, > } > =20 > pci_reset_function(dev); > - pci_save_state(dev); > =20 > match->assigned_dev_id =3D assigned_dev->assigned_dev_id; > match->host_segnr =3D assigned_dev->segnr; > @@ -546,7 +544,6 @@ out: > mutex_unlock(&kvm->lock); > return r; > out_list_del: > - pci_restore_state(dev); > list_del(&match->list); > pci_release_regions(dev); > out_disable: >=20 >=20 >=20 Acked-by: Jan Kiszka Jan --------------enigE774990F5E4E074219029881 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/ iEYEARECAAYFAk3vKi8ACgkQitSsb3rl5xQ3agCfUEfL2dXwVt1owHTfrijFD6Z1 YeAAoKGaUihbsB+hn+FCiOMp2VlwQt2d =Nbni -----END PGP SIGNATURE----- --------------enigE774990F5E4E074219029881--