Skip to contentSkip to conversation

KVM: unmap SMM memslots in vt-d table

PATCH New herongguang 3 participants 7 messages Mar 25, 2017 – Mar 28, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 KVM: unmap SMM memslots in vt-d table

or pages are not unmaped and freed

---
 arch/x86/kvm/iommu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Well, do we should change pci-assign to not map SMM slots instead? Like vfio.

arch/x86/kvm/iommu.c +4 -2
index b181426..5b931bb 100644
@@ -320,15 +320,17 @@ void kvm_iommu_unmap_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 static int kvm_iommu_unmap_memslots(struct kvm *kvm)
 {
 	int idx;
-	struct kvm_memslots *slots;
+	struct kvm_memslots *slots, *smm_slots;
 	struct kvm_memory_slot *memslot;
 
 	idx = srcu_read_lock(&kvm->srcu);
 	slots = kvm_memslots(kvm);
-
 	kvm_for_each_memslot(memslot, slots)
 		kvm_iommu_unmap_pages(kvm, memslot);
 
+	smm_slots = __kvm_memslots(kvm, 1);
+	kvm_for_each_memslot(memslot, smm_slots)
+		kvm_iommu_unmap_pages(kvm, memslot);
 	srcu_read_unlock(&kvm->srcu, idx);
 
 	if (kvm->arch.iommu_noncoherent)
Signature · 1 line
1.7.12.4
#1 lore raw

PATCH v2 1/1 KVM: pci-assign: do not map smm memory slot pages

From f6f0ee6831488bef7af841cb86f3d85a04848fe5 Mon Sep 17 00:00:00 2001
From: herongguang <[email protected]>
Date: Mon, 27 Mar 2017 15:08:59 +0800
Subject: [PATCH] KVM: pci-assign: do not map smm memory slot pages
in vt-d page table

or VM memory are not put thus leaked in kvm_iommu_unmap_memslots() when destroy VM.

This is consistent with current vfio implementation.

---
 virt/kvm/kvm_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/kvm_main.c +1 -1
index 482612b..9018d06 100644
@@ -1052,7 +1052,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	 * changes) is disallowed above, so any other attribute changes getting
 	 * here can be skipped.
 	 */
-	if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) {
+	if ((as_id == 0) && ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE))) {
 		r = kvm_iommu_map_pages(kvm, &new);
 		return r;
 	}
Signature · 1 line
1.7.12.4
Quoted 34 lines · herongguang

or pages are not unmaped and freed

---
  arch/x86/kvm/iommu.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

Well, do we should change pci-assign to not map SMM slots instead? Like vfio.

arch/x86/kvm/iommu.c +4 -2
index b181426..5b931bb 100644
@@ -320,15 +320,17 @@ void kvm_iommu_unmap_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
  static int kvm_iommu_unmap_memslots(struct kvm *kvm)
  {
  	int idx;
-	struct kvm_memslots *slots;
+	struct kvm_memslots *slots, *smm_slots;
  	struct kvm_memory_slot *memslot;
 
  	idx = srcu_read_lock(&kvm->srcu);
  	slots = kvm_memslots(kvm);
-
  	kvm_for_each_memslot(memslot, slots)
  		kvm_iommu_unmap_pages(kvm, memslot);
 
+	smm_slots = __kvm_memslots(kvm, 1);
+	kvm_for_each_memslot(memslot, smm_slots)
+		kvm_iommu_unmap_pages(kvm, memslot);
  	srcu_read_unlock(&kvm->srcu, idx);
 
  	if (kvm->arch.iommu_noncoherent)
maintainer Applied
#2 lore raw

Re: [PATCH v2] KVM: pci-assign: do not map smm memory slot pages

Quoted 25 lines · Herongguang (Stephen)

From f6f0ee6831488bef7af841cb86f3d85a04848fe5 Mon Sep 17 00:00:00 2001
From: herongguang <[email protected]>
Date: Mon, 27 Mar 2017 15:08:59 +0800
Subject: [PATCH] KVM: pci-assign: do not map smm memory slot pages
in vt-d page table

or VM memory are not put thus leaked in kvm_iommu_unmap_memslots() when destroy VM.

This is consistent with current vfio implementation.

---
 virt/kvm/kvm_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/kvm_main.c +0 -0
index 482612b..9018d06 100644
@@ -1052,7 +1052,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
      * changes) is disallowed above, so any other attribute changes

getting
* here can be skipped.
*/

virt/kvm/kvm_main.c +1 -1
-    if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) {
+    if ((as_id == 0) && ((change == KVM_MR_CREATE) || (change ==
Quoted 4 lines
KVM_MR_MOVE))) {
         r = kvm_iommu_map_pages(kvm, &new);
         return r;
     }

This makes more sense. :)

Applied to kvm/master, thanks.

Paolo

maintainer
#3 lore raw

Re: [PATCH v2] KVM: pci-assign: do not map smm memory slot pages

In reply to Herongguang (Stephen)
Quoted 10 lines · Herongguang (Stephen)

From f6f0ee6831488bef7af841cb86f3d85a04848fe5 Mon Sep 17 00:00:00 2001
From: herongguang <[email protected]>
Date: Mon, 27 Mar 2017 15:08:59 +0800
Subject: [PATCH] KVM: pci-assign: do not map smm memory slot pages
in vt-d page table

or VM memory are not put thus leaked in kvm_iommu_unmap_memslots() when destroy VM.

This is consistent with current vfio implementation.

Oops, you forgot a Signed-off-by.

Paolo

Quoted 19 lines
---
 virt/kvm/kvm_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/kvm_main.c +0 -0
index 482612b..9018d06 100644
@@ -1052,7 +1052,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
      * changes) is disallowed above, so any other attribute changes

getting
* here can be skipped.
*/

virt/kvm/kvm_main.c +1 -1
-    if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) {
+    if ((as_id == 0) && ((change == KVM_MR_CREATE) || (change ==
KVM_MR_MOVE))) {
         r = kvm_iommu_map_pages(kvm, &new);
         return r;
     }
#4 lore raw

Re: [Qemu-devel] [PATCH v2] KVM: pci-assign: do not map smm memory slot pages

Quoted 9 lines · Paolo Bonzini
Quoted 6 lines · Herongguang (Stephen)

From f6f0ee6831488bef7af841cb86f3d85a04848fe5 Mon Sep 17 00:00:00 2001
From: herongguang <[email protected]>
Date: Mon, 27 Mar 2017 15:08:59 +0800
Subject: [PATCH] KVM: pci-assign: do not map smm memory slot pages
in vt-d page table

Quoted 6 lines
Quoted 4 lines

or VM memory are not put thus leaked in kvm_iommu_unmap_memslots() when destroy VM.

This is consistent with current vfio implementation.

Oops, you forgot a Signed-off-by.

Sorry, I forget this, I'll resend it tomorrow since I am home now.

PS, what's your opinion about this
(http://www.spinics.net/lists/kvm/msg146914.html)?

Quoted 23 lines

Paolo

Quoted 19 lines
---
 virt/kvm/kvm_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/kvm_main.c +0 -0
index 482612b..9018d06 100644
@@ -1052,7 +1052,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
      * changes) is disallowed above, so any other attribute changes

getting
* here can be skipped.
*/

virt/kvm/kvm_main.c +1 -1
-    if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) {
+    if ((as_id == 0) && ((change == KVM_MR_CREATE) || (change ==
KVM_MR_MOVE))) {
         r = kvm_iommu_map_pages(kvm, &new);
         return r;
     }
#5 lore raw

Re: [PATCH v2] KVM: pci-assign: do not map smm memory slot pages

In reply to Paolo Bonzini

From 8f5b9d2c2944ea7cd8149e9d3b4088f487217d20 Mon Sep 17 00:00:00 2001
From: herongguang <[email protected]>
Date: Mon, 27 Mar 2017 15:08:59 +0800
Subject: [PATCH] KVM: pci-assign: do not map smm memory slot pages in vt-d
page table

or VM memory are not put thus leaked in kvm_iommu_unmap_memslots() when destroy VM.

This is consistent with current vfio implementation.

---
 virt/kvm/kvm_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/kvm_main.c +1 -1
index 482612b..9018d06 100644
@@ -1052,7 +1052,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	 * changes) is disallowed above, so any other attribute changes getting
 	 * here can be skipped.
 	 */
-	if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) {
+	if ((as_id == 0) && ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE))) {
 		r = kvm_iommu_map_pages(kvm, &new);
 		return r;
 	}
Signature · 1 line
1.7.12.4
Quoted 40 lines · Paolo Bonzini
Quoted 10 lines · Herongguang (Stephen)

From f6f0ee6831488bef7af841cb86f3d85a04848fe5 Mon Sep 17 00:00:00 2001
From: herongguang <[email protected]>
Date: Mon, 27 Mar 2017 15:08:59 +0800
Subject: [PATCH] KVM: pci-assign: do not map smm memory slot pages
in vt-d page table

or VM memory are not put thus leaked in kvm_iommu_unmap_memslots() when destroy VM.

This is consistent with current vfio implementation.

Oops, you forgot a Signed-off-by.

Paolo

Quoted 15 lines
---
  virt/kvm/kvm_main.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/kvm_main.c +0 -0
index 482612b..9018d06 100644
@@ -1052,7 +1052,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
       * changes) is disallowed above, so any other attribute changes

getting
* here can be skipped.
*/

virt/kvm/kvm_main.c +1 -1
-    if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) {
+    if ((as_id == 0) && ((change == KVM_MR_CREATE) || (change ==
Quoted 4 lines
KVM_MR_MOVE))) {
          r = kvm_iommu_map_pages(kvm, &new);
          return r;
      }

.

maintainer
#6 lore raw

Re: [PATCH v2] KVM: pci-assign: do not map smm memory slot pages

Quoted 27 lines · Herongguang (Stephen)

From 8f5b9d2c2944ea7cd8149e9d3b4088f487217d20 Mon Sep 17 00:00:00 2001
From: herongguang <[email protected]>
Date: Mon, 27 Mar 2017 15:08:59 +0800
Subject: [PATCH] KVM: pci-assign: do not map smm memory slot pages in vt-d
page table

or VM memory are not put thus leaked in kvm_iommu_unmap_memslots() when destroy VM.

This is consistent with current vfio implementation.

---
 virt/kvm/kvm_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/kvm_main.c +0 -0
index 482612b..9018d06 100644
@@ -1052,7 +1052,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
      * changes) is disallowed above, so any other attribute changes

getting
* here can be skipped.
*/

virt/kvm/kvm_main.c +1 -1
-    if ((change == KVM_MR_CREATE) || (change == KVM_MR_MOVE)) {
+    if ((as_id == 0) && ((change == KVM_MR_CREATE) || (change ==
Quoted 4 lines
KVM_MR_MOVE))) {
         r = kvm_iommu_map_pages(kvm, &new);
         return r;
     }

Thanks!

Paolo