Re: [PATCH v5 15/19] hw/s390x: Build an IPLB for each boot device

[Thomas Huth <thuth@redhat.com>]

On 20/10/2024 03.29, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Build an IPLB for any device with a bootindex (up to a maximum of 8 devices).
> 
> The IPLB chain is placed immediately before the BIOS in memory. Because this
> is not a fixed address, the location of the next IPLB and number of remaining
> boot devices is stored in the QIPL global variable for possible later access by
> the guest during IPL.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
...
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 656996b500..ed152a9dd2 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -56,6 +56,13 @@ static bool iplb_extended_needed(void *opaque)
>       return ipl->iplbext_migration;
>   }
>   
> +/* Place the IPLB chain immediately before the BIOS in memory */
> +static uint64_t find_iplb_chain_addr(uint64_t bios_addr, uint16_t count)
> +{
> +    return (bios_addr & TARGET_PAGE_MASK)
> +            - (count * sizeof(IplParameterBlock));
> +}
> +
>   static const VMStateDescription vmstate_iplb_extended = {
>       .name = "ipl/iplb_extended",
>       .version_id = 0,
> @@ -398,6 +405,17 @@ static CcwDevice *s390_get_ccw_device(DeviceState *dev_st, int *devtype)
>       return ccw_dev;
>   }
>   
> +static uint64_t s390_ipl_map_iplb_chain(IplParameterBlock *iplb_chain)
> +{
> +    S390IPLState *ipl = get_ipl_device();
> +    uint16_t count = ipl->qipl.chain_len;

I ran into problems when running the cdrom-test on my x86 laptop.

I think the above line needs to be changed into this to get it working on 
little endian hosts:

     uint16_t count = be16_to_cpu(ipl->qipl.chain_len)


> +    uint64_t len = sizeof(IplParameterBlock) * count;
> +    uint64_t chain_addr = find_iplb_chain_addr(ipl->bios_start_addr, count);
> +
> +    cpu_physical_memory_write(chain_addr, iplb_chain, len);
> +    return chain_addr;
> +}
> +
>   void s390_ipl_fmt_loadparm(uint8_t *loadparm, char *str, Error **errp)
>   {
>       int i;
> @@ -428,54 +446,51 @@ void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp)
>       }
>   }
>   
> -static bool s390_gen_initial_iplb(S390IPLState *ipl)
> +static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
>   {
> -    DeviceState *dev_st;
> +    S390IPLState *ipl = get_ipl_device();
>       CcwDevice *ccw_dev = NULL;
>       SCSIDevice *sd;
>       int devtype;
>       uint8_t *lp;
>   
> -    dev_st = get_boot_device(0);
> -    if (dev_st) {
> -        ccw_dev = s390_get_ccw_device(dev_st, &devtype);
> -    }
> -
>       /*
>        * Currently allow IPL only from CCW devices.
>        */
> +    ccw_dev = s390_get_ccw_device(dev_st, &devtype);
>       if (ccw_dev) {
>           lp = ccw_dev->loadparm;
>   
>           switch (devtype) {
>           case CCW_DEVTYPE_SCSI:
>               sd = SCSI_DEVICE(dev_st);
> -            ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN);
> -            ipl->iplb.blk0_len =
> +            iplb->len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN);
> +            iplb->blk0_len =
>                   cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN - S390_IPLB_HEADER_LEN);
> -            ipl->iplb.pbt = S390_IPL_TYPE_QEMU_SCSI;
> -            ipl->iplb.scsi.lun = cpu_to_be32(sd->lun);
> -            ipl->iplb.scsi.target = cpu_to_be16(sd->id);
> -            ipl->iplb.scsi.channel = cpu_to_be16(sd->channel);
> -            ipl->iplb.scsi.devno = cpu_to_be16(ccw_dev->sch->devno);
> -            ipl->iplb.scsi.ssid = ccw_dev->sch->ssid & 3;
> +            iplb->pbt = S390_IPL_TYPE_QEMU_SCSI;
> +            iplb->scsi.lun = cpu_to_be32(sd->lun);
> +            iplb->scsi.target = cpu_to_be16(sd->id);
> +            iplb->scsi.channel = cpu_to_be16(sd->channel);
> +            iplb->scsi.devno = cpu_to_be16(ccw_dev->sch->devno);
> +            iplb->scsi.ssid = ccw_dev->sch->ssid & 3;
>               break;
>           case CCW_DEVTYPE_VFIO:
> -            ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
> -            ipl->iplb.pbt = S390_IPL_TYPE_CCW;
> -            ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
> -            ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3;
> +            iplb->len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
> +            iplb->pbt = S390_IPL_TYPE_CCW;
> +            iplb->ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
> +            iplb->ccw.ssid = ccw_dev->sch->ssid & 3;
>               break;
>           case CCW_DEVTYPE_VIRTIO_NET:
> +            /* The S390IPLState netboot is true if ANY IPLB may use netboot */
>               ipl->netboot = true;
>               /* Fall through to CCW_DEVTYPE_VIRTIO case */
>           case CCW_DEVTYPE_VIRTIO:
> -            ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
> -            ipl->iplb.blk0_len =
> +            iplb->len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
> +            iplb->blk0_len =
>                   cpu_to_be32(S390_IPLB_MIN_CCW_LEN - S390_IPLB_HEADER_LEN);
> -            ipl->iplb.pbt = S390_IPL_TYPE_CCW;
> -            ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
> -            ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3;
> +            iplb->pbt = S390_IPL_TYPE_CCW;
> +            iplb->ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
> +            iplb->ccw.ssid = ccw_dev->sch->ssid & 3;
>               break;
>           }
>   
> @@ -484,8 +499,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
>               lp = S390_CCW_MACHINE(qdev_get_machine())->loadparm;
>           }
>   
> -        s390_ipl_convert_loadparm((char *)lp, ipl->iplb.loadparm);
> -        ipl->iplb.flags |= DIAG308_FLAGS_LP_VALID;
> +        s390_ipl_convert_loadparm((char *)lp, iplb->loadparm);
> +        iplb->flags |= DIAG308_FLAGS_LP_VALID;
>   
>           return true;
>       }
> @@ -493,6 +508,62 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
>       return false;
>   }
>   
> +static bool s390_init_all_iplbs(S390IPLState *ipl)
> +{
> +    int iplb_num = 0;
> +    IplParameterBlock iplb_chain[7];
> +    DeviceState *dev_st = get_boot_device(0);
> +    Object *machine = qdev_get_machine();
> +
> +    /*
> +     * Parse the boot devices.  Generate an IPLB for only the first boot device
> +     * which will later be set with DIAG308.
> +     */
> +    if (!dev_st) {
> +        ipl->qipl.chain_len = 0;
> +        return false;
> +    }
> +
> +    /* If no machine loadparm was defined fill it with spaces */
> +    if (memcmp(S390_CCW_MACHINE(machine)->loadparm, NO_LOADPARM, 8) == 0) {
> +        object_property_set_str(machine, "loadparm", "        ", NULL);
> +    }
> +
> +    iplb_num = 1;
> +    s390_build_iplb(dev_st, &ipl->iplb);
> +
> +    /*  Index any fallback boot devices */
> +    while (get_boot_device(iplb_num)) {
> +        iplb_num++;
> +    }
> +
> +    if (iplb_num > MAX_BOOT_DEVS) {
> +        warn_report("Excess boot devices defined! %d boot devices found, "
> +                    "but only the first %d will be considered.",
> +                    iplb_num, MAX_BOOT_DEVS);
> +
> +        iplb_num = MAX_BOOT_DEVS;
> +    }
> +
> +    ipl->qipl.chain_len = iplb_num - 1;

... and this needs to be changed into:

     ipl->qipl.chain_len = cpu_to_be16(iplb_num - 1);

> +    /*
> +     * Build fallback IPLBs for any boot devices above index 0, up to a
> +     * maximum amount as defined in ipl.h
> +     */
> +    if (iplb_num > 1) {
> +        /* Start at 1 because the IPLB for boot index 0 is not chained */
> +        for (int i = 1; i < iplb_num; i++) {
> +            dev_st = get_boot_device(i);
> +            s390_build_iplb(dev_st, &iplb_chain[i - 1]);
> +        }
> +
> +        ipl->qipl.next_iplb = s390_ipl_map_iplb_chain(iplb_chain);

And this needs to be change into:

     ipl->qipl.next_iplb = cpu_to_be64(s390_ipl_map_iplb_chain(iplb_chain));

I am already in progress of picking up your patches, so if you agree with my 
suggestions, I can fix it on my side, too.

  Thomas


> +    }
> +
> +    return iplb_num;
> +}