| Message ID | 1307472373.5901.33.camel@x201 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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<jan.kiszka@siemens.com> >>>>>> >>>>>> At least kernels 2.6.38 and 2.6.39 do not properly support issuing 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 there, >>>>> 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 problematic >>>> commit in 39, whatever is preferred. We should just resolve the issue >>>> 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. > > How's this? For stable only or course. Thanks, > > Alex > > Revert "KVM: Save/restore state of assigned PCI device" > > From: Alex Williamson <alex.williamson@redhat.com> > > 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. > > 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. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > > virt/kvm/assigned-dev.c | 5 +---- > 1 files changed, 1 insertions(+), 4 deletions(-) > > > 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 *kvm, > { > kvm_free_assigned_irq(kvm, assigned_dev); > > - __pci_reset_function(assigned_dev->dev); > - pci_restore_state(assigned_dev->dev); > + pci_reset_function(assigned_dev->dev); > > 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 *kvm, > } > > pci_reset_function(dev); > - pci_save_state(dev); > > match->assigned_dev_id = assigned_dev->assigned_dev_id; > match->host_segnr = 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: > > > Acked-by: Jan Kiszka <jan.kiszka@siemens.com> Jan
On 06/07/2011 09:46 PM, Alex Williamson wrote: > Revert "KVM: Save/restore state of assigned PCI device" > > From: Alex Williamson<alex.williamson@redhat.com> > > 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. > > 3.0.0 will add an interface for KVM to save the PCI state in a > 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. > Thanks, queued for 2.6.39.
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 *kvm, { kvm_free_assigned_irq(kvm, assigned_dev); - __pci_reset_function(assigned_dev->dev); - pci_restore_state(assigned_dev->dev); + pci_reset_function(assigned_dev->dev); 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 *kvm, } pci_reset_function(dev); - pci_save_state(dev); match->assigned_dev_id = assigned_dev->assigned_dev_id; match->host_segnr = 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: