Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img
On 8/27/24 8:43 AM, Thomas Huth wrote: I think it's maybe best if you'd include my patches at the top of your patch series, so you could also rework them in case you need something to be changed there. That way, we also do not have to rebuild the binaries in the git repo multiple times and just have to update them one time once your series is ready to go. Alternatively, if you don't want to juggle with my patches, I can also try to get them merged as soon as QEMU 9.1 has been released. Just let me know if you prefer that. Thomas I am fine with including your patches in my series. I will get all that put together and submit a V2 after QEMU 9.1 is squared away. Thanks again, Jared Rossi
Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img
On 26/08/2024 19.07, Jared Rossi wrote: Hi Thomas, I just wanted to get your thoughts on the status of the netboot loader merge. I see that the first patch from this series was merged, but not the others. Is there any issue with the rest of the changes? Hi Jared, there's no issue with that patch series - it's just that QEMU is in freeze now and only accepts bug fixes. See https://wiki.qemu.org/Planning/9.1 I would like to put together a comprehensive rework for all device types that replaces the IPL panics with returns, but as we discussed this is best applied after the netboot loader has been unified with the main s390-ccw.img file. I can base my patches on either current master (with a special case for netboot) or the combined bootloader, depending on how you would like to proceed. Let me know if there is anything I can do to help with test/review of the changes from your side. I think it's maybe best if you'd include my patches at the top of your patch series, so you could also rework them in case you need something to be changed there. That way, we also do not have to rebuild the binaries in the git repo multiple times and just have to update them one time once your series is ready to go. Alternatively, if you don't want to juggle with my patches, I can also try to get them merged as soon as QEMU 9.1 has been released. Just let me know if you prefer that. Thomas
Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img
Hi Thomas,
I just wanted to get your thoughts on the status of the netboot loader merge.
I see that the first patch from this series was merged, but not the others. Is
there any issue with the rest of the changes?
I would like to put together a comprehensive rework for all device types that
replaces the IPL panics with returns, but as we discussed this is best applied
after the netboot loader has been unified with the main s390-ccw.img file.
I can base my patches on either current master (with a special case for netboot)
or the combined bootloader, depending on how you would like to proceed. Let me
know if there is anything I can do to help with test/review of the changes from
your side.
Thanks,
Jared Rossi
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.
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
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
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
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
Thanks!
Thomas
Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img
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
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
>
> 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
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
Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img
[...] 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
