Re: [PATCHv3 2/2] x86/selftests: Add test for mapping placement for 5-level paging

From: Kirill A. Shutemov
Date: Wed Nov 22 2017 - 07:40:27 EST

Next message: Zhang, Rui: "RE: [RFC PATCH v6 2/3] ACPI / EC: Add event detection support for noirq stages"
Previous message: Sergey Senozhatsky: "Re: kernel CI: printk loglevels in kernel boot logs?"
In reply to: Aneesh Kumar K.V: "Re: [PATCHv3 2/2] x86/selftests: Add test for mapping placement for 5-level paging"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Nov 22, 2017 at 05:06:27PM +0530, Aneesh Kumar K.V wrote:
> "Kirill A. Shutemov" <kirill@xxxxxxxxxxxxx> writes:
>
> > On Wed, Nov 22, 2017 at 11:11:36AM +0530, Aneesh Kumar K.V wrote:
> >> "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx> writes:
> >>
> >> > With 5-level paging, we have 56-bit virtual address space available for
> >> > userspace. But we don't want to expose userspace to addresses above
> >> > 47-bits, unless it asked specifically for it.
> >> >
> >> > We use mmap(2) hint address as a way for kernel to know if it's okay to
> >> > allocate virtual memory above 47-bit.
> >> >
> >> > Let's add a self-test that covers few corner cases of the interface.
> >> >
> >> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> >>
> >> Can we move this to selftest/vm/ ? I had a variant which i was using to
> >> test issues on ppc64. One change we did recently was to use >=128TB as
> >> the hint addr value to select larger address space. I also would like to
> >> check for exact mmap return addr in some case. Attaching below the test
> >> i was using. I will check whether this patch can be updated to test what
> >> is converted in my selftest. I also want to do the boundary check twice.
> >> The hash trasnslation mode in POWER require us to track addr limit and
> >> we had bugs around address space slection before and after updating the
> >> addr limit.
> >
> > Feel free to move it to selftest/vm. I don't have time to test setup and
> > test it on Power myself, but this would be great.
> >
>
> How about the below? Do you want me to send this as a patch to the list?

Yes, please. It actually triggered one hugetlb bug I made (patch sent).

Some feedback below.

> #include <stdio.h>
> #include <sys/mman.h>
> #include <string.h>
>
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>
> #ifdef __powerpc64__
> #define PAGE_SIZE 64*1024
> /*
> * This will work with 16M and 2M hugepage size
> */
> #define HUGETLB_SIZE 16*1024*1024
> #else
> #define PAGE_SIZE 4096
> #define HUGETLB_SIZE 2*1024*1024
> #endif
>
> /*
> * >= 128TB is the hint addr value we used to select
> * large address space.
> */
> #define ADDR_SWITCH_HINT (1UL << 47)
> #define LOW_ADDR ((void *) (1UL << 30))
> #define HIGH_ADDR ((void *) (1UL << 48))
>
> struct testcase {
> void *addr;
> unsigned long size;
> unsigned long flags;
> const char *msg;
> unsigned int addr_check_cond;

Unused?

> unsigned int low_addr_required:1;
> unsigned int keep_mapped:1;
> };
>
> static struct testcase testcases[] = {
> {
> /*
> * If stack is moved, we could possibly allocate
> * this at the requested address.
> */
> .addr = ((void *)(ADDR_SWITCH_HINT - PAGE_SIZE)),
> .size = PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(ADDR_SWITCH_HINT - PAGE_SIZE, PAGE_SIZE)",
> .low_addr_required = 1,
> },
> {
> /*
> * We should never allocate at the requested address or above it
> * The len cross the 128TB boundary. Without MAP_FIXED
> * we will always search in the lower address space.
> */
> .addr = ((void *)(ADDR_SWITCH_HINT - PAGE_SIZE)),
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(ADDR_SWITCH_HINT - PAGE_SIZE, (2 * PAGE_SIZE))",
> .low_addr_required = 1,
> },
> {
> /*
> * Exact mapping at 128TB, the area is free we should get that
> * even without MAP_FIXED.
> */
> .addr = ((void *)(ADDR_SWITCH_HINT)),
> .size = PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(ADDR_SWITCH_HINT, PAGE_SIZE)",
> .keep_mapped = 1,
> },
> {
> .addr = (void *)(ADDR_SWITCH_HINT),
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
> .msg = "mmap(ADDR_SWITCH_HINT, 2 * PAGE_SIZE, MAP_FIXED)",
> },
> {
> .addr = NULL,
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(NULL)",
> .low_addr_required = 1,
> },
> {
> .addr = LOW_ADDR,
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(LOW_ADDR)",
> .low_addr_required = 1,
> },
> {
> .addr = HIGH_ADDR,
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(HIGH_ADDR)",
> .keep_mapped = 1,
> },
> {
> .addr = HIGH_ADDR,
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(HIGH_ADDR) again",
> .keep_mapped = 1,
> },
> {
> .addr = HIGH_ADDR,
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
> .msg = "mmap(HIGH_ADDR, MAP_FIXED)",
> },
> {
> .addr = (void*) -1,
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(-1)",
> .keep_mapped = 1,
> },
> {
> .addr = (void*) -1,
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(-1) again",
> },
> {
> .addr = ((void *)(ADDR_SWITCH_HINT - PAGE_SIZE)),
> .size = PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(ADDR_SWITCH_HINT - PAGE_SIZE, PAGE_SIZE)",
> .low_addr_required = 1,
> },
> {
> .addr = (void *)(ADDR_SWITCH_HINT - PAGE_SIZE),
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(ADDR_SWITCH_HINT - PAGE_SIZE, 2 * PAGE_SIZE)",
> .low_addr_required = 1,
> .keep_mapped = 1,
> },
> {
> .addr = (void *)(ADDR_SWITCH_HINT - PAGE_SIZE / 2),
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(ADDR_SWITCH_HINT - PAGE_SIZE/2 , 2 * PAGE_SIZE)",
> .low_addr_required = 1,
> .keep_mapped = 1,
> },
> {
> .addr = ((void *)(ADDR_SWITCH_HINT)),
> .size = PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(ADDR_SWITCH_HINT, PAGE_SIZE)",
> },
> {
> .addr = (void *)(ADDR_SWITCH_HINT),
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
> .msg = "mmap(ADDR_SWITCH_HINT, 2 * PAGE_SIZE, MAP_FIXED)",
> },
> };
>
> static struct testcase hugetlb_testcases[] = {
> {
> .addr = NULL,
> .size = HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(NULL, MAP_HUGETLB)",
> .low_addr_required = 1,
> },
> {
> .addr = LOW_ADDR,
> .size = HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(LOW_ADDR, MAP_HUGETLB)",
> .low_addr_required = 1,
> },
> {
> .addr = HIGH_ADDR,
> .size = HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(HIGH_ADDR, MAP_HUGETLB)",
> .keep_mapped = 1,
> },
> {
> .addr = HIGH_ADDR,
> .size = HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(HIGH_ADDR, MAP_HUGETLB) again",
> .keep_mapped = 1,
> },
> {
> .addr = HIGH_ADDR,
> .size = HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
> .msg = "mmap(HIGH_ADDR, MAP_FIXED | MAP_HUGETLB)",
> },
> {
> .addr = (void*) -1,
> .size = HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(-1, MAP_HUGETLB)",
> .keep_mapped = 1,
> },
> {
> .addr = (void*) -1,
> .size = HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(-1, MAP_HUGETLB) again",
> },
> {
> .addr = (void *)(ADDR_SWITCH_HINT - PAGE_SIZE),
> .size = 2 * HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap((1UL << 47), 4UL << 20, MAP_HUGETLB)",
> .low_addr_required = 1,
> .keep_mapped = 1,
> },
> {
> .addr = (void *)(ADDR_SWITCH_HINT),
> .size = 2 * HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
> .msg = "mmap(ADDR_SWITCH_HINT , 2 * HUGETLB_SIZE, MAP_FIXED | MAP_HUGETLB)",
> },
> };
>
> static void run_test(struct testcase *test, int count)
> {
> int i;
> void *p;
>
> for (i = 0; i < count; i++) {
> struct testcase *t = test + i;
>
> p = mmap(t->addr, t->size, PROT_READ | PROT_WRITE, t->flags, -1, 0);
>
> printf("%s: %p - ", t->msg, p);
>
> if (p == MAP_FAILED) {
> printf("FAILED\n");
> continue;
> }
>
> if (t->low_addr_required && p >= (void *)(1UL << 47))
> printf("FAILED\n");
> else {
> /*
> * Do a dereference of the address returned so that we catch
> * bugs in page fault handling
> */
> *(int *)p = 10;
> printf("OK\n");
> }
> if (!t->keep_mapped)
> munmap(p, t->size);
> }
> }
>
> static int supported_arch(void)
> {
> #if defined(__powerpc64__)
> return 1;
> #elif defined(__x86_64__)
> return 1;
> #else
> return 0;
> #endif
> }
>
> int main(int argc, char **argv)
> {
> if (!supported_arch())
> return 0;
>
> run_test(testcases, ARRAY_SIZE(testcases));
> if (argc == 2 && !strcmp(argv[1], "--run_hugetlb"))

--run-hugetlb would be better I think.

> run_test(hugetlb_testcases, ARRAY_SIZE(hugetlb_testcases));
> return 0;
> }
>

--
Kirill A. Shutemov

Next message: Zhang, Rui: "RE: [RFC PATCH v6 2/3] ACPI / EC: Add event detection support for noirq stages"
Previous message: Sergey Senozhatsky: "Re: kernel CI: printk loglevels in kernel boot logs?"
In reply to: Aneesh Kumar K.V: "Re: [PATCHv3 2/2] x86/selftests: Add test for mapping placement for 5-level paging"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]