[RFC PATCH 1/2] arm64: mm: Make virt_addr_valid to check for pfn_valid again

[RFC PATCH 1/2] arm64: mm: Make virt_addr_valid to check for pfn_valid again

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

The problem is that Arm's implementation of virt_addr_valid()
leads to memblock_is_map_memory() check, which will fail for
ZONE_DEVICE based addresses. But, the pfn_valid() check in turn
is able to cope with ZONE_DEVICE based memory.

You can find a good explanation of that problem at:
https://lore.kernel.org/lkml/1614921898-4099-2-git-send-email-anshuman.khandual@arm.com

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
I am not quite sure whether it is a "correct" place and
the change itself, I just partially restored a behaviour before:
https://lore.kernel.org/lkml/20210511100550.28178-4-rppt@kernel.org
So, the target of this patch is to get a feedback how to resolve
this properly if, of course, this really needs to be resolved
(I might miss important bits here).

It is worth mentioning that patch doesn't fix the current code base
(if I am not mistaken, no one calls virt_addr_valid() on Arm64 for
ZONE_DEVICE based addresses at the moment, so it seems that nothing
is broken), the fix is intended for the subsequent patch in this
series that will try to enable Xen's "unpopulated-alloc" usage
on Arm (it was enabled on x86 so far).
Please see:
[RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide
unallocated space

The subsequent patch will enable the code where virt_addr_valid()
is used in drivers/xen/unpopulated-alloc.c:fill_list() to check that
a virtual address returned by memremap_pages() is valid.
---
 arch/arm64/include/asm/memory.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 824a365..1a35a44 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -351,7 +351,7 @@ static inline void *phys_to_virt(phys_addr_t x)
 
 #define virt_addr_valid(addr)	({					\
 	__typeof__(addr) __addr = __tag_reset(addr);			\
-	__is_lm_address(__addr) && pfn_is_map_memory(virt_to_pfn(__addr));	\
+	__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr));	\
 })
 
 void dump_mem_limit(void);
-- 
2.7.4

[RFC PATCH 1/2] arm64: mm: Make virt_addr_valid to check for pfn_valid again

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

The problem is that Arm's implementation of virt_addr_valid()
leads to memblock_is_map_memory() check, which will fail for
ZONE_DEVICE based addresses. But, the pfn_valid() check in turn
is able to cope with ZONE_DEVICE based memory.

You can find a good explanation of that problem at:
https://lore.kernel.org/lkml/1614921898-4099-2-git-send-email-anshuman.khandual@arm.com

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
I am not quite sure whether it is a "correct" place and
the change itself, I just partially restored a behaviour before:
https://lore.kernel.org/lkml/20210511100550.28178-4-rppt@kernel.org
So, the target of this patch is to get a feedback how to resolve
this properly if, of course, this really needs to be resolved
(I might miss important bits here).

It is worth mentioning that patch doesn't fix the current code base
(if I am not mistaken, no one calls virt_addr_valid() on Arm64 for
ZONE_DEVICE based addresses at the moment, so it seems that nothing
is broken), the fix is intended for the subsequent patch in this
series that will try to enable Xen's "unpopulated-alloc" usage
on Arm (it was enabled on x86 so far).
Please see:
[RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide
unallocated space

The subsequent patch will enable the code where virt_addr_valid()
is used in drivers/xen/unpopulated-alloc.c:fill_list() to check that
a virtual address returned by memremap_pages() is valid.
---
 arch/arm64/include/asm/memory.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 824a365..1a35a44 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -351,7 +351,7 @@ static inline void *phys_to_virt(phys_addr_t x)
 
 #define virt_addr_valid(addr)	({					\
 	__typeof__(addr) __addr = __tag_reset(addr);			\
-	__is_lm_address(__addr) && pfn_is_map_memory(virt_to_pfn(__addr));	\
+	__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr));	\
 })
 
 void dump_mem_limit(void);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide unallocated space

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Add a logic to query hypervisor to find regions of guest physical
address space which are unused and can be safely used to create
grant/foreign mappings. Without that the XEN_UNPOPULATED_ALLOC
feature can't be used on Arm.

Introduce new helper arch_xen_unpopulated_init_resource() which
purpose is to create specific Xen resource based on the unused
memory regions provided by the hypervisor to be used as unallocated
address space for Xen scratch pages.
Re-use new XENMEM_get_unallocated_space hypercall on Arm to query
for the unallocated space. In case if arch doesn't offer Xen resource
the default IOMEM resource will be used in fill_list(). Since this
patch only implements Arm part, the behavior on x86 won't be changed.

The "unallocated space" can't be figured out precisely by
the domain on Arm without hypervisor involvement:
- not all device I/O regions are known by the time domain starts
  creating grant/foreign mappings
- the Dom0 is not aware of memory regions used for the identity
  mappings needed for the PV drivers to work
In both cases we might end up re-using these regions by
a mistake. So, the hypervisor which maintains the P2M for the domain
is in the best position to provide "unallocated space".

This patch also removes XEN_UNPOPULATED_ALLOC dependency on x86.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
TODO: Likely we would need to fall back to alloc_xenballooned_pages
if xen_alloc_unpopulated_pages fails on Arm, to be able to run
old hypervisor (without unallocated space support), but the next
question would be how to implement such "fallback" properly?

The corresponding Xen changes you can find at:
https://lore.kernel.org/xen-devel/1627489110-25633-1-git-send-email-olekstysh@gmail.com/
---
 arch/arm/xen/enlighten.c        | 94 +++++++++++++++++++++++++++++++++++++++++
 drivers/xen/Kconfig             |  2 +-
 drivers/xen/unpopulated-alloc.c | 62 +++++++++++++++++++++++++--
 include/xen/interface/memory.h  | 33 +++++++++++++++
 include/xen/xen.h               |  2 +
 5 files changed, 189 insertions(+), 4 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 7f1c106b..838696d 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -300,6 +300,100 @@ static void __init xen_acpi_guest_init(void)
 #endif
 }
 
+#ifdef CONFIG_XEN_UNPOPULATED_ALLOC
+int arch_xen_unpopulated_init_resource(struct resource *res)
+{
+	struct xen_get_unallocated_space xgus;
+	struct xen_unallocated_region *regions;
+	xen_pfn_t min_gpfn = -1, max_gpfn = 0;
+	unsigned int i, nr_regions;
+	struct resource *tmp_res;
+	int rc;
+
+	if (!xen_domain())
+		return -ENODEV;
+
+	/* Query hypervisor to find as many unused memory regions as possible */
+	nr_regions = XEN_MAX_UNALLOCATED_REGIONS;
+	regions = kcalloc(nr_regions, sizeof(regions[0]), GFP_KERNEL);
+	if (!regions)
+		return -ENOMEM;
+
+	xgus.domid = DOMID_SELF;
+	xgus.nr_regions = nr_regions;
+	set_xen_guest_handle(xgus.buffer, regions);
+
+	rc = HYPERVISOR_memory_op(XENMEM_get_unallocated_space, &xgus);
+	if (rc) {
+		pr_warn("XENMEM_get_unallocated_space failed, err=%d\n", rc);
+		goto err;
+	}
+
+	if (WARN_ON(xgus.nr_regions == 0)) {
+		rc = -EINVAL;
+		goto err;
+	}
+	nr_regions = xgus.nr_regions;
+
+	/*
+	 * Create resource from memory regions provided by the hypervisor to be
+	 * used as unallocated address space for Xen scratch pages.
+	 */
+	for (i = 0; i < nr_regions; i++) {
+		if (max_gpfn < regions[i].start_gpfn + regions[i].nr_gpfns)
+			max_gpfn = regions[i].start_gpfn + regions[i].nr_gpfns;
+		if (min_gpfn > regions[i].start_gpfn)
+			min_gpfn = regions[i].start_gpfn;
+	}
+	res->start = min_gpfn << PAGE_SHIFT;
+	res->end = (max_gpfn << PAGE_SHIFT) - 1;
+
+	/*
+	 * As memory regions are not necessarily completely sequential calculate
+	 * and reserve the possible holes. The rest of that address space will be
+	 * available for the allocation.
+	 */
+	for (i = 1; i < nr_regions; i++) {
+		resource_size_t start, end;
+
+		start = (regions[i - 1].start_gpfn << PAGE_SHIFT) +
+				regions[i - 1].nr_gpfns * PAGE_SIZE;
+		end = regions[i].start_gpfn << PAGE_SHIFT;
+
+		if (WARN_ON(start > end)) {
+			rc = -EINVAL;
+			goto err;
+		}
+
+		if (start == end)
+			continue;
+
+		tmp_res = kzalloc(sizeof(*tmp_res), GFP_KERNEL);
+		if (!tmp_res) {
+			rc = -ENOMEM;
+			goto err;
+		}
+
+		tmp_res->name = "Unavailable space";
+		tmp_res->start = start;
+		tmp_res->end = end - 1;
+
+		rc = request_resource(res, tmp_res);
+		if (rc) {
+			pr_err("Cannot insert IOMEM resource [%llx - %llx]\n",
+			       tmp_res->start, tmp_res->end);
+			kfree(tmp_res);
+			goto err;
+		}
+	}
+
+err:
+	kfree(regions);
+
+	return rc;
+}
+#endif
+
 static void __init xen_dt_guest_init(void)
 {
 	struct device_node *xen_node;
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 5f1ce59..86aa1ab 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -296,7 +296,7 @@ config XEN_FRONT_PGDIR_SHBUF
 
 config XEN_UNPOPULATED_ALLOC
 	bool "Use unpopulated memory ranges for guest mappings"
-	depends on X86 && ZONE_DEVICE
+	depends on ZONE_DEVICE
 	default XEN_BACKEND || XEN_GNTDEV || XEN_DOM0
 	help
 	  Use unpopulated memory ranges in order to create mappings for guest
diff --git a/drivers/xen/unpopulated-alloc.c b/drivers/xen/unpopulated-alloc.c
index 87e6b7d..ebf7d23 100644
--- a/drivers/xen/unpopulated-alloc.c
+++ b/drivers/xen/unpopulated-alloc.c
@@ -15,13 +15,39 @@ static DEFINE_MUTEX(list_lock);
 static struct page *page_list;
 static unsigned int list_count;
 
+static struct resource *target_resource;
+static struct resource xen_resource = {
+	.name = "Xen unallocated space",
+};
+
+int __weak arch_xen_unpopulated_init_resource(struct resource *res)
+{
+	return -ENOSYS;
+}
+
 static int fill_list(unsigned int nr_pages)
 {
 	struct dev_pagemap *pgmap;
-	struct resource *res;
+	struct resource *res, *tmp_res = NULL;
 	void *vaddr;
 	unsigned int i, alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
-	int ret = -ENOMEM;
+	int ret;
+
+	/*
+	 * Try to use Xen resource the first and fall back to default resource
+	 * if arch doesn't offer one.
+	 */
+	if (!target_resource) {
+		ret = arch_xen_unpopulated_init_resource(&xen_resource);
+		if (!ret) {
+			target_resource = &xen_resource;
+		} else if (ret == -ENOSYS) {
+			target_resource = &iomem_resource;
+		} else {
+			pr_err("Cannot initialize Xen resource\n");
+			return ret;
+		}
+	}
 
 	res = kzalloc(sizeof(*res), GFP_KERNEL);
 	if (!res)
@@ -30,7 +56,7 @@ static int fill_list(unsigned int nr_pages)
 	res->name = "Xen scratch";
 	res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
 
-	ret = allocate_resource(&iomem_resource, res,
+	ret = allocate_resource(target_resource, res,
 				alloc_pages * PAGE_SIZE, 0, -1,
 				PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
 	if (ret < 0) {
@@ -38,6 +64,31 @@ static int fill_list(unsigned int nr_pages)
 		goto err_resource;
 	}
 
+	/*
+	 * Reserve the region previously allocated from Xen resource to avoid
+	 * re-using it by someone else.
+	 */
+	if (target_resource != &iomem_resource) {
+		tmp_res = kzalloc(sizeof(*tmp_res), GFP_KERNEL);
+		if (!res) {
+			ret = -ENOMEM;
+			goto err_insert;
+		}
+
+		tmp_res->name = res->name;
+		tmp_res->start = res->start;
+		tmp_res->end = res->end;
+		tmp_res->flags = res->flags;
+
+		ret = insert_resource(&iomem_resource, tmp_res);
+		if (ret < 0) {
+			pr_err("Cannot insert IOMEM resource [%llx - %llx]\n",
+			       tmp_res->start, tmp_res->end);
+			kfree(tmp_res);
+			goto err_insert;
+		}
+	}
+
 	pgmap = kzalloc(sizeof(*pgmap), GFP_KERNEL);
 	if (!pgmap) {
 		ret = -ENOMEM;
@@ -96,6 +147,11 @@ static int fill_list(unsigned int nr_pages)
 err_memremap:
 	kfree(pgmap);
 err_pgmap:
+	if (tmp_res) {
+		release_resource(tmp_res);
+		kfree(tmp_res);
+	}
+err_insert:
 	release_resource(res);
 err_resource:
 	kfree(res);
diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
index 4470048..c6a2a63 100644
--- a/include/xen/interface/memory.h
+++ b/include/xen/interface/memory.h
@@ -325,4 +325,37 @@ struct xen_mem_acquire_resource {
 };
 DEFINE_GUEST_HANDLE_STRUCT(xen_mem_acquire_resource);
 
+/*
+ * Get the unallocated space (regions of guest physical address space which
+ * are unused) and can be used to create grant/foreign mappings.
+ */
+#define XENMEM_get_unallocated_space 29
+struct xen_unallocated_region {
+    xen_pfn_t start_gpfn;
+    xen_ulong_t nr_gpfns;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_unallocated_region);
+
+#define XEN_MAX_UNALLOCATED_REGIONS 32
+
+struct xen_get_unallocated_space {
+    /* IN - Which domain to provide unallocated space for */
+    domid_t domid;
+
+    /*
+     * IN/OUT - As an IN parameter number of memory regions which
+     *          can be written to the buffer (maximum size of the array)
+     *          As OUT parameter number of memory regions which
+     *          have been written to the buffer
+     */
+    unsigned int nr_regions;
+
+    /*
+     * OUT - An array of memory regions, the regions must be placed in
+     *       ascending order, there must be no overlap between them.
+     */
+    GUEST_HANDLE(xen_unallocated_region) buffer;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_get_unallocated_space);
+
 #endif /* __XEN_PUBLIC_MEMORY_H__ */
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 43efba0..f623905 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -55,6 +55,8 @@ extern u64 xen_saved_max_mem_size;
 #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);
+struct resource;
+int arch_xen_unpopulated_init_resource(struct resource *res);
 #else
 #define xen_alloc_unpopulated_pages alloc_xenballooned_pages
 #define xen_free_unpopulated_pages free_xenballooned_pages
-- 
2.7.4

[RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide unallocated space

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Add a logic to query hypervisor to find regions of guest physical
address space which are unused and can be safely used to create
grant/foreign mappings. Without that the XEN_UNPOPULATED_ALLOC
feature can't be used on Arm.

Introduce new helper arch_xen_unpopulated_init_resource() which
purpose is to create specific Xen resource based on the unused
memory regions provided by the hypervisor to be used as unallocated
address space for Xen scratch pages.
Re-use new XENMEM_get_unallocated_space hypercall on Arm to query
for the unallocated space. In case if arch doesn't offer Xen resource
the default IOMEM resource will be used in fill_list(). Since this
patch only implements Arm part, the behavior on x86 won't be changed.

The "unallocated space" can't be figured out precisely by
the domain on Arm without hypervisor involvement:
- not all device I/O regions are known by the time domain starts
  creating grant/foreign mappings
- the Dom0 is not aware of memory regions used for the identity
  mappings needed for the PV drivers to work
In both cases we might end up re-using these regions by
a mistake. So, the hypervisor which maintains the P2M for the domain
is in the best position to provide "unallocated space".

This patch also removes XEN_UNPOPULATED_ALLOC dependency on x86.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
TODO: Likely we would need to fall back to alloc_xenballooned_pages
if xen_alloc_unpopulated_pages fails on Arm, to be able to run
old hypervisor (without unallocated space support), but the next
question would be how to implement such "fallback" properly?

The corresponding Xen changes you can find at:
https://lore.kernel.org/xen-devel/1627489110-25633-1-git-send-email-olekstysh@gmail.com/
---
 arch/arm/xen/enlighten.c        | 94 +++++++++++++++++++++++++++++++++++++++++
 drivers/xen/Kconfig             |  2 +-
 drivers/xen/unpopulated-alloc.c | 62 +++++++++++++++++++++++++--
 include/xen/interface/memory.h  | 33 +++++++++++++++
 include/xen/xen.h               |  2 +
 5 files changed, 189 insertions(+), 4 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 7f1c106b..838696d 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -300,6 +300,100 @@ static void __init xen_acpi_guest_init(void)
 #endif
 }
 
+#ifdef CONFIG_XEN_UNPOPULATED_ALLOC
+int arch_xen_unpopulated_init_resource(struct resource *res)
+{
+	struct xen_get_unallocated_space xgus;
+	struct xen_unallocated_region *regions;
+	xen_pfn_t min_gpfn = -1, max_gpfn = 0;
+	unsigned int i, nr_regions;
+	struct resource *tmp_res;
+	int rc;
+
+	if (!xen_domain())
+		return -ENODEV;
+
+	/* Query hypervisor to find as many unused memory regions as possible */
+	nr_regions = XEN_MAX_UNALLOCATED_REGIONS;
+	regions = kcalloc(nr_regions, sizeof(regions[0]), GFP_KERNEL);
+	if (!regions)
+		return -ENOMEM;
+
+	xgus.domid = DOMID_SELF;
+	xgus.nr_regions = nr_regions;
+	set_xen_guest_handle(xgus.buffer, regions);
+
+	rc = HYPERVISOR_memory_op(XENMEM_get_unallocated_space, &xgus);
+	if (rc) {
+		pr_warn("XENMEM_get_unallocated_space failed, err=%d\n", rc);
+		goto err;
+	}
+
+	if (WARN_ON(xgus.nr_regions == 0)) {
+		rc = -EINVAL;
+		goto err;
+	}
+	nr_regions = xgus.nr_regions;
+
+	/*
+	 * Create resource from memory regions provided by the hypervisor to be
+	 * used as unallocated address space for Xen scratch pages.
+	 */
+	for (i = 0; i < nr_regions; i++) {
+		if (max_gpfn < regions[i].start_gpfn + regions[i].nr_gpfns)
+			max_gpfn = regions[i].start_gpfn + regions[i].nr_gpfns;
+		if (min_gpfn > regions[i].start_gpfn)
+			min_gpfn = regions[i].start_gpfn;
+	}
+	res->start = min_gpfn << PAGE_SHIFT;
+	res->end = (max_gpfn << PAGE_SHIFT) - 1;
+
+	/*
+	 * As memory regions are not necessarily completely sequential calculate
+	 * and reserve the possible holes. The rest of that address space will be
+	 * available for the allocation.
+	 */
+	for (i = 1; i < nr_regions; i++) {
+		resource_size_t start, end;
+
+		start = (regions[i - 1].start_gpfn << PAGE_SHIFT) +
+				regions[i - 1].nr_gpfns * PAGE_SIZE;
+		end = regions[i].start_gpfn << PAGE_SHIFT;
+
+		if (WARN_ON(start > end)) {
+			rc = -EINVAL;
+			goto err;
+		}
+
+		if (start == end)
+			continue;
+
+		tmp_res = kzalloc(sizeof(*tmp_res), GFP_KERNEL);
+		if (!tmp_res) {
+			rc = -ENOMEM;
+			goto err;
+		}
+
+		tmp_res->name = "Unavailable space";
+		tmp_res->start = start;
+		tmp_res->end = end - 1;
+
+		rc = request_resource(res, tmp_res);
+		if (rc) {
+			pr_err("Cannot insert IOMEM resource [%llx - %llx]\n",
+			       tmp_res->start, tmp_res->end);
+			kfree(tmp_res);
+			goto err;
+		}
+	}
+
+err:
+	kfree(regions);
+
+	return rc;
+}
+#endif
+
 static void __init xen_dt_guest_init(void)
 {
 	struct device_node *xen_node;
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 5f1ce59..86aa1ab 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -296,7 +296,7 @@ config XEN_FRONT_PGDIR_SHBUF
 
 config XEN_UNPOPULATED_ALLOC
 	bool "Use unpopulated memory ranges for guest mappings"
-	depends on X86 && ZONE_DEVICE
+	depends on ZONE_DEVICE
 	default XEN_BACKEND || XEN_GNTDEV || XEN_DOM0
 	help
 	  Use unpopulated memory ranges in order to create mappings for guest
diff --git a/drivers/xen/unpopulated-alloc.c b/drivers/xen/unpopulated-alloc.c
index 87e6b7d..ebf7d23 100644
--- a/drivers/xen/unpopulated-alloc.c
+++ b/drivers/xen/unpopulated-alloc.c
@@ -15,13 +15,39 @@ static DEFINE_MUTEX(list_lock);
 static struct page *page_list;
 static unsigned int list_count;
 
+static struct resource *target_resource;
+static struct resource xen_resource = {
+	.name = "Xen unallocated space",
+};
+
+int __weak arch_xen_unpopulated_init_resource(struct resource *res)
+{
+	return -ENOSYS;
+}
+
 static int fill_list(unsigned int nr_pages)
 {
 	struct dev_pagemap *pgmap;
-	struct resource *res;
+	struct resource *res, *tmp_res = NULL;
 	void *vaddr;
 	unsigned int i, alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
-	int ret = -ENOMEM;
+	int ret;
+
+	/*
+	 * Try to use Xen resource the first and fall back to default resource
+	 * if arch doesn't offer one.
+	 */
+	if (!target_resource) {
+		ret = arch_xen_unpopulated_init_resource(&xen_resource);
+		if (!ret) {
+			target_resource = &xen_resource;
+		} else if (ret == -ENOSYS) {
+			target_resource = &iomem_resource;
+		} else {
+			pr_err("Cannot initialize Xen resource\n");
+			return ret;
+		}
+	}
 
 	res = kzalloc(sizeof(*res), GFP_KERNEL);
 	if (!res)
@@ -30,7 +56,7 @@ static int fill_list(unsigned int nr_pages)
 	res->name = "Xen scratch";
 	res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
 
-	ret = allocate_resource(&iomem_resource, res,
+	ret = allocate_resource(target_resource, res,
 				alloc_pages * PAGE_SIZE, 0, -1,
 				PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
 	if (ret < 0) {
@@ -38,6 +64,31 @@ static int fill_list(unsigned int nr_pages)
 		goto err_resource;
 	}
 
+	/*
+	 * Reserve the region previously allocated from Xen resource to avoid
+	 * re-using it by someone else.
+	 */
+	if (target_resource != &iomem_resource) {
+		tmp_res = kzalloc(sizeof(*tmp_res), GFP_KERNEL);
+		if (!res) {
+			ret = -ENOMEM;
+			goto err_insert;
+		}
+
+		tmp_res->name = res->name;
+		tmp_res->start = res->start;
+		tmp_res->end = res->end;
+		tmp_res->flags = res->flags;
+
+		ret = insert_resource(&iomem_resource, tmp_res);
+		if (ret < 0) {
+			pr_err("Cannot insert IOMEM resource [%llx - %llx]\n",
+			       tmp_res->start, tmp_res->end);
+			kfree(tmp_res);
+			goto err_insert;
+		}
+	}
+
 	pgmap = kzalloc(sizeof(*pgmap), GFP_KERNEL);
 	if (!pgmap) {
 		ret = -ENOMEM;
@@ -96,6 +147,11 @@ static int fill_list(unsigned int nr_pages)
 err_memremap:
 	kfree(pgmap);
 err_pgmap:
+	if (tmp_res) {
+		release_resource(tmp_res);
+		kfree(tmp_res);
+	}
+err_insert:
 	release_resource(res);
 err_resource:
 	kfree(res);
diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
index 4470048..c6a2a63 100644
--- a/include/xen/interface/memory.h
+++ b/include/xen/interface/memory.h
@@ -325,4 +325,37 @@ struct xen_mem_acquire_resource {
 };
 DEFINE_GUEST_HANDLE_STRUCT(xen_mem_acquire_resource);
 
+/*
+ * Get the unallocated space (regions of guest physical address space which
+ * are unused) and can be used to create grant/foreign mappings.
+ */
+#define XENMEM_get_unallocated_space 29
+struct xen_unallocated_region {
+    xen_pfn_t start_gpfn;
+    xen_ulong_t nr_gpfns;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_unallocated_region);
+
+#define XEN_MAX_UNALLOCATED_REGIONS 32
+
+struct xen_get_unallocated_space {
+    /* IN - Which domain to provide unallocated space for */
+    domid_t domid;
+
+    /*
+     * IN/OUT - As an IN parameter number of memory regions which
+     *          can be written to the buffer (maximum size of the array)
+     *          As OUT parameter number of memory regions which
+     *          have been written to the buffer
+     */
+    unsigned int nr_regions;
+
+    /*
+     * OUT - An array of memory regions, the regions must be placed in
+     *       ascending order, there must be no overlap between them.
+     */
+    GUEST_HANDLE(xen_unallocated_region) buffer;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_get_unallocated_space);
+
 #endif /* __XEN_PUBLIC_MEMORY_H__ */
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 43efba0..f623905 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -55,6 +55,8 @@ extern u64 xen_saved_max_mem_size;
 #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);
+struct resource;
+int arch_xen_unpopulated_init_resource(struct resource *res);
 #else
 #define xen_alloc_unpopulated_pages alloc_xenballooned_pages
 #define xen_free_unpopulated_pages free_xenballooned_pages
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 1/2] arm64: mm: Make virt_addr_valid to check for pfn_valid again

[Catalin Marinas]

Adding Mike and Anshuman,

On Wed, Jul 28, 2021 at 07:44:15PM +0300, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> The problem is that Arm's implementation of virt_addr_valid()
> leads to memblock_is_map_memory() check, which will fail for
> ZONE_DEVICE based addresses. But, the pfn_valid() check in turn
> is able to cope with ZONE_DEVICE based memory.
> 
> You can find a good explanation of that problem at:
> https://lore.kernel.org/lkml/1614921898-4099-2-git-send-email-anshuman.khandual@arm.com
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
> I am not quite sure whether it is a "correct" place and
> the change itself, I just partially restored a behaviour before:
> https://lore.kernel.org/lkml/20210511100550.28178-4-rppt@kernel.org
> So, the target of this patch is to get a feedback how to resolve
> this properly if, of course, this really needs to be resolved
> (I might miss important bits here).
> 
> It is worth mentioning that patch doesn't fix the current code base
> (if I am not mistaken, no one calls virt_addr_valid() on Arm64 for
> ZONE_DEVICE based addresses at the moment, so it seems that nothing
> is broken), the fix is intended for the subsequent patch in this
> series that will try to enable Xen's "unpopulated-alloc" usage
> on Arm (it was enabled on x86 so far).
> Please see:
> [RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide
> unallocated space
> 
> The subsequent patch will enable the code where virt_addr_valid()
> is used in drivers/xen/unpopulated-alloc.c:fill_list() to check that
> a virtual address returned by memremap_pages() is valid.

I wonder what the point of calling virt_addr_valid() in fill_list() is?
If memremap_pages() succeeded, the pages were mapped at the returned
vaddr, there's no need for an additional virt_addr_valid() check.

> ---
>  arch/arm64/include/asm/memory.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 824a365..1a35a44 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -351,7 +351,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>  
>  #define virt_addr_valid(addr)	({					\
>  	__typeof__(addr) __addr = __tag_reset(addr);			\
> -	__is_lm_address(__addr) && pfn_is_map_memory(virt_to_pfn(__addr));	\
> +	__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr));	\
>  })

pfn_valid() only guarantees the presence of a struct page but not
necessarily that the virtual address is accessible (valid). So this
change would break the NOMAP ranges case.

-- 
Catalin

Re: [RFC PATCH 1/2] arm64: mm: Make virt_addr_valid to check for pfn_valid again

[Catalin Marinas]

Adding Mike and Anshuman,

On Wed, Jul 28, 2021 at 07:44:15PM +0300, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> The problem is that Arm's implementation of virt_addr_valid()
> leads to memblock_is_map_memory() check, which will fail for
> ZONE_DEVICE based addresses. But, the pfn_valid() check in turn
> is able to cope with ZONE_DEVICE based memory.
> 
> You can find a good explanation of that problem at:
> https://lore.kernel.org/lkml/1614921898-4099-2-git-send-email-anshuman.khandual@arm.com
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
> I am not quite sure whether it is a "correct" place and
> the change itself, I just partially restored a behaviour before:
> https://lore.kernel.org/lkml/20210511100550.28178-4-rppt@kernel.org
> So, the target of this patch is to get a feedback how to resolve
> this properly if, of course, this really needs to be resolved
> (I might miss important bits here).
> 
> It is worth mentioning that patch doesn't fix the current code base
> (if I am not mistaken, no one calls virt_addr_valid() on Arm64 for
> ZONE_DEVICE based addresses at the moment, so it seems that nothing
> is broken), the fix is intended for the subsequent patch in this
> series that will try to enable Xen's "unpopulated-alloc" usage
> on Arm (it was enabled on x86 so far).
> Please see:
> [RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide
> unallocated space
> 
> The subsequent patch will enable the code where virt_addr_valid()
> is used in drivers/xen/unpopulated-alloc.c:fill_list() to check that
> a virtual address returned by memremap_pages() is valid.

I wonder what the point of calling virt_addr_valid() in fill_list() is?
If memremap_pages() succeeded, the pages were mapped at the returned
vaddr, there's no need for an additional virt_addr_valid() check.

> ---
>  arch/arm64/include/asm/memory.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 824a365..1a35a44 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -351,7 +351,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>  
>  #define virt_addr_valid(addr)	({					\
>  	__typeof__(addr) __addr = __tag_reset(addr);			\
> -	__is_lm_address(__addr) && pfn_is_map_memory(virt_to_pfn(__addr));	\
> +	__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr));	\
>  })

pfn_valid() only guarantees the presence of a struct page but not
necessarily that the virtual address is accessible (valid). So this
change would break the NOMAP ranges case.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 1/2] arm64: mm: Make virt_addr_valid to check for pfn_valid again

[Mike Rapoport]

Hi,

On Mon, Aug 02, 2021 at 01:19:48PM +0100, Catalin Marinas wrote:
> Adding Mike and Anshuman,
> 
> On Wed, Jul 28, 2021 at 07:44:15PM +0300, Oleksandr Tyshchenko wrote:
> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > 
> > The problem is that Arm's implementation of virt_addr_valid()
> > leads to memblock_is_map_memory() check, which will fail for
> > ZONE_DEVICE based addresses. But, the pfn_valid() check in turn
> > is able to cope with ZONE_DEVICE based memory.
> > 
> > You can find a good explanation of that problem at:
> > https://lore.kernel.org/lkml/1614921898-4099-2-git-send-email-anshuman.khandual@arm.com
> > 
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > ---
> > I am not quite sure whether it is a "correct" place and
> > the change itself, I just partially restored a behaviour before:
> > https://lore.kernel.org/lkml/20210511100550.28178-4-rppt@kernel.org
> > So, the target of this patch is to get a feedback how to resolve
> > this properly if, of course, this really needs to be resolved
> > (I might miss important bits here).
> > 
> > It is worth mentioning that patch doesn't fix the current code base
> > (if I am not mistaken, no one calls virt_addr_valid() on Arm64 for
> > ZONE_DEVICE based addresses at the moment, so it seems that nothing
> > is broken), the fix is intended for the subsequent patch in this
> > series that will try to enable Xen's "unpopulated-alloc" usage
> > on Arm (it was enabled on x86 so far).
> > Please see:
> > [RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide
> > unallocated space
> > 
> > The subsequent patch will enable the code where virt_addr_valid()
> > is used in drivers/xen/unpopulated-alloc.c:fill_list() to check that
> > a virtual address returned by memremap_pages() is valid.
 
> I wonder what the point of calling virt_addr_valid() in fill_list() is?
> If memremap_pages() succeeded, the pages were mapped at the returned
> vaddr, there's no need for an additional virt_addr_valid() check.

The virt_addr_valid() check in fill_list() looks bogus to me as well. If
memremap_pages() succeeds the range is guaranteed to have proper page
table.

I believe the first patch should be rather removal of the virt_addr_valid()
check in fill_list().
 
> > ---
> >  arch/arm64/include/asm/memory.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> > index 824a365..1a35a44 100644
> > --- a/arch/arm64/include/asm/memory.h
> > +++ b/arch/arm64/include/asm/memory.h
> > @@ -351,7 +351,7 @@ static inline void *phys_to_virt(phys_addr_t x)
> >  
> >  #define virt_addr_valid(addr)	({					\
> >  	__typeof__(addr) __addr = __tag_reset(addr);			\
> > -	__is_lm_address(__addr) && pfn_is_map_memory(virt_to_pfn(__addr));	\
> > +	__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr));	\
> >  })
> 
> pfn_valid() only guarantees the presence of a struct page but not
> necessarily that the virtual address is accessible (valid). So this
> change would break the NOMAP ranges case.

+1

-- 
Sincerely yours,
Mike.

Re: [RFC PATCH 1/2] arm64: mm: Make virt_addr_valid to check for pfn_valid again

[Mike Rapoport]

Hi,

On Mon, Aug 02, 2021 at 01:19:48PM +0100, Catalin Marinas wrote:
> Adding Mike and Anshuman,
> 
> On Wed, Jul 28, 2021 at 07:44:15PM +0300, Oleksandr Tyshchenko wrote:
> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > 
> > The problem is that Arm's implementation of virt_addr_valid()
> > leads to memblock_is_map_memory() check, which will fail for
> > ZONE_DEVICE based addresses. But, the pfn_valid() check in turn
> > is able to cope with ZONE_DEVICE based memory.
> > 
> > You can find a good explanation of that problem at:
> > https://lore.kernel.org/lkml/1614921898-4099-2-git-send-email-anshuman.khandual@arm.com
> > 
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > ---
> > I am not quite sure whether it is a "correct" place and
> > the change itself, I just partially restored a behaviour before:
> > https://lore.kernel.org/lkml/20210511100550.28178-4-rppt@kernel.org
> > So, the target of this patch is to get a feedback how to resolve
> > this properly if, of course, this really needs to be resolved
> > (I might miss important bits here).
> > 
> > It is worth mentioning that patch doesn't fix the current code base
> > (if I am not mistaken, no one calls virt_addr_valid() on Arm64 for
> > ZONE_DEVICE based addresses at the moment, so it seems that nothing
> > is broken), the fix is intended for the subsequent patch in this
> > series that will try to enable Xen's "unpopulated-alloc" usage
> > on Arm (it was enabled on x86 so far).
> > Please see:
> > [RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide
> > unallocated space
> > 
> > The subsequent patch will enable the code where virt_addr_valid()
> > is used in drivers/xen/unpopulated-alloc.c:fill_list() to check that
> > a virtual address returned by memremap_pages() is valid.
 
> I wonder what the point of calling virt_addr_valid() in fill_list() is?
> If memremap_pages() succeeded, the pages were mapped at the returned
> vaddr, there's no need for an additional virt_addr_valid() check.

The virt_addr_valid() check in fill_list() looks bogus to me as well. If
memremap_pages() succeeds the range is guaranteed to have proper page
table.

I believe the first patch should be rather removal of the virt_addr_valid()
check in fill_list().
 
> > ---
> >  arch/arm64/include/asm/memory.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> > index 824a365..1a35a44 100644
> > --- a/arch/arm64/include/asm/memory.h
> > +++ b/arch/arm64/include/asm/memory.h
> > @@ -351,7 +351,7 @@ static inline void *phys_to_virt(phys_addr_t x)
> >  
> >  #define virt_addr_valid(addr)	({					\
> >  	__typeof__(addr) __addr = __tag_reset(addr);			\
> > -	__is_lm_address(__addr) && pfn_is_map_memory(virt_to_pfn(__addr));	\
> > +	__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr));	\
> >  })
> 
> pfn_valid() only guarantees the presence of a struct page but not
> necessarily that the virtual address is accessible (valid). So this
> change would break the NOMAP ranges case.

+1

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 1/2] arm64: mm: Make virt_addr_valid to check for pfn_valid again

[Oleksandr]

Hello, all.


On 02.08.21 18:08, Mike Rapoport wrote:
> Hi,
>
> On Mon, Aug 02, 2021 at 01:19:48PM +0100, Catalin Marinas wrote:
>> Adding Mike and Anshuman,
>>
>> On Wed, Jul 28, 2021 at 07:44:15PM +0300, Oleksandr Tyshchenko wrote:
>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>
>>> The problem is that Arm's implementation of virt_addr_valid()
>>> leads to memblock_is_map_memory() check, which will fail for
>>> ZONE_DEVICE based addresses. But, the pfn_valid() check in turn
>>> is able to cope with ZONE_DEVICE based memory.
>>>
>>> You can find a good explanation of that problem at:
>>> https://lore.kernel.org/lkml/1614921898-4099-2-git-send-email-anshuman.khandual@arm.com
>>>
>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>> ---
>>> I am not quite sure whether it is a "correct" place and
>>> the change itself, I just partially restored a behaviour before:
>>> https://lore.kernel.org/lkml/20210511100550.28178-4-rppt@kernel.org
>>> So, the target of this patch is to get a feedback how to resolve
>>> this properly if, of course, this really needs to be resolved
>>> (I might miss important bits here).
>>>
>>> It is worth mentioning that patch doesn't fix the current code base
>>> (if I am not mistaken, no one calls virt_addr_valid() on Arm64 for
>>> ZONE_DEVICE based addresses at the moment, so it seems that nothing
>>> is broken), the fix is intended for the subsequent patch in this
>>> series that will try to enable Xen's "unpopulated-alloc" usage
>>> on Arm (it was enabled on x86 so far).
>>> Please see:
>>> [RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide
>>> unallocated space
>>>
>>> The subsequent patch will enable the code where virt_addr_valid()
>>> is used in drivers/xen/unpopulated-alloc.c:fill_list() to check that
>>> a virtual address returned by memremap_pages() is valid.
>   
>> I wonder what the point of calling virt_addr_valid() in fill_list() is?
>> If memremap_pages() succeeded, the pages were mapped at the returned
>> vaddr, there's no need for an additional virt_addr_valid() check.
> The virt_addr_valid() check in fill_list() looks bogus to me as well. If
> memremap_pages() succeeds the range is guaranteed to have proper page
> table.
>
> I believe the first patch should be rather removal of the virt_addr_valid()
> check in fill_list().
Thank you for the clarification, I will send a patch to remove 
virt_addr_valid()
check in fill_list() for the non-RFC version.


>   
>>> ---
>>>   arch/arm64/include/asm/memory.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>>> index 824a365..1a35a44 100644
>>> --- a/arch/arm64/include/asm/memory.h
>>> +++ b/arch/arm64/include/asm/memory.h
>>> @@ -351,7 +351,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>>>   
>>>   #define virt_addr_valid(addr)	({					\
>>>   	__typeof__(addr) __addr = __tag_reset(addr);			\
>>> -	__is_lm_address(__addr) && pfn_is_map_memory(virt_to_pfn(__addr));	\
>>> +	__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr));	\
>>>   })
>> pfn_valid() only guarantees the presence of a struct page but not
>> necessarily that the virtual address is accessible (valid). So this
>> change would break the NOMAP ranges case.
> +1
Oh, I got it.

-- 
Regards,

Oleksandr Tyshchenko

Re: [RFC PATCH 1/2] arm64: mm: Make virt_addr_valid to check for pfn_valid again

[Oleksandr]

Hello, all.


On 02.08.21 18:08, Mike Rapoport wrote:
> Hi,
>
> On Mon, Aug 02, 2021 at 01:19:48PM +0100, Catalin Marinas wrote:
>> Adding Mike and Anshuman,
>>
>> On Wed, Jul 28, 2021 at 07:44:15PM +0300, Oleksandr Tyshchenko wrote:
>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>
>>> The problem is that Arm's implementation of virt_addr_valid()
>>> leads to memblock_is_map_memory() check, which will fail for
>>> ZONE_DEVICE based addresses. But, the pfn_valid() check in turn
>>> is able to cope with ZONE_DEVICE based memory.
>>>
>>> You can find a good explanation of that problem at:
>>> https://lore.kernel.org/lkml/1614921898-4099-2-git-send-email-anshuman.khandual@arm.com
>>>
>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>> ---
>>> I am not quite sure whether it is a "correct" place and
>>> the change itself, I just partially restored a behaviour before:
>>> https://lore.kernel.org/lkml/20210511100550.28178-4-rppt@kernel.org
>>> So, the target of this patch is to get a feedback how to resolve
>>> this properly if, of course, this really needs to be resolved
>>> (I might miss important bits here).
>>>
>>> It is worth mentioning that patch doesn't fix the current code base
>>> (if I am not mistaken, no one calls virt_addr_valid() on Arm64 for
>>> ZONE_DEVICE based addresses at the moment, so it seems that nothing
>>> is broken), the fix is intended for the subsequent patch in this
>>> series that will try to enable Xen's "unpopulated-alloc" usage
>>> on Arm (it was enabled on x86 so far).
>>> Please see:
>>> [RFC PATCH 2/2] xen/unpopulated-alloc: Query hypervisor to provide
>>> unallocated space
>>>
>>> The subsequent patch will enable the code where virt_addr_valid()
>>> is used in drivers/xen/unpopulated-alloc.c:fill_list() to check that
>>> a virtual address returned by memremap_pages() is valid.
>   
>> I wonder what the point of calling virt_addr_valid() in fill_list() is?
>> If memremap_pages() succeeded, the pages were mapped at the returned
>> vaddr, there's no need for an additional virt_addr_valid() check.
> The virt_addr_valid() check in fill_list() looks bogus to me as well. If
> memremap_pages() succeeds the range is guaranteed to have proper page
> table.
>
> I believe the first patch should be rather removal of the virt_addr_valid()
> check in fill_list().
Thank you for the clarification, I will send a patch to remove 
virt_addr_valid()
check in fill_list() for the non-RFC version.


>   
>>> ---
>>>   arch/arm64/include/asm/memory.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>>> index 824a365..1a35a44 100644
>>> --- a/arch/arm64/include/asm/memory.h
>>> +++ b/arch/arm64/include/asm/memory.h
>>> @@ -351,7 +351,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>>>   
>>>   #define virt_addr_valid(addr)	({					\
>>>   	__typeof__(addr) __addr = __tag_reset(addr);			\
>>> -	__is_lm_address(__addr) && pfn_is_map_memory(virt_to_pfn(__addr));	\
>>> +	__is_lm_address(__addr) && pfn_valid(virt_to_pfn(__addr));	\
>>>   })
>> pfn_valid() only guarantees the presence of a struct page but not
>> necessarily that the virtual address is accessible (valid). So this
>> change would break the NOMAP ranges case.
> +1
Oh, I got it.

-- 
Regards,

Oleksandr Tyshchenko


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel