[Qemu-devel] [PATCH RESEND v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Qemu-devel] [PATCH RESEND v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Sukrit Bhatnagar]

Changes made in v2:
- added a patch to declare hvf_handle_io only if NEED_CPU_H
  is defined so that the poisoned type CPUArchState does not
  produce an error when include/sysemu/hvf.h is included for
  common object compilation

Link to v1:
https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06902.html

Sukrit Bhatnagar (2):
  hvf: declare hvf_handle_io if NEED_CPU_H is defined
  cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

 cpus.c                    | 12 ------------
 include/sysemu/hvf.h      |  4 ++++
 include/sysemu/hw_accel.h | 10 ++++++++++
 3 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH RESEND v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Sukrit Bhatnagar]

Changes made in v2:
- added a patch to declare hvf_handle_io only if NEED_CPU_H
  is defined so that the poisoned type CPUArchState does not
  produce an error when include/sysemu/hvf.h is included for
  common object compilation

Link to v1:
https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06902.html

Sukrit Bhatnagar (2):
  hvf: declare hvf_handle_io if NEED_CPU_H is defined
  cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

 cpus.c                    | 12 ------------
 include/sysemu/hvf.h      |  4 ++++
 include/sysemu/hw_accel.h | 10 ++++++++++
 3 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH RESEND v2 1/2] hvf: declare hvf_handle_io if NEED_CPU_H is defined

[Sukrit Bhatnagar]

hvf_handle_io needs the poisoned type CPUArchState as its argument.
Declaring it if NEED_CPU_H is defined enables include/sysemu/hvf.h
to be included for common object compilation as well.

Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
---
 include/sysemu/hvf.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index aaa51d2c51..7eca3ec7be 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -63,8 +63,12 @@ typedef struct HVFState {
 extern HVFState *hvf_state;
 
 void hvf_set_phys_mem(MemoryRegionSection *, bool);
+
+#ifdef NEED_CPU_H
 void hvf_handle_io(CPUArchState *, uint16_t, void *,
                   int, int, int);
+#endif
+
 hvf_slot *hvf_find_overlap_slot(uint64_t, uint64_t);
 
 /* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
-- 
2.20.1

[Qemu-devel] [PATCH RESEND v2 1/2] hvf: declare hvf_handle_io if NEED_CPU_H is defined

[Sukrit Bhatnagar]

hvf_handle_io needs the poisoned type CPUArchState as its argument.
Declaring it if NEED_CPU_H is defined enables include/sysemu/hvf.h
to be included for common object compilation as well.

Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
---
 include/sysemu/hvf.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index aaa51d2c51..7eca3ec7be 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -63,8 +63,12 @@ typedef struct HVFState {
 extern HVFState *hvf_state;
 
 void hvf_set_phys_mem(MemoryRegionSection *, bool);
+
+#ifdef NEED_CPU_H
 void hvf_handle_io(CPUArchState *, uint16_t, void *,
                   int, int, int);
+#endif
+
 hvf_slot *hvf_find_overlap_slot(uint64_t, uint64_t);
 
 /* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
-- 
2.20.1

[Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Sukrit Bhatnagar]

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

[Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Sukrit Bhatnagar]

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

Re: [Qemu-devel] [PATCH RESEND v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Sukrit Bhatnagar]

Hi,
Are the patches in good enough shape to be merged?

Thanks,
Sukrit

Re: [Qemu-devel] [PATCH RESEND v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Sukrit Bhatnagar]

Hi,
Are the patches in good enough shape to be merged?

Thanks,
Sukrit

Re: [Qemu-devel] [PATCH RESEND v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Marcel Apfelbaum]

On 4/7/19 3:05 PM, Sukrit Bhatnagar wrote:
> Hi,
> Are the patches in good enough shape to be merged?

Hi Sukrit,

Where is the patch itself :) ?

Also be sure to add the maintainers to cc.
You can get them by running ./scripts/get_maintainers.pl script on your 
patch.

Please re-send it.

Thanks,
Marcel

> Thanks,
> Sukrit

Re: [Qemu-devel] [PATCH RESEND v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Marcel Apfelbaum]

On 4/7/19 3:05 PM, Sukrit Bhatnagar wrote:
> Hi,
> Are the patches in good enough shape to be merged?

Hi Sukrit,

Where is the patch itself :) ?

Also be sure to add the maintainers to cc.
You can get them by running ./scripts/get_maintainers.pl script on your 
patch.

Please re-send it.

Thanks,
Marcel

> Thanks,
> Sukrit

Re: [Qemu-devel] [PATCH RESEND v2 1/2] hvf: declare hvf_handle_io if NEED_CPU_H is defined

[Roman Bolshakov]

On Sun, Apr 07, 2019 at 05:28:38PM +0530, Sukrit Bhatnagar wrote:
> hvf_handle_io needs the poisoned type CPUArchState as its argument.
> Declaring it if NEED_CPU_H is defined enables include/sysemu/hvf.h
> to be included for common object compilation as well.
> 

Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>

Thanks,
Roman

Re: [Qemu-devel] [PATCH RESEND v2 1/2] hvf: declare hvf_handle_io if NEED_CPU_H is defined

[Roman Bolshakov]

On Sun, Apr 07, 2019 at 05:28:38PM +0530, Sukrit Bhatnagar wrote:
> hvf_handle_io needs the poisoned type CPUArchState as its argument.
> Declaring it if NEED_CPU_H is defined enables include/sysemu/hvf.h
> to be included for common object compilation as well.
> 

Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>

Thanks,
Roman

Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Roman Bolshakov]

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

Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Roman Bolshakov]

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

Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Sukrit Bhatnagar]

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

Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Sukrit Bhatnagar]

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

Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Roman Bolshakov]

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

Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Roman Bolshakov]

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

Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

[Paolo Bonzini]

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

[Paolo Bonzini]

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