* [PATCH v3 1/5] s390x: Don't do a normal reset on the initial cpu
2019-11-25 9:03 [PATCH v3 0/5] s390x: Reset cleanup Janosch Frank
@ 2019-11-25 9:03 ` Janosch Frank
2019-11-25 13:11 ` Cornelia Huck
2019-11-25 9:03 ` [PATCH v3 2/5] s390x: Move reset normal to shared reset handler Janosch Frank
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-11-25 9:03 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, pmorel, david, cohuck, borntraeger, qemu-s390x, mihajlov
The initiating cpu needs to be reset with an initial reset. While
doing a normal reset followed by a initial reset is not wrong per se,
the Ultravisor will only allow the correct reset to be performed.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
hw/s390x/s390-virtio-ccw.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index d3edeef0ad..c1d1440272 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -348,6 +348,9 @@ static void s390_machine_reset(MachineState *machine)
break;
case S390_RESET_LOAD_NORMAL:
CPU_FOREACH(t) {
+ if (t == cs) {
+ continue;
+ }
run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
}
subsystem_reset();
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 1/5] s390x: Don't do a normal reset on the initial cpu
2019-11-25 9:03 ` [PATCH v3 1/5] s390x: Don't do a normal reset on the initial cpu Janosch Frank
@ 2019-11-25 13:11 ` Cornelia Huck
0 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-11-25 13:11 UTC (permalink / raw)
To: Janosch Frank
Cc: thuth, pmorel, david, qemu-devel, borntraeger, qemu-s390x,
mihajlov
On Mon, 25 Nov 2019 04:03:44 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:
> The initiating cpu needs to be reset with an initial reset. While
> doing a normal reset followed by a initial reset is not wrong per se,
> the Ultravisor will only allow the correct reset to be performed.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> ---
> hw/s390x/s390-virtio-ccw.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index d3edeef0ad..c1d1440272 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -348,6 +348,9 @@ static void s390_machine_reset(MachineState *machine)
> break;
> case S390_RESET_LOAD_NORMAL:
> CPU_FOREACH(t) {
> + if (t == cs) {
> + continue;
> + }
> run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
> }
> subsystem_reset();
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/5] s390x: Move reset normal to shared reset handler
2019-11-25 9:03 [PATCH v3 0/5] s390x: Reset cleanup Janosch Frank
2019-11-25 9:03 ` [PATCH v3 1/5] s390x: Don't do a normal reset on the initial cpu Janosch Frank
@ 2019-11-25 9:03 ` Janosch Frank
2019-11-25 13:19 ` Cornelia Huck
2019-11-25 9:03 ` [PATCH v3 3/5] s390x: Move initial reset Janosch Frank
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-11-25 9:03 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, pmorel, david, cohuck, borntraeger, qemu-s390x, mihajlov
Let's start moving the cpu reset functions into a single function with
a switch/case, so we can use fallthroughs and share more code between
resets.
This patch introduces the reset function by renaming cpu_reset() and
cleaning up leftovers.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
target/s390x/cpu-qom.h | 6 +++++-
target/s390x/cpu.c | 16 ++++++++++------
target/s390x/cpu.h | 2 +-
target/s390x/sigp.c | 2 +-
4 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/target/s390x/cpu-qom.h b/target/s390x/cpu-qom.h
index b809ec8418..f3b71bac67 100644
--- a/target/s390x/cpu-qom.h
+++ b/target/s390x/cpu-qom.h
@@ -34,6 +34,10 @@
typedef struct S390CPUModel S390CPUModel;
typedef struct S390CPUDef S390CPUDef;
+typedef enum cpu_reset_type {
+ S390_CPU_RESET_NORMAL,
+} cpu_reset_type;
+
/**
* S390CPUClass:
* @parent_realize: The parent class' realize handler.
@@ -57,7 +61,7 @@ typedef struct S390CPUClass {
DeviceRealize parent_realize;
void (*parent_reset)(CPUState *cpu);
void (*load_normal)(CPUState *cpu);
- void (*cpu_reset)(CPUState *cpu);
+ void (*reset)(CPUState *cpu, cpu_reset_type type);
void (*initial_cpu_reset)(CPUState *cpu);
} S390CPUClass;
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 3abe7e80fd..8d35a2a723 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -82,18 +82,22 @@ static void s390_cpu_load_normal(CPUState *s)
}
#endif
-/* S390CPUClass::cpu_reset() */
-static void s390_cpu_reset(CPUState *s)
+/* S390CPUClass::reset() */
+static void s390_cpu_reset(CPUState *s, cpu_reset_type type)
{
S390CPU *cpu = S390_CPU(s);
S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
CPUS390XState *env = &cpu->env;
- env->pfault_token = -1UL;
- env->bpbc = false;
scc->parent_reset(s);
cpu->env.sigp_order = 0;
s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
+
+ switch (type) {
+ case S390_CPU_RESET_NORMAL:
+ env->pfault_token = -1UL;
+ env->bpbc = false;
+ }
}
/* S390CPUClass::initial_reset() */
@@ -102,7 +106,7 @@ static void s390_cpu_initial_reset(CPUState *s)
S390CPU *cpu = S390_CPU(s);
CPUS390XState *env = &cpu->env;
- s390_cpu_reset(s);
+ s390_cpu_reset(s, S390_CPU_RESET_NORMAL);
/* initial reset does not clear everything! */
memset(&env->start_initial_reset_fields, 0,
offsetof(CPUS390XState, end_reset_fields) -
@@ -473,7 +477,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
#if !defined(CONFIG_USER_ONLY)
scc->load_normal = s390_cpu_load_normal;
#endif
- scc->cpu_reset = s390_cpu_reset;
+ scc->reset = s390_cpu_reset;
scc->initial_cpu_reset = s390_cpu_initial_reset;
cc->reset = s390_cpu_full_reset;
cc->class_by_name = s390_cpu_class_by_name,
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 17460ed7b3..18123dfd5b 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -741,7 +741,7 @@ static inline void s390_do_cpu_reset(CPUState *cs, run_on_cpu_data arg)
{
S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
- scc->cpu_reset(cs);
+ scc->reset(cs, S390_CPU_RESET_NORMAL);
}
static inline void s390_do_cpu_initial_reset(CPUState *cs, run_on_cpu_data arg)
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index 2ce22d4dc1..850139b9cd 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -266,7 +266,7 @@ static void sigp_cpu_reset(CPUState *cs, run_on_cpu_data arg)
SigpInfo *si = arg.host_ptr;
cpu_synchronize_state(cs);
- scc->cpu_reset(cs);
+ scc->reset(cs, S390_CPU_RESET_NORMAL);
cpu_synchronize_post_reset(cs);
si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 2/5] s390x: Move reset normal to shared reset handler
2019-11-25 9:03 ` [PATCH v3 2/5] s390x: Move reset normal to shared reset handler Janosch Frank
@ 2019-11-25 13:19 ` Cornelia Huck
2019-11-27 17:45 ` David Hildenbrand
0 siblings, 1 reply; 12+ messages in thread
From: Cornelia Huck @ 2019-11-25 13:19 UTC (permalink / raw)
To: Janosch Frank
Cc: thuth, pmorel, david, qemu-devel, borntraeger, qemu-s390x,
mihajlov
On Mon, 25 Nov 2019 04:03:45 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:
> Let's start moving the cpu reset functions into a single function with
> a switch/case, so we can use fallthroughs and share more code between
> resets.
>
> This patch introduces the reset function by renaming cpu_reset() and
> cleaning up leftovers.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> ---
> target/s390x/cpu-qom.h | 6 +++++-
> target/s390x/cpu.c | 16 ++++++++++------
> target/s390x/cpu.h | 2 +-
> target/s390x/sigp.c | 2 +-
> 4 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 3abe7e80fd..8d35a2a723 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -82,18 +82,22 @@ static void s390_cpu_load_normal(CPUState *s)
> }
> #endif
>
> -/* S390CPUClass::cpu_reset() */
> -static void s390_cpu_reset(CPUState *s)
> +/* S390CPUClass::reset() */
> +static void s390_cpu_reset(CPUState *s, cpu_reset_type type)
> {
> S390CPU *cpu = S390_CPU(s);
> S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
> CPUS390XState *env = &cpu->env;
>
> - env->pfault_token = -1UL;
> - env->bpbc = false;
> scc->parent_reset(s);
> cpu->env.sigp_order = 0;
> s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
> +
> + switch (type) {
> + case S390_CPU_RESET_NORMAL:
> + env->pfault_token = -1UL;
> + env->bpbc = false;
I'm wondering whether we want a default case here to catch programming
errors.
> + }
> }
>
> /* S390CPUClass::initial_reset() */
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 2/5] s390x: Move reset normal to shared reset handler
2019-11-25 13:19 ` Cornelia Huck
@ 2019-11-27 17:45 ` David Hildenbrand
0 siblings, 0 replies; 12+ messages in thread
From: David Hildenbrand @ 2019-11-27 17:45 UTC (permalink / raw)
To: Cornelia Huck, Janosch Frank
Cc: thuth, pmorel, qemu-devel, borntraeger, qemu-s390x, mihajlov
On 25.11.19 14:19, Cornelia Huck wrote:
> On Mon, 25 Nov 2019 04:03:45 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
>
>> Let's start moving the cpu reset functions into a single function with
>> a switch/case, so we can use fallthroughs and share more code between
>> resets.
>>
>> This patch introduces the reset function by renaming cpu_reset() and
>> cleaning up leftovers.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> ---
>> target/s390x/cpu-qom.h | 6 +++++-
>> target/s390x/cpu.c | 16 ++++++++++------
>> target/s390x/cpu.h | 2 +-
>> target/s390x/sigp.c | 2 +-
>> 4 files changed, 17 insertions(+), 9 deletions(-)
>>
>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index 3abe7e80fd..8d35a2a723 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -82,18 +82,22 @@ static void s390_cpu_load_normal(CPUState *s)
>> }
>> #endif
>>
>> -/* S390CPUClass::cpu_reset() */
>> -static void s390_cpu_reset(CPUState *s)
>> +/* S390CPUClass::reset() */
>> +static void s390_cpu_reset(CPUState *s, cpu_reset_type type)
>> {
>> S390CPU *cpu = S390_CPU(s);
>> S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
>> CPUS390XState *env = &cpu->env;
>>
>> - env->pfault_token = -1UL;
>> - env->bpbc = false;
>> scc->parent_reset(s);
>> cpu->env.sigp_order = 0;
>> s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
>> +
>> + switch (type) {
>> + case S390_CPU_RESET_NORMAL:
>> + env->pfault_token = -1UL;
>> + env->bpbc = false;
>
> I'm wondering whether we want a default case here to catch programming
> errors.
+1, g_assert_not_reached().
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/5] s390x: Move initial reset
2019-11-25 9:03 [PATCH v3 0/5] s390x: Reset cleanup Janosch Frank
2019-11-25 9:03 ` [PATCH v3 1/5] s390x: Don't do a normal reset on the initial cpu Janosch Frank
2019-11-25 9:03 ` [PATCH v3 2/5] s390x: Move reset normal to shared reset handler Janosch Frank
@ 2019-11-25 9:03 ` Janosch Frank
2019-11-25 9:03 ` [PATCH v3 4/5] s390x: Move clear reset Janosch Frank
2019-11-25 9:03 ` [PATCH v3 5/5] s390x: Beautify diag308 handling Janosch Frank
4 siblings, 0 replies; 12+ messages in thread
From: Janosch Frank @ 2019-11-25 9:03 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, pmorel, david, cohuck, borntraeger, qemu-s390x, mihajlov
Let's move the intial reset into the reset handler and cleanup
afterwards.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
target/s390x/cpu-qom.h | 2 +-
target/s390x/cpu.c | 44 ++++++++++++++++--------------------------
target/s390x/cpu.h | 2 +-
target/s390x/sigp.c | 2 +-
4 files changed, 20 insertions(+), 30 deletions(-)
diff --git a/target/s390x/cpu-qom.h b/target/s390x/cpu-qom.h
index f3b71bac67..6f0a12042e 100644
--- a/target/s390x/cpu-qom.h
+++ b/target/s390x/cpu-qom.h
@@ -36,6 +36,7 @@ typedef struct S390CPUDef S390CPUDef;
typedef enum cpu_reset_type {
S390_CPU_RESET_NORMAL,
+ S390_CPU_RESET_INITIAL,
} cpu_reset_type;
/**
@@ -62,7 +63,6 @@ typedef struct S390CPUClass {
void (*parent_reset)(CPUState *cpu);
void (*load_normal)(CPUState *cpu);
void (*reset)(CPUState *cpu, cpu_reset_type type);
- void (*initial_cpu_reset)(CPUState *cpu);
} S390CPUClass;
typedef struct S390CPU S390CPU;
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 8d35a2a723..9f009b226c 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -94,36 +94,27 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type type)
s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
switch (type) {
+ case S390_CPU_RESET_INITIAL:
+ /* initial reset does not clear everything! */
+ memset(&env->start_initial_reset_fields, 0,
+ offsetof(CPUS390XState, end_reset_fields) -
+ offsetof(CPUS390XState, start_initial_reset_fields));
+
+ /* architectured initial value for Breaking-Event-Address register */
+ env->gbea = 1;
+
+ /* architectured initial values for CR 0 and 14 */
+ env->cregs[0] = CR0_RESET;
+ env->cregs[14] = CR14_RESET;
+
+ /* tininess for underflow is detected before rounding */
+ set_float_detect_tininess(float_tininess_before_rounding,
+ &env->fpu_status);
+ /* fall through */
case S390_CPU_RESET_NORMAL:
env->pfault_token = -1UL;
env->bpbc = false;
}
-}
-
-/* S390CPUClass::initial_reset() */
-static void s390_cpu_initial_reset(CPUState *s)
-{
- S390CPU *cpu = S390_CPU(s);
- CPUS390XState *env = &cpu->env;
-
- s390_cpu_reset(s, S390_CPU_RESET_NORMAL);
- /* initial reset does not clear everything! */
- memset(&env->start_initial_reset_fields, 0,
- offsetof(CPUS390XState, end_reset_fields) -
- offsetof(CPUS390XState, start_initial_reset_fields));
-
- /* architectured initial values for CR 0 and 14 */
- env->cregs[0] = CR0_RESET;
- env->cregs[14] = CR14_RESET;
-
- /* architectured initial value for Breaking-Event-Address register */
- env->gbea = 1;
-
- env->pfault_token = -1UL;
-
- /* tininess for underflow is detected before rounding */
- set_float_detect_tininess(float_tininess_before_rounding,
- &env->fpu_status);
/* Reset state inside the kernel that we cannot access yet from QEMU. */
if (kvm_enabled()) {
@@ -478,7 +469,6 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
scc->load_normal = s390_cpu_load_normal;
#endif
scc->reset = s390_cpu_reset;
- scc->initial_cpu_reset = s390_cpu_initial_reset;
cc->reset = s390_cpu_full_reset;
cc->class_by_name = s390_cpu_class_by_name,
cc->has_work = s390_cpu_has_work;
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 18123dfd5b..d2af13b345 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -748,7 +748,7 @@ static inline void s390_do_cpu_initial_reset(CPUState *cs, run_on_cpu_data arg)
{
S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
- scc->initial_cpu_reset(cs);
+ scc->reset(cs, S390_CPU_RESET_INITIAL);
}
static inline void s390_do_cpu_load_normal(CPUState *cs, run_on_cpu_data arg)
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index 850139b9cd..727875bb4a 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -254,7 +254,7 @@ static void sigp_initial_cpu_reset(CPUState *cs, run_on_cpu_data arg)
SigpInfo *si = arg.host_ptr;
cpu_synchronize_state(cs);
- scc->initial_cpu_reset(cs);
+ scc->reset(cs, S390_CPU_RESET_INITIAL);
cpu_synchronize_post_reset(cs);
si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 4/5] s390x: Move clear reset
2019-11-25 9:03 [PATCH v3 0/5] s390x: Reset cleanup Janosch Frank
` (2 preceding siblings ...)
2019-11-25 9:03 ` [PATCH v3 3/5] s390x: Move initial reset Janosch Frank
@ 2019-11-25 9:03 ` Janosch Frank
2019-11-25 13:37 ` Cornelia Huck
2019-11-25 9:03 ` [PATCH v3 5/5] s390x: Beautify diag308 handling Janosch Frank
4 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-11-25 9:03 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, pmorel, david, cohuck, borntraeger, qemu-s390x, mihajlov
Let's also move the clear reset function into the reset handler.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
target/s390x/cpu-qom.h | 1 +
target/s390x/cpu.c | 58 +++++++++++++-----------------------------
2 files changed, 18 insertions(+), 41 deletions(-)
diff --git a/target/s390x/cpu-qom.h b/target/s390x/cpu-qom.h
index 6f0a12042e..dbe5346ec9 100644
--- a/target/s390x/cpu-qom.h
+++ b/target/s390x/cpu-qom.h
@@ -37,6 +37,7 @@ typedef struct S390CPUDef S390CPUDef;
typedef enum cpu_reset_type {
S390_CPU_RESET_NORMAL,
S390_CPU_RESET_INITIAL,
+ S390_CPU_RESET_CLEAR,
} cpu_reset_type;
/**
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 9f009b226c..4853cef204 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -94,6 +94,9 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type type)
s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
switch (type) {
+ case S390_CPU_RESET_CLEAR:
+ memset(env, 0, offsetof(CPUS390XState, start_initial_reset_fields));
+ /* fall through */
case S390_CPU_RESET_INITIAL:
/* initial reset does not clear everything! */
memset(&env->start_initial_reset_fields, 0,
@@ -107,6 +110,14 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type type)
env->cregs[0] = CR0_RESET;
env->cregs[14] = CR14_RESET;
+#if defined(CONFIG_USER_ONLY)
+ /* user mode should always be allowed to use the full FPU */
+ env->cregs[0] |= CR0_AFP;
+ if (s390_has_feat(S390_FEAT_VECTOR)) {
+ env->cregs[0] |= CR0_VECTOR;
+ }
+#endif
+
/* tininess for underflow is detected before rounding */
set_float_detect_tininess(float_tininess_before_rounding,
&env->fpu_status);
@@ -122,46 +133,6 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type type)
}
}
-/* CPUClass:reset() */
-static void s390_cpu_full_reset(CPUState *s)
-{
- S390CPU *cpu = S390_CPU(s);
- S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
- CPUS390XState *env = &cpu->env;
-
- scc->parent_reset(s);
- cpu->env.sigp_order = 0;
- s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
-
- memset(env, 0, offsetof(CPUS390XState, end_reset_fields));
-
- /* architectured initial values for CR 0 and 14 */
- env->cregs[0] = CR0_RESET;
- env->cregs[14] = CR14_RESET;
-
-#if defined(CONFIG_USER_ONLY)
- /* user mode should always be allowed to use the full FPU */
- env->cregs[0] |= CR0_AFP;
- if (s390_has_feat(S390_FEAT_VECTOR)) {
- env->cregs[0] |= CR0_VECTOR;
- }
-#endif
-
- /* architectured initial value for Breaking-Event-Address register */
- env->gbea = 1;
-
- env->pfault_token = -1UL;
-
- /* tininess for underflow is detected before rounding */
- set_float_detect_tininess(float_tininess_before_rounding,
- &env->fpu_status);
-
- /* Reset state inside the kernel that we cannot access yet from QEMU. */
- if (kvm_enabled()) {
- kvm_s390_reset_vcpu(cpu);
- }
-}
-
#if !defined(CONFIG_USER_ONLY)
static void s390_cpu_machine_reset_cb(void *opaque)
{
@@ -453,6 +424,11 @@ static Property s390x_cpu_properties[] = {
DEFINE_PROP_END_OF_LIST()
};
+static void s390_cpu_reset_clear(CPUState *s)
+{
+ return s390_cpu_reset(s, S390_CPU_RESET_CLEAR);
+}
+
static void s390_cpu_class_init(ObjectClass *oc, void *data)
{
S390CPUClass *scc = S390_CPU_CLASS(oc);
@@ -469,7 +445,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
scc->load_normal = s390_cpu_load_normal;
#endif
scc->reset = s390_cpu_reset;
- cc->reset = s390_cpu_full_reset;
+ cc->reset = s390_cpu_reset_clear;
cc->class_by_name = s390_cpu_class_by_name,
cc->has_work = s390_cpu_has_work;
#ifdef CONFIG_TCG
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/5] s390x: Move clear reset
2019-11-25 9:03 ` [PATCH v3 4/5] s390x: Move clear reset Janosch Frank
@ 2019-11-25 13:37 ` Cornelia Huck
2019-11-25 13:49 ` Janosch Frank
0 siblings, 1 reply; 12+ messages in thread
From: Cornelia Huck @ 2019-11-25 13:37 UTC (permalink / raw)
To: Janosch Frank
Cc: thuth, pmorel, david, qemu-devel, borntraeger, qemu-s390x,
mihajlov
On Mon, 25 Nov 2019 04:03:47 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:
> Let's also move the clear reset function into the reset handler.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> target/s390x/cpu-qom.h | 1 +
> target/s390x/cpu.c | 58 +++++++++++++-----------------------------
> 2 files changed, 18 insertions(+), 41 deletions(-)
>
> @@ -453,6 +424,11 @@ static Property s390x_cpu_properties[] = {
> DEFINE_PROP_END_OF_LIST()
> };
>
> +static void s390_cpu_reset_clear(CPUState *s)
> +{
> + return s390_cpu_reset(s, S390_CPU_RESET_CLEAR);
> +}
> +
> static void s390_cpu_class_init(ObjectClass *oc, void *data)
> {
> S390CPUClass *scc = S390_CPU_CLASS(oc);
> @@ -469,7 +445,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
> scc->load_normal = s390_cpu_load_normal;
> #endif
> scc->reset = s390_cpu_reset;
> - cc->reset = s390_cpu_full_reset;
> + cc->reset = s390_cpu_reset_clear;
> cc->class_by_name = s390_cpu_class_by_name,
> cc->has_work = s390_cpu_has_work;
> #ifdef CONFIG_TCG
One thing I liked about the previous naming is that it is more obvious
that the clear reset is actually the full reset of a cpu. Not sure if
keeping that is better than matching the function name to the name of
the reset being performed. Opinions?
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/5] s390x: Move clear reset
2019-11-25 13:37 ` Cornelia Huck
@ 2019-11-25 13:49 ` Janosch Frank
2019-11-25 15:42 ` Cornelia Huck
0 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-11-25 13:49 UTC (permalink / raw)
To: Cornelia Huck
Cc: thuth, pmorel, david, qemu-devel, borntraeger, qemu-s390x,
mihajlov
[-- Attachment #1.1: Type: text/plain, Size: 1601 bytes --]
On 11/25/19 2:37 PM, Cornelia Huck wrote:
> On Mon, 25 Nov 2019 04:03:47 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
>
>> Let's also move the clear reset function into the reset handler.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> target/s390x/cpu-qom.h | 1 +
>> target/s390x/cpu.c | 58 +++++++++++++-----------------------------
>> 2 files changed, 18 insertions(+), 41 deletions(-)
>>
>
>> @@ -453,6 +424,11 @@ static Property s390x_cpu_properties[] = {
>> DEFINE_PROP_END_OF_LIST()
>> };
>>
>> +static void s390_cpu_reset_clear(CPUState *s)
>> +{
>> + return s390_cpu_reset(s, S390_CPU_RESET_CLEAR);
>> +}
>> +
>> static void s390_cpu_class_init(ObjectClass *oc, void *data)
>> {
>> S390CPUClass *scc = S390_CPU_CLASS(oc);
>> @@ -469,7 +445,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
>> scc->load_normal = s390_cpu_load_normal;
>> #endif
>> scc->reset = s390_cpu_reset;
>> - cc->reset = s390_cpu_full_reset;
>> + cc->reset = s390_cpu_reset_clear;
>> cc->class_by_name = s390_cpu_class_by_name,
>> cc->has_work = s390_cpu_has_work;
>> #ifdef CONFIG_TCG
>
> One thing I liked about the previous naming is that it is more obvious
> that the clear reset is actually the full reset of a cpu. Not sure if
> keeping that is better than matching the function name to the name of
> the reset being performed. Opinions?
>
Are you only worrying for this particular wrapper or in general?
I'd be happy to rename the wrapper to s390_cpu_reset_full()
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/5] s390x: Move clear reset
2019-11-25 13:49 ` Janosch Frank
@ 2019-11-25 15:42 ` Cornelia Huck
0 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2019-11-25 15:42 UTC (permalink / raw)
To: Janosch Frank
Cc: thuth, pmorel, david, qemu-devel, borntraeger, qemu-s390x,
mihajlov
[-- Attachment #1: Type: text/plain, Size: 1857 bytes --]
On Mon, 25 Nov 2019 14:49:54 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:
> On 11/25/19 2:37 PM, Cornelia Huck wrote:
> > On Mon, 25 Nov 2019 04:03:47 -0500
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >
> >> Let's also move the clear reset function into the reset handler.
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> ---
> >> target/s390x/cpu-qom.h | 1 +
> >> target/s390x/cpu.c | 58 +++++++++++++-----------------------------
> >> 2 files changed, 18 insertions(+), 41 deletions(-)
> >>
> >
> >> @@ -453,6 +424,11 @@ static Property s390x_cpu_properties[] = {
> >> DEFINE_PROP_END_OF_LIST()
> >> };
> >>
> >> +static void s390_cpu_reset_clear(CPUState *s)
> >> +{
> >> + return s390_cpu_reset(s, S390_CPU_RESET_CLEAR);
> >> +}
> >> +
> >> static void s390_cpu_class_init(ObjectClass *oc, void *data)
> >> {
> >> S390CPUClass *scc = S390_CPU_CLASS(oc);
> >> @@ -469,7 +445,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
> >> scc->load_normal = s390_cpu_load_normal;
> >> #endif
> >> scc->reset = s390_cpu_reset;
> >> - cc->reset = s390_cpu_full_reset;
> >> + cc->reset = s390_cpu_reset_clear;
> >> cc->class_by_name = s390_cpu_class_by_name,
> >> cc->has_work = s390_cpu_has_work;
> >> #ifdef CONFIG_TCG
> >
> > One thing I liked about the previous naming is that it is more obvious
> > that the clear reset is actually the full reset of a cpu. Not sure if
> > keeping that is better than matching the function name to the name of
> > the reset being performed. Opinions?
> >
>
> Are you only worrying for this particular wrapper or in general?
> I'd be happy to rename the wrapper to s390_cpu_reset_full()
Yes, I was thinking about this wrapper only, but don't feel too
strongly.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 5/5] s390x: Beautify diag308 handling
2019-11-25 9:03 [PATCH v3 0/5] s390x: Reset cleanup Janosch Frank
` (3 preceding siblings ...)
2019-11-25 9:03 ` [PATCH v3 4/5] s390x: Move clear reset Janosch Frank
@ 2019-11-25 9:03 ` Janosch Frank
4 siblings, 0 replies; 12+ messages in thread
From: Janosch Frank @ 2019-11-25 9:03 UTC (permalink / raw)
To: qemu-devel
Cc: thuth, pmorel, david, cohuck, borntraeger, qemu-s390x, mihajlov
Let's improve readability by:
* Using constants for the subcodes
* Moving parameter checking into a function
* Removing subcode > 6 check as the default case catches that
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
target/s390x/diag.c | 54 +++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 53c2f81f2a..b5aec06d6b 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -53,6 +53,29 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
#define DIAG_308_RC_NO_CONF 0x0102
#define DIAG_308_RC_INVALID 0x0402
+#define DIAG308_RESET_MOD_CLR 0
+#define DIAG308_RESET_LOAD_NORM 1
+#define DIAG308_LOAD_CLEAR 3
+#define DIAG308_LOAD_NORMAL_DUMP 4
+#define DIAG308_SET 5
+#define DIAG308_STORE 6
+
+static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr,
+ uintptr_t ra, bool write)
+{
+ if ((r1 & 1) || (addr & ~TARGET_PAGE_MASK)) {
+ s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+ return -1;
+ }
+ if (!address_space_access_valid(&address_space_memory, addr,
+ sizeof(IplParameterBlock), write,
+ MEMTXATTRS_UNSPECIFIED)) {
+ s390_program_interrupt(env, PGM_ADDRESSING, ra);
+ return -1;
+ }
+ return 0;
+}
+
void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
{
CPUState *cs = env_cpu(env);
@@ -65,30 +88,24 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
return;
}
- if ((subcode & ~0x0ffffULL) || (subcode > 6)) {
+ if (subcode & ~0x0ffffULL) {
s390_program_interrupt(env, PGM_SPECIFICATION, ra);
return;
}
switch (subcode) {
- case 0:
+ case DIAG308_RESET_MOD_CLR:
s390_ipl_reset_request(cs, S390_RESET_MODIFIED_CLEAR);
break;
- case 1:
+ case DIAG308_RESET_LOAD_NORM:
s390_ipl_reset_request(cs, S390_RESET_LOAD_NORMAL);
break;
- case 3:
+ case DIAG308_LOAD_CLEAR:
+ /* Well we still lack the clearing bit... */
s390_ipl_reset_request(cs, S390_RESET_REIPL);
break;
- case 5:
- if ((r1 & 1) || (addr & 0x0fffULL)) {
- s390_program_interrupt(env, PGM_SPECIFICATION, ra);
- return;
- }
- if (!address_space_access_valid(&address_space_memory, addr,
- sizeof(IplParameterBlock), false,
- MEMTXATTRS_UNSPECIFIED)) {
- s390_program_interrupt(env, PGM_ADDRESSING, ra);
+ case DIAG308_SET:
+ if (diag308_parm_check(env, r1, addr, ra, false)) {
return;
}
iplb = g_new0(IplParameterBlock, 1);
@@ -110,15 +127,8 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
out:
g_free(iplb);
return;
- case 6:
- if ((r1 & 1) || (addr & 0x0fffULL)) {
- s390_program_interrupt(env, PGM_SPECIFICATION, ra);
- return;
- }
- if (!address_space_access_valid(&address_space_memory, addr,
- sizeof(IplParameterBlock), true,
- MEMTXATTRS_UNSPECIFIED)) {
- s390_program_interrupt(env, PGM_ADDRESSING, ra);
+ case DIAG308_STORE:
+ if (diag308_parm_check(env, r1, addr, ra, true)) {
return;
}
iplb = s390_ipl_get_iplb();
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread