tests/boot-sector: Fix the bad s390x assembler code
diff mbox series

Message ID 20191217150642.27946-1-thuth@redhat.com
State New
Headers show
Series
  • tests/boot-sector: Fix the bad s390x assembler code
Related show

Commit Message

Thomas Huth Dec. 17, 2019, 3:06 p.m. UTC
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 <thuth@redhat.com>
---
 tests/boot-sector.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Cornelia Huck Dec. 18, 2019, 11:47 a.m. UTC | #1
On Tue, 17 Dec 2019 16:06:42 +0100
Thomas Huth <thuth@redhat.com> 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 <thuth@redhat.com>
> ---
>  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.
Christian Borntraeger Dec. 18, 2019, 2:49 p.m. UTC | #2
On 17.12.19 16:06, Thomas Huth 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.
                                              ^^^^    

10014/10010 instead of 1014/1010


> 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 <thuth@redhat.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>


> ---
>  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 */
>
Cornelia Huck Dec. 18, 2019, 4:53 p.m. UTC | #3
On Tue, 17 Dec 2019 16:06:42 +0100
Thomas Huth <thuth@redhat.com> 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 <thuth@redhat.com>
> ---
>  tests/boot-sector.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Thanks, applied.

Patch
diff mbox series

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 */