Re: [PATCH 12/13] i386: hvf: Move mmio_buf into CPUX86State

[Paolo Bonzini]

On 28/05/20 21:37, Roman Bolshakov wrote:
> There's no similar field in CPUX86State, but it's needed for MMIO traps.
> 
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>  target/i386/cpu.h         |  1 +
>  target/i386/hvf/hvf.c     |  5 +++++
>  target/i386/hvf/x86.h     |  1 -
>  target/i386/hvf/x86_emu.c | 12 ++++++------
>  4 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 7e6566565a..be44e19154 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1593,6 +1593,7 @@ typedef struct CPUX86State {
>  #endif
>  #if defined(CONFIG_HVF)
>      hvf_lazy_flags hvf_lflags;
> +    void *hvf_mmio_buf;
>      HVFX86EmulatorState *hvf_emul;
>  #endif
>  
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 4cee496d71..57696c46c7 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -533,7 +533,11 @@ void hvf_reset_vcpu(CPUState *cpu) {
>  
>  void hvf_vcpu_destroy(CPUState *cpu)
>  {
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +
>      hv_return_t ret = hv_vcpu_destroy((hv_vcpuid_t)cpu->hvf_fd);
> +    g_free(env->hvf_mmio_buf);
>      assert_hvf_ok(ret);
>  }
>  
> @@ -563,6 +567,7 @@ int hvf_init_vcpu(CPUState *cpu)
>      init_decoder();
>  
>      hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
> +    env->hvf_mmio_buf = g_new(char, 4096);
>      env->hvf_emul = g_new0(HVFX86EmulatorState, 1);
>  
>      r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
> diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
> index 2363616c07..483fcea762 100644
> --- a/target/i386/hvf/x86.h
> +++ b/target/i386/hvf/x86.h
> @@ -230,7 +230,6 @@ typedef struct x68_segment_selector {
>  
>  /* Definition of hvf_x86_state is here */
>  struct HVFX86EmulatorState {
> -    uint8_t mmio_buf[4096];
>  };
>  
>  /* useful register access  macros */
> diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
> index 1ad2c30e16..d3e289ed87 100644
> --- a/target/i386/hvf/x86_emu.c
> +++ b/target/i386/hvf/x86_emu.c
> @@ -187,8 +187,8 @@ void write_val_ext(struct CPUX86State *env, target_ulong 
> ptr, target_ulong val,
>  
>  uint8_t *read_mmio(struct CPUX86State *env, target_ulong ptr, int bytes)
>  {
> -    vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, ptr, bytes);
> -    return env->hvf_emul->mmio_buf;
> +    vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, ptr, bytes);
> +    return env->hvf_mmio_buf;
>  }
>  
>  
> @@ -489,9 +489,9 @@ static void exec_ins_single(struct CPUX86State *env, 
> struct x86_decode *decode)
>      target_ulong addr = linear_addr_size(env_cpu(env), RDI(env),
>                                           decode->addressing_size, R_ES);
>  
> -    hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 0,
> +    hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 0,
>                    decode->operand_size, 1);
> -    vmx_write_mem(env_cpu(env), addr, env->hvf_emul->mmio_buf,
> +    vmx_write_mem(env_cpu(env), addr, env->hvf_mmio_buf,
>                    decode->operand_size);
>  
>      string_increment_reg(env, R_EDI, decode);
> @@ -512,9 +512,9 @@ static void exec_outs_single(struct CPUX86State *env, 
> struct x86_decode *decode)
>  {
>      target_ulong addr = decode_linear_addr(env, decode, RSI(env), R_DS);
>  
> -    vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, addr,
> +    vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, addr,
>                   decode->operand_size);
> -    hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 1,
> +    hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 1,
>                    decode->operand_size, 1);
>  
>      string_increment_reg(env, R_ESI, decode);
> 

It should be possible to get rid of the buffer altogether, but it's ok
to do it separately.

I queued the series, thanks.

Paolo