Re: [PATCH] target/i386/mshv: add MshvPutState for vcpu writes

["Doru Blânzeanu" <dblanzeanu@linux.microsoft.com>]

On Mon, Nov 09, 2026 at 01:39:49PM +0100, Magnus Kulke wrote:
> Currently QEMU marks a vcpu state as dirty also on merely inspectional
> reads, e.g. via "info registers", which triggers a writeback to the
> hypervisor. For MSRs that cause a side-effect when written to this is
> undesired.
> 
> We introduce an MshvPutState flag into the writeback logic to differentiate
> "runtime" from "full" and "reset" writebacks that should occur e.g. after
> a migration or startup.
> 
> Discrete side-effectful MSRs are excluded in runtime writebacks. So far
> we only mark one MSR as not runtime-safe, but probably more will be
> added eventually.
> 
> Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
> ---
>  accel/mshv/mshv-all.c       | 10 +++++-----
>  include/system/mshv_int.h   | 13 +++++++++++--
>  target/i386/mshv/mshv-cpu.c | 10 ++++++++--
>  target/i386/mshv/msr.c      | 21 ++++++++++++++++++++-
>  4 files changed, 44 insertions(+), 10 deletions(-)
> 
> diff --git a/accel/mshv/mshv-all.c b/accel/mshv/mshv-all.c
> index d820a67b95..d64599e2e2 100644
> --- a/accel/mshv/mshv-all.c
> +++ b/accel/mshv/mshv-all.c
> @@ -759,10 +759,10 @@ static int mshv_cpu_exec(CPUState *cpu)
>  
>      do {
>          if (cpu->vcpu_dirty) {
> -            ret = mshv_arch_store_vcpu_state(cpu);
> +            ret = mshv_arch_store_vcpu_state(cpu, MSHV_PUT_RUNTIME_STATE);
>              if (ret) {
> -                error_report("Failed to put registers after init: %s",
> -                              strerror(-ret));
> +                error_report("Failed to put registers at runtime: %s",
> +                             strerror(-ret));
>                  ret = -1;
>                  break;
>              }
> @@ -890,7 +890,7 @@ static void mshv_start_vcpu_thread(CPUState *cpu)
>  static void do_mshv_cpu_synchronize_post_init(CPUState *cpu,
>                                                run_on_cpu_data arg)
>  {
> -    int ret = mshv_arch_store_vcpu_state(cpu);
> +    int ret = mshv_arch_store_vcpu_state(cpu, MSHV_PUT_FULL_STATE);
>      if (ret < 0) {
>          error_report("Failed to put registers after init: %s", strerror(-ret));
>          abort();
> @@ -906,7 +906,7 @@ static void mshv_cpu_synchronize_post_init(CPUState *cpu)
>  
>  static void mshv_cpu_synchronize_post_reset(CPUState *cpu)
>  {
> -    int ret = mshv_arch_store_vcpu_state(cpu);
> +    int ret = mshv_arch_store_vcpu_state(cpu, MSHV_PUT_RESET_STATE);
>      if (ret) {
>          error_report("Failed to put registers after reset: %s",
>                       strerror(-ret));
> diff --git a/include/system/mshv_int.h b/include/system/mshv_int.h
> index 1f77ffd152..b7b7c56443 100644
> --- a/include/system/mshv_int.h
> +++ b/include/system/mshv_int.h
> @@ -98,6 +98,15 @@ typedef enum MshvVmExit {
>      MshvVmExitDebug    = 3,
>  } MshvVmExit;
>  
> +typedef enum MshvPutState {
> +    /* state subset only touched by the VCPU itself during runtime */
> +    MSHV_PUT_RUNTIME_STATE = 1,
> +    /* state subset modified during VCPU reset */
> +    MSHV_PUT_RESET_STATE = 2,
> +    /* full state set, modified during initialization or on vmload */
> +    MSHV_PUT_FULL_STATE = 3,
> +} MshvPutState;
> +
>  void mshv_init_mmio_emu(void);
>  int mshv_create_vcpu(int vm_fd, uint8_t vp_index, int *cpu_fd);
>  void mshv_remove_vcpu(int vm_fd, int cpu_fd);
> @@ -107,7 +116,7 @@ int mshv_set_generic_regs(const CPUState *cpu, const hv_register_assoc *assocs,
>                            size_t n_regs);
>  int mshv_get_generic_regs(CPUState *cpu, hv_register_assoc *assocs,
>                            size_t n_regs);
> -int mshv_arch_store_vcpu_state(const CPUState *cpu);
> +int mshv_arch_store_vcpu_state(const CPUState *cpu, MshvPutState state);
>  int mshv_arch_load_vcpu_state(CPUState *cpu);
>  int mshv_arch_set_partition_msrs(const CPUState *cpu);
>  int mshv_arch_set_mp_state(const CPUState *cpu);
> @@ -152,7 +161,7 @@ void mshv_log_global_stop(MemoryListener *listener);
>  /* msr */
>  int mshv_init_msrs(const CPUState *cpu);
>  int mshv_get_msrs(CPUState *cpu);
> -int mshv_set_msrs(const CPUState *cpu);
> +int mshv_set_msrs(const CPUState *cpu, MshvPutState put_state);
>  
>  /* synic */
>  int mshv_get_simp(int cpu_fd, uint8_t *page);
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index 4df73bb497..72ef01a597 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -1511,7 +1511,13 @@ static int set_synic_state(const CPUState *cpu)
>      return 0;
>  }
>  
> -int mshv_arch_store_vcpu_state(const CPUState *cpu)
> +/*
> + * The put_state param is passed to decide whether we need a full restoration
> + * of all registers (e.g. after a migration ) or just a subset during runtime
> + * (e.g after "info registers"). The latter should avoid writing registers that
> + * cause a side-effect.
> + */
> +int mshv_arch_store_vcpu_state(const CPUState *cpu, MshvPutState put_state)
>  {
>      int ret;
>  
> @@ -1541,7 +1547,7 @@ int mshv_arch_store_vcpu_state(const CPUState *cpu)
>          return ret;
>      }
>  
> -    ret = mshv_set_msrs(cpu);
> +    ret = mshv_set_msrs(cpu, put_state);
>      if (ret < 0) {
>          return ret;
>      }
> diff --git a/target/i386/mshv/msr.c b/target/i386/mshv/msr.c
> index 96bd552916..a06844cc99 100644
> --- a/target/i386/mshv/msr.c
> +++ b/target/i386/mshv/msr.c
> @@ -324,6 +324,20 @@ static bool msr_supported(uint32_t name)
>      return true;
>  }
>  
> +static bool msr_is_runtime_safe(uint32_t name)
> +{
> +    switch (name) {
> +    /*
> +     * HV_X64_REGISTER_TSC will invalidate the guest's tsc_page if written to
> +     * while the guest's timer is not frozen.
> +     */
> +    case HV_X64_REGISTER_TSC:
> +        return false;
> +    default:
> +        return true;
> +    }
> +}
> +
>  int mshv_get_msrs(CPUState *cpu)
>  {
>      int ret = 0;
> @@ -406,13 +420,14 @@ static void load_from_env(const CPUState *cpu, struct hv_register_assoc *assocs,
>      load_from_env_mtrr_phys(cpu, &assocs[mtrr_offset], MSHV_MTRR_MSR_COUNT);
>  }
>  
> -int mshv_set_msrs(const CPUState *cpu)
> +int mshv_set_msrs(const CPUState *cpu, MshvPutState put_state)
>  {
>      size_t n_assocs = MSHV_MSR_TOTAL_COUNT;
>      struct hv_register_assoc assocs[MSHV_MSR_TOTAL_COUNT] = { 0 };
>      int ret;
>      size_t i, j;
>      X86CPU *x86cpu = X86_CPU(cpu);
> +    bool runtime = (put_state <= MSHV_PUT_RUNTIME_STATE);
>  
>      load_from_env(cpu, assocs, n_assocs);
>  
> @@ -432,6 +447,10 @@ int mshv_set_msrs(const CPUState *cpu)
>              continue;
>          }
>  
> +        if (runtime && !msr_is_runtime_safe(name)) {
> +            continue;
> +        }
> +
>          if (j != i) {
>              assocs[j] = assocs[i];
>          }
> -- 
> 2.34.1

Tested-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>