Re: [PATCH] pc-bios: s390x: Add a comment to the io and external new PSW setup

[Christian Borntraeger]

On 22.07.20 09:24, Janosch Frank wrote:
> On 7/22/20 8:43 AM, Christian Borntraeger wrote:
>>
>>
>> On 15.07.20 16:08, Janosch Frank wrote:
>>> Normally they don't need to be set up before waiting for an interrupt
>>> but are set up on boot. The BIOS however might overwrite the lowcore
>>> (and hence the PSWs) when loading a blob into memory and therefore
>>> needs to set up those PSWs more often.
>>
>> Now when I read the new comment this actually inidicates a bug. 
>> When do we restore the original content? If the loaded program
>> does have interrupt handlers in the original image and relies on that
>> then we are broken, no?
> 
> I haven't seen references to a save/restore functionality for those
> PSWs. And I also think it's not that easy to do because we have multiple
> ways of loading data and if we want to print when loading we might end
> up overwriting and then saving the written value for a later restore.
> 
> I need to have a closer look at how virtio works, but wouldn't we have a
> chicken - egg problem with IO interrupts for IO that writes the prefix?
> 
> The BIOS often has "interesting" solutions to problems.
> If you have a quick fix, be my guest and send it. If not I'd put it on
> my todo list or let Stefan make it a proper dev item.

Maybe a global fixup table in BIOS memory that restores all the memory that
we messed with when we hand over control? Can you at least change the comment
here to add a fixme?

> 
>>
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>  pc-bios/s390-ccw/start.S | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S
>>> index 01c4c21b26..b0fcb918cc 100644
>>> --- a/pc-bios/s390-ccw/start.S
>>> +++ b/pc-bios/s390-ccw/start.S
>>> @@ -64,7 +64,10 @@ consume_sclp_int:
>>>          stctg   %c0,%c0,0(%r15)
>>>          oi      6(%r15),0x2
>>>          lctlg   %c0,%c0,0(%r15)
>>> -        /* prepare external call handler */
>>> +        /*
>>> +         * Prepare external new PSW as it might have been overwritten
>>> +         * by a loaded blob
>>> +         */
>>>          larl %r1, external_new_code
>>>          stg %r1, 0x1b8
>>>          larl %r1, external_new_mask
>>> @@ -84,7 +87,10 @@ consume_io_int:
>>>          stctg %c6,%c6,0(%r15)
>>>          oi    4(%r15), 0xff
>>>          lctlg %c6,%c6,0(%r15)
>>> -        /* prepare i/o call handler */
>>> +        /*
>>> +         * Prepare i/o new PSW as it might have been overwritten
>>> +         * by a loaded blob
>>> +         */
>>>          larl  %r1, io_new_code
>>>          stg   %r1, 0x1f8
>>>          larl  %r1, io_new_mask
>>>
> 
>