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(-)
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>
---
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
--
2.39.5
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
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
© 2016 - 2025 Red Hat, Inc.