| Message ID | 20190407115839.25054-3-skrtbhtngr@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Move hvf_cpu_synchronize* calls to cpu_synchronize* functions | expand |
On Sun, Apr 07, 2019 at 05:28:39PM +0530, Sukrit Bhatnagar wrote: > Keep the calls made to synchronize cpu by all hypervisors in one place > inside cpu_synchronize_* functions in include/sysemu/hw_accel.h > > Cc: Richard Henderson <rth@twiddle.net> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> > --- > cpus.c | 12 ------------ > include/sysemu/hw_accel.h | 10 ++++++++++ > 2 files changed, 10 insertions(+), 12 deletions(-) > > diff --git a/cpus.c b/cpus.c > index e83f72b48b..026df0dc5f 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -1021,10 +1021,6 @@ void cpu_synchronize_all_states(void) > > CPU_FOREACH(cpu) { > cpu_synchronize_state(cpu); > - /* TODO: move to cpu_synchronize_state() */ > - if (hvf_enabled()) { > - hvf_cpu_synchronize_state(cpu); > - } > } > } > > @@ -1034,10 +1030,6 @@ void cpu_synchronize_all_post_reset(void) > > CPU_FOREACH(cpu) { > cpu_synchronize_post_reset(cpu); > - /* TODO: move to cpu_synchronize_post_reset() */ > - if (hvf_enabled()) { > - hvf_cpu_synchronize_post_reset(cpu); > - } > } > } > > @@ -1047,10 +1039,6 @@ void cpu_synchronize_all_post_init(void) > > CPU_FOREACH(cpu) { > cpu_synchronize_post_init(cpu); > - /* TODO: move to cpu_synchronize_post_init() */ > - if (hvf_enabled()) { > - hvf_cpu_synchronize_post_init(cpu); > - } > } > } > > diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h > index d2ddfb5ad0..bf081b4026 100644 > --- a/include/sysemu/hw_accel.h > +++ b/include/sysemu/hw_accel.h > @@ -15,6 +15,7 @@ > #include "sysemu/hax.h" > #include "sysemu/kvm.h" > #include "sysemu/whpx.h" > +#include "sysemu/hvf.h" > > static inline void cpu_synchronize_state(CPUState *cpu) > { > @@ -27,6 +28,9 @@ static inline void cpu_synchronize_state(CPUState *cpu) > if (whpx_enabled()) { > whpx_cpu_synchronize_state(cpu); > } > + if (hvf_enabled()) { > + hvf_cpu_synchronize_state(cpu); > + } > } > > static inline void cpu_synchronize_post_reset(CPUState *cpu) > @@ -40,6 +44,9 @@ static inline void cpu_synchronize_post_reset(CPUState *cpu) > if (whpx_enabled()) { > whpx_cpu_synchronize_post_reset(cpu); > } > + if (hvf_enabled()) { > + hvf_cpu_synchronize_post_reset(cpu); > + } > } > > static inline void cpu_synchronize_post_init(CPUState *cpu) > @@ -53,6 +60,9 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) > if (whpx_enabled()) { > whpx_cpu_synchronize_post_init(cpu); > } > + if (hvf_enabled()) { > + hvf_cpu_synchronize_post_init(cpu); > + } > } > > static inline void cpu_synchronize_pre_loadvm(CPUState *cpu) > -- > 2.20.1 > > Hi Sukrit, The commit itself looks sane but qemu starts to hang during BIOS boot after I've applied it if I run it with hvf accel. Thanks, Roman
On Wed, 10 Apr 2019 at 17:20, Roman Bolshakov <r.bolshakov@yadro.com> wrote: > > On Sun, Apr 07, 2019 at 05:28:39PM +0530, Sukrit Bhatnagar wrote: > > Keep the calls made to synchronize cpu by all hypervisors in one place > > inside cpu_synchronize_* functions in include/sysemu/hw_accel.h > > > > Cc: Richard Henderson <rth@twiddle.net> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> > > --- > > cpus.c | 12 ------------ > > include/sysemu/hw_accel.h | 10 ++++++++++ > > 2 files changed, 10 insertions(+), 12 deletions(-) > > > > diff --git a/cpus.c b/cpus.c > > index e83f72b48b..026df0dc5f 100644 > > --- a/cpus.c > > +++ b/cpus.c > > @@ -1021,10 +1021,6 @@ void cpu_synchronize_all_states(void) > > > > CPU_FOREACH(cpu) { > > cpu_synchronize_state(cpu); > > - /* TODO: move to cpu_synchronize_state() */ > > - if (hvf_enabled()) { > > - hvf_cpu_synchronize_state(cpu); > > - } > > } > > } > > > > @@ -1034,10 +1030,6 @@ void cpu_synchronize_all_post_reset(void) > > > > CPU_FOREACH(cpu) { > > cpu_synchronize_post_reset(cpu); > > - /* TODO: move to cpu_synchronize_post_reset() */ > > - if (hvf_enabled()) { > > - hvf_cpu_synchronize_post_reset(cpu); > > - } > > } > > } > > > > @@ -1047,10 +1039,6 @@ void cpu_synchronize_all_post_init(void) > > > > CPU_FOREACH(cpu) { > > cpu_synchronize_post_init(cpu); > > - /* TODO: move to cpu_synchronize_post_init() */ > > - if (hvf_enabled()) { > > - hvf_cpu_synchronize_post_init(cpu); > > - } > > } > > } > > > > diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h > > index d2ddfb5ad0..bf081b4026 100644 > > --- a/include/sysemu/hw_accel.h > > +++ b/include/sysemu/hw_accel.h > > @@ -15,6 +15,7 @@ > > #include "sysemu/hax.h" > > #include "sysemu/kvm.h" > > #include "sysemu/whpx.h" > > +#include "sysemu/hvf.h" > > > > static inline void cpu_synchronize_state(CPUState *cpu) > > { > > @@ -27,6 +28,9 @@ static inline void cpu_synchronize_state(CPUState *cpu) > > if (whpx_enabled()) { > > whpx_cpu_synchronize_state(cpu); > > } > > + if (hvf_enabled()) { > > + hvf_cpu_synchronize_state(cpu); > > + } > > } > > > > static inline void cpu_synchronize_post_reset(CPUState *cpu) > > @@ -40,6 +44,9 @@ static inline void cpu_synchronize_post_reset(CPUState *cpu) > > if (whpx_enabled()) { > > whpx_cpu_synchronize_post_reset(cpu); > > } > > + if (hvf_enabled()) { > > + hvf_cpu_synchronize_post_reset(cpu); > > + } > > } > > > > static inline void cpu_synchronize_post_init(CPUState *cpu) > > @@ -53,6 +60,9 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) > > if (whpx_enabled()) { > > whpx_cpu_synchronize_post_init(cpu); > > } > > + if (hvf_enabled()) { > > + hvf_cpu_synchronize_post_init(cpu); > > + } > > } > > > > static inline void cpu_synchronize_pre_loadvm(CPUState *cpu) > > -- > > 2.20.1 > > > > > > Hi Sukrit, > > The commit itself looks sane but qemu starts to hang during BIOS boot > after I've applied it if I run it with hvf accel. Hi Roman, Thanks for reviewing the patches. If this is happening, then I assume patches are not ready to be merged into qemu-stable. Do you have any idea why the problem is not detected during tests, but rather at runtime? I am not really sure if this patch (2/2) has anything to do with the problem but the previous patch (1/2) might. > Thanks, > Roman
On Wed, Apr 10, 2019 at 05:35:23PM +0530, Sukrit Bhatnagar wrote: > On Wed, 10 Apr 2019 at 17:20, Roman Bolshakov <r.bolshakov@yadro.com> wrote: > > > > On Sun, Apr 07, 2019 at 05:28:39PM +0530, Sukrit Bhatnagar wrote: > > > Keep the calls made to synchronize cpu by all hypervisors in one place > > > inside cpu_synchronize_* functions in include/sysemu/hw_accel.h > > > > > > > The commit itself looks sane but qemu starts to hang during BIOS boot > > after I've applied it if I run it with hvf accel. > > Hi Roman, > Thanks for reviewing the patches. > If this is happening, then I assume patches are not ready > to be merged into qemu-stable. > Do you have any idea why the problem is not detected > during tests, but rather at runtime? > I am not really sure if this patch (2/2) has anything to do with > the problem but the previous patch (1/2) might. > I've applied, built and tested both sequentially. Applying and running with patch 1/2 alone doesn't result in the behavior I mentioned. I also tried to apply only the first hunk that moves hvf_cpu_synchronize_state into cpu_synchronize_state and it also causes the issue with BIOS. Honestly, I don't know if the tests run qemu with hvf accel. AFAIK kvm-unit-tests can be used to test an accel itself. Thanks, Roman
On 10/04/19 16:02, Roman Bolshakov wrote: > I've applied, built and tested both sequentially. Applying and running > with patch 1/2 alone doesn't result in the behavior I mentioned. I also > tried to apply only the first hunk that moves hvf_cpu_synchronize_state > into cpu_synchronize_state and it also causes the issue with BIOS. > > Honestly, I don't know if the tests run qemu with hvf accel. AFAIK > kvm-unit-tests can be used to test an accel itself. No, tests do not cover HVF. Paolo
diff --git a/cpus.c b/cpus.c index e83f72b48b..026df0dc5f 100644 --- a/cpus.c +++ b/cpus.c @@ -1021,10 +1021,6 @@ void cpu_synchronize_all_states(void) CPU_FOREACH(cpu) { cpu_synchronize_state(cpu); - /* TODO: move to cpu_synchronize_state() */ - if (hvf_enabled()) { - hvf_cpu_synchronize_state(cpu); - } } } @@ -1034,10 +1030,6 @@ void cpu_synchronize_all_post_reset(void) CPU_FOREACH(cpu) { cpu_synchronize_post_reset(cpu); - /* TODO: move to cpu_synchronize_post_reset() */ - if (hvf_enabled()) { - hvf_cpu_synchronize_post_reset(cpu); - } } } @@ -1047,10 +1039,6 @@ void cpu_synchronize_all_post_init(void) CPU_FOREACH(cpu) { cpu_synchronize_post_init(cpu); - /* TODO: move to cpu_synchronize_post_init() */ - if (hvf_enabled()) { - hvf_cpu_synchronize_post_init(cpu); - } } } diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h index d2ddfb5ad0..bf081b4026 100644 --- a/include/sysemu/hw_accel.h +++ b/include/sysemu/hw_accel.h @@ -15,6 +15,7 @@ #include "sysemu/hax.h" #include "sysemu/kvm.h" #include "sysemu/whpx.h" +#include "sysemu/hvf.h" static inline void cpu_synchronize_state(CPUState *cpu) { @@ -27,6 +28,9 @@ static inline void cpu_synchronize_state(CPUState *cpu) if (whpx_enabled()) { whpx_cpu_synchronize_state(cpu); } + if (hvf_enabled()) { + hvf_cpu_synchronize_state(cpu); + } } static inline void cpu_synchronize_post_reset(CPUState *cpu) @@ -40,6 +44,9 @@ static inline void cpu_synchronize_post_reset(CPUState *cpu) if (whpx_enabled()) { whpx_cpu_synchronize_post_reset(cpu); } + if (hvf_enabled()) { + hvf_cpu_synchronize_post_reset(cpu); + } } static inline void cpu_synchronize_post_init(CPUState *cpu) @@ -53,6 +60,9 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) if (whpx_enabled()) { whpx_cpu_synchronize_post_init(cpu); } + if (hvf_enabled()) { + hvf_cpu_synchronize_post_init(cpu); + } } static inline void cpu_synchronize_pre_loadvm(CPUState *cpu)
Keep the calls made to synchronize cpu by all hypervisors in one place inside cpu_synchronize_* functions in include/sysemu/hw_accel.h Cc: Richard Henderson <rth@twiddle.net> Cc: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- cpus.c | 12 ------------ include/sysemu/hw_accel.h | 10 ++++++++++ 2 files changed, 10 insertions(+), 12 deletions(-)