Skip to contentSkip to conversation

Fix kvm misconceives NVDIMM pages as reserved mmio

PATCH Superseded Zhang Yi 3 participants 11 messages Aug 22, 2018 – Sep 19, 2018

Subsystems (6)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Zhang Yi

author
#0 lore raw

v4 cover Fix kvm misconceives NVDIMM pages as reserved mmio

For device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.

Now, we map these dev_dax or fs_dax pages to kvm for DIMM/NVDIMM backend, since these pages are reserved. the check of kvm_is_reserved_pfn() misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to indentify these pages are from NVDIMM pmem. and let kvm treat these as normal pages.

Without this patch, Many operations will be missed due to this mistreatment to pmem pages. For example, a page may not have chance to be unpinned for KVM guest(in kvm_release_pfn_clean); not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.

V1:
https://lkml.org/lkml/2018/7/4/91

V2:
https://lkml.org/lkml/2018/7/10/135

V3:
https://lkml.org/lkml/2018/8/9/17

V4:
[PATCH V3 1/4] Added "Reviewed-by: David / Acked-by: Pankaj" [PATCH V3 2/4] Added "Reviewed-by: Jan"
[PATCH V3 3/4] Added "Acked-by: Jan"
[PATCH V3 4/4] Fix several typos

Zhang Yi (4):
kvm: remove redundant reserved page check
mm: introduce memory type MEMORY_DEVICE_DEV_DAX
mm: add a function to differentiate the pages is from DAX device
memory
kvm: add a check if pfn is from NVDIMM pmem.

 drivers/dax/pmem.c       |  1 +
 include/linux/memremap.h |  8 ++++++++
 include/linux/mm.h       | 12 ++++++++++++
 virt/kvm/kvm_main.c      | 16 ++++++++--------
 4 files changed, 29 insertions(+), 8 deletions(-)
Signature · 6 lines
2.7.4

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
author
#1 lore raw

PATCH v4 1/4 kvm: remove redundant reserved page check

PageReserved() is already checked inside kvm_is_reserved_pfn(), remove it from kvm_set_pfn_dirty().

---
 virt/kvm/kvm_main.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
virt/kvm/kvm_main.c +2 -6
index 8b47507f..c44c406 100644
@@ -1690,12 +1690,8 @@ EXPORT_SYMBOL_GPL(kvm_release_pfn_dirty);
 
 void kvm_set_pfn_dirty(kvm_pfn_t pfn)
 {
-	if (!kvm_is_reserved_pfn(pfn)) {
-		struct page *page = pfn_to_page(pfn);
-
-		if (!PageReserved(page))
-			SetPageDirty(page);
-	}
+	if (!kvm_is_reserved_pfn(pfn))
+		SetPageDirty(pfn_to_page(pfn));
 }
 EXPORT_SYMBOL_GPL(kvm_set_pfn_dirty);
 
Signature · 6 lines
2.7.4

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
author
#2 lore raw

PATCH v4 2/4 mm: introduce memory type MEMORY_DEVICE_DEV_DAX

In reply to Zhang Yi

Currently, NVDIMM pages will be marked 'PageReserved'. However, unlike other reserved PFNs, pages on NVDIMM shall still behave like normal ones in many cases, i.e. when used as backend memory of KVM guest. This patch introduces a new memory type, MEMORY_DEVICE_DEV_DAX. And set this flag while dax driver hotplug the device memory.

---
 drivers/dax/pmem.c       | 1 +
 include/linux/memremap.h | 8 ++++++++
 2 files changed, 9 insertions(+)
drivers/dax/pmem.c +1 -0
index fd49b24..fb3f363 100644
@@ -111,6 +111,7 @@ static int dax_pmem_probe(struct device *dev)
 		return rc;
 
 	dax_pmem->pgmap.ref = &dax_pmem->ref;
+	dax_pmem->pgmap.type = MEMORY_DEVICE_DEV_DAX;
 	addr = devm_memremap_pages(dev, &dax_pmem->pgmap);
 	if (IS_ERR(addr))
 		return PTR_ERR(addr);
include/linux/memremap.h +8 -0
index f91f9e7..cd07ca8 100644
@@ -53,11 +53,19 @@ struct vmem_altmap {
  * wakeup event whenever a page is unpinned and becomes idle. This
  * wakeup is used to coordinate physical address space management (ex:
  * fs truncate/hole punch) vs pinned pages (ex: device dma).
+ *
+ * MEMORY_DEVICE_DEV_DAX:
+ * Device memory that support raw access to persistent memory. Without need
+ * of an intervening filesystem, it could be directed mapped via an mmap
+ * capable character device. Together with the type MEMORY_DEVICE_FS_DAX,
+ * we could distinguish the persistent memory pages from normal ZONE_DEVICE
+ * pages.
  */
 enum memory_type {
 	MEMORY_DEVICE_PRIVATE = 1,
 	MEMORY_DEVICE_PUBLIC,
 	MEMORY_DEVICE_FS_DAX,
+	MEMORY_DEVICE_DEV_DAX,
 };
 
 /*
Signature · 6 lines
2.7.4

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Reviewed-by on v4 2/4
#3 lore raw

Re: [PATCH V4 2/4] mm: introduce memory type MEMORY_DEVICE_DEV_DAX

Quoted 43 lines

Currently, NVDIMM pages will be marked 'PageReserved'. However, unlike other reserved PFNs, pages on NVDIMM shall still behave like normal ones in many cases, i.e. when used as backend memory of KVM guest. This patch introduces a new memory type, MEMORY_DEVICE_DEV_DAX. And set this flag while dax driver hotplug the device memory.

---
 drivers/dax/pmem.c       | 1 +
 include/linux/memremap.h | 8 ++++++++
 2 files changed, 9 insertions(+)
drivers/dax/pmem.c +1 -0
index fd49b24..fb3f363 100644
@@ -111,6 +111,7 @@ static int dax_pmem_probe(struct device *dev)
                 return rc;
 
         dax_pmem->pgmap.ref = &dax_pmem->ref;
+        dax_pmem->pgmap.type = MEMORY_DEVICE_DEV_DAX;
         addr = devm_memremap_pages(dev, &dax_pmem->pgmap);
         if (IS_ERR(addr))
                 return PTR_ERR(addr);
include/linux/memremap.h +7 -0
index f91f9e7..cd07ca8 100644
@@ -53,11 +53,19 @@ struct vmem_altmap {
  * wakeup event whenever a page is unpinned and becomes idle. This
  * wakeup is used to coordinate physical address space management (ex:
  * fs truncate/hole punch) vs pinned pages (ex: device dma).
+ *
+ * MEMORY_DEVICE_DEV_DAX:
+ * Device memory that support raw access to persistent memory. Without need
+ * of an intervening filesystem, it could be directed mapped via an mmap
+ * capable character device. Together with the type MEMORY_DEVICE_FS_DAX,
+ * we could distinguish the persistent memory pages from normal ZONE_DEVICE
+ * pages.

include/linux/memremap.h struct vmem_altmap {

Quoted 10 lines
include/linux/memremap.h +1 -1
  */
 enum memory_type {
         MEMORY_DEVICE_PRIVATE = 1,
         MEMORY_DEVICE_PUBLIC,
         MEMORY_DEVICE_FS_DAX,
+        MEMORY_DEVICE_DEV_DAX,
 };
 
 /*
--
Quoted 3 lines

2.7.4

author
#4 lore raw

PATCH v4 3/4 mm: add a function to differentiate the pages is from DAX device memory

In reply to Zhang Yi

DAX driver hotplug the device memory and move it to memory zone, these pages will be marked reserved flag, however, some other kernel componet will misconceive these pages are reserved mmio (ex: we map these dev_dax or fs_dax pages to kvm for DIMM/NVDIMM backend). Together with the type MEMORY_DEVICE_FS_DAX, we can use is_dax_page() to differentiate the pages is DAX device memory or not.

---
 include/linux/mm.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)
include/linux/mm.h +12 -0
index 68a5121..de5cbc3 100644
@@ -889,6 +889,13 @@ static inline bool is_device_public_page(const struct page *page)
 		page->pgmap->type == MEMORY_DEVICE_PUBLIC;
 }
 
+static inline bool is_dax_page(const struct page *page)
+{
+	return is_zone_device_page(page) &&
+		(page->pgmap->type == MEMORY_DEVICE_FS_DAX ||
+		page->pgmap->type == MEMORY_DEVICE_DEV_DAX);
+}
+
 #else /* CONFIG_DEV_PAGEMAP_OPS */
 static inline void dev_pagemap_get_ops(void)
 {
@@ -912,6 +919,11 @@ static inline bool is_device_public_page(const struct page *page)
 {
 	return false;
 }
+
+static inline bool is_dax_page(const struct page *page)
+{
+	return false;
+}
 #endif /* CONFIG_DEV_PAGEMAP_OPS */
 
 static inline void get_page(struct page *page)
Signature · 6 lines
2.7.4

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Reviewed-by on v4 3/4
#5 lore raw

Re: [PATCH V4 3/4] mm: add a function to differentiate the pages is from DAX device memory

Quoted 47 lines

DAX driver hotplug the device memory and move it to memory zone, these pages will be marked reserved flag, however, some other kernel componet will misconceive these pages are reserved mmio (ex: we map these dev_dax or fs_dax pages to kvm for DIMM/NVDIMM backend). Together with the type MEMORY_DEVICE_FS_DAX, we can use is_dax_page() to differentiate the pages is DAX device memory or not.

---
 include/linux/mm.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)
include/linux/mm.h +7 -0
index 68a5121..de5cbc3 100644
@@ -889,6 +889,13 @@ static inline bool is_device_public_page(const struct
 page *page)
 		page->pgmap->type == MEMORY_DEVICE_PUBLIC;
 }
 
+static inline bool is_dax_page(const struct page *page)
+{
+	return is_zone_device_page(page) &&
+		(page->pgmap->type == MEMORY_DEVICE_FS_DAX ||
+		page->pgmap->type == MEMORY_DEVICE_DEV_DAX);
+}
+
 #else /* CONFIG_DEV_PAGEMAP_OPS */
 static inline void dev_pagemap_get_ops(void)

{

include/linux/mm.h +5 -0
@@ -912,6 +919,11 @@ static inline bool is_device_public_page(const struct
 page *page)
 {
 	return false;
 }
+
+static inline bool is_dax_page(const struct page *page)
+{
+	return false;
+}
 #endif /* CONFIG_DEV_PAGEMAP_OPS */
 
Quoted 2 lines

static inline void get_page(struct page *page)
--

Quoted 3 lines

2.7.4

author
#6 lore raw

PATCH v4 4/4 kvm: add a check if pfn is from NVDIMM pmem.

In reply to Zhang Yi

For device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.

Now, we map these dev_dax or fs_dax pages to kvm for DIMM/NVDIMM backend, since these pages are reserved, the check of kvm_is_reserved_pfn() misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to identify these pages are from NVDIMM pmem and let kvm treat these as normal pages.

Without this patch, many operations will be missed due to this mistreatment to pmem pages, for example, a page may not have chance to be unpinned for KVM guest(in kvm_release_pfn_clean), not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.

---
 virt/kvm/kvm_main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
virt/kvm/kvm_main.c +6 -2
index c44c406..969b6ca 100644
@@ -147,8 +147,12 @@ __weak void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,
 
 bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
 {
-	if (pfn_valid(pfn))
-		return PageReserved(pfn_to_page(pfn));
+	struct page *page;
+
+	if (pfn_valid(pfn)) {
+		page = pfn_to_page(pfn);
+		return PageReserved(page) && !is_dax_page(page);
+	}
 
 	return true;
 }
Signature · 6 lines
2.7.4

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
Acked-by on v4 4/4
#7 lore raw

Re: [PATCH V4 4/4] kvm: add a check if pfn is from NVDIMM pmem.

Quoted 43 lines

For device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.

Now, we map these dev_dax or fs_dax pages to kvm for DIMM/NVDIMM backend, since these pages are reserved, the check of kvm_is_reserved_pfn() misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to identify these pages are from NVDIMM pmem and let kvm treat these as normal pages.

Without this patch, many operations will be missed due to this mistreatment to pmem pages, for example, a page may not have chance to be unpinned for KVM guest(in kvm_release_pfn_clean), not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.

---
 virt/kvm/kvm_main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
virt/kvm/kvm_main.c +6 -2
index c44c406..969b6ca 100644
@@ -147,8 +147,12 @@ __weak void
 kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,
 
 bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
 {
-	if (pfn_valid(pfn))
-		return PageReserved(pfn_to_page(pfn));
+	struct page *page;
+
+	if (pfn_valid(pfn)) {
+		page = pfn_to_page(pfn);
+		return PageReserved(page) && !is_dax_page(page);
+	}
 
 	return true;
Quoted 1 line

}

Quoted 4 lines

--
2.7.4

author
#8 lore raw

Re: [PATCH V4 4/4] kvm: add a check if pfn is from NVDIMM pmem.

Quoted 42 lines · Pankaj Gupta
Quoted 42 lines

For device specific memory space, when we move these area of pfn to memory zone, we will set the page reserved flag at that time, some of these reserved for device mmio, and some of these are not, such as NVDIMM pmem.

Now, we map these dev_dax or fs_dax pages to kvm for DIMM/NVDIMM backend, since these pages are reserved, the check of kvm_is_reserved_pfn() misconceives those pages as MMIO. Therefor, we introduce 2 page map types, MEMORY_DEVICE_FS_DAX/MEMORY_DEVICE_DEV_DAX, to identify these pages are from NVDIMM pmem and let kvm treat these as normal pages.

Without this patch, many operations will be missed due to this mistreatment to pmem pages, for example, a page may not have chance to be unpinned for KVM guest(in kvm_release_pfn_clean), not able to be marked as dirty/accessed(in kvm_set_pfn_dirty/accessed) etc.

---
 virt/kvm/kvm_main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
virt/kvm/kvm_main.c +6 -2
index c44c406..969b6ca 100644
@@ -147,8 +147,12 @@ __weak void
 kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,
 
 bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
 {
-	if (pfn_valid(pfn))
-		return PageReserved(pfn_to_page(pfn));
+	struct page *page;
+
+	if (pfn_valid(pfn)) {
+		page = pfn_to_page(pfn);
+		return PageReserved(page) && !is_dax_page(page);
+	}
 
Quoted 4 lines
Quoted 2 lines
virt/kvm/kvm_main.c +0 -0
 	return true;

}

Thanks for your kindly review, Pankaj, as all the patch [1,2,3,4]/4 got the reviewed[acked]-by, can we Queue this by now?

Quoted 5 lines
Quoted 5 lines

--
2.7.4

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

#9 lore raw

Re: [PATCH V4 4/4] kvm: add a check if pfn is from NVDIMM pmem.

In reply to Zhang Yi

virt/kvm/kvm_main.c __weak void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm, · line 155

Quoted 10 lines · Zhang Yi

bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
{

virt/kvm/kvm_main.c +6 -2
-	if (pfn_valid(pfn))
-		return PageReserved(pfn_to_page(pfn));
+	struct page *page;
+
+	if (pfn_valid(pfn)) {
+		page = pfn_to_page(pfn);
+		return PageReserved(page) && !is_dax_page(page);
+	}

This is in desperate need of commenting about what it is doing and why.

The changelog alone doesn't cut it.

author
#10 lore raw

Re: [PATCH V4 4/4] kvm: add a check if pfn is from NVDIMM pmem.

Quoted 14 lines · Dave Hansen

virt/kvm/kvm_main.c __weak void kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm, · line 155

Quoted 10 lines · Zhang Yi

bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
{

virt/kvm/kvm_main.c +6 -2
-	if (pfn_valid(pfn))
-		return PageReserved(pfn_to_page(pfn));
+	struct page *page;
+
+	if (pfn_valid(pfn)) {
+		page = pfn_to_page(pfn);
+		return PageReserved(page) && !is_dax_page(page);
+	}

This is in desperate need of commenting about what it is doing and why.

Quoted 1 line

The changelog alone doesn't cut it.

Thanks, Dave, Will add some comments
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm