Re: [PATCH 2/2] KVM: Use pci_store/load_saved_state() around VM device usage

Jan Kiszka
2011-04-15T20:28:09Z

[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]

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 <alex.williamson@redhat.com>
>>> ---
>>>
>>> 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.

Right, I'm just unsure if we should encode this knowledge about how
pci_save/restore_state works internally into KVM.

Jan

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]