mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

target/i386: pcrel: store low bits of physical address in data[0]

Browse Source 
For PC-relative translation blocks, env->eip changes during the
execution of a translation block, Therefore, QEMU must be able to
recover an instruction's PC just from the TranslationBlock struct and
the instruction data with.  Because a TB will not span two pages, QEMU
stores all the low bits of EIP in the instruction data and replaces them
in x86_restore_state_to_opc.  Bits 12 and higher (which may vary between
executions of a PCREL TB, since these only use the physical address in
the hash key) are kept unmodified from env->eip.  The assumption is that
these bits of EIP, unlike bits 0-11, will not change as the translation
block executes.

Unfortunately, this is incorrect when the CS base is not aligned to a page.
Then the linear address of the instructions (i.e. the one with the
CS base addred) indeed will never span two pages, but bits 12+ of EIP
can actually change.  For example, if CS base is 0x80262200 and EIP =
0x6FF4, the first instruction in the translation block will be at linear
address 0x802691F4.  Even a very small TB will cross to EIP = 0x7xxx,
while the linear addresses will remain comfortably within a single page.

The fix is simply to use the low bits of the linear address for data[0],
since those don't change.  Then x86_restore_state_to_opc uses tb->cs_base
to compute a temporary linear address (referring to some unknown
instruction in the TB, but with the correct values of bits 12 and higher);
the low bits are replaced with data[0], and EIP is obtained by subtracting
again the CS base.

Huge thanks to Mark Cave-Ayland for the image and initial debugging,
and to Gitlab user @kjliew for help with bisecting another occurrence
of (hopefully!) the same bug.

It should be relatively easy to write a testcase that performs MMIO on
an EIP with different bits 12+ than the first instruction of the translation
block; any help is welcome.

Fixes: e3a79e0e87 ("target/i386: Enable TARGET_TB_PCREL", 2022-10-11)
Cc: qemu-stable@nongnu.org
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Richard Henderson <richard.henderson@linaro.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1759
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1964
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2012
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Paolo Bonzini 2024-01-17 16:27:42 +01:00
parent 2926eab896
commit 729ba8e933
2 changed files with 16 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
target/i386/tcg/tcg-cpu.c
View File

@ -68,14 +68,26 @@ static void x86_restore_state_to_opc(CPUState *cs,
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = &cpu->env;
int cc_op = data[1];
uint64_t new_pc;
if (tb_cflags(tb) & CF_PCREL) {
env->eip = (env->eip & TARGET_PAGE_MASK) | data[0];
} else if (tb->flags & HF_CS64_MASK) {
env->eip = data[0];
/*
* data[0] in PC-relative TBs is also a linear address, i.e. an address with
* the CS base added, because it is not guaranteed that EIP bits 12 and higher
* stay the same across the translation block. Add the CS base back before
* replacing the low bits, and subtract it below just like for !CF_PCREL.
*/
uint64_t pc = env->eip + tb->cs_base;
new_pc = (pc & TARGET_PAGE_MASK) | data[0];
} else {
env->eip = (uint32_t)(data[0] - tb->cs_base);
new_pc = data[0];
}
if (tb->flags & HF_CS64_MASK) {
env->eip = new_pc;
} else {
env->eip = (uint32_t)(new_pc - tb->cs_base);
}
if (cc_op != CC_OP_DYNAMIC) {
env->cc_op = cc_op;
}

1
target/i386/tcg/translate.c
View File

@ -6996,7 +6996,6 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
dc->prev_insn_end = tcg_last_op();
if (tb_cflags(dcbase->tb) & CF_PCREL) {
pc_arg -= dc->cs_base;
pc_arg &= ~TARGET_PAGE_MASK;
}
tcg_gen_insn_start(pc_arg, dc->cc_op);