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 <alex.williamson@xxxxxxxxxx>
> > ---
> >
> > 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;
> > };
> >
> > 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);
> >
> > - __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);
> >
> > pci_release_regions(assigned_dev->dev);
> > @@ -516,7 +518,7 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> >
> > pci_reset_function(dev);
> > pci_save_state(dev);
> > -
> > + match->pci_saved_state = pci_store_saved_state(dev);
> > match->assigned_dev_id = assigned_dev->assigned_dev_id;
> > match->host_segnr = assigned_dev->segnr;
> > match->host_busnr = 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?
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. The only reason we even really need to call
pci_load_and_free_saved_state() here is to free the buffer Thanks,
Alex
> > list_del(&match->list);
> > pci_release_regions(dev);
> > out_disable:
> >
>
> Thanks for addressing the issue!
>
> Jan
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
