* [PULL 01/13] target/i386/tcg: fix POP to memory in long mode
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 02/13] target/i386/tcg: Remove SEG_ADDL Paolo Bonzini
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Clément Chigot, Richard Henderson
In long mode, POP to memory will write a full 64-bit value. However,
the call to gen_writeback() in gen_POP will use MO_32 because the
decoding table is incorrect.
The bug was latent until commit aea49fbb01a ("target/i386: use gen_writeback()
within gen_POP()", 2024-06-08), and then became visible because gen_op_st_v
now receives op->ot instead of the "ot" returned by gen_pop_T0.
Analyzed-by: Clément Chigot <chigot@adacore.com>
Fixes: 5e9e21bcc4d ("target/i386: move 60-BF opcodes to new decoder", 2024-05-07)
Tested-by: Clément Chigot <chigot@adacore.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/decode-new.c.inc | 2 +-
target/i386/tcg/emit.c.inc | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index 0d846c32c22..d2da1d396d5 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -1717,7 +1717,7 @@ static const X86OpEntry opcodes_root[256] = {
[0x8C] = X86_OP_ENTRYwr(MOV, E,v, S,w, op0_Mw),
[0x8D] = X86_OP_ENTRYwr(LEA, G,v, M,v, nolea),
[0x8E] = X86_OP_ENTRYwr(MOV, S,w, E,w),
- [0x8F] = X86_OP_GROUPw(group1A, E,v),
+ [0x8F] = X86_OP_GROUPw(group1A, E,d64),
[0x98] = X86_OP_ENTRY1(CBW, 0,v), /* rAX */
[0x99] = X86_OP_ENTRYwr(CWD, 2,v, 0,v), /* rDX, rAX */
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index fc7477833bc..016dce81464 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -2788,6 +2788,7 @@ static void gen_POP(DisasContext *s, X86DecodedInsn *decode)
X86DecodedOp *op = &decode->op[0];
MemOp ot = gen_pop_T0(s);
+ assert(ot >= op->ot);
if (op->has_ea || op->unit == X86_OP_SEG) {
/* NOTE: order is important for MMU exceptions */
gen_writeback(s, decode, 0, s->T0);
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 02/13] target/i386/tcg: Remove SEG_ADDL
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
2024-07-14 11:10 ` [PULL 01/13] target/i386/tcg: fix POP to memory in long mode Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 03/13] target/i386/tcg: Allow IRET from user mode to user mode with SMAP Paolo Bonzini
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
From: Richard Henderson <richard.henderson@linaro.org>
This truncation is now handled by MMU_*32_IDX. The introduction of
MMU_*32_IDX in fact applied correct 32-bit wraparound to 16-bit accesses
with a high segment base (e.g. big real mode or vm86 mode), which did
not use SEG_ADDL.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Link: https://lore.kernel.org/r/20240617161210.4639-3-richard.henderson@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index aee3d19f29b..19d6b41a589 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -579,10 +579,6 @@ int exception_has_error_code(int intno)
} while (0)
#endif
-/* in 64-bit machines, this can overflow. So this segment addition macro
- * can be used to trim the value to 32-bit whenever needed */
-#define SEG_ADDL(ssp, sp, sp_mask) ((uint32_t)((ssp) + (sp & (sp_mask))))
-
/* XXX: add a is_user flag to have proper security support */
#define PUSHW_RA(ssp, sp, sp_mask, val, ra) \
{ \
@@ -593,7 +589,7 @@ int exception_has_error_code(int intno)
#define PUSHL_RA(ssp, sp, sp_mask, val, ra) \
{ \
sp -= 4; \
- cpu_stl_kernel_ra(env, SEG_ADDL(ssp, sp, sp_mask), (uint32_t)(val), ra); \
+ cpu_stl_kernel_ra(env, (ssp) + (sp & (sp_mask)), (val), ra); \
}
#define POPW_RA(ssp, sp, sp_mask, val, ra) \
@@ -604,7 +600,7 @@ int exception_has_error_code(int intno)
#define POPL_RA(ssp, sp, sp_mask, val, ra) \
{ \
- val = (uint32_t)cpu_ldl_kernel_ra(env, SEG_ADDL(ssp, sp, sp_mask), ra); \
+ val = (uint32_t)cpu_ldl_kernel_ra(env, (ssp) + (sp & (sp_mask)), ra); \
sp += 4; \
}
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 03/13] target/i386/tcg: Allow IRET from user mode to user mode with SMAP
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
2024-07-14 11:10 ` [PULL 01/13] target/i386/tcg: fix POP to memory in long mode Paolo Bonzini
2024-07-14 11:10 ` [PULL 02/13] target/i386/tcg: Remove SEG_ADDL Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 04/13] target/i386/tcg: use PUSHL/PUSHW for error code Paolo Bonzini
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Robert R . Henry, Richard Henderson
This fixes a bug wherein i386/tcg assumed an interrupt return using
the IRET instruction was always returning from kernel mode to either
kernel mode or user mode. This assumption is violated when IRET is used
as a clever way to restore thread state, as for example in the dotnet
runtime. There, IRET returns from user mode to user mode.
This bug is that stack accesses from IRET and RETF, as well as accesses
to the parameters in a call gate, are normal data accesses using the
current CPL. This manifested itself as a page fault in the guest Linux
kernel due to SMAP preventing the access.
This bug appears to have been in QEMU since the beginning.
Analyzed-by: Robert R. Henry <rrh.henry@gmail.com>
Co-developed-by: Robert R. Henry <rrh.henry@gmail.com>
Signed-off-by: Robert R. Henry <rrh.henry@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 19d6b41a589..224e73e9ed0 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -594,13 +594,13 @@ int exception_has_error_code(int intno)
#define POPW_RA(ssp, sp, sp_mask, val, ra) \
{ \
- val = cpu_lduw_kernel_ra(env, (ssp) + (sp & (sp_mask)), ra); \
+ val = cpu_lduw_data_ra(env, (ssp) + (sp & (sp_mask)), ra); \
sp += 2; \
}
#define POPL_RA(ssp, sp, sp_mask, val, ra) \
{ \
- val = (uint32_t)cpu_ldl_kernel_ra(env, (ssp) + (sp & (sp_mask)), ra); \
+ val = (uint32_t)cpu_ldl_data_ra(env, (ssp) + (sp & (sp_mask)), ra); \
sp += 4; \
}
@@ -847,7 +847,7 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
#define POPQ_RA(sp, val, ra) \
{ \
- val = cpu_ldq_kernel_ra(env, sp, ra); \
+ val = cpu_ldq_data_ra(env, sp, ra); \
sp += 8; \
}
@@ -1797,18 +1797,18 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
PUSHL_RA(ssp, sp, sp_mask, env->segs[R_SS].selector, GETPC());
PUSHL_RA(ssp, sp, sp_mask, env->regs[R_ESP], GETPC());
for (i = param_count - 1; i >= 0; i--) {
- val = cpu_ldl_kernel_ra(env, old_ssp +
- ((env->regs[R_ESP] + i * 4) &
- old_sp_mask), GETPC());
+ val = cpu_ldl_data_ra(env,
+ old_ssp + ((env->regs[R_ESP] + i * 4) & old_sp_mask),
+ GETPC());
PUSHL_RA(ssp, sp, sp_mask, val, GETPC());
}
} else {
PUSHW_RA(ssp, sp, sp_mask, env->segs[R_SS].selector, GETPC());
PUSHW_RA(ssp, sp, sp_mask, env->regs[R_ESP], GETPC());
for (i = param_count - 1; i >= 0; i--) {
- val = cpu_lduw_kernel_ra(env, old_ssp +
- ((env->regs[R_ESP] + i * 2) &
- old_sp_mask), GETPC());
+ val = cpu_lduw_data_ra(env,
+ old_ssp + ((env->regs[R_ESP] + i * 2) & old_sp_mask),
+ GETPC());
PUSHW_RA(ssp, sp, sp_mask, val, GETPC());
}
}
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 04/13] target/i386/tcg: use PUSHL/PUSHW for error code
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (2 preceding siblings ...)
2024-07-14 11:10 ` [PULL 03/13] target/i386/tcg: Allow IRET from user mode to user mode with SMAP Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 05/13] target/i386/tcg: Reorg push/pop within seg_helper.c Paolo Bonzini
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
Do not pre-decrement esp, let the macros subtract the appropriate
operand size.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 224e73e9ed0..b985382d704 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -670,22 +670,20 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
}
shift = switch_tss(env, intno * 8, e1, e2, SWITCH_TSS_CALL, old_eip);
if (has_error_code) {
- uint32_t mask;
-
/* push the error code */
if (env->segs[R_SS].flags & DESC_B_MASK) {
- mask = 0xffffffff;
+ sp_mask = 0xffffffff;
} else {
- mask = 0xffff;
+ sp_mask = 0xffff;
}
- esp = (env->regs[R_ESP] - (2 << shift)) & mask;
- ssp = env->segs[R_SS].base + esp;
+ esp = env->regs[R_ESP];
+ ssp = env->segs[R_SS].base;
if (shift) {
- cpu_stl_kernel(env, ssp, error_code);
+ PUSHL(ssp, esp, sp_mask, error_code);
} else {
- cpu_stw_kernel(env, ssp, error_code);
+ PUSHW(ssp, esp, sp_mask, error_code);
}
- SET_ESP(esp, mask);
+ SET_ESP(esp, sp_mask);
}
return;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 05/13] target/i386/tcg: Reorg push/pop within seg_helper.c
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (3 preceding siblings ...)
2024-07-14 11:10 ` [PULL 04/13] target/i386/tcg: use PUSHL/PUSHW for error code Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 06/13] target/i386/tcg: Introduce x86_mmu_index_{kernel_,}pl Paolo Bonzini
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
From: Richard Henderson <richard.henderson@linaro.org>
Interrupts and call gates should use accesses with the DPL as
the privilege level. While computing the applicable MMU index
is easy, the harder thing is how to plumb it in the code.
One possibility could be to add a single argument to the PUSH* macros
for the privilege level, but this is repetitive and risks confusion
between the involved privilege levels.
Another possibility is to pass both CPL and DPL, and adjusting both
PUSH* and POP* to use specific privilege levels (instead of using
cpu_{ld,st}*_data). This makes the code more symmetric.
However, a more complicated but much nicer approach is to use a structure
to contain the stack parameters, env, unwind return address, and rewrite
the macros into functions. The struct provides an easy home for the MMU
index as well.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Link: https://lore.kernel.org/r/20240617161210.4639-4-richard.henderson@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 481 +++++++++++++++++++----------------
1 file changed, 259 insertions(+), 222 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index b985382d704..b6902ca3fba 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -28,6 +28,68 @@
#include "helper-tcg.h"
#include "seg_helper.h"
+#ifdef TARGET_X86_64
+#define SET_ESP(val, sp_mask) \
+ do { \
+ if ((sp_mask) == 0xffff) { \
+ env->regs[R_ESP] = (env->regs[R_ESP] & ~0xffff) | \
+ ((val) & 0xffff); \
+ } else if ((sp_mask) == 0xffffffffLL) { \
+ env->regs[R_ESP] = (uint32_t)(val); \
+ } else { \
+ env->regs[R_ESP] = (val); \
+ } \
+ } while (0)
+#else
+#define SET_ESP(val, sp_mask) \
+ do { \
+ env->regs[R_ESP] = (env->regs[R_ESP] & ~(sp_mask)) | \
+ ((val) & (sp_mask)); \
+ } while (0)
+#endif
+
+/* XXX: use mmu_index to have proper DPL support */
+typedef struct StackAccess
+{
+ CPUX86State *env;
+ uintptr_t ra;
+ target_ulong ss_base;
+ target_ulong sp;
+ target_ulong sp_mask;
+} StackAccess;
+
+static void pushw(StackAccess *sa, uint16_t val)
+{
+ sa->sp -= 2;
+ cpu_stw_kernel_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
+ val, sa->ra);
+}
+
+static void pushl(StackAccess *sa, uint32_t val)
+{
+ sa->sp -= 4;
+ cpu_stl_kernel_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
+ val, sa->ra);
+}
+
+static uint16_t popw(StackAccess *sa)
+{
+ uint16_t ret = cpu_lduw_data_ra(sa->env,
+ sa->ss_base + (sa->sp & sa->sp_mask),
+ sa->ra);
+ sa->sp += 2;
+ return ret;
+}
+
+static uint32_t popl(StackAccess *sa)
+{
+ uint32_t ret = cpu_ldl_data_ra(sa->env,
+ sa->ss_base + (sa->sp & sa->sp_mask),
+ sa->ra);
+ sa->sp += 4;
+ return ret;
+}
+
int get_pg_mode(CPUX86State *env)
{
int pg_mode = 0;
@@ -559,68 +621,19 @@ int exception_has_error_code(int intno)
return 0;
}
-#ifdef TARGET_X86_64
-#define SET_ESP(val, sp_mask) \
- do { \
- if ((sp_mask) == 0xffff) { \
- env->regs[R_ESP] = (env->regs[R_ESP] & ~0xffff) | \
- ((val) & 0xffff); \
- } else if ((sp_mask) == 0xffffffffLL) { \
- env->regs[R_ESP] = (uint32_t)(val); \
- } else { \
- env->regs[R_ESP] = (val); \
- } \
- } while (0)
-#else
-#define SET_ESP(val, sp_mask) \
- do { \
- env->regs[R_ESP] = (env->regs[R_ESP] & ~(sp_mask)) | \
- ((val) & (sp_mask)); \
- } while (0)
-#endif
-
-/* XXX: add a is_user flag to have proper security support */
-#define PUSHW_RA(ssp, sp, sp_mask, val, ra) \
- { \
- sp -= 2; \
- cpu_stw_kernel_ra(env, (ssp) + (sp & (sp_mask)), (val), ra); \
- }
-
-#define PUSHL_RA(ssp, sp, sp_mask, val, ra) \
- { \
- sp -= 4; \
- cpu_stl_kernel_ra(env, (ssp) + (sp & (sp_mask)), (val), ra); \
- }
-
-#define POPW_RA(ssp, sp, sp_mask, val, ra) \
- { \
- val = cpu_lduw_data_ra(env, (ssp) + (sp & (sp_mask)), ra); \
- sp += 2; \
- }
-
-#define POPL_RA(ssp, sp, sp_mask, val, ra) \
- { \
- val = (uint32_t)cpu_ldl_data_ra(env, (ssp) + (sp & (sp_mask)), ra); \
- sp += 4; \
- }
-
-#define PUSHW(ssp, sp, sp_mask, val) PUSHW_RA(ssp, sp, sp_mask, val, 0)
-#define PUSHL(ssp, sp, sp_mask, val) PUSHL_RA(ssp, sp, sp_mask, val, 0)
-#define POPW(ssp, sp, sp_mask, val) POPW_RA(ssp, sp, sp_mask, val, 0)
-#define POPL(ssp, sp, sp_mask, val) POPL_RA(ssp, sp, sp_mask, val, 0)
-
/* protected mode interrupt */
static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
int error_code, unsigned int next_eip,
int is_hw)
{
SegmentCache *dt;
- target_ulong ptr, ssp;
+ target_ulong ptr;
int type, dpl, selector, ss_dpl, cpl;
int has_error_code, new_stack, shift;
- uint32_t e1, e2, offset, ss = 0, esp, ss_e1 = 0, ss_e2 = 0;
- uint32_t old_eip, sp_mask, eflags;
+ uint32_t e1, e2, offset, ss = 0, ss_e1 = 0, ss_e2 = 0;
+ uint32_t old_eip, eflags;
int vm86 = env->eflags & VM_MASK;
+ StackAccess sa;
bool set_rf;
has_error_code = 0;
@@ -662,6 +675,9 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
raise_exception_err(env, EXCP0D_GPF, intno * 8 + 2);
}
+ sa.env = env;
+ sa.ra = 0;
+
if (type == 5) {
/* task gate */
/* must do that check here to return the correct error code */
@@ -672,18 +688,18 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
if (has_error_code) {
/* push the error code */
if (env->segs[R_SS].flags & DESC_B_MASK) {
- sp_mask = 0xffffffff;
+ sa.sp_mask = 0xffffffff;
} else {
- sp_mask = 0xffff;
+ sa.sp_mask = 0xffff;
}
- esp = env->regs[R_ESP];
- ssp = env->segs[R_SS].base;
+ sa.sp = env->regs[R_ESP];
+ sa.ss_base = env->segs[R_SS].base;
if (shift) {
- PUSHL(ssp, esp, sp_mask, error_code);
+ pushl(&sa, error_code);
} else {
- PUSHW(ssp, esp, sp_mask, error_code);
+ pushw(&sa, error_code);
}
- SET_ESP(esp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
}
return;
}
@@ -717,6 +733,7 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
}
if (dpl < cpl) {
/* to inner privilege */
+ uint32_t esp;
get_ss_esp_from_tss(env, &ss, &esp, dpl, 0);
if ((ss & 0xfffc) == 0) {
raise_exception_err(env, EXCP0A_TSS, ss & 0xfffc);
@@ -740,17 +757,18 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
raise_exception_err(env, EXCP0A_TSS, ss & 0xfffc);
}
new_stack = 1;
- sp_mask = get_sp_mask(ss_e2);
- ssp = get_seg_base(ss_e1, ss_e2);
+ sa.sp = esp;
+ sa.sp_mask = get_sp_mask(ss_e2);
+ sa.ss_base = get_seg_base(ss_e1, ss_e2);
} else {
/* to same privilege */
if (vm86) {
raise_exception_err(env, EXCP0D_GPF, selector & 0xfffc);
}
new_stack = 0;
- sp_mask = get_sp_mask(env->segs[R_SS].flags);
- ssp = env->segs[R_SS].base;
- esp = env->regs[R_ESP];
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.ss_base = env->segs[R_SS].base;
}
shift = type >> 3;
@@ -775,36 +793,36 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
if (shift == 1) {
if (new_stack) {
if (vm86) {
- PUSHL(ssp, esp, sp_mask, env->segs[R_GS].selector);
- PUSHL(ssp, esp, sp_mask, env->segs[R_FS].selector);
- PUSHL(ssp, esp, sp_mask, env->segs[R_DS].selector);
- PUSHL(ssp, esp, sp_mask, env->segs[R_ES].selector);
+ pushl(&sa, env->segs[R_GS].selector);
+ pushl(&sa, env->segs[R_FS].selector);
+ pushl(&sa, env->segs[R_DS].selector);
+ pushl(&sa, env->segs[R_ES].selector);
}
- PUSHL(ssp, esp, sp_mask, env->segs[R_SS].selector);
- PUSHL(ssp, esp, sp_mask, env->regs[R_ESP]);
+ pushl(&sa, env->segs[R_SS].selector);
+ pushl(&sa, env->regs[R_ESP]);
}
- PUSHL(ssp, esp, sp_mask, eflags);
- PUSHL(ssp, esp, sp_mask, env->segs[R_CS].selector);
- PUSHL(ssp, esp, sp_mask, old_eip);
+ pushl(&sa, eflags);
+ pushl(&sa, env->segs[R_CS].selector);
+ pushl(&sa, old_eip);
if (has_error_code) {
- PUSHL(ssp, esp, sp_mask, error_code);
+ pushl(&sa, error_code);
}
} else {
if (new_stack) {
if (vm86) {
- PUSHW(ssp, esp, sp_mask, env->segs[R_GS].selector);
- PUSHW(ssp, esp, sp_mask, env->segs[R_FS].selector);
- PUSHW(ssp, esp, sp_mask, env->segs[R_DS].selector);
- PUSHW(ssp, esp, sp_mask, env->segs[R_ES].selector);
+ pushw(&sa, env->segs[R_GS].selector);
+ pushw(&sa, env->segs[R_FS].selector);
+ pushw(&sa, env->segs[R_DS].selector);
+ pushw(&sa, env->segs[R_ES].selector);
}
- PUSHW(ssp, esp, sp_mask, env->segs[R_SS].selector);
- PUSHW(ssp, esp, sp_mask, env->regs[R_ESP]);
+ pushw(&sa, env->segs[R_SS].selector);
+ pushw(&sa, env->regs[R_ESP]);
}
- PUSHW(ssp, esp, sp_mask, eflags);
- PUSHW(ssp, esp, sp_mask, env->segs[R_CS].selector);
- PUSHW(ssp, esp, sp_mask, old_eip);
+ pushw(&sa, eflags);
+ pushw(&sa, env->segs[R_CS].selector);
+ pushw(&sa, old_eip);
if (has_error_code) {
- PUSHW(ssp, esp, sp_mask, error_code);
+ pushw(&sa, error_code);
}
}
@@ -822,10 +840,10 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
cpu_x86_load_seg_cache(env, R_GS, 0, 0, 0, 0);
}
ss = (ss & ~3) | dpl;
- cpu_x86_load_seg_cache(env, R_SS, ss,
- ssp, get_seg_limit(ss_e1, ss_e2), ss_e2);
+ cpu_x86_load_seg_cache(env, R_SS, ss, sa.ss_base,
+ get_seg_limit(ss_e1, ss_e2), ss_e2);
}
- SET_ESP(esp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
selector = (selector & ~3) | dpl;
cpu_x86_load_seg_cache(env, R_CS, selector,
@@ -837,20 +855,18 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
#ifdef TARGET_X86_64
-#define PUSHQ_RA(sp, val, ra) \
- { \
- sp -= 8; \
- cpu_stq_kernel_ra(env, sp, (val), ra); \
- }
+static void pushq(StackAccess *sa, uint64_t val)
+{
+ sa->sp -= 8;
+ cpu_stq_kernel_ra(sa->env, sa->sp, val, sa->ra);
+}
-#define POPQ_RA(sp, val, ra) \
- { \
- val = cpu_ldq_data_ra(env, sp, ra); \
- sp += 8; \
- }
-
-#define PUSHQ(sp, val) PUSHQ_RA(sp, val, 0)
-#define POPQ(sp, val) POPQ_RA(sp, val, 0)
+static uint64_t popq(StackAccess *sa)
+{
+ uint64_t ret = cpu_ldq_data_ra(sa->env, sa->sp, sa->ra);
+ sa->sp += 8;
+ return ret;
+}
static inline target_ulong get_rsp_from_tss(CPUX86State *env, int level)
{
@@ -893,8 +909,9 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
int type, dpl, selector, cpl, ist;
int has_error_code, new_stack;
uint32_t e1, e2, e3, ss, eflags;
- target_ulong old_eip, esp, offset;
+ target_ulong old_eip, offset;
bool set_rf;
+ StackAccess sa;
has_error_code = 0;
if (!is_int && !is_hw) {
@@ -962,10 +979,15 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
if (e2 & DESC_C_MASK) {
dpl = cpl;
}
+
+ sa.env = env;
+ sa.ra = 0;
+ sa.sp_mask = -1;
+ sa.ss_base = 0;
if (dpl < cpl || ist != 0) {
/* to inner privilege */
new_stack = 1;
- esp = get_rsp_from_tss(env, ist != 0 ? ist + 3 : dpl);
+ sa.sp = get_rsp_from_tss(env, ist != 0 ? ist + 3 : dpl);
ss = 0;
} else {
/* to same privilege */
@@ -973,9 +995,9 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
raise_exception_err(env, EXCP0D_GPF, selector & 0xfffc);
}
new_stack = 0;
- esp = env->regs[R_ESP];
+ sa.sp = env->regs[R_ESP];
}
- esp &= ~0xfLL; /* align stack */
+ sa.sp &= ~0xfLL; /* align stack */
/* See do_interrupt_protected. */
eflags = cpu_compute_eflags(env);
@@ -983,13 +1005,13 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
eflags |= RF_MASK;
}
- PUSHQ(esp, env->segs[R_SS].selector);
- PUSHQ(esp, env->regs[R_ESP]);
- PUSHQ(esp, eflags);
- PUSHQ(esp, env->segs[R_CS].selector);
- PUSHQ(esp, old_eip);
+ pushq(&sa, env->segs[R_SS].selector);
+ pushq(&sa, env->regs[R_ESP]);
+ pushq(&sa, eflags);
+ pushq(&sa, env->segs[R_CS].selector);
+ pushq(&sa, old_eip);
if (has_error_code) {
- PUSHQ(esp, error_code);
+ pushq(&sa, error_code);
}
/* interrupt gate clear IF mask */
@@ -1002,7 +1024,7 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
ss = 0 | dpl;
cpu_x86_load_seg_cache(env, R_SS, ss, 0, 0, dpl << DESC_DPL_SHIFT);
}
- env->regs[R_ESP] = esp;
+ env->regs[R_ESP] = sa.sp;
selector = (selector & ~3) | dpl;
cpu_x86_load_seg_cache(env, R_CS, selector,
@@ -1074,10 +1096,11 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
int error_code, unsigned int next_eip)
{
SegmentCache *dt;
- target_ulong ptr, ssp;
+ target_ulong ptr;
int selector;
- uint32_t offset, esp;
+ uint32_t offset;
uint32_t old_cs, old_eip;
+ StackAccess sa;
/* real mode (simpler!) */
dt = &env->idt;
@@ -1087,8 +1110,13 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
ptr = dt->base + intno * 4;
offset = cpu_lduw_kernel(env, ptr);
selector = cpu_lduw_kernel(env, ptr + 2);
- esp = env->regs[R_ESP];
- ssp = env->segs[R_SS].base;
+
+ sa.env = env;
+ sa.ra = 0;
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = 0xffff;
+ sa.ss_base = env->segs[R_SS].base;
+
if (is_int) {
old_eip = next_eip;
} else {
@@ -1096,12 +1124,12 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
}
old_cs = env->segs[R_CS].selector;
/* XXX: use SS segment size? */
- PUSHW(ssp, esp, 0xffff, cpu_compute_eflags(env));
- PUSHW(ssp, esp, 0xffff, old_cs);
- PUSHW(ssp, esp, 0xffff, old_eip);
+ pushw(&sa, cpu_compute_eflags(env));
+ pushw(&sa, old_cs);
+ pushw(&sa, old_eip);
/* update processor state */
- env->regs[R_ESP] = (env->regs[R_ESP] & ~0xffff) | (esp & 0xffff);
+ SET_ESP(sa.sp, sa.sp_mask);
env->eip = offset;
env->segs[R_CS].selector = selector;
env->segs[R_CS].base = (selector << 4);
@@ -1544,21 +1572,23 @@ void helper_ljmp_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
void helper_lcall_real(CPUX86State *env, uint32_t new_cs, uint32_t new_eip,
int shift, uint32_t next_eip)
{
- uint32_t esp, esp_mask;
- target_ulong ssp;
+ StackAccess sa;
+
+ sa.env = env;
+ sa.ra = GETPC();
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.ss_base = env->segs[R_SS].base;
- esp = env->regs[R_ESP];
- esp_mask = get_sp_mask(env->segs[R_SS].flags);
- ssp = env->segs[R_SS].base;
if (shift) {
- PUSHL_RA(ssp, esp, esp_mask, env->segs[R_CS].selector, GETPC());
- PUSHL_RA(ssp, esp, esp_mask, next_eip, GETPC());
+ pushl(&sa, env->segs[R_CS].selector);
+ pushl(&sa, next_eip);
} else {
- PUSHW_RA(ssp, esp, esp_mask, env->segs[R_CS].selector, GETPC());
- PUSHW_RA(ssp, esp, esp_mask, next_eip, GETPC());
+ pushw(&sa, env->segs[R_CS].selector);
+ pushw(&sa, next_eip);
}
- SET_ESP(esp, esp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
env->eip = new_eip;
env->segs[R_CS].selector = new_cs;
env->segs[R_CS].base = (new_cs << 4);
@@ -1570,9 +1600,10 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
{
int new_stack, i;
uint32_t e1, e2, cpl, dpl, rpl, selector, param_count;
- uint32_t ss = 0, ss_e1 = 0, ss_e2 = 0, type, ss_dpl, sp_mask;
+ uint32_t ss = 0, ss_e1 = 0, ss_e2 = 0, type, ss_dpl;
uint32_t val, limit, old_sp_mask;
- target_ulong ssp, old_ssp, offset, sp;
+ target_ulong old_ssp, offset;
+ StackAccess sa;
LOG_PCALL("lcall %04x:" TARGET_FMT_lx " s=%d\n", new_cs, new_eip, shift);
LOG_PCALL_STATE(env_cpu(env));
@@ -1584,6 +1615,10 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
}
cpl = env->hflags & HF_CPL_MASK;
LOG_PCALL("desc=%08x:%08x\n", e1, e2);
+
+ sa.env = env;
+ sa.ra = GETPC();
+
if (e2 & DESC_S_MASK) {
if (!(e2 & DESC_CS_MASK)) {
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, GETPC());
@@ -1611,14 +1646,14 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
#ifdef TARGET_X86_64
/* XXX: check 16/32 bit cases in long mode */
if (shift == 2) {
- target_ulong rsp;
-
/* 64 bit case */
- rsp = env->regs[R_ESP];
- PUSHQ_RA(rsp, env->segs[R_CS].selector, GETPC());
- PUSHQ_RA(rsp, next_eip, GETPC());
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = -1;
+ sa.ss_base = 0;
+ pushq(&sa, env->segs[R_CS].selector);
+ pushq(&sa, next_eip);
/* from this point, not restartable */
- env->regs[R_ESP] = rsp;
+ env->regs[R_ESP] = sa.sp;
cpu_x86_load_seg_cache(env, R_CS, (new_cs & 0xfffc) | cpl,
get_seg_base(e1, e2),
get_seg_limit(e1, e2), e2);
@@ -1626,15 +1661,15 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
} else
#endif
{
- sp = env->regs[R_ESP];
- sp_mask = get_sp_mask(env->segs[R_SS].flags);
- ssp = env->segs[R_SS].base;
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.ss_base = env->segs[R_SS].base;
if (shift) {
- PUSHL_RA(ssp, sp, sp_mask, env->segs[R_CS].selector, GETPC());
- PUSHL_RA(ssp, sp, sp_mask, next_eip, GETPC());
+ pushl(&sa, env->segs[R_CS].selector);
+ pushl(&sa, next_eip);
} else {
- PUSHW_RA(ssp, sp, sp_mask, env->segs[R_CS].selector, GETPC());
- PUSHW_RA(ssp, sp, sp_mask, next_eip, GETPC());
+ pushw(&sa, env->segs[R_CS].selector);
+ pushw(&sa, next_eip);
}
limit = get_seg_limit(e1, e2);
@@ -1642,7 +1677,7 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, GETPC());
}
/* from this point, not restartable */
- SET_ESP(sp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
cpu_x86_load_seg_cache(env, R_CS, (new_cs & 0xfffc) | cpl,
get_seg_base(e1, e2), limit, e2);
env->eip = new_eip;
@@ -1737,13 +1772,13 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
/* to inner privilege */
#ifdef TARGET_X86_64
if (shift == 2) {
- sp = get_rsp_from_tss(env, dpl);
ss = dpl; /* SS = NULL selector with RPL = new CPL */
new_stack = 1;
- sp_mask = 0;
- ssp = 0; /* SS base is always zero in IA-32e mode */
+ sa.sp = get_rsp_from_tss(env, dpl);
+ sa.sp_mask = -1;
+ sa.ss_base = 0; /* SS base is always zero in IA-32e mode */
LOG_PCALL("new ss:rsp=%04x:%016llx env->regs[R_ESP]="
- TARGET_FMT_lx "\n", ss, sp, env->regs[R_ESP]);
+ TARGET_FMT_lx "\n", ss, sa.sp, env->regs[R_ESP]);
} else
#endif
{
@@ -1752,7 +1787,6 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
LOG_PCALL("new ss:esp=%04x:%08x param_count=%d env->regs[R_ESP]="
TARGET_FMT_lx "\n", ss, sp32, param_count,
env->regs[R_ESP]);
- sp = sp32;
if ((ss & 0xfffc) == 0) {
raise_exception_err_ra(env, EXCP0A_TSS, ss & 0xfffc, GETPC());
}
@@ -1775,63 +1809,64 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
raise_exception_err_ra(env, EXCP0A_TSS, ss & 0xfffc, GETPC());
}
- sp_mask = get_sp_mask(ss_e2);
- ssp = get_seg_base(ss_e1, ss_e2);
+ sa.sp = sp32;
+ sa.sp_mask = get_sp_mask(ss_e2);
+ sa.ss_base = get_seg_base(ss_e1, ss_e2);
}
/* push_size = ((param_count * 2) + 8) << shift; */
-
old_sp_mask = get_sp_mask(env->segs[R_SS].flags);
old_ssp = env->segs[R_SS].base;
+
#ifdef TARGET_X86_64
if (shift == 2) {
/* XXX: verify if new stack address is canonical */
- PUSHQ_RA(sp, env->segs[R_SS].selector, GETPC());
- PUSHQ_RA(sp, env->regs[R_ESP], GETPC());
+ pushq(&sa, env->segs[R_SS].selector);
+ pushq(&sa, env->regs[R_ESP]);
/* parameters aren't supported for 64-bit call gates */
} else
#endif
if (shift == 1) {
- PUSHL_RA(ssp, sp, sp_mask, env->segs[R_SS].selector, GETPC());
- PUSHL_RA(ssp, sp, sp_mask, env->regs[R_ESP], GETPC());
+ pushl(&sa, env->segs[R_SS].selector);
+ pushl(&sa, env->regs[R_ESP]);
for (i = param_count - 1; i >= 0; i--) {
val = cpu_ldl_data_ra(env,
old_ssp + ((env->regs[R_ESP] + i * 4) & old_sp_mask),
GETPC());
- PUSHL_RA(ssp, sp, sp_mask, val, GETPC());
+ pushl(&sa, val);
}
} else {
- PUSHW_RA(ssp, sp, sp_mask, env->segs[R_SS].selector, GETPC());
- PUSHW_RA(ssp, sp, sp_mask, env->regs[R_ESP], GETPC());
+ pushw(&sa, env->segs[R_SS].selector);
+ pushw(&sa, env->regs[R_ESP]);
for (i = param_count - 1; i >= 0; i--) {
val = cpu_lduw_data_ra(env,
old_ssp + ((env->regs[R_ESP] + i * 2) & old_sp_mask),
GETPC());
- PUSHW_RA(ssp, sp, sp_mask, val, GETPC());
+ pushw(&sa, val);
}
}
new_stack = 1;
} else {
/* to same privilege */
- sp = env->regs[R_ESP];
- sp_mask = get_sp_mask(env->segs[R_SS].flags);
- ssp = env->segs[R_SS].base;
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.ss_base = env->segs[R_SS].base;
/* push_size = (4 << shift); */
new_stack = 0;
}
#ifdef TARGET_X86_64
if (shift == 2) {
- PUSHQ_RA(sp, env->segs[R_CS].selector, GETPC());
- PUSHQ_RA(sp, next_eip, GETPC());
+ pushq(&sa, env->segs[R_CS].selector);
+ pushq(&sa, next_eip);
} else
#endif
if (shift == 1) {
- PUSHL_RA(ssp, sp, sp_mask, env->segs[R_CS].selector, GETPC());
- PUSHL_RA(ssp, sp, sp_mask, next_eip, GETPC());
+ pushl(&sa, env->segs[R_CS].selector);
+ pushl(&sa, next_eip);
} else {
- PUSHW_RA(ssp, sp, sp_mask, env->segs[R_CS].selector, GETPC());
- PUSHW_RA(ssp, sp, sp_mask, next_eip, GETPC());
+ pushw(&sa, env->segs[R_CS].selector);
+ pushw(&sa, next_eip);
}
/* from this point, not restartable */
@@ -1845,7 +1880,7 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
{
ss = (ss & ~3) | dpl;
cpu_x86_load_seg_cache(env, R_SS, ss,
- ssp,
+ sa.ss_base,
get_seg_limit(ss_e1, ss_e2),
ss_e2);
}
@@ -1856,7 +1891,7 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
get_seg_base(e1, e2),
get_seg_limit(e1, e2),
e2);
- SET_ESP(sp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
env->eip = offset;
}
}
@@ -1864,26 +1899,28 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
/* real and vm86 mode iret */
void helper_iret_real(CPUX86State *env, int shift)
{
- uint32_t sp, new_cs, new_eip, new_eflags, sp_mask;
- target_ulong ssp;
+ uint32_t new_cs, new_eip, new_eflags;
int eflags_mask;
+ StackAccess sa;
+
+ sa.env = env;
+ sa.ra = GETPC();
+ sa.sp_mask = 0xffff; /* XXXX: use SS segment size? */
+ sa.sp = env->regs[R_ESP];
+ sa.ss_base = env->segs[R_SS].base;
- sp_mask = 0xffff; /* XXXX: use SS segment size? */
- sp = env->regs[R_ESP];
- ssp = env->segs[R_SS].base;
if (shift == 1) {
/* 32 bits */
- POPL_RA(ssp, sp, sp_mask, new_eip, GETPC());
- POPL_RA(ssp, sp, sp_mask, new_cs, GETPC());
- new_cs &= 0xffff;
- POPL_RA(ssp, sp, sp_mask, new_eflags, GETPC());
+ new_eip = popl(&sa);
+ new_cs = popl(&sa) & 0xffff;
+ new_eflags = popl(&sa);
} else {
/* 16 bits */
- POPW_RA(ssp, sp, sp_mask, new_eip, GETPC());
- POPW_RA(ssp, sp, sp_mask, new_cs, GETPC());
- POPW_RA(ssp, sp, sp_mask, new_eflags, GETPC());
+ new_eip = popw(&sa);
+ new_cs = popw(&sa);
+ new_eflags = popw(&sa);
}
- env->regs[R_ESP] = (env->regs[R_ESP] & ~sp_mask) | (sp & sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
env->segs[R_CS].selector = new_cs;
env->segs[R_CS].base = (new_cs << 4);
env->eip = new_eip;
@@ -1936,47 +1973,49 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
uint32_t new_es, new_ds, new_fs, new_gs;
uint32_t e1, e2, ss_e1, ss_e2;
int cpl, dpl, rpl, eflags_mask, iopl;
- target_ulong ssp, sp, new_eip, new_esp, sp_mask;
+ target_ulong new_eip, new_esp;
+ StackAccess sa;
+
+ sa.env = env;
+ sa.ra = retaddr;
#ifdef TARGET_X86_64
if (shift == 2) {
- sp_mask = -1;
+ sa.sp_mask = -1;
} else
#endif
{
- sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
}
- sp = env->regs[R_ESP];
- ssp = env->segs[R_SS].base;
+ sa.sp = env->regs[R_ESP];
+ sa.ss_base = env->segs[R_SS].base;
new_eflags = 0; /* avoid warning */
#ifdef TARGET_X86_64
if (shift == 2) {
- POPQ_RA(sp, new_eip, retaddr);
- POPQ_RA(sp, new_cs, retaddr);
- new_cs &= 0xffff;
+ new_eip = popq(&sa);
+ new_cs = popq(&sa) & 0xffff;
if (is_iret) {
- POPQ_RA(sp, new_eflags, retaddr);
+ new_eflags = popq(&sa);
}
} else
#endif
{
if (shift == 1) {
/* 32 bits */
- POPL_RA(ssp, sp, sp_mask, new_eip, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_cs, retaddr);
- new_cs &= 0xffff;
+ new_eip = popl(&sa);
+ new_cs = popl(&sa) & 0xffff;
if (is_iret) {
- POPL_RA(ssp, sp, sp_mask, new_eflags, retaddr);
+ new_eflags = popl(&sa);
if (new_eflags & VM_MASK) {
goto return_to_vm86;
}
}
} else {
/* 16 bits */
- POPW_RA(ssp, sp, sp_mask, new_eip, retaddr);
- POPW_RA(ssp, sp, sp_mask, new_cs, retaddr);
+ new_eip = popw(&sa);
+ new_cs = popw(&sa);
if (is_iret) {
- POPW_RA(ssp, sp, sp_mask, new_eflags, retaddr);
+ new_eflags = popw(&sa);
}
}
}
@@ -2012,7 +2051,7 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
raise_exception_err_ra(env, EXCP0B_NOSEG, new_cs & 0xfffc, retaddr);
}
- sp += addend;
+ sa.sp += addend;
if (rpl == cpl && (!(env->hflags & HF_CS64_MASK) ||
((env->hflags & HF_CS64_MASK) && !is_iret))) {
/* return to same privilege level */
@@ -2024,21 +2063,19 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
/* return to different privilege level */
#ifdef TARGET_X86_64
if (shift == 2) {
- POPQ_RA(sp, new_esp, retaddr);
- POPQ_RA(sp, new_ss, retaddr);
- new_ss &= 0xffff;
+ new_esp = popq(&sa);
+ new_ss = popq(&sa) & 0xffff;
} else
#endif
{
if (shift == 1) {
/* 32 bits */
- POPL_RA(ssp, sp, sp_mask, new_esp, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_ss, retaddr);
- new_ss &= 0xffff;
+ new_esp = popl(&sa);
+ new_ss = popl(&sa) & 0xffff;
} else {
/* 16 bits */
- POPW_RA(ssp, sp, sp_mask, new_esp, retaddr);
- POPW_RA(ssp, sp, sp_mask, new_ss, retaddr);
+ new_esp = popw(&sa);
+ new_ss = popw(&sa);
}
}
LOG_PCALL("new ss:esp=%04x:" TARGET_FMT_lx "\n",
@@ -2088,14 +2125,14 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
get_seg_base(e1, e2),
get_seg_limit(e1, e2),
e2);
- sp = new_esp;
+ sa.sp = new_esp;
#ifdef TARGET_X86_64
if (env->hflags & HF_CS64_MASK) {
- sp_mask = -1;
+ sa.sp_mask = -1;
} else
#endif
{
- sp_mask = get_sp_mask(ss_e2);
+ sa.sp_mask = get_sp_mask(ss_e2);
}
/* validate data segments */
@@ -2104,9 +2141,9 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
validate_seg(env, R_FS, rpl);
validate_seg(env, R_GS, rpl);
- sp += addend;
+ sa.sp += addend;
}
- SET_ESP(sp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
env->eip = new_eip;
if (is_iret) {
/* NOTE: 'cpl' is the _old_ CPL */
@@ -2126,12 +2163,12 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
return;
return_to_vm86:
- POPL_RA(ssp, sp, sp_mask, new_esp, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_ss, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_es, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_ds, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_fs, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_gs, retaddr);
+ new_esp = popl(&sa);
+ new_ss = popl(&sa);
+ new_es = popl(&sa);
+ new_ds = popl(&sa);
+ new_fs = popl(&sa);
+ new_gs = popl(&sa);
/* modify processor state */
cpu_load_eflags(env, new_eflags, TF_MASK | AC_MASK | ID_MASK |
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 06/13] target/i386/tcg: Introduce x86_mmu_index_{kernel_,}pl
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (4 preceding siblings ...)
2024-07-14 11:10 ` [PULL 05/13] target/i386/tcg: Reorg push/pop within seg_helper.c Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 07/13] target/i386/tcg: Compute MMU index once Paolo Bonzini
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
From: Richard Henderson <richard.henderson@linaro.org>
Disconnect mmu index computation from the current pl
as stored in env->hflags.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Link: https://lore.kernel.org/r/20240617161210.4639-2-richard.henderson@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.h | 11 ++---------
target/i386/cpu.c | 27 ++++++++++++++++++++++++---
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c43ac01c794..1e121acef54 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2445,15 +2445,8 @@ static inline bool is_mmu_index_32(int mmu_index)
return mmu_index & 1;
}
-static inline int cpu_mmu_index_kernel(CPUX86State *env)
-{
- int mmu_index_32 = (env->hflags & HF_LMA_MASK) ? 0 : 1;
- int mmu_index_base =
- !(env->hflags & HF_SMAP_MASK) ? MMU_KNOSMAP64_IDX :
- ((env->hflags & HF_CPL_MASK) < 3 && (env->eflags & AC_MASK)) ? MMU_KNOSMAP64_IDX : MMU_KSMAP64_IDX;
-
- return mmu_index_base + mmu_index_32;
-}
+int x86_mmu_index_pl(CPUX86State *env, unsigned pl);
+int cpu_mmu_index_kernel(CPUX86State *env);
#define CC_DST (env->cc_dst)
#define CC_SRC (env->cc_src)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c05765eeafc..4688d140c2d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8122,18 +8122,39 @@ static bool x86_cpu_has_work(CPUState *cs)
return x86_cpu_pending_interrupt(cs, cs->interrupt_request) != 0;
}
-static int x86_cpu_mmu_index(CPUState *cs, bool ifetch)
+int x86_mmu_index_pl(CPUX86State *env, unsigned pl)
{
- CPUX86State *env = cpu_env(cs);
int mmu_index_32 = (env->hflags & HF_CS64_MASK) ? 0 : 1;
int mmu_index_base =
- (env->hflags & HF_CPL_MASK) == 3 ? MMU_USER64_IDX :
+ pl == 3 ? MMU_USER64_IDX :
!(env->hflags & HF_SMAP_MASK) ? MMU_KNOSMAP64_IDX :
(env->eflags & AC_MASK) ? MMU_KNOSMAP64_IDX : MMU_KSMAP64_IDX;
return mmu_index_base + mmu_index_32;
}
+static int x86_cpu_mmu_index(CPUState *cs, bool ifetch)
+{
+ CPUX86State *env = cpu_env(cs);
+ return x86_mmu_index_pl(env, env->hflags & HF_CPL_MASK);
+}
+
+static int x86_mmu_index_kernel_pl(CPUX86State *env, unsigned pl)
+{
+ int mmu_index_32 = (env->hflags & HF_LMA_MASK) ? 0 : 1;
+ int mmu_index_base =
+ !(env->hflags & HF_SMAP_MASK) ? MMU_KNOSMAP64_IDX :
+ (pl < 3 && (env->eflags & AC_MASK)
+ ? MMU_KNOSMAP64_IDX : MMU_KSMAP64_IDX);
+
+ return mmu_index_base + mmu_index_32;
+}
+
+int cpu_mmu_index_kernel(CPUX86State *env)
+{
+ return x86_mmu_index_kernel_pl(env, env->hflags & HF_CPL_MASK);
+}
+
static void x86_disas_set_info(CPUState *cs, disassemble_info *info)
{
X86CPU *cpu = X86_CPU(cs);
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 07/13] target/i386/tcg: Compute MMU index once
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (5 preceding siblings ...)
2024-07-14 11:10 ` [PULL 06/13] target/i386/tcg: Introduce x86_mmu_index_{kernel_,}pl Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 08/13] target/i386/tcg: Use DPL-level accesses for interrupts and call gates Paolo Bonzini
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
Add the MMU index to the StackAccess struct, so that it can be cached
or (in the next patch) computed from information that is not in
CPUX86State.
Co-developed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index b6902ca3fba..8a6d92b3583 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -56,36 +56,37 @@ typedef struct StackAccess
target_ulong ss_base;
target_ulong sp;
target_ulong sp_mask;
+ int mmu_index;
} StackAccess;
static void pushw(StackAccess *sa, uint16_t val)
{
sa->sp -= 2;
- cpu_stw_kernel_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
- val, sa->ra);
+ cpu_stw_mmuidx_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
+ val, sa->mmu_index, sa->ra);
}
static void pushl(StackAccess *sa, uint32_t val)
{
sa->sp -= 4;
- cpu_stl_kernel_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
- val, sa->ra);
+ cpu_stl_mmuidx_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
+ val, sa->mmu_index, sa->ra);
}
static uint16_t popw(StackAccess *sa)
{
- uint16_t ret = cpu_lduw_data_ra(sa->env,
- sa->ss_base + (sa->sp & sa->sp_mask),
- sa->ra);
+ uint16_t ret = cpu_lduw_mmuidx_ra(sa->env,
+ sa->ss_base + (sa->sp & sa->sp_mask),
+ sa->mmu_index, sa->ra);
sa->sp += 2;
return ret;
}
static uint32_t popl(StackAccess *sa)
{
- uint32_t ret = cpu_ldl_data_ra(sa->env,
- sa->ss_base + (sa->sp & sa->sp_mask),
- sa->ra);
+ uint32_t ret = cpu_ldl_mmuidx_ra(sa->env,
+ sa->ss_base + (sa->sp & sa->sp_mask),
+ sa->mmu_index, sa->ra);
sa->sp += 4;
return ret;
}
@@ -677,6 +678,7 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
sa.env = env;
sa.ra = 0;
+ sa.mmu_index = cpu_mmu_index_kernel(env);
if (type == 5) {
/* task gate */
@@ -858,12 +860,12 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
static void pushq(StackAccess *sa, uint64_t val)
{
sa->sp -= 8;
- cpu_stq_kernel_ra(sa->env, sa->sp, val, sa->ra);
+ cpu_stq_mmuidx_ra(sa->env, sa->sp, val, sa->mmu_index, sa->ra);
}
static uint64_t popq(StackAccess *sa)
{
- uint64_t ret = cpu_ldq_data_ra(sa->env, sa->sp, sa->ra);
+ uint64_t ret = cpu_ldq_mmuidx_ra(sa->env, sa->sp, sa->mmu_index, sa->ra);
sa->sp += 8;
return ret;
}
@@ -982,6 +984,7 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
sa.env = env;
sa.ra = 0;
+ sa.mmu_index = cpu_mmu_index_kernel(env);
sa.sp_mask = -1;
sa.ss_base = 0;
if (dpl < cpl || ist != 0) {
@@ -1116,6 +1119,7 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
sa.sp = env->regs[R_ESP];
sa.sp_mask = 0xffff;
sa.ss_base = env->segs[R_SS].base;
+ sa.mmu_index = cpu_mmu_index_kernel(env);
if (is_int) {
old_eip = next_eip;
@@ -1579,6 +1583,7 @@ void helper_lcall_real(CPUX86State *env, uint32_t new_cs, uint32_t new_eip,
sa.sp = env->regs[R_ESP];
sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
sa.ss_base = env->segs[R_SS].base;
+ sa.mmu_index = cpu_mmu_index_kernel(env);
if (shift) {
pushl(&sa, env->segs[R_CS].selector);
@@ -1618,6 +1623,7 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
sa.env = env;
sa.ra = GETPC();
+ sa.mmu_index = cpu_mmu_index_kernel(env);
if (e2 & DESC_S_MASK) {
if (!(e2 & DESC_CS_MASK)) {
@@ -1905,6 +1911,7 @@ void helper_iret_real(CPUX86State *env, int shift)
sa.env = env;
sa.ra = GETPC();
+ sa.mmu_index = x86_mmu_index_pl(env, 0);
sa.sp_mask = 0xffff; /* XXXX: use SS segment size? */
sa.sp = env->regs[R_ESP];
sa.ss_base = env->segs[R_SS].base;
@@ -1976,8 +1983,11 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
target_ulong new_eip, new_esp;
StackAccess sa;
+ cpl = env->hflags & HF_CPL_MASK;
+
sa.env = env;
sa.ra = retaddr;
+ sa.mmu_index = x86_mmu_index_pl(env, cpl);
#ifdef TARGET_X86_64
if (shift == 2) {
@@ -2032,7 +2042,6 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
!(e2 & DESC_CS_MASK)) {
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, retaddr);
}
- cpl = env->hflags & HF_CPL_MASK;
rpl = new_cs & 3;
if (rpl < cpl) {
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, retaddr);
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 08/13] target/i386/tcg: Use DPL-level accesses for interrupts and call gates
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (6 preceding siblings ...)
2024-07-14 11:10 ` [PULL 07/13] target/i386/tcg: Compute MMU index once Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 09/13] target/i386/tcg: check for correct busy state before switching to a new task Paolo Bonzini
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Robert R . Henry, Richard Henderson
This fixes a bug wherein i386/tcg assumed an interrupt return using
the CALL or JMP instructions were always going from kernel or user mode to
kernel mode, when using a call gate. This assumption is violated if
the call gate has a DPL that is greater than 0.
In addition, the stack accesses should count as explicit, not implicit
("kernel" in QEMU code), so that SMAP is not applied if DPL=3.
Analyzed-by: Robert R. Henry <rrh.henry@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/249
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 8a6d92b3583..809ee3d9833 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -678,7 +678,7 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
sa.env = env;
sa.ra = 0;
- sa.mmu_index = cpu_mmu_index_kernel(env);
+ sa.mmu_index = x86_mmu_index_pl(env, dpl);
if (type == 5) {
/* task gate */
@@ -984,7 +984,7 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
sa.env = env;
sa.ra = 0;
- sa.mmu_index = cpu_mmu_index_kernel(env);
+ sa.mmu_index = x86_mmu_index_pl(env, dpl);
sa.sp_mask = -1;
sa.ss_base = 0;
if (dpl < cpl || ist != 0) {
@@ -1119,7 +1119,7 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
sa.sp = env->regs[R_ESP];
sa.sp_mask = 0xffff;
sa.ss_base = env->segs[R_SS].base;
- sa.mmu_index = cpu_mmu_index_kernel(env);
+ sa.mmu_index = x86_mmu_index_pl(env, 0);
if (is_int) {
old_eip = next_eip;
@@ -1583,7 +1583,7 @@ void helper_lcall_real(CPUX86State *env, uint32_t new_cs, uint32_t new_eip,
sa.sp = env->regs[R_ESP];
sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
sa.ss_base = env->segs[R_SS].base;
- sa.mmu_index = cpu_mmu_index_kernel(env);
+ sa.mmu_index = x86_mmu_index_pl(env, 0);
if (shift) {
pushl(&sa, env->segs[R_CS].selector);
@@ -1619,17 +1619,17 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, GETPC());
}
cpl = env->hflags & HF_CPL_MASK;
+ dpl = (e2 >> DESC_DPL_SHIFT) & 3;
LOG_PCALL("desc=%08x:%08x\n", e1, e2);
sa.env = env;
sa.ra = GETPC();
- sa.mmu_index = cpu_mmu_index_kernel(env);
+ sa.mmu_index = x86_mmu_index_pl(env, dpl);
if (e2 & DESC_S_MASK) {
if (!(e2 & DESC_CS_MASK)) {
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, GETPC());
}
- dpl = (e2 >> DESC_DPL_SHIFT) & 3;
if (e2 & DESC_C_MASK) {
/* conforming code segment */
if (dpl > cpl) {
@@ -1691,7 +1691,6 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
} else {
/* check gate type */
type = (e2 >> DESC_TYPE_SHIFT) & 0x1f;
- dpl = (e2 >> DESC_DPL_SHIFT) & 3;
rpl = new_cs & 3;
#ifdef TARGET_X86_64
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 09/13] target/i386/tcg: check for correct busy state before switching to a new task
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (7 preceding siblings ...)
2024-07-14 11:10 ` [PULL 08/13] target/i386/tcg: Use DPL-level accesses for interrupts and call gates Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 10/13] target/i386/tcg: use X86Access for TSS access Paolo Bonzini
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
This step is listed in the Intel manual: "Checks that the new task is available
(call, jump, exception, or interrupt) or busy (IRET return)".
The AMD manual lists the same operation under the "Preventing recursion"
paragraph of "12.3.4 Nesting Tasks", though it is not clear if the processor
checks the busy bit in the IRET case.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 809ee3d9833..0242f9d8b58 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -369,6 +369,11 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
old_tss_limit_max = 43;
}
+ /* new TSS must be busy iff the source is an IRET instruction */
+ if (!!(e2 & DESC_TSS_BUSY_MASK) != (source == SWITCH_TSS_IRET)) {
+ raise_exception_err_ra(env, EXCP0A_TSS, tss_selector & 0xfffc, retaddr);
+ }
+
/* read all the registers from the new TSS */
if (type & 8) {
/* 32 bit */
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 10/13] target/i386/tcg: use X86Access for TSS access
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (8 preceding siblings ...)
2024-07-14 11:10 ` [PULL 09/13] target/i386/tcg: check for correct busy state before switching to a new task Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 11/13] target/i386/tcg: save current task state before loading new one Paolo Bonzini
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
This takes care of probing the vaddr range in advance, and is also faster
because it avoids repeated TLB lookups. It also matches the Intel manual
better, as it says "Checks that the current (old) TSS, new TSS, and all
segment descriptors used in the task switch are paged into system memory";
note however that it's not clear how the processor checks for segment
descriptors, and this check is not included in the AMD manual.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 110 ++++++++++++++++++-----------------
1 file changed, 58 insertions(+), 52 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 0242f9d8b58..fea08a2ba0f 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -27,6 +27,7 @@
#include "exec/log.h"
#include "helper-tcg.h"
#include "seg_helper.h"
+#include "access.h"
#ifdef TARGET_X86_64
#define SET_ESP(val, sp_mask) \
@@ -313,14 +314,15 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
uint32_t e1, uint32_t e2, int source,
uint32_t next_eip, uintptr_t retaddr)
{
- int tss_limit, tss_limit_max, type, old_tss_limit_max, old_type, v1, v2, i;
+ int tss_limit, tss_limit_max, type, old_tss_limit_max, old_type, i;
target_ulong tss_base;
uint32_t new_regs[8], new_segs[6];
uint32_t new_eflags, new_eip, new_cr3, new_ldt, new_trap;
uint32_t old_eflags, eflags_mask;
SegmentCache *dt;
- int index;
+ int mmu_index, index;
target_ulong ptr;
+ X86Access old, new;
type = (e2 >> DESC_TYPE_SHIFT) & 0xf;
LOG_PCALL("switch_tss: sel=0x%04x type=%d src=%d\n", tss_selector, type,
@@ -374,35 +376,45 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
raise_exception_err_ra(env, EXCP0A_TSS, tss_selector & 0xfffc, retaddr);
}
+ /* X86Access avoids memory exceptions during the task switch */
+ mmu_index = cpu_mmu_index_kernel(env);
+ access_prepare_mmu(&old, env, env->tr.base, old_tss_limit_max,
+ MMU_DATA_STORE, mmu_index, retaddr);
+
+ if (source == SWITCH_TSS_CALL) {
+ /* Probe for future write of parent task */
+ probe_access(env, tss_base, 2, MMU_DATA_STORE,
+ mmu_index, retaddr);
+ }
+ access_prepare_mmu(&new, env, tss_base, tss_limit,
+ MMU_DATA_LOAD, mmu_index, retaddr);
+
/* read all the registers from the new TSS */
if (type & 8) {
/* 32 bit */
- new_cr3 = cpu_ldl_kernel_ra(env, tss_base + 0x1c, retaddr);
- new_eip = cpu_ldl_kernel_ra(env, tss_base + 0x20, retaddr);
- new_eflags = cpu_ldl_kernel_ra(env, tss_base + 0x24, retaddr);
+ new_cr3 = access_ldl(&new, tss_base + 0x1c);
+ new_eip = access_ldl(&new, tss_base + 0x20);
+ new_eflags = access_ldl(&new, tss_base + 0x24);
for (i = 0; i < 8; i++) {
- new_regs[i] = cpu_ldl_kernel_ra(env, tss_base + (0x28 + i * 4),
- retaddr);
+ new_regs[i] = access_ldl(&new, tss_base + (0x28 + i * 4));
}
for (i = 0; i < 6; i++) {
- new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x48 + i * 4),
- retaddr);
+ new_segs[i] = access_ldw(&new, tss_base + (0x48 + i * 4));
}
- new_ldt = cpu_lduw_kernel_ra(env, tss_base + 0x60, retaddr);
- new_trap = cpu_ldl_kernel_ra(env, tss_base + 0x64, retaddr);
+ new_ldt = access_ldw(&new, tss_base + 0x60);
+ new_trap = access_ldl(&new, tss_base + 0x64);
} else {
/* 16 bit */
new_cr3 = 0;
- new_eip = cpu_lduw_kernel_ra(env, tss_base + 0x0e, retaddr);
- new_eflags = cpu_lduw_kernel_ra(env, tss_base + 0x10, retaddr);
+ new_eip = access_ldw(&new, tss_base + 0x0e);
+ new_eflags = access_ldw(&new, tss_base + 0x10);
for (i = 0; i < 8; i++) {
- new_regs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x12 + i * 2), retaddr);
+ new_regs[i] = access_ldw(&new, tss_base + (0x12 + i * 2));
}
for (i = 0; i < 4; i++) {
- new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 2),
- retaddr);
+ new_segs[i] = access_ldw(&new, tss_base + (0x22 + i * 2));
}
- new_ldt = cpu_lduw_kernel_ra(env, tss_base + 0x2a, retaddr);
+ new_ldt = access_ldw(&new, tss_base + 0x2a);
new_segs[R_FS] = 0;
new_segs[R_GS] = 0;
new_trap = 0;
@@ -412,16 +424,6 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
chapters 12.2.5 and 13.2.4 on how to implement TSS Trap bit */
(void)new_trap;
- /* NOTE: we must avoid memory exceptions during the task switch,
- so we make dummy accesses before */
- /* XXX: it can still fail in some cases, so a bigger hack is
- necessary to valid the TLB after having done the accesses */
-
- v1 = cpu_ldub_kernel_ra(env, env->tr.base, retaddr);
- v2 = cpu_ldub_kernel_ra(env, env->tr.base + old_tss_limit_max, retaddr);
- cpu_stb_kernel_ra(env, env->tr.base, v1, retaddr);
- cpu_stb_kernel_ra(env, env->tr.base + old_tss_limit_max, v2, retaddr);
-
/* clear busy bit (it is restartable) */
if (source == SWITCH_TSS_JMP || source == SWITCH_TSS_IRET) {
tss_set_busy(env, env->tr.selector, 0, retaddr);
@@ -434,35 +436,35 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
/* save the current state in the old TSS */
if (old_type & 8) {
/* 32 bit */
- cpu_stl_kernel_ra(env, env->tr.base + 0x20, next_eip, retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + 0x24, old_eflags, retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 0 * 4), env->regs[R_EAX], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 1 * 4), env->regs[R_ECX], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 2 * 4), env->regs[R_EDX], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 3 * 4), env->regs[R_EBX], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 4 * 4), env->regs[R_ESP], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 5 * 4), env->regs[R_EBP], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 6 * 4), env->regs[R_ESI], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 7 * 4), env->regs[R_EDI], retaddr);
+ access_stl(&old, env->tr.base + 0x20, next_eip);
+ access_stl(&old, env->tr.base + 0x24, old_eflags);
+ access_stl(&old, env->tr.base + (0x28 + 0 * 4), env->regs[R_EAX]);
+ access_stl(&old, env->tr.base + (0x28 + 1 * 4), env->regs[R_ECX]);
+ access_stl(&old, env->tr.base + (0x28 + 2 * 4), env->regs[R_EDX]);
+ access_stl(&old, env->tr.base + (0x28 + 3 * 4), env->regs[R_EBX]);
+ access_stl(&old, env->tr.base + (0x28 + 4 * 4), env->regs[R_ESP]);
+ access_stl(&old, env->tr.base + (0x28 + 5 * 4), env->regs[R_EBP]);
+ access_stl(&old, env->tr.base + (0x28 + 6 * 4), env->regs[R_ESI]);
+ access_stl(&old, env->tr.base + (0x28 + 7 * 4), env->regs[R_EDI]);
for (i = 0; i < 6; i++) {
- cpu_stw_kernel_ra(env, env->tr.base + (0x48 + i * 4),
- env->segs[i].selector, retaddr);
+ access_stw(&old, env->tr.base + (0x48 + i * 4),
+ env->segs[i].selector);
}
} else {
/* 16 bit */
- cpu_stw_kernel_ra(env, env->tr.base + 0x0e, next_eip, retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + 0x10, old_eflags, retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 0 * 2), env->regs[R_EAX], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 1 * 2), env->regs[R_ECX], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 2 * 2), env->regs[R_EDX], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 3 * 2), env->regs[R_EBX], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 4 * 2), env->regs[R_ESP], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 5 * 2), env->regs[R_EBP], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI], retaddr);
+ access_stw(&old, env->tr.base + 0x0e, next_eip);
+ access_stw(&old, env->tr.base + 0x10, old_eflags);
+ access_stw(&old, env->tr.base + (0x12 + 0 * 2), env->regs[R_EAX]);
+ access_stw(&old, env->tr.base + (0x12 + 1 * 2), env->regs[R_ECX]);
+ access_stw(&old, env->tr.base + (0x12 + 2 * 2), env->regs[R_EDX]);
+ access_stw(&old, env->tr.base + (0x12 + 3 * 2), env->regs[R_EBX]);
+ access_stw(&old, env->tr.base + (0x12 + 4 * 2), env->regs[R_ESP]);
+ access_stw(&old, env->tr.base + (0x12 + 5 * 2), env->regs[R_EBP]);
+ access_stw(&old, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI]);
+ access_stw(&old, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI]);
for (i = 0; i < 4; i++) {
- cpu_stw_kernel_ra(env, env->tr.base + (0x22 + i * 2),
- env->segs[i].selector, retaddr);
+ access_stw(&old, env->tr.base + (0x22 + i * 2),
+ env->segs[i].selector);
}
}
@@ -470,7 +472,11 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
context */
if (source == SWITCH_TSS_CALL) {
- cpu_stw_kernel_ra(env, tss_base, env->tr.selector, retaddr);
+ /*
+ * Thanks to the probe_access above, we know the first two
+ * bytes addressed by &new are writable too.
+ */
+ access_stw(&new, tss_base, env->tr.selector);
new_eflags |= NT_MASK;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 11/13] target/i386/tcg: save current task state before loading new one
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (9 preceding siblings ...)
2024-07-14 11:10 ` [PULL 10/13] target/i386/tcg: use X86Access for TSS access Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 12/13] i386/sev: Don't allow automatic fallback to legacy KVM_SEV*_INIT Paolo Bonzini
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
This is how the steps are ordered in the manual. EFLAGS.NT is
overwritten after the fact in the saved image.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 85 +++++++++++++++++++-----------------
1 file changed, 45 insertions(+), 40 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index fea08a2ba0f..c0641a79c70 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -389,6 +389,42 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
access_prepare_mmu(&new, env, tss_base, tss_limit,
MMU_DATA_LOAD, mmu_index, retaddr);
+ /* save the current state in the old TSS */
+ old_eflags = cpu_compute_eflags(env);
+ if (old_type & 8) {
+ /* 32 bit */
+ access_stl(&old, env->tr.base + 0x20, next_eip);
+ access_stl(&old, env->tr.base + 0x24, old_eflags);
+ access_stl(&old, env->tr.base + (0x28 + 0 * 4), env->regs[R_EAX]);
+ access_stl(&old, env->tr.base + (0x28 + 1 * 4), env->regs[R_ECX]);
+ access_stl(&old, env->tr.base + (0x28 + 2 * 4), env->regs[R_EDX]);
+ access_stl(&old, env->tr.base + (0x28 + 3 * 4), env->regs[R_EBX]);
+ access_stl(&old, env->tr.base + (0x28 + 4 * 4), env->regs[R_ESP]);
+ access_stl(&old, env->tr.base + (0x28 + 5 * 4), env->regs[R_EBP]);
+ access_stl(&old, env->tr.base + (0x28 + 6 * 4), env->regs[R_ESI]);
+ access_stl(&old, env->tr.base + (0x28 + 7 * 4), env->regs[R_EDI]);
+ for (i = 0; i < 6; i++) {
+ access_stw(&old, env->tr.base + (0x48 + i * 4),
+ env->segs[i].selector);
+ }
+ } else {
+ /* 16 bit */
+ access_stw(&old, env->tr.base + 0x0e, next_eip);
+ access_stw(&old, env->tr.base + 0x10, old_eflags);
+ access_stw(&old, env->tr.base + (0x12 + 0 * 2), env->regs[R_EAX]);
+ access_stw(&old, env->tr.base + (0x12 + 1 * 2), env->regs[R_ECX]);
+ access_stw(&old, env->tr.base + (0x12 + 2 * 2), env->regs[R_EDX]);
+ access_stw(&old, env->tr.base + (0x12 + 3 * 2), env->regs[R_EBX]);
+ access_stw(&old, env->tr.base + (0x12 + 4 * 2), env->regs[R_ESP]);
+ access_stw(&old, env->tr.base + (0x12 + 5 * 2), env->regs[R_EBP]);
+ access_stw(&old, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI]);
+ access_stw(&old, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI]);
+ for (i = 0; i < 4; i++) {
+ access_stw(&old, env->tr.base + (0x22 + i * 2),
+ env->segs[i].selector);
+ }
+ }
+
/* read all the registers from the new TSS */
if (type & 8) {
/* 32 bit */
@@ -428,49 +464,16 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
if (source == SWITCH_TSS_JMP || source == SWITCH_TSS_IRET) {
tss_set_busy(env, env->tr.selector, 0, retaddr);
}
- old_eflags = cpu_compute_eflags(env);
+
if (source == SWITCH_TSS_IRET) {
old_eflags &= ~NT_MASK;
+ if (old_type & 8) {
+ access_stl(&old, env->tr.base + 0x24, old_eflags);
+ } else {
+ access_stw(&old, env->tr.base + 0x10, old_eflags);
+ }
}
- /* save the current state in the old TSS */
- if (old_type & 8) {
- /* 32 bit */
- access_stl(&old, env->tr.base + 0x20, next_eip);
- access_stl(&old, env->tr.base + 0x24, old_eflags);
- access_stl(&old, env->tr.base + (0x28 + 0 * 4), env->regs[R_EAX]);
- access_stl(&old, env->tr.base + (0x28 + 1 * 4), env->regs[R_ECX]);
- access_stl(&old, env->tr.base + (0x28 + 2 * 4), env->regs[R_EDX]);
- access_stl(&old, env->tr.base + (0x28 + 3 * 4), env->regs[R_EBX]);
- access_stl(&old, env->tr.base + (0x28 + 4 * 4), env->regs[R_ESP]);
- access_stl(&old, env->tr.base + (0x28 + 5 * 4), env->regs[R_EBP]);
- access_stl(&old, env->tr.base + (0x28 + 6 * 4), env->regs[R_ESI]);
- access_stl(&old, env->tr.base + (0x28 + 7 * 4), env->regs[R_EDI]);
- for (i = 0; i < 6; i++) {
- access_stw(&old, env->tr.base + (0x48 + i * 4),
- env->segs[i].selector);
- }
- } else {
- /* 16 bit */
- access_stw(&old, env->tr.base + 0x0e, next_eip);
- access_stw(&old, env->tr.base + 0x10, old_eflags);
- access_stw(&old, env->tr.base + (0x12 + 0 * 2), env->regs[R_EAX]);
- access_stw(&old, env->tr.base + (0x12 + 1 * 2), env->regs[R_ECX]);
- access_stw(&old, env->tr.base + (0x12 + 2 * 2), env->regs[R_EDX]);
- access_stw(&old, env->tr.base + (0x12 + 3 * 2), env->regs[R_EBX]);
- access_stw(&old, env->tr.base + (0x12 + 4 * 2), env->regs[R_ESP]);
- access_stw(&old, env->tr.base + (0x12 + 5 * 2), env->regs[R_EBP]);
- access_stw(&old, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI]);
- access_stw(&old, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI]);
- for (i = 0; i < 4; i++) {
- access_stw(&old, env->tr.base + (0x22 + i * 2),
- env->segs[i].selector);
- }
- }
-
- /* now if an exception occurs, it will occurs in the next task
- context */
-
if (source == SWITCH_TSS_CALL) {
/*
* Thanks to the probe_access above, we know the first two
@@ -486,7 +489,9 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
}
/* set the new CPU state */
- /* from this point, any exception which occurs can give problems */
+
+ /* now if an exception occurs, it will occur in the next task context */
+
env->cr[0] |= CR0_TS_MASK;
env->hflags |= HF_TS_MASK;
env->tr.selector = tss_selector;
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 12/13] i386/sev: Don't allow automatic fallback to legacy KVM_SEV*_INIT
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (10 preceding siblings ...)
2024-07-14 11:10 ` [PULL 11/13] target/i386/tcg: save current task state before loading new one Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 11:10 ` [PULL 13/13] Revert "qemu-char: do not operate on sources from finalize callbacks" Paolo Bonzini
2024-07-14 22:47 ` [PULL 00/13] target/i386 changes for 2024-07-12 Richard Henderson
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth, Daniel P . Berrangé, kvm
From: Michael Roth <michael.roth@amd.com>
Currently if the 'legacy-vm-type' property of the sev-guest object is
'on', QEMU will attempt to use the newer KVM_SEV_INIT2 kernel
interface in conjunction with the newer KVM_X86_SEV_VM and
KVM_X86_SEV_ES_VM KVM VM types.
This can lead to measurement changes if, for instance, an SEV guest was
created on a host that originally had an older kernel that didn't
support KVM_SEV_INIT2, but is booted on the same host later on after the
host kernel was upgraded.
Instead, if legacy-vm-type is 'off', QEMU should fail if the
KVM_SEV_INIT2 interface is not provided by the current host kernel.
Modify the fallback handling accordingly.
In the future, VMSA features and other flags might be added to QEMU
which will require legacy-vm-type to be 'off' because they will rely
on the newer KVM_SEV_INIT2 interface. It may be difficult to convey to
users what values of legacy-vm-type are compatible with which
features/options, so as part of this rework, switch legacy-vm-type to a
tri-state OnOffAuto option. 'auto' in this case will automatically
switch to using the newer KVM_SEV_INIT2, but only if it is required to
make use of new VMSA features or other options only available via
KVM_SEV_INIT2.
Defining 'auto' in this way would avoid inadvertantly breaking
compatibility with older kernels since it would only be used in cases
where users opt into newer features that are only available via
KVM_SEV_INIT2 and newer kernels, and provide better default behavior
than the legacy-vm-type=off behavior that was previously in place, so
make it the default for 9.1+ machine types.
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
cc: kvm@vger.kernel.org
Signed-off-by: Michael Roth <michael.roth@amd.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Link: https://lore.kernel.org/r/20240710041005.83720-1-michael.roth@amd.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qapi/qom.json | 18 ++++++----
hw/i386/pc.c | 2 +-
target/i386/sev.c | 87 +++++++++++++++++++++++++++++++++++++++--------
3 files changed, 84 insertions(+), 23 deletions(-)
diff --git a/qapi/qom.json b/qapi/qom.json
index 8e75a419c30..7eccd2e14e2 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -924,12 +924,16 @@
# @handle: SEV firmware handle (default: 0)
#
# @legacy-vm-type: Use legacy KVM_SEV_INIT KVM interface for creating the VM.
-# The newer KVM_SEV_INIT2 interface syncs additional vCPU
-# state when initializing the VMSA structures, which will
-# result in a different guest measurement. Set this to
-# maintain compatibility with older QEMU or kernel versions
-# that rely on legacy KVM_SEV_INIT behavior.
-# (default: false) (since 9.1)
+# The newer KVM_SEV_INIT2 interface, from Linux >= 6.10, syncs
+# additional vCPU state when initializing the VMSA structures,
+# which will result in a different guest measurement. Set
+# this to 'on' to force compatibility with older QEMU or kernel
+# versions that rely on legacy KVM_SEV_INIT behavior. 'auto'
+# will behave identically to 'on', but will automatically
+# switch to using KVM_SEV_INIT2 if the user specifies any
+# additional options that require it. If set to 'off', QEMU
+# will require KVM_SEV_INIT2 unconditionally.
+# (default: off) (since 9.1)
#
# Since: 2.12
##
@@ -939,7 +943,7 @@
'*session-file': 'str',
'*policy': 'uint32',
'*handle': 'uint32',
- '*legacy-vm-type': 'bool' } }
+ '*legacy-vm-type': 'OnOffAuto' } }
##
# @SevSnpGuestProperties:
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4fbc5774708..c74931d577a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -83,7 +83,7 @@ GlobalProperty pc_compat_9_0[] = {
{ TYPE_X86_CPU, "x-amd-topoext-features-only", "false" },
{ TYPE_X86_CPU, "x-l1-cache-per-thread", "false" },
{ TYPE_X86_CPU, "guest-phys-bits", "0" },
- { "sev-guest", "legacy-vm-type", "true" },
+ { "sev-guest", "legacy-vm-type", "on" },
{ TYPE_X86_CPU, "legacy-multi-node", "on" },
};
const size_t pc_compat_9_0_len = G_N_ELEMENTS(pc_compat_9_0);
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2ba5f517228..a1157c0ede6 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -144,7 +144,7 @@ struct SevGuestState {
uint32_t policy;
char *dh_cert_file;
char *session_file;
- bool legacy_vm_type;
+ OnOffAuto legacy_vm_type;
};
struct SevSnpGuestState {
@@ -1369,6 +1369,17 @@ sev_vm_state_change(void *opaque, bool running, RunState state)
}
}
+/*
+ * This helper is to examine sev-guest properties and determine if any options
+ * have been set which rely on the newer KVM_SEV_INIT2 interface and associated
+ * KVM VM types.
+ */
+static bool sev_init2_required(SevGuestState *sev_guest)
+{
+ /* Currently no KVM_SEV_INIT2-specific options are exposed via QEMU */
+ return false;
+}
+
static int sev_kvm_type(X86ConfidentialGuest *cg)
{
SevCommonState *sev_common = SEV_COMMON(cg);
@@ -1379,14 +1390,39 @@ static int sev_kvm_type(X86ConfidentialGuest *cg)
goto out;
}
- kvm_type = (sev_guest->policy & SEV_POLICY_ES) ?
- KVM_X86_SEV_ES_VM : KVM_X86_SEV_VM;
- if (kvm_is_vm_type_supported(kvm_type) && !sev_guest->legacy_vm_type) {
- sev_common->kvm_type = kvm_type;
- } else {
+ /* These are the only cases where legacy VM types can be used. */
+ if (sev_guest->legacy_vm_type == ON_OFF_AUTO_ON ||
+ (sev_guest->legacy_vm_type == ON_OFF_AUTO_AUTO &&
+ !sev_init2_required(sev_guest))) {
sev_common->kvm_type = KVM_X86_DEFAULT_VM;
+ goto out;
}
+ /*
+ * Newer VM types are required, either explicitly via legacy-vm-type=on, or
+ * implicitly via legacy-vm-type=auto along with additional sev-guest
+ * properties that require the newer VM types.
+ */
+ kvm_type = (sev_guest->policy & SEV_POLICY_ES) ?
+ KVM_X86_SEV_ES_VM : KVM_X86_SEV_VM;
+ if (!kvm_is_vm_type_supported(kvm_type)) {
+ if (sev_guest->legacy_vm_type == ON_OFF_AUTO_AUTO) {
+ error_report("SEV: host kernel does not support requested %s VM type, which is required "
+ "for the set of options specified. To allow use of the legacy "
+ "KVM_X86_DEFAULT_VM VM type, please disable any options that are not "
+ "compatible with the legacy VM type, or upgrade your kernel.",
+ kvm_type == KVM_X86_SEV_VM ? "KVM_X86_SEV_VM" : "KVM_X86_SEV_ES_VM");
+ } else {
+ error_report("SEV: host kernel does not support requested %s VM type. To allow use of "
+ "the legacy KVM_X86_DEFAULT_VM VM type, the 'legacy-vm-type' argument "
+ "must be set to 'on' or 'auto' for the sev-guest object.",
+ kvm_type == KVM_X86_SEV_VM ? "KVM_X86_SEV_VM" : "KVM_X86_SEV_ES_VM");
+ }
+
+ return -1;
+ }
+
+ sev_common->kvm_type = kvm_type;
out:
return sev_common->kvm_type;
}
@@ -1477,14 +1513,24 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
}
trace_kvm_sev_init();
- if (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common)) == KVM_X86_DEFAULT_VM) {
+ switch (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common))) {
+ case KVM_X86_DEFAULT_VM:
cmd = sev_es_enabled() ? KVM_SEV_ES_INIT : KVM_SEV_INIT;
ret = sev_ioctl(sev_common->sev_fd, cmd, NULL, &fw_error);
- } else {
+ break;
+ case KVM_X86_SEV_VM:
+ case KVM_X86_SEV_ES_VM:
+ case KVM_X86_SNP_VM: {
struct kvm_sev_init args = { 0 };
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
+ break;
+ }
+ default:
+ error_setg(errp, "%s: host kernel does not support the requested SEV configuration.",
+ __func__);
+ return -1;
}
if (ret) {
@@ -2074,14 +2120,23 @@ sev_guest_set_session_file(Object *obj, const char *value, Error **errp)
SEV_GUEST(obj)->session_file = g_strdup(value);
}
-static bool sev_guest_get_legacy_vm_type(Object *obj, Error **errp)
+static void sev_guest_get_legacy_vm_type(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
{
- return SEV_GUEST(obj)->legacy_vm_type;
+ SevGuestState *sev_guest = SEV_GUEST(obj);
+ OnOffAuto legacy_vm_type = sev_guest->legacy_vm_type;
+
+ visit_type_OnOffAuto(v, name, &legacy_vm_type, errp);
}
-static void sev_guest_set_legacy_vm_type(Object *obj, bool value, Error **errp)
+static void sev_guest_set_legacy_vm_type(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
{
- SEV_GUEST(obj)->legacy_vm_type = value;
+ SevGuestState *sev_guest = SEV_GUEST(obj);
+
+ visit_type_OnOffAuto(v, name, &sev_guest->legacy_vm_type, errp);
}
static void
@@ -2107,9 +2162,9 @@ sev_guest_class_init(ObjectClass *oc, void *data)
sev_guest_set_session_file);
object_class_property_set_description(oc, "session-file",
"guest owners session parameters (encoded with base64)");
- object_class_property_add_bool(oc, "legacy-vm-type",
- sev_guest_get_legacy_vm_type,
- sev_guest_set_legacy_vm_type);
+ object_class_property_add(oc, "legacy-vm-type", "OnOffAuto",
+ sev_guest_get_legacy_vm_type,
+ sev_guest_set_legacy_vm_type, NULL, NULL);
object_class_property_set_description(oc, "legacy-vm-type",
"use legacy VM type to maintain measurement compatibility with older QEMU or kernel versions.");
}
@@ -2125,6 +2180,8 @@ sev_guest_instance_init(Object *obj)
object_property_add_uint32_ptr(obj, "policy", &sev_guest->policy,
OBJ_PROP_FLAG_READWRITE);
object_apply_compat_props(obj);
+
+ sev_guest->legacy_vm_type = ON_OFF_AUTO_AUTO;
}
/* guest info specific sev/sev-es */
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 13/13] Revert "qemu-char: do not operate on sources from finalize callbacks"
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (11 preceding siblings ...)
2024-07-14 11:10 ` [PULL 12/13] i386/sev: Don't allow automatic fallback to legacy KVM_SEV*_INIT Paolo Bonzini
@ 2024-07-14 11:10 ` Paolo Bonzini
2024-07-14 22:47 ` [PULL 00/13] target/i386 changes for 2024-07-12 Richard Henderson
13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2024-07-14 11:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Sergey Dyasli
From: Sergey Dyasli <sergey.dyasli@nutanix.com>
This reverts commit 2b316774f60291f57ca9ecb6a9f0712c532cae34.
After 038b4217884c ("Revert "chardev: use a child source for qio input
source"") we've been observing the "iwp->src == NULL" assertion
triggering periodically during the initial capabilities querying by
libvirtd. One of possible backtraces:
Thread 1 (Thread 0x7f16cd4f0700 (LWP 43858)):
0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
1 0x00007f16c6c21e65 in __GI_abort () at abort.c:79
2 0x00007f16c6c21d39 in __assert_fail_base at assert.c:92
3 0x00007f16c6c46e86 in __GI___assert_fail (assertion=assertion@entry=0x562e9bcdaadd "iwp->src == NULL", file=file@entry=0x562e9bcdaac8 "../chardev/char-io.c", line=line@entry=99, function=function@entry=0x562e9bcdab10 <__PRETTY_FUNCTION__.20549> "io_watch_poll_finalize") at assert.c:101
4 0x0000562e9ba20c2c in io_watch_poll_finalize (source=<optimized out>) at ../chardev/char-io.c:99
5 io_watch_poll_finalize (source=<optimized out>) at ../chardev/char-io.c:88
6 0x00007f16c904aae0 in g_source_unref_internal () from /lib64/libglib-2.0.so.0
7 0x00007f16c904baf9 in g_source_destroy_internal () from /lib64/libglib-2.0.so.0
8 0x0000562e9ba20db0 in io_remove_watch_poll (source=0x562e9d6720b0) at ../chardev/char-io.c:147
9 remove_fd_in_watch (chr=chr@entry=0x562e9d5f3800) at ../chardev/char-io.c:153
10 0x0000562e9ba23ffb in update_ioc_handlers (s=0x562e9d5f3800) at ../chardev/char-socket.c:592
11 0x0000562e9ba2072f in qemu_chr_fe_set_handlers_full at ../chardev/char-fe.c:279
12 0x0000562e9ba207a9 in qemu_chr_fe_set_handlers at ../chardev/char-fe.c:304
13 0x0000562e9ba2ca75 in monitor_qmp_setup_handlers_bh (opaque=0x562e9d4c2c60) at ../monitor/qmp.c:509
14 0x0000562e9bb6222e in aio_bh_poll (ctx=ctx@entry=0x562e9d4c2f20) at ../util/async.c:216
15 0x0000562e9bb4de0a in aio_poll (ctx=0x562e9d4c2f20, blocking=blocking@entry=true) at ../util/aio-posix.c:722
16 0x0000562e9b99dfaa in iothread_run (opaque=0x562e9d4c26f0) at ../iothread.c:63
17 0x0000562e9bb505a4 in qemu_thread_start (args=0x562e9d4c7ea0) at ../util/qemu-thread-posix.c:543
18 0x00007f16c70081ca in start_thread (arg=<optimized out>) at pthread_create.c:479
19 0x00007f16c6c398d3 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
io_remove_watch_poll(), which makes sure that iwp->src is NULL, calls
g_source_destroy() which finds that iwp->src is not NULL in the finalize
callback. This can only happen if another thread has managed to trigger
io_watch_poll_prepare() callback in the meantime.
Move iwp->src destruction back to the finalize callback to prevent the
described race, and also remove the stale comment. The deadlock glib bug
was fixed back in 2010 by b35820285668 ("gmain: move finalization of
GSource outside of context lock").
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sergey Dyasli <sergey.dyasli@nutanix.com>
Link: https://lore.kernel.org/r/20240712092659.216206-1-sergey.dyasli@nutanix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
chardev/char-io.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/chardev/char-io.c b/chardev/char-io.c
index dab77b112e3..3be17b51ca5 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -87,16 +87,12 @@ static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
static void io_watch_poll_finalize(GSource *source)
{
- /*
- * Due to a glib bug, removing the last reference to a source
- * inside a finalize callback causes recursive locking (and a
- * deadlock). This is not a problem inside other callbacks,
- * including dispatch callbacks, so we call io_remove_watch_poll
- * to remove this source. At this point, iwp->src must
- * be NULL, or we would leak it.
- */
IOWatchPoll *iwp = io_watch_poll_from_source(source);
- assert(iwp->src == NULL);
+ if (iwp->src) {
+ g_source_destroy(iwp->src);
+ g_source_unref(iwp->src);
+ iwp->src = NULL;
+ }
}
static GSourceFuncs io_watch_poll_funcs = {
@@ -139,11 +135,6 @@ static void io_remove_watch_poll(GSource *source)
IOWatchPoll *iwp;
iwp = io_watch_poll_from_source(source);
- if (iwp->src) {
- g_source_destroy(iwp->src);
- g_source_unref(iwp->src);
- iwp->src = NULL;
- }
g_source_destroy(&iwp->parent);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PULL 00/13] target/i386 changes for 2024-07-12
2024-07-14 11:10 [PULL 00/13] target/i386 changes for 2024-07-12 Paolo Bonzini
` (12 preceding siblings ...)
2024-07-14 11:10 ` [PULL 13/13] Revert "qemu-char: do not operate on sources from finalize callbacks" Paolo Bonzini
@ 2024-07-14 22:47 ` Richard Henderson
13 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2024-07-14 22:47 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
On 7/14/24 04:10, Paolo Bonzini wrote:
> The following changes since commit 23901b2b721c0576007ab7580da8aa855d6042a9:
>
> Merge tag 'pull-target-arm-20240711' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-07-11 12:00:00 -0700)
>
> are available in the Git repository at:
>
> https://gitlab.com/bonzini/qemu.git tags/for-upstream-i386
>
> for you to fetch changes up to cdcadf9ee9efef96323e0b88fccff589f06fc0ee:
>
> i386/sev: Don't allow automatic fallback to legacy KVM_SEV*_INIT (2024-07-12 15:35:54 +0200)
>
> ----------------------------------------------------------------
> * target/i386/tcg: fixes for seg_helper.c
> * SEV: Don't allow automatic fallback to legacy KVM_SEV_INIT,
> but also don't use it by default
Fails testing:
https://gitlab.com/qemu-project/qemu/-/jobs/7338361630
2024-07-14 23:45:07,744 __init__ L0153 DEBUG| EIP: alternative_instructions+0x2b/0xfa
2024-07-14 23:45:07,746 __init__ L0153 DEBUG| Code: 89 e5 83 ec 08 64 a1 c0 06 f4
c3 89 45 fc 31 c0 b8 e4 f7 ef c3 c7 45 f8
00 00 00 00 e8 84 6f 7a ff 85 c0 74 02 0f 0b 8d 45 f8 <cc> 90 90 90 90 83 7d f8 01 74 02
0f 0b b8 e4 f7 ef c3 e8 04 6e 7a
2024-07-14 23:45:07,747 __init__ L0153 DEBUG| EAX: c3e0bf38 EBX: 00000000 ECX:
00000000 EDX: 00200292
2024-07-14 23:45:07,747 __init__ L0153 DEBUG| ESI: c3d54b3f EDI: c3d555e0 EBP:
c3e0bf40 ESP: c3e0bf38
2024-07-14 23:45:07,748 __init__ L0153 DEBUG| DS: 007b ES: 007b FS: 00d8 GS: 0000
SS: 0068 EFLAGS: 00210246
2024-07-14 23:45:07,748 __init__ L0153 DEBUG| CR0: 80050033 CR2: c3e0bf34 CR3:
03f4c000 CR4: 000006d0
2024-07-14 23:45:07,748 __init__ L0153 DEBUG| Call Trace:
2024-07-14 23:45:07,750 __init__ L0153 DEBUG| check_bugs+0x900/0x91e
2024-07-14 23:45:07,750 __init__ L0153 DEBUG| ? __get_locked_pte+0x67/0xb0
2024-07-14 23:45:07,750 __init__ L0153 DEBUG| start_kernel+0x4d3/0x501
2024-07-14 23:45:07,750 __init__ L0153 DEBUG| ? set_intr_gate+0x42/0x55
2024-07-14 23:45:07,750 __init__ L0153 DEBUG| i386_start_kernel+0x43/0x45
2024-07-14 23:45:07,751 __init__ L0153 DEBUG| startup_32_smp+0x161/0x164
2024-07-14 23:45:07,751 __init__ L0153 DEBUG| Modules linked in:
2024-07-14 23:45:07,751 __init__ L0153 DEBUG| CR2: 00000000c3e0bf34
2024-07-14 23:45:07,752 __init__ L0153 DEBUG| ---[ end trace 7adaac7a13f2a45f ]---
2024-07-14 23:45:07,752 __init__ L0153 DEBUG| EIP: alternative_instructions+0x2b/0xfa
2024-07-14 23:45:07,753 __init__ L0153 DEBUG| Code: 89 e5 83 ec 08 64 a1 c0 06 f4
c3 89 45 fc 31 c0 b8 e4 f7 ef c3 c7 45 f8
00 00 00 00 e8 84 6f 7a ff 85 c0 74 02 0f 0b 8d 45 f8 <cc> 90 90 90 90 83 7d f8 01 74 02
0f 0b b8 e4 f7 ef c3 e8 04 6e 7a
2024-07-14 23:45:07,753 __init__ L0153 DEBUG| EAX: c3e0bf38 EBX: 00000000 ECX:
00000000 EDX: 00200292
2024-07-14 23:45:07,753 __init__ L0153 DEBUG| ESI: c3d54b3f EDI: c3d555e0 EBP:
c3e0bf40 ESP: c3e0bf38
2024-07-14 23:45:07,754 __init__ L0153 DEBUG| DS: 007b ES: 007b FS: 00d8 GS: 0000
SS: 0068 EFLAGS: 00210246
2024-07-14 23:45:07,754 __init__ L0153 DEBUG| CR0: 80050033 CR2: c3e0bf34 CR3:
03f4c000 CR4: 000006d0
2024-07-14 23:45:07,754 __init__ L0153 DEBUG| Kernel panic - not syncing:
Attempted to kill the idle task!
r~
^ permalink raw reply [flat|nested] 15+ messages in thread