When a device handler (e.g. vmport) calls cpu_synchronize_state() during I/O port dispatch, it sets cpu->accel->dirty = true and may modify registers directly in env. The old PIO code ignored this: it unconditionally wrote the stale info->rax from the VM-exit intercept message back to the hypervisor and then cleared dirty, discarding any register changes made by the device.
Bifurcate both handlers on cpu->accel->dirty: handle_pio_non_str: - dirty path: update env->eip directly. For reads (IN), merge the I/O result into env->regs[R_EAX] (which may have been modified by the device) rather than info->rax. For writes (OUT), leave RAX untouched. Flush all registers via mshv_store_regs() and clear dirty. - non-dirty path: write RIP and RAX via set_x64_registers hypercall as before. handle_pio_str: - dirty path: update env->eip and the appropriate index register (RSI for OUTS, RDI for INS) directly. Flush via mshv_store_regs() and clear dirty. - non-dirty path: write the index register and RIP via set_x64_registers. Drop the RAX assignment that was here before; string I/O does not modify RAX, and set_x64_registers is hardcoded to write only 2 registers so the third slot was silently ignored anyway. Remove the unconditional "cpu->accel->dirty = false" at the end of both handlers. In the non-dirty fast path it was redundant (already false). In the dirty path it was actively harmful: it told the vcpu run loop that env was clean when it was not, losing the device's modifications. Signed-off-by: Doru Blânzeanu <[email protected]> --- target/i386/mshv/mshv-cpu.c | 82 ++++++++++++++++++++++++++----------- 1 file changed, 59 insertions(+), 23 deletions(-) diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c index 0cfac26a5c..7be3fdcc45 100644 --- a/target/i386/mshv/mshv-cpu.c +++ b/target/i386/mshv/mshv-cpu.c @@ -1348,7 +1348,7 @@ static int pio_write(uint64_t port, const uint8_t *data, uintptr_t size, return ret; } -static int handle_pio_non_str(const CPUState *cpu, +static int handle_pio_non_str(CPUState *cpu, hv_x64_io_port_intercept_message *info) { size_t len = info->access_info.access_size; @@ -1357,10 +1357,12 @@ static int handle_pio_non_str(const CPUState *cpu, uint32_t val, eax; const uint32_t eax_mask = 0xffffffffu >> (32 - len * 8); size_t insn_len; - uint64_t rip, rax; + uint64_t rip; uint32_t reg_names[2]; uint64_t reg_values[2]; uint16_t port = info->port_number; + X86CPU *x86_cpu = X86_CPU(cpu); + CPUX86State *env = &x86_cpu->env; if (access_type == HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) { union { @@ -1391,21 +1393,40 @@ static int handle_pio_non_str(const CPUState *cpu, /* Advance RIP and update RAX */ rip = info->header.rip + insn_len; - rax = info->rax; - reg_names[0] = HV_X64_REGISTER_RIP; - reg_values[0] = rip; - reg_names[1] = HV_X64_REGISTER_RAX; - reg_values[1] = rax; + if (cpu->accel->dirty) { + env->eip = rip; + if (access_type != HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) { + /* + * For reads, merge the I/O result into the current RAX. + * Use env->regs[R_EAX] as the base since a device handler + * (e.g. vmport) may have called cpu_synchronize_state() + * and modified registers. + */ + eax = (((uint32_t)env->regs[R_EAX]) & ~eax_mask) + | (val & eax_mask); + env->regs[R_EAX] = (uint64_t)eax; + } + /* Sync modified standard registers back and clear dirty. */ + ret = mshv_store_regs(cpu); + if (ret < 0) { + error_report("Failed to store registers after PIO"); + return -1; + } + cpu->accel->dirty = false; + } else { + reg_names[0] = HV_X64_REGISTER_RIP; + reg_values[0] = rip; + reg_names[1] = HV_X64_REGISTER_RAX; + reg_values[1] = info->rax; - ret = set_x64_registers(cpu, reg_names, reg_values); - if (ret < 0) { - error_report("Failed to set x64 registers"); - return -1; + ret = set_x64_registers(cpu, reg_names, reg_values); + if (ret < 0) { + error_report("Failed to set x64 registers"); + return -1; + } } - cpu->accel->dirty = false; - return 0; } @@ -1521,6 +1542,7 @@ static int handle_pio_str(CPUState *cpu, hv_x64_io_port_intercept_message *info) bool repop = info->access_info.rep_prefix == 1; size_t repeat = repop ? info->rcx : 1; size_t insn_len = info->header.instruction_length; + uint64_t rip; bool direction_flag; uint32_t reg_names[3]; uint64_t reg_values[3]; @@ -1554,18 +1576,32 @@ static int handle_pio_str(CPUState *cpu, hv_x64_io_port_intercept_message *info) reg_values[0] = info->rdi; } - reg_names[1] = HV_X64_REGISTER_RIP; - reg_values[1] = info->header.rip + insn_len; - reg_names[2] = HV_X64_REGISTER_RAX; - reg_values[2] = info->rax; + rip = info->header.rip + insn_len; - ret = set_x64_registers(cpu, reg_names, reg_values); - if (ret < 0) { - error_report("Failed to set x64 registers"); - return -1; - } + if (cpu->accel->dirty) { + env->eip = rip; + if (access_type == HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) { + env->regs[R_ESI] = info->rsi; + } else { + env->regs[R_EDI] = info->rdi; + } + /* Sync modified standard registers back and clear dirty. */ + ret = mshv_store_regs(cpu); + if (ret < 0) { + error_report("Failed to store registers after string PIO"); + return -1; + } + cpu->accel->dirty = false; + } else { + reg_names[1] = HV_X64_REGISTER_RIP; + reg_values[1] = rip; - cpu->accel->dirty = false; + ret = set_x64_registers(cpu, reg_names, reg_values); + if (ret < 0) { + error_report("Failed to set x64 registers"); + return -1; + } + } return 0; } -- 2.53.0
