diff mbox series

s390-bios: Skip writing iplb location to low core for ccw ipl

Message ID 20201030122823.347140-1-borntraeger@de.ibm.com
State New
Headers show
Series s390-bios: Skip writing iplb location to low core for ccw ipl | expand

Commit Message

Christian Borntraeger Oct. 30, 2020, 12:28 p.m. UTC
From: "Jason J. Herne" <jjherne@linux.ibm.com>

The architecture states that the iplb location is only written to low
core for list directed ipl and not for traditional ccw ipl. If we don't
skip this then operating systems that load by reading into low core
memory may fail to start.

We should also not write the iplb pointer for network boot as it might
overwrite content that we got via network.

Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 pc-bios/s390-ccw/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Christian Borntraeger Nov. 3, 2020, 12:08 p.m. UTC | #1
On 30.10.20 13:28, Christian Borntraeger wrote:
> From: "Jason J. Herne" <jjherne@linux.ibm.com>
> 
> The architecture states that the iplb location is only written to low
> core for list directed ipl and not for traditional ccw ipl. If we don't
> skip this then operating systems that load by reading into low core
> memory may fail to start.
> 
> We should also not write the iplb pointer for network boot as it might
> overwrite content that we got via network.
> 
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

FWIW, this fixes the vfio-ccw IPL for some non Linux binaries.
> ---
>  pc-bios/s390-ccw/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 43c792cf9509..fc4bfaa45529 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -43,7 +43,9 @@ void write_subsystem_identification(void)
>  
>  void write_iplb_location(void)
>  {
> -    lowcore->ptr_iplb = ptr2u32(&iplb);
> +    if (cutype == CU_TYPE_VIRTIO && virtio_get_device_type() != VIRTIO_ID_NET) {
> +        lowcore->ptr_iplb = ptr2u32(&iplb);
> +    }
>  }
>  
>  unsigned int get_loadparm_index(void)
>
Thomas Huth Nov. 3, 2020, 12:32 p.m. UTC | #2
On 30/10/2020 13.28, Christian Borntraeger wrote:
> From: "Jason J. Herne" <jjherne@linux.ibm.com>
> 
> The architecture states that the iplb location is only written to low
> core for list directed ipl and not for traditional ccw ipl. If we don't
> skip this then operating systems that load by reading into low core
> memory may fail to start.

Just double-checking: But doing write_subsystem_identification()
unconditionally is ok, right?

> We should also not write the iplb pointer for network boot as it might
> overwrite content that we got via network.

FWIW, write_iplb_location() is already just a dummy function in netmain.c,
so this should not have been an issue in the network bootloader, I hope.

> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  pc-bios/s390-ccw/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 43c792cf9509..fc4bfaa45529 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -43,7 +43,9 @@ void write_subsystem_identification(void)
>  
>  void write_iplb_location(void)
>  {
> -    lowcore->ptr_iplb = ptr2u32(&iplb);
> +    if (cutype == CU_TYPE_VIRTIO && virtio_get_device_type() != VIRTIO_ID_NET) {
> +        lowcore->ptr_iplb = ptr2u32(&iplb);
> +    }
>  }

Acked-by: Thomas Huth <thuth@redhat.com>

Christian, Cornelia, could you please pick up the patch? I'm not sure
whether I can do another PR this week for the RC...
Cornelia Huck Nov. 3, 2020, 12:35 p.m. UTC | #3
On Tue, 3 Nov 2020 13:32:47 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 30/10/2020 13.28, Christian Borntraeger wrote:
> > From: "Jason J. Herne" <jjherne@linux.ibm.com>
> > 
> > The architecture states that the iplb location is only written to low
> > core for list directed ipl and not for traditional ccw ipl. If we don't
> > skip this then operating systems that load by reading into low core
> > memory may fail to start.  
> 
> Just double-checking: But doing write_subsystem_identification()
> unconditionally is ok, right?
> 
> > We should also not write the iplb pointer for network boot as it might
> > overwrite content that we got via network.  
> 
> FWIW, write_iplb_location() is already just a dummy function in netmain.c,
> so this should not have been an issue in the network bootloader, I hope.
> 
> > Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> > Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> >  pc-bios/s390-ccw/main.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> > index 43c792cf9509..fc4bfaa45529 100644
> > --- a/pc-bios/s390-ccw/main.c
> > +++ b/pc-bios/s390-ccw/main.c
> > @@ -43,7 +43,9 @@ void write_subsystem_identification(void)
> >  
> >  void write_iplb_location(void)
> >  {
> > -    lowcore->ptr_iplb = ptr2u32(&iplb);
> > +    if (cutype == CU_TYPE_VIRTIO && virtio_get_device_type() != VIRTIO_ID_NET) {
> > +        lowcore->ptr_iplb = ptr2u32(&iplb);
> > +    }
> >  }  
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> 
> Christian, Cornelia, could you please pick up the patch? I'm not sure
> whether I can do another PR this week for the RC...

Will do, I have other things as well anyway.
Christian Borntraeger Nov. 3, 2020, 5:15 p.m. UTC | #4
On 03.11.20 13:32, Thomas Huth wrote:
> On 30/10/2020 13.28, Christian Borntraeger wrote:
>> From: "Jason J. Herne" <jjherne@linux.ibm.com>
>>
>> The architecture states that the iplb location is only written to low
>> core for list directed ipl and not for traditional ccw ipl. If we don't
>> skip this then operating systems that load by reading into low core
>> memory may fail to start.
> 
> Just double-checking: But doing write_subsystem_identification()
> unconditionally is ok, right?

Yes, for any channel device IPL the subsystem ID is stored in absolute locations
184-187 so this should be good as virtio is a channel device.
> 
>> We should also not write the iplb pointer for network boot as it might
>> overwrite content that we got via network.
> 
> FWIW, write_iplb_location() is already just a dummy function in netmain.c,
> so this should not have been an issue in the network bootloader, I hope.

OK. I think we can keep the check for !VIRTIO_ID_NET anyway.

> 
>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  pc-bios/s390-ccw/main.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
>> index 43c792cf9509..fc4bfaa45529 100644
>> --- a/pc-bios/s390-ccw/main.c
>> +++ b/pc-bios/s390-ccw/main.c
>> @@ -43,7 +43,9 @@ void write_subsystem_identification(void)
>>  
>>  void write_iplb_location(void)
>>  {
>> -    lowcore->ptr_iplb = ptr2u32(&iplb);
>> +    if (cutype == CU_TYPE_VIRTIO && virtio_get_device_type() != VIRTIO_ID_NET) {
>> +        lowcore->ptr_iplb = ptr2u32(&iplb);
>> +    }
>>  }
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> 
> Christian, Cornelia, could you please pick up the patch? I'm not sure
> whether I can do another PR this week for the RC...
>
Cornelia Huck Nov. 4, 2020, 12:22 p.m. UTC | #5
On Fri, 30 Oct 2020 13:28:23 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: "Jason J. Herne" <jjherne@linux.ibm.com>
> 
> The architecture states that the iplb location is only written to low
> core for list directed ipl and not for traditional ccw ipl. If we don't
> skip this then operating systems that load by reading into low core
> memory may fail to start.
> 
> We should also not write the iplb pointer for network boot as it might
> overwrite content that we got via network.
> 
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  pc-bios/s390-ccw/main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thanks, queued (together with a rebuild) to s390-fixes.
diff mbox series

Patch

diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 43c792cf9509..fc4bfaa45529 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -43,7 +43,9 @@  void write_subsystem_identification(void)
 
 void write_iplb_location(void)
 {
-    lowcore->ptr_iplb = ptr2u32(&iplb);
+    if (cutype == CU_TYPE_VIRTIO && virtio_get_device_type() != VIRTIO_ID_NET) {
+        lowcore->ptr_iplb = ptr2u32(&iplb);
+    }
 }
 
 unsigned int get_loadparm_index(void)