| Message ID | 20220513095017.16301-3-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: uv-host: Access check extensions and improvements | expand |
On Fri, 2022-05-13 at 09:50 +0000, Janosch Frank wrote: > Let's also test for rc 0x3 > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Nico Boehr <nrb@linux.ibm.com> if you fix the nit below. > --- > s390x/uv-host.c | 78 > +++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 76 insertions(+), 2 deletions(-) > > diff --git a/s390x/uv-host.c b/s390x/uv-host.c > index 0f0b18a1..f846fc42 100644 > --- a/s390x/uv-host.c > +++ b/s390x/uv-host.c > @@ -83,6 +83,24 @@ static void test_priv(void) > report_prefix_pop(); > } > > +static void test_uv_uninitialized(void) > +{ > + struct uv_cb_header uvcb = {}; > + int i; > + > + report_prefix_push("uninitialized"); > + > + /* i = 1 to skip over initialize */ Just say if (cmds[i].cmd == UVC_CMD_INIT_UV) continue; inside the loop.
On 5/13/22 11:50, Janosch Frank wrote: > Let's also test for rc 0x3 > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com>Reviewed-by: Steffen Eiden <seiden@linux.ibm.com> I, however, have some nits below. > --- > s390x/uv-host.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 76 insertions(+), 2 deletions(-) > > diff --git a/s390x/uv-host.c b/s390x/uv-host.c > index 0f0b18a1..f846fc42 100644 > --- a/s390x/uv-host.c > +++ b/s390x/uv-host.c > @@ -83,6 +83,24 @@ static void test_priv(void) > report_prefix_pop(); > } > > +static void test_uv_uninitialized(void) > +{ > + struct uv_cb_header uvcb = {}; > + int i; > + > + report_prefix_push("uninitialized"); > + > + /* i = 1 to skip over initialize */ > + for (i = 1; cmds[i].name; i++) { > + expect_pgm_int(); > + uvcb.cmd = cmds[i].cmd; > + uvcb.len = cmds[i].len; > + uv_call_once(0, (uint64_t)&uvcb); > + report(uvcb.rc == UVC_RC_INV_STATE, "%s", cmds[i].name); > + } > + report_prefix_pop(); > +} > + > static void test_config_destroy(void) > { > int rc; > @@ -477,13 +495,68 @@ static void test_invalid(void) > report_prefix_pop(); > } > > +static void test_clear_setup(void) maybe rename this to setup_test_clear(void) I initially mistook this function as a test and not a setup function for a test > +{ > + unsigned long vsize; > + int rc; > + > + uvcb_cgc.header.cmd = UVC_CMD_CREATE_SEC_CONF; > + uvcb_cgc.header.len = sizeof(uvcb_cgc); > + > + uvcb_cgc.guest_stor_origin = 0; > + uvcb_cgc.guest_stor_len = 42 * (1UL << 20); > + vsize = uvcb_qui.conf_base_virt_stor_len + > + ((uvcb_cgc.guest_stor_len / (1UL << 20)) * uvcb_qui.conf_virt_var_stor_len); > + > + uvcb_cgc.conf_base_stor_origin = (uint64_t)memalign(PAGE_SIZE * 4, uvcb_qui.conf_base_phys_stor_len); > + uvcb_cgc.conf_var_stor_origin = (uint64_t)memalign(PAGE_SIZE, vsize); > + uvcb_cgc.guest_asce = (uint64_t)memalign(PAGE_SIZE, 4 * PAGE_SIZE) | ASCE_DT_SEGMENT | REGION_TABLE_LENGTH | ASCE_P; > + uvcb_cgc.guest_sca = (uint64_t)memalign(PAGE_SIZE * 4, PAGE_SIZE * 4); > + > + rc = uv_call(0, (uint64_t)&uvcb_cgc); > + assert(rc == 0); > + > + uvcb_csc.header.len = sizeof(uvcb_csc); > + uvcb_csc.header.cmd = UVC_CMD_CREATE_SEC_CPU; > + uvcb_csc.guest_handle = uvcb_cgc.guest_handle; > + uvcb_csc.stor_origin = (unsigned long)memalign(PAGE_SIZE, uvcb_qui.cpu_stor_len); > + uvcb_csc.state_origin = (unsigned long)memalign(PAGE_SIZE, PAGE_SIZE); > + > + rc = uv_call(0, (uint64_t)&uvcb_csc); > + assert(rc == 0); > +} > + > static void test_clear(void) > { > - uint64_t *tmp = (void *)uvcb_init.stor_origin; > + uint64_t *tmp; > + > + report_prefix_push("load normal reset"); > + > + /* > + * Setup a config and a cpu so we can check if a diag308 reset > + * clears the donated memory and makes the pages unsecure. > + */ > + test_clear_setup(); > > diag308_load_reset(1); > sclp_console_setup(); > - report(!*tmp, "memory cleared after reset 1"); > + > + tmp = (void *)uvcb_init.stor_origin; > + report(!*tmp, "uv init donated memory cleared"); > + > + tmp = (void *)uvcb_cgc.conf_base_stor_origin; > + report(!*tmp, "config base donated memory cleared"); > + > + tmp = (void *)uvcb_cgc.conf_base_stor_origin; > + report(!*tmp, "config variable donated memory cleared"); > + > + tmp = (void *)uvcb_csc.stor_origin; > + report(!*tmp, "cpu donated memory cleared after reset 1"); > + > + /* Check if uninitialized after reset */ > + test_uv_uninitialized(); > + > + report_prefix_pop(); > } > > static void setup_vmem(void) > @@ -514,6 +587,7 @@ int main(void) > > test_priv(); > test_invalid(); > + test_uv_uninitialized(); > test_query(); > test_init(); IIRC this test must be done last, as a following test has an uninitialized UV. Maybe add a short comment for that here. >
On 5/16/22 17:02, Steffen Eiden wrote: > > > On 5/13/22 11:50, Janosch Frank wrote: >> Let's also test for rc 0x3 >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>Reviewed-by: Steffen Eiden <seiden@linux.ibm.com> > I, however, have some nits below. > >> --- >> s390x/uv-host.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 76 insertions(+), 2 deletions(-) >> >> diff --git a/s390x/uv-host.c b/s390x/uv-host.c >> index 0f0b18a1..f846fc42 100644 >> --- a/s390x/uv-host.c >> +++ b/s390x/uv-host.c >> @@ -83,6 +83,24 @@ static void test_priv(void) >> report_prefix_pop(); >> } >> >> +static void test_uv_uninitialized(void) >> +{ >> + struct uv_cb_header uvcb = {}; >> + int i; >> + >> + report_prefix_push("uninitialized"); >> + >> + /* i = 1 to skip over initialize */ >> + for (i = 1; cmds[i].name; i++) { >> + expect_pgm_int(); >> + uvcb.cmd = cmds[i].cmd; >> + uvcb.len = cmds[i].len; >> + uv_call_once(0, (uint64_t)&uvcb); >> + report(uvcb.rc == UVC_RC_INV_STATE, "%s", cmds[i].name); >> + } >> + report_prefix_pop(); >> +} >> + >> static void test_config_destroy(void) >> { >> int rc; >> @@ -477,13 +495,68 @@ static void test_invalid(void) >> report_prefix_pop(); >> } >> >> +static void test_clear_setup(void) > maybe rename this to setup_test_clear(void) > I initially mistook this function as a test and not a setup function for > a test Sure > >> +{ >> + unsigned long vsize; >> + int rc; >> + [...] >> static void setup_vmem(void) >> @@ -514,6 +587,7 @@ int main(void) >> >> test_priv(); >> test_invalid(); >> + test_uv_uninitialized(); >> test_query(); >> test_init(); > IIRC this test must be done last, as a following test has an > uninitialized UV. Maybe add a short comment for that here. You're referring to the test_init()? The test_clear() function must be done last but you're commenting under the test_init() call. So I'm a bit confused about what you want me to do here. >> >
Hi Janosch, On 5/16/22 17:15, Janosch Frank wrote: > On 5/16/22 17:02, Steffen Eiden wrote: >> [snip] > [...] >>> static void setup_vmem(void) >>> @@ -514,6 +587,7 @@ int main(void) >>> test_priv(); >>> test_invalid(); >>> + test_uv_uninitialized(); >>> test_query(); >>> test_init(); >> IIRC this test must be done last, as a following test has an >> uninitialized UV. Maybe add a short comment for that here. > > You're referring to the test_init()? > > The test_clear() function must be done last but you're commenting under > the test_init() call. So I'm a bit confused about what you want me to do > here. Sorry, my fault. Yes I wanted to suggest a comment for the 'test_clear' function.
diff --git a/s390x/uv-host.c b/s390x/uv-host.c index 0f0b18a1..f846fc42 100644 --- a/s390x/uv-host.c +++ b/s390x/uv-host.c @@ -83,6 +83,24 @@ static void test_priv(void) report_prefix_pop(); } +static void test_uv_uninitialized(void) +{ + struct uv_cb_header uvcb = {}; + int i; + + report_prefix_push("uninitialized"); + + /* i = 1 to skip over initialize */ + for (i = 1; cmds[i].name; i++) { + expect_pgm_int(); + uvcb.cmd = cmds[i].cmd; + uvcb.len = cmds[i].len; + uv_call_once(0, (uint64_t)&uvcb); + report(uvcb.rc == UVC_RC_INV_STATE, "%s", cmds[i].name); + } + report_prefix_pop(); +} + static void test_config_destroy(void) { int rc; @@ -477,13 +495,68 @@ static void test_invalid(void) report_prefix_pop(); } +static void test_clear_setup(void) +{ + unsigned long vsize; + int rc; + + uvcb_cgc.header.cmd = UVC_CMD_CREATE_SEC_CONF; + uvcb_cgc.header.len = sizeof(uvcb_cgc); + + uvcb_cgc.guest_stor_origin = 0; + uvcb_cgc.guest_stor_len = 42 * (1UL << 20); + vsize = uvcb_qui.conf_base_virt_stor_len + + ((uvcb_cgc.guest_stor_len / (1UL << 20)) * uvcb_qui.conf_virt_var_stor_len); + + uvcb_cgc.conf_base_stor_origin = (uint64_t)memalign(PAGE_SIZE * 4, uvcb_qui.conf_base_phys_stor_len); + uvcb_cgc.conf_var_stor_origin = (uint64_t)memalign(PAGE_SIZE, vsize); + uvcb_cgc.guest_asce = (uint64_t)memalign(PAGE_SIZE, 4 * PAGE_SIZE) | ASCE_DT_SEGMENT | REGION_TABLE_LENGTH | ASCE_P; + uvcb_cgc.guest_sca = (uint64_t)memalign(PAGE_SIZE * 4, PAGE_SIZE * 4); + + rc = uv_call(0, (uint64_t)&uvcb_cgc); + assert(rc == 0); + + uvcb_csc.header.len = sizeof(uvcb_csc); + uvcb_csc.header.cmd = UVC_CMD_CREATE_SEC_CPU; + uvcb_csc.guest_handle = uvcb_cgc.guest_handle; + uvcb_csc.stor_origin = (unsigned long)memalign(PAGE_SIZE, uvcb_qui.cpu_stor_len); + uvcb_csc.state_origin = (unsigned long)memalign(PAGE_SIZE, PAGE_SIZE); + + rc = uv_call(0, (uint64_t)&uvcb_csc); + assert(rc == 0); +} + static void test_clear(void) { - uint64_t *tmp = (void *)uvcb_init.stor_origin; + uint64_t *tmp; + + report_prefix_push("load normal reset"); + + /* + * Setup a config and a cpu so we can check if a diag308 reset + * clears the donated memory and makes the pages unsecure. + */ + test_clear_setup(); diag308_load_reset(1); sclp_console_setup(); - report(!*tmp, "memory cleared after reset 1"); + + tmp = (void *)uvcb_init.stor_origin; + report(!*tmp, "uv init donated memory cleared"); + + tmp = (void *)uvcb_cgc.conf_base_stor_origin; + report(!*tmp, "config base donated memory cleared"); + + tmp = (void *)uvcb_cgc.conf_base_stor_origin; + report(!*tmp, "config variable donated memory cleared"); + + tmp = (void *)uvcb_csc.stor_origin; + report(!*tmp, "cpu donated memory cleared after reset 1"); + + /* Check if uninitialized after reset */ + test_uv_uninitialized(); + + report_prefix_pop(); } static void setup_vmem(void) @@ -514,6 +587,7 @@ int main(void) test_priv(); test_invalid(); + test_uv_uninitialized(); test_query(); test_init();
Let's also test for rc 0x3 Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- s390x/uv-host.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 2 deletions(-)