diff mbox series

[PATCH-for-9.1,22/27] target/s390x: Convert to TCGCPUOps::get_cpu_state()

Message ID 20240319154258.71206-23-philmd@linaro.org
State New
Headers show
Series accel/tcg: Introduce TCGCPUOps::get_cpu_state() handler | expand

Commit Message

Philippe Mathieu-Daudé March 19, 2024, 3:42 p.m. UTC 
Convert cpu_get_tb_cpu_state() to TCGCPUOps::get_cpu_state().

Note, now s390x_get_cpu_state() is restricted to TCG.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/s390x/cpu.h            | 30 ------------------------------
 target/s390x/s390x-internal.h |  2 ++
 target/s390x/cpu.c            |  1 +
 target/s390x/tcg/mem_helper.c |  2 +-
 target/s390x/tcg/translate.c  | 23 +++++++++++++++++++++++
 5 files changed, 27 insertions(+), 31 deletions(-)

Comments

Richard Henderson March 19, 2024, 9:05 p.m. UTC | #1 
On 3/19/24 05:42, Philippe Mathieu-Daudé wrote:
> Convert cpu_get_tb_cpu_state() to TCGCPUOps::get_cpu_state().
> 
> Note, now s390x_get_cpu_state() is restricted to TCG.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/s390x/cpu.h            | 30 ------------------------------
>   target/s390x/s390x-internal.h |  2 ++
>   target/s390x/cpu.c            |  1 +
>   target/s390x/tcg/mem_helper.c |  2 +-
>   target/s390x/tcg/translate.c  | 23 +++++++++++++++++++++++
>   5 files changed, 27 insertions(+), 31 deletions(-)

Why is the function in translate.c, not cpu.c (with or without ifdefs)?


r~
Philippe Mathieu-Daudé March 20, 2024, 7:09 a.m. UTC | #2 
On 19/3/24 22:05, Richard Henderson wrote:
> On 3/19/24 05:42, Philippe Mathieu-Daudé wrote:
>> Convert cpu_get_tb_cpu_state() to TCGCPUOps::get_cpu_state().
>>
>> Note, now s390x_get_cpu_state() is restricted to TCG.
>>
>> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
>> ---
>>   target/s390x/cpu.h            | 30 ------------------------------
>>   target/s390x/s390x-internal.h |  2 ++
>>   target/s390x/cpu.c            |  1 +
>>   target/s390x/tcg/mem_helper.c |  2 +-
>>   target/s390x/tcg/translate.c  | 23 +++++++++++++++++++++++
>>   5 files changed, 27 insertions(+), 31 deletions(-)
> 
> Why is the function in translate.c, not cpu.c (with or without ifdefs)?

My understanding is target/foo/tcg/ is better for TCG-specific handlers,
less #ifdef'ry and stubs. Then bar_helper.c are meant for TCG helpers
(including "exec/helper-proto.h").

Can you think of a better file (new name?) in tcg/ or do you rather
keep it in the main cpu.c?

Thanks,

Phil.
Richard Henderson March 21, 2024, 2:20 a.m. UTC | #3 
On 3/19/24 21:09, Philippe Mathieu-Daudé wrote:
> On 19/3/24 22:05, Richard Henderson wrote:
>> On 3/19/24 05:42, Philippe Mathieu-Daudé wrote:
>>> Convert cpu_get_tb_cpu_state() to TCGCPUOps::get_cpu_state().
>>>
>>> Note, now s390x_get_cpu_state() is restricted to TCG.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
>>> ---
>>>   target/s390x/cpu.h            | 30 ------------------------------
>>>   target/s390x/s390x-internal.h |  2 ++
>>>   target/s390x/cpu.c            |  1 +
>>>   target/s390x/tcg/mem_helper.c |  2 +-
>>>   target/s390x/tcg/translate.c  | 23 +++++++++++++++++++++++
>>>   5 files changed, 27 insertions(+), 31 deletions(-)
>>
>> Why is the function in translate.c, not cpu.c (with or without ifdefs)?
> 
> My understanding is target/foo/tcg/ is better for TCG-specific handlers,
> less #ifdef'ry and stubs. Then bar_helper.c are meant for TCG helpers
> (including "exec/helper-proto.h").
> 
> Can you think of a better file (new name?) in tcg/ or do you rather
> keep it in the main cpu.c?

Given that all other targets to this point used cpu.c, I would prefer s390x and sparc to 
not be the only exceptions.


r~
diff mbox series

Patch

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 2e184aabf5..c84ab43928 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -412,36 +412,6 @@  static inline int s390x_env_mmu_index(CPUS390XState *env, bool ifetch)
 #endif
 }
 
-#ifdef CONFIG_TCG
-
-#include "tcg/tcg_s390x.h"
-
-#define TARGET_HAS_CPU_GET_TB_CPU_STATE
-
-static inline void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
-                                        uint64_t *cs_base, uint32_t *flags)
-{
-    if (env->psw.addr & 1) {
-        /*
-         * Instructions must be at even addresses.
-         * This needs to be checked before address translation.
-         */
-        env->int_pgm_ilen = 2; /* see s390_cpu_tlb_fill() */
-        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, 0);
-    }
-    *pc = env->psw.addr;
-    *cs_base = env->ex_value;
-    *flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
-    if (env->cregs[0] & CR0_AFP) {
-        *flags |= FLAG_MASK_AFP;
-    }
-    if (env->cregs[0] & CR0_VECTOR) {
-        *flags |= FLAG_MASK_VECTOR;
-    }
-}
-
-#endif /* CONFIG_TCG */
-
 /* PER bits from control register 9 */
 #define PER_CR9_EVENT_BRANCH           0x80000000
 #define PER_CR9_EVENT_IFETCH           0x40000000
diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
index 559c9f561d..f9796c2742 100644
--- a/target/s390x/s390x-internal.h
+++ b/target/s390x/s390x-internal.h
@@ -403,6 +403,8 @@  void s390x_translate_init(void);
 void s390x_restore_state_to_opc(CPUState *cs,
                                 const TranslationBlock *tb,
                                 const uint64_t *data);
+void s390x_get_cpu_state(CPUS390XState *env, vaddr *pc,
+                         uint64_t *cs_base, uint32_t *flags);
 #endif /* CONFIG_TCG */
 
 /* sigp.c */
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index f7194534ae..afade52b76 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -327,6 +327,7 @@  static void s390_cpu_reset_full(DeviceState *dev)
 static const TCGCPUOps s390_tcg_ops = {
     .initialize = s390x_translate_init,
     .restore_state_to_opc = s390x_restore_state_to_opc,
+    .get_cpu_state = s390x_get_cpu_state,
 
 #ifdef CONFIG_USER_ONLY
     .record_sigsegv = s390_cpu_record_sigsegv,
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 557831def4..e1d0133439 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -2391,7 +2391,7 @@  uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t addr)
    it does not change the program counter.
 
    Perform this by recording the modified instruction in env->ex_value.
-   This will be noticed by cpu_get_tb_cpu_state and thus tb translation.
+   This will be noticed by s390x_get_cpu_state and thus tb translation.
 */
 void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
 {
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 0d0c672c95..bf8d00ecef 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -44,6 +44,7 @@ 
 #include "exec/translator.h"
 #include "exec/log.h"
 #include "qemu/atomic128.h"
+#include "tcg_s390x.h"
 
 #define HELPER_H "helper.h"
 #include "exec/helper-info.c.inc"
@@ -6569,3 +6570,25 @@  void s390x_restore_state_to_opc(CPUState *cs,
     /* Record ILEN.  */
     env->int_pgm_ilen = data[2];
 }
+
+void s390x_get_cpu_state(CPUS390XState *env, vaddr *pc,
+                         uint64_t *cs_base, uint32_t *flags)
+{
+    if (env->psw.addr & 1) {
+        /*
+         * Instructions must be at even addresses.
+         * This needs to be checked before address translation.
+         */
+        env->int_pgm_ilen = 2; /* see s390_cpu_tlb_fill() */
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, 0);
+    }
+    *pc = env->psw.addr;
+    *cs_base = env->ex_value;
+    *flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
+    if (env->cregs[0] & CR0_AFP) {
+        *flags |= FLAG_MASK_AFP;
+    }
+    if (env->cregs[0] & CR0_VECTOR) {
+        *flags |= FLAG_MASK_VECTOR;
+    }
+}