On 3/17/23 11:51, Nina Schoetterl-Glausch wrote:
On Fri, 2023-03-17 at 10:26 +0100, Janosch Frank wrote:On 3/15/23 16:54, Nina Schoetterl-Glausch wrote:Instructions on s390 must be halfword aligned. Introducing an odd instruction address into the PSW leads to a specification exception when attempting to execute the instruction at the odd address. Add a test for this. Signed-off-by: Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx>Acked-by: Janosch Frank <frankja@xxxxxxxxxxxxx> Some nits below.--- s390x/spec_ex.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c index 2adc5996..83b8c58e 100644 --- a/s390x/spec_ex.c +++ b/s390x/spec_ex.c @@ -88,12 +88,23 @@ static void expect_invalid_psw(struct psw psw) invalid_psw_expected = true; }+static void clear_invalid_psw(void)+{ + expected_psw = PSW(0, 0); + invalid_psw_expected = false; +} + static int check_invalid_psw(void) { /* Since the fixup sets this to false we check for false here. */ if (!invalid_psw_expected) { + /* + * Early exception recognition: pgm_int_id == 0. + * Late exception recognition: psw address has been + * incremented by pgm_int_id (unpredictable value) + */ if (expected_psw.mask == invalid_psw.mask && - expected_psw.addr == invalid_psw.addr) + expected_psw.addr == invalid_psw.addr - lowcore.pgm_int_id) return 0; report_fail("Wrong invalid PSW"); } else { @@ -112,6 +123,42 @@ static int psw_bit_12_is_1(void) return check_invalid_psw(); }+extern char misaligned_code[];+asm ( ".balign 2\n"Is the double space intended?Yes, so stuff lines up.
ahhh, right.
Looking at the file itself some asm blocks have no space before the "(" and some have one.In spec_ex.c? Where?
Should have said: "after the (" but seems like the point doesn't matter anyway just fixup the xgr.
+" . = . + 1\n" +"misaligned_code:\n" +" larl %r0,0\n" +" br %r1\n" +);Any reason this is not indented?You mean the whole asm block, so it looks more like a function body to the misaligned_code symbol? I'm indifferent about it, can do that if you think it's nicer.+ +static int psw_odd_address(void) +{ + struct psw odd = PSW_WITH_CUR_MASK((uint64_t)&misaligned_code); + uint64_t executed_addr; + + expect_invalid_psw(odd); + fixup_psw.mask = extract_psw_mask(); + asm volatile ( "xr %%r0,%%r0\n"While it will likely never make a difference I'd still use xgr here instead of xr.Yes, needs xgr.+ " larl %%r1,0f\n" + " stg %%r1,%[fixup_addr]\n" + " lpswe %[odd_psw]\n" + "0: lr %[executed_addr],%%r0\n" + : [fixup_addr] "=&T" (fixup_psw.addr), + [executed_addr] "=d" (executed_addr) + : [odd_psw] "Q" (odd) + : "cc", "%r0", "%r1" + ); + + if (!executed_addr) { + return check_invalid_psw(); + } else { + assert(executed_addr == odd.addr); + clear_invalid_psw(); + report_fail("did not execute unaligned instructions"); + return 1; + } +} + /* A short PSW needs to have bit 12 set to be valid. */ static int short_psw_bit_12_is_0(void) { @@ -170,6 +217,7 @@ struct spec_ex_trigger { static const struct spec_ex_trigger spec_ex_triggers[] = { { "psw_bit_12_is_1", &psw_bit_12_is_1, false, &fixup_invalid_psw }, { "short_psw_bit_12_is_0", &short_psw_bit_12_is_0, false, &fixup_invalid_psw }, + { "psw_odd_address", &psw_odd_address, false, &fixup_invalid_psw }, { "bad_alignment", &bad_alignment, true, NULL }, { "not_even", ¬_even, true, NULL }, { NULL, NULL, false, NULL },
