pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

[PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

Posted by Thomas Huth 6 months, 1 week ago

We originally built a separate binary for the netboot code since it
was considered as experimental and we could not be sure that the
necessary SLOF module had been checked out. Time passed, the netboot
code proved its usefulness, and the build system nowadays makes sure
that the SLOF module is checked out if you have a s390x compiler available
for building the s390-ccw bios. In fact, the possibility to build the
s390-ccw.img without s390-netboot.img has been removed in commit
bf6903f6944f ("pc-bios/s390-ccw: always build network bootloader")
already.

So it does not make too much sense anymore to keep the netboot code
in a separate binary. To make it easier to support a more flexible
boot process soon that supports more than one boot device via the
bootindex properties, let's finally merge the netboot code into the
main s390-ccw.img binary now.

Thomas Huth (7):
  pc-bios/s390-ccw: Remove duplicated LDFLAGS
  hw/s390x/ipl: Provide more memory to the s390-ccw.img firmware
  pc-bios/s390-ccw: Use the libc from SLOF for the main s390-ccw.img
    binary, too
  pc-bios/s390-ccw: Link the netboot code into the main s390-ccw.img
    binary
  hw/s390x: Remove the possibility to load the s390-netboot.img binary
  pc-bios/s390-ccw: Merge netboot.mak into the main Makefile
  docs/system/s390x/bootdevices: Update the documentation about network
    booting

 docs/system/s390x/bootdevices.rst |  20 +++----
 pc-bios/s390-ccw/netboot.mak      |  62 ---------------------
 hw/s390x/ipl.h                    |  12 ++--
 pc-bios/s390-ccw/cio.h            |   2 +
 pc-bios/s390-ccw/iplb.h           |   4 +-
 pc-bios/s390-ccw/libc.h           |  89 ------------------------------
 pc-bios/s390-ccw/s390-ccw.h       |  10 +++-
 pc-bios/s390-ccw/virtio.h         |   1 -
 hw/s390x/ipl.c                    |  65 +++-------------------
 hw/s390x/s390-virtio-ccw.c        |  10 +---
 pc-bios/s390-ccw/bootmap.c        |   4 +-
 pc-bios/s390-ccw/cio.c            |   2 +-
 pc-bios/s390-ccw/dasd-ipl.c       |   2 +-
 pc-bios/s390-ccw/jump2ipl.c       |   2 +-
 pc-bios/s390-ccw/libc.c           |  88 -----------------------------
 pc-bios/s390-ccw/main.c           |  15 +++--
 pc-bios/s390-ccw/menu.c           |  25 ++++-----
 pc-bios/s390-ccw/netmain.c        |  15 +----
 pc-bios/s390-ccw/sclp.c           |   2 +-
 pc-bios/s390-ccw/virtio-blkdev.c  |   1 -
 pc-bios/s390-ccw/virtio-scsi.c    |   2 +-
 pc-bios/s390-ccw/virtio.c         |   2 +-
 pc-bios/meson.build               |   1 -
 pc-bios/s390-ccw/Makefile         |  69 +++++++++++++++++++----
 pc-bios/s390-netboot.img          | Bin 67232 -> 0 bytes
 25 files changed, 122 insertions(+), 383 deletions(-)
 delete mode 100644 pc-bios/s390-ccw/netboot.mak
 delete mode 100644 pc-bios/s390-ccw/libc.h
 delete mode 100644 pc-bios/s390-ccw/libc.c
 delete mode 100644 pc-bios/s390-netboot.img

-- 
2.45.2

Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

Posted by Jared Rossi 6 months, 1 week ago


On 6/21/24 4:24 AM, Thomas Huth wrote:
> We originally built a separate binary for the netboot code since it
> was considered as experimental and we could not be sure that the
> necessary SLOF module had been checked out. Time passed, the netboot
> code proved its usefulness, and the build system nowadays makes sure
> that the SLOF module is checked out if you have a s390x compiler available
> for building the s390-ccw bios. In fact, the possibility to build the
> s390-ccw.img without s390-netboot.img has been removed in commit
> bf6903f6944f ("pc-bios/s390-ccw: always build network bootloader")
> already.
>
> So it does not make too much sense anymore to keep the netboot code
> in a separate binary. To make it easier to support a more flexible
> boot process soon that supports more than one boot device via the
> bootindex properties, let's finally merge the netboot code into the
> main s390-ccw.img binary now.

Hi Thomas, One area that could possibly be cleaned up further are places 
where net devices are treated as corner cases due to the separate 
bootloader. Off the top of my head I know in pc-bios main.c, the 
is_dev_possibly_bootable() function rejects net devices for this reason. 
I'm not sure if that is the only place though. Otherwise it looks good 
to me. I can work on a v2 of the boot order support that assumes the 
network bootloader is integrated. Regards, Jared Rossi

>
> Thomas Huth (7):
>    pc-bios/s390-ccw: Remove duplicated LDFLAGS
>    hw/s390x/ipl: Provide more memory to the s390-ccw.img firmware
>    pc-bios/s390-ccw: Use the libc from SLOF for the main s390-ccw.img
>      binary, too
>    pc-bios/s390-ccw: Link the netboot code into the main s390-ccw.img
>      binary
>    hw/s390x: Remove the possibility to load the s390-netboot.img binary
>    pc-bios/s390-ccw: Merge netboot.mak into the main Makefile
>    docs/system/s390x/bootdevices: Update the documentation about network
>      booting
>
>   docs/system/s390x/bootdevices.rst |  20 +++----
>   pc-bios/s390-ccw/netboot.mak      |  62 ---------------------
>   hw/s390x/ipl.h                    |  12 ++--
>   pc-bios/s390-ccw/cio.h            |   2 +
>   pc-bios/s390-ccw/iplb.h           |   4 +-
>   pc-bios/s390-ccw/libc.h           |  89 ------------------------------
>   pc-bios/s390-ccw/s390-ccw.h       |  10 +++-
>   pc-bios/s390-ccw/virtio.h         |   1 -
>   hw/s390x/ipl.c                    |  65 +++-------------------
>   hw/s390x/s390-virtio-ccw.c        |  10 +---
>   pc-bios/s390-ccw/bootmap.c        |   4 +-
>   pc-bios/s390-ccw/cio.c            |   2 +-
>   pc-bios/s390-ccw/dasd-ipl.c       |   2 +-
>   pc-bios/s390-ccw/jump2ipl.c       |   2 +-
>   pc-bios/s390-ccw/libc.c           |  88 -----------------------------
>   pc-bios/s390-ccw/main.c           |  15 +++--
>   pc-bios/s390-ccw/menu.c           |  25 ++++-----
>   pc-bios/s390-ccw/netmain.c        |  15 +----
>   pc-bios/s390-ccw/sclp.c           |   2 +-
>   pc-bios/s390-ccw/virtio-blkdev.c  |   1 -
>   pc-bios/s390-ccw/virtio-scsi.c    |   2 +-
>   pc-bios/s390-ccw/virtio.c         |   2 +-
>   pc-bios/meson.build               |   1 -
>   pc-bios/s390-ccw/Makefile         |  69 +++++++++++++++++++----
>   pc-bios/s390-netboot.img          | Bin 67232 -> 0 bytes
>   25 files changed, 122 insertions(+), 383 deletions(-)
>   delete mode 100644 pc-bios/s390-ccw/netboot.mak
>   delete mode 100644 pc-bios/s390-ccw/libc.h
>   delete mode 100644 pc-bios/s390-ccw/libc.c
>   delete mode 100644 pc-bios/s390-netboot.img
>

Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

Posted by Eric Farman 6 months, 1 week ago

On Fri, 2024-06-21 at 10:24 +0200, Thomas Huth wrote:
> We originally built a separate binary for the netboot code since it
> was considered as experimental and we could not be sure that the
> necessary SLOF module had been checked out. Time passed, the netboot
> code proved its usefulness, and the build system nowadays makes sure
> that the SLOF module is checked out if you have a s390x compiler
> available
> for building the s390-ccw bios. In fact, the possibility to build the
> s390-ccw.img without s390-netboot.img has been removed in commit
> bf6903f6944f ("pc-bios/s390-ccw: always build network bootloader")
> already.
> 
> So it does not make too much sense anymore to keep the netboot code
> in a separate binary. To make it easier to support a more flexible
> boot process soon that supports more than one boot device via the
> bootindex properties, let's finally merge the netboot code into the
> main s390-ccw.img binary now.

Hi Thomas,

I find myself wondering about the side effects of the
s/sclp_print/printf/ changes, but I haven't come up with anything I can
put my finger on. Maybe something will come to me over the weekend, but
all-in-all I like the looks of this. So, FWIW:

Reviewed-by: Eric Farman <farman@linux.ibm.com>

> 
> Thomas Huth (7):
>   pc-bios/s390-ccw: Remove duplicated LDFLAGS
>   hw/s390x/ipl: Provide more memory to the s390-ccw.img firmware
>   pc-bios/s390-ccw: Use the libc from SLOF for the main s390-ccw.img
>     binary, too
>   pc-bios/s390-ccw: Link the netboot code into the main s390-ccw.img
>     binary
>   hw/s390x: Remove the possibility to load the s390-netboot.img
> binary
>   pc-bios/s390-ccw: Merge netboot.mak into the main Makefile
>   docs/system/s390x/bootdevices: Update the documentation about
> network
>     booting
> 
>  docs/system/s390x/bootdevices.rst |  20 +++----
>  pc-bios/s390-ccw/netboot.mak      |  62 ---------------------
>  hw/s390x/ipl.h                    |  12 ++--
>  pc-bios/s390-ccw/cio.h            |   2 +
>  pc-bios/s390-ccw/iplb.h           |   4 +-
>  pc-bios/s390-ccw/libc.h           |  89 ----------------------------
> --
>  pc-bios/s390-ccw/s390-ccw.h       |  10 +++-
>  pc-bios/s390-ccw/virtio.h         |   1 -
>  hw/s390x/ipl.c                    |  65 +++-------------------
>  hw/s390x/s390-virtio-ccw.c        |  10 +---
>  pc-bios/s390-ccw/bootmap.c        |   4 +-
>  pc-bios/s390-ccw/cio.c            |   2 +-
>  pc-bios/s390-ccw/dasd-ipl.c       |   2 +-
>  pc-bios/s390-ccw/jump2ipl.c       |   2 +-
>  pc-bios/s390-ccw/libc.c           |  88 ----------------------------
> -
>  pc-bios/s390-ccw/main.c           |  15 +++--
>  pc-bios/s390-ccw/menu.c           |  25 ++++-----
>  pc-bios/s390-ccw/netmain.c        |  15 +----
>  pc-bios/s390-ccw/sclp.c           |   2 +-
>  pc-bios/s390-ccw/virtio-blkdev.c  |   1 -
>  pc-bios/s390-ccw/virtio-scsi.c    |   2 +-
>  pc-bios/s390-ccw/virtio.c         |   2 +-
>  pc-bios/meson.build               |   1 -
>  pc-bios/s390-ccw/Makefile         |  69 +++++++++++++++++++----
>  pc-bios/s390-netboot.img          | Bin 67232 -> 0 bytes
>  25 files changed, 122 insertions(+), 383 deletions(-)
>  delete mode 100644 pc-bios/s390-ccw/netboot.mak
>  delete mode 100644 pc-bios/s390-ccw/libc.h
>  delete mode 100644 pc-bios/s390-ccw/libc.c
>  delete mode 100644 pc-bios/s390-netboot.img
>

Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

Posted by Thomas Huth 6 months, 1 week ago

On 21/06/2024 22.51, Eric Farman wrote:
> On Fri, 2024-06-21 at 10:24 +0200, Thomas Huth wrote:
>> We originally built a separate binary for the netboot code since it
>> was considered as experimental and we could not be sure that the
>> necessary SLOF module had been checked out. Time passed, the netboot
>> code proved its usefulness, and the build system nowadays makes sure
>> that the SLOF module is checked out if you have a s390x compiler
>> available
>> for building the s390-ccw bios. In fact, the possibility to build the
>> s390-ccw.img without s390-netboot.img has been removed in commit
>> bf6903f6944f ("pc-bios/s390-ccw: always build network bootloader")
>> already.
>>
>> So it does not make too much sense anymore to keep the netboot code
>> in a separate binary. To make it easier to support a more flexible
>> boot process soon that supports more than one boot device via the
>> bootindex properties, let's finally merge the netboot code into the
>> main s390-ccw.img binary now.
> 
> Hi Thomas,
> 
> I find myself wondering about the side effects of the
> s/sclp_print/printf/ changes, but I haven't come up with anything I can
> put my finger on. Maybe something will come to me over the weekend, but
> all-in-all I like the looks of this.

I think it should be fine, both functions are basically just a wrapper 
around the write() function in sclp.c, with sclp_print() being rather dumb 
while printf() is doing the usual string formatting before writing it out. I 
think in the long run, it would be nice to get rid of sclp_print() and 
replace it by puts() or printf() in the whole code, but doing that right now 
would likely cause quite some conflicts for Jared with his patch series, so 
I'd rather postpone that to a later point in time.

> Reviewed-by: Eric Farman <farman@linux.ibm.com>

Thanks!

  Thomas

Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

Posted by Jared Rossi 6 months ago

On 6/24/24 1:55 AM, Thomas Huth wrote:
> [...]
>
> I think it should be fine, both functions are basically just a wrapper 
> around the write() function in sclp.c, with sclp_print() being rather 
> dumb while printf() is doing the usual string formatting before 
> writing it out. I think in the long run, it would be nice to get rid 
> of sclp_print() and replace it by puts() or printf() in the whole 
> code, but doing that right now would likely cause quite some conflicts 
> for Jared with his patch series, so I'd rather postpone that to a 
> later point in time.

Hi Thomas,

Converting the panics to returns will require me to modify/move some of 
the sclp_print() calls.  Shall I go ahead and change them to printf() 
and puts() while I'm at it, or would you rather preserve the 
sclp_print() for now and then have a dedicated patch for the all 
replacements later?  I'm not sure if we want to try to maintain some 
amount of consistency until we do a total conversion, or if you are OK 
with a mix of sclp_print() and printf() throughout in the meantime.

Regards,

Jared Rossi

Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

Posted by Thomas Huth 6 months ago

On 28/06/2024 20.01, Jared Rossi wrote:
> 
> 
> On 6/24/24 1:55 AM, Thomas Huth wrote:
>> [...]
>>
>> I think it should be fine, both functions are basically just a wrapper 
>> around the write() function in sclp.c, with sclp_print() being rather dumb 
>> while printf() is doing the usual string formatting before writing it out. 
>> I think in the long run, it would be nice to get rid of sclp_print() and 
>> replace it by puts() or printf() in the whole code, but doing that right 
>> now would likely cause quite some conflicts for Jared with his patch 
>> series, so I'd rather postpone that to a later point in time.
> 
> Hi Thomas,
> 
> Converting the panics to returns will require me to modify/move some of the 
> sclp_print() calls.  Shall I go ahead and change them to printf() and puts() 
> while I'm at it, or would you rather preserve the sclp_print() for now and 
> then have a dedicated patch for the all replacements later?  I'm not sure if 
> we want to try to maintain some amount of consistency until we do a total 
> conversion, or if you are OK with a mix of sclp_print() and printf() 
> throughout in the meantime.

  Hi,

I'm ok if we mix them for a while, so I'd say go ahead and use printf or 
puts for the new code.

  Thomas

Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

Posted by Christian Borntraeger 6 months, 1 week ago

[...]
>   docs/system/s390x/bootdevices.rst |  20 +++----
>   pc-bios/s390-ccw/netboot.mak      |  62 ---------------------
>   hw/s390x/ipl.h                    |  12 ++--
>   pc-bios/s390-ccw/cio.h            |   2 +
>   pc-bios/s390-ccw/iplb.h           |   4 +-
>   pc-bios/s390-ccw/libc.h           |  89 ------------------------------
>   pc-bios/s390-ccw/s390-ccw.h       |  10 +++-
>   pc-bios/s390-ccw/virtio.h         |   1 -
>   hw/s390x/ipl.c                    |  65 +++-------------------
>   hw/s390x/s390-virtio-ccw.c        |  10 +---
>   pc-bios/s390-ccw/bootmap.c        |   4 +-
>   pc-bios/s390-ccw/cio.c            |   2 +-
>   pc-bios/s390-ccw/dasd-ipl.c       |   2 +-
>   pc-bios/s390-ccw/jump2ipl.c       |   2 +-
>   pc-bios/s390-ccw/libc.c           |  88 -----------------------------
>   pc-bios/s390-ccw/main.c           |  15 +++--
>   pc-bios/s390-ccw/menu.c           |  25 ++++-----
>   pc-bios/s390-ccw/netmain.c        |  15 +----
>   pc-bios/s390-ccw/sclp.c           |   2 +-
>   pc-bios/s390-ccw/virtio-blkdev.c  |   1 -
>   pc-bios/s390-ccw/virtio-scsi.c    |   2 +-
>   pc-bios/s390-ccw/virtio.c         |   2 +-
>   pc-bios/meson.build               |   1 -
>   pc-bios/s390-ccw/Makefile         |  69 +++++++++++++++++++----
>   pc-bios/s390-netboot.img          | Bin 67232 -> 0 bytes

Shouldnt you also update the s390-ccw.img file?

>   25 files changed, 122 insertions(+), 383 deletions(-)
>   delete mode 100644 pc-bios/s390-ccw/netboot.mak
>   delete mode 100644 pc-bios/s390-ccw/libc.h
>   delete mode 100644 pc-bios/s390-ccw/libc.c
>   delete mode 100644 pc-bios/s390-netboot.img
>

Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

Posted by Thomas Huth 6 months, 1 week ago

On 21/06/2024 11.39, Christian Borntraeger wrote:
> [...]
>>   docs/system/s390x/bootdevices.rst |  20 +++----
>>   pc-bios/s390-ccw/netboot.mak      |  62 ---------------------
>>   hw/s390x/ipl.h                    |  12 ++--
>>   pc-bios/s390-ccw/cio.h            |   2 +
>>   pc-bios/s390-ccw/iplb.h           |   4 +-
>>   pc-bios/s390-ccw/libc.h           |  89 ------------------------------
>>   pc-bios/s390-ccw/s390-ccw.h       |  10 +++-
>>   pc-bios/s390-ccw/virtio.h         |   1 -
>>   hw/s390x/ipl.c                    |  65 +++-------------------
>>   hw/s390x/s390-virtio-ccw.c        |  10 +---
>>   pc-bios/s390-ccw/bootmap.c        |   4 +-
>>   pc-bios/s390-ccw/cio.c            |   2 +-
>>   pc-bios/s390-ccw/dasd-ipl.c       |   2 +-
>>   pc-bios/s390-ccw/jump2ipl.c       |   2 +-
>>   pc-bios/s390-ccw/libc.c           |  88 -----------------------------
>>   pc-bios/s390-ccw/main.c           |  15 +++--
>>   pc-bios/s390-ccw/menu.c           |  25 ++++-----
>>   pc-bios/s390-ccw/netmain.c        |  15 +----
>>   pc-bios/s390-ccw/sclp.c           |   2 +-
>>   pc-bios/s390-ccw/virtio-blkdev.c  |   1 -
>>   pc-bios/s390-ccw/virtio-scsi.c    |   2 +-
>>   pc-bios/s390-ccw/virtio.c         |   2 +-
>>   pc-bios/meson.build               |   1 -
>>   pc-bios/s390-ccw/Makefile         |  69 +++++++++++++++++++----
>>   pc-bios/s390-netboot.img          | Bin 67232 -> 0 bytes
> 
> Shouldnt you also update the s390-ccw.img file?

Sure, but I didn't want to spam the mailing list with a binary blob as long 
as this is not final yet (not sure whether we want to commit this separately 
or wait 'til Jared finished his series, too). Sorry, I should have mentioned 
it in the cover letter.

  Thomas