| Message ID | 20240923062820.319308-2-nrb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: add tests for diag258 | expand |
On Mon, 23 Sep 2024 08:26:03 +0200 Nico Boehr <nrb@linux.ibm.com> wrote: > This adds a test for diag258 (page ref service/async page fault). > > There recently was a virtual-real address confusion bug, so we should > test: > - diag258 parameter Rx is a real adress > - crossing the end of RAM with the parameter list yields an addressing > exception > - invalid diagcode in the parameter block yields an specification > exception > - diag258 correctly applies prefixing. > > Note that we're just testing error cases as of now. > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> > --- > s390x/Makefile | 1 + > s390x/diag258.c | 258 ++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 3 + > 3 files changed, 262 insertions(+) > create mode 100644 s390x/diag258.c > > diff --git a/s390x/Makefile b/s390x/Makefile > index 23342bd64f44..66d71351caab 100644 > --- a/s390x/Makefile > +++ b/s390x/Makefile > @@ -44,6 +44,7 @@ tests += $(TEST_DIR)/exittime.elf > tests += $(TEST_DIR)/ex.elf > tests += $(TEST_DIR)/topology.elf > tests += $(TEST_DIR)/sie-dat.elf > +tests += $(TEST_DIR)/diag258.elf > > pv-tests += $(TEST_DIR)/pv-diags.elf > pv-tests += $(TEST_DIR)/pv-icptcode.elf > diff --git a/s390x/diag258.c b/s390x/diag258.c > new file mode 100644 > index 000000000000..5337534f60d1 > --- /dev/null > +++ b/s390x/diag258.c > @@ -0,0 +1,258 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Diag 258: Async Page Fault Handler > + * > + * Copyright (c) 2024 IBM Corp > + * > + * Authors: > + * Nico Boehr <nrb@linux.ibm.com> > + */ > + > +#include <libcflat.h> > +#include <asm-generic/barrier.h> > +#include <asm/asm-offsets.h> > +#include <asm/interrupt.h> > +#include <asm/mem.h> > +#include <asm/pgtable.h> > +#include <mmu.h> > +#include <sclp.h> > +#include <vmalloc.h> > + > +static uint8_t prefix_buf[LC_SIZE] __attribute__((aligned(LC_SIZE))); wait, you are using LC_SIZE in this file... even though it's only defined in the next patch. I think you should swap patch 1 and 2 > + > +#define __PF_RES_FIELD 0x8000000000000000UL > + > +/* copied from Linux arch/s390/mm/pfault.c */ > +struct pfault_refbk { > + u16 refdiagc; > + u16 reffcode; > + u16 refdwlen; > + u16 refversn; > + u64 refgaddr; > + u64 refselmk; > + u64 refcmpmk; > + u64 reserved; > +}; > + > +uint64_t pfault_token = 0x0123fadec0fe3210UL; > + > +static struct pfault_refbk pfault_init_refbk __attribute__((aligned(8))) = { > + .refdiagc = 0x258, > + .reffcode = 0, /* TOKEN */ > + .refdwlen = sizeof(struct pfault_refbk) / sizeof(uint64_t), > + .refversn = 2, > + .refgaddr = (u64)&pfault_token, > + .refselmk = 1UL << 48, > + .refcmpmk = 1UL << 48, > + .reserved = __PF_RES_FIELD > +}; > + > +static struct pfault_refbk pfault_cancel_refbk __attribute((aligned(8))) = { > + .refdiagc = 0x258, > + .reffcode = 1, /* CANCEL */ > + .refdwlen = sizeof(struct pfault_refbk) / sizeof(uint64_t), > + .refversn = 2, > + .refgaddr = 0, > + .refselmk = 0, > + .refcmpmk = 0, > + .reserved = 0 > +}; > + > +static inline int diag258(struct pfault_refbk *refbk) > +{ > + int rc = -1; > + > + asm volatile( > + " diag %[refbk],%[rc],0x258\n" > + : [rc] "+d" (rc) > + : [refbk] "a" (refbk), "m" (*(refbk)) > + : "cc"); > + return rc; > +} > + > +static void test_priv(void) > +{ > + report_prefix_push("privileged"); > + expect_pgm_int(); > + enter_pstate(); > + diag258(&pfault_init_refbk); > + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); > + report_prefix_pop(); > +} > + > +static void* page_map_outside_real_space(phys_addr_t page_real) > +{ > + pgd_t *root = (pgd_t *)(stctg(1) & PAGE_MASK); > + void* vaddr = alloc_vpage(); > + > + install_page(root, page_real, vaddr); > + > + return vaddr; > +} > + > +/* > + * Verify that the refbk pointer is a real address and not a virtual > + * address. This is tested by enabling DAT and establishing a mapping > + * for the refbk that is outside of the bounds of our (guest-)physical s/physical/real/ (or absolute) > + * address space. > + */ > +static void test_refbk_real(void) > +{ > + pgd_t *root; > + struct pfault_refbk *refbk; > + void *refbk_page; please reverse Christmas tree > + > + report_prefix_push("refbk is real"); > + > + /* Set up virtual memory and allocate a physical page for storing the refbk */ > + setup_vm(); > + refbk_page = alloc_page(); > + > + /* Map refblk page outside of physical memory identity mapping */ > + root = (pgd_t *)(stctg(1) & PAGE_MASK); > + refbk = page_map_outside_real_space(virt_to_pte_phys(root, refbk_page)); > + > + /* Assert the mapping really is outside identity mapping */ > + report_info("refbk is at 0x%lx", (u64)refbk); > + report_info("ram size is 0x%lx", get_ram_size()); > + assert((u64)refbk > get_ram_size()); > + > + /* Copy the init refbk to the page */ > + memcpy(refbk, &pfault_init_refbk, sizeof(struct pfault_refbk)); > + > + /* Protect the virtual mapping to avoid diag258 actually doing something */ > + protect_page(refbk, PAGE_ENTRY_I); > + > + expect_pgm_int(); > + diag258(refbk); > + check_pgm_int_code(PGM_INT_CODE_ADDRESSING); > + report_prefix_pop(); > + > + free_page(refbk_page); > + disable_dat(); > + irq_set_dat_mode(false, 0); > +} > + > +/* > + * Verify diag258 correctly applies prefixing. > + */ > +static void test_refbk_prefixing(void) > +{ > + uint64_t ry; > + uint32_t old_prefix; > + struct pfault_refbk *refbk_in_prefix, *refbk_in_reverse_prefix; > + const size_t lowcore_offset_for_refbk = offsetof(struct lowcore, pad_0x03a0); reverse Christmas tree here too > + > + report_prefix_push("refbk prefixing"); > + > + report_info("refbk at lowcore offset 0x%lx", lowcore_offset_for_refbk); > + > + assert((unsigned long)&prefix_buf < SZ_2G); > + > + memcpy(prefix_buf, 0, LC_SIZE); > + > + /* > + * After the call to set_prefix() below, this will refer to absolute > + * address lowcore_offset_for_refbk (reverse prefixing). > + */ > + refbk_in_reverse_prefix = (struct pfault_refbk *)(&prefix_buf[0] + lowcore_offset_for_refbk); > + > + /* > + * After the call to set_prefix() below, this will refer to absolute > + * address &prefix_buf[0] + lowcore_offset_for_refbk (forward prefixing). > + */ > + refbk_in_prefix = (struct pfault_refbk *)OPAQUE_PTR(lowcore_offset_for_refbk); > + > + old_prefix = get_prefix(); > + set_prefix((uint32_t)(uintptr_t)prefix_buf); > + > + /* > + * If diag258 would not be applying prefixing on access to > + * refbk_in_reverse_prefix correctly, it would access absolute address > + * refbk_in_reverse_prefix (which to us is accessible at real address > + * refbk_in_prefix). > + * Make sure it really fails by putting invalid function code > + * at refbk_in_prefix. > + */ > + refbk_in_prefix->refdiagc = 0xc0fe; > + > + /* > + * Put a valid refbk at refbk_in_reverse_prefix. > + */ > + memcpy(refbk_in_reverse_prefix, &pfault_init_refbk, sizeof(pfault_init_refbk)); > + > + ry = diag258(refbk_in_reverse_prefix); > + report(!ry, "real address refbk accessed"); > + > + /* > + * Activating should have worked. Cancel the activation and expect > + * return 0. If activation would not have worked, this should return with > + * 4 (pfault handshaking not active). > + */ > + ry = diag258(&pfault_cancel_refbk); > + report(!ry, "handshaking canceled"); > + > + set_prefix(old_prefix); > + > + report_prefix_pop(); > +} it seems like you are only testing the first page of lowcore; can you expand the test to also test the second page? > + > +/* > + * Verify that a refbk exceeding physical memory is not accepted, even > + * when crossing a frame boundary. > + */ > +static void test_refbk_crossing(void) > +{ > + const size_t bytes_in_last_page = 8; are there any alignment requirements for the buffer? if so, that should also be tested (either that a fault is triggered or that the lowest bytes are ignored, depending on how it is defined to work) > + struct pfault_refbk *refbk = (struct pfault_refbk *)(get_ram_size() - bytes_in_last_page); > + > + report_prefix_push("refbk crossing"); > + > + report_info("refbk is at 0x%lx", (u64)refbk); > + report_info("ram size is 0x%lx", get_ram_size()); > + assert(sizeof(struct pfault_refbk) > bytes_in_last_page); > + > + /* Copy bytes_in_last_page bytes of the init refbk to the page */ > + memcpy(refbk, &pfault_init_refbk, bytes_in_last_page); > + > + expect_pgm_int(); > + diag258(refbk); > + check_pgm_int_code(PGM_INT_CODE_ADDRESSING); > + report_prefix_pop(); > +} > + > +/* > + * Verify that a refbk with an invalid refdiagc is not accepted. > + */ > +static void test_refbk_invalid_diagcode(void) > +{ > + struct pfault_refbk refbk = pfault_init_refbk; > + > + report_prefix_push("invalid refdiagc"); > + refbk.refdiagc = 0xc0fe; other testcases in this file depend on invalid codes failing; maybe move this test up? > + > + expect_pgm_int(); > + diag258(&refbk); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > +} > + > +int main(void) > +{ > + report_prefix_push("diag258"); > + > + expect_pgm_int(); > + diag258(&pfault_init_refbk); > + if (clear_pgm_int() == PGM_INT_CODE_SPECIFICATION) { > + report_skip("diag258 not supported"); > + } else { > + test_priv(); > + test_refbk_real(); should probably go here.... > + test_refbk_prefixing(); > + test_refbk_crossing(); > + test_refbk_invalid_diagcode(); .... this one ^ > + } > + > + report_prefix_pop(); > + return report_summary(); > +} > diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg > index 3a9decc932f2..8131ba105d3f 100644 > --- a/s390x/unittests.cfg > +++ b/s390x/unittests.cfg > @@ -392,3 +392,6 @@ file = sie-dat.elf > > [pv-attest] > file = pv-attest.elf > + > +[diag258] > +file = diag258.elf
Quoting Claudio Imbrenda (2024-09-25 17:34:52) > > +static uint8_t prefix_buf[LC_SIZE] __attribute__((aligned(LC_SIZE))); > > wait, you are using LC_SIZE in this file... even though it's only > defined in the next patch. > > I think you should swap patch 1 and 2 will do [...] > > +/* > > + * Verify that the refbk pointer is a real address and not a virtual > > + * address. This is tested by enabling DAT and establishing a mapping > > + * for the refbk that is outside of the bounds of our (guest-)physical > > s/physical/real/ (or absolute) Huh? Why? Talking about the size here, so absolute, real and physical are all equivalent here. [...] > > + /* > > + * Put a valid refbk at refbk_in_reverse_prefix. > > + */ > > + memcpy(refbk_in_reverse_prefix, &pfault_init_refbk, sizeof(pfault_init_refbk)); > > + > > + ry = diag258(refbk_in_reverse_prefix); > > + report(!ry, "real address refbk accessed"); > > + > > + /* > > + * Activating should have worked. Cancel the activation and expect > > + * return 0. If activation would not have worked, this should return with > > + * 4 (pfault handshaking not active). > > + */ > > + ry = diag258(&pfault_cancel_refbk); > > + report(!ry, "handshaking canceled"); > > + > > + set_prefix(old_prefix); > > + > > + report_prefix_pop(); > > +} > > it seems like you are only testing the first page of lowcore; can you > expand the test to also test the second page? Would you mind leaving this for a future extension? > > + > > +/* > > + * Verify that a refbk exceeding physical memory is not accepted, even > > + * when crossing a frame boundary. > > + */ > > +static void test_refbk_crossing(void) > > +{ > > + const size_t bytes_in_last_page = 8; > > are there any alignment requirements for the buffer? > if so, that should also be tested (either that a fault is triggered or > that the lowest bytes are ignored, depending on how it is defined to > work) There are alignment requirements. Would it be OK to do this in a future extension? > > +/* > > + * Verify that a refbk with an invalid refdiagc is not accepted. > > + */ > > +static void test_refbk_invalid_diagcode(void) > > +{ > > + struct pfault_refbk refbk = pfault_init_refbk; > > + > > + report_prefix_push("invalid refdiagc"); > > + refbk.refdiagc = 0xc0fe; > > other testcases in this file depend on invalid codes failing; maybe > move this test up? Yes, thanks, done. > > +int main(void) > > +{ > > + report_prefix_push("diag258"); > > + > > + expect_pgm_int(); > > + diag258(&pfault_init_refbk); > > + if (clear_pgm_int() == PGM_INT_CODE_SPECIFICATION) { > > + report_skip("diag258 not supported"); > > + } else { > > + test_priv(); > > + test_refbk_real(); > > should probably go here.... test_refbk_real() relies on the invalid diagcode doing nothing, so it should go *before* that one.
diff --git a/s390x/Makefile b/s390x/Makefile index 23342bd64f44..66d71351caab 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -44,6 +44,7 @@ tests += $(TEST_DIR)/exittime.elf tests += $(TEST_DIR)/ex.elf tests += $(TEST_DIR)/topology.elf tests += $(TEST_DIR)/sie-dat.elf +tests += $(TEST_DIR)/diag258.elf pv-tests += $(TEST_DIR)/pv-diags.elf pv-tests += $(TEST_DIR)/pv-icptcode.elf diff --git a/s390x/diag258.c b/s390x/diag258.c new file mode 100644 index 000000000000..5337534f60d1 --- /dev/null +++ b/s390x/diag258.c @@ -0,0 +1,258 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Diag 258: Async Page Fault Handler + * + * Copyright (c) 2024 IBM Corp + * + * Authors: + * Nico Boehr <nrb@linux.ibm.com> + */ + +#include <libcflat.h> +#include <asm-generic/barrier.h> +#include <asm/asm-offsets.h> +#include <asm/interrupt.h> +#include <asm/mem.h> +#include <asm/pgtable.h> +#include <mmu.h> +#include <sclp.h> +#include <vmalloc.h> + +static uint8_t prefix_buf[LC_SIZE] __attribute__((aligned(LC_SIZE))); + +#define __PF_RES_FIELD 0x8000000000000000UL + +/* copied from Linux arch/s390/mm/pfault.c */ +struct pfault_refbk { + u16 refdiagc; + u16 reffcode; + u16 refdwlen; + u16 refversn; + u64 refgaddr; + u64 refselmk; + u64 refcmpmk; + u64 reserved; +}; + +uint64_t pfault_token = 0x0123fadec0fe3210UL; + +static struct pfault_refbk pfault_init_refbk __attribute__((aligned(8))) = { + .refdiagc = 0x258, + .reffcode = 0, /* TOKEN */ + .refdwlen = sizeof(struct pfault_refbk) / sizeof(uint64_t), + .refversn = 2, + .refgaddr = (u64)&pfault_token, + .refselmk = 1UL << 48, + .refcmpmk = 1UL << 48, + .reserved = __PF_RES_FIELD +}; + +static struct pfault_refbk pfault_cancel_refbk __attribute((aligned(8))) = { + .refdiagc = 0x258, + .reffcode = 1, /* CANCEL */ + .refdwlen = sizeof(struct pfault_refbk) / sizeof(uint64_t), + .refversn = 2, + .refgaddr = 0, + .refselmk = 0, + .refcmpmk = 0, + .reserved = 0 +}; + +static inline int diag258(struct pfault_refbk *refbk) +{ + int rc = -1; + + asm volatile( + " diag %[refbk],%[rc],0x258\n" + : [rc] "+d" (rc) + : [refbk] "a" (refbk), "m" (*(refbk)) + : "cc"); + return rc; +} + +static void test_priv(void) +{ + report_prefix_push("privileged"); + expect_pgm_int(); + enter_pstate(); + diag258(&pfault_init_refbk); + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); + report_prefix_pop(); +} + +static void* page_map_outside_real_space(phys_addr_t page_real) +{ + pgd_t *root = (pgd_t *)(stctg(1) & PAGE_MASK); + void* vaddr = alloc_vpage(); + + install_page(root, page_real, vaddr); + + return vaddr; +} + +/* + * Verify that the refbk pointer is a real address and not a virtual + * address. This is tested by enabling DAT and establishing a mapping + * for the refbk that is outside of the bounds of our (guest-)physical + * address space. + */ +static void test_refbk_real(void) +{ + pgd_t *root; + struct pfault_refbk *refbk; + void *refbk_page; + + report_prefix_push("refbk is real"); + + /* Set up virtual memory and allocate a physical page for storing the refbk */ + setup_vm(); + refbk_page = alloc_page(); + + /* Map refblk page outside of physical memory identity mapping */ + root = (pgd_t *)(stctg(1) & PAGE_MASK); + refbk = page_map_outside_real_space(virt_to_pte_phys(root, refbk_page)); + + /* Assert the mapping really is outside identity mapping */ + report_info("refbk is at 0x%lx", (u64)refbk); + report_info("ram size is 0x%lx", get_ram_size()); + assert((u64)refbk > get_ram_size()); + + /* Copy the init refbk to the page */ + memcpy(refbk, &pfault_init_refbk, sizeof(struct pfault_refbk)); + + /* Protect the virtual mapping to avoid diag258 actually doing something */ + protect_page(refbk, PAGE_ENTRY_I); + + expect_pgm_int(); + diag258(refbk); + check_pgm_int_code(PGM_INT_CODE_ADDRESSING); + report_prefix_pop(); + + free_page(refbk_page); + disable_dat(); + irq_set_dat_mode(false, 0); +} + +/* + * Verify diag258 correctly applies prefixing. + */ +static void test_refbk_prefixing(void) +{ + uint64_t ry; + uint32_t old_prefix; + struct pfault_refbk *refbk_in_prefix, *refbk_in_reverse_prefix; + const size_t lowcore_offset_for_refbk = offsetof(struct lowcore, pad_0x03a0); + + report_prefix_push("refbk prefixing"); + + report_info("refbk at lowcore offset 0x%lx", lowcore_offset_for_refbk); + + assert((unsigned long)&prefix_buf < SZ_2G); + + memcpy(prefix_buf, 0, LC_SIZE); + + /* + * After the call to set_prefix() below, this will refer to absolute + * address lowcore_offset_for_refbk (reverse prefixing). + */ + refbk_in_reverse_prefix = (struct pfault_refbk *)(&prefix_buf[0] + lowcore_offset_for_refbk); + + /* + * After the call to set_prefix() below, this will refer to absolute + * address &prefix_buf[0] + lowcore_offset_for_refbk (forward prefixing). + */ + refbk_in_prefix = (struct pfault_refbk *)OPAQUE_PTR(lowcore_offset_for_refbk); + + old_prefix = get_prefix(); + set_prefix((uint32_t)(uintptr_t)prefix_buf); + + /* + * If diag258 would not be applying prefixing on access to + * refbk_in_reverse_prefix correctly, it would access absolute address + * refbk_in_reverse_prefix (which to us is accessible at real address + * refbk_in_prefix). + * Make sure it really fails by putting invalid function code + * at refbk_in_prefix. + */ + refbk_in_prefix->refdiagc = 0xc0fe; + + /* + * Put a valid refbk at refbk_in_reverse_prefix. + */ + memcpy(refbk_in_reverse_prefix, &pfault_init_refbk, sizeof(pfault_init_refbk)); + + ry = diag258(refbk_in_reverse_prefix); + report(!ry, "real address refbk accessed"); + + /* + * Activating should have worked. Cancel the activation and expect + * return 0. If activation would not have worked, this should return with + * 4 (pfault handshaking not active). + */ + ry = diag258(&pfault_cancel_refbk); + report(!ry, "handshaking canceled"); + + set_prefix(old_prefix); + + report_prefix_pop(); +} + +/* + * Verify that a refbk exceeding physical memory is not accepted, even + * when crossing a frame boundary. + */ +static void test_refbk_crossing(void) +{ + const size_t bytes_in_last_page = 8; + struct pfault_refbk *refbk = (struct pfault_refbk *)(get_ram_size() - bytes_in_last_page); + + report_prefix_push("refbk crossing"); + + report_info("refbk is at 0x%lx", (u64)refbk); + report_info("ram size is 0x%lx", get_ram_size()); + assert(sizeof(struct pfault_refbk) > bytes_in_last_page); + + /* Copy bytes_in_last_page bytes of the init refbk to the page */ + memcpy(refbk, &pfault_init_refbk, bytes_in_last_page); + + expect_pgm_int(); + diag258(refbk); + check_pgm_int_code(PGM_INT_CODE_ADDRESSING); + report_prefix_pop(); +} + +/* + * Verify that a refbk with an invalid refdiagc is not accepted. + */ +static void test_refbk_invalid_diagcode(void) +{ + struct pfault_refbk refbk = pfault_init_refbk; + + report_prefix_push("invalid refdiagc"); + refbk.refdiagc = 0xc0fe; + + expect_pgm_int(); + diag258(&refbk); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); +} + +int main(void) +{ + report_prefix_push("diag258"); + + expect_pgm_int(); + diag258(&pfault_init_refbk); + if (clear_pgm_int() == PGM_INT_CODE_SPECIFICATION) { + report_skip("diag258 not supported"); + } else { + test_priv(); + test_refbk_real(); + test_refbk_prefixing(); + test_refbk_crossing(); + test_refbk_invalid_diagcode(); + } + + report_prefix_pop(); + return report_summary(); +} diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index 3a9decc932f2..8131ba105d3f 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -392,3 +392,6 @@ file = sie-dat.elf [pv-attest] file = pv-attest.elf + +[diag258] +file = diag258.elf
This adds a test for diag258 (page ref service/async page fault). There recently was a virtual-real address confusion bug, so we should test: - diag258 parameter Rx is a real adress - crossing the end of RAM with the parameter list yields an addressing exception - invalid diagcode in the parameter block yields an specification exception - diag258 correctly applies prefixing. Note that we're just testing error cases as of now. Signed-off-by: Nico Boehr <nrb@linux.ibm.com> --- s390x/Makefile | 1 + s390x/diag258.c | 258 ++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 3 + 3 files changed, 262 insertions(+) create mode 100644 s390x/diag258.c