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

[Janosch Frank]

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.

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

signature.asc
Description: OpenPGP digital signature