Re: [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu

[Roman Bolshakov]

On Thu, Jun 04, 2020 at 11:53:53AM +0200, Claudio Fontana wrote:
> On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> > They're either declared elsewhere or have no use.
> > 
> > While at it, rename _hvf_cpu_synchronize_post_init() to
> > do_hvf_cpu_synchronize_post_init().
> > 
> > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> > ---
> >  include/sysemu/hvf.h  | 22 ----------------------
> >  target/i386/hvf/hvf.c |  7 ++++---
> >  2 files changed, 4 insertions(+), 25 deletions(-)
> > 
> > diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
> > index 30a565ab73..03f3cd7db3 100644
> > --- a/include/sysemu/hvf.h
> > +++ b/include/sysemu/hvf.h
> > @@ -30,35 +30,13 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, 
> > uint32_t idx,
> >  #define hvf_get_supported_cpuid(func, idx, reg) 0
> >  #endif
> >  
> > -/* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported 
> > by
> > - * the host CPU. Use hvf_enabled() after this to get the result. */
> > -void hvf_disable(int disable);
> > -
> > -/* Returns non-0 if the host CPU supports the VMX "unrestricted guest" 
> > feature
> > - * which allows the virtual CPU to directly run in "real mode". If true, 
> > this
> > - * allows QEMU to run several vCPU threads in parallel (see cpus.c). 
> > Otherwise,
> > - * only a a single TCG thread can run, and it will call HVF to run the 
> > current
> > - * instructions, except in case of "real mode" (paging disabled, typically 
> > at
> > - * boot time), or MMIO operations. */
> > -
> > -int hvf_sync_vcpus(void);
> > -
> >  int hvf_init_vcpu(CPUState *);
> >  int hvf_vcpu_exec(CPUState *);
> > -int hvf_smp_cpu_exec(CPUState *);
> >  void hvf_cpu_synchronize_state(CPUState *);
> >  void hvf_cpu_synchronize_post_reset(CPUState *);
> >  void hvf_cpu_synchronize_post_init(CPUState *);
> > -void _hvf_cpu_synchronize_post_init(CPUState *, run_on_cpu_data);
> > -
> >  void hvf_vcpu_destroy(CPUState *);
> > -void hvf_raise_event(CPUState *);
> > -/* void hvf_reset_vcpu_state(void *opaque); */
> >  void hvf_reset_vcpu(CPUState *);
> > -void vmx_update_tpr(CPUState *);
> > -void update_apic_tpr(CPUState *);
> > -int hvf_put_registers(CPUState *);
> > -void vmx_clear_int_window_exiting(CPUState *cpu);
> >  
> >  #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
> >  
> > diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> > index d72543dc31..9ccdb7e7c7 100644
> > --- a/target/i386/hvf/hvf.c
> > +++ b/target/i386/hvf/hvf.c
> > @@ -251,7 +251,7 @@ void vmx_update_tpr(CPUState *cpu)
> >      }
> >  }
> >  
> > -void update_apic_tpr(CPUState *cpu)
> > +static void update_apic_tpr(CPUState *cpu)
> >  {
> >      X86CPU *x86_cpu = X86_CPU(cpu);
> >      int tpr = rreg(cpu->hvf_fd, HV_X86_TPR) >> 4;
> > @@ -312,7 +312,8 @@ void hvf_cpu_synchronize_post_reset(CPUState *cpu_state)
> >      run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_reset, 
> > RUN_ON_CPU_NULL);
> >  }
> >  
> > -void _hvf_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
> > +static void do_hvf_cpu_synchronize_post_init(CPUState *cpu,
> > +                                             run_on_cpu_data arg)
> >  {
> >      CPUState *cpu_state = cpu;
> >      hvf_put_registers(cpu_state);
> > @@ -321,7 +322,7 @@ void _hvf_cpu_synchronize_post_init(CPUState *cpu, 
> > run_on_cpu_data arg)
> >  
> >  void hvf_cpu_synchronize_post_init(CPUState *cpu_state)
> >  {
> > -    run_on_cpu(cpu_state, _hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
> > +    run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_init, 
> > RUN_ON_CPU_NULL);
> >  }
> >  
> >  static bool ept_emulation_fault(hvf_slot *slot, uint64_t gpa, uint64_t 
> > ept_qual)
> > 
> 
> in this file (hvf.c) there is a comment:
> 
> /* TODO: synchronize vcpu state */
> 
> is the TODO still valid after this change? Or should the TODO be eliminated?
> 

Hi Claudio,

Yeah, it's still valid. There will be another series to have only one
function where emulator state is synchronized.

Thanks,
Roman