| Message ID | 20241218170820.364253-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] x86/hvm: Use constants for x86 modes | expand |
On 18.12.2024 18:08, Andrew Cooper wrote: > @@ -2952,11 +2954,11 @@ static const char *guest_x86_mode_to_str(int mode) > { > switch ( mode ) > { > - case 0: return "Real"; > - case 1: return "v86"; > - case 2: return "16bit"; > - case 4: return "32bit"; > - case 8: return "64bit"; > + case X86_MODE_REAL: return "Real"; > + case X86_MODE_VM86: return "v86"; I did ask for the string literal to also become "vm86". I'm simply unaware of "v86" being used anywhere in the vendor docs. > --- a/xen/arch/x86/include/asm/hvm/hvm.h > +++ b/xen/arch/x86/include/asm/hvm/hvm.h > @@ -26,6 +26,20 @@ extern bool opt_hvm_fep; > #define opt_hvm_fep 0 > #endif > > +/* > + * Results for hvm_guest_x86_mode(). > + * > + * Note, some callers depend on the order of these constants. > + * > + * TODO: Rework this helper to avoid implying mixing the architectural > + * concepts of mode and operand size. > + */ > +#define X86_MODE_REAL 0 > +#define X86_MODE_VM86 1 > +#define X86_MODE_16BIT 2 > +#define X86_MODE_32BIT 4 > +#define X86_MODE_64BIT 8 As this block is no longer adjacent to hvm_guest_x86_mode()'s decl, perhaps s/this/that/ in the comment? Preferably with these adjustments: Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
Hello, Le 18/12/2024 à 18:08, Andrew Cooper a écrit : > From: Teddy Astie <teddy.astie@vates.tech> > > In many places of x86 HVM code, constants integer are used to indicate in what mode is > running the CPU (real, vm86, 16-bits, 32-bits, 64-bits). However, these constants are > are written directly as integer which hides the actual meaning of these modes. > > This patch introduces X86_MODE_* macros and replace those occurences with it. > > Signed-off-by: Teddy Astie <teddy.astie@vates.tech> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Teddy Astie <teddy.astie@vates.tech> Thanks > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Teddy Astie <teddy.astie@vates.tech> > > v3: > * Leave a comment behind explaining why these aren't all modes > * Fix long lines > * Convert more instances (svm_guest_x86_mode, hvm_latch_shinfo_size, xenoprof) > --- > xen/arch/x86/hvm/emulate.c | 18 ++++++++++-------- > xen/arch/x86/hvm/hvm.c | 4 +++- > xen/arch/x86/hvm/hypercall.c | 17 +++++++++-------- > xen/arch/x86/hvm/svm/svm.c | 8 ++++---- > xen/arch/x86/hvm/viridian/viridian.c | 8 ++++---- > xen/arch/x86/hvm/vmx/vmx.c | 9 +++++---- > xen/arch/x86/hvm/vmx/vvmx.c | 5 +++-- > xen/arch/x86/include/asm/hvm/hvm.h | 14 ++++++++++++++ > xen/arch/x86/oprofile/xenoprof.c | 6 +++--- > 9 files changed, 55 insertions(+), 34 deletions(-) > > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c > index d3006f094a69..76467b76c047 100644 > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -2433,14 +2433,15 @@ static void cf_check hvmemul_put_fpu( > > switch ( mode ) > { > - case 8: > + case X86_MODE_64BIT: > fpu_ctxt->fip.addr = aux->ip; > if ( dval ) > fpu_ctxt->fdp.addr = aux->dp; > fpu_ctxt->x[FPU_WORD_SIZE_OFFSET] = 8; > break; > > - case 4: case 2: > + case X86_MODE_32BIT: > + case X86_MODE_16BIT: > fpu_ctxt->fip.offs = aux->ip; > fpu_ctxt->fip.sel = aux->cs; > if ( dval ) > @@ -2451,7 +2452,8 @@ static void cf_check hvmemul_put_fpu( > fpu_ctxt->x[FPU_WORD_SIZE_OFFSET] = mode; > break; > > - case 0: case 1: > + case X86_MODE_REAL: > + case X86_MODE_VM86: > fpu_ctxt->fip.addr = aux->ip | (aux->cs << 4); > if ( dval ) > fpu_ctxt->fdp.addr = aux->dp | (aux->ds << 4); > @@ -2952,11 +2954,11 @@ static const char *guest_x86_mode_to_str(int mode) > { > switch ( mode ) > { > - case 0: return "Real"; > - case 1: return "v86"; > - case 2: return "16bit"; > - case 4: return "32bit"; > - case 8: return "64bit"; > + case X86_MODE_REAL: return "Real"; > + case X86_MODE_VM86: return "v86"; > + case X86_MODE_16BIT: return "16bit"; > + case X86_MODE_32BIT: return "32bit"; > + case X86_MODE_64BIT: return "64bit"; > default: return "Unknown"; > } > } > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 74e58c653e6f..922c9b3af64d 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -3974,7 +3974,9 @@ static void hvm_latch_shinfo_size(struct domain *d) > */ > if ( current->domain == d ) > { > - d->arch.has_32bit_shinfo = (hvm_guest_x86_mode(current) != 8); > + d->arch.has_32bit_shinfo = > + hvm_guest_x86_mode(current) != X86_MODE_64BIT; > + > /* > * Make sure that the timebase in the shared info structure is correct. > * > diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c > index 81883c8d4f60..6f8dfdff4ac6 100644 > --- a/xen/arch/x86/hvm/hypercall.c > +++ b/xen/arch/x86/hvm/hypercall.c > @@ -112,23 +112,24 @@ int hvm_hypercall(struct cpu_user_regs *regs) > > switch ( mode ) > { > - case 8: > + case X86_MODE_64BIT: > eax = regs->rax; > fallthrough; > - case 4: > - case 2: > + case X86_MODE_32BIT: > + case X86_MODE_16BIT: > if ( currd->arch.monitor.guest_request_userspace_enabled && > - eax == __HYPERVISOR_hvm_op && > - (mode == 8 ? regs->rdi : regs->ebx) == HVMOP_guest_request_vm_event ) > + eax == __HYPERVISOR_hvm_op && > + (mode == X86_MODE_64BIT ? regs->rdi : regs->ebx) == > + HVMOP_guest_request_vm_event ) > break; > > if ( likely(!hvm_get_cpl(curr)) ) > break; > fallthrough; > - default: > + case X86_MODE_VM86: > regs->rax = -EPERM; > return HVM_HCALL_completed; > - case 0: > + case X86_MODE_REAL: > break; > } > > @@ -198,7 +199,7 @@ enum mc_disposition hvm_do_multicall_call(struct mc_state *state) > { > struct vcpu *curr = current; > > - if ( hvm_guest_x86_mode(curr) == 8 ) > + if ( hvm_guest_x86_mode(curr) == X86_MODE_64BIT ) > { > struct multicall_entry *call = &state->call; > > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index b8f87aa1ed08..62905c2c7acd 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -571,12 +571,12 @@ static int cf_check svm_guest_x86_mode(struct vcpu *v) > struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb; > > if ( unlikely(!(v->arch.hvm.guest_cr[0] & X86_CR0_PE)) ) > - return 0; > + return X86_MODE_REAL; > if ( unlikely(guest_cpu_user_regs()->eflags & X86_EFLAGS_VM) ) > - return 1; > + return X86_MODE_VM86; > if ( hvm_long_mode_active(v) && likely(vmcb->cs.l) ) > - return 8; > - return likely(vmcb->cs.db) ? 4 : 2; > + return X86_MODE_64BIT; > + return vmcb->cs.db ? X86_MODE_32BIT : X86_MODE_16BIT; > } > > static void cf_check svm_cpuid_policy_changed(struct vcpu *v) > diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c > index 21480d9ee700..33d54e587edf 100644 > --- a/xen/arch/x86/hvm/viridian/viridian.c > +++ b/xen/arch/x86/hvm/viridian/viridian.c > @@ -933,13 +933,13 @@ int viridian_hypercall(struct cpu_user_regs *regs) > > switch ( mode ) > { > - case 8: > + case X86_MODE_64BIT: > input.raw = regs->rcx; > input_params_gpa = regs->rdx; > output_params_gpa = regs->r8; > break; > > - case 4: > + case X86_MODE_32BIT: > input.raw = (regs->rdx << 32) | regs->eax; > input_params_gpa = (regs->rbx << 32) | regs->ecx; > output_params_gpa = (regs->rdi << 32) | regs->esi; > @@ -1038,11 +1038,11 @@ int viridian_hypercall(struct cpu_user_regs *regs) > > switch ( mode ) > { > - case 8: > + case X86_MODE_64BIT: > regs->rax = output.raw; > break; > > - case 4: > + case X86_MODE_32BIT: > regs->rdx = output.raw >> 32; > regs->rax = (uint32_t)output.raw; > break; > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index b6885d0e2764..eee1d4b47a13 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -886,14 +886,15 @@ int cf_check vmx_guest_x86_mode(struct vcpu *v) > unsigned long cs_ar_bytes; > > if ( unlikely(!(v->arch.hvm.guest_cr[0] & X86_CR0_PE)) ) > - return 0; > + return X86_MODE_REAL; > if ( unlikely(guest_cpu_user_regs()->eflags & X86_EFLAGS_VM) ) > - return 1; > + return X86_MODE_VM86; > __vmread(GUEST_CS_AR_BYTES, &cs_ar_bytes); > if ( hvm_long_mode_active(v) && > likely(cs_ar_bytes & X86_SEG_AR_CS_LM_ACTIVE) ) > - return 8; > - return (likely(cs_ar_bytes & X86_SEG_AR_DEF_OP_SIZE) ? 4 : 2); > + return X86_MODE_64BIT; > + return (likely(cs_ar_bytes & X86_SEG_AR_DEF_OP_SIZE) > + ? X86_MODE_32BIT : X86_MODE_16BIT); > } > > static void vmx_save_dr(struct vcpu *v) > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > index 78135ca23be8..cf47d61b1473 100644 > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -411,7 +411,7 @@ static int decode_vmx_inst(struct cpu_user_regs *regs, > } > else > { > - bool mode_64bit = (vmx_guest_x86_mode(v) == 8); > + bool mode_64bit = vmx_guest_x86_mode(v) == X86_MODE_64BIT; > > decode->type = VMX_INST_MEMREG_TYPE_MEMORY; > > @@ -2073,7 +2073,8 @@ int nvmx_handle_vmx_insn(struct cpu_user_regs *regs, unsigned int exit_reason) > > if ( !(curr->arch.hvm.guest_cr[4] & X86_CR4_VMXE) || > !nestedhvm_enabled(curr->domain) || > - (vmx_guest_x86_mode(curr) < (hvm_long_mode_active(curr) ? 8 : 2)) || > + (vmx_guest_x86_mode(curr) < > + (hvm_long_mode_active(curr) ? X86_MODE_64BIT : X86_MODE_16BIT)) || > (exit_reason != EXIT_REASON_VMXON && !nvmx_vcpu_in_vmx(curr)) ) > { > hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC); > diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h > index 02de18c7d4a8..124906a548da 100644 > --- a/xen/arch/x86/include/asm/hvm/hvm.h > +++ b/xen/arch/x86/include/asm/hvm/hvm.h > @@ -26,6 +26,20 @@ extern bool opt_hvm_fep; > #define opt_hvm_fep 0 > #endif > > +/* > + * Results for hvm_guest_x86_mode(). > + * > + * Note, some callers depend on the order of these constants. > + * > + * TODO: Rework this helper to avoid implying mixing the architectural > + * concepts of mode and operand size. > + */ > +#define X86_MODE_REAL 0 > +#define X86_MODE_VM86 1 > +#define X86_MODE_16BIT 2 > +#define X86_MODE_32BIT 4 > +#define X86_MODE_64BIT 8 > + > /* Interrupt acknowledgement sources. */ > enum hvm_intsrc { > hvm_intsrc_none, > diff --git a/xen/arch/x86/oprofile/xenoprof.c b/xen/arch/x86/oprofile/xenoprof.c > index 247a0deca822..7f2525bfb4d6 100644 > --- a/xen/arch/x86/oprofile/xenoprof.c > +++ b/xen/arch/x86/oprofile/xenoprof.c > @@ -86,11 +86,11 @@ int xenoprofile_get_mode(struct vcpu *curr, const struct cpu_user_regs *regs) > > switch ( hvm_guest_x86_mode(curr) ) > { > - case 0: /* real mode */ > + case X86_MODE_REAL: > return 1; > - case 1: /* vm86 mode */ > + case X86_MODE_VM86: > return 0; > - default: > + default: /* 16BIT | 32BIT | 64BIT */ > return hvm_get_cpl(curr) != 3; > } > } > > base-commit: 826a9eb072d449cb777d71f52923e6f5f20cefbe Teddy | Vates XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
On 19/12/2024 10:46 am, Teddy Astie wrote: > Hello, > > Le 18/12/2024 à 18:08, Andrew Cooper a écrit : >> From: Teddy Astie <teddy.astie@vates.tech> >> >> In many places of x86 HVM code, constants integer are used to indicate in what mode is >> running the CPU (real, vm86, 16-bits, 32-bits, 64-bits). However, these constants are >> are written directly as integer which hides the actual meaning of these modes. >> >> This patch introduces X86_MODE_* macros and replace those occurences with it. >> >> Signed-off-by: Teddy Astie <teddy.astie@vates.tech> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Acked-by: Teddy Astie <teddy.astie@vates.tech> Thanks, but as you're not a maintainer of this code, Ack doesn't carry any weight. Reviewed-by OTOH would, if you're happy with that adjustment? ~Andrew
Le 19/12/2024 à 12:35, Andrew Cooper a écrit : > On 19/12/2024 10:46 am, Teddy Astie wrote: >> Hello, >> >> Le 18/12/2024 à 18:08, Andrew Cooper a écrit : >>> From: Teddy Astie <teddy.astie@vates.tech> >>> >>> In many places of x86 HVM code, constants integer are used to indicate in what mode is >>> running the CPU (real, vm86, 16-bits, 32-bits, 64-bits). However, these constants are >>> are written directly as integer which hides the actual meaning of these modes. >>> >>> This patch introduces X86_MODE_* macros and replace those occurences with it. >>> >>> Signed-off-by: Teddy Astie <teddy.astie@vates.tech> >>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Acked-by: Teddy Astie <teddy.astie@vates.tech> > > Thanks, but as you're not a maintainer of this code, Ack doesn't carry > any weight. Reviewed-by OTOH would, if you're happy with that adjustment? > > ~Andrew Yes, I meant Reviewed-by in my Acked-by. So, Reviewed-by: Teddy Astie <teddy.astie@vates.tech> Teddy Teddy Astie | Vates XCP-ng Developer XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index d3006f094a69..76467b76c047 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -2433,14 +2433,15 @@ static void cf_check hvmemul_put_fpu( switch ( mode ) { - case 8: + case X86_MODE_64BIT: fpu_ctxt->fip.addr = aux->ip; if ( dval ) fpu_ctxt->fdp.addr = aux->dp; fpu_ctxt->x[FPU_WORD_SIZE_OFFSET] = 8; break; - case 4: case 2: + case X86_MODE_32BIT: + case X86_MODE_16BIT: fpu_ctxt->fip.offs = aux->ip; fpu_ctxt->fip.sel = aux->cs; if ( dval ) @@ -2451,7 +2452,8 @@ static void cf_check hvmemul_put_fpu( fpu_ctxt->x[FPU_WORD_SIZE_OFFSET] = mode; break; - case 0: case 1: + case X86_MODE_REAL: + case X86_MODE_VM86: fpu_ctxt->fip.addr = aux->ip | (aux->cs << 4); if ( dval ) fpu_ctxt->fdp.addr = aux->dp | (aux->ds << 4); @@ -2952,11 +2954,11 @@ static const char *guest_x86_mode_to_str(int mode) { switch ( mode ) { - case 0: return "Real"; - case 1: return "v86"; - case 2: return "16bit"; - case 4: return "32bit"; - case 8: return "64bit"; + case X86_MODE_REAL: return "Real"; + case X86_MODE_VM86: return "v86"; + case X86_MODE_16BIT: return "16bit"; + case X86_MODE_32BIT: return "32bit"; + case X86_MODE_64BIT: return "64bit"; default: return "Unknown"; } } diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 74e58c653e6f..922c9b3af64d 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3974,7 +3974,9 @@ static void hvm_latch_shinfo_size(struct domain *d) */ if ( current->domain == d ) { - d->arch.has_32bit_shinfo = (hvm_guest_x86_mode(current) != 8); + d->arch.has_32bit_shinfo = + hvm_guest_x86_mode(current) != X86_MODE_64BIT; + /* * Make sure that the timebase in the shared info structure is correct. * diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c index 81883c8d4f60..6f8dfdff4ac6 100644 --- a/xen/arch/x86/hvm/hypercall.c +++ b/xen/arch/x86/hvm/hypercall.c @@ -112,23 +112,24 @@ int hvm_hypercall(struct cpu_user_regs *regs) switch ( mode ) { - case 8: + case X86_MODE_64BIT: eax = regs->rax; fallthrough; - case 4: - case 2: + case X86_MODE_32BIT: + case X86_MODE_16BIT: if ( currd->arch.monitor.guest_request_userspace_enabled && - eax == __HYPERVISOR_hvm_op && - (mode == 8 ? regs->rdi : regs->ebx) == HVMOP_guest_request_vm_event ) + eax == __HYPERVISOR_hvm_op && + (mode == X86_MODE_64BIT ? regs->rdi : regs->ebx) == + HVMOP_guest_request_vm_event ) break; if ( likely(!hvm_get_cpl(curr)) ) break; fallthrough; - default: + case X86_MODE_VM86: regs->rax = -EPERM; return HVM_HCALL_completed; - case 0: + case X86_MODE_REAL: break; } @@ -198,7 +199,7 @@ enum mc_disposition hvm_do_multicall_call(struct mc_state *state) { struct vcpu *curr = current; - if ( hvm_guest_x86_mode(curr) == 8 ) + if ( hvm_guest_x86_mode(curr) == X86_MODE_64BIT ) { struct multicall_entry *call = &state->call; diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index b8f87aa1ed08..62905c2c7acd 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -571,12 +571,12 @@ static int cf_check svm_guest_x86_mode(struct vcpu *v) struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb; if ( unlikely(!(v->arch.hvm.guest_cr[0] & X86_CR0_PE)) ) - return 0; + return X86_MODE_REAL; if ( unlikely(guest_cpu_user_regs()->eflags & X86_EFLAGS_VM) ) - return 1; + return X86_MODE_VM86; if ( hvm_long_mode_active(v) && likely(vmcb->cs.l) ) - return 8; - return likely(vmcb->cs.db) ? 4 : 2; + return X86_MODE_64BIT; + return vmcb->cs.db ? X86_MODE_32BIT : X86_MODE_16BIT; } static void cf_check svm_cpuid_policy_changed(struct vcpu *v) diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c index 21480d9ee700..33d54e587edf 100644 --- a/xen/arch/x86/hvm/viridian/viridian.c +++ b/xen/arch/x86/hvm/viridian/viridian.c @@ -933,13 +933,13 @@ int viridian_hypercall(struct cpu_user_regs *regs) switch ( mode ) { - case 8: + case X86_MODE_64BIT: input.raw = regs->rcx; input_params_gpa = regs->rdx; output_params_gpa = regs->r8; break; - case 4: + case X86_MODE_32BIT: input.raw = (regs->rdx << 32) | regs->eax; input_params_gpa = (regs->rbx << 32) | regs->ecx; output_params_gpa = (regs->rdi << 32) | regs->esi; @@ -1038,11 +1038,11 @@ int viridian_hypercall(struct cpu_user_regs *regs) switch ( mode ) { - case 8: + case X86_MODE_64BIT: regs->rax = output.raw; break; - case 4: + case X86_MODE_32BIT: regs->rdx = output.raw >> 32; regs->rax = (uint32_t)output.raw; break; diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index b6885d0e2764..eee1d4b47a13 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -886,14 +886,15 @@ int cf_check vmx_guest_x86_mode(struct vcpu *v) unsigned long cs_ar_bytes; if ( unlikely(!(v->arch.hvm.guest_cr[0] & X86_CR0_PE)) ) - return 0; + return X86_MODE_REAL; if ( unlikely(guest_cpu_user_regs()->eflags & X86_EFLAGS_VM) ) - return 1; + return X86_MODE_VM86; __vmread(GUEST_CS_AR_BYTES, &cs_ar_bytes); if ( hvm_long_mode_active(v) && likely(cs_ar_bytes & X86_SEG_AR_CS_LM_ACTIVE) ) - return 8; - return (likely(cs_ar_bytes & X86_SEG_AR_DEF_OP_SIZE) ? 4 : 2); + return X86_MODE_64BIT; + return (likely(cs_ar_bytes & X86_SEG_AR_DEF_OP_SIZE) + ? X86_MODE_32BIT : X86_MODE_16BIT); } static void vmx_save_dr(struct vcpu *v) diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index 78135ca23be8..cf47d61b1473 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -411,7 +411,7 @@ static int decode_vmx_inst(struct cpu_user_regs *regs, } else { - bool mode_64bit = (vmx_guest_x86_mode(v) == 8); + bool mode_64bit = vmx_guest_x86_mode(v) == X86_MODE_64BIT; decode->type = VMX_INST_MEMREG_TYPE_MEMORY; @@ -2073,7 +2073,8 @@ int nvmx_handle_vmx_insn(struct cpu_user_regs *regs, unsigned int exit_reason) if ( !(curr->arch.hvm.guest_cr[4] & X86_CR4_VMXE) || !nestedhvm_enabled(curr->domain) || - (vmx_guest_x86_mode(curr) < (hvm_long_mode_active(curr) ? 8 : 2)) || + (vmx_guest_x86_mode(curr) < + (hvm_long_mode_active(curr) ? X86_MODE_64BIT : X86_MODE_16BIT)) || (exit_reason != EXIT_REASON_VMXON && !nvmx_vcpu_in_vmx(curr)) ) { hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC); diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h index 02de18c7d4a8..124906a548da 100644 --- a/xen/arch/x86/include/asm/hvm/hvm.h +++ b/xen/arch/x86/include/asm/hvm/hvm.h @@ -26,6 +26,20 @@ extern bool opt_hvm_fep; #define opt_hvm_fep 0 #endif +/* + * Results for hvm_guest_x86_mode(). + * + * Note, some callers depend on the order of these constants. + * + * TODO: Rework this helper to avoid implying mixing the architectural + * concepts of mode and operand size. + */ +#define X86_MODE_REAL 0 +#define X86_MODE_VM86 1 +#define X86_MODE_16BIT 2 +#define X86_MODE_32BIT 4 +#define X86_MODE_64BIT 8 + /* Interrupt acknowledgement sources. */ enum hvm_intsrc { hvm_intsrc_none, diff --git a/xen/arch/x86/oprofile/xenoprof.c b/xen/arch/x86/oprofile/xenoprof.c index 247a0deca822..7f2525bfb4d6 100644 --- a/xen/arch/x86/oprofile/xenoprof.c +++ b/xen/arch/x86/oprofile/xenoprof.c @@ -86,11 +86,11 @@ int xenoprofile_get_mode(struct vcpu *curr, const struct cpu_user_regs *regs) switch ( hvm_guest_x86_mode(curr) ) { - case 0: /* real mode */ + case X86_MODE_REAL: return 1; - case 1: /* vm86 mode */ + case X86_MODE_VM86: return 0; - default: + default: /* 16BIT | 32BIT | 64BIT */ return hvm_get_cpl(curr) != 3; } }