Re: [PATCH] tests/boot-sector: Fix the bad s390x assembler code

[Cornelia Huck]

On Tue, 17 Dec 2019 16:06:42 +0100
Thomas Huth <address@hidden> wrote:

> There are currently two bugs in s390x_code[]: First, the initial jump
> uses the wrong offset, so it was jumping to 0x1014 instead of 0x1010.
> Second, LHI only loads the lower 32-bit of the register.
> 
> Everything worked fine as long as the s390-ccw bios code was jumping
> here with r3 containing zeroes in the uppermost 48 bit - which just
> happened to be the case so far by accident. But we can not rely on this
> fact, and indeed one of the recent suggested patches to jump2ipl.c cause
> the newer GCCs to put different values into r3. In that case the code
> from s390x_code[] crashes very ungracefully.
> 
> Thus let's make sure to jump to the right instruction, and use LGHI
> instead of LHI to make sure that we always zero out the upper bits
> of the register.
> 
> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  tests/boot-sector.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> index 7824286b9a..9e66c6d013 100644
> --- a/tests/boot-sector.c
> +++ b/tests/boot-sector.c
> @@ -75,11 +75,11 @@ static const uint8_t s390x_psw_and_magic[] = {
>      0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40   /* in the s390-ccw bios 
> */
>  };
>  static const uint8_t s390x_code[] = {
> -    0xa7, 0xf4, 0x00, 0x0a,                                /* j 0x10010 */
> +    0xa7, 0xf4, 0x00, 0x08,                                /* j 0x10010 */
>      0x00, 0x00, 0x00, 0x00,
>      'S', '3', '9', '0',
>      'E', 'P', 0x00, 0x01,
> -    0xa7, 0x38, HIGH(SIGNATURE_ADDR), LOW(SIGNATURE_ADDR), /* lhi r3,0x7c10 
> */
> +    0xa7, 0x39, HIGH(SIGNATURE_ADDR), LOW(SIGNATURE_ADDR), /* lghi r3,0x7c10 
> */
>      0xa7, 0x48, LOW(SIGNATURE), HIGH(SIGNATURE),           /* lhi r4,0xadde 
> */
>      0x40, 0x40, 0x30, 0x00,                                /* sth r4,0(r3) */
>      0xa7, 0xf4, 0xff, 0xfa                                 /* j 0x10010 */

Looks good to me. I plan to queue this (and re-queue the other patches
I had dropped), but would not mind another review.