Re: [PATCH v2 2/8] hw/s390x/ipl: Fix incorrect PCI IPL block length constant

[Jared Rossi <jrossi@linux.ibm.com>]

On 8/25/26 3:26 PM, Matthew Rosato wrote:
>>>> 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.
>> All of those suggestions would be valid, but I'm leaning toward keeping it
>> defined without using sizeof() for the sake of consistency if nothing
>> else.  The other IPLB types are already defined as fixed numbers and for
>> the IplBlockQemuScsi being used later in this patch series, it actually has
>> a minimum value that is less than the sizeof() itself due to the same
>> struct
>> servicing both PCI and CCW controllers, where the minimum size for CCW is
>> less than PCI.
>>
>> I can envision several ways to improve the definitions and/or naming
>> conventions, but I think it is outside the scope of this series because the
>> changes should be uniformly applied to all definitions, not just PCI.
> Ehh...  If you already think the other definitions should be fixed then
> convention is not a good enough reason to propagate the bad practice to
> new code.  Doing the new definition the right way now in this patch is
> certainly within the scope of this series.
>
> If we have structures that define the entire 336 bytes, I personally
> would much prefer to see that written out with sizeof()s vs a magic
> number that coincidentally must line up with the size of one or more
> well-defined structures.
>
> As for existing definitions, I agree that is out of scope: you could
> follow-up later with one or more patches that cleanup the existing
> definitions.
>
> Thanks,
> Matt

OK.  I will use offsetof(IplParameterBlock, pci) + sizeof(IplBlockPci) for
the new definition, with the intention of cleanly splitting the IPLB header
from Block0 and uniformly updating the definitions in a follow-up series
later.

Regards,
Jared Rossi