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

[Christian Borntraeger <borntraeger@xxxxxxxxxx>]

Am 25.10.21 um 19:30 schrieb Claudio Imbrenda:
> On Fri, 22 Oct 2021 14:01:56 +0200
> Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > ---
> >   lib/s390x/asm/arch_def.h |   1 +
> >   s390x/spec_ex.c          | 172 +++++++++++++++++++++++++++++++++++++--
> >   2 files changed, 168 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> > index 40626d7..f7fb467 100644
> > --- a/lib/s390x/asm/arch_def.h
> > +++ b/lib/s390x/asm/arch_def.h
> > @@ -55,6 +55,7 @@ struct psw {
> >   #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_IEP			(63 - 43)
> > diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
> > index ec3322a..f3628bd 100644
> > --- a/s390x/spec_ex.c
> > +++ b/s390x/spec_ex.c
> > @@ -4,9 +4,14 @@
> >    *
> >    * 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).
> >    */
> >   #include <stdlib.h>
> > +#include <htmintrin.h>
> >   #include <libcflat.h>
> > +#include <asm/barrier.h>
> >   #include <asm/interrupt.h>
> >   #include <asm/facility.h>
> >   
> > @@ -92,18 +97,23 @@ static void not_even(void)
> >   struct spec_ex_trigger {
> >   	const char *name;
> >   	void (*func)(void);
> > +	bool transactable;
> >   	void (*fixup)(void);
> >   };
> >   
> >   static const struct spec_ex_trigger spec_ex_triggers[] = {
> > -	{ "psw_bit_12_is_1", &psw_bit_12_is_1, &fixup_invalid_psw},
> > -	{ "bad_alignment", &bad_alignment, NULL},
> > -	{ "not_even", &not_even, NULL},
> > -	{ NULL, NULL, NULL},
> > +	{ "psw_bit_12_is_1", &psw_bit_12_is_1, false, &fixup_invalid_psw},
> > +	{ "bad_alignment", &bad_alignment, true, NULL},
> > +	{ "not_even", &not_even, true, NULL},
> > +	{ NULL, NULL, true, NULL},
> >   };
> >   
> >   struct args {
> >   	uint64_t iterations;
> > +	uint64_t max_retries;
> > +	uint64_t suppress_info;
> > +	uint64_t max_failures;
> > +	bool diagnose;
> >   };
> >   
> >   static void test_spec_ex(struct args *args,
> > @@ -131,14 +141,132 @@ static void test_spec_ex(struct args *args,
> >   		    expected_pgm);
> >   }
> >   
> > +#define TRANSACTION_COMPLETED 4
> > +#define TRANSACTION_MAX_RETRIES 5
> > +
> > +/* NULL must be passed to __builtin_tbegin via constant, forbid diagnose from
> > + * being NULL to keep things simple
> > + */
> > +static int __attribute__((nonnull))
> > +with_transaction(void (*trigger)(void), struct __htm_tdb *diagnose)
> > +{
> > +	int cc;
> > +
>
> if you want to be extra sure, put an assert here (although I'm not sure
> how nonnull works, I have never seen it before)
>
> > +	cc = __builtin_tbegin(diagnose);
> > +	if (cc == _HTM_TBEGIN_STARTED) {
> > +		trigger();
> > +		__builtin_tend();
> > +		return -TRANSACTION_COMPLETED;
> > +	} else {
> > +		return -cc;
> > +	}
> > +}
> > +
> > +static int retry_transaction(const struct spec_ex_trigger *trigger, unsigned int max_retries,
> > +			     struct __htm_tdb *tdb, uint16_t expected_pgm)
> > +{
> > +	int trans_result, i;
> > +	uint16_t pgm;
> > +
> > +	for (i = 0; i < max_retries; i++) {
> > +		expect_pgm_int();
> > +		trans_result = with_transaction(trigger->func, tdb);
> > +		if (trans_result == -_HTM_TBEGIN_TRANSIENT) {
> > +			mb();
> > +			pgm = lc->pgm_int_code;
> > +			if (pgm == 0)
> > +				continue;
> > +			else if (pgm == expected_pgm)
> > +				return 0;
> > +		}
> > +		return trans_result;
> > +	}
> > +	return -TRANSACTION_MAX_RETRIES;
>
> so this means that a test will be considered failed if the transaction
> failed too many times?
> this means that could fail if the test is run on busy system, even if
> the host running the unit test is correct

Can we use constrained transactions for this test? those will succeed.