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 <fran...@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? > + 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? > + /* 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 ? Thomas