1. Patchew
  2. QEMU
  3. View series

[PATCH] hw/s390x/ipl: Remove TCG dependency in handle_diag_308()

Philippe Mathieu-Daudé posted 1 patch 1 month, 1 week ago
Failed in applying to current master (apply log)
There is a newer version of this series
target/s390x/s390x-internal.h  |  3 ++-
hw/s390x/ipl.c                 |  5 -----
target/s390x/diag.c            | 28 ++++++++++++++++++----------
target/s390x/tcg/misc_helper.c |  5 ++++-
4 files changed, 24 insertions(+), 17 deletions(-)
[PATCH] hw/s390x/ipl: Remove TCG dependency in handle_diag_308()
Posted by Philippe Mathieu-Daudé 1 month, 1 week ago 
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            | 28 ++++++++++++++++++----------
 target/s390x/tcg/misc_helper.c |  5 ++++-
 4 files changed, 24 insertions(+), 17 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..714ff88e0d0 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,17 +177,17 @@ 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()) {
@@ -193,9 +198,12 @@ out:
         }
 
         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
Re: [PATCH] hw/s390x/ipl: Remove TCG dependency in handle_diag_308()
Posted by Philippe Mathieu-Daudé 1 month, 1 week ago 
On 13/6/26 19:33, 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            | 28 ++++++++++++++++++----------
>   target/s390x/tcg/misc_helper.c |  5 ++++-
>   4 files changed, 24 insertions(+), 17 deletions(-)


> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index 01cc802eaed..714ff88e0d0 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,17 +177,17 @@ 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()) {
> @@ -193,9 +198,12 @@ out:

Oops, missing:

-- >8 --
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -196,3 +196,3 @@ out:
              env->regs[r1 + 1] = DIAG_308_RC_INVAL_FOR_PV;
-            return;
+            return false;
          }
---

>           }
>   
>           s390_ipl_reset_request(cs, S390_RESET_PV);
> +        reset_requested = true;
>           break;
>       default:
>           s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>           break;
>       }
> +
> +    return reset_requested;
>   }

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.