mbox series

[kvm-unit-tests,v7,0/3] s390x: Test effect of storage keys on some instructions

Message ID 20220502154101.3663941-1-scgl@linux.ibm.com (mailing list archive)
Headers show
Series s390x: Test effect of storage keys on some instructions | expand

Message

Janis Schoetterl-Glausch May 2, 2022, 3:40 p.m. UTC 
See range diff below for changes. Since I messed up the addressees
for the cover letter for the last version, the diff is against v5.

v6 -> v7:
 * Add fetch-protection override test case to TPROT test
 * Change reporting of TPROT test to be more in line with other tests

v5 -> v6:
 * Disable skey test in GitLab CI, needs kernel 5.18
 * Added comment to test_set_prefix
 * Introduce names for tprot return values

...

v2 -> v3:
 * fix asm for SET PREFIX zero key test: make input
 * implement Thomas' suggestions:
   https://lore.kernel.org/kvm/f050da01-4d50-5da5-7f08-6da30f5dbbbe@redhat.com/

v1 -> v2:
 * use install_page instead of manual page table entry manipulation
 * check that no store occurred if none is expected
 * try to check that no fetch occurred if not expected, although in
   practice a fetch would probably cause the test to crash
 * reset storage key to 0 after test


Janis Schoetterl-Glausch (3):
  s390x: Give name to return value of tprot()
  s390x: Test effect of storage keys on some instructions
  Disable s390x skey test in GitLab CI

 lib/s390x/asm/arch_def.h |  31 +++--
 lib/s390x/sclp.c         |   6 +-
 s390x/skey.c             | 250 +++++++++++++++++++++++++++++++++++++++
 s390x/tprot.c            |  24 ++--
 .gitlab-ci.yml           |   2 +-
 5 files changed, 286 insertions(+), 27 deletions(-)

Range-diff against v5:
-:  -------- > 1:  6b11f01d s390x: Give name to return value of tprot()
1:  89e59626 ! 2:  e3df88c6 s390x: Test effect of storage keys on some instructions
    @@ s390x/skey.c: static void test_invalid_address(void)
     +	report_prefix_push("TPROT");
     +
     +	set_storage_key(pagebuf, 0x10, 0);
    -+	report(tprot(addr, 0) == 0, "access key 0 -> no protection");
    -+	report(tprot(addr, 1) == 0, "access key matches -> no protection");
    -+	report(tprot(addr, 2) == 1, "access key mismatches, no fetch protection -> store protection");
    ++	report(tprot(addr, 0) == TPROT_READ_WRITE, "zero key: no protection");
    ++	report(tprot(addr, 1) == TPROT_READ_WRITE, "matching key: no protection");
    ++
    ++	report_prefix_push("mismatching key");
    ++
    ++	report(tprot(addr, 2) == TPROT_READ, "no fetch protection: store protection");
    ++
     +
     +	set_storage_key(pagebuf, 0x18, 0);
    -+	report(tprot(addr, 2) == 2, "access key mismatches, fetch protection -> fetch & store protection");
    ++	report(tprot(addr, 2) == TPROT_RW_PROTECTED,
    ++	       "fetch protection: fetch & store protection");
    ++
    ++	report_prefix_push("fetch-protection override");
    ++	set_storage_key(0, 0x18, 0);
    ++	report(tprot(0, 2) == TPROT_RW_PROTECTED, "disabled: fetch & store protection");
    ++	ctl_set_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
    ++	report(tprot(0, 2) == TPROT_READ, "enabled: store protection");
    ++	report(tprot(2048, 2) == TPROT_RW_PROTECTED, "invalid: fetch & store protection");
    ++	ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
    ++	set_storage_key(0, 0x00, 0);
    ++	report_prefix_pop();
     +
     +	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
     +	set_storage_key(pagebuf, 0x90, 0);
    -+	report(tprot(addr, 2) == 0, "access key mismatches, storage protection override -> no protection");
    ++	report(tprot(addr, 2) == TPROT_READ_WRITE,
    ++	       "storage-protection override: no protection");
     +	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
     +
    ++	report_prefix_pop();
     +	set_storage_key(pagebuf, 0x00, 0);
     +	report_prefix_pop();
     +}
    @@ s390x/skey.c: static void test_invalid_address(void)
     +#define PREFIX_AREA_SIZE (PAGE_SIZE * 2)
     +static char lowcore_tmp[PREFIX_AREA_SIZE] __attribute__((aligned(PREFIX_AREA_SIZE)));
     +
    ++/*
    ++ * Test accessibility of the operand to SET PREFIX given different configurations
    ++ * with regards to storage keys. That is, check the accessibility of the location
    ++ * holding the new prefix, not that of the new prefix area. The new prefix area
    ++ * is a valid lowcore, so that the test does not crash on failure.
    ++ */
     +static void test_set_prefix(void)
     +{
     +	uint32_t *prefix_ptr = (uint32_t *)pagebuf;
-:  -------- > 3:  c3236718 Disable s390x skey test in GitLab CI

base-commit: 6a7a83ed106211fc0ee530a3a05f171f6a4c4e66

Comments

Claudio Imbrenda May 5, 2022, 1:16 p.m. UTC | #1 
On Mon,  2 May 2022 17:40:58 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

> See range diff below for changes. Since I messed up the addressees
> for the cover letter for the last version, the diff is against v5.

looks good, if there are no objections I would queue this

> 
> v6 -> v7:
>  * Add fetch-protection override test case to TPROT test
>  * Change reporting of TPROT test to be more in line with other tests
> 
> v5 -> v6:
>  * Disable skey test in GitLab CI, needs kernel 5.18
>  * Added comment to test_set_prefix
>  * Introduce names for tprot return values
> 
> ...
> 
> v2 -> v3:
>  * fix asm for SET PREFIX zero key test: make input
>  * implement Thomas' suggestions:
>    https://lore.kernel.org/kvm/f050da01-4d50-5da5-7f08-6da30f5dbbbe@redhat.com/
> 
> v1 -> v2:
>  * use install_page instead of manual page table entry manipulation
>  * check that no store occurred if none is expected
>  * try to check that no fetch occurred if not expected, although in
>    practice a fetch would probably cause the test to crash
>  * reset storage key to 0 after test
> 
> 
> Janis Schoetterl-Glausch (3):
>   s390x: Give name to return value of tprot()
>   s390x: Test effect of storage keys on some instructions
>   Disable s390x skey test in GitLab CI
> 
>  lib/s390x/asm/arch_def.h |  31 +++--
>  lib/s390x/sclp.c         |   6 +-
>  s390x/skey.c             | 250 +++++++++++++++++++++++++++++++++++++++
>  s390x/tprot.c            |  24 ++--
>  .gitlab-ci.yml           |   2 +-
>  5 files changed, 286 insertions(+), 27 deletions(-)
> 
> Range-diff against v5:
> -:  -------- > 1:  6b11f01d s390x: Give name to return value of tprot()
> 1:  89e59626 ! 2:  e3df88c6 s390x: Test effect of storage keys on some instructions
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +	report_prefix_push("TPROT");
>      +
>      +	set_storage_key(pagebuf, 0x10, 0);
>     -+	report(tprot(addr, 0) == 0, "access key 0 -> no protection");
>     -+	report(tprot(addr, 1) == 0, "access key matches -> no protection");
>     -+	report(tprot(addr, 2) == 1, "access key mismatches, no fetch protection -> store protection");
>     ++	report(tprot(addr, 0) == TPROT_READ_WRITE, "zero key: no protection");
>     ++	report(tprot(addr, 1) == TPROT_READ_WRITE, "matching key: no protection");
>     ++
>     ++	report_prefix_push("mismatching key");
>     ++
>     ++	report(tprot(addr, 2) == TPROT_READ, "no fetch protection: store protection");
>     ++
>      +
>      +	set_storage_key(pagebuf, 0x18, 0);
>     -+	report(tprot(addr, 2) == 2, "access key mismatches, fetch protection -> fetch & store protection");
>     ++	report(tprot(addr, 2) == TPROT_RW_PROTECTED,
>     ++	       "fetch protection: fetch & store protection");
>     ++
>     ++	report_prefix_push("fetch-protection override");
>     ++	set_storage_key(0, 0x18, 0);
>     ++	report(tprot(0, 2) == TPROT_RW_PROTECTED, "disabled: fetch & store protection");
>     ++	ctl_set_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
>     ++	report(tprot(0, 2) == TPROT_READ, "enabled: store protection");
>     ++	report(tprot(2048, 2) == TPROT_RW_PROTECTED, "invalid: fetch & store protection");
>     ++	ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
>     ++	set_storage_key(0, 0x00, 0);
>     ++	report_prefix_pop();
>      +
>      +	ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
>      +	set_storage_key(pagebuf, 0x90, 0);
>     -+	report(tprot(addr, 2) == 0, "access key mismatches, storage protection override -> no protection");
>     ++	report(tprot(addr, 2) == TPROT_READ_WRITE,
>     ++	       "storage-protection override: no protection");
>      +	ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
>      +
>     ++	report_prefix_pop();
>      +	set_storage_key(pagebuf, 0x00, 0);
>      +	report_prefix_pop();
>      +}
>     @@ s390x/skey.c: static void test_invalid_address(void)
>      +#define PREFIX_AREA_SIZE (PAGE_SIZE * 2)
>      +static char lowcore_tmp[PREFIX_AREA_SIZE] __attribute__((aligned(PREFIX_AREA_SIZE)));
>      +
>     ++/*
>     ++ * Test accessibility of the operand to SET PREFIX given different configurations
>     ++ * with regards to storage keys. That is, check the accessibility of the location
>     ++ * holding the new prefix, not that of the new prefix area. The new prefix area
>     ++ * is a valid lowcore, so that the test does not crash on failure.
>     ++ */
>      +static void test_set_prefix(void)
>      +{
>      +	uint32_t *prefix_ptr = (uint32_t *)pagebuf;
> -:  -------- > 3:  c3236718 Disable s390x skey test in GitLab CI
> 
> base-commit: 6a7a83ed106211fc0ee530a3a05f171f6a4c4e66