target/s390x/s390x-internal.h | 3 ++- hw/s390x/ipl.c | 5 ----- target/s390x/diag.c | 30 +++++++++++++++++++----------- target/s390x/tcg/misc_helper.c | 5 ++++- 4 files changed, 25 insertions(+), 18 deletions(-)
Rather than calling a TCG specific method in s390_ipl_reset_request(),
have handle_diag_308() return whether a vCPU reset is pending, and use
that in the TCG DIAG helper to return to the main loop.
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
target/s390x/s390x-internal.h | 3 ++-
hw/s390x/ipl.c | 5 -----
target/s390x/diag.c | 30 +++++++++++++++++++-----------
target/s390x/tcg/misc_helper.c | 5 ++++-
4 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
index e7e4f2b45d4..35d1e34ef4d 100644
--- a/target/s390x/s390x-internal.h
+++ b/target/s390x/s390x-internal.h
@@ -385,7 +385,8 @@ int mmu_translate_real(CPUS390XState *env, hwaddr raddr, int rw,
/* misc_helper.c */
int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
-void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
+/* Return whether a CPU reset is pending */
+bool handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
uintptr_t ra);
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index fa50749a7d3..4cca21c6217 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -18,7 +18,6 @@
#include "system/physmem.h"
#include "system/reset.h"
#include "system/runstate.h"
-#include "system/tcg.h"
#include "elf.h"
#include "hw/core/loader.h"
#include "hw/core/qdev-properties.h"
@@ -690,10 +689,6 @@ void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
} else {
qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
}
- /* as this is triggered by a CPU, make sure to exit the loop */
- if (tcg_enabled()) {
- cpu_loop_exit(cs);
- }
}
void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type)
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 01cc802eaed..618ae41289b 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -95,44 +95,49 @@ static void s390_ipl_write(CPUS390XState *env, uint64_t addr,
}
}
-void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
+bool handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
{
bool valid;
CPUState *cs = env_cpu(env);
uint64_t addr = env->regs[r1];
uint64_t subcode = env->regs[r3];
IplParameterBlock *iplb;
+ bool reset_requested;
if (env->psw.mask & PSW_MASK_PSTATE) {
s390_program_interrupt(env, PGM_PRIVILEGED, ra);
- return;
+ return false;
}
if (subcode & ~0x0ffffULL) {
s390_program_interrupt(env, PGM_SPECIFICATION, ra);
- return;
+ return false;
}
if (subcode >= DIAG308_PV_SET && !s390_has_feat(S390_FEAT_UNPACK)) {
s390_program_interrupt(env, PGM_SPECIFICATION, ra);
- return;
+ return false;
}
+ reset_requested = false;
switch (subcode) {
case DIAG308_RESET_MOD_CLR:
s390_ipl_reset_request(cs, S390_RESET_MODIFIED_CLEAR);
+ reset_requested = true;
break;
case DIAG308_RESET_LOAD_NORM:
s390_ipl_reset_request(cs, S390_RESET_LOAD_NORMAL);
+ reset_requested = true;
break;
case DIAG308_LOAD_CLEAR:
/* Well we still lack the clearing bit... */
s390_ipl_reset_request(cs, S390_RESET_REIPL);
+ reset_requested = true;
break;
case DIAG308_SET:
case DIAG308_PV_SET:
if (diag308_parm_check(env, r1, addr, ra, false)) {
- return;
+ break;
}
iplb = g_new0(IplParameterBlock, 1);
s390_ipl_read(env, addr, iplb, sizeof(iplb->len));
@@ -159,11 +164,11 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
env->regs[r1 + 1] = DIAG_308_RC_OK;
out:
g_free(iplb);
- return;
+ break;
case DIAG308_STORE:
case DIAG308_PV_STORE:
if (diag308_parm_check(env, r1, addr, ra, true)) {
- return;
+ break;
}
if (subcode == DIAG308_PV_STORE) {
iplb = s390_ipl_get_iplb_pv();
@@ -172,30 +177,33 @@ out:
}
if (!iplb) {
env->regs[r1 + 1] = DIAG_308_RC_NO_CONF;
- return;
+ break;
}
s390_ipl_write(env, addr, iplb, be32_to_cpu(iplb->len));
env->regs[r1 + 1] = DIAG_308_RC_OK;
- return;
+ break;
case DIAG308_PV_START:
iplb = s390_ipl_get_iplb_pv();
if (!iplb) {
env->regs[r1 + 1] = DIAG_308_RC_NO_PV_CONF;
- return;
+ break;
}
if (kvm_enabled() && kvm_s390_get_hpage_1m()) {
error_report("Protected VMs can currently not be backed with "
"huge pages");
env->regs[r1 + 1] = DIAG_308_RC_INVAL_FOR_PV;
- return;
+ break;
}
s390_ipl_reset_request(cs, S390_RESET_PV);
+ reset_requested = true;
break;
default:
s390_program_interrupt(env, PGM_SPECIFICATION, ra);
break;
}
+
+ return reset_requested;
}
diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 3d13c8bd8ea..036be93fb32 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -135,7 +135,10 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, uint32_t r3, uint32_t num)
case 0x308:
/* ipl */
bql_lock();
- handle_diag_308(env, r1, r3, GETPC());
+ if (handle_diag_308(env, r1, r3, GETPC())) {
+ /* As reset is triggered by the CPU, make sure to exit the loop */
+ cpu_loop_exit(CPU(env_archcpu(env)));
+ }
bql_unlock();
r = 0;
break;
--
2.53.0
Hi Philippe,
This patch does not apply for me. Is there any prerequisite for this?
On my side it fails to match the existing includes in hw/s390x/ipl.c because
system/physmem.h is not currently there, but is expected by this patch.
Also in target/s390x/diag.c the s390_ipl_write() function is not present.
Taking a quick look, it seems like you added that function in a different
series, specifically in:
[PATCH v2 4/6] target/s390x: Factor common s390_ipl_read/write() helpers
Sorry if I've missed something, but is this meant to be part of a series?
Thanks,
Jared Rossi
On 6/15/26 10:16 PM, Philippe Mathieu-Daudé wrote:
> Rather than calling a TCG specific method in s390_ipl_reset_request(),
> have handle_diag_308() return whether a vCPU reset is pending, and use
> that in the TCG DIAG helper to return to the main loop.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
> target/s390x/s390x-internal.h | 3 ++-
> hw/s390x/ipl.c | 5 -----
> target/s390x/diag.c | 30 +++++++++++++++++++-----------
> target/s390x/tcg/misc_helper.c | 5 ++++-
> 4 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
> index e7e4f2b45d4..35d1e34ef4d 100644
> --- a/target/s390x/s390x-internal.h
> +++ b/target/s390x/s390x-internal.h
> @@ -385,7 +385,8 @@ int mmu_translate_real(CPUS390XState *env, hwaddr raddr, int rw,
>
> /* misc_helper.c */
> int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
> -void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
> +/* Return whether a CPU reset is pending */
> +bool handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
> uintptr_t ra);
>
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index fa50749a7d3..4cca21c6217 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -18,7 +18,6 @@
> #include "system/physmem.h"
> #include "system/reset.h"
> #include "system/runstate.h"
> -#include "system/tcg.h"
> #include "elf.h"
> #include "hw/core/loader.h"
> #include "hw/core/qdev-properties.h"
> @@ -690,10 +689,6 @@ void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
> } else {
> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> }
> - /* as this is triggered by a CPU, make sure to exit the loop */
> - if (tcg_enabled()) {
> - cpu_loop_exit(cs);
> - }
> }
>
> void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type)
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index 01cc802eaed..618ae41289b 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -95,44 +95,49 @@ static void s390_ipl_write(CPUS390XState *env, uint64_t addr,
> }
> }
>
> -void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
> +bool handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
> {
> bool valid;
> CPUState *cs = env_cpu(env);
> uint64_t addr = env->regs[r1];
> uint64_t subcode = env->regs[r3];
> IplParameterBlock *iplb;
> + bool reset_requested;
>
> if (env->psw.mask & PSW_MASK_PSTATE) {
> s390_program_interrupt(env, PGM_PRIVILEGED, ra);
> - return;
> + return false;
> }
>
> if (subcode & ~0x0ffffULL) {
> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
> - return;
> + return false;
> }
>
> if (subcode >= DIAG308_PV_SET && !s390_has_feat(S390_FEAT_UNPACK)) {
> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
> - return;
> + return false;
> }
>
> + reset_requested = false;
> switch (subcode) {
> case DIAG308_RESET_MOD_CLR:
> s390_ipl_reset_request(cs, S390_RESET_MODIFIED_CLEAR);
> + reset_requested = true;
> break;
> case DIAG308_RESET_LOAD_NORM:
> s390_ipl_reset_request(cs, S390_RESET_LOAD_NORMAL);
> + reset_requested = true;
> break;
> case DIAG308_LOAD_CLEAR:
> /* Well we still lack the clearing bit... */
> s390_ipl_reset_request(cs, S390_RESET_REIPL);
> + reset_requested = true;
> break;
> case DIAG308_SET:
> case DIAG308_PV_SET:
> if (diag308_parm_check(env, r1, addr, ra, false)) {
> - return;
> + break;
> }
> iplb = g_new0(IplParameterBlock, 1);
> s390_ipl_read(env, addr, iplb, sizeof(iplb->len));
> @@ -159,11 +164,11 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
> env->regs[r1 + 1] = DIAG_308_RC_OK;
> out:
> g_free(iplb);
> - return;
> + break;
> case DIAG308_STORE:
> case DIAG308_PV_STORE:
> if (diag308_parm_check(env, r1, addr, ra, true)) {
> - return;
> + break;
> }
> if (subcode == DIAG308_PV_STORE) {
> iplb = s390_ipl_get_iplb_pv();
> @@ -172,30 +177,33 @@ out:
> }
> if (!iplb) {
> env->regs[r1 + 1] = DIAG_308_RC_NO_CONF;
> - return;
> + break;
> }
>
> s390_ipl_write(env, addr, iplb, be32_to_cpu(iplb->len));
> env->regs[r1 + 1] = DIAG_308_RC_OK;
> - return;
> + break;
> case DIAG308_PV_START:
> iplb = s390_ipl_get_iplb_pv();
> if (!iplb) {
> env->regs[r1 + 1] = DIAG_308_RC_NO_PV_CONF;
> - return;
> + break;
> }
>
> if (kvm_enabled() && kvm_s390_get_hpage_1m()) {
> error_report("Protected VMs can currently not be backed with "
> "huge pages");
> env->regs[r1 + 1] = DIAG_308_RC_INVAL_FOR_PV;
> - return;
> + break;
> }
>
> s390_ipl_reset_request(cs, S390_RESET_PV);
> + reset_requested = true;
> break;
> default:
> s390_program_interrupt(env, PGM_SPECIFICATION, ra);
> break;
> }
> +
> + return reset_requested;
> }
> diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
> index 3d13c8bd8ea..036be93fb32 100644
> --- a/target/s390x/tcg/misc_helper.c
> +++ b/target/s390x/tcg/misc_helper.c
> @@ -135,7 +135,10 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, uint32_t r3, uint32_t num)
> case 0x308:
> /* ipl */
> bql_lock();
> - handle_diag_308(env, r1, r3, GETPC());
> + if (handle_diag_308(env, r1, r3, GETPC())) {
> + /* As reset is triggered by the CPU, make sure to exit the loop */
> + cpu_loop_exit(CPU(env_archcpu(env)));
> + }
> bql_unlock();
> r = 0;
> break;
Hi Jared, On 17/6/26 14:12, Jared Rossi wrote: > Hi Philippe, > > This patch does not apply for me. Is there any prerequisite for this? > > On my side it fails to match the existing includes in hw/s390x/ipl.c > because > system/physmem.h is not currently there, but is expected by this patch. > > Also in target/s390x/diag.c the s390_ipl_write() function is not present. > > Taking a quick look, it seems like you added that function in a different > series, specifically in: > > [PATCH v2 4/6] target/s390x: Factor common s390_ipl_read/write() helpers > > Sorry if I've missed something, but is this meant to be part of a series? Well my bad, I'm never sure when to repost already reviewed series. Considering there is now a v3 for this patch, I'll repost as v4 including the reviewed pre-requisites. Thanks for giving it a try! v3: https://lore.kernel.org/qemu-devel/20260616191716.12932-3-philmd@oss.qualcomm.com/ > > Thanks, > Jared Rossi > > On 6/15/26 10:16 PM, Philippe Mathieu-Daudé wrote: >> Rather than calling a TCG specific method in s390_ipl_reset_request(), >> have handle_diag_308() return whether a vCPU reset is pending, and use >> that in the TCG DIAG helper to return to the main loop. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> >> --- >> target/s390x/s390x-internal.h | 3 ++- >> hw/s390x/ipl.c | 5 ----- >> target/s390x/diag.c | 30 +++++++++++++++++++----------- >> target/s390x/tcg/misc_helper.c | 5 ++++- >> 4 files changed, 25 insertions(+), 18 deletions(-)
© 2016 - 2026 Red Hat, Inc.