Re: [XEN][PATCH 3/3] x86/hvm: vmx: refactor cache disable mode data

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.10.2025 13:26, Grygorii Strashko wrote:
> On 30.10.25 13:23, Jan Beulich wrote:
>> On 30.10.2025 00:54, Grygorii Strashko wrote:
>>> --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
>>> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
>>> @@ -46,7 +46,9 @@ struct ept_data {
>>>   
>>>   #define _VMX_DOMAIN_PML_ENABLED    0
>>>   #define VMX_DOMAIN_PML_ENABLED     (1ul << _VMX_DOMAIN_PML_ENABLED)
>>> +
>>>   struct vmx_domain {
>>> +    spinlock_t uc_lock;
>>>       mfn_t apic_access_mfn;
>>>       /* VMX_DOMAIN_* */
>>>       unsigned int status;
>>
>> Any reason to make this the very first field of the struct? It might better
>> live adjacent to the other field you move; there's going to be some padding
>> anyway, afaict.
> 
> I've tried to put fields in holes and checked with pahole.
> 
> With current change it is:
> struct vmx_domain {
>       spinlock_t                 uc_lock;              /*     0     8 */
>       mfn_t                      apic_access_mfn;      /*     8     8 */
>       unsigned int               status;               /*    16     4 */
>       _Bool                      exec_sp;              /*    20     1 */
>       _Bool                      is_in_uc_mode;        /*    21     1 */
> 
>       /* size: 24, cachelines: 1, members: 5 */
>       /* padding: 2 */
>       /* last cacheline: 24 bytes */
> };
> 
> It seems can be grouped like below?:
> struct vmx_domain {
>       mfn_t                      apic_access_mfn;      /*     0     8 */
>       unsigned int               status;               /*     8     4 */
>       _Bool                      exec_sp;              /*    12     1 */
>       _Bool                      is_in_uc_mode;        /*    13     1 */
> 
>       /* XXX 2 bytes hole, try to pack */
> 
>       spinlock_t                 uc_lock;              /*    16     8 */
> 
>       /* size: 24, cachelines: 1, members: 5 */
>       /* sum members: 22, holes: 1, sum holes: 2 */
>       /* last cacheline: 24 bytes */
> };

Yes, this is what I was thinking of.

Jan