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 <[email protected]> > In principle > Reviewed-by: Jan Beulich <[email protected]> > 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
