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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.09.2019 11:08, Paul Durrant wrote:
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 24 September 2019 10:02
>>
>> On 23.09.2019 18:25, Paul Durrant wrote:
>>>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan 
>>>> Beulich
>>>> Sent: 19 September 2019 14:24
>>>>
>>>> --- 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)?
> 
> Yes, I'd failed to note the 'unsigned long' afterwards, but...
> 
>> This was my main argument against going
>> this further step; let me know what you think.
>>
> 
> I still think a pre-emptive squash into a uint8_t bit-field followed
> by the device_flags field would give the struct a nice 32-bit hole
> for potential future use.

Okay, will do then. I take it your R-b can remain in place with this
extra change.

Jan

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