Re: [PATCH v2 3/8] s390x/ipl: Add PCI and bus fields to iplBlockQemuScsi

[Eric Farman <farman@linux.ibm.com>]

On 8/11/26 10:46 AM, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Rather than add a second non-architected IPLB type for scsi-pci devices,
> let's add the relevant fields to the existing struct so that it can handle
> both PCI and CCW scsi controllers.
> 
> The PCI and CCW fields of the IplBlockQemuScsi are aligned with same
> offsets as the corresponding fields in IplBlockPci and IplBlockCcw for
> potential union access.
> 
> Add comments for PCI/CCW specifics and also denote that the S390IplType
> enum values must fit in uint8_t.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>   hw/s390x/ipl.c              |  1 +
>   hw/s390x/ipl.h              |  2 ++
>   include/hw/s390x/ipl/qipl.h | 13 +++++++++----
>   3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index fd6e9100f9..7e68be4c20 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -482,6 +482,7 @@ static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
>               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.bus = S390_IPL_TYPE_CCW;

Ah, so you're just initializing this here. Patch 4 will actually rely on 
this. That is fine.

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

>               iplb->scsi.devno = cpu_to_be16(ccw_dev->sch->devno);
>               iplb->scsi.ssid = ccw_dev->sch->ssid & 3;
>               break;
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index b4d93054a7..0734d72117 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -86,6 +86,8 @@ struct S390IPLState {
>       uint16_t devno;
>   };
>   QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, "alignment of iplb wrong");
> +QEMU_BUILD_BUG_MSG(offsetof(IplBlockQemuScsi, fid) != offsetof(IplBlockPci, fid),
> +                   "iplb fields misaligned across bus types");
>   
>   #define DIAG_308_RC_OK              0x0001
>   #define DIAG_308_RC_NO_CONF         0x0102
> diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
> index 8d3c83a80b..8d3937376b 100644
> --- a/include/hw/s390x/ipl/qipl.h
> +++ b/include/hw/s390x/ipl/qipl.h
> @@ -25,7 +25,7 @@ enum S390IplType {
>       S390_IPL_TYPE_CCW = 0x02,
>       S390_IPL_TYPE_PCI = 0x04,
>       S390_IPL_TYPE_PV = 0x05,
> -    S390_IPL_TYPE_QEMU_SCSI = 0xff
> +    S390_IPL_TYPE_QEMU_SCSI = 0xff /* Max value, must fit in uint8_t */
>   };
>   typedef enum S390IplType S390IplType;
>   
> @@ -103,9 +103,14 @@ struct IplBlockQemuScsi {
>       uint32_t lun;
>       uint16_t target;
>       uint16_t channel;
> -    uint8_t  reserved0[77];
> -    uint8_t  ssid;
> -    uint16_t devno;
> +    uint8_t  bus;
> +    uint8_t  reserved0[76];
> +    uint8_t  ssid;              /* CCW only */
> +    uint16_t devno;             /* CCW only */
> +    uint8_t  reserved1[216];
> +    uint8_t  opt;               /* PCI only */
> +    uint8_t  reserved2[3];
> +    uint32_t fid;               /* PCI only */
>   } QEMU_PACKED;
>   typedef struct IplBlockQemuScsi IplBlockQemuScsi;
>