Re: [PATCH] x86/vmx: Provide named fields for IO exit qualification

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 21/03/2023 7:59 am, Jan Beulich wrote:
> On 20.03.2023 19:20, Andrew Cooper wrote:
>> This removes most of the opencoded bit logic on the exit qualification.
>> Unfortunately, size is 1-based not 0-based, so need adjusting in a separate
>> variable.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> In principle
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> but ...
>
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -4560,23 +4560,37 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>          break;
>>  
>>      case EXIT_REASON_IO_INSTRUCTION:
>> -        __vmread(EXIT_QUALIFICATION, &exit_qualification);
>> -        if ( exit_qualification & 0x10 )
>> +    {
>> +        union {
>> +            unsigned long raw;
>> +            struct {
>> +                uint32_t size:3;
>> +                bool     in:1;
>> +                bool     str:1;
>> +                bool     rep:1;
>> +                bool     imm:1;
>> +                uint32_t :9;
>> +                uint16_t port;
> ... I'm not sure this is sufficiently portable: Whether a bitfield of type
> uint32_t followed by a non-bitfield is padded to fill the rest of the
> containing 32-bit field is left unspecified by C99; this particular aspect
> isn't even "implementation defined" (afaics). Therefore I think it would
> be better if either uint32_t was replaced by uint16_t, or if port also was
> made a bit field (and then perhaps also of type uint32_t, or unsigned int).

Port is a naturally aligned uint16_t field so I'd prefer to keep it as is.

I'll drop the others to uint16_t.  Furthermore, as it's simple cleanup
specifically for another pending patch, I'll commit this now and follow
up on the other thread.

~Andrew