Re: [kvm-unit-tests PATCH v6 2/2] s390x: Test specification exceptions during transaction

[Janosch Frank <frankja@linux.ibm.com>]

On 8/26/22 18:11, Janis Schoetterl-Glausch wrote:
> Program interruptions during transactional execution cause other
> interruption codes.
> Check that we see the expected code for (some) specification exceptions.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>

First off a disclaimer stating that I don't know anything about our TB 
facility and I'm currently lacking the time to read the documentation.

But the code looks good to me and I don't see a reason that keeps me 
from picking this.

Acked-by: Janosch Frank <frankja@linux.ibm.com>

Minor nits below

> ---
>   lib/s390x/asm/arch_def.h |   1 +
>   s390x/spec_ex.c          | 199 ++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 195 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index b6e60fb0..c841871c 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -73,6 +73,7 @@ struct cpu {
>   #define PSW_MASK_BA			0x0000000080000000UL
>   #define PSW_MASK_64			(PSW_MASK_BA | PSW_MASK_EA)
>   
> +#define CTL0_TRANSACT_EX_CTL			(63 -  8)
>   #define CTL0_LOW_ADDR_PROT			(63 - 35)
>   #define CTL0_EDAT				(63 - 40)
>   #define CTL0_FETCH_PROTECTION_OVERRIDE		(63 - 38)
> diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
> index 68469e4b..56f26564 100644
> --- a/s390x/spec_ex.c
> +++ b/s390x/spec_ex.c
> @@ -4,13 +4,19 @@
>    *
>    * Specification exception test.
>    * Tests that specification exceptions occur when expected.
> + * This includes specification exceptions occurring during transactional execution
> + * as these result in another interruption code (the transactional-execution-aborted
> + * bit is set).
>    *
>    * Can be extended by adding triggers to spec_ex_triggers, see comments below.
>    */
>   #include <stdlib.h>
> +#include <htmintrin.h>
>   #include <libcflat.h>
>   #include <bitops.h>
> +#include <asm/barrier.h>
>   #include <asm/interrupt.h>
> +#include <asm/facility.h>
>   
>   /* toggled to signal occurrence of invalid psw fixup */
>   static bool invalid_psw_expected;
> @@ -148,20 +154,22 @@ static int not_even(void)
>   /*
>    * Harness for specification exception testing.
>    * func only triggers exception, reporting is taken care of automatically.
> + * If a trigger is transactable it will also  be executed during a transaction.

Double space

> +
> +static void test_spec_ex_trans(struct args *args, const struct spec_ex_trigger *trigger)
> +{
> +	const uint16_t expected_pgm = PGM_INT_CODE_SPECIFICATION
> +				      | PGM_INT_CODE_TX_ABORTED_EVENT;

I usually prefer having | and & at the end so it's easier to read.

> +	union {
> +		struct __htm_tdb tdb;
> +		uint64_t dwords[sizeof(struct __htm_tdb) / sizeof(uint64_t)];
> +	} diag;
> +	unsigned int i;
> +	int trans_result;
> +