Re: [RFC PATCH 02/10] ppc/pnv: Move timebase state into PnvCore
On Tue May 28, 2024 at 5:52 PM AEST, Cédric Le Goater wrote:
> On 5/28/24 08:28, Harsh Prateek Bora wrote:
> >
> >
> > On 5/26/24 17:56, Nicholas Piggin wrote:
> >> The timebase state machine is per per-core state and can be driven
> >> by any thread in the core. It is currently implemented as a hack
> >> where the state is in a CPU structure and only thread 0's state is
> >> accessed by the chiptod, which limits programming the timebase
> >> side of the state machine to thread 0 of a core.
> >>
> >> Move the state out into PnvCore and share it among all threads.
> >>
> >> Signed-off-by: Nicholas Piggin
> >> ---
> >> include/hw/ppc/pnv_core.h | 17
> >> target/ppc/cpu.h | 20 --
> >> hw/ppc/pnv_chiptod.c | 6 ++--
> >> target/ppc/timebase_helper.c | 53
> >> 4 files changed, 49 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> >> index 30c1e5b1a3..f434c71547 100644
> >> --- a/include/hw/ppc/pnv_core.h
> >> +++ b/include/hw/ppc/pnv_core.h
> >> @@ -25,6 +25,20 @@
> >> #include "hw/ppc/pnv.h"
> >> #include "qom/object.h"
> >> +/* ChipTOD and TimeBase State Machine */
> >> +struct pnv_tod_tbst {
> >> + int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
> >> + int tod_sent_to_tb; /* chiptod sent TOD to the core TB */
> >> +
> >> + /*
> >> + * "Timers" for async TBST events are simulated by mfTFAC because TFAC
> >> + * is polled for such events. These are just used to ensure firmware
> >> + * performs the polling at least a few times.
> >> + */
> >> + int tb_state_timer;
> >> + int tb_sync_pulse_timer;
> >> +};
> >> +
> >> #define TYPE_PNV_CORE "powernv-cpu-core"
> >> OBJECT_DECLARE_TYPE(PnvCore, PnvCoreClass,
> >> PNV_CORE)
> >> @@ -38,6 +52,9 @@ struct PnvCore {
> >> uint32_t pir;
> >> uint32_t hwid;
> >> uint64_t hrmor;
> >> +
> >> + struct pnv_tod_tbst pnv_tod_tbst;
> >> +
> >
> > Now that it is part of struct PnvCore itself, we can drop pnv_ prefix
> > and just call the member variable as tod_tbst ?
>
> yes and rename pnv_tod_tbst using CamelCase please.
Okay will do. That'll look nicer.
Thanks,
Nick
Re: [RFC PATCH 02/10] ppc/pnv: Move timebase state into PnvCore
On 5/28/24 08:28, Harsh Prateek Bora wrote:
On 5/26/24 17:56, Nicholas Piggin wrote:
The timebase state machine is per per-core state and can be driven
by any thread in the core. It is currently implemented as a hack
where the state is in a CPU structure and only thread 0's state is
accessed by the chiptod, which limits programming the timebase
side of the state machine to thread 0 of a core.
Move the state out into PnvCore and share it among all threads.
Signed-off-by: Nicholas Piggin
---
include/hw/ppc/pnv_core.h | 17
target/ppc/cpu.h | 20 --
hw/ppc/pnv_chiptod.c | 6 ++--
target/ppc/timebase_helper.c | 53
4 files changed, 49 insertions(+), 47 deletions(-)
diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index 30c1e5b1a3..f434c71547 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -25,6 +25,20 @@
#include "hw/ppc/pnv.h"
#include "qom/object.h"
+/* ChipTOD and TimeBase State Machine */
+struct pnv_tod_tbst {
+ int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
+ int tod_sent_to_tb; /* chiptod sent TOD to the core TB */
+
+ /*
+ * "Timers" for async TBST events are simulated by mfTFAC because TFAC
+ * is polled for such events. These are just used to ensure firmware
+ * performs the polling at least a few times.
+ */
+ int tb_state_timer;
+ int tb_sync_pulse_timer;
+};
+
#define TYPE_PNV_CORE "powernv-cpu-core"
OBJECT_DECLARE_TYPE(PnvCore, PnvCoreClass,
PNV_CORE)
@@ -38,6 +52,9 @@ struct PnvCore {
uint32_t pir;
uint32_t hwid;
uint64_t hrmor;
+
+ struct pnv_tod_tbst pnv_tod_tbst;
+
Now that it is part of struct PnvCore itself, we can drop pnv_ prefix
and just call the member variable as tod_tbst ?
yes and rename pnv_tod_tbst using CamelCase please.
Thanks,
C.
PnvChip *chip;
MemoryRegion xscom_regs;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 2015e603d4..1e86658da6 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1196,21 +1196,6 @@ DEXCR_ASPECT(SRAPD, 4)
DEXCR_ASPECT(NPHIE, 5)
DEXCR_ASPECT(PHIE, 6)
-/*/
-/* PowerNV ChipTOD and TimeBase State Machine */
-struct pnv_tod_tbst {
- int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
- int tod_sent_to_tb; /* chiptod sent TOD to the core TB */
-
- /*
- * "Timers" for async TBST events are simulated by mfTFAC because TFAC
- * is polled for such events. These are just used to ensure firmware
- * performs the polling at least a few times.
- */
- int tb_state_timer;
- int tb_sync_pulse_timer;
-};
-
/*/
/* The whole PowerPC CPU context */
@@ -1292,11 +1277,6 @@ struct CPUArchState {
#define TLB_NEED_LOCAL_FLUSH 0x1
#define TLB_NEED_GLOBAL_FLUSH 0x2
-#if defined(TARGET_PPC64)
- /* PowerNV chiptod / timebase facility state. */
- /* Would be nice to put these into PnvCore */
- struct pnv_tod_tbst pnv_tod_tbst;
-#endif
#endif
/* Other registers */
diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
index 3831a72101..3eaddd66f0 100644
--- a/hw/ppc/pnv_chiptod.c
+++ b/hw/ppc/pnv_chiptod.c
@@ -365,7 +365,7 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr
addr,
" TOD_MOVE_TOD_TO_TB_REG with no slave target\n");
} else {
PowerPCCPU *cpu = chiptod->slave_pc_target->threads[0];
- CPUPPCState *env = >env;
+ PnvCore *pc = pnv_cpu_state(cpu)->core;
/*
* Moving TOD to TB will set the TB of all threads in a
@@ -377,8 +377,8 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr
addr,
* thread 0.
*/
- if (env->pnv_tod_tbst.tb_ready_for_tod) {
- env->pnv_tod_tbst.tod_sent_to_tb = 1;
+ if (pc->pnv_tod_tbst.tb_ready_for_tod) {
+ pc->pnv_tod_tbst.tod_sent_to_tb = 1;
} else {
qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
" TOD_MOVE_TOD_TO_TB_REG with TB not ready to"
diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index 39d397416e..788c498d63 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -19,6 +19,7 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "hw/ppc/ppc.h"
+#include "hw/ppc/pnv_core.h"
#include "exec/helper-proto.h"
#include "exec/exec-all.h"
#include "qemu/log.h"
@@ -298,8 +299,17 @@ static void write_tfmr(CPUPPCState *env, target_ulong val)
}
}
+static struct pnv_tod_tbst *cpu_get_tbst(PowerPCCPU *cpu)
+{
+ PnvCore *pc = pnv_cpu_state(cpu)->core;
+
+ return >pnv_tod_tbst;
+}
+
static void
Re: [RFC PATCH 02/10] ppc/pnv: Move timebase state into PnvCore
On 5/26/24 17:56, Nicholas Piggin wrote:
The timebase state machine is per per-core state and can be driven
by any thread in the core. It is currently implemented as a hack
where the state is in a CPU structure and only thread 0's state is
accessed by the chiptod, which limits programming the timebase
side of the state machine to thread 0 of a core.
Move the state out into PnvCore and share it among all threads.
Signed-off-by: Nicholas Piggin
---
include/hw/ppc/pnv_core.h| 17
target/ppc/cpu.h | 20 --
hw/ppc/pnv_chiptod.c | 6 ++--
target/ppc/timebase_helper.c | 53
4 files changed, 49 insertions(+), 47 deletions(-)
diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index 30c1e5b1a3..f434c71547 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -25,6 +25,20 @@
#include "hw/ppc/pnv.h"
#include "qom/object.h"
+/* ChipTOD and TimeBase State Machine */
+struct pnv_tod_tbst {
+int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
+int tod_sent_to_tb; /* chiptod sent TOD to the core TB */
+
+/*
+ * "Timers" for async TBST events are simulated by mfTFAC because TFAC
+ * is polled for such events. These are just used to ensure firmware
+ * performs the polling at least a few times.
+ */
+int tb_state_timer;
+int tb_sync_pulse_timer;
+};
+
#define TYPE_PNV_CORE "powernv-cpu-core"
OBJECT_DECLARE_TYPE(PnvCore, PnvCoreClass,
PNV_CORE)
@@ -38,6 +52,9 @@ struct PnvCore {
uint32_t pir;
uint32_t hwid;
uint64_t hrmor;
+
+struct pnv_tod_tbst pnv_tod_tbst;
+
Now that it is part of struct PnvCore itself, we can drop pnv_ prefix
and just call the member variable as tod_tbst ?
PnvChip *chip;
MemoryRegion xscom_regs;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 2015e603d4..1e86658da6 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1196,21 +1196,6 @@ DEXCR_ASPECT(SRAPD, 4)
DEXCR_ASPECT(NPHIE, 5)
DEXCR_ASPECT(PHIE, 6)
-/*/
-/* PowerNV ChipTOD and TimeBase State Machine */
-struct pnv_tod_tbst {
-int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
-int tod_sent_to_tb; /* chiptod sent TOD to the core TB */
-
-/*
- * "Timers" for async TBST events are simulated by mfTFAC because TFAC
- * is polled for such events. These are just used to ensure firmware
- * performs the polling at least a few times.
- */
-int tb_state_timer;
-int tb_sync_pulse_timer;
-};
-
/*/
/* The whole PowerPC CPU context */
@@ -1292,11 +1277,6 @@ struct CPUArchState {
#define TLB_NEED_LOCAL_FLUSH 0x1
#define TLB_NEED_GLOBAL_FLUSH 0x2
-#if defined(TARGET_PPC64)
-/* PowerNV chiptod / timebase facility state. */
-/* Would be nice to put these into PnvCore */
-struct pnv_tod_tbst pnv_tod_tbst;
-#endif
#endif
/* Other registers */
diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
index 3831a72101..3eaddd66f0 100644
--- a/hw/ppc/pnv_chiptod.c
+++ b/hw/ppc/pnv_chiptod.c
@@ -365,7 +365,7 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr
addr,
" TOD_MOVE_TOD_TO_TB_REG with no slave target\n");
} else {
PowerPCCPU *cpu = chiptod->slave_pc_target->threads[0];
-CPUPPCState *env = >env;
+PnvCore *pc = pnv_cpu_state(cpu)->core;
/*
* Moving TOD to TB will set the TB of all threads in a
@@ -377,8 +377,8 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr
addr,
* thread 0.
*/
-if (env->pnv_tod_tbst.tb_ready_for_tod) {
-env->pnv_tod_tbst.tod_sent_to_tb = 1;
+if (pc->pnv_tod_tbst.tb_ready_for_tod) {
+pc->pnv_tod_tbst.tod_sent_to_tb = 1;
} else {
qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
" TOD_MOVE_TOD_TO_TB_REG with TB not ready to"
diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index 39d397416e..788c498d63 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -19,6 +19,7 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "hw/ppc/ppc.h"
+#include "hw/ppc/pnv_core.h"
#include "exec/helper-proto.h"
#include "exec/exec-all.h"
#include "qemu/log.h"
@@ -298,8 +299,17 @@ static void write_tfmr(CPUPPCState *env, target_ulong val)
}
}
+static struct pnv_tod_tbst *cpu_get_tbst(PowerPCCPU *cpu)
+{
+PnvCore *pc = pnv_cpu_state(cpu)->core;
+
+return >pnv_tod_tbst;
+}
+
static void tb_state_machine_step(CPUPPCState *env)
{
+PowerPCCPU *cpu =
[RFC PATCH 02/10] ppc/pnv: Move timebase state into PnvCore
The timebase state machine is per per-core state and can be driven
by any thread in the core. It is currently implemented as a hack
where the state is in a CPU structure and only thread 0's state is
accessed by the chiptod, which limits programming the timebase
side of the state machine to thread 0 of a core.
Move the state out into PnvCore and share it among all threads.
Signed-off-by: Nicholas Piggin
---
include/hw/ppc/pnv_core.h| 17
target/ppc/cpu.h | 20 --
hw/ppc/pnv_chiptod.c | 6 ++--
target/ppc/timebase_helper.c | 53
4 files changed, 49 insertions(+), 47 deletions(-)
diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index 30c1e5b1a3..f434c71547 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -25,6 +25,20 @@
#include "hw/ppc/pnv.h"
#include "qom/object.h"
+/* ChipTOD and TimeBase State Machine */
+struct pnv_tod_tbst {
+int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
+int tod_sent_to_tb; /* chiptod sent TOD to the core TB */
+
+/*
+ * "Timers" for async TBST events are simulated by mfTFAC because TFAC
+ * is polled for such events. These are just used to ensure firmware
+ * performs the polling at least a few times.
+ */
+int tb_state_timer;
+int tb_sync_pulse_timer;
+};
+
#define TYPE_PNV_CORE "powernv-cpu-core"
OBJECT_DECLARE_TYPE(PnvCore, PnvCoreClass,
PNV_CORE)
@@ -38,6 +52,9 @@ struct PnvCore {
uint32_t pir;
uint32_t hwid;
uint64_t hrmor;
+
+struct pnv_tod_tbst pnv_tod_tbst;
+
PnvChip *chip;
MemoryRegion xscom_regs;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 2015e603d4..1e86658da6 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1196,21 +1196,6 @@ DEXCR_ASPECT(SRAPD, 4)
DEXCR_ASPECT(NPHIE, 5)
DEXCR_ASPECT(PHIE, 6)
-/*/
-/* PowerNV ChipTOD and TimeBase State Machine */
-struct pnv_tod_tbst {
-int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
-int tod_sent_to_tb; /* chiptod sent TOD to the core TB */
-
-/*
- * "Timers" for async TBST events are simulated by mfTFAC because TFAC
- * is polled for such events. These are just used to ensure firmware
- * performs the polling at least a few times.
- */
-int tb_state_timer;
-int tb_sync_pulse_timer;
-};
-
/*/
/* The whole PowerPC CPU context */
@@ -1292,11 +1277,6 @@ struct CPUArchState {
#define TLB_NEED_LOCAL_FLUSH 0x1
#define TLB_NEED_GLOBAL_FLUSH 0x2
-#if defined(TARGET_PPC64)
-/* PowerNV chiptod / timebase facility state. */
-/* Would be nice to put these into PnvCore */
-struct pnv_tod_tbst pnv_tod_tbst;
-#endif
#endif
/* Other registers */
diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
index 3831a72101..3eaddd66f0 100644
--- a/hw/ppc/pnv_chiptod.c
+++ b/hw/ppc/pnv_chiptod.c
@@ -365,7 +365,7 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr
addr,
" TOD_MOVE_TOD_TO_TB_REG with no slave target\n");
} else {
PowerPCCPU *cpu = chiptod->slave_pc_target->threads[0];
-CPUPPCState *env = >env;
+PnvCore *pc = pnv_cpu_state(cpu)->core;
/*
* Moving TOD to TB will set the TB of all threads in a
@@ -377,8 +377,8 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr
addr,
* thread 0.
*/
-if (env->pnv_tod_tbst.tb_ready_for_tod) {
-env->pnv_tod_tbst.tod_sent_to_tb = 1;
+if (pc->pnv_tod_tbst.tb_ready_for_tod) {
+pc->pnv_tod_tbst.tod_sent_to_tb = 1;
} else {
qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
" TOD_MOVE_TOD_TO_TB_REG with TB not ready to"
diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index 39d397416e..788c498d63 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -19,6 +19,7 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "hw/ppc/ppc.h"
+#include "hw/ppc/pnv_core.h"
#include "exec/helper-proto.h"
#include "exec/exec-all.h"
#include "qemu/log.h"
@@ -298,8 +299,17 @@ static void write_tfmr(CPUPPCState *env, target_ulong val)
}
}
+static struct pnv_tod_tbst *cpu_get_tbst(PowerPCCPU *cpu)
+{
+PnvCore *pc = pnv_cpu_state(cpu)->core;
+
+return >pnv_tod_tbst;
+}
+
static void tb_state_machine_step(CPUPPCState *env)
{
+PowerPCCPU *cpu = env_archcpu(env);
+struct pnv_tod_tbst *pnv_tod_tbst = cpu_get_tbst(cpu);
uint64_t tfmr = env->spr[SPR_TFMR];
unsigned int tbst = tfmr_get_tb_state(tfmr);
@@ -307,15 +317,15 @@ static void tb_state_machine_step(CPUPPCState *env)
