xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] xen/balloon: fixes for memory hotplug
@ 2020-08-11  9:44 Roger Pau Monne
  2020-08-11  9:44 ` [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC Roger Pau Monne
  2020-08-11  9:44 ` [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory Roger Pau Monne
  0 siblings, 2 replies; 21+ messages in thread
From: Roger Pau Monne @ 2020-08-11  9:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: xen-devel, Roger Pau Monne

Hello,

The following series contain some fixes in order to split Xen
unpopulated memory handling from the ballooning driver if ZONE_DEVICE is
available, so that physical memory regions used to map foreign pages are
not tied to memory hotplug.

The main difference in this version is that MEMORY_DEVICE_DEVDAX is
renamed to MEMORY_DEVICE_GENERIC, as using DEVDAX in the Xen code to
allocate unpopulated memory felt wrong.

Thanks, Roger.

Roger Pau Monne (2):
  memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC
  xen: add helpers to allocate unpopulated memory

 drivers/dax/device.c                    |   2 +-
 drivers/gpu/drm/xen/xen_drm_front_gem.c |   9 +-
 drivers/xen/Kconfig                     |   4 +
 drivers/xen/Makefile                    |   1 +
 drivers/xen/balloon.c                   |   4 +-
 drivers/xen/grant-table.c               |   4 +-
 drivers/xen/privcmd.c                   |   4 +-
 drivers/xen/unpopulated-alloc.c         | 185 ++++++++++++++++++++++++
 drivers/xen/xenbus/xenbus_client.c      |   6 +-
 drivers/xen/xlate_mmu.c                 |   4 +-
 include/linux/memremap.h                |   9 +-
 include/xen/xen.h                       |   9 ++
 mm/memremap.c                           |   2 +-
 13 files changed, 221 insertions(+), 22 deletions(-)
 create mode 100644 drivers/xen/unpopulated-alloc.c

-- 
2.28.0



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC
  2020-08-11  9:44 [PATCH v4 0/2] xen/balloon: fixes for memory hotplug Roger Pau Monne
@ 2020-08-11  9:44 ` Roger Pau Monne
  2020-08-11 21:07   ` David Hildenbrand
  2020-08-31 19:20   ` Andrew Morton
  2020-08-11  9:44 ` [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory Roger Pau Monne
  1 sibling, 2 replies; 21+ messages in thread
From: Roger Pau Monne @ 2020-08-11  9:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dave Jiang, linux-nvdimm, Vishal Verma, Johannes Thumshirn,
	linux-mm, Jason Gunthorpe, Logan Gunthorpe, Aneesh Kumar K.V,
	xen-devel, Dan Williams, Ira Weiny, Andrew Morton,
	Roger Pau Monne

This is in preparation for the logic behind MEMORY_DEVICE_DEVDAX also
being used by non DAX devices.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: linux-nvdimm@lists.01.org
Cc: xen-devel@lists.xenproject.org
Cc: linux-mm@kvack.org
---
 drivers/dax/device.c     | 2 +-
 include/linux/memremap.h | 9 ++++-----
 mm/memremap.c            | 2 +-
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index 4c0af2eb7e19..1e89513f3c59 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -429,7 +429,7 @@ int dev_dax_probe(struct device *dev)
 		return -EBUSY;
 	}
 
-	dev_dax->pgmap.type = MEMORY_DEVICE_DEVDAX;
+	dev_dax->pgmap.type = MEMORY_DEVICE_GENERIC;
 	addr = devm_memremap_pages(dev, &dev_dax->pgmap);
 	if (IS_ERR(addr))
 		return PTR_ERR(addr);
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 5f5b2df06e61..e5862746751b 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -46,11 +46,10 @@ struct vmem_altmap {
  * wakeup is used to coordinate physical address space management (ex:
  * fs truncate/hole punch) vs pinned pages (ex: device dma).
  *
- * MEMORY_DEVICE_DEVDAX:
+ * MEMORY_DEVICE_GENERIC:
  * Host memory that has similar access semantics as System RAM i.e. DMA
- * coherent and supports page pinning. In contrast to
- * MEMORY_DEVICE_FS_DAX, this memory is access via a device-dax
- * character device.
+ * coherent and supports page pinning. This is for example used by DAX devices
+ * that expose memory using a character device.
  *
  * MEMORY_DEVICE_PCI_P2PDMA:
  * Device memory residing in a PCI BAR intended for use with Peer-to-Peer
@@ -60,7 +59,7 @@ enum memory_type {
 	/* 0 is reserved to catch uninitialized type fields */
 	MEMORY_DEVICE_PRIVATE = 1,
 	MEMORY_DEVICE_FS_DAX,
-	MEMORY_DEVICE_DEVDAX,
+	MEMORY_DEVICE_GENERIC,
 	MEMORY_DEVICE_PCI_P2PDMA,
 };
 
diff --git a/mm/memremap.c b/mm/memremap.c
index 03e38b7a38f1..006dace60b1a 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -216,7 +216,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
 			return ERR_PTR(-EINVAL);
 		}
 		break;
-	case MEMORY_DEVICE_DEVDAX:
+	case MEMORY_DEVICE_GENERIC:
 		need_devmap_managed = false;
 		break;
 	case MEMORY_DEVICE_PCI_P2PDMA:
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-11  9:44 [PATCH v4 0/2] xen/balloon: fixes for memory hotplug Roger Pau Monne
  2020-08-11  9:44 ` [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC Roger Pau Monne
@ 2020-08-11  9:44 ` Roger Pau Monne
  2020-08-12  7:28   ` Jürgen Groß
  2020-08-13  7:33   ` Christoph Hellwig
  1 sibling, 2 replies; 21+ messages in thread
From: Roger Pau Monne @ 2020-08-11  9:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Juergen Gross, Stefano Stabellini, Wei Liu,
	Oleksandr Andrushchenko, David Airlie, Yan Yankovskyi,
	David Hildenbrand, dri-devel, Michal Hocko, linux-mm,
	Daniel Vetter, xen-devel, Boris Ostrovsky, Dan Williams,
	Dan Carpenter, Roger Pau Monne

To be used in order to create foreign mappings. This is based on the
ZONE_DEVICE facility which is used by persistent memory devices in
order to create struct pages and kernel virtual mappings for the IOMEM
areas of such devices. Note that on kernels without support for
ZONE_DEVICE Xen will fallback to use ballooned pages in order to
create foreign mappings.

The newly added helpers use the same parameters as the existing
{alloc/free}_xenballooned_pages functions, which allows for in-place
replacement of the callers. Once a memory region has been added to be
used as scratch mapping space it will no longer be released, and pages
returned are kept in a linked list. This allows to have a buffer of
pages and prevents resorting to frequent additions and removals of
regions.

If enabled (because ZONE_DEVICE is supported) the usage of the new
functionality untangles Xen balloon and RAM hotplug from the usage of
unpopulated physical memory ranges to map foreign pages, which is the
correct thing to do in order to avoid mappings of foreign pages depend
on memory hotplug.

Note the driver is currently not enabled on Arm platforms because it
would interfere with the identity mapping required on some platforms.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Roger Pau Monne <roger.pau@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: Yan Yankovskyi <yyankovskyi@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: xen-devel@lists.xenproject.org
Cc: linux-mm@kvack.org
Cc: David Hildenbrand <david@redhat.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
---
Changes since v3:
 - Introduce a Kconfig option that gates the addition of the
   unpopulated alloc driver. This allows to easily disable it on Arm
   platforms.
 - Dropped Juergen RB due to the addition of the Kconfig option.
 - Switched from MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC.

Changes since v2:
 - Drop BUILD_BUG_ON regarding PVMMU page sizes.
 - Use a SPDX license identifier.
 - Call fill with only the minimum required number of pages.
 - Include xen.h header in xen_drm_front_gem.c.
 - Use less generic function names.
 - Exit early from the init function if not a PV guest.
 - Don't use all caps for region name.
---
 drivers/gpu/drm/xen/xen_drm_front_gem.c |   9 +-
 drivers/xen/Kconfig                     |   4 +
 drivers/xen/Makefile                    |   1 +
 drivers/xen/balloon.c                   |   4 +-
 drivers/xen/grant-table.c               |   4 +-
 drivers/xen/privcmd.c                   |   4 +-
 drivers/xen/unpopulated-alloc.c         | 185 ++++++++++++++++++++++++
 drivers/xen/xenbus/xenbus_client.c      |   6 +-
 drivers/xen/xlate_mmu.c                 |   4 +-
 include/xen/xen.h                       |   9 ++
 10 files changed, 215 insertions(+), 15 deletions(-)
 create mode 100644 drivers/xen/unpopulated-alloc.c

diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
index f0b85e094111..270e1bd3e4da 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -18,6 +18,7 @@
 #include <drm/drm_probe_helper.h>
 
 #include <xen/balloon.h>
+#include <xen/xen.h>
 
 #include "xen_drm_front.h"
 #include "xen_drm_front_gem.h"
@@ -99,8 +100,8 @@ static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)
 		 * allocate ballooned pages which will be used to map
 		 * grant references provided by the backend
 		 */
-		ret = alloc_xenballooned_pages(xen_obj->num_pages,
-					       xen_obj->pages);
+		ret = xen_alloc_unpopulated_pages(xen_obj->num_pages,
+					          xen_obj->pages);
 		if (ret < 0) {
 			DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
 				  xen_obj->num_pages, ret);
@@ -152,8 +153,8 @@ void xen_drm_front_gem_free_object_unlocked(struct drm_gem_object *gem_obj)
 	} else {
 		if (xen_obj->pages) {
 			if (xen_obj->be_alloc) {
-				free_xenballooned_pages(xen_obj->num_pages,
-							xen_obj->pages);
+				xen_free_unpopulated_pages(xen_obj->num_pages,
+							   xen_obj->pages);
 				gem_free_pages_array(xen_obj);
 			} else {
 				drm_gem_put_pages(&xen_obj->base,
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 1d339ef92422..018020b91baa 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -327,4 +327,8 @@ config XEN_HAVE_VPMU
 config XEN_FRONT_PGDIR_SHBUF
 	tristate
 
+config XEN_UNPOPULATED_ALLOC
+	bool
+	default y if ZONE_DEVICE && !ARM && !ARM64
+
 endmenu
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index c25c9a699b48..babdca808861 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -41,3 +41,4 @@ xen-gntdev-$(CONFIG_XEN_GNTDEV_DMABUF)	+= gntdev-dmabuf.o
 xen-gntalloc-y				:= gntalloc.o
 xen-privcmd-y				:= privcmd.o privcmd-buf.o
 obj-$(CONFIG_XEN_FRONT_PGDIR_SHBUF)	+= xen-front-pgdir-shbuf.o
+obj-$(CONFIG_XEN_UNPOPULATED_ALLOC)	+= unpopulated-alloc.o
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 37ffccda8bb8..51427c752b37 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -653,7 +653,7 @@ void free_xenballooned_pages(int nr_pages, struct page **pages)
 }
 EXPORT_SYMBOL(free_xenballooned_pages);
 
-#ifdef CONFIG_XEN_PV
+#if defined(CONFIG_XEN_PV) && !defined(CONFIG_XEN_UNPOPULATED_ALLOC)
 static void __init balloon_add_region(unsigned long start_pfn,
 				      unsigned long pages)
 {
@@ -707,7 +707,7 @@ static int __init balloon_init(void)
 	register_sysctl_table(xen_root);
 #endif
 
-#ifdef CONFIG_XEN_PV
+#if defined(CONFIG_XEN_PV) && !defined(CONFIG_XEN_UNPOPULATED_ALLOC)
 	{
 		int i;
 
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 8d06bf1cc347..523dcdf39cc9 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -801,7 +801,7 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
 {
 	int ret;
 
-	ret = alloc_xenballooned_pages(nr_pages, pages);
+	ret = xen_alloc_unpopulated_pages(nr_pages, pages);
 	if (ret < 0)
 		return ret;
 
@@ -836,7 +836,7 @@ EXPORT_SYMBOL_GPL(gnttab_pages_clear_private);
 void gnttab_free_pages(int nr_pages, struct page **pages)
 {
 	gnttab_pages_clear_private(nr_pages, pages);
-	free_xenballooned_pages(nr_pages, pages);
+	xen_free_unpopulated_pages(nr_pages, pages);
 }
 EXPORT_SYMBOL_GPL(gnttab_free_pages);
 
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 63abe6c3642b..b0c73c58f987 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -424,7 +424,7 @@ static int alloc_empty_pages(struct vm_area_struct *vma, int numpgs)
 	if (pages == NULL)
 		return -ENOMEM;
 
-	rc = alloc_xenballooned_pages(numpgs, pages);
+	rc = xen_alloc_unpopulated_pages(numpgs, pages);
 	if (rc != 0) {
 		pr_warn("%s Could not alloc %d pfns rc:%d\n", __func__,
 			numpgs, rc);
@@ -895,7 +895,7 @@ static void privcmd_close(struct vm_area_struct *vma)
 
 	rc = xen_unmap_domain_gfn_range(vma, numgfns, pages);
 	if (rc == 0)
-		free_xenballooned_pages(numpgs, pages);
+		xen_free_unpopulated_pages(numpgs, pages);
 	else
 		pr_crit("unable to unmap MFN range: leaking %d pages. rc=%d\n",
 			numpgs, rc);
diff --git a/drivers/xen/unpopulated-alloc.c b/drivers/xen/unpopulated-alloc.c
new file mode 100644
index 000000000000..1b5d157c6977
--- /dev/null
+++ b/drivers/xen/unpopulated-alloc.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/errno.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/memremap.h>
+#include <linux/slab.h>
+
+#include <asm/page.h>
+
+#include <xen/page.h>
+#include <xen/xen.h>
+
+static DEFINE_MUTEX(list_lock);
+static LIST_HEAD(page_list);
+static unsigned int list_count;
+
+static int fill_list(unsigned int nr_pages)
+{
+	struct dev_pagemap *pgmap;
+	void *vaddr;
+	unsigned int i, alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
+	int nid, ret;
+
+	pgmap = kzalloc(sizeof(*pgmap), GFP_KERNEL);
+	if (!pgmap)
+		return -ENOMEM;
+
+	pgmap->type = MEMORY_DEVICE_GENERIC;
+	pgmap->res.name = "Xen scratch";
+	pgmap->res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+
+	ret = allocate_resource(&iomem_resource, &pgmap->res,
+				alloc_pages * PAGE_SIZE, 0, -1,
+				PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
+	if (ret < 0) {
+		pr_err("Cannot allocate new IOMEM resource\n");
+		kfree(pgmap);
+		return ret;
+	}
+
+	nid = memory_add_physaddr_to_nid(pgmap->res.start);
+
+#ifdef CONFIG_XEN_HAVE_PVMMU
+        /*
+         * memremap will build page tables for the new memory so
+         * the p2m must contain invalid entries so the correct
+         * non-present PTEs will be written.
+         *
+         * If a failure occurs, the original (identity) p2m entries
+         * are not restored since this region is now known not to
+         * conflict with any devices.
+         */
+	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+		xen_pfn_t pfn = PFN_DOWN(pgmap->res.start);
+
+		for (i = 0; i < alloc_pages; i++) {
+			if (!set_phys_to_machine(pfn + i, INVALID_P2M_ENTRY)) {
+				pr_warn("set_phys_to_machine() failed, no memory added\n");
+				release_resource(&pgmap->res);
+				kfree(pgmap);
+				return -ENOMEM;
+			}
+                }
+	}
+#endif
+
+	vaddr = memremap_pages(pgmap, nid);
+	if (IS_ERR(vaddr)) {
+		pr_err("Cannot remap memory range\n");
+		release_resource(&pgmap->res);
+		kfree(pgmap);
+		return PTR_ERR(vaddr);
+	}
+
+	for (i = 0; i < alloc_pages; i++) {
+		struct page *pg = virt_to_page(vaddr + PAGE_SIZE * i);
+
+		BUG_ON(!virt_addr_valid(vaddr + PAGE_SIZE * i));
+		list_add(&pg->lru, &page_list);
+		list_count++;
+	}
+
+	return 0;
+}
+
+/**
+ * xen_alloc_unpopulated_pages - alloc unpopulated pages
+ * @nr_pages: Number of pages
+ * @pages: pages returned
+ * @return 0 on success, error otherwise
+ */
+int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages)
+{
+	unsigned int i;
+	int ret = 0;
+
+	mutex_lock(&list_lock);
+	if (list_count < nr_pages) {
+		ret = fill_list(nr_pages - list_count);
+		if (ret)
+			goto out;
+	}
+
+	for (i = 0; i < nr_pages; i++) {
+		struct page *pg = list_first_entry_or_null(&page_list,
+							   struct page,
+							   lru);
+
+		BUG_ON(!pg);
+		list_del(&pg->lru);
+		list_count--;
+		pages[i] = pg;
+
+#ifdef CONFIG_XEN_HAVE_PVMMU
+		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+			ret = xen_alloc_p2m_entry(page_to_pfn(pg));
+			if (ret < 0) {
+				unsigned int j;
+
+				for (j = 0; j <= i; j++) {
+					list_add(&pages[j]->lru, &page_list);
+					list_count++;
+				}
+				goto out;
+			}
+		}
+#endif
+	}
+
+out:
+	mutex_unlock(&list_lock);
+	return ret;
+}
+EXPORT_SYMBOL(xen_alloc_unpopulated_pages);
+
+/**
+ * xen_free_unpopulated_pages - return unpopulated pages
+ * @nr_pages: Number of pages
+ * @pages: pages to return
+ */
+void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages)
+{
+	unsigned int i;
+
+	mutex_lock(&list_lock);
+	for (i = 0; i < nr_pages; i++) {
+		list_add(&pages[i]->lru, &page_list);
+		list_count++;
+	}
+	mutex_unlock(&list_lock);
+}
+EXPORT_SYMBOL(xen_free_unpopulated_pages);
+
+#ifdef CONFIG_XEN_PV
+static int __init init(void)
+{
+	unsigned int i;
+
+	if (!xen_domain())
+		return -ENODEV;
+
+	if (!xen_pv_domain())
+		return 0;
+
+	/*
+	 * Initialize with pages from the extra memory regions (see
+	 * arch/x86/xen/setup.c).
+	 */
+	for (i = 0; i < XEN_EXTRA_MEM_MAX_REGIONS; i++) {
+		unsigned int j;
+
+		for (j = 0; j < xen_extra_mem[i].n_pfns; j++) {
+			struct page *pg =
+				pfn_to_page(xen_extra_mem[i].start_pfn + j);
+
+			list_add(&pg->lru, &page_list);
+			list_count++;
+		}
+	}
+
+	return 0;
+}
+subsys_initcall(init);
+#endif
diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
index 786fbb7d8be0..70b6c4780fbd 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -615,7 +615,7 @@ static int xenbus_map_ring_hvm(struct xenbus_device *dev,
 	bool leaked = false;
 	unsigned int nr_pages = XENBUS_PAGES(nr_grefs);
 
-	err = alloc_xenballooned_pages(nr_pages, node->hvm.pages);
+	err = xen_alloc_unpopulated_pages(nr_pages, node->hvm.pages);
 	if (err)
 		goto out_err;
 
@@ -656,7 +656,7 @@ static int xenbus_map_ring_hvm(struct xenbus_device *dev,
 			 addr, nr_pages);
  out_free_ballooned_pages:
 	if (!leaked)
-		free_xenballooned_pages(nr_pages, node->hvm.pages);
+		xen_free_unpopulated_pages(nr_pages, node->hvm.pages);
  out_err:
 	return err;
 }
@@ -852,7 +852,7 @@ static int xenbus_unmap_ring_hvm(struct xenbus_device *dev, void *vaddr)
 			       info.addrs);
 	if (!rv) {
 		vunmap(vaddr);
-		free_xenballooned_pages(nr_pages, node->hvm.pages);
+		xen_free_unpopulated_pages(nr_pages, node->hvm.pages);
 	}
 	else
 		WARN(1, "Leaking %p, size %u page(s)\n", vaddr, nr_pages);
diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
index 7b1077f0abcb..34742c6e189e 100644
--- a/drivers/xen/xlate_mmu.c
+++ b/drivers/xen/xlate_mmu.c
@@ -232,7 +232,7 @@ int __init xen_xlate_map_ballooned_pages(xen_pfn_t **gfns, void **virt,
 		kfree(pages);
 		return -ENOMEM;
 	}
-	rc = alloc_xenballooned_pages(nr_pages, pages);
+	rc = xen_alloc_unpopulated_pages(nr_pages, pages);
 	if (rc) {
 		pr_warn("%s Couldn't balloon alloc %ld pages rc:%d\n", __func__,
 			nr_pages, rc);
@@ -249,7 +249,7 @@ int __init xen_xlate_map_ballooned_pages(xen_pfn_t **gfns, void **virt,
 	if (!vaddr) {
 		pr_warn("%s Couldn't map %ld pages rc:%d\n", __func__,
 			nr_pages, rc);
-		free_xenballooned_pages(nr_pages, pages);
+		xen_free_unpopulated_pages(nr_pages, pages);
 		kfree(pages);
 		kfree(pfns);
 		return -ENOMEM;
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 19a72f591e2b..43efba045acc 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -52,4 +52,13 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
 extern u64 xen_saved_max_mem_size;
 #endif
 
+#ifdef CONFIG_XEN_UNPOPULATED_ALLOC
+int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages);
+void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages);
+#else
+#define xen_alloc_unpopulated_pages alloc_xenballooned_pages
+#define xen_free_unpopulated_pages free_xenballooned_pages
+#include <xen/balloon.h>
+#endif
+
 #endif	/* _XEN_XEN_H */
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC
  2020-08-11  9:44 ` [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC Roger Pau Monne
@ 2020-08-11 21:07   ` David Hildenbrand
  2020-08-20 11:37     ` Roger Pau Monné
  2020-08-31 19:20   ` Andrew Morton
  1 sibling, 1 reply; 21+ messages in thread
From: David Hildenbrand @ 2020-08-11 21:07 UTC (permalink / raw)
  To: Roger Pau Monne, linux-kernel
  Cc: Dave Jiang, linux-nvdimm, Vishal Verma, Johannes Thumshirn,
	linux-mm, Jason Gunthorpe, Logan Gunthorpe, Aneesh Kumar K.V,
	xen-devel, Dan Williams, Ira Weiny, Andrew Morton

On 11.08.20 11:44, Roger Pau Monne wrote:
> This is in preparation for the logic behind MEMORY_DEVICE_DEVDAX also
> being used by non DAX devices.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> Cc: Johannes Thumshirn <jthumshirn@suse.de>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: linux-nvdimm@lists.01.org
> Cc: xen-devel@lists.xenproject.org
> Cc: linux-mm@kvack.org
> ---
>  drivers/dax/device.c     | 2 +-
>  include/linux/memremap.h | 9 ++++-----
>  mm/memremap.c            | 2 +-
>  3 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index 4c0af2eb7e19..1e89513f3c59 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -429,7 +429,7 @@ int dev_dax_probe(struct device *dev)
>  		return -EBUSY;
>  	}
>  
> -	dev_dax->pgmap.type = MEMORY_DEVICE_DEVDAX;
> +	dev_dax->pgmap.type = MEMORY_DEVICE_GENERIC;
>  	addr = devm_memremap_pages(dev, &dev_dax->pgmap);
>  	if (IS_ERR(addr))
>  		return PTR_ERR(addr);
> diff --git a/include/linux/memremap.h b/include/linux/memremap.h
> index 5f5b2df06e61..e5862746751b 100644
> --- a/include/linux/memremap.h
> +++ b/include/linux/memremap.h
> @@ -46,11 +46,10 @@ struct vmem_altmap {
>   * wakeup is used to coordinate physical address space management (ex:
>   * fs truncate/hole punch) vs pinned pages (ex: device dma).
>   *
> - * MEMORY_DEVICE_DEVDAX:
> + * MEMORY_DEVICE_GENERIC:
>   * Host memory that has similar access semantics as System RAM i.e. DMA
> - * coherent and supports page pinning. In contrast to
> - * MEMORY_DEVICE_FS_DAX, this memory is access via a device-dax
> - * character device.
> + * coherent and supports page pinning. This is for example used by DAX devices
> + * that expose memory using a character device.
>   *
>   * MEMORY_DEVICE_PCI_P2PDMA:
>   * Device memory residing in a PCI BAR intended for use with Peer-to-Peer
> @@ -60,7 +59,7 @@ enum memory_type {
>  	/* 0 is reserved to catch uninitialized type fields */
>  	MEMORY_DEVICE_PRIVATE = 1,
>  	MEMORY_DEVICE_FS_DAX,
> -	MEMORY_DEVICE_DEVDAX,
> +	MEMORY_DEVICE_GENERIC,
>  	MEMORY_DEVICE_PCI_P2PDMA,
>  };
>  
> diff --git a/mm/memremap.c b/mm/memremap.c
> index 03e38b7a38f1..006dace60b1a 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -216,7 +216,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
>  			return ERR_PTR(-EINVAL);
>  		}
>  		break;
> -	case MEMORY_DEVICE_DEVDAX:
> +	case MEMORY_DEVICE_GENERIC:
>  		need_devmap_managed = false;
>  		break;
>  	case MEMORY_DEVICE_PCI_P2PDMA:
> 

No strong opinion (@Dan?), I do wonder if a separate type would make sense.

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-11  9:44 ` [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory Roger Pau Monne
@ 2020-08-12  7:28   ` Jürgen Groß
  2020-08-12  7:38     ` Roger Pau Monné
  2020-08-13  7:33   ` Christoph Hellwig
  1 sibling, 1 reply; 21+ messages in thread
From: Jürgen Groß @ 2020-08-12  7:28 UTC (permalink / raw)
  To: Roger Pau Monne, linux-kernel
  Cc: Stefano Stabellini, Wei Liu, Oleksandr Andrushchenko,
	David Airlie, Yan Yankovskyi, David Hildenbrand, dri-devel,
	Michal Hocko, linux-mm, Daniel Vetter, xen-devel,
	Boris Ostrovsky, Dan Williams, Dan Carpenter

On 11.08.20 11:44, Roger Pau Monne wrote:
> To be used in order to create foreign mappings. This is based on the
> ZONE_DEVICE facility which is used by persistent memory devices in
> order to create struct pages and kernel virtual mappings for the IOMEM
> areas of such devices. Note that on kernels without support for
> ZONE_DEVICE Xen will fallback to use ballooned pages in order to
> create foreign mappings.
> 
> The newly added helpers use the same parameters as the existing
> {alloc/free}_xenballooned_pages functions, which allows for in-place
> replacement of the callers. Once a memory region has been added to be
> used as scratch mapping space it will no longer be released, and pages
> returned are kept in a linked list. This allows to have a buffer of
> pages and prevents resorting to frequent additions and removals of
> regions.
> 
> If enabled (because ZONE_DEVICE is supported) the usage of the new
> functionality untangles Xen balloon and RAM hotplug from the usage of
> unpopulated physical memory ranges to map foreign pages, which is the
> correct thing to do in order to avoid mappings of foreign pages depend
> on memory hotplug.
> 
> Note the driver is currently not enabled on Arm platforms because it
> would interfere with the identity mapping required on some platforms.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Roger Pau Monne <roger.pau@citrix.com>
> Cc: Wei Liu <wl@xen.org>
> Cc: Yan Yankovskyi <yyankovskyi@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: xen-devel@lists.xenproject.org
> Cc: linux-mm@kvack.org
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Dan Williams <dan.j.williams@intel.com>
> ---
> Changes since v3:
>   - Introduce a Kconfig option that gates the addition of the
>     unpopulated alloc driver. This allows to easily disable it on Arm
>     platforms.
>   - Dropped Juergen RB due to the addition of the Kconfig option.
>   - Switched from MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC.
> 
> Changes since v2:
>   - Drop BUILD_BUG_ON regarding PVMMU page sizes.
>   - Use a SPDX license identifier.
>   - Call fill with only the minimum required number of pages.
>   - Include xen.h header in xen_drm_front_gem.c.
>   - Use less generic function names.
>   - Exit early from the init function if not a PV guest.
>   - Don't use all caps for region name.
> ---
>   drivers/gpu/drm/xen/xen_drm_front_gem.c |   9 +-
>   drivers/xen/Kconfig                     |   4 +
>   drivers/xen/Makefile                    |   1 +
>   drivers/xen/balloon.c                   |   4 +-
>   drivers/xen/grant-table.c               |   4 +-
>   drivers/xen/privcmd.c                   |   4 +-
>   drivers/xen/unpopulated-alloc.c         | 185 ++++++++++++++++++++++++
>   drivers/xen/xenbus/xenbus_client.c      |   6 +-
>   drivers/xen/xlate_mmu.c                 |   4 +-
>   include/xen/xen.h                       |   9 ++
>   10 files changed, 215 insertions(+), 15 deletions(-)
>   create mode 100644 drivers/xen/unpopulated-alloc.c
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 1d339ef92422..018020b91baa 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -327,4 +327,8 @@ config XEN_HAVE_VPMU
>   config XEN_FRONT_PGDIR_SHBUF
>   	tristate
>   
> +config XEN_UNPOPULATED_ALLOC
> +	bool
> +	default y if ZONE_DEVICE && !ARM && !ARM64

There is a current effort to enable Xen on RISC-V. Do we expect this
option to be usable for this architecture? If yes, I'm fine with the
exclusion of Arm, otherwise I'd opt for defaulting to yes only for
X86.

Either way you can have my:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-12  7:28   ` Jürgen Groß
@ 2020-08-12  7:38     ` Roger Pau Monné
  0 siblings, 0 replies; 21+ messages in thread
From: Roger Pau Monné @ 2020-08-12  7:38 UTC (permalink / raw)
  To: Jürgen Groß
  Cc: Stefano Stabellini, Wei Liu, Oleksandr Andrushchenko,
	David Airlie, Yan Yankovskyi, David Hildenbrand, linux-kernel,
	dri-devel, Michal Hocko, linux-mm, Daniel Vetter, xen-devel,
	Boris Ostrovsky, Dan Williams, Dan Carpenter

On Wed, Aug 12, 2020 at 09:28:45AM +0200, Jürgen Groß wrote:
> On 11.08.20 11:44, Roger Pau Monne wrote:
> > To be used in order to create foreign mappings. This is based on the
> > ZONE_DEVICE facility which is used by persistent memory devices in
> > order to create struct pages and kernel virtual mappings for the IOMEM
> > areas of such devices. Note that on kernels without support for
> > ZONE_DEVICE Xen will fallback to use ballooned pages in order to
> > create foreign mappings.
> > 
> > The newly added helpers use the same parameters as the existing
> > {alloc/free}_xenballooned_pages functions, which allows for in-place
> > replacement of the callers. Once a memory region has been added to be
> > used as scratch mapping space it will no longer be released, and pages
> > returned are kept in a linked list. This allows to have a buffer of
> > pages and prevents resorting to frequent additions and removals of
> > regions.
> > 
> > If enabled (because ZONE_DEVICE is supported) the usage of the new
> > functionality untangles Xen balloon and RAM hotplug from the usage of
> > unpopulated physical memory ranges to map foreign pages, which is the
> > correct thing to do in order to avoid mappings of foreign pages depend
> > on memory hotplug.
> > 
> > Note the driver is currently not enabled on Arm platforms because it
> > would interfere with the identity mapping required on some platforms.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Dan Carpenter <dan.carpenter@oracle.com>
> > Cc: Roger Pau Monne <roger.pau@citrix.com>
> > Cc: Wei Liu <wl@xen.org>
> > Cc: Yan Yankovskyi <yyankovskyi@gmail.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: xen-devel@lists.xenproject.org
> > Cc: linux-mm@kvack.org
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Michal Hocko <mhocko@kernel.org>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > ---
> > Changes since v3:
> >   - Introduce a Kconfig option that gates the addition of the
> >     unpopulated alloc driver. This allows to easily disable it on Arm
> >     platforms.
> >   - Dropped Juergen RB due to the addition of the Kconfig option.
> >   - Switched from MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC.
> > 
> > Changes since v2:
> >   - Drop BUILD_BUG_ON regarding PVMMU page sizes.
> >   - Use a SPDX license identifier.
> >   - Call fill with only the minimum required number of pages.
> >   - Include xen.h header in xen_drm_front_gem.c.
> >   - Use less generic function names.
> >   - Exit early from the init function if not a PV guest.
> >   - Don't use all caps for region name.
> > ---
> >   drivers/gpu/drm/xen/xen_drm_front_gem.c |   9 +-
> >   drivers/xen/Kconfig                     |   4 +
> >   drivers/xen/Makefile                    |   1 +
> >   drivers/xen/balloon.c                   |   4 +-
> >   drivers/xen/grant-table.c               |   4 +-
> >   drivers/xen/privcmd.c                   |   4 +-
> >   drivers/xen/unpopulated-alloc.c         | 185 ++++++++++++++++++++++++
> >   drivers/xen/xenbus/xenbus_client.c      |   6 +-
> >   drivers/xen/xlate_mmu.c                 |   4 +-
> >   include/xen/xen.h                       |   9 ++
> >   10 files changed, 215 insertions(+), 15 deletions(-)
> >   create mode 100644 drivers/xen/unpopulated-alloc.c
> > 
> > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > index 1d339ef92422..018020b91baa 100644
> > --- a/drivers/xen/Kconfig
> > +++ b/drivers/xen/Kconfig
> > @@ -327,4 +327,8 @@ config XEN_HAVE_VPMU
> >   config XEN_FRONT_PGDIR_SHBUF
> >   	tristate
> > +config XEN_UNPOPULATED_ALLOC
> > +	bool
> > +	default y if ZONE_DEVICE && !ARM && !ARM64
> 
> There is a current effort to enable Xen on RISC-V. Do we expect this
> option to be usable for this architecture?

It will depend on whether the Xen RISC-V implementation mandates an
IOMMU, for example Arm doesn't and hence uses an identity p2m for
dom0. If RISC-V does the same then I would assume this won't be
suitable as-is (not that it couldn't be made to work).

IMO it wasn't clear from the community call what was the RISC-V port
position regarding this, but IIRC the IOMMU spec for RISC-V was behind
the virtualization one, so it's likely that quite a lot of hardware
will have hardware virtualization support but no IOMMU, in which case
I think it's likely the RISC-V port will follow Arm and implement an
identity p2m.

> If yes, I'm fine with the
> exclusion of Arm, otherwise I'd opt for defaulting to yes only for
> X86.
> 
> Either way you can have my:
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

Thanks. Feel free to change the 'ZONE_DEVICE && !ARM && !ARM64' to
'ZONE_DEVICE && X86' if you think it's safer.

Roger.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-11  9:44 ` [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory Roger Pau Monne
  2020-08-12  7:28   ` Jürgen Groß
@ 2020-08-13  7:33   ` Christoph Hellwig
  2020-08-13  7:54     ` Roger Pau Monné
  1 sibling, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-08-13  7:33 UTC (permalink / raw)
  To: Roger Pau Monne
  Cc: linux-kernel, Oleksandr Andrushchenko, David Airlie,
	Daniel Vetter, Boris Ostrovsky, Juergen Gross,
	Stefano Stabellini, Dan Carpenter, Wei Liu, Yan Yankovskyi,
	dri-devel, xen-devel, linux-mm, David Hildenbrand, Michal Hocko,
	Dan Williams

On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
> If enabled (because ZONE_DEVICE is supported) the usage of the new
> functionality untangles Xen balloon and RAM hotplug from the usage of
> unpopulated physical memory ranges to map foreign pages, which is the
> correct thing to do in order to avoid mappings of foreign pages depend
> on memory hotplug.

So please just select ZONE_DEVICE if this is so much better rather
than maintaining two variants.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-13  7:33   ` Christoph Hellwig
@ 2020-08-13  7:54     ` Roger Pau Monné
  2020-08-13  9:49       ` Daniel Vetter
  2020-08-14  7:29       ` Christoph Hellwig
  0 siblings, 2 replies; 21+ messages in thread
From: Roger Pau Monné @ 2020-08-13  7:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-kernel, Oleksandr Andrushchenko, David Airlie,
	Daniel Vetter, Boris Ostrovsky, Juergen Gross,
	Stefano Stabellini, Dan Carpenter, Wei Liu, Yan Yankovskyi,
	dri-devel, xen-devel, linux-mm, David Hildenbrand, Michal Hocko,
	Dan Williams

On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
> On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
> > If enabled (because ZONE_DEVICE is supported) the usage of the new
> > functionality untangles Xen balloon and RAM hotplug from the usage of
> > unpopulated physical memory ranges to map foreign pages, which is the
> > correct thing to do in order to avoid mappings of foreign pages depend
> > on memory hotplug.
> 
> So please just select ZONE_DEVICE if this is so much better rather
> than maintaining two variants.

We still need to other variant for Arm at least, so both need to be
maintained anyway, even if we force ZONE_DEVICE on x86.

Thanks, Roger.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-13  7:54     ` Roger Pau Monné
@ 2020-08-13  9:49       ` Daniel Vetter
  2020-08-13 10:02         ` Roger Pau Monné
  2020-08-14  7:29       ` Christoph Hellwig
  1 sibling, 1 reply; 21+ messages in thread
From: Daniel Vetter @ 2020-08-13  9:49 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Christoph Hellwig, linux-kernel, Oleksandr Andrushchenko,
	David Airlie, Daniel Vetter, Boris Ostrovsky, Juergen Gross,
	Stefano Stabellini, Dan Carpenter, Wei Liu, Yan Yankovskyi,
	dri-devel, xen-devel, linux-mm, David Hildenbrand, Michal Hocko,
	Dan Williams

On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monné wrote:
> On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
> > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
> > > If enabled (because ZONE_DEVICE is supported) the usage of the new
> > > functionality untangles Xen balloon and RAM hotplug from the usage of
> > > unpopulated physical memory ranges to map foreign pages, which is the
> > > correct thing to do in order to avoid mappings of foreign pages depend
> > > on memory hotplug.
> > 
> > So please just select ZONE_DEVICE if this is so much better rather
> > than maintaining two variants.
> 
> We still need to other variant for Arm at least, so both need to be
> maintained anyway, even if we force ZONE_DEVICE on x86.

Why does arm not have ZONE_DEVICE?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-13  9:49       ` Daniel Vetter
@ 2020-08-13 10:02         ` Roger Pau Monné
  0 siblings, 0 replies; 21+ messages in thread
From: Roger Pau Monné @ 2020-08-13 10:02 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Christoph Hellwig, linux-kernel, Oleksandr Andrushchenko,
	David Airlie, Boris Ostrovsky, Juergen Gross, Stefano Stabellini,
	Dan Carpenter, Wei Liu, Yan Yankovskyi, dri-devel, xen-devel,
	linux-mm, David Hildenbrand, Michal Hocko, Dan Williams

Your email client seems to set 'Reply-to:' to point to everyone on the
'Cc:' field, but not yourself, which is kind of weird. I've manually
fixed it on this reply by moving everyone to the 'Cc:' field and
setting you on 'To:'.

On Thu, Aug 13, 2020 at 11:49:46AM +0200, Daniel Vetter wrote:
> On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monné wrote:
> > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
> > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
> > > > If enabled (because ZONE_DEVICE is supported) the usage of the new
> > > > functionality untangles Xen balloon and RAM hotplug from the usage of
> > > > unpopulated physical memory ranges to map foreign pages, which is the
> > > > correct thing to do in order to avoid mappings of foreign pages depend
> > > > on memory hotplug.
> > > 
> > > So please just select ZONE_DEVICE if this is so much better rather
> > > than maintaining two variants.
> > 
> > We still need to other variant for Arm at least, so both need to be
> > maintained anyway, even if we force ZONE_DEVICE on x86.
> 
> Why does arm not have ZONE_DEVICE?

It's not that Arm doesn't have ZONE_DEVICE, it's just that the
approach used here won't work correctly on an Arm Xen dom0 as-is.

This is due to the usage of an identity second stage translation in
order to workaround the lack of an IOMMU in some Arm boards.

It can be made to work on Arm, but will likely require someone from
the Arm side doing that.

Roger.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-13  7:54     ` Roger Pau Monné
  2020-08-13  9:49       ` Daniel Vetter
@ 2020-08-14  7:29       ` Christoph Hellwig
  2020-08-14  9:56         ` Roger Pau Monné
  1 sibling, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-08-14  7:29 UTC (permalink / raw)
  To: Roger Pau Monn??
  Cc: Christoph Hellwig, linux-kernel, Oleksandr Andrushchenko,
	David Airlie, Daniel Vetter, Boris Ostrovsky, Juergen Gross,
	Stefano Stabellini, Dan Carpenter, Wei Liu, Yan Yankovskyi,
	dri-devel, xen-devel, linux-mm, David Hildenbrand, Michal Hocko,
	Dan Williams

On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote:
> On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
> > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
> > > If enabled (because ZONE_DEVICE is supported) the usage of the new
> > > functionality untangles Xen balloon and RAM hotplug from the usage of
> > > unpopulated physical memory ranges to map foreign pages, which is the
> > > correct thing to do in order to avoid mappings of foreign pages depend
> > > on memory hotplug.
> > 
> > So please just select ZONE_DEVICE if this is so much better rather
> > than maintaining two variants.
> 
> We still need to other variant for Arm at least, so both need to be
> maintained anyway, even if we force ZONE_DEVICE on x86.

Well, it still really helps reproducability if you stick to one
implementation of x86.

The alternative would be an explicit config option to opt into it,
but just getting a different implementation based on a random
kernel option is strange.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-14  7:29       ` Christoph Hellwig
@ 2020-08-14  9:56         ` Roger Pau Monné
  2020-08-14 10:27           ` Jürgen Groß
  0 siblings, 1 reply; 21+ messages in thread
From: Roger Pau Monné @ 2020-08-14  9:56 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-kernel, Oleksandr Andrushchenko, David Airlie,
	Daniel Vetter, Boris Ostrovsky, Juergen Gross,
	Stefano Stabellini, Dan Carpenter, Wei Liu, Yan Yankovskyi,
	dri-devel, xen-devel, linux-mm, David Hildenbrand, Michal Hocko,
	Dan Williams

On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote:
> On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote:
> > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
> > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
> > > > If enabled (because ZONE_DEVICE is supported) the usage of the new
> > > > functionality untangles Xen balloon and RAM hotplug from the usage of
> > > > unpopulated physical memory ranges to map foreign pages, which is the
> > > > correct thing to do in order to avoid mappings of foreign pages depend
> > > > on memory hotplug.
> > > 
> > > So please just select ZONE_DEVICE if this is so much better rather
> > > than maintaining two variants.
> > 
> > We still need to other variant for Arm at least, so both need to be
> > maintained anyway, even if we force ZONE_DEVICE on x86.
> 
> Well, it still really helps reproducability if you stick to one
> implementation of x86.
> 
> The alternative would be an explicit config option to opt into it,
> but just getting a different implementation based on a random
> kernel option is strange.

Would adding something like the chunk below to the patch be OK?

---8<---
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 018020b91baa..5f321a1319e6 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF
 	tristate
 
 config XEN_UNPOPULATED_ALLOC
-	bool
-	default y if ZONE_DEVICE && !ARM && !ARM64
+	bool "Use unpopulated memory ranges for guest mappings"
+	depends on X86
+	select ZONE_DEVICE
+	default y
+	help
+	  Use unpopulated memory ranges in order to create mappings for guest
+	  memory regions, including grants maps and foreign pages. This avoids
+	  having to balloon out RAM regions in order to obtain physical memory
+	  space to create such mappings.
 
 endmenu



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-14  9:56         ` Roger Pau Monné
@ 2020-08-14 10:27           ` Jürgen Groß
  2020-08-14 12:47             ` Roger Pau Monné
  0 siblings, 1 reply; 21+ messages in thread
From: Jürgen Groß @ 2020-08-14 10:27 UTC (permalink / raw)
  To: Roger Pau Monné, Christoph Hellwig
  Cc: linux-kernel, Oleksandr Andrushchenko, David Airlie,
	Daniel Vetter, Boris Ostrovsky, Stefano Stabellini,
	Dan Carpenter, Wei Liu, Yan Yankovskyi, dri-devel, xen-devel,
	linux-mm, David Hildenbrand, Michal Hocko, Dan Williams

On 14.08.20 11:56, Roger Pau Monné wrote:
> On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote:
>> On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote:
>>> On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
>>>> On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
>>>>> If enabled (because ZONE_DEVICE is supported) the usage of the new
>>>>> functionality untangles Xen balloon and RAM hotplug from the usage of
>>>>> unpopulated physical memory ranges to map foreign pages, which is the
>>>>> correct thing to do in order to avoid mappings of foreign pages depend
>>>>> on memory hotplug.
>>>>
>>>> So please just select ZONE_DEVICE if this is so much better rather
>>>> than maintaining two variants.
>>>
>>> We still need to other variant for Arm at least, so both need to be
>>> maintained anyway, even if we force ZONE_DEVICE on x86.
>>
>> Well, it still really helps reproducability if you stick to one
>> implementation of x86.
>>
>> The alternative would be an explicit config option to opt into it,
>> but just getting a different implementation based on a random
>> kernel option is strange.
> 
> Would adding something like the chunk below to the patch be OK?
> 
> ---8<---
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 018020b91baa..5f321a1319e6 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF
>   	tristate
>   
>   config XEN_UNPOPULATED_ALLOC
> -	bool
> -	default y if ZONE_DEVICE && !ARM && !ARM64
> +	bool "Use unpopulated memory ranges for guest mappings"
> +	depends on X86
> +	select ZONE_DEVICE
> +	default y

I'd rather use "default XEN_BACKEND" here, as mappings of other guest's
memory is rarely used for non-backend guests.


Juergen



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-14 10:27           ` Jürgen Groß
@ 2020-08-14 12:47             ` Roger Pau Monné
  2020-08-14 12:54               ` Jürgen Groß
  0 siblings, 1 reply; 21+ messages in thread
From: Roger Pau Monné @ 2020-08-14 12:47 UTC (permalink / raw)
  To: Jürgen Groß
  Cc: Christoph Hellwig, linux-kernel, Oleksandr Andrushchenko,
	David Airlie, Daniel Vetter, Boris Ostrovsky, Stefano Stabellini,
	Dan Carpenter, Wei Liu, Yan Yankovskyi, dri-devel, xen-devel,
	linux-mm, David Hildenbrand, Michal Hocko, Dan Williams

On Fri, Aug 14, 2020 at 12:27:32PM +0200, Jürgen Groß wrote:
> On 14.08.20 11:56, Roger Pau Monné wrote:
> > On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote:
> > > On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote:
> > > > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
> > > > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
> > > > > > If enabled (because ZONE_DEVICE is supported) the usage of the new
> > > > > > functionality untangles Xen balloon and RAM hotplug from the usage of
> > > > > > unpopulated physical memory ranges to map foreign pages, which is the
> > > > > > correct thing to do in order to avoid mappings of foreign pages depend
> > > > > > on memory hotplug.
> > > > > 
> > > > > So please just select ZONE_DEVICE if this is so much better rather
> > > > > than maintaining two variants.
> > > > 
> > > > We still need to other variant for Arm at least, so both need to be
> > > > maintained anyway, even if we force ZONE_DEVICE on x86.
> > > 
> > > Well, it still really helps reproducability if you stick to one
> > > implementation of x86.
> > > 
> > > The alternative would be an explicit config option to opt into it,
> > > but just getting a different implementation based on a random
> > > kernel option is strange.
> > 
> > Would adding something like the chunk below to the patch be OK?
> > 
> > ---8<---
> > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > index 018020b91baa..5f321a1319e6 100644
> > --- a/drivers/xen/Kconfig
> > +++ b/drivers/xen/Kconfig
> > @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF
> >   	tristate
> >   config XEN_UNPOPULATED_ALLOC
> > -	bool
> > -	default y if ZONE_DEVICE && !ARM && !ARM64
> > +	bool "Use unpopulated memory ranges for guest mappings"
> > +	depends on X86
> > +	select ZONE_DEVICE
> > +	default y
> 
> I'd rather use "default XEN_BACKEND" here, as mappings of other guest's
> memory is rarely used for non-backend guests.

There's also the privcmd and gnt devices which make heavy use of this,
so I'm not sure only selecting by default on XEN_BACKEND is the best
option.

Thanks, Roger.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-14 12:47             ` Roger Pau Monné
@ 2020-08-14 12:54               ` Jürgen Groß
  2020-08-14 13:35                 ` Roger Pau Monné
  0 siblings, 1 reply; 21+ messages in thread
From: Jürgen Groß @ 2020-08-14 12:54 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Christoph Hellwig, linux-kernel, Oleksandr Andrushchenko,
	David Airlie, Daniel Vetter, Boris Ostrovsky, Stefano Stabellini,
	Dan Carpenter, Wei Liu, Yan Yankovskyi, dri-devel, xen-devel,
	linux-mm, David Hildenbrand, Michal Hocko, Dan Williams

On 14.08.20 14:47, Roger Pau Monné wrote:
> On Fri, Aug 14, 2020 at 12:27:32PM +0200, Jürgen Groß wrote:
>> On 14.08.20 11:56, Roger Pau Monné wrote:
>>> On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote:
>>>> On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote:
>>>>> On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
>>>>>> On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
>>>>>>> If enabled (because ZONE_DEVICE is supported) the usage of the new
>>>>>>> functionality untangles Xen balloon and RAM hotplug from the usage of
>>>>>>> unpopulated physical memory ranges to map foreign pages, which is the
>>>>>>> correct thing to do in order to avoid mappings of foreign pages depend
>>>>>>> on memory hotplug.
>>>>>>
>>>>>> So please just select ZONE_DEVICE if this is so much better rather
>>>>>> than maintaining two variants.
>>>>>
>>>>> We still need to other variant for Arm at least, so both need to be
>>>>> maintained anyway, even if we force ZONE_DEVICE on x86.
>>>>
>>>> Well, it still really helps reproducability if you stick to one
>>>> implementation of x86.
>>>>
>>>> The alternative would be an explicit config option to opt into it,
>>>> but just getting a different implementation based on a random
>>>> kernel option is strange.
>>>
>>> Would adding something like the chunk below to the patch be OK?
>>>
>>> ---8<---
>>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>>> index 018020b91baa..5f321a1319e6 100644
>>> --- a/drivers/xen/Kconfig
>>> +++ b/drivers/xen/Kconfig
>>> @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF
>>>    	tristate
>>>    config XEN_UNPOPULATED_ALLOC
>>> -	bool
>>> -	default y if ZONE_DEVICE && !ARM && !ARM64
>>> +	bool "Use unpopulated memory ranges for guest mappings"
>>> +	depends on X86
>>> +	select ZONE_DEVICE
>>> +	default y
>>
>> I'd rather use "default XEN_BACKEND" here, as mappings of other guest's
>> memory is rarely used for non-backend guests.
> 
> There's also the privcmd and gnt devices which make heavy use of this,
> so I'm not sure only selecting by default on XEN_BACKEND is the best
> option.

I just want to avoid that kernels built for running as Xen guest, but
not as dom0, will be forced to select ZONE_DEVICE.

As privcmd is dom0-only, this is no problem.

In case you are worrying about gnt devices, I'd be fine to switch to

default XEN_BACKEND || XEN_GNTDEV


Juergen



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-14 12:54               ` Jürgen Groß
@ 2020-08-14 13:35                 ` Roger Pau Monné
  2020-08-14 13:52                   ` Jürgen Groß
  0 siblings, 1 reply; 21+ messages in thread
From: Roger Pau Monné @ 2020-08-14 13:35 UTC (permalink / raw)
  To: Jürgen Groß
  Cc: Christoph Hellwig, linux-kernel, Oleksandr Andrushchenko,
	David Airlie, Daniel Vetter, Boris Ostrovsky, Stefano Stabellini,
	Dan Carpenter, Wei Liu, Yan Yankovskyi, dri-devel, xen-devel,
	linux-mm, David Hildenbrand, Michal Hocko, Dan Williams

On Fri, Aug 14, 2020 at 02:54:38PM +0200, Jürgen Groß wrote:
> On 14.08.20 14:47, Roger Pau Monné wrote:
> > On Fri, Aug 14, 2020 at 12:27:32PM +0200, Jürgen Groß wrote:
> > > On 14.08.20 11:56, Roger Pau Monné wrote:
> > > > On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote:
> > > > > On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote:
> > > > > > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
> > > > > > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
> > > > > > > > If enabled (because ZONE_DEVICE is supported) the usage of the new
> > > > > > > > functionality untangles Xen balloon and RAM hotplug from the usage of
> > > > > > > > unpopulated physical memory ranges to map foreign pages, which is the
> > > > > > > > correct thing to do in order to avoid mappings of foreign pages depend
> > > > > > > > on memory hotplug.
> > > > > > > 
> > > > > > > So please just select ZONE_DEVICE if this is so much better rather
> > > > > > > than maintaining two variants.
> > > > > > 
> > > > > > We still need to other variant for Arm at least, so both need to be
> > > > > > maintained anyway, even if we force ZONE_DEVICE on x86.
> > > > > 
> > > > > Well, it still really helps reproducability if you stick to one
> > > > > implementation of x86.
> > > > > 
> > > > > The alternative would be an explicit config option to opt into it,
> > > > > but just getting a different implementation based on a random
> > > > > kernel option is strange.
> > > > 
> > > > Would adding something like the chunk below to the patch be OK?
> > > > 
> > > > ---8<---
> > > > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > > > index 018020b91baa..5f321a1319e6 100644
> > > > --- a/drivers/xen/Kconfig
> > > > +++ b/drivers/xen/Kconfig
> > > > @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF
> > > >    	tristate
> > > >    config XEN_UNPOPULATED_ALLOC
> > > > -	bool
> > > > -	default y if ZONE_DEVICE && !ARM && !ARM64
> > > > +	bool "Use unpopulated memory ranges for guest mappings"
> > > > +	depends on X86
> > > > +	select ZONE_DEVICE
> > > > +	default y
> > > 
> > > I'd rather use "default XEN_BACKEND" here, as mappings of other guest's
> > > memory is rarely used for non-backend guests.
> > 
> > There's also the privcmd and gnt devices which make heavy use of this,
> > so I'm not sure only selecting by default on XEN_BACKEND is the best
> > option.
> 
> I just want to avoid that kernels built for running as Xen guest, but
> not as dom0, will be forced to select ZONE_DEVICE.
> 
> As privcmd is dom0-only, this is no problem.

Oh, didn't know that, I somehow assumed privcmd would be available to
all Xen guests regardless of whether dom0 support was selected.

> In case you are worrying about gnt devices, I'd be fine to switch to
> 
> default XEN_BACKEND || XEN_GNTDEV

Sure. maybe even:

default XEN_BACKEND || XEN_GNTDEV || XEN_DOM0

Do you want me to resend with this changed or are you happy to fixup
if there are no further comments?

Thanks, Roger.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
  2020-08-14 13:35                 ` Roger Pau Monné
@ 2020-08-14 13:52                   ` Jürgen Groß
  0 siblings, 0 replies; 21+ messages in thread
From: Jürgen Groß @ 2020-08-14 13:52 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Christoph Hellwig, linux-kernel, Oleksandr Andrushchenko,
	David Airlie, Daniel Vetter, Boris Ostrovsky, Stefano Stabellini,
	Dan Carpenter, Wei Liu, Yan Yankovskyi, dri-devel, xen-devel,
	linux-mm, David Hildenbrand, Michal Hocko, Dan Williams

On 14.08.20 15:35, Roger Pau Monné wrote:
> On Fri, Aug 14, 2020 at 02:54:38PM +0200, Jürgen Groß wrote:
>> On 14.08.20 14:47, Roger Pau Monné wrote:
>>> On Fri, Aug 14, 2020 at 12:27:32PM +0200, Jürgen Groß wrote:
>>>> On 14.08.20 11:56, Roger Pau Monné wrote:
>>>>> On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote:
>>>>>> On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote:
>>>>>>> On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote:
>>>>>>>> On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote:
>>>>>>>>> If enabled (because ZONE_DEVICE is supported) the usage of the new
>>>>>>>>> functionality untangles Xen balloon and RAM hotplug from the usage of
>>>>>>>>> unpopulated physical memory ranges to map foreign pages, which is the
>>>>>>>>> correct thing to do in order to avoid mappings of foreign pages depend
>>>>>>>>> on memory hotplug.
>>>>>>>>
>>>>>>>> So please just select ZONE_DEVICE if this is so much better rather
>>>>>>>> than maintaining two variants.
>>>>>>>
>>>>>>> We still need to other variant for Arm at least, so both need to be
>>>>>>> maintained anyway, even if we force ZONE_DEVICE on x86.
>>>>>>
>>>>>> Well, it still really helps reproducability if you stick to one
>>>>>> implementation of x86.
>>>>>>
>>>>>> The alternative would be an explicit config option to opt into it,
>>>>>> but just getting a different implementation based on a random
>>>>>> kernel option is strange.
>>>>>
>>>>> Would adding something like the chunk below to the patch be OK?
>>>>>
>>>>> ---8<---
>>>>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>>>>> index 018020b91baa..5f321a1319e6 100644
>>>>> --- a/drivers/xen/Kconfig
>>>>> +++ b/drivers/xen/Kconfig
>>>>> @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF
>>>>>     	tristate
>>>>>     config XEN_UNPOPULATED_ALLOC
>>>>> -	bool
>>>>> -	default y if ZONE_DEVICE && !ARM && !ARM64
>>>>> +	bool "Use unpopulated memory ranges for guest mappings"
>>>>> +	depends on X86
>>>>> +	select ZONE_DEVICE
>>>>> +	default y
>>>>
>>>> I'd rather use "default XEN_BACKEND" here, as mappings of other guest's
>>>> memory is rarely used for non-backend guests.
>>>
>>> There's also the privcmd and gnt devices which make heavy use of this,
>>> so I'm not sure only selecting by default on XEN_BACKEND is the best
>>> option.
>>
>> I just want to avoid that kernels built for running as Xen guest, but
>> not as dom0, will be forced to select ZONE_DEVICE.
>>
>> As privcmd is dom0-only, this is no problem.
> 
> Oh, didn't know that, I somehow assumed privcmd would be available to
> all Xen guests regardless of whether dom0 support was selected.

My remark above should have been more precise in this regard:

privcmd operations dealing with foreign mappings are normally limited
to dom0 (there might be special cases, like linux-based stubdoms, but
those will be configured carefully for that purpose, so they don't
need to be considered for selecting the default IMO).

> 
>> In case you are worrying about gnt devices, I'd be fine to switch to
>>
>> default XEN_BACKEND || XEN_GNTDEV
> 
> Sure. maybe even:
> 
> default XEN_BACKEND || XEN_GNTDEV || XEN_DOM0

Yes.

> 
> Do you want me to resend with this changed or are you happy to fixup
> if there are no further comments?

I'd prefer a resend (maybe after Patch 1 has gained its missing Ack, and
then with Patch 1 sent to me, too).


Juergen


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC
  2020-08-11 21:07   ` David Hildenbrand
@ 2020-08-20 11:37     ` Roger Pau Monné
  2020-08-31 10:19       ` Roger Pau Monné
  0 siblings, 1 reply; 21+ messages in thread
From: Roger Pau Monné @ 2020-08-20 11:37 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-kernel, David Hildenbrand, Vishal Verma, Dave Jiang,
	Andrew Morton, Jason Gunthorpe, Ira Weiny, Aneesh Kumar K.V,
	Johannes Thumshirn, Logan Gunthorpe, linux-nvdimm, xen-devel,
	linux-mm

On Tue, Aug 11, 2020 at 11:07:36PM +0200, David Hildenbrand wrote:
> On 11.08.20 11:44, Roger Pau Monne wrote:
> > This is in preparation for the logic behind MEMORY_DEVICE_DEVDAX also
> > being used by non DAX devices.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: Vishal Verma <vishal.l.verma@intel.com>
> > Cc: Dave Jiang <dave.jiang@intel.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > Cc: Ira Weiny <ira.weiny@intel.com>
> > Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> > Cc: Johannes Thumshirn <jthumshirn@suse.de>
> > Cc: Logan Gunthorpe <logang@deltatee.com>
> > Cc: linux-nvdimm@lists.01.org
> > Cc: xen-devel@lists.xenproject.org
> > Cc: linux-mm@kvack.org
> > ---
> >  drivers/dax/device.c     | 2 +-
> >  include/linux/memremap.h | 9 ++++-----
> >  mm/memremap.c            | 2 +-
> >  3 files changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> > index 4c0af2eb7e19..1e89513f3c59 100644
> > --- a/drivers/dax/device.c
> > +++ b/drivers/dax/device.c
> > @@ -429,7 +429,7 @@ int dev_dax_probe(struct device *dev)
> >  		return -EBUSY;
> >  	}
> >  
> > -	dev_dax->pgmap.type = MEMORY_DEVICE_DEVDAX;
> > +	dev_dax->pgmap.type = MEMORY_DEVICE_GENERIC;
> >  	addr = devm_memremap_pages(dev, &dev_dax->pgmap);
> >  	if (IS_ERR(addr))
> >  		return PTR_ERR(addr);
> > diff --git a/include/linux/memremap.h b/include/linux/memremap.h
> > index 5f5b2df06e61..e5862746751b 100644
> > --- a/include/linux/memremap.h
> > +++ b/include/linux/memremap.h
> > @@ -46,11 +46,10 @@ struct vmem_altmap {
> >   * wakeup is used to coordinate physical address space management (ex:
> >   * fs truncate/hole punch) vs pinned pages (ex: device dma).
> >   *
> > - * MEMORY_DEVICE_DEVDAX:
> > + * MEMORY_DEVICE_GENERIC:
> >   * Host memory that has similar access semantics as System RAM i.e. DMA
> > - * coherent and supports page pinning. In contrast to
> > - * MEMORY_DEVICE_FS_DAX, this memory is access via a device-dax
> > - * character device.
> > + * coherent and supports page pinning. This is for example used by DAX devices
> > + * that expose memory using a character device.
> >   *
> >   * MEMORY_DEVICE_PCI_P2PDMA:
> >   * Device memory residing in a PCI BAR intended for use with Peer-to-Peer
> > @@ -60,7 +59,7 @@ enum memory_type {
> >  	/* 0 is reserved to catch uninitialized type fields */
> >  	MEMORY_DEVICE_PRIVATE = 1,
> >  	MEMORY_DEVICE_FS_DAX,
> > -	MEMORY_DEVICE_DEVDAX,
> > +	MEMORY_DEVICE_GENERIC,
> >  	MEMORY_DEVICE_PCI_P2PDMA,
> >  };
> >  
> > diff --git a/mm/memremap.c b/mm/memremap.c
> > index 03e38b7a38f1..006dace60b1a 100644
> > --- a/mm/memremap.c
> > +++ b/mm/memremap.c
> > @@ -216,7 +216,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
> >  			return ERR_PTR(-EINVAL);
> >  		}
> >  		break;
> > -	case MEMORY_DEVICE_DEVDAX:
> > +	case MEMORY_DEVICE_GENERIC:
> >  		need_devmap_managed = false;
> >  		break;
> >  	case MEMORY_DEVICE_PCI_P2PDMA:
> > 
> 
> No strong opinion (@Dan?), I do wonder if a separate type would make sense.

Gentle ping.

Thanks, Roger.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC
  2020-08-20 11:37     ` Roger Pau Monné
@ 2020-08-31 10:19       ` Roger Pau Monné
  2020-08-31 20:56         ` Ira Weiny
  0 siblings, 1 reply; 21+ messages in thread
From: Roger Pau Monné @ 2020-08-31 10:19 UTC (permalink / raw)
  To: Dan Williams, David Hildenbrand
  Cc: linux-kernel, Vishal Verma, Dave Jiang, Andrew Morton,
	Jason Gunthorpe, Ira Weiny, Aneesh Kumar K.V, Johannes Thumshirn,
	Logan Gunthorpe, linux-nvdimm, xen-devel, linux-mm

On Thu, Aug 20, 2020 at 01:37:41PM +0200, Roger Pau Monné wrote:
> On Tue, Aug 11, 2020 at 11:07:36PM +0200, David Hildenbrand wrote:
> > On 11.08.20 11:44, Roger Pau Monne wrote:
> > > This is in preparation for the logic behind MEMORY_DEVICE_DEVDAX also
> > > being used by non DAX devices.
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > ---
> > > Cc: Dan Williams <dan.j.williams@intel.com>
> > > Cc: Vishal Verma <vishal.l.verma@intel.com>
> > > Cc: Dave Jiang <dave.jiang@intel.com>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > > Cc: Ira Weiny <ira.weiny@intel.com>
> > > Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> > > Cc: Johannes Thumshirn <jthumshirn@suse.de>
> > > Cc: Logan Gunthorpe <logang@deltatee.com>
> > > Cc: linux-nvdimm@lists.01.org
> > > Cc: xen-devel@lists.xenproject.org
> > > Cc: linux-mm@kvack.org
> > > ---
> > >  drivers/dax/device.c     | 2 +-
> > >  include/linux/memremap.h | 9 ++++-----
> > >  mm/memremap.c            | 2 +-
> > >  3 files changed, 6 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> > > index 4c0af2eb7e19..1e89513f3c59 100644
> > > --- a/drivers/dax/device.c
> > > +++ b/drivers/dax/device.c
> > > @@ -429,7 +429,7 @@ int dev_dax_probe(struct device *dev)
> > >  		return -EBUSY;
> > >  	}
> > >  
> > > -	dev_dax->pgmap.type = MEMORY_DEVICE_DEVDAX;
> > > +	dev_dax->pgmap.type = MEMORY_DEVICE_GENERIC;
> > >  	addr = devm_memremap_pages(dev, &dev_dax->pgmap);
> > >  	if (IS_ERR(addr))
> > >  		return PTR_ERR(addr);
> > > diff --git a/include/linux/memremap.h b/include/linux/memremap.h
> > > index 5f5b2df06e61..e5862746751b 100644
> > > --- a/include/linux/memremap.h
> > > +++ b/include/linux/memremap.h
> > > @@ -46,11 +46,10 @@ struct vmem_altmap {
> > >   * wakeup is used to coordinate physical address space management (ex:
> > >   * fs truncate/hole punch) vs pinned pages (ex: device dma).
> > >   *
> > > - * MEMORY_DEVICE_DEVDAX:
> > > + * MEMORY_DEVICE_GENERIC:
> > >   * Host memory that has similar access semantics as System RAM i.e. DMA
> > > - * coherent and supports page pinning. In contrast to
> > > - * MEMORY_DEVICE_FS_DAX, this memory is access via a device-dax
> > > - * character device.
> > > + * coherent and supports page pinning. This is for example used by DAX devices
> > > + * that expose memory using a character device.
> > >   *
> > >   * MEMORY_DEVICE_PCI_P2PDMA:
> > >   * Device memory residing in a PCI BAR intended for use with Peer-to-Peer
> > > @@ -60,7 +59,7 @@ enum memory_type {
> > >  	/* 0 is reserved to catch uninitialized type fields */
> > >  	MEMORY_DEVICE_PRIVATE = 1,
> > >  	MEMORY_DEVICE_FS_DAX,
> > > -	MEMORY_DEVICE_DEVDAX,
> > > +	MEMORY_DEVICE_GENERIC,
> > >  	MEMORY_DEVICE_PCI_P2PDMA,
> > >  };
> > >  
> > > diff --git a/mm/memremap.c b/mm/memremap.c
> > > index 03e38b7a38f1..006dace60b1a 100644
> > > --- a/mm/memremap.c
> > > +++ b/mm/memremap.c
> > > @@ -216,7 +216,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
> > >  			return ERR_PTR(-EINVAL);
> > >  		}
> > >  		break;
> > > -	case MEMORY_DEVICE_DEVDAX:
> > > +	case MEMORY_DEVICE_GENERIC:
> > >  		need_devmap_managed = false;
> > >  		break;
> > >  	case MEMORY_DEVICE_PCI_P2PDMA:
> > > 
> > 
> > No strong opinion (@Dan?), I do wonder if a separate type would make sense.
> 
> Gentle ping.

Sorry to ping again, but I would rather get this out of my queue if
possible, seeing as the other patch is OK to go in but depends on this
one going in first.

Thanks, Roger.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC
  2020-08-11  9:44 ` [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC Roger Pau Monne
  2020-08-11 21:07   ` David Hildenbrand
@ 2020-08-31 19:20   ` Andrew Morton
  1 sibling, 0 replies; 21+ messages in thread
From: Andrew Morton @ 2020-08-31 19:20 UTC (permalink / raw)
  To: Roger Pau Monne
  Cc: linux-kernel, Dan Williams, Vishal Verma, Dave Jiang,
	Jason Gunthorpe, Ira Weiny, Aneesh Kumar K.V, Johannes Thumshirn,
	Logan Gunthorpe, linux-nvdimm, xen-devel, linux-mm

On Tue, 11 Aug 2020 11:44:46 +0200 Roger Pau Monne <roger.pau@citrix.com> wrote:

> This is in preparation for the logic behind MEMORY_DEVICE_DEVDAX also
> being used by non DAX devices.

Acked-by: Andrew Morton <akpm@linux-foundation.org>.

Please add it to the Xen tree when appropriate.

(I'm not sure what David means by "separate type", but we can do that
later if desired.  Dan is taking a taking a bit of downtime).


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC
  2020-08-31 10:19       ` Roger Pau Monné
@ 2020-08-31 20:56         ` Ira Weiny
  0 siblings, 0 replies; 21+ messages in thread
From: Ira Weiny @ 2020-08-31 20:56 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Dan Williams, David Hildenbrand, linux-kernel, Vishal Verma,
	Dave Jiang, Andrew Morton, Jason Gunthorpe, Aneesh Kumar K.V,
	Johannes Thumshirn, Logan Gunthorpe, linux-nvdimm, xen-devel,
	linux-mm

On Mon, Aug 31, 2020 at 12:19:07PM +0200, Roger Pau Monné wrote:
> On Thu, Aug 20, 2020 at 01:37:41PM +0200, Roger Pau Monné wrote:
> > On Tue, Aug 11, 2020 at 11:07:36PM +0200, David Hildenbrand wrote:
> > > On 11.08.20 11:44, Roger Pau Monne wrote:
> > > > This is in preparation for the logic behind MEMORY_DEVICE_DEVDAX also
> > > > being used by non DAX devices.
> > > > 
> > > > No functional change intended.
> > > > 
> > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Dan is out on leave so I'll chime in.

I can't really justify keeping this as DEVDAX if there is another user who
needs the same type of mapping.

Sorry for the delay.

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> > > > ---
> > > > Cc: Dan Williams <dan.j.williams@intel.com>
> > > > Cc: Vishal Verma <vishal.l.verma@intel.com>
> > > > Cc: Dave Jiang <dave.jiang@intel.com>
> > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > > > Cc: Ira Weiny <ira.weiny@intel.com>
> > > > Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> > > > Cc: Johannes Thumshirn <jthumshirn@suse.de>
> > > > Cc: Logan Gunthorpe <logang@deltatee.com>
> > > > Cc: linux-nvdimm@lists.01.org
> > > > Cc: xen-devel@lists.xenproject.org
> > > > Cc: linux-mm@kvack.org
> > > > ---
> > > >  drivers/dax/device.c     | 2 +-
> > > >  include/linux/memremap.h | 9 ++++-----
> > > >  mm/memremap.c            | 2 +-
> > > >  3 files changed, 6 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> > > > index 4c0af2eb7e19..1e89513f3c59 100644
> > > > --- a/drivers/dax/device.c
> > > > +++ b/drivers/dax/device.c
> > > > @@ -429,7 +429,7 @@ int dev_dax_probe(struct device *dev)
> > > >  		return -EBUSY;
> > > >  	}
> > > >  
> > > > -	dev_dax->pgmap.type = MEMORY_DEVICE_DEVDAX;
> > > > +	dev_dax->pgmap.type = MEMORY_DEVICE_GENERIC;
> > > >  	addr = devm_memremap_pages(dev, &dev_dax->pgmap);
> > > >  	if (IS_ERR(addr))
> > > >  		return PTR_ERR(addr);
> > > > diff --git a/include/linux/memremap.h b/include/linux/memremap.h
> > > > index 5f5b2df06e61..e5862746751b 100644
> > > > --- a/include/linux/memremap.h
> > > > +++ b/include/linux/memremap.h
> > > > @@ -46,11 +46,10 @@ struct vmem_altmap {
> > > >   * wakeup is used to coordinate physical address space management (ex:
> > > >   * fs truncate/hole punch) vs pinned pages (ex: device dma).
> > > >   *
> > > > - * MEMORY_DEVICE_DEVDAX:
> > > > + * MEMORY_DEVICE_GENERIC:
> > > >   * Host memory that has similar access semantics as System RAM i.e. DMA
> > > > - * coherent and supports page pinning. In contrast to
> > > > - * MEMORY_DEVICE_FS_DAX, this memory is access via a device-dax
> > > > - * character device.
> > > > + * coherent and supports page pinning. This is for example used by DAX devices
> > > > + * that expose memory using a character device.
> > > >   *
> > > >   * MEMORY_DEVICE_PCI_P2PDMA:
> > > >   * Device memory residing in a PCI BAR intended for use with Peer-to-Peer
> > > > @@ -60,7 +59,7 @@ enum memory_type {
> > > >  	/* 0 is reserved to catch uninitialized type fields */
> > > >  	MEMORY_DEVICE_PRIVATE = 1,
> > > >  	MEMORY_DEVICE_FS_DAX,
> > > > -	MEMORY_DEVICE_DEVDAX,
> > > > +	MEMORY_DEVICE_GENERIC,
> > > >  	MEMORY_DEVICE_PCI_P2PDMA,
> > > >  };
> > > >  
> > > > diff --git a/mm/memremap.c b/mm/memremap.c
> > > > index 03e38b7a38f1..006dace60b1a 100644
> > > > --- a/mm/memremap.c
> > > > +++ b/mm/memremap.c
> > > > @@ -216,7 +216,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
> > > >  			return ERR_PTR(-EINVAL);
> > > >  		}
> > > >  		break;
> > > > -	case MEMORY_DEVICE_DEVDAX:
> > > > +	case MEMORY_DEVICE_GENERIC:
> > > >  		need_devmap_managed = false;
> > > >  		break;
> > > >  	case MEMORY_DEVICE_PCI_P2PDMA:
> > > > 
> > > 
> > > No strong opinion (@Dan?), I do wonder if a separate type would make sense.
> > 
> > Gentle ping.
> 
> Sorry to ping again, but I would rather get this out of my queue if
> possible, seeing as the other patch is OK to go in but depends on this
> one going in first.
> 
> Thanks, Roger.


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-08-31 20:57 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-11  9:44 [PATCH v4 0/2] xen/balloon: fixes for memory hotplug Roger Pau Monne
2020-08-11  9:44 ` [PATCH v4 1/2] memremap: rename MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC Roger Pau Monne
2020-08-11 21:07   ` David Hildenbrand
2020-08-20 11:37     ` Roger Pau Monné
2020-08-31 10:19       ` Roger Pau Monné
2020-08-31 20:56         ` Ira Weiny
2020-08-31 19:20   ` Andrew Morton
2020-08-11  9:44 ` [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory Roger Pau Monne
2020-08-12  7:28   ` Jürgen Groß
2020-08-12  7:38     ` Roger Pau Monné
2020-08-13  7:33   ` Christoph Hellwig
2020-08-13  7:54     ` Roger Pau Monné
2020-08-13  9:49       ` Daniel Vetter
2020-08-13 10:02         ` Roger Pau Monné
2020-08-14  7:29       ` Christoph Hellwig
2020-08-14  9:56         ` Roger Pau Monné
2020-08-14 10:27           ` Jürgen Groß
2020-08-14 12:47             ` Roger Pau Monné
2020-08-14 12:54               ` Jürgen Groß
2020-08-14 13:35                 ` Roger Pau Monné
2020-08-14 13:52                   ` Jürgen Groß

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).