On 8/25/26 6:29 AM, Eric Farman wrote:
> 
> 
> On 8/24/26 1:53 PM, Zhuoying Cai wrote:
>> On 8/11/26 10:46 AM, [email protected] wrote:
>>> From: Jared Rossi <[email protected]>
>>>
>>> The IplBlockPci struct is 336 bytes in length.
>>>
>>
>> The commit message is slightly misleading because IplBlockPci itself is
>> 312 bytes in length.
>>
>> Maybe reword it to something like:
>> "IplBlockPci is 312 bytes, so the minimum PCI IPLB length is 336 bytes."
>>
> 
> Given that, should the #define be sizeof(IplBlockPci) + 24?
> 

If IplBlockPci is likely to expand in the future, it might make sense to
derive this value from the structure layout. For example,
offsetof(IplParameterBlock, pci) + sizeof(IplBlockPci) could be more
accurate. Otherwise, keeping it as a constant like the other definitions
seems reasonable as well.

> And does the patch itself need to be cc'd to stable?
> 
>> Otherwise:
>> Reviewed-by: Zhuoying Cai <[email protected]>
>>
>>> Fix the S390_IPLB_MIN_PCI_LEN to reflect this, and assign a valid blk0_len
>>> while we are at it.
>>>
>>> Signed-off-by: Jared Rossi <[email protected]>
>>> ---
>>>   hw/s390x/ipl.c | 2 ++
>>>   hw/s390x/ipl.h | 2 +-
>>>   2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>>> index d5fdc3ea0f..fd6e9100f9 100644
>>> --- a/hw/s390x/ipl.c
>>> +++ b/hw/s390x/ipl.c
>>> @@ -526,6 +526,8 @@ static bool s390_build_iplb(DeviceState *dev_st, 
>>> IplParameterBlock *iplb)
>>>           switch (devtype) {
>>>           case PCI_DEVTYPE_VIRTIO:
>>>               iplb->len = cpu_to_be32(S390_IPLB_MIN_PCI_LEN);
>>> +            iplb->blk0_len =
>>> +                cpu_to_be32(S390_IPLB_MIN_PCI_LEN - S390_IPLB_HEADER_LEN);
>>>               iplb->pbt = S390_IPL_TYPE_PCI;
>>>               iplb->pci.fid = cpu_to_be32(pbdev->fid);
>>>               break;
>>> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
>>> index fac30763df..b4d93054a7 100644
>>> --- a/hw/s390x/ipl.h
>>> +++ b/hw/s390x/ipl.h
>>> @@ -107,7 +107,7 @@ QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, 
>>> "alignment of iplb wrong");
>>>   #define S390_IPLB_MIN_PV_LEN 148
>>>   #define S390_IPLB_MIN_CCW_LEN 200
>>>   #define S390_IPLB_MIN_FCP_LEN 384
>>> -#define S390_IPLB_MIN_PCI_LEN 376
>>> +#define S390_IPLB_MIN_PCI_LEN 336
>>>   #define S390_IPLB_MIN_QEMU_SCSI_LEN 200
>>>   
>>>   static inline bool iplb_valid_len(IplParameterBlock *iplb)
>>
> 


Reply via email to