[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86/vtx: Introduce a typed union for CR access exit information
On 20/03/18 10:51, Jan Beulich wrote: >>>> On 15.03.18 at 13:07, <andrew.cooper3@xxxxxxxxxx> wrote: >> --- a/xen/arch/x86/hvm/vmx/vvmx.c >> +++ b/xen/arch/x86/hvm/vmx/vvmx.c >> @@ -2448,27 +2448,24 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs >> *regs, >> break; >> case EXIT_REASON_CR_ACCESS: >> { >> - unsigned long exit_qualification; >> - int cr, write; >> + cr_access_qual_t qual; >> u32 mask = 0; >> >> - __vmread(EXIT_QUALIFICATION, &exit_qualification); >> - cr = VMX_CONTROL_REG_ACCESS_NUM(exit_qualification); >> - write = VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification); >> + __vmread(EXIT_QUALIFICATION, &qual.raw); >> /* also according to guest exec_control */ >> ctrl = __n2_exec_control(v); >> >> - if ( cr == 3 ) >> + if ( qual.cr == 3 ) >> { >> - mask = write? CPU_BASED_CR3_STORE_EXITING: >> - CPU_BASED_CR3_LOAD_EXITING; >> + mask = qual.access_type ? CPU_BASED_CR3_STORE_EXITING >> + : CPU_BASED_CR3_LOAD_EXITING; > I realize the old code has the same problem, but is this correct? > Only type 1 is a read from the CR, types 0, 2, and 3 are writes. > At least have an assertion here that types 2 and 3 can't occur? If we trust hardware not to give us junk here, then types 2 and 3 can't occur. I'll add an assert. > >> if ( ctrl & mask ) >> nvcpu->nv_vmexit_pending = 1; >> } >> - else if ( cr == 8 ) >> + else if ( qual.cr == 8 ) >> { >> - mask = write? CPU_BASED_CR8_STORE_EXITING: >> - CPU_BASED_CR8_LOAD_EXITING; >> + mask = qual.access_type ? CPU_BASED_CR3_STORE_EXITING >> + : CPU_BASED_CR3_LOAD_EXITING; > Copy-and-paste mistake (ought to be CR8 here). Oops. > >> +}; >> +typedef union cr_access_qual { >> + unsigned long raw; >> + struct { >> + uint16_t cr:4, >> + access_type:2, /* VMX_CR_ACCESS_TYPE_* */ >> + lmsw_op_type:1, /* 0 => reg, 1 => mem */ >> + :1, >> + gpr:4, >> + :4; >> + uint16_t lmsw_data; >> + uint32_t :32; > Strictly speaking this doesn't belong here, as it doesn't exist for > 32-bit VMX implementations. It is only here to keep clang happy. See c/s ac6e7fd7a482 As an alternative, we could see about not using transparent unions, and explicitly casting in the function calls. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |