Re: [RFC] powerpc/mm: Add validation for platform reserved memory ranges
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-03-02 12:10:55
On Wed, 2016-02-03 at 08:46:12 UTC, 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 inside 'setup_system' 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.
Good plan.
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h index 3d5abfe..95257c1 100644 diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index 5c03a6a..04bc592 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c@@ -546,6 +546,8 @@ void __init setup_system(void) smp_release_cpus(); #endif + validate_reserved_va_range(); +
I don't see why this can't just be an initcall in hash_utils_64.c, rather than being called from here.
quoted hunk ↗ jump to hunk
pr_info("Starting Linux %s %s\n", init_utsname()->machine, init_utsname()->version);diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c index ba59d59..03adafc 100644 --- a/arch/powerpc/mm/hash_utils_64.c +++ b/arch/powerpc/mm/hash_utils_64.c@@ -810,6 +810,57 @@ void __init early_init_mmu(void) slb_initialize(); } +/* + * PAPR says that each record contains 3 * 32 bit element, hence 12 bytes. + * First two element contains the abbreviated virtual address (high order + * 32 bits and low order 32 bits generates the abbreviated virtual address + * of 64 bits which need to be concatenated with 12 bits of 0 at the end + * to generate the actual 76 bit reserved virtual address) and size of the + * reserved virtual address range is encoded in next 32 bit element as number + * of 4K pages. + */ +#define BYTES_PER_RVA_RECORD 12
Please define a properly endian-annotated struct which encodes the layout. It can be local to the function if that works.
+/* + * Linux uses 65 bits (MAX_PHYSMEM_BITS + CONTEXT_BITS) from available 78 + * bit wide virtual address range. As reserved virtual address range comes + * as an abbreviated form of 64 bits, we will use a partial address mask + * (65 bit mask >> 12) to match it for simplicity. + */ +#define PARTIAL_USED_VA_MASK 0x1FFFFFFFFFFFFFULL
Please calculate this from the appropriate constants. We don't want to have to update it in future.
+void __init validate_reserved_va_range(void)
+{
+ struct device_node *np;
+ struct property *prop;
+ unsigned int size, count, i;
+ const __be32 *value;
+ __be64 vaddr;
+
+ np = of_find_node_by_name(NULL, "vdevice");
+ if (!np)
+ return;
+
+ prop = of_find_property(np, "ibm,reserved-virtual-addresses", NULL);
+ if (!prop)
+ return;You don't need to do a find, the get below will do it for you.
+ value = of_get_property(np, "ibm,reserved-virtual-addresses", &size);
+ if (!value)
+ return;
+
+ count = size / BYTES_PER_RVA_RECORD;
+ for (i = 0; i < count; i++) {
+ vaddr = ((__be64) value[i * 3] << 32) | value[i * 3 + 1];
+ if (vaddr & ~PARTIAL_USED_VA_MASK) {How can it work to test a __be64 against a non-byte-swapped mask ? But like I said above, please do this with a struct and proper endian conversions.
+ pr_info("Reserved virtual address range starting "
+ "at [%llx000] verified for overlap\n", vaddr);This should print nothing in the success case.
+ continue;
+ }
+ BUG_ON("Reserved virtual address range overlapping");But here you should provide more detail. The first thing a debugger will need to know is what address overlapped. cheers