[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 4/5] pc-bios: s390x: Save io and external new PSWs before
From: |
Janosch Frank |
Subject: |
Re: [PATCH v3 4/5] pc-bios: s390x: Save io and external new PSWs before overwriting them |
Date: |
Wed, 2 Sep 2020 12:30:25 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 |
On 9/1/20 7:22 PM, Thomas Huth wrote:
> On 31/08/2020 17.09, Janosch Frank wrote:
>> Currently we always overwrite the mentioned exception new PSWs before
>> loading the enabled wait PSW. Let's save the PSW before overwriting
>> and restore it right before starting the loaded kernel.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> pc-bios/s390-ccw/jump2ipl.c | 4 +++
>> pc-bios/s390-ccw/netmain.c | 3 ++
>> pc-bios/s390-ccw/start.S | 62 +++++++++++++++++++++++++++----------
>> 3 files changed, 52 insertions(+), 17 deletions(-)
>
> Patch looks basically fine to me, I just got some questions for my
> understanding below...
>
>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>> index 5b8352d257..bb94ba7550 100644
>> --- a/pc-bios/s390-ccw/jump2ipl.c
>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>> @@ -14,6 +14,7 @@
>> #define RESET_PSW_MASK (PSW_MASK_SHORTPSW | PSW_MASK_64)
>> #define RESET_PSW ((uint64_t)&jump_to_IPL_addr | RESET_PSW_MASK)
>>
>> +extern uint64_t psw_save_io[], psw_save_ext[];
>> static uint64_t *reset_psw = 0, save_psw, ipl_continue;
>>
>> void write_reset_psw(uint64_t psw)
>> @@ -59,6 +60,9 @@ void jump_to_IPL_code(uint64_t address)
>> /* Ensure the guest output starts fresh */
>> sclp_print("\n");
>>
>> + memcpy(&lowcore->io_new_psw, psw_save_io, 16);
>> + memcpy(&lowcore->external_new_psw, psw_save_ext, 16);
>> +
>> /*
>> * HACK ALERT.
>> * We use the load normal reset to keep r15 unchanged. jump_to_IPL_2
>> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
>> index 056e93a818..74ef28fbc6 100644
>> --- a/pc-bios/s390-ccw/netmain.c
>> +++ b/pc-bios/s390-ccw/netmain.c
>> @@ -32,6 +32,7 @@
>> #include <time.h>
>> #include <pxelinux.h>
>>
>> +#include "s390-arch.h"
>> #include "s390-ccw.h"
>> #include "cio.h"
>> #include "virtio.h"
>> @@ -43,6 +44,8 @@
>> extern char _start[];
>> void write_iplb_location(void) {}
>>
>> +LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */
>> +
>> #define KERNEL_ADDR ((void *)0L)
>> #define KERNEL_MAX_SIZE ((long)_start)
>> #define ARCH_COMMAND_LINE_SIZE 896 /* Taken from Linux kernel
>> */
>> diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
>> index ce519300a1..939aac3a7c 100644
>> --- a/pc-bios/s390-ccw/start.S
>> +++ b/pc-bios/s390-ccw/start.S
>> @@ -34,7 +34,17 @@ remainder:
>> larl %r2,memsetxc
>> ex %r3,0(%r2)
>> done:
>> - j main /* And call C */
>> + /* prepare i/o call handler */
>> + larl %r1, io_new_code
>> + larl %r2, io_new_psw
>> + stg %r1, 8(%r2)
>> + mvc 0x1f0(16),0(%r2)
>> + /* prepare external call handler */
>> + larl %r1, external_new_code
>> + larl %r2, external_new_psw
>> + stg %r1, 8(%r2)
>
> Can't you specify the external_new_code and io_new_code in the
> external_new_psw / io_new_psw directly? Or is our relocation code not
> good enough for this?
Initially I had some problems with this. I just had another try and it
seems to work well, but as the testing infrastructure doesn't really
work, I can't vouch for that.
>
>> + mvc 0x1b0(16),0(%r2)
>> + j main /* And call C */
>>
>> memsetxc:
>> xc 0(1,%r1),0(%r1)
>> @@ -64,13 +74,16 @@ consume_sclp_int:
>> oi 6(%r15),0x2
>> lctlg %c0,%c0,0(%r15)
>> /* prepare external call handler */
>> - larl %r1, external_new_code
>> - stg %r1, 0x1b8
>> - larl %r1, external_new_mask
>> - mvc 0x1b0(8),0(%r1)
>> - /* load enabled wait PSW */
>> - larl %r1, enabled_wait_psw
>> - lpswe 0(%r1)
>> + larl %r1, external_new_psw
>> + lghi %r2, 0x1b0
>> + /* Is the BIOS' external new PSW already set? */
>> + clc 0(16, %r1), 0(%r2)
>> + je load_ewait
>> + /* No, save old PSW and write BIOS PSW */
>> + larl %r3, psw_save_ext
>> + mvc 0(16, %r3), 0x1b0
>> + mvc 0x1b0(16),0(%r1)
>> + j load_ewait
>>
>> /*
>> * void consume_io_int(void)
>> @@ -84,11 +97,20 @@ consume_io_int:
>> oi 4(%r15), 0xff
>> lctlg %c6,%c6,0(%r15)
>> /* prepare i/o call handler */
>> - larl %r1, io_new_code
>> - stg %r1, 0x1f8
>> - larl %r1, io_new_mask
>> - mvc 0x1f0(8),0(%r1)
>> - /* load enabled wait PSW */
>> + larl %r1, io_new_psw
>> + lghi %r2, 0x1f0
>> + /* Is the BIOS' PSW already set? */
>> + larl %r3, load_ewait
>> + clc 0(16, %r1), 0(%r2)
>> + bcr 8, %r3
>
> Why not a "je load_ewait" again, like in the consume_sclp_int handler?
Great question
>
>> + /* No, save old PSW and write BIOS PSW */
>> + larl %r3, psw_save_io
>> + mvc 0(16, %r3), 0x1f0
>> + mvc 0x1f0(16),0(%r1)
>> + j load_ewait
>> +
>> +load_ewait:
>> + /* PSW is the correct one, time to load the enabled wait PSW */
>> larl %r1, enabled_wait_psw
>> lpswe 0(%r1)
>>
>> @@ -107,11 +129,17 @@ io_new_code:
>> br %r14
>>
>> .align 8
>> + .globl psw_save_io
>> + .globl psw_save_ext
>> disabled_wait_psw:
>> .quad 0x0002000180000000,0x0000000000000000
>> enabled_wait_psw:
>> .quad 0x0302000180000000,0x0000000000000000
>> -external_new_mask:
>> - .quad 0x0000000180000000
>> -io_new_mask:
>> - .quad 0x0000000180000000
>> +external_new_psw:
>> + .quad 0x0000000180000000,0
>> +io_new_psw:
>> + .quad 0x0000000180000000,0
>> +psw_save_io:
>> + .quad 0,0
>> +psw_save_ext:
>> + .quad 0,0
>>
>
> In case you respin, could you maybe add some local #defines for 0x1f0
> and 0x1b0 ?
At the top of this file?
>
> Thomas
>
signature.asc
Description: OpenPGP digital signature