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

[Jan Beulich <jbeulich@xxxxxxxx>]

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).

Jan