Re: [PATCH v6 14/28] pc-bios/s390-ccw: Define memory for IPLB and convert IPLB to pointers

[Thomas Huth <thuth@redhat.com>]

On 18/09/2025 01.21, Zhuoying Cai wrote:
> Define a memory space for both IPL Parameter Block (IPLB) and
> IPL Information Report Block (IIRB) since IIRB is stored immediately
> following IPLB.
> 
> Convert IPLB to pointer and it points to the start of the defined memory space.
> IIRB points to the end of IPLB.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/iplb.h     | 12 ++++++++++--
>   pc-bios/s390-ccw/jump2ipl.c |  6 +++---
>   pc-bios/s390-ccw/main.c     | 34 +++++++++++++++++++---------------
>   pc-bios/s390-ccw/netmain.c  |  8 ++++----
>   4 files changed, 36 insertions(+), 24 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
> index bdbc733e16..11302e004d 100644
> --- a/pc-bios/s390-ccw/iplb.h
> +++ b/pc-bios/s390-ccw/iplb.h
> @@ -20,7 +20,7 @@
>   #include <string.h>
>   
>   extern QemuIplParameters qipl;
> -extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> +extern IplParameterBlock *iplb;
>   extern bool have_iplb;
>   
>   struct IplInfoReportBlockHeader {
> @@ -85,6 +85,14 @@ struct IplInfoReportBlock {
>   } __attribute__ ((packed));
>   typedef struct IplInfoReportBlock IplInfoReportBlock;
>   
> +struct IplBlocks {
> +    IplParameterBlock   iplb;
> +    IplInfoReportBlock  iirb;
> +} __attribute__ ((packed));
> +typedef struct IplBlocks IplBlocks;
> +
> +extern IplBlocks ipl_data __attribute__((__aligned__(PAGE_SIZE)));

I think it makes more sense to specify the attribute in the .c file instead 
of using it here in the header.

  Thomas


>   #define S390_IPL_TYPE_FCP 0x00
>   #define S390_IPL_TYPE_CCW 0x02
>   #define S390_IPL_TYPE_QEMU_SCSI 0xff
> @@ -127,7 +135,7 @@ static inline bool load_next_iplb(void)
>   
>       qipl.index++;
>       next_iplb = (IplParameterBlock *) qipl.next_iplb;
> -    memcpy(&iplb, next_iplb, sizeof(IplParameterBlock));
> +    memcpy(iplb, next_iplb, sizeof(IplParameterBlock));
>   
>       qipl.chain_len--;
>       qipl.next_iplb = qipl.next_iplb + sizeof(IplParameterBlock);
> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
> index 86321d0f46..fa2ca5cbe1 100644
> --- a/pc-bios/s390-ccw/jump2ipl.c
> +++ b/pc-bios/s390-ccw/jump2ipl.c
> @@ -43,11 +43,11 @@ int jump_to_IPL_code(uint64_t address)
>        * The IPLB for QEMU SCSI type devices must be rebuilt during re-ipl. The
>        * iplb.devno is set to the boot position of the target SCSI device.
>        */
> -    if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {
> -        iplb.devno = qipl.index;
> +    if (iplb->pbt == S390_IPL_TYPE_QEMU_SCSI) {
> +        iplb->devno = qipl.index;
>       }
>   
> -    if (have_iplb && !set_iplb(&iplb)) {
> +    if (have_iplb && !set_iplb(iplb)) {
>           panic("Failed to set IPLB");
>       }
>   
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 76bf743900..c9328f1c51 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -22,7 +22,9 @@
>   static SubChannelId blk_schid = { .one = 1 };
>   static char loadparm_str[LOADPARM_LEN + 1];
>   QemuIplParameters qipl;
> -IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> +/* Ensure that IPLB and IIRB are page aligned and sequential in memory */
> +IplBlocks ipl_data;
> +IplParameterBlock *iplb;
>   bool have_iplb;
>   static uint16_t cutype;
>   LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */
> @@ -51,7 +53,7 @@ void write_subsystem_identification(void)
>   void write_iplb_location(void)
>   {
>       if (cutype == CU_TYPE_VIRTIO && virtio_get_device_type() != VIRTIO_ID_NET) {
> -        lowcore->ptr_iplb = ptr2u32(&iplb);
> +        lowcore->ptr_iplb = ptr2u32(iplb);
>       }
>   }
>   
> @@ -162,7 +164,7 @@ static void menu_setup(void)
>           return;
>       }
>   
> -    switch (iplb.pbt) {
> +    switch (iplb->pbt) {
>       case S390_IPL_TYPE_CCW:
>       case S390_IPL_TYPE_QEMU_SCSI:
>           menu_set_parms(qipl.qipl_flags & BOOT_MENU_FLAG_MASK,
> @@ -191,8 +193,8 @@ static void boot_setup(void)
>   {
>       char lpmsg[] = "LOADPARM=[________]\n";
>   
> -    if (have_iplb && memcmp(iplb.loadparm, NO_LOADPARM, LOADPARM_LEN) != 0) {
> -        ebcdic_to_ascii((char *) iplb.loadparm, loadparm_str, LOADPARM_LEN);
> +    if (have_iplb && memcmp(iplb->loadparm, NO_LOADPARM, LOADPARM_LEN) != 0) {
> +        ebcdic_to_ascii((char *) iplb->loadparm, loadparm_str, LOADPARM_LEN);
>       } else {
>           sclp_get_loadparm_ascii(loadparm_str);
>       }
> @@ -216,21 +218,21 @@ static bool find_boot_device(void)
>       VDev *vdev = virtio_get_device();
>       bool found = false;
>   
> -    switch (iplb.pbt) {
> +    switch (iplb->pbt) {
>       case S390_IPL_TYPE_CCW:
>           vdev->scsi_device_selected = false;
> -        debug_print_int("device no. ", iplb.ccw.devno);
> -        blk_schid.ssid = iplb.ccw.ssid & 0x3;
> +        debug_print_int("device no. ", iplb->ccw.devno);
> +        blk_schid.ssid = iplb->ccw.ssid & 0x3;
>           debug_print_int("ssid ", blk_schid.ssid);
> -        found = find_subch(iplb.ccw.devno);
> +        found = find_subch(iplb->ccw.devno);
>           break;
>       case S390_IPL_TYPE_QEMU_SCSI:
>           vdev->scsi_device_selected = true;
> -        vdev->selected_scsi_device.channel = iplb.scsi.channel;
> -        vdev->selected_scsi_device.target = iplb.scsi.target;
> -        vdev->selected_scsi_device.lun = iplb.scsi.lun;
> -        blk_schid.ssid = iplb.scsi.ssid & 0x3;
> -        found = find_subch(iplb.scsi.devno);
> +        vdev->selected_scsi_device.channel = iplb->scsi.channel;
> +        vdev->selected_scsi_device.target = iplb->scsi.target;
> +        vdev->selected_scsi_device.lun = iplb->scsi.lun;
> +        blk_schid.ssid = iplb->scsi.ssid & 0x3;
> +        found = find_subch(iplb->scsi.devno);
>           break;
>       default:
>           puts("Unsupported IPLB");
> @@ -311,10 +313,12 @@ static void probe_boot_device(void)
>   
>   void main(void)
>   {
> +    iplb = &ipl_data.iplb;
> +
>       copy_qipl();
>       sclp_setup();
>       css_setup();
> -    have_iplb = store_iplb(&iplb);
> +    have_iplb = store_iplb(iplb);
>       if (!have_iplb) {
>           boot_setup();
>           probe_boot_device();
> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
> index a9521dff41..457fbc3095 100644
> --- a/pc-bios/s390-ccw/netmain.c
> +++ b/pc-bios/s390-ccw/netmain.c
> @@ -528,11 +528,11 @@ static bool virtio_setup(void)
>        */
>       enable_mss_facility();
>   
> -    if (have_iplb || store_iplb(&iplb)) {
> -        IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
> -        dev_no = iplb.ccw.devno;
> +    if (have_iplb || store_iplb(iplb)) {
> +        IPL_assert(iplb->pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
> +        dev_no = iplb->ccw.devno;
>           debug_print_int("device no. ", dev_no);
> -        net_schid.ssid = iplb.ccw.ssid & 0x3;
> +        net_schid.ssid = iplb->ccw.ssid & 0x3;
>           debug_print_int("ssid ", net_schid.ssid);
>           found = find_net_dev(&schib, dev_no);
>       } else {