From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 2/2] KVM: Use pci_store/load_saved_state() around VM device usage Date: Fri, 15 Apr 2011 22:28:09 +0200 Message-ID: <4DA8AA59.9030003@web.de> References: <20110415194220.2838.10750.stgit@s20.home> <20110415195439.2838.58928.stgit@s20.home> <4DA8A493.8000509@web.de> <1302898430.3110.31.camel@x201> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigFA36016F0910491299031886" Cc: linux-pci@vger.kernel.org, kvm@vger.kernel.org, avi@redhat.com, jbarnes@virtuousgeek.org To: Alex Williamson Return-path: In-Reply-To: <1302898430.3110.31.camel@x201> Sender: linux-pci-owner@vger.kernel.org List-Id: kvm.vger.kernel.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigFA36016F0910491299031886 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2011-04-15 22:13, Alex Williamson wrote: > On Fri, 2011-04-15 at 22:03 +0200, Jan Kiszka wrote: >> On 2011-04-15 21:54, Alex Williamson wrote: >>> Store the device saved state so that we can reload the device back >>> to the original state when it's unassigned. This has the benefit >>> that the state survives across pci_reset_function() calls via >>> the PCI sysfs reset interface while the VM is using the device. >>> >>> Signed-off-by: Alex Williamson >>> --- >>> >>> include/linux/kvm_host.h | 1 + >>> virt/kvm/assigned-dev.c | 8 +++++--- >>> 2 files changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >>> index ab42855..d8a1d18 100644 >>> --- a/include/linux/kvm_host.h >>> +++ b/include/linux/kvm_host.h >>> @@ -513,6 +513,7 @@ struct kvm_assigned_dev_kernel { >>> struct kvm *kvm; >>> spinlock_t intx_lock; >>> char irq_name[32]; >>> + void *pci_saved_state; >>> }; >>> =20 >>> struct kvm_irq_mask_notifier { >>> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c >>> index ae72ae6..66c6ccd 100644 >>> --- a/virt/kvm/assigned-dev.c >>> +++ b/virt/kvm/assigned-dev.c >>> @@ -197,7 +197,9 @@ static void kvm_free_assigned_device(struct kvm *= kvm, >>> { >>> kvm_free_assigned_irq(kvm, assigned_dev); >>> =20 >>> - __pci_reset_function(assigned_dev->dev); >>> + pci_reset_function(assigned_dev->dev); >>> + pci_load_and_free_saved_state(assigned_dev->dev, >>> + &assigned_dev->pci_saved_state); >>> pci_restore_state(assigned_dev->dev); >>> =20 >>> pci_release_regions(assigned_dev->dev); >>> @@ -516,7 +518,7 @@ static int kvm_vm_ioctl_assign_device(struct kvm = *kvm, >>> =20 >>> pci_reset_function(dev); >>> pci_save_state(dev); >>> - >>> + match->pci_saved_state =3D pci_store_saved_state(dev); >>> match->assigned_dev_id =3D assigned_dev->assigned_dev_id; >>> match->host_segnr =3D assigned_dev->segnr; >>> match->host_busnr =3D assigned_dev->busnr; >>> @@ -546,7 +548,7 @@ out: >>> mutex_unlock(&kvm->lock); >>> return r; >>> out_list_del: >>> - pci_restore_state(dev); >>> + pci_load_and_free_saved_state(dev, &match->pci_saved_state); >> >> Don't you need to keep the balance, ie. load_and_free, then restore? >=20 > I don't see that pci_save_state() does anything more than buffer the > hardware device state into save areas in struct pci_dev. So by not > doing a restore, we are leaving that valid, but I don't really see how > that hurts anything.=20 Right, I'm just unsure if we should encode this knowledge about how pci_save/restore_state works internally into KVM. Jan --------------enigFA36016F0910491299031886 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/ iEYEARECAAYFAk2oql0ACgkQitSsb3rl5xRWHgCfQQgGvkWnVSAlXGn2fyWQbw6W X+AAnjQZLM3fvdYu0DoQ3K5aPAM5JaVW =o/p7 -----END PGP SIGNATURE----- --------------enigFA36016F0910491299031886--