| Message ID | 1436276739-50326-2-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2015-07-07 at 15:45 +0200, Paolo Bonzini wrote: > If there are no assigned devices, the guest PAT are not providing > any useful information and can be overridden to writeback; VMX > always does this because it has the "IPAT" bit in its extended > page table entries, but SVM does not have anything similar. > Hook into VFIO and legacy device assignment so that they > provide this information to KVM. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/include/asm/kvm_host.h | 3 ++- > arch/x86/kvm/iommu.c | 2 ++ > arch/x86/kvm/x86.c | 18 ++++++++++++++++++ > include/linux/kvm_host.h | 18 +++++++++++++++++- > virt/kvm/vfio.c | 5 +++++ > 5 files changed, 44 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 2a7f5d782c33..7fe26ba9082e 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -602,8 +602,9 @@ struct kvm_arch { > struct list_head assigned_dev_head; > struct iommu_domain *iommu_domain; > bool iommu_noncoherent; > -#define __KVM_HAVE_ARCH_NONCOHERENT_DMA > +#define __KVM_HAVE_ARCH_VFIO_HOOKS Do we really want to tie these two things together under something that's not strictly a "vfio" option? Legacy assignment also makes use of these, as shown in this patch, but even if we consider that temporary until legacy assignment is removed, I can imagine platforms that might care about one but not the other. I don't really see the harm in using a separate #define, perhaps __KVM_HAVE_ARCH_ASSIGNED_DEVICE. Thanks, Alex > atomic_t noncoherent_dma_count; > + atomic_t assigned_device_count; > struct kvm_pic *vpic; > struct kvm_ioapic *vioapic; > struct kvm_pit *vpit; > diff --git a/arch/x86/kvm/iommu.c b/arch/x86/kvm/iommu.c > index 7dbced309ddb..5c520ebf6343 100644 > --- a/arch/x86/kvm/iommu.c > +++ b/arch/x86/kvm/iommu.c > @@ -200,6 +200,7 @@ int kvm_assign_device(struct kvm *kvm, struct pci_dev *pdev) > goto out_unmap; > } > > + kvm_arch_start_assignment(kvm); > pci_set_dev_assigned(pdev); > > dev_info(&pdev->dev, "kvm assign device\n"); > @@ -224,6 +225,7 @@ int kvm_deassign_device(struct kvm *kvm, struct pci_dev *pdev) > iommu_detach_device(domain, &pdev->dev); > > pci_clear_dev_assigned(pdev); > + kvm_arch_end_assignment(kvm); > > dev_info(&pdev->dev, "kvm deassign device\n"); > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index bbaf44e8f0d3..6e455875d2e4 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -8218,6 +8218,24 @@ bool kvm_arch_can_inject_async_page_present(struct kvm_vcpu *vcpu) > kvm_x86_ops->interrupt_allowed(vcpu); > } > > +void kvm_arch_start_assignment(struct kvm *kvm) > +{ > + atomic_inc(&kvm->arch.assigned_device_count); > +} > +EXPORT_SYMBOL_GPL(kvm_arch_start_assignment); > + > +void kvm_arch_end_assignment(struct kvm *kvm) > +{ > + atomic_dec(&kvm->arch.assigned_device_count); > +} > +EXPORT_SYMBOL_GPL(kvm_arch_end_assignment); > + > +bool kvm_arch_has_assigned_device(struct kvm *kvm) > +{ > + return atomic_read(&kvm->arch.assigned_device_count); > +} > +EXPORT_SYMBOL_GPL(kvm_arch_has_assigned_device); > + > void kvm_arch_register_noncoherent_dma(struct kvm *kvm) > { > atomic_inc(&kvm->arch.noncoherent_dma_count); > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 9564fd78c547..56146fdb1ff0 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -716,10 +716,13 @@ static inline void kvm_arch_free_vm(struct kvm *kvm) > } > #endif > > -#ifdef __KVM_HAVE_ARCH_NONCOHERENT_DMA > +#ifdef __KVM_HAVE_ARCH_VFIO_HOOKS > void kvm_arch_register_noncoherent_dma(struct kvm *kvm); > void kvm_arch_unregister_noncoherent_dma(struct kvm *kvm); > bool kvm_arch_has_noncoherent_dma(struct kvm *kvm); > +void kvm_arch_start_assignment(struct kvm *kvm); > +void kvm_arch_end_assignment(struct kvm *kvm); > +bool kvm_arch_has_assigned_device(struct kvm *kvm); > #else > static inline void kvm_arch_register_noncoherent_dma(struct kvm *kvm) > { > @@ -733,6 +736,19 @@ static inline bool kvm_arch_has_noncoherent_dma(struct kvm *kvm) > { > return false; > } > + > +static inline void kvm_arch_start_assignment(struct kvm *kvm) > +{ > +} > + > +static inline void kvm_arch_end_assignment(struct kvm *kvm) > +{ > +} > + > +static inline bool kvm_arch_has_assigned_device(struct kvm *kvm) > +{ > + return false; > +} > #endif > > static inline wait_queue_head_t *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu) > diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c > index 620e37f741b8..1dd087da6f31 100644 > --- a/virt/kvm/vfio.c > +++ b/virt/kvm/vfio.c > @@ -155,6 +155,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) > list_add_tail(&kvg->node, &kv->group_list); > kvg->vfio_group = vfio_group; > > + kvm_arch_start_assignment(dev->kvm); > + > mutex_unlock(&kv->lock); > > kvm_vfio_update_coherency(dev); > @@ -190,6 +192,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) > break; > } > > + kvm_arch_end_assignment(dev->kvm); > + > mutex_unlock(&kv->lock); > > kvm_vfio_group_put_external_user(vfio_group); > @@ -239,6 +243,7 @@ static void kvm_vfio_destroy(struct kvm_device *dev) > kvm_vfio_group_put_external_user(kvg->vfio_group); > list_del(&kvg->node); > kfree(kvg); > + kvm_arch_end_assignment(dev->kvm); > } > > kvm_vfio_update_coherency(dev); -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/07/2015 17:22, Alex Williamson wrote: >> > -#define __KVM_HAVE_ARCH_NONCOHERENT_DMA >> > +#define __KVM_HAVE_ARCH_VFIO_HOOKS > Do we really want to tie these two things together under something > that's not strictly a "vfio" option? Legacy assignment also makes use > of these, as shown in this patch, but even if we consider that temporary > until legacy assignment is removed, I can imagine platforms that might > care about one but not the other. I don't really see the harm in using > a separate #define, perhaps __KVM_HAVE_ARCH_ASSIGNED_DEVICE. Thanks, Sure, that's okay. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 2a7f5d782c33..7fe26ba9082e 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -602,8 +602,9 @@ struct kvm_arch { struct list_head assigned_dev_head; struct iommu_domain *iommu_domain; bool iommu_noncoherent; -#define __KVM_HAVE_ARCH_NONCOHERENT_DMA +#define __KVM_HAVE_ARCH_VFIO_HOOKS atomic_t noncoherent_dma_count; + atomic_t assigned_device_count; struct kvm_pic *vpic; struct kvm_ioapic *vioapic; struct kvm_pit *vpit; diff --git a/arch/x86/kvm/iommu.c b/arch/x86/kvm/iommu.c index 7dbced309ddb..5c520ebf6343 100644 --- a/arch/x86/kvm/iommu.c +++ b/arch/x86/kvm/iommu.c @@ -200,6 +200,7 @@ int kvm_assign_device(struct kvm *kvm, struct pci_dev *pdev) goto out_unmap; } + kvm_arch_start_assignment(kvm); pci_set_dev_assigned(pdev); dev_info(&pdev->dev, "kvm assign device\n"); @@ -224,6 +225,7 @@ int kvm_deassign_device(struct kvm *kvm, struct pci_dev *pdev) iommu_detach_device(domain, &pdev->dev); pci_clear_dev_assigned(pdev); + kvm_arch_end_assignment(kvm); dev_info(&pdev->dev, "kvm deassign device\n"); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index bbaf44e8f0d3..6e455875d2e4 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -8218,6 +8218,24 @@ bool kvm_arch_can_inject_async_page_present(struct kvm_vcpu *vcpu) kvm_x86_ops->interrupt_allowed(vcpu); } +void kvm_arch_start_assignment(struct kvm *kvm) +{ + atomic_inc(&kvm->arch.assigned_device_count); +} +EXPORT_SYMBOL_GPL(kvm_arch_start_assignment); + +void kvm_arch_end_assignment(struct kvm *kvm) +{ + atomic_dec(&kvm->arch.assigned_device_count); +} +EXPORT_SYMBOL_GPL(kvm_arch_end_assignment); + +bool kvm_arch_has_assigned_device(struct kvm *kvm) +{ + return atomic_read(&kvm->arch.assigned_device_count); +} +EXPORT_SYMBOL_GPL(kvm_arch_has_assigned_device); + void kvm_arch_register_noncoherent_dma(struct kvm *kvm) { atomic_inc(&kvm->arch.noncoherent_dma_count); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 9564fd78c547..56146fdb1ff0 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -716,10 +716,13 @@ static inline void kvm_arch_free_vm(struct kvm *kvm) } #endif -#ifdef __KVM_HAVE_ARCH_NONCOHERENT_DMA +#ifdef __KVM_HAVE_ARCH_VFIO_HOOKS void kvm_arch_register_noncoherent_dma(struct kvm *kvm); void kvm_arch_unregister_noncoherent_dma(struct kvm *kvm); bool kvm_arch_has_noncoherent_dma(struct kvm *kvm); +void kvm_arch_start_assignment(struct kvm *kvm); +void kvm_arch_end_assignment(struct kvm *kvm); +bool kvm_arch_has_assigned_device(struct kvm *kvm); #else static inline void kvm_arch_register_noncoherent_dma(struct kvm *kvm) { @@ -733,6 +736,19 @@ static inline bool kvm_arch_has_noncoherent_dma(struct kvm *kvm) { return false; } + +static inline void kvm_arch_start_assignment(struct kvm *kvm) +{ +} + +static inline void kvm_arch_end_assignment(struct kvm *kvm) +{ +} + +static inline bool kvm_arch_has_assigned_device(struct kvm *kvm) +{ + return false; +} #endif static inline wait_queue_head_t *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu) diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c index 620e37f741b8..1dd087da6f31 100644 --- a/virt/kvm/vfio.c +++ b/virt/kvm/vfio.c @@ -155,6 +155,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) list_add_tail(&kvg->node, &kv->group_list); kvg->vfio_group = vfio_group; + kvm_arch_start_assignment(dev->kvm); + mutex_unlock(&kv->lock); kvm_vfio_update_coherency(dev); @@ -190,6 +192,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) break; } + kvm_arch_end_assignment(dev->kvm); + mutex_unlock(&kv->lock); kvm_vfio_group_put_external_user(vfio_group); @@ -239,6 +243,7 @@ static void kvm_vfio_destroy(struct kvm_device *dev) kvm_vfio_group_put_external_user(kvg->vfio_group); list_del(&kvg->node); kfree(kvg); + kvm_arch_end_assignment(dev->kvm); } kvm_vfio_update_coherency(dev);
If there are no assigned devices, the guest PAT are not providing any useful information and can be overridden to writeback; VMX always does this because it has the "IPAT" bit in its extended page table entries, but SVM does not have anything similar. Hook into VFIO and legacy device assignment so that they provide this information to KVM. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/include/asm/kvm_host.h | 3 ++- arch/x86/kvm/iommu.c | 2 ++ arch/x86/kvm/x86.c | 18 ++++++++++++++++++ include/linux/kvm_host.h | 18 +++++++++++++++++- virt/kvm/vfio.c | 5 +++++ 5 files changed, 44 insertions(+), 2 deletions(-)