On 19.06.2017 11:15, David Hildenbrand wrote:
> Let's add a test case for the STORE CPU ID instruction.
>
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> ---
> lib/s390x/asm/arch_def.h | 8 ++++++++
> s390x/intercept.c | 27 +++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 07d467e..72e5c60 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -143,4 +143,12 @@ struct lowcore {
> #define PGM_INT_CODE_CRYPTO_OPERATION 0x119
> #define PGM_INT_CODE_TX_ABORTED_EVENT 0x200
>
> +struct cpuid {
> + uint64_t version : 8;
> + uint64_t id : 24;
> + uint64_t type : 16;
> + uint64_t format : 1;
> + uint64_t reserved : 15;
> +};
> +
> #endif
> diff --git a/s390x/intercept.c b/s390x/intercept.c
> index 4558860..09c9a62 100644
> --- a/s390x/intercept.c
> +++ b/s390x/intercept.c
> @@ -105,6 +105,32 @@ static void test_stap(void)
> check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
> }
>
> +/* Test the STORE CPU ID instruction */
> +static void test_stidp(void)
> +{
> + struct cpuid id = {};
> +
> + asm volatile ("stidp %0\n" : "+Q"(id));
> + report("id.type != 0", id.type != 0);
> + report("id.version == 0 || id.version == 0xff)",
Superfluous parenthesis -----------------------------^
> + id.version == 0 || id.version == 0xff);
> + report("id.reserved == 0", !id.reserved);
I also think you should not use C code in the text output here.
Not everybody who's running the kvm-unit-tests might be familiar with
the C language syntax. So it'd be nicer to write something like
"reserved bits are zero" instead of "id.reserved == 0" ?
> + expect_pgm_int();
> + low_prot_enable();
> + asm volatile ("stidp 0(%0)\n" : : "r"(8));
> + low_prot_disable();
> + check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> +
> + expect_pgm_int();
> + asm volatile ("stidp 0(%0)\n" : : "r"(1));
> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> +
> + expect_pgm_int();
> + asm volatile ("stidp 0(%0)\n" : : "r"(-8));
> + check_pgm_int_code(PGM_INT_CODE_ADDRESSING);
> +}
> +
> /* Test the TEST BLOCK instruction */
> static void test_testblock(void)
> {
> @@ -152,6 +178,7 @@ struct {
> { "stpx", test_stpx, false },
> { "spx", test_spx, false },
> { "stap", test_stap, false },
> + { "stidp", test_stidp, false },
> { "testblock", test_testblock, false },
> { NULL, NULL, false }
> };
>
Apart from the text output, the patch looks fine to me.
Thomas
