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 = &cpu->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 &pc->pnv_tod_tbst; +} + stati
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 = &cpu->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 &pc->pnv_tod_tbst; +} + static void tb_state_machine_step(CPUPPCState *env) { +PowerPCCPU *cpu = env_arc
[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 = &cpu->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 &pc->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(CPUPPCStat