Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions
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
Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions
On Wed, Apr 10, 2019 at 05:35:23PM +0530, Sukrit Bhatnagar wrote: > On Wed, 10 Apr 2019 at 17:20, Roman Bolshakov 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
Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions
On Wed, 10 Apr 2019 at 17:20, Roman Bolshakov 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
> > Cc: Paolo Bonzini
> > Signed-off-by: Sukrit Bhatnagar
> > ---
> > 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
Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions
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
> Cc: Paolo Bonzini
> Signed-off-by: Sukrit Bhatnagar
> ---
> 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
