| Message ID | 1382579799-13273-1-git-send-email-yang.z.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Il 24/10/2013 03:56, Yang Zhang ha scritto: > From: Yang Zhang <yang.z.zhang@Intel.com> > > In kvm_iommu_map_pages(), we need to know the page size via call > kvm_host_page_size(). And it will check whether the target slot > is valid before return the right page size. > Currently, we will map the iommu pages when creating a new slot. > But we call kvm_iommu_map_pages() during preparing the new slot. > At that time, the new slot is not visible by domain(still in preparing). > So we cannot get the right page size from kvm_host_page_size() and > this will break the IOMMU super page logic. > The solution is to map the iommu pages after we insert the new slot > into domain. > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > Tested-by: Patrick Lu <patrick.lu@intel.com> > --- > virt/kvm/kvm_main.c | 29 ++++++++++++++--------------- > 1 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index d469114..9ec60a2 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -873,21 +873,6 @@ int __kvm_set_memory_region(struct kvm *kvm, > goto out_free; > } > > - /* > - * IOMMU mapping: New slots need to be mapped. Old slots need to be > - * un-mapped and re-mapped if their base changes. Since base change > - * unmapping is handled above with slot deletion, mapping alone is > - * needed here. Anything else the iommu might care about for existing > - * slots (size changes, userspace addr changes and read-only flag > - * changes) is disallowed above, so any other attribute changes getting > - * here can be skipped. > - */ > - if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) { > - r = kvm_iommu_map_pages(kvm, &new); > - if (r) > - goto out_slots; > - } > - > /* actual memory is freed via old in kvm_free_physmem_slot below */ > if (change == KVM_MR_DELETE) { > new.dirty_bitmap = NULL; > @@ -901,6 +886,20 @@ int __kvm_set_memory_region(struct kvm *kvm, > kvm_free_physmem_slot(&old, &new); > kfree(old_memslots); > > + /* > + * IOMMU mapping: New slots need to be mapped. Old slots need to be > + * un-mapped and re-mapped if their base changes. Since base change > + * unmapping is handled above with slot deletion, mapping alone is > + * needed here. Anything else the iommu might care about for existing > + * slots (size changes, userspace addr changes and read-only flag > + * changes) is disallowed above, so any other attribute changes getting > + * here can be skipped. > + */ > + if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) { > + r = kvm_iommu_map_pages(kvm, &new); > + return r; > + } > + > return 0; > > out_slots: > Applied to kvm.git queue, thanks. 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
On Thu, 2013-10-24 at 09:56 +0800, Yang Zhang wrote: > From: Yang Zhang <yang.z.zhang@Intel.com> > > In kvm_iommu_map_pages(), we need to know the page size via call > kvm_host_page_size(). And it will check whether the target slot > is valid before return the right page size. > Currently, we will map the iommu pages when creating a new slot. > But we call kvm_iommu_map_pages() during preparing the new slot. > At that time, the new slot is not visible by domain(still in preparing). > So we cannot get the right page size from kvm_host_page_size() and > this will break the IOMMU super page logic. > The solution is to map the iommu pages after we insert the new slot > into domain. > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > Tested-by: Patrick Lu <patrick.lu@intel.com> > --- > virt/kvm/kvm_main.c | 29 ++++++++++++++--------------- > 1 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index d469114..9ec60a2 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -873,21 +873,6 @@ int __kvm_set_memory_region(struct kvm *kvm, > goto out_free; > } > > - /* > - * IOMMU mapping: New slots need to be mapped. Old slots need to be > - * un-mapped and re-mapped if their base changes. Since base change > - * unmapping is handled above with slot deletion, mapping alone is > - * needed here. Anything else the iommu might care about for existing > - * slots (size changes, userspace addr changes and read-only flag > - * changes) is disallowed above, so any other attribute changes getting > - * here can be skipped. > - */ > - if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) { > - r = kvm_iommu_map_pages(kvm, &new); > - if (r) > - goto out_slots; > - } > - > /* actual memory is freed via old in kvm_free_physmem_slot below */ > if (change == KVM_MR_DELETE) { > new.dirty_bitmap = NULL; > @@ -901,6 +886,20 @@ int __kvm_set_memory_region(struct kvm *kvm, > kvm_free_physmem_slot(&old, &new); > kfree(old_memslots); > > + /* > + * IOMMU mapping: New slots need to be mapped. Old slots need to be > + * un-mapped and re-mapped if their base changes. Since base change > + * unmapping is handled above with slot deletion, mapping alone is > + * needed here. Anything else the iommu might care about for existing > + * slots (size changes, userspace addr changes and read-only flag > + * changes) is disallowed above, so any other attribute changes getting > + * here can be skipped. > + */ > + if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) { > + r = kvm_iommu_map_pages(kvm, &new); > + return r; > + } > + An addition to the comment noting that this needs to be done after install/commit would be helpful. Thanks, Alex > return 0; > > out_slots: -- 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/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index d469114..9ec60a2 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -873,21 +873,6 @@ int __kvm_set_memory_region(struct kvm *kvm, goto out_free; } - /* - * IOMMU mapping: New slots need to be mapped. Old slots need to be - * un-mapped and re-mapped if their base changes. Since base change - * unmapping is handled above with slot deletion, mapping alone is - * needed here. Anything else the iommu might care about for existing - * slots (size changes, userspace addr changes and read-only flag - * changes) is disallowed above, so any other attribute changes getting - * here can be skipped. - */ - if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) { - r = kvm_iommu_map_pages(kvm, &new); - if (r) - goto out_slots; - } - /* actual memory is freed via old in kvm_free_physmem_slot below */ if (change == KVM_MR_DELETE) { new.dirty_bitmap = NULL; @@ -901,6 +886,20 @@ int __kvm_set_memory_region(struct kvm *kvm, kvm_free_physmem_slot(&old, &new); kfree(old_memslots); + /* + * IOMMU mapping: New slots need to be mapped. Old slots need to be + * un-mapped and re-mapped if their base changes. Since base change + * unmapping is handled above with slot deletion, mapping alone is + * needed here. Anything else the iommu might care about for existing + * slots (size changes, userspace addr changes and read-only flag + * changes) is disallowed above, so any other attribute changes getting + * here can be skipped. + */ + if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) { + r = kvm_iommu_map_pages(kvm, &new); + return r; + } + return 0; out_slots: