All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL
@ 2020-08-17 14:17 Jason J. Herne
  2020-08-17 16:30 ` Cornelia Huck
  0 siblings, 1 reply; 8+ messages in thread
From: Jason J. Herne @ 2020-08-17 14:17 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x

The POP states that the IPLB location is only written to 0x14 for
list-directed IPL. Some operating systems expect 0x14 to not change on
boot and will fail IPL if it does change.

Fixes: 9bfc04f9ef6802fff0
Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@de.ibm.com>
---
 pc-bios/s390-ccw/jump2ipl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
index 767012bf0c..5e3e13f4b0 100644
--- a/pc-bios/s390-ccw/jump2ipl.c
+++ b/pc-bios/s390-ccw/jump2ipl.c
@@ -33,7 +33,10 @@ void jump_to_IPL_code(uint64_t address)
 {
     /* store the subsystem information _after_ the bootmap was loaded */
     write_subsystem_identification();
-    write_iplb_location();
+
+    if (iplb.pbt != S390_IPL_TYPE_CCW) {
+            write_iplb_location();
+    }
 
     /* prevent unknown IPL types in the guest */
     if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL
  2020-08-17 14:17 [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL Jason J. Herne
@ 2020-08-17 16:30 ` Cornelia Huck
  2020-08-17 17:51   ` Jason J. Herne
  0 siblings, 1 reply; 8+ messages in thread
From: Cornelia Huck @ 2020-08-17 16:30 UTC (permalink / raw)
  To: Jason J. Herne; +Cc: qemu-s390x, qemu-devel

On Mon, 17 Aug 2020 10:17:34 -0400
"Jason J. Herne" <jjherne@linux.ibm.com> wrote:

> The POP states that the IPLB location is only written to 0x14 for
> list-directed IPL. Some operating systems expect 0x14 to not change on
> boot and will fail IPL if it does change.
> 
> Fixes: 9bfc04f9ef6802fff0

Should be

Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")

> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> Reviewed-by: Janosch Frank <frankja@de.ibm.com>
> ---
>  pc-bios/s390-ccw/jump2ipl.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
> index 767012bf0c..5e3e13f4b0 100644
> --- a/pc-bios/s390-ccw/jump2ipl.c
> +++ b/pc-bios/s390-ccw/jump2ipl.c
> @@ -33,7 +33,10 @@ void jump_to_IPL_code(uint64_t address)
>  {
>      /* store the subsystem information _after_ the bootmap was loaded */
>      write_subsystem_identification();
> -    write_iplb_location();
> +
> +    if (iplb.pbt != S390_IPL_TYPE_CCW) {
> +            write_iplb_location();
> +    }

What happens for ipl types other than CCW and FCP? IOW, should that
rather be a positive check for S390_IPL_TYPE_FCP?

>  
>      /* prevent unknown IPL types in the guest */
>      if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL
  2020-08-17 16:30 ` Cornelia Huck
@ 2020-08-17 17:51   ` Jason J. Herne
  2020-08-19  9:32     ` Janosch Frank
  0 siblings, 1 reply; 8+ messages in thread
From: Jason J. Herne @ 2020-08-17 17:51 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: qemu-s390x, qemu-devel

On 8/17/20 12:30 PM, Cornelia Huck wrote:
> On Mon, 17 Aug 2020 10:17:34 -0400
> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> 
>> The POP states that the IPLB location is only written to 0x14 for
>> list-directed IPL. Some operating systems expect 0x14 to not change on
>> boot and will fail IPL if it does change.
>>
>> Fixes: 9bfc04f9ef6802fff0
> 
> Should be
> 
> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
> 
>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>> Reviewed-by: Janosch Frank <frankja@de.ibm.com>
>> ---
>>   pc-bios/s390-ccw/jump2ipl.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>> index 767012bf0c..5e3e13f4b0 100644
>> --- a/pc-bios/s390-ccw/jump2ipl.c
>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>> @@ -33,7 +33,10 @@ void jump_to_IPL_code(uint64_t address)
>>   {
>>       /* store the subsystem information _after_ the bootmap was loaded */
>>       write_subsystem_identification();
>> -    write_iplb_location();
>> +
>> +    if (iplb.pbt != S390_IPL_TYPE_CCW) {
>> +            write_iplb_location();
>> +    }
> 
> What happens for ipl types other than CCW and FCP? IOW, should that
> rather be a positive check for S390_IPL_TYPE_FCP?
> 
>>   
>>       /* prevent unknown IPL types in the guest */
>>       if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {
> 

Based on my (admittedly limited) understanding of the architecture and
code, I believe write_iplb_location() should be called at least for
S390_IPL_TYPE_FCP but I'm not 100% sure on S390_IPL_TYPE_QEMU_SCSI.
Perhaps Janosch has an idea?

It was originally unconditional, and my new conditional excludes vfio
CCW which is definitely a step in the right direction, in any case :).


-- 
-- Jason J. Herne (jjherne@linux.ibm.com)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL
  2020-08-17 17:51   ` Jason J. Herne
@ 2020-08-19  9:32     ` Janosch Frank
  2020-08-19  9:45       ` Cornelia Huck
  0 siblings, 1 reply; 8+ messages in thread
From: Janosch Frank @ 2020-08-19  9:32 UTC (permalink / raw)
  To: jjherne, Cornelia Huck; +Cc: qemu-s390x, Viktor Mihajlovski, qemu-devel


[-- Attachment #1.1: Type: text/plain, Size: 2341 bytes --]

On 8/17/20 7:51 PM, Jason J. Herne wrote:
> On 8/17/20 12:30 PM, Cornelia Huck wrote:
>> On Mon, 17 Aug 2020 10:17:34 -0400
>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>>
>>> The POP states that the IPLB location is only written to 0x14 for
>>> list-directed IPL. Some operating systems expect 0x14 to not change on
>>> boot and will fail IPL if it does change.
>>>
>>> Fixes: 9bfc04f9ef6802fff0
>>
>> Should be
>>
>> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
>>
>>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>>> Reviewed-by: Janosch Frank <frankja@de.ibm.com>
>>> ---
>>>   pc-bios/s390-ccw/jump2ipl.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>>> index 767012bf0c..5e3e13f4b0 100644
>>> --- a/pc-bios/s390-ccw/jump2ipl.c
>>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>>> @@ -33,7 +33,10 @@ void jump_to_IPL_code(uint64_t address)
>>>   {
>>>       /* store the subsystem information _after_ the bootmap was loaded */
>>>       write_subsystem_identification();
>>> -    write_iplb_location();
>>> +
>>> +    if (iplb.pbt != S390_IPL_TYPE_CCW) {
>>> +            write_iplb_location();
>>> +    }
>>
>> What happens for ipl types other than CCW and FCP? IOW, should that
>> rather be a positive check for S390_IPL_TYPE_FCP?
>>
>>>   
>>>       /* prevent unknown IPL types in the guest */
>>>       if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {
>>
> 
> Based on my (admittedly limited) understanding of the architecture and
> code, I believe write_iplb_location() should be called at least for
> S390_IPL_TYPE_FCP but I'm not 100% sure on S390_IPL_TYPE_QEMU_SCSI.
> Perhaps Janosch has an idea?
> 
> It was originally unconditional, and my new conditional excludes vfio
> CCW which is definitely a step in the right direction, in any case :).

If I remember correctly the problem was that ZIPL used the IPLB lowcore
ptr without checking how it was booted (CCW or FCP). That was fixed in
mid of July by testing if diag308 gives back a config or not.

So we might have a deadlock situation here which I need to think about
for a bit. I'm setting Viktor CC to get a bit more information about the
state of the zipl backports into the distros.




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL
  2020-08-19  9:32     ` Janosch Frank
@ 2020-08-19  9:45       ` Cornelia Huck
  2020-08-19 10:46         ` Janosch Frank
  0 siblings, 1 reply; 8+ messages in thread
From: Cornelia Huck @ 2020-08-19  9:45 UTC (permalink / raw)
  To: Janosch Frank; +Cc: jjherne, qemu-s390x, Viktor Mihajlovski, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 3043 bytes --]

On Wed, 19 Aug 2020 11:32:34 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 8/17/20 7:51 PM, Jason J. Herne wrote:
> > On 8/17/20 12:30 PM, Cornelia Huck wrote:  
> >> On Mon, 17 Aug 2020 10:17:34 -0400
> >> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
> >>  
> >>> The POP states that the IPLB location is only written to 0x14 for
> >>> list-directed IPL. Some operating systems expect 0x14 to not change on
> >>> boot and will fail IPL if it does change.
> >>>
> >>> Fixes: 9bfc04f9ef6802fff0  
> >>
> >> Should be
> >>
> >> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
> >>  
> >>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> >>> Reviewed-by: Janosch Frank <frankja@de.ibm.com>
> >>> ---
> >>>   pc-bios/s390-ccw/jump2ipl.c | 5 ++++-
> >>>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
> >>> index 767012bf0c..5e3e13f4b0 100644
> >>> --- a/pc-bios/s390-ccw/jump2ipl.c
> >>> +++ b/pc-bios/s390-ccw/jump2ipl.c
> >>> @@ -33,7 +33,10 @@ void jump_to_IPL_code(uint64_t address)
> >>>   {
> >>>       /* store the subsystem information _after_ the bootmap was loaded */
> >>>       write_subsystem_identification();
> >>> -    write_iplb_location();
> >>> +
> >>> +    if (iplb.pbt != S390_IPL_TYPE_CCW) {
> >>> +            write_iplb_location();
> >>> +    }  
> >>
> >> What happens for ipl types other than CCW and FCP? IOW, should that
> >> rather be a positive check for S390_IPL_TYPE_FCP?
> >>  
> >>>   
> >>>       /* prevent unknown IPL types in the guest */
> >>>       if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {  
> >>  
> > 
> > Based on my (admittedly limited) understanding of the architecture and
> > code, I believe write_iplb_location() should be called at least for
> > S390_IPL_TYPE_FCP but I'm not 100% sure on S390_IPL_TYPE_QEMU_SCSI.
> > Perhaps Janosch has an idea?
> > 
> > It was originally unconditional, and my new conditional excludes vfio
> > CCW which is definitely a step in the right direction, in any case :).  
> 
> If I remember correctly the problem was that ZIPL used the IPLB lowcore
> ptr without checking how it was booted (CCW or FCP). That was fixed in
> mid of July by testing if diag308 gives back a config or not.

So we have the problem that old zipl relies on the presence of a value
that must not be there if you follow the architecture? Nasty.

(Is it really "must not change" vs "don't expect anything here"? Not
sure if I'm looking at the right part of the documentation.)

> So we might have a deadlock situation here which I need to think about
> for a bit. I'm setting Viktor CC to get a bit more information about the
> state of the zipl backports into the distros.

Changing this is problematic unless everything we support as a guest is
fixed. Does the guest go boom in a way that it is at least easy to
figure out what went wrong? What breaks when the value continues to be
set?

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL
  2020-08-19  9:45       ` Cornelia Huck
@ 2020-08-19 10:46         ` Janosch Frank
  2020-08-25 11:38           ` Thomas Huth
  0 siblings, 1 reply; 8+ messages in thread
From: Janosch Frank @ 2020-08-19 10:46 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: jjherne, qemu-s390x, Viktor Mihajlovski, qemu-devel


[-- Attachment #1.1: Type: text/plain, Size: 3705 bytes --]

On 8/19/20 11:45 AM, Cornelia Huck wrote:
> On Wed, 19 Aug 2020 11:32:34 +0200
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 8/17/20 7:51 PM, Jason J. Herne wrote:
>>> On 8/17/20 12:30 PM, Cornelia Huck wrote:  
>>>> On Mon, 17 Aug 2020 10:17:34 -0400
>>>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>>>>  
>>>>> The POP states that the IPLB location is only written to 0x14 for
>>>>> list-directed IPL. Some operating systems expect 0x14 to not change on
>>>>> boot and will fail IPL if it does change.
>>>>>
>>>>> Fixes: 9bfc04f9ef6802fff0  
>>>>
>>>> Should be
>>>>
>>>> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
>>>>  
>>>>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>>>>> Reviewed-by: Janosch Frank <frankja@de.ibm.com>
>>>>> ---
>>>>>   pc-bios/s390-ccw/jump2ipl.c | 5 ++++-
>>>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>>>>> index 767012bf0c..5e3e13f4b0 100644
>>>>> --- a/pc-bios/s390-ccw/jump2ipl.c
>>>>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>>>>> @@ -33,7 +33,10 @@ void jump_to_IPL_code(uint64_t address)
>>>>>   {
>>>>>       /* store the subsystem information _after_ the bootmap was loaded */
>>>>>       write_subsystem_identification();
>>>>> -    write_iplb_location();
>>>>> +
>>>>> +    if (iplb.pbt != S390_IPL_TYPE_CCW) {
>>>>> +            write_iplb_location();
>>>>> +    }  
>>>>
>>>> What happens for ipl types other than CCW and FCP? IOW, should that
>>>> rather be a positive check for S390_IPL_TYPE_FCP?
>>>>  
>>>>>   
>>>>>       /* prevent unknown IPL types in the guest */
>>>>>       if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {  
>>>>  
>>>
>>> Based on my (admittedly limited) understanding of the architecture and
>>> code, I believe write_iplb_location() should be called at least for
>>> S390_IPL_TYPE_FCP but I'm not 100% sure on S390_IPL_TYPE_QEMU_SCSI.
>>> Perhaps Janosch has an idea?
>>>
>>> It was originally unconditional, and my new conditional excludes vfio
>>> CCW which is definitely a step in the right direction, in any case :).  
>>
>> If I remember correctly the problem was that ZIPL used the IPLB lowcore
>> ptr without checking how it was booted (CCW or FCP). That was fixed in
>> mid of July by testing if diag308 gives back a config or not.
> 
> So we have the problem that old zipl relies on the presence of a value
> that must not be there if you follow the architecture? Nasty.
> 
> (Is it really "must not change" vs "don't expect anything here"? Not
> sure if I'm looking at the right part of the documentation.)

Well if the loaded program overwrites absolute 0x0, we shouldn't modify
it if we are not explicitly allowed to, no?

We already talked about saving the exception new addresses and restoring
them before jumping to the new kernel. I think we might need to go a
step further and use a non zero prefix for the bios to avoid any changes
to absolute 0x0.

However that wouldn't fix this dilemma.

> 
>> So we might have a deadlock situation here which I need to think about
>> for a bit. I'm setting Viktor CC to get a bit more information about the
>> state of the zipl backports into the distros.
> 
> Changing this is problematic unless everything we support as a guest is
> fixed. Does the guest go boom in a way that it is at least easy to
> figure out what went wrong? What breaks when the value continues to be
> set?
> 
I think it goes into disabled wait because of the failed secure boot
verification or gets a PGM Addressing and then goes into disabled wait,
so no, not very user friendly.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL
  2020-08-19 10:46         ` Janosch Frank
@ 2020-08-25 11:38           ` Thomas Huth
  2020-08-25 14:45             ` Jason J. Herne
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2020-08-25 11:38 UTC (permalink / raw)
  To: Janosch Frank, Cornelia Huck
  Cc: jjherne, qemu-s390x, Viktor Mihajlovski, qemu-devel

On 19/08/2020 12.46, Janosch Frank wrote:
> On 8/19/20 11:45 AM, Cornelia Huck wrote:
>> On Wed, 19 Aug 2020 11:32:34 +0200
>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>
>>> On 8/17/20 7:51 PM, Jason J. Herne wrote:
>>>> On 8/17/20 12:30 PM, Cornelia Huck wrote:  
>>>>> On Mon, 17 Aug 2020 10:17:34 -0400
>>>>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>>>>>  
>>>>>> The POP states that the IPLB location is only written to 0x14 for
>>>>>> list-directed IPL. Some operating systems expect 0x14 to not change on
>>>>>> boot and will fail IPL if it does change.
>>>>>>
>>>>>> Fixes: 9bfc04f9ef6802fff0  
>>>>>
>>>>> Should be
>>>>>
>>>>> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
>>>>>  
>>>>>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>>>>>> Reviewed-by: Janosch Frank <frankja@de.ibm.com>
>>>>>> ---
>>>>>>   pc-bios/s390-ccw/jump2ipl.c | 5 ++++-
>>>>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>>>>>> index 767012bf0c..5e3e13f4b0 100644
>>>>>> --- a/pc-bios/s390-ccw/jump2ipl.c
>>>>>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>>>>>> @@ -33,7 +33,10 @@ void jump_to_IPL_code(uint64_t address)
>>>>>>   {
>>>>>>       /* store the subsystem information _after_ the bootmap was loaded */
>>>>>>       write_subsystem_identification();
>>>>>> -    write_iplb_location();
>>>>>> +
>>>>>> +    if (iplb.pbt != S390_IPL_TYPE_CCW) {
>>>>>> +            write_iplb_location();
>>>>>> +    }  
>>>>>
>>>>> What happens for ipl types other than CCW and FCP? IOW, should that
>>>>> rather be a positive check for S390_IPL_TYPE_FCP?
>>>>>  
>>>>>>   
>>>>>>       /* prevent unknown IPL types in the guest */
>>>>>>       if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {  
>>>>>  
>>>>
>>>> Based on my (admittedly limited) understanding of the architecture and
>>>> code, I believe write_iplb_location() should be called at least for
>>>> S390_IPL_TYPE_FCP but I'm not 100% sure on S390_IPL_TYPE_QEMU_SCSI.
>>>> Perhaps Janosch has an idea?
>>>>
>>>> It was originally unconditional, and my new conditional excludes vfio
>>>> CCW which is definitely a step in the right direction, in any case :).  
>>>
>>> If I remember correctly the problem was that ZIPL used the IPLB lowcore
>>> ptr without checking how it was booted (CCW or FCP). That was fixed in
>>> mid of July by testing if diag308 gives back a config or not.
>>
>> So we have the problem that old zipl relies on the presence of a value
>> that must not be there if you follow the architecture? Nasty.
>>
>> (Is it really "must not change" vs "don't expect anything here"? Not
>> sure if I'm looking at the right part of the documentation.)
> 
> Well if the loaded program overwrites absolute 0x0, we shouldn't modify
> it if we are not explicitly allowed to, no?
> 
> We already talked about saving the exception new addresses and restoring
> them before jumping to the new kernel. I think we might need to go a
> step further and use a non zero prefix for the bios to avoid any changes
> to absolute 0x0.
> 
> However that wouldn't fix this dilemma.

Sorry, I'm just back from summer vacation ... so what's the conclusion
for Jason's patch here? Should it be included as-is now or do we rather
neeed another rework here instead?

 Thomas



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL
  2020-08-25 11:38           ` Thomas Huth
@ 2020-08-25 14:45             ` Jason J. Herne
  0 siblings, 0 replies; 8+ messages in thread
From: Jason J. Herne @ 2020-08-25 14:45 UTC (permalink / raw)
  To: Thomas Huth, Janosch Frank, Cornelia Huck
  Cc: qemu-s390x, Viktor Mihajlovski, qemu-devel

On 8/25/20 7:38 AM, Thomas Huth wrote:
> On 19/08/2020 12.46, Janosch Frank wrote:
>> On 8/19/20 11:45 AM, Cornelia Huck wrote:
>>> On Wed, 19 Aug 2020 11:32:34 +0200
>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>
>>>> On 8/17/20 7:51 PM, Jason J. Herne wrote:
>>>>> On 8/17/20 12:30 PM, Cornelia Huck wrote:
>>>>>> On Mon, 17 Aug 2020 10:17:34 -0400
>>>>>> "Jason J. Herne" <jjherne@linux.ibm.com> wrote:
>>>>>>   
>>>>>>> The POP states that the IPLB location is only written to 0x14 for
>>>>>>> list-directed IPL. Some operating systems expect 0x14 to not change on
>>>>>>> boot and will fail IPL if it does change.
>>>>>>>
>>>>>>> Fixes: 9bfc04f9ef6802fff0
>>>>>>
>>>>>> Should be
>>>>>>
>>>>>> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
>>>>>>   
>>>>>>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>>>>>>> Reviewed-by: Janosch Frank <frankja@de.ibm.com>
>>>>>>> ---
>>>>>>>    pc-bios/s390-ccw/jump2ipl.c | 5 ++++-
>>>>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>>>>>>> index 767012bf0c..5e3e13f4b0 100644
>>>>>>> --- a/pc-bios/s390-ccw/jump2ipl.c
>>>>>>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>>>>>>> @@ -33,7 +33,10 @@ void jump_to_IPL_code(uint64_t address)
>>>>>>>    {
>>>>>>>        /* store the subsystem information _after_ the bootmap was loaded */
>>>>>>>        write_subsystem_identification();
>>>>>>> -    write_iplb_location();
>>>>>>> +
>>>>>>> +    if (iplb.pbt != S390_IPL_TYPE_CCW) {
>>>>>>> +            write_iplb_location();
>>>>>>> +    }
>>>>>>
>>>>>> What happens for ipl types other than CCW and FCP? IOW, should that
>>>>>> rather be a positive check for S390_IPL_TYPE_FCP?
>>>>>>   
>>>>>>>    
>>>>>>>        /* prevent unknown IPL types in the guest */
>>>>>>>        if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {
>>>>>>   
>>>>>
>>>>> Based on my (admittedly limited) understanding of the architecture and
>>>>> code, I believe write_iplb_location() should be called at least for
>>>>> S390_IPL_TYPE_FCP but I'm not 100% sure on S390_IPL_TYPE_QEMU_SCSI.
>>>>> Perhaps Janosch has an idea?
>>>>>
>>>>> It was originally unconditional, and my new conditional excludes vfio
>>>>> CCW which is definitely a step in the right direction, in any case :).
>>>>
>>>> If I remember correctly the problem was that ZIPL used the IPLB lowcore
>>>> ptr without checking how it was booted (CCW or FCP). That was fixed in
>>>> mid of July by testing if diag308 gives back a config or not.
>>>
>>> So we have the problem that old zipl relies on the presence of a value
>>> that must not be there if you follow the architecture? Nasty.
>>>
>>> (Is it really "must not change" vs "don't expect anything here"? Not
>>> sure if I'm looking at the right part of the documentation.)
>>
>> Well if the loaded program overwrites absolute 0x0, we shouldn't modify
>> it if we are not explicitly allowed to, no?
>>
>> We already talked about saving the exception new addresses and restoring
>> them before jumping to the new kernel. I think we might need to go a
>> step further and use a non zero prefix for the bios to avoid any changes
>> to absolute 0x0.
>>
>> However that wouldn't fix this dilemma.
> 
> Sorry, I'm just back from summer vacation ... so what's the conclusion
> for Jason's patch here? Should it be included as-is now or do we rather
> neeed another rework here instead?
> 
>   Thomas
> 

After some discussion we've decided to keep this internal for now. As it turns out, 
Janosch plans on making some changes here that will remove the need for this fix. So we'll 
just use it internally for our own testing purposes until that time.

Sorry for the noise :)

-- 
-- Jason J. Herne (jjherne@linux.ibm.com)


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-08-25 14:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-17 14:17 [PATCH] pc-bios: s390x: Only set lowcore iplb address on list-directed IPL Jason J. Herne
2020-08-17 16:30 ` Cornelia Huck
2020-08-17 17:51   ` Jason J. Herne
2020-08-19  9:32     ` Janosch Frank
2020-08-19  9:45       ` Cornelia Huck
2020-08-19 10:46         ` Janosch Frank
2020-08-25 11:38           ` Thomas Huth
2020-08-25 14:45             ` Jason J. Herne

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.