| Message ID | 20240526122612.473476-3-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc/pnv: Better big-core model, lpar-per-core, PC unit | expand |
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 <npiggin@gmail.com> > --- > 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_archcpu(env); > + struct pnv_tod_tbst *pnv_tod_tbst = cpu_get_tbst(cpu); Since cpu is not used anywhere later, we could just do cpu_get_tbst(env_archcpu(env)) ? > 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) > return; > } > > - if (env->pnv_tod_tbst.tb_sync_pulse_timer) { > - env->pnv_tod_tbst.tb_sync_pulse_timer--; > + if (pnv_tod_tbst->tb_sync_pulse_timer) { > + pnv_tod_tbst->tb_sync_pulse_timer--; > } else { > tfmr |= TFMR_TB_SYNC_OCCURED; > write_tfmr(env, tfmr); > } > > - if (env->pnv_tod_tbst.tb_state_timer) { > - env->pnv_tod_tbst.tb_state_timer--; > + if (pnv_tod_tbst->tb_state_timer) { > + pnv_tod_tbst->tb_state_timer--; > return; > } > > @@ -332,20 +342,20 @@ static void tb_state_machine_step(CPUPPCState *env) > } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) { > if (tbst == TBST_SYNC_WAIT) { > tfmr = tfmr_new_tb_state(tfmr, TBST_GET_TOD); > - env->pnv_tod_tbst.tb_state_timer = 3; > + pnv_tod_tbst->tb_state_timer = 3; > } else if (tbst == TBST_GET_TOD) { > - if (env->pnv_tod_tbst.tod_sent_to_tb) { > + if (pnv_tod_tbst->tod_sent_to_tb) { > tfmr = tfmr_new_tb_state(tfmr, TBST_TB_RUNNING); > tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB; > - env->pnv_tod_tbst.tb_ready_for_tod = 0; > - env->pnv_tod_tbst.tod_sent_to_tb = 0; > + pnv_tod_tbst->tb_ready_for_tod = 0; > + pnv_tod_tbst->tod_sent_to_tb = 0; > } > } else { > qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB " > "state machine in invalid state 0x%x\n", tbst); > tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR); > tfmr |= TFMR_FIRMWARE_CONTROL_ERROR; > - env->pnv_tod_tbst.tb_ready_for_tod = 0; > + pnv_tod_tbst->tb_ready_for_tod = 0; > } > } > > @@ -361,6 +371,8 @@ target_ulong helper_load_tfmr(CPUPPCState *env) > > void helper_store_tfmr(CPUPPCState *env, target_ulong val) > { > + PowerPCCPU *cpu = env_archcpu(env); > + struct pnv_tod_tbst *pnv_tod_tbst = cpu_get_tbst(cpu); ... similarly here as well. With suggested minor improvements, Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > uint64_t tfmr = env->spr[SPR_TFMR]; > uint64_t clear_on_write; > unsigned int tbst = tfmr_get_tb_state(tfmr); > @@ -384,14 +396,7 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) > * after the second mfspr. > */ > tfmr &= ~TFMR_TB_SYNC_OCCURED; > - env->pnv_tod_tbst.tb_sync_pulse_timer = 1; > - > - if (ppc_cpu_tir(env_archcpu(env)) != 0 && > - (val & (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB))) { > - qemu_log_mask(LOG_UNIMP, "TFMR timebase state machine can only be " > - "driven by thread 0\n"); > - goto out; > - } > + pnv_tod_tbst->tb_sync_pulse_timer = 1; > > if (((tfmr | val) & (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) == > (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) { > @@ -399,7 +404,7 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) > "MOVE_CHIP_TOD_TO_TB both set\n"); > tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR); > tfmr |= TFMR_FIRMWARE_CONTROL_ERROR; > - env->pnv_tod_tbst.tb_ready_for_tod = 0; > + pnv_tod_tbst->tb_ready_for_tod = 0; > goto out; > } > > @@ -413,8 +418,8 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) > tfmr &= ~TFMR_LOAD_TOD_MOD; > tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB; > tfmr &= ~TFMR_FIRMWARE_CONTROL_ERROR; /* XXX: should this be cleared? */ > - env->pnv_tod_tbst.tb_ready_for_tod = 0; > - env->pnv_tod_tbst.tod_sent_to_tb = 0; > + pnv_tod_tbst->tb_ready_for_tod = 0; > + pnv_tod_tbst->tod_sent_to_tb = 0; > goto out; > } > > @@ -427,19 +432,19 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) > > if (tfmr & TFMR_LOAD_TOD_MOD) { > /* Wait for an arbitrary 3 mfspr until the next state transition. */ > - env->pnv_tod_tbst.tb_state_timer = 3; > + pnv_tod_tbst->tb_state_timer = 3; > } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) { > if (tbst == TBST_NOT_SET) { > tfmr = tfmr_new_tb_state(tfmr, TBST_SYNC_WAIT); > - env->pnv_tod_tbst.tb_ready_for_tod = 1; > - env->pnv_tod_tbst.tb_state_timer = 3; /* arbitrary */ > + pnv_tod_tbst->tb_ready_for_tod = 1; > + pnv_tod_tbst->tb_state_timer = 3; /* arbitrary */ > } else { > qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB " > "not in TB not set state 0x%x\n", > tbst); > tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR); > tfmr |= TFMR_FIRMWARE_CONTROL_ERROR; > - env->pnv_tod_tbst.tb_ready_for_tod = 0; > + pnv_tod_tbst->tb_ready_for_tod = 0; > } > } >
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 <npiggin@gmail.com> >> --- >> 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; >> +} >> + >> static void tb_state_machine_step(CPUPPCState *env) >> { >> + PowerPCCPU *cpu = env_archcpu(env); >> + struct pnv_tod_tbst *pnv_tod_tbst = cpu_get_tbst(cpu); > > Since cpu is not used anywhere later, we could just do cpu_get_tbst(env_archcpu(env)) ? > >> 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) >> return; >> } >> - if (env->pnv_tod_tbst.tb_sync_pulse_timer) { >> - env->pnv_tod_tbst.tb_sync_pulse_timer--; >> + if (pnv_tod_tbst->tb_sync_pulse_timer) { >> + pnv_tod_tbst->tb_sync_pulse_timer--; >> } else { >> tfmr |= TFMR_TB_SYNC_OCCURED; >> write_tfmr(env, tfmr); >> } >> - if (env->pnv_tod_tbst.tb_state_timer) { >> - env->pnv_tod_tbst.tb_state_timer--; >> + if (pnv_tod_tbst->tb_state_timer) { >> + pnv_tod_tbst->tb_state_timer--; >> return; >> } >> @@ -332,20 +342,20 @@ static void tb_state_machine_step(CPUPPCState *env) >> } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) { >> if (tbst == TBST_SYNC_WAIT) { >> tfmr = tfmr_new_tb_state(tfmr, TBST_GET_TOD); >> - env->pnv_tod_tbst.tb_state_timer = 3; >> + pnv_tod_tbst->tb_state_timer = 3; >> } else if (tbst == TBST_GET_TOD) { >> - if (env->pnv_tod_tbst.tod_sent_to_tb) { >> + if (pnv_tod_tbst->tod_sent_to_tb) { >> tfmr = tfmr_new_tb_state(tfmr, TBST_TB_RUNNING); >> tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB; >> - env->pnv_tod_tbst.tb_ready_for_tod = 0; >> - env->pnv_tod_tbst.tod_sent_to_tb = 0; >> + pnv_tod_tbst->tb_ready_for_tod = 0; >> + pnv_tod_tbst->tod_sent_to_tb = 0; >> } >> } else { >> qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB " >> "state machine in invalid state 0x%x\n", tbst); >> tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR); >> tfmr |= TFMR_FIRMWARE_CONTROL_ERROR; >> - env->pnv_tod_tbst.tb_ready_for_tod = 0; >> + pnv_tod_tbst->tb_ready_for_tod = 0; >> } >> } >> @@ -361,6 +371,8 @@ target_ulong helper_load_tfmr(CPUPPCState *env) >> void helper_store_tfmr(CPUPPCState *env, target_ulong val) >> { >> + PowerPCCPU *cpu = env_archcpu(env); >> + struct pnv_tod_tbst *pnv_tod_tbst = cpu_get_tbst(cpu); > > ... similarly here as well. > > With suggested minor improvements, > Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > >> uint64_t tfmr = env->spr[SPR_TFMR]; >> uint64_t clear_on_write; >> unsigned int tbst = tfmr_get_tb_state(tfmr); >> @@ -384,14 +396,7 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) >> * after the second mfspr. >> */ >> tfmr &= ~TFMR_TB_SYNC_OCCURED; >> - env->pnv_tod_tbst.tb_sync_pulse_timer = 1; >> - >> - if (ppc_cpu_tir(env_archcpu(env)) != 0 && >> - (val & (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB))) { >> - qemu_log_mask(LOG_UNIMP, "TFMR timebase state machine can only be " >> - "driven by thread 0\n"); >> - goto out; >> - } >> + pnv_tod_tbst->tb_sync_pulse_timer = 1; >> if (((tfmr | val) & (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) == >> (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) { >> @@ -399,7 +404,7 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) >> "MOVE_CHIP_TOD_TO_TB both set\n"); >> tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR); >> tfmr |= TFMR_FIRMWARE_CONTROL_ERROR; >> - env->pnv_tod_tbst.tb_ready_for_tod = 0; >> + pnv_tod_tbst->tb_ready_for_tod = 0; >> goto out; >> } >> @@ -413,8 +418,8 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) >> tfmr &= ~TFMR_LOAD_TOD_MOD; >> tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB; >> tfmr &= ~TFMR_FIRMWARE_CONTROL_ERROR; /* XXX: should this be cleared? */ >> - env->pnv_tod_tbst.tb_ready_for_tod = 0; >> - env->pnv_tod_tbst.tod_sent_to_tb = 0; >> + pnv_tod_tbst->tb_ready_for_tod = 0; >> + pnv_tod_tbst->tod_sent_to_tb = 0; >> goto out; >> } >> @@ -427,19 +432,19 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) >> if (tfmr & TFMR_LOAD_TOD_MOD) { >> /* Wait for an arbitrary 3 mfspr until the next state transition. */ >> - env->pnv_tod_tbst.tb_state_timer = 3; >> + pnv_tod_tbst->tb_state_timer = 3; >> } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) { >> if (tbst == TBST_NOT_SET) { >> tfmr = tfmr_new_tb_state(tfmr, TBST_SYNC_WAIT); >> - env->pnv_tod_tbst.tb_ready_for_tod = 1; >> - env->pnv_tod_tbst.tb_state_timer = 3; /* arbitrary */ >> + pnv_tod_tbst->tb_ready_for_tod = 1; >> + pnv_tod_tbst->tb_state_timer = 3; /* arbitrary */ >> } else { >> qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB " >> "not in TB not set state 0x%x\n", >> tbst); >> tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR); >> tfmr |= TFMR_FIRMWARE_CONTROL_ERROR; >> - env->pnv_tod_tbst.tb_ready_for_tod = 0; >> + pnv_tod_tbst->tb_ready_for_tod = 0; >> } >> }
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 <npiggin@gmail.com> > >> --- > >> 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
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(CPUPPCState *env) return; } - if (env->pnv_tod_tbst.tb_sync_pulse_timer) { - env->pnv_tod_tbst.tb_sync_pulse_timer--; + if (pnv_tod_tbst->tb_sync_pulse_timer) { + pnv_tod_tbst->tb_sync_pulse_timer--; } else { tfmr |= TFMR_TB_SYNC_OCCURED; write_tfmr(env, tfmr); } - if (env->pnv_tod_tbst.tb_state_timer) { - env->pnv_tod_tbst.tb_state_timer--; + if (pnv_tod_tbst->tb_state_timer) { + pnv_tod_tbst->tb_state_timer--; return; } @@ -332,20 +342,20 @@ static void tb_state_machine_step(CPUPPCState *env) } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) { if (tbst == TBST_SYNC_WAIT) { tfmr = tfmr_new_tb_state(tfmr, TBST_GET_TOD); - env->pnv_tod_tbst.tb_state_timer = 3; + pnv_tod_tbst->tb_state_timer = 3; } else if (tbst == TBST_GET_TOD) { - if (env->pnv_tod_tbst.tod_sent_to_tb) { + if (pnv_tod_tbst->tod_sent_to_tb) { tfmr = tfmr_new_tb_state(tfmr, TBST_TB_RUNNING); tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB; - env->pnv_tod_tbst.tb_ready_for_tod = 0; - env->pnv_tod_tbst.tod_sent_to_tb = 0; + pnv_tod_tbst->tb_ready_for_tod = 0; + pnv_tod_tbst->tod_sent_to_tb = 0; } } else { qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB " "state machine in invalid state 0x%x\n", tbst); tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR); tfmr |= TFMR_FIRMWARE_CONTROL_ERROR; - env->pnv_tod_tbst.tb_ready_for_tod = 0; + pnv_tod_tbst->tb_ready_for_tod = 0; } } @@ -361,6 +371,8 @@ target_ulong helper_load_tfmr(CPUPPCState *env) void helper_store_tfmr(CPUPPCState *env, target_ulong val) { + PowerPCCPU *cpu = env_archcpu(env); + struct pnv_tod_tbst *pnv_tod_tbst = cpu_get_tbst(cpu); uint64_t tfmr = env->spr[SPR_TFMR]; uint64_t clear_on_write; unsigned int tbst = tfmr_get_tb_state(tfmr); @@ -384,14 +396,7 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) * after the second mfspr. */ tfmr &= ~TFMR_TB_SYNC_OCCURED; - env->pnv_tod_tbst.tb_sync_pulse_timer = 1; - - if (ppc_cpu_tir(env_archcpu(env)) != 0 && - (val & (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB))) { - qemu_log_mask(LOG_UNIMP, "TFMR timebase state machine can only be " - "driven by thread 0\n"); - goto out; - } + pnv_tod_tbst->tb_sync_pulse_timer = 1; if (((tfmr | val) & (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) == (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) { @@ -399,7 +404,7 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) "MOVE_CHIP_TOD_TO_TB both set\n"); tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR); tfmr |= TFMR_FIRMWARE_CONTROL_ERROR; - env->pnv_tod_tbst.tb_ready_for_tod = 0; + pnv_tod_tbst->tb_ready_for_tod = 0; goto out; } @@ -413,8 +418,8 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) tfmr &= ~TFMR_LOAD_TOD_MOD; tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB; tfmr &= ~TFMR_FIRMWARE_CONTROL_ERROR; /* XXX: should this be cleared? */ - env->pnv_tod_tbst.tb_ready_for_tod = 0; - env->pnv_tod_tbst.tod_sent_to_tb = 0; + pnv_tod_tbst->tb_ready_for_tod = 0; + pnv_tod_tbst->tod_sent_to_tb = 0; goto out; } @@ -427,19 +432,19 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val) if (tfmr & TFMR_LOAD_TOD_MOD) { /* Wait for an arbitrary 3 mfspr until the next state transition. */ - env->pnv_tod_tbst.tb_state_timer = 3; + pnv_tod_tbst->tb_state_timer = 3; } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) { if (tbst == TBST_NOT_SET) { tfmr = tfmr_new_tb_state(tfmr, TBST_SYNC_WAIT); - env->pnv_tod_tbst.tb_ready_for_tod = 1; - env->pnv_tod_tbst.tb_state_timer = 3; /* arbitrary */ + pnv_tod_tbst->tb_ready_for_tod = 1; + pnv_tod_tbst->tb_state_timer = 3; /* arbitrary */ } else { qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB " "not in TB not set state 0x%x\n", tbst); tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR); tfmr |= TFMR_FIRMWARE_CONTROL_ERROR; - env->pnv_tod_tbst.tb_ready_for_tod = 0; + pnv_tod_tbst->tb_ready_for_tod = 0; } }
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 <npiggin@gmail.com> --- 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(-)