[v2] s390x cleanups and CPU hotplug via device_add

[Qemu-devel] [PATCH v2 07/19] s390x: move two function declarations to s390-virtio-ccw.h

Posted by David Hildenbrand 8 years, 2 months ago

We can also drop the extern. Fix up includes.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 include/hw/s390x/s390-virtio-ccw.h | 3 +++
 target/s390x/cpu.h                 | 2 --
 target/s390x/diag.c                | 1 +
 target/s390x/interrupt.c           | 3 +++
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index 41a9d2862b..42f2ccdb43 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -56,4 +56,7 @@ bool gs_allowed(void);
  */
 bool css_migration_enabled(void);
 
+S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
+void subsystem_reset(void);
+
 #endif
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 2343f2f1f7..6c0abb9894 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -720,8 +720,6 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
 
 
 /* outside of target/s390x/ */
-S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
-extern void subsystem_reset(void);
 int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
 
 #endif
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index e6b5e6de37..82a623948d 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -20,6 +20,7 @@
 #include "hw/watchdog/wdt_diag288.h"
 #include "sysemu/cpus.h"
 #include "hw/s390x/ipl.h"
+#include "hw/s390x/s390-virtio-ccw.h"
 
 static int modified_clear_reset(S390CPU *cpu)
 {
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index 058e219fe5..9aa6b89301 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -15,6 +15,9 @@
 #include "exec/exec-all.h"
 #include "sysemu/kvm.h"
 #include "hw/s390x/ioinst.h"
+#if !defined(CONFIG_USER_ONLY)
+#include "hw/s390x/s390-virtio-ccw.h"
+#endif
 
 /* Ensure to exit the TB after this call! */
 void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen)
-- 
2.13.5

Re: [Qemu-devel] [PATCH v2 07/19] s390x: move two function declarations to s390-virtio-ccw.h

Posted by Thomas Huth 8 years, 1 month ago

On 04.09.2017 17:43, David Hildenbrand wrote:
> We can also drop the extern. Fix up includes.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  include/hw/s390x/s390-virtio-ccw.h | 3 +++
>  target/s390x/cpu.h                 | 2 --
>  target/s390x/diag.c                | 1 +
>  target/s390x/interrupt.c           | 3 +++
>  4 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> index 41a9d2862b..42f2ccdb43 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -56,4 +56,7 @@ bool gs_allowed(void);
>   */
>  bool css_migration_enabled(void);
>  
> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
> +void subsystem_reset(void);
> +
>  #endif
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 2343f2f1f7..6c0abb9894 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -720,8 +720,6 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
>  
>  
>  /* outside of target/s390x/ */
> -S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
> -extern void subsystem_reset(void);
>  int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
>  
>  #endif
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index e6b5e6de37..82a623948d 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -20,6 +20,7 @@
>  #include "hw/watchdog/wdt_diag288.h"
>  #include "sysemu/cpus.h"
>  #include "hw/s390x/ipl.h"
> +#include "hw/s390x/s390-virtio-ccw.h"
>  
>  static int modified_clear_reset(S390CPU *cpu)
>  {
> diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
> index 058e219fe5..9aa6b89301 100644
> --- a/target/s390x/interrupt.c
> +++ b/target/s390x/interrupt.c
> @@ -15,6 +15,9 @@
>  #include "exec/exec-all.h"
>  #include "sysemu/kvm.h"
>  #include "hw/s390x/ioinst.h"
> +#if !defined(CONFIG_USER_ONLY)
> +#include "hw/s390x/s390-virtio-ccw.h"
> +#endif
>  
>  /* Ensure to exit the TB after this call! */
>  void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen)
> 

If I listen to my gut feeling, I still think we should rather move
s390_cpu_addr2state() to cpu.c instead. It does not sound as it's really
specific to the virtio-ccw machine... Just my 0.02 €

 Thomas

Re: [Qemu-devel] [PATCH v2 07/19] s390x: move two function declarations to s390-virtio-ccw.h

Posted by David Hildenbrand 8 years, 1 month ago

>> +++ b/target/s390x/interrupt.c
>> @@ -15,6 +15,9 @@
>>  #include "exec/exec-all.h"
>>  #include "sysemu/kvm.h"
>>  #include "hw/s390x/ioinst.h"
>> +#if !defined(CONFIG_USER_ONLY)
>> +#include "hw/s390x/s390-virtio-ccw.h"
>> +#endif
>>  
>>  /* Ensure to exit the TB after this call! */
>>  void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen)
>>
> 
> If I listen to my gut feeling, I still think we should rather move
> s390_cpu_addr2state() to cpu.c instead. It does not sound as it's really
> specific to the virtio-ccw machine... Just my 0.02 €
> 

I prefer to leave it as is for now, so just a header cleanup. E.g. with
patch 18 we could implement it directly using ms->possible_cpus.

>  Thomas


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v2 07/19] s390x: move two function declarations to s390-virtio-ccw.h

Posted by Cornelia Huck 8 years, 1 month ago

On Thu, 7 Sep 2017 14:46:44 +0200
David Hildenbrand <david@redhat.com> wrote:

> >> +++ b/target/s390x/interrupt.c
> >> @@ -15,6 +15,9 @@
> >>  #include "exec/exec-all.h"
> >>  #include "sysemu/kvm.h"
> >>  #include "hw/s390x/ioinst.h"
> >> +#if !defined(CONFIG_USER_ONLY)
> >> +#include "hw/s390x/s390-virtio-ccw.h"
> >> +#endif
> >>  
> >>  /* Ensure to exit the TB after this call! */
> >>  void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen)
> >>  
> > 
> > If I listen to my gut feeling, I still think we should rather move
> > s390_cpu_addr2state() to cpu.c instead. It does not sound as it's really
> > specific to the virtio-ccw machine... Just my 0.02 €
> >   
> 
> I prefer to leave it as is for now, so just a header cleanup. E.g. with
> patch 18 we could implement it directly using ms->possible_cpus.

If you do that, would you also get rid of the conditional include above?