On 02/09/2020 12.30, Janosch Frank wrote:
> 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 <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?
>
> 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.
You could maybe dump the memory in both cases to see whether
external_new_psw and io_new_psw contain the same values before and after
the change?
>> In case you respin, could you maybe add some local #defines for 0x1f0
>> and 0x1b0 ?
>
> At the top of this file?
Yes, please.
Thomas
