On Thu, Jun 11, 2020 at 03:24:31PM +0200, Claudio Fontana wrote:
> On 6/4/20 8:27 PM, Paolo Bonzini wrote:
> > 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.bolsha...@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
> >
> >
>
> Thanks Paolo, I am waiting for this (and maybe another series from Roman) to
> be able to do the cpus refactoring.
>
> Incidentally, would it not be great to have some machinery that automatically
> tracks which series is already queued where?
> Maybe there is already a mechanism I am unaware of?
>
> Ciao,
>
> Claudio
Hi Claudio,
I also had the same question earlier today on IRC but I've just recalled
that PULL requests typically have a reference to the queue repo/branch:
https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg06825.html
I'll rebase on it and prepare the series.
Regards,
Roman
