[PATCH V2] powerpc/mm: Add validation for platform reserved memory ranges

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE3673d

5 messages, 2 authors, 2016-07-14 · open the first message on its own page

[PATCH V2] powerpc/mm: Add validation for platform reserved memory ranges

From: Anshuman Khandual <hidden>
Date: 2016-03-07 09:56:35

For partition running on PHYP, there can be a adjunct partition
which shares the virtual address range with the operating system.
Virtual address ranges which can be used by the adjunct partition
are communicated with virtual device node of the device tree with
a property known as "ibm,reserved-virtual-addresses". This patch
introduces a new function named 'validate_reserved_va_range' which
is called  during initialization to validate that these reserved
virtual address ranges do not overlap with the address ranges used
by the kernel for all supported memory contexts. This helps prevent
the possibility of getting return codes similar to H_RESOURCE for
H_PROTECT hcalls for conflicting HPTE entries.

Signed-off-by: Anshuman Khandual <redacted>
---
- Tested on both POWER8 LE and BE platforms

Changes in V2:
- Added braces to the definition of LINUX_VA_BITS
- Adjusted tabs as spaces for the definition of PARTIAL_LINUX_VA_MASK

 arch/powerpc/mm/hash_utils_64.c | 77 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index ba59d59..b47f667 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -1564,3 +1564,80 @@ void setup_initial_memory_limit(phys_addr_t first_memblock_base,
 	/* Finally limit subsequent allocations */
 	memblock_set_current_limit(ppc64_rma_size);
 }
+
+/*
+ * PAPR says that each reserved virtual address range record
+ * contains three be32 elements which is of toal 12 bytes.
+ * First two be32 elements contain the abbreviated virtual
+ * address (high order 32 bits and low order 32 bits that
+ * generate the abbreviated virtual address of 64 bits which
+ * need to be concatenated with 24 bits of 0 at the end) and
+ * the third be32 element contains the size of the reserved
+ * virtual address range as number of consecutive 4K pages.
+ */
+struct reserved_va_record {
+	__be32	high_addr;
+	__be32	low_addr;
+	__be32	nr_pages_4K;
+};
+
+/*
+ * Linux uses 65 bits (CONTEXT_BITS + ESID_BITS + SID_SHIFT)
+ * of virtual address. As reserved virtual address comes in
+ * as an abbreviated form (64 bits) from the device tree, we
+ * will use a partial address bit mask (65 >> 24) to match it
+ * for simplicity.
+ */
+#define RVA_LESS_BITS		24
+#define LINUX_VA_BITS		(CONTEXT_BITS + ESID_BITS + SID_SHIFT)
+#define PARTIAL_LINUX_VA_MASK	((1ULL << (LINUX_VA_BITS - RVA_LESS_BITS)) - 1)
+
+static int __init validate_reserved_va_range(void)
+{
+	struct reserved_va_record rva;
+	struct device_node *np;
+	int records, ret, i;
+	__be64 vaddr;
+
+	np = of_find_node_by_name(NULL, "vdevice");
+	if (!np)
+		return -ENODEV;
+
+	records = of_property_count_elems_of_size(np,
+			"ibm,reserved-virtual-addresses",
+				sizeof(struct reserved_va_record));
+	if (records < 0)
+		return records;
+
+	for (i = 0; i < records; i++) {
+		ret = of_property_read_u32_index(np,
+			"ibm,reserved-virtual-addresses",
+				3 * i, &rva.high_addr);
+		if (ret)
+			return ret;
+
+		ret = of_property_read_u32_index(np,
+			"ibm,reserved-virtual-addresses",
+				3 * i + 1, &rva.low_addr);
+		if (ret)
+			return ret;
+
+		ret = of_property_read_u32_index(np,
+			"ibm,reserved-virtual-addresses",
+				3 * i + 2, &rva.nr_pages_4K);
+		if (ret)
+			return ret;
+
+		vaddr =  rva.high_addr;
+		vaddr =  (vaddr << 32) | rva.low_addr;
+		if (vaddr & cpu_to_be64(~PARTIAL_LINUX_VA_MASK))
+			continue;
+
+		pr_err("RVA [0x%llx000000 (0x%x in bytes)] overlapped\n",
+					vaddr, rva.nr_pages_4K * 4096);
+		BUG();
+	}
+	of_node_put(np);
+	return 0;
+}
+__initcall(validate_reserved_va_range);
-- 
2.1.0

Re: [PATCH V2] powerpc/mm: Add validation for platform reserved memory ranges

From: Anshuman Khandual <hidden>
Date: 2016-04-11 06:14:35

On 03/07/2016 03:25 PM, Anshuman Khandual wrote:
For partition running on PHYP, there can be a adjunct partition
which shares the virtual address range with the operating system.
Virtual address ranges which can be used by the adjunct partition
are communicated with virtual device node of the device tree with
a property known as "ibm,reserved-virtual-addresses". This patch
introduces a new function named 'validate_reserved_va_range' which
is called  during initialization to validate that these reserved
virtual address ranges do not overlap with the address ranges used
by the kernel for all supported memory contexts. This helps prevent
the possibility of getting return codes similar to H_RESOURCE for
H_PROTECT hcalls for conflicting HPTE entries.

Signed-off-by: Anshuman Khandual <redacted>
---
- Tested on both POWER8 LE and BE platforms

Changes in V2:
- Added braces to the definition of LINUX_VA_BITS
- Adjusted tabs as spaces for the definition of PARTIAL_LINUX_VA_MASK
mpe, any updates on this ?

Re: [PATCH V2] powerpc/mm: Add validation for platform reserved memory ranges

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-04-11 13:32:04

On Mon, 2016-04-11 at 11:43 +0530, Anshuman Khandual wrote:
On 03/07/2016 03:25 PM, Anshuman Khandual wrote:
quoted
For partition running on PHYP, there can be a adjunct partition
which shares the virtual address range with the operating system.
Virtual address ranges which can be used by the adjunct partition
are communicated with virtual device node of the device tree with
a property known as "ibm,reserved-virtual-addresses". This patch
introduces a new function named 'validate_reserved_va_range' which
is called  during initialization to validate that these reserved
virtual address ranges do not overlap with the address ranges used
by the kernel for all supported memory contexts. This helps prevent
the possibility of getting return codes similar to H_RESOURCE for
H_PROTECT hcalls for conflicting HPTE entries.

Signed-off-by: Anshuman Khandual <redacted>
---
- Tested on both POWER8 LE and BE platforms

Changes in V2:
- Added braces to the definition of LINUX_VA_BITS
- Adjusted tabs as spaces for the definition of PARTIAL_LINUX_VA_MASK
mpe, any updates on this ?
I'm just starting to look at things for next. So will try and get to it this
week.

cheers

Re: [PATCH V2] powerpc/mm: Add validation for platform reserved memory ranges

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-07-05 01:55:34

Anshuman Khandual [off-list ref] writes:
For partition running on PHYP, there can be a adjunct partition
which shares the virtual address range with the operating system.
Virtual address ranges which can be used by the adjunct partition
are communicated with virtual device node of the device tree with
a property known as "ibm,reserved-virtual-addresses". This patch
introduces a new function named 'validate_reserved_va_range' which
is called  during initialization to validate that these reserved
virtual address ranges do not overlap with the address ranges used
by the kernel for all supported memory contexts. This helps prevent
the possibility of getting return codes similar to H_RESOURCE for
H_PROTECT hcalls for conflicting HPTE entries.
Have you tested this? The endian conversions look wrong to me.
quoted hunk
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index ba59d59..b47f667 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -1564,3 +1564,80 @@ void setup_initial_memory_limit(phys_addr_t first_memblock_base,
 	/* Finally limit subsequent allocations */
 	memblock_set_current_limit(ppc64_rma_size);
 }
+
+/*
+ * PAPR says that each reserved virtual address range record
+ * contains three be32 elements which is of toal 12 bytes.
+ * First two be32 elements contain the abbreviated virtual
+ * address (high order 32 bits and low order 32 bits that
+ * generate the abbreviated virtual address of 64 bits which
+ * need to be concatenated with 24 bits of 0 at the end) and
+ * the third be32 element contains the size of the reserved
+ * virtual address range as number of consecutive 4K pages.
+ */
+struct reserved_va_record {
+	__be32	high_addr;
+	__be32	low_addr;
+	__be32	nr_pages_4K;
+};
Here you define those fields as __be32.
+/*
+ * Linux uses 65 bits (CONTEXT_BITS + ESID_BITS + SID_SHIFT)
+ * of virtual address. As reserved virtual address comes in
+ * as an abbreviated form (64 bits) from the device tree, we
+ * will use a partial address bit mask (65 >> 24) to match it
+ * for simplicity.
+ */
+#define RVA_LESS_BITS		24
+#define LINUX_VA_BITS		(CONTEXT_BITS + ESID_BITS + SID_SHIFT)
+#define PARTIAL_LINUX_VA_MASK	((1ULL << (LINUX_VA_BITS - RVA_LESS_BITS)) - 1)
+
+static int __init validate_reserved_va_range(void)
+{
+	struct reserved_va_record rva;
+	struct device_node *np;
+	int records, ret, i;
+	__be64 vaddr;
+
+	np = of_find_node_by_name(NULL, "vdevice");
+	if (!np)
+		return -ENODEV;
+
+	records = of_property_count_elems_of_size(np,
+			"ibm,reserved-virtual-addresses",
+				sizeof(struct reserved_va_record));
+	if (records < 0)
+		return records;
+
+	for (i = 0; i < records; i++) {
+		ret = of_property_read_u32_index(np,
+			"ibm,reserved-virtual-addresses",
+				3 * i, &rva.high_addr);
But then here you use of_property_read_u32_index(), which does the
endian conversion (to CPU endian) for you.
+		ret = of_property_read_u32_index(np,
+			"ibm,reserved-virtual-addresses",
+				3 * i + 1, &rva.low_addr);
+		ret = of_property_read_u32_index(np,
+			"ibm,reserved-virtual-addresses",
+				3 * i + 2, &rva.nr_pages_4K);
So now all the values in rva are CPU endian.
+		vaddr =  rva.high_addr;
+		vaddr =  (vaddr << 32) | rva.low_addr;
+		if (vaddr & cpu_to_be64(~PARTIAL_LINUX_VA_MASK))
+			continue;
But then here you do the comparison against a __be64 value.

I know I told you to use "properly endian-annotated struct", but you
stil need to use the right conversions in the right places.

I think the best option is to use of_property_read_u32_array() and just
read the three 32 values into a CPU endian struct.

cheers

Re: [PATCH V2] powerpc/mm: Add validation for platform reserved memory ranges

From: Anshuman Khandual <hidden>
Date: 2016-07-14 03:29:17

On 07/05/2016 07:25 AM, Michael Ellerman wrote:
Anshuman Khandual [off-list ref] writes:
quoted
For partition running on PHYP, there can be a adjunct partition
which shares the virtual address range with the operating system.
Virtual address ranges which can be used by the adjunct partition
are communicated with virtual device node of the device tree with
a property known as "ibm,reserved-virtual-addresses". This patch
introduces a new function named 'validate_reserved_va_range' which
is called  during initialization to validate that these reserved
virtual address ranges do not overlap with the address ranges used
by the kernel for all supported memory contexts. This helps prevent
the possibility of getting return codes similar to H_RESOURCE for
H_PROTECT hcalls for conflicting HPTE entries.
Have you tested this? The endian conversions look wrong to me.
I had tested this both on LE and BE LPARs on PVM environment.
quoted
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index ba59d59..b47f667 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -1564,3 +1564,80 @@ void setup_initial_memory_limit(phys_addr_t first_memblock_base,
 	/* Finally limit subsequent allocations */
 	memblock_set_current_limit(ppc64_rma_size);
 }
+
+/*
+ * PAPR says that each reserved virtual address range record
+ * contains three be32 elements which is of toal 12 bytes.
+ * First two be32 elements contain the abbreviated virtual
+ * address (high order 32 bits and low order 32 bits that
+ * generate the abbreviated virtual address of 64 bits which
+ * need to be concatenated with 24 bits of 0 at the end) and
+ * the third be32 element contains the size of the reserved
+ * virtual address range as number of consecutive 4K pages.
+ */
+struct reserved_va_record {
+	__be32	high_addr;
+	__be32	low_addr;
+	__be32	nr_pages_4K;
+};
Here you define those fields as __be32.
Hmm, I believe we had agreed upon this. Will check back.
quoted
+/*
+ * Linux uses 65 bits (CONTEXT_BITS + ESID_BITS + SID_SHIFT)
+ * of virtual address. As reserved virtual address comes in
+ * as an abbreviated form (64 bits) from the device tree, we
+ * will use a partial address bit mask (65 >> 24) to match it
+ * for simplicity.
+ */
+#define RVA_LESS_BITS		24
+#define LINUX_VA_BITS		(CONTEXT_BITS + ESID_BITS + SID_SHIFT)
+#define PARTIAL_LINUX_VA_MASK	((1ULL << (LINUX_VA_BITS - RVA_LESS_BITS)) - 1)
+
+static int __init validate_reserved_va_range(void)
+{
+	struct reserved_va_record rva;
+	struct device_node *np;
+	int records, ret, i;
+	__be64 vaddr;
+
+	np = of_find_node_by_name(NULL, "vdevice");
+	if (!np)
+		return -ENODEV;
+
+	records = of_property_count_elems_of_size(np,
+			"ibm,reserved-virtual-addresses",
+				sizeof(struct reserved_va_record));
+	if (records < 0)
+		return records;
+
+	for (i = 0; i < records; i++) {
+		ret = of_property_read_u32_index(np,
+			"ibm,reserved-virtual-addresses",
+				3 * i, &rva.high_addr);
But then here you use of_property_read_u32_index(), which does the
endian conversion (to CPU endian) for you.
Okay.
quoted
+		ret = of_property_read_u32_index(np,
+			"ibm,reserved-virtual-addresses",
+				3 * i + 1, &rva.low_addr);
quoted
+		ret = of_property_read_u32_index(np,
+			"ibm,reserved-virtual-addresses",
+				3 * i + 2, &rva.nr_pages_4K);
So now all the values in rva are CPU endian.
Okay.
quoted
+		vaddr =  rva.high_addr;
+		vaddr =  (vaddr << 32) | rva.low_addr;
+		if (vaddr & cpu_to_be64(~PARTIAL_LINUX_VA_MASK))
+			continue;
But then here you do the comparison against a __be64 value.

I know I told you to use "properly endian-annotated struct", but you
stil need to use the right conversions in the right places.

I think the best option is to use of_property_read_u32_array() and just
read the three 32 values into a CPU endian struct.
Sure. But I have kind of lost context of this patch, will look into these
details and get back.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help