diff mbox series

[kvm-unit-tests,v1] s390x: spec_ex: Add test for misaligned load

Message ID 20230301132638.3336040-1-nsg@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests,v1] s390x: spec_ex: Add test for misaligned load | expand

Commit Message

Nina Schoetterl-Glausch March 1, 2023, 1:26 p.m. UTC
The operand of LOAD RELATIVE LONG must be word aligned, otherwise a
specification exception occurs. Test that this exception occurs.

Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---


Noticed while writing another test that TCG fails this requirement,
so thought it best do document this in the form of a test.


 s390x/spec_ex.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)


base-commit: e3c5c3ef2524c58023073c0fadde2e8ae3c04ec6

Comments

Claudio Imbrenda March 6, 2023, 10:59 a.m. UTC | #1
On Wed,  1 Mar 2023 14:26:38 +0100
Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote:

> The operand of LOAD RELATIVE LONG must be word aligned, otherwise a
> specification exception occurs. Test that this exception occurs.

you're only testing halfword misalignment; would it make sense to test
all possible misalignments? (it's only 3 of them after all)

> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
> 
> 
> Noticed while writing another test that TCG fails this requirement,
> so thought it best do document this in the form of a test.
> 
> 
>  s390x/spec_ex.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
> index 42ecaed3..42e86070 100644
> --- a/s390x/spec_ex.c
> +++ b/s390x/spec_ex.c
> @@ -136,7 +136,7 @@ static int short_psw_bit_12_is_0(void)
>  	return 0;
>  }
>  
> -static int bad_alignment(void)
> +static int bad_alignment_lqp(void)
>  {
>  	uint32_t words[5] __attribute__((aligned(16)));
>  	uint32_t (*bad_aligned)[4] = (uint32_t (*)[4])&words[1];
> @@ -149,6 +149,22 @@ static int bad_alignment(void)
>  	return 0;
>  }
>  
> +static int bad_alignment_lrl(void)
> +{
> +	uint64_t r;
> +
> +	asm volatile ( ".pushsection .rodata\n"

why not declare this as a local array?

uint8_t stuff[8] __attribute__((aligned(8)));

> +		"	.balign	4\n"
> +		"	. = . + 2\n"
> +		"0:	.fill	4\n"
> +		"	.popsection\n"
> +
> +		"	lrl	%0,0b\n"
> +		: "=d" (r)

and here pass stuff + 1 or something like that?

less asm = more readable

> +	);
> +	return 0;
> +}
> +
>  static int not_even(void)
>  {
>  	uint64_t quad[2] __attribute__((aligned(16))) = {0};
> @@ -176,7 +192,8 @@ 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 },
> -	{ "bad_alignment", &bad_alignment, true, NULL },
> +	{ "bad_alignment_lqp", &bad_alignment_lqp, true, NULL },
> +	{ "bad_alignment_lrl", &bad_alignment_lrl, true, NULL },
>  	{ "not_even", &not_even, true, NULL },
>  	{ NULL, NULL, false, NULL },
>  };
> 
> base-commit: e3c5c3ef2524c58023073c0fadde2e8ae3c04ec6
Nina Schoetterl-Glausch March 6, 2023, 11:31 a.m. UTC | #2
On Mon, 2023-03-06 at 11:59 +0100, Claudio Imbrenda wrote:
> On Wed,  1 Mar 2023 14:26:38 +0100
> Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote:
> 
> > The operand of LOAD RELATIVE LONG must be word aligned, otherwise a
> > specification exception occurs. Test that this exception occurs.
> 
> you're only testing halfword misalignment; would it make sense to test
> all possible misalignments? (it's only 3 of them after all)

No, that's not possible, the address calculation is:
insn_addr + immediate * 2

So for LRL there is only one possible misalignment.
For LGRL there are multiple, tho.

> 
> > 
> > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > ---
> > 
> > 
> > Noticed while writing another test that TCG fails this requirement,
> > so thought it best do document this in the form of a test.
> > 
> > 
> >  s390x/spec_ex.c | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
> > index 42ecaed3..42e86070 100644
> > --- a/s390x/spec_ex.c
> > +++ b/s390x/spec_ex.c
> > @@ -136,7 +136,7 @@ static int short_psw_bit_12_is_0(void)
> >  	return 0;
> >  }
> >  
> > -static int bad_alignment(void)
> > +static int bad_alignment_lqp(void)
> >  {
> >  	uint32_t words[5] __attribute__((aligned(16)));
> >  	uint32_t (*bad_aligned)[4] = (uint32_t (*)[4])&words[1];
> > @@ -149,6 +149,22 @@ static int bad_alignment(void)
> >  	return 0;
> >  }
> >  
> > +static int bad_alignment_lrl(void)
> > +{
> > +	uint64_t r;
> > +
> > +	asm volatile ( ".pushsection .rodata\n"
> 
> why not declare this as a local array?

I cannot put it on the stack, since I need a relative offset.
I guess I could use a global symbol, but that also makes the
test less self-contained.

> 
> uint8_t stuff[8] __attribute__((aligned(8)));
> 
> > +		"	.balign	4\n"
> > +		"	. = . + 2\n"
> > +		"0:	.fill	4\n"
> > +		"	.popsection\n"
> > +
> > +		"	lrl	%0,0b\n"
> > +		: "=d" (r)
> 
> and here pass stuff + 1 or something like that?
> 
> less asm = more readable
> 
> > +	);
> > +	return 0;
> > +}
> > +
> >  static int not_even(void)
> >  {
> >  	uint64_t quad[2] __attribute__((aligned(16))) = {0};
> > @@ -176,7 +192,8 @@ 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 },
> > -	{ "bad_alignment", &bad_alignment, true, NULL },
> > +	{ "bad_alignment_lqp", &bad_alignment_lqp, true, NULL },
> > +	{ "bad_alignment_lrl", &bad_alignment_lrl, true, NULL },
> >  	{ "not_even", &not_even, true, NULL },
> >  	{ NULL, NULL, false, NULL },
> >  };
> > 
> > base-commit: e3c5c3ef2524c58023073c0fadde2e8ae3c04ec6
>
Claudio Imbrenda March 20, 2023, 12:22 p.m. UTC | #3
On Wed,  1 Mar 2023 14:26:38 +0100
Nina Schoetterl-Glausch <nsg@linux.ibm.com> wrote:

> The operand of LOAD RELATIVE LONG must be word aligned, otherwise a
> specification exception occurs. Test that this exception occurs.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
> 
> 
> Noticed while writing another test that TCG fails this requirement,
> so thought it best do document this in the form of a test.
> 
> 
>  s390x/spec_ex.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
> index 42ecaed3..42e86070 100644
> --- a/s390x/spec_ex.c
> +++ b/s390x/spec_ex.c
> @@ -136,7 +136,7 @@ static int short_psw_bit_12_is_0(void)
>  	return 0;
>  }
>  
> -static int bad_alignment(void)
> +static int bad_alignment_lqp(void)
>  {
>  	uint32_t words[5] __attribute__((aligned(16)));
>  	uint32_t (*bad_aligned)[4] = (uint32_t (*)[4])&words[1];
> @@ -149,6 +149,22 @@ static int bad_alignment(void)
>  	return 0;
>  }
>  
> +static int bad_alignment_lrl(void)
> +{
> +	uint64_t r;
> +
> +	asm volatile ( ".pushsection .rodata\n"
> +		"	.balign	4\n"
> +		"	. = . + 2\n"
> +		"0:	.fill	4\n"
> +		"	.popsection\n"
> +
> +		"	lrl	%0,0b\n"
> +		: "=d" (r)
> +	);
> +	return 0;
> +}
> +
>  static int not_even(void)
>  {
>  	uint64_t quad[2] __attribute__((aligned(16))) = {0};
> @@ -176,7 +192,8 @@ 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 },
> -	{ "bad_alignment", &bad_alignment, true, NULL },
> +	{ "bad_alignment_lqp", &bad_alignment_lqp, true, NULL },
> +	{ "bad_alignment_lrl", &bad_alignment_lrl, true, NULL },
>  	{ "not_even", &not_even, true, NULL },
>  	{ NULL, NULL, false, NULL },
>  };
> 
> base-commit: e3c5c3ef2524c58023073c0fadde2e8ae3c04ec6
Nico Boehr March 22, 2023, 2:55 p.m. UTC | #4
Quoting Nina Schoetterl-Glausch (2023-03-01 14:26:38)
> The operand of LOAD RELATIVE LONG must be word aligned, otherwise a
> specification exception occurs. Test that this exception occurs.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
Nico Boehr March 23, 2023, 12:42 p.m. UTC | #5
Quoting Nina Schoetterl-Glausch (2023-03-01 14:26:38)
> The operand of LOAD RELATIVE LONG must be word aligned, otherwise a
> specification exception occurs. Test that this exception occurs.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

queued, thanks.
diff mbox series

Patch

diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
index 42ecaed3..42e86070 100644
--- a/s390x/spec_ex.c
+++ b/s390x/spec_ex.c
@@ -136,7 +136,7 @@  static int short_psw_bit_12_is_0(void)
 	return 0;
 }
 
-static int bad_alignment(void)
+static int bad_alignment_lqp(void)
 {
 	uint32_t words[5] __attribute__((aligned(16)));
 	uint32_t (*bad_aligned)[4] = (uint32_t (*)[4])&words[1];
@@ -149,6 +149,22 @@  static int bad_alignment(void)
 	return 0;
 }
 
+static int bad_alignment_lrl(void)
+{
+	uint64_t r;
+
+	asm volatile ( ".pushsection .rodata\n"
+		"	.balign	4\n"
+		"	. = . + 2\n"
+		"0:	.fill	4\n"
+		"	.popsection\n"
+
+		"	lrl	%0,0b\n"
+		: "=d" (r)
+	);
+	return 0;
+}
+
 static int not_even(void)
 {
 	uint64_t quad[2] __attribute__((aligned(16))) = {0};
@@ -176,7 +192,8 @@  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 },
-	{ "bad_alignment", &bad_alignment, true, NULL },
+	{ "bad_alignment_lqp", &bad_alignment_lqp, true, NULL },
+	{ "bad_alignment_lrl", &bad_alignment_lrl, true, NULL },
 	{ "not_even", &not_even, true, NULL },
 	{ NULL, NULL, false, NULL },
 };