Re: [Xen-devel] [PATCH v6 6/8] AMD/IOMMU: tidy struct ivrs_mappings

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.09.2019 18:25, Paul Durrant wrote:
>> -----Original Message-----
>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan 
>> Beulich
>> Sent: 19 September 2019 14:24
>> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Suravee Suthikulpanit 
>> <suravee.suthikulpanit@xxxxxxx>
>> Subject: [Xen-devel] [PATCH v6 6/8] AMD/IOMMU: tidy struct ivrs_mappings
>>
>> Move the device flags field up into an unused hole, thus shrinking
>> overall structure size by 8 bytes. Use bool and uint<N>_t as
>> appropriate. Drop pointless (redundant) initializations.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

Thanks.

> ...although I wonder...
> 
>> --- a/xen/include/asm-x86/amd-iommu.h
>> +++ b/xen/include/asm-x86/amd-iommu.h
>> @@ -106,12 +106,16 @@ struct amd_iommu {
>>  };
>>
>>  struct ivrs_mappings {
>> -    u16 dte_requestor_id;
>> -    u8 dte_allow_exclusion;
>> -    u8 unity_map_enable;
>> -    u8 write_permission;
>> -    u8 read_permission;
>> +    uint16_t dte_requestor_id;
>>      bool valid;
>> +    bool dte_allow_exclusion;
>> +    bool unity_map_enable;
>> +    bool write_permission;
>> +    bool read_permission;
> 
> Could you shrink this even more by using a bit-field instead of this sequence 
> of bools?

Indeed I had been considering this. Besides the fact that making
such a move simply didn't look to fit other things here very well
when introducing the "valid" flag in an earlier path, and then
also not here, do you realize though that this wouldn't shrink
the structure's size right now (i.e. it would only be potentially
reducing future growth)? This was my main argument against going
this further step; let me know what you think.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel