On Tue, May 05, 2026 at 09:50:28PM +0300, Doru Blânzeanu wrote: > 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
Reviewed-by: Magnus Kulke <[email protected]>
