One of the functions of the ChipTOD is to transfer TOD to the Core
(aka PC - Pervasive Core) timebase facility.
The ChipTOD can be programmed with a target address to send the TOD
value to. The hardware implementation seems to perform this by
sending the TOD value to a SCOM addres.
This implementation grabs the core directly and manipulates the
timebase facility state in the core. This is a hack, but it works
enough for now. A better implementation would implement the transfer
to the PnvCore xscom register and drive the timebase state machine
from there.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
include/hw/ppc/pnv_chiptod.h | 3 ++
include/hw/ppc/pnv_core.h | 4 ++
target/ppc/cpu.h | 7 +++
hw/ppc/pnv.c | 33 +++++++++++++
hw/ppc/pnv_chiptod.c | 92 ++++++++++++++++++++++++++++++++++++
5 files changed, 139 insertions(+)
diff --git a/include/hw/ppc/pnv_chiptod.h b/include/hw/ppc/pnv_chiptod.h
index e2765c3bfc..ffcc376e80 100644
--- a/include/hw/ppc/pnv_chiptod.h
+++ b/include/hw/ppc/pnv_chiptod.h
@@ -29,6 +29,8 @@ enum tod_state {
tod_stopped = 1,
};
+typedef struct PnvCore PnvCore;
+
struct PnvChipTOD {
DeviceState xd;
@@ -40,6 +42,7 @@ struct PnvChipTOD {
enum tod_state tod_state;
uint64_t tod_error;
uint64_t pss_mss_ctrl_reg;
+ PnvCore *slave_pc_target;
};
struct PnvChipTODClass {
diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index 4db21229a6..5f52ae7dbf 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -85,4 +85,8 @@ struct PnvQuad {
MemoryRegion xscom_regs;
MemoryRegion xscom_qme_regs;
};
+
+PnvCore *pnv_get_core_by_xscom_base(PnvChip *chip, uint32_t xscom_base);
+PnvCore *pnv_get_core_by_id(PnvChip *chip, uint32_t core_id);
+
#endif /* PPC_PNV_CORE_H */
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 848e583c2d..8df5626939 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1258,6 +1258,13 @@ struct CPUArchState {
uint32_t tlb_need_flush; /* Delayed flush needed */
#define TLB_NEED_LOCAL_FLUSH 0x1
#define TLB_NEED_GLOBAL_FLUSH 0x2
+
+#if defined(TARGET_PPC64)
+ /* Would be nice to put these into PnvCore */
+ /* PowerNV chiptod / timebase facility state. */
+ 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 */
+#endif
#endif
/* Other registers */
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 546266ae3d..fa0dc26732 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -2032,6 +2032,39 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
}
}
+PnvCore *pnv_get_core_by_xscom_base(PnvChip *chip, uint32_t xscom_base)
+{
+ PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
+ int i;
+
+ for (i = 0; i < chip->nr_cores; i++) {
+ PnvCore *pc = chip->cores[i];
+ CPUCore *cc = CPU_CORE(pc);
+ int core_hwid = cc->core_id;
+
+ if (pcc->xscom_core_base(chip, core_hwid) == xscom_base) {
+ return pc;
+ }
+ }
+ return NULL;
+}
+
+PnvCore *pnv_get_core_by_id(PnvChip *chip, uint32_t core_id)
+{
+ int i;
+
+ for (i = 0; i < chip->nr_cores; i++) {
+ PnvCore *pc = chip->cores[i];
+ CPUCore *cc = CPU_CORE(pc);
+
+ if (cc->core_id == core_id) {
+ return pc;
+ }
+ }
+ return NULL;
+}
+
+
static void pnv_chip_realize(DeviceState *dev, Error **errp)
{
PnvChip *chip = PNV_CHIP(dev);
diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
index a7bfe4298d..a2c1c83800 100644
--- a/hw/ppc/pnv_chiptod.c
+++ b/hw/ppc/pnv_chiptod.c
@@ -201,6 +201,62 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr addr,
chiptod->pss_mss_ctrl_reg = val & PPC_BITMASK(0, 31);
break;
+ case TOD_TX_TTYPE_CTRL_REG:
+ /*
+ * This register sets the target of the TOD value transfer initiated
+ * by TOD_MOVE_TOD_TO_TB. The TOD is able to send the address to
+ * any target register, though in practice only the PC TOD register
+ * should be used. ChipTOD has a "SCOM addressing" mode which fully
+ * specifies the SCOM address, and a core-ID mode which uses the
+ * core ID to target the PC TOD for a given core.
+ *
+ * skiboot uses SCOM for P10 and ID for P9, possibly due to hardware
+ * weirdness. For this reason, that is all we implement here.
+ */
+ if (val & PPC_BIT(35)) { /* SCOM addressing */
+ uint32_t addr2 = val >> 32;
+ uint32_t reg = addr2 & 0xfff;
+
+ if (reg != PC_TOD) {
+ qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: SCOM addressing: "
+ "unimplemented slave register 0x%" PRIx32 "\n",
+ reg);
+ return;
+ }
+
+ /*
+ * This may not deal with P10 big-core addressing at the moment.
+ * The big-core code in skiboot syncs small cores, but it targets
+ * the even PIR (first small-core) when syncing second small-core.
+ */
+ chiptod->slave_pc_target =
+ pnv_get_core_by_xscom_base(chiptod->chip, addr2 & ~0xfff);
+ if (!chiptod->slave_pc_target) {
+ qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+ " TOD_TX_TTYPE_CTRL_REG val 0x%" PRIx64
+ " invalid slave XSCOM address\n", val);
+ }
+
+ } else { /* PIR addressing */
+ uint32_t core_id;
+
+ if (!is_power9) {
+ qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: PIR addressing"
+ " is only implemented for POWER9\n");
+ return;
+ }
+
+ core_id = GETFIELD(TOD_TX_TTYPE_PIB_SLAVE_ADDR, val) & 0x1f;
+ chiptod->slave_pc_target = pnv_get_core_by_id(chiptod->chip,
+ core_id);
+ if (!chiptod->slave_pc_target) {
+ qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+ " TOD_TX_TTYPE_CTRL_REG val 0x%" PRIx64
+ " invalid slave core ID 0x%" PRIx32 "\n",
+ val, core_id);
+ }
+ }
+ break;
case TOD_ERROR_REG:
chiptod->tod_error &= ~val;
break;
@@ -215,6 +271,42 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr addr,
case TOD_LOAD_TOD_REG:
chiptod->tod_state = tod_running;
break;
+ case TOD_MOVE_TOD_TO_TB_REG:
+ /*
+ * XXX: it should be a cleaner model to have this drive a SCOM
+ * transaction to the target address, and implement the state machine
+ * in the PnvCore. For now, this hack makes things work.
+ */
+ if (!(val & PPC_BIT(0))) {
+ qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+ " TOD_MOVE_TOD_TO_TB_REG with bad val 0x%016lx\n",
+ val);
+ } else if (chiptod->slave_pc_target == NULL) {
+ qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+ " TOD_MOVE_TOD_TO_TB_REG with no slave target\n");
+ } else {
+ PowerPCCPU *cpu = chiptod->slave_pc_target->threads[0];
+ CPUPPCState *env = &cpu->env;
+
+ /*
+ * Moving TOD to TB will set the TB of all threads in a
+ * core, so skiboot only does this once per thread0, so
+ * that is where we keep the timebase state machine.
+ *
+ * It is likely possible for TBST to be driven from other
+ * threads in the core, but for now we only implement it for
+ * thread 0.
+ */
+
+ if (env->tb_ready_for_tod) {
+ env->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"
+ " receive TOD\n");
+ }
+ }
+ break;
case TOD_TX_TTYPE_4_REG:
if (is_power9) {
chiptod_power9_send_remotes(chiptod);
--
2.42.0On 11/23/23 11:30, Nicholas Piggin wrote:
> One of the functions of the ChipTOD is to transfer TOD to the Core
> (aka PC - Pervasive Core) timebase facility.
>
> The ChipTOD can be programmed with a target address to send the TOD
> value to. The hardware implementation seems to perform this by
> sending the TOD value to a SCOM addres.
address
>
> This implementation grabs the core directly and manipulates the
> timebase facility state in the core. This is a hack, but it works
> enough for now. A better implementation would implement the transfer
> to the PnvCore xscom register and drive the timebase state machine
> from there.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> include/hw/ppc/pnv_chiptod.h | 3 ++
> include/hw/ppc/pnv_core.h | 4 ++
> target/ppc/cpu.h | 7 +++
> hw/ppc/pnv.c | 33 +++++++++++++
> hw/ppc/pnv_chiptod.c | 92 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 139 insertions(+)
>
> diff --git a/include/hw/ppc/pnv_chiptod.h b/include/hw/ppc/pnv_chiptod.h
> index e2765c3bfc..ffcc376e80 100644
> --- a/include/hw/ppc/pnv_chiptod.h
> +++ b/include/hw/ppc/pnv_chiptod.h
> @@ -29,6 +29,8 @@ enum tod_state {
> tod_stopped = 1,
> };
>
> +typedef struct PnvCore PnvCore;
> +
> struct PnvChipTOD {
> DeviceState xd;
>
> @@ -40,6 +42,7 @@ struct PnvChipTOD {
> enum tod_state tod_state;
> uint64_t tod_error;
> uint64_t pss_mss_ctrl_reg;
> + PnvCore *slave_pc_target;
> };
>
> struct PnvChipTODClass {
> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> index 4db21229a6..5f52ae7dbf 100644
> --- a/include/hw/ppc/pnv_core.h
> +++ b/include/hw/ppc/pnv_core.h
> @@ -85,4 +85,8 @@ struct PnvQuad {
> MemoryRegion xscom_regs;
> MemoryRegion xscom_qme_regs;
> };
> +
> +PnvCore *pnv_get_core_by_xscom_base(PnvChip *chip, uint32_t xscom_base);
> +PnvCore *pnv_get_core_by_id(PnvChip *chip, uint32_t core_id);
> +
> #endif /* PPC_PNV_CORE_H */
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 848e583c2d..8df5626939 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1258,6 +1258,13 @@ struct CPUArchState {
> uint32_t tlb_need_flush; /* Delayed flush needed */
> #define TLB_NEED_LOCAL_FLUSH 0x1
> #define TLB_NEED_GLOBAL_FLUSH 0x2
> +
> +#if defined(TARGET_PPC64)
> + /* Would be nice to put these into PnvCore */
This is going to be complex to do from the insn implementation.
> + /* PowerNV chiptod / timebase facility state. */
> + 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 */
> +#endif
> #endif
>
> /* Other registers */
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 546266ae3d..fa0dc26732 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -2032,6 +2032,39 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
> }
> }
>
> +PnvCore *pnv_get_core_by_xscom_base(PnvChip *chip, uint32_t xscom_base)
please use a pnv_chip_ prefix and move this routine definition in file
pnv_chiptod.c if possible. We don't want this routine to be used too
widely ... if not possible, please add a comment saying this is a
shortcut to avoid complex modeling of HW which is not exposed to the
software. In any case, add the comment.
> +{
> + PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> + int i;
> +
> + for (i = 0; i < chip->nr_cores; i++) {
> + PnvCore *pc = chip->cores[i];
> + CPUCore *cc = CPU_CORE(pc);
> + int core_hwid = cc->core_id;
> +
> + if (pcc->xscom_core_base(chip, core_hwid) == xscom_base) {
> + return pc;
> + }
> + }
> + return NULL;
> +}
> +
> +PnvCore *pnv_get_core_by_id(PnvChip *chip, uint32_t core_id)
please use a pnv_chip_ prefix and move this routine definition close
to pnv_chip_find_cpu()
> +{
> + int i;
> +
> + for (i = 0; i < chip->nr_cores; i++) {
> + PnvCore *pc = chip->cores[i];
> + CPUCore *cc = CPU_CORE(pc);
> +
> + if (cc->core_id == core_id) {
> + return pc;
> + }
> + }
> + return NULL;
> +}
> +
> +
> static void pnv_chip_realize(DeviceState *dev, Error **errp)
> {
> PnvChip *chip = PNV_CHIP(dev);
> diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
> index a7bfe4298d..a2c1c83800 100644
> --- a/hw/ppc/pnv_chiptod.c
> +++ b/hw/ppc/pnv_chiptod.c
> @@ -201,6 +201,62 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr addr,
> chiptod->pss_mss_ctrl_reg = val & PPC_BITMASK(0, 31);
> break;
>
> + case TOD_TX_TTYPE_CTRL_REG:
> + /*
> + * This register sets the target of the TOD value transfer initiated
> + * by TOD_MOVE_TOD_TO_TB. The TOD is able to send the address to
> + * any target register, though in practice only the PC TOD register
> + * should be used. ChipTOD has a "SCOM addressing" mode which fully
> + * specifies the SCOM address, and a core-ID mode which uses the
> + * core ID to target the PC TOD for a given core.
> + *
> + * skiboot uses SCOM for P10 and ID for P9, possibly due to hardware
> + * weirdness. For this reason, that is all we implement here.
> + */
> + if (val & PPC_BIT(35)) { /* SCOM addressing */
> + uint32_t addr2 = val >> 32;
> + uint32_t reg = addr2 & 0xfff;
> +
> + if (reg != PC_TOD) {
> + qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: SCOM addressing: "
> + "unimplemented slave register 0x%" PRIx32 "\n",
> + reg);
> + return;
> + }
> +
> + /*
> + * This may not deal with P10 big-core addressing at the moment.
> + * The big-core code in skiboot syncs small cores, but it targets
> + * the even PIR (first small-core) when syncing second small-core.
> + */
> + chiptod->slave_pc_target =
> + pnv_get_core_by_xscom_base(chiptod->chip, addr2 & ~0xfff);
> + if (!chiptod->slave_pc_target) {
> + qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> + " TOD_TX_TTYPE_CTRL_REG val 0x%" PRIx64
> + " invalid slave XSCOM address\n", val);
> + }
So this is preparing the chiptod to initiate a SCOM memop on the
targetted core.
> + } else { /* PIR addressing */
> + uint32_t core_id;
I suppose "PIR addressing" is the previous way of doing the same.
> +
> + if (!is_power9) {
Please transform 'is_power9' into a class attribute
bool PnvChipTODClass::pir_addressing
> + qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: PIR addressing"
> + " is only implemented for POWER9\n");
".... for POWER10" or "for this CPU"
> + return;
> + }
> +
> + core_id = GETFIELD(TOD_TX_TTYPE_PIB_SLAVE_ADDR, val) & 0x1f;
> + chiptod->slave_pc_target = pnv_get_core_by_id(chiptod->chip,
> + core_id);
> + if (!chiptod->slave_pc_target) {
> + qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> + " TOD_TX_TTYPE_CTRL_REG val 0x%" PRIx64
> + " invalid slave core ID 0x%" PRIx32 "\n",
> + val, core_id);
> + }
> + }
> + break;
> case TOD_ERROR_REG:
> chiptod->tod_error &= ~val;
> break;
> @@ -215,6 +271,42 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr addr,
> case TOD_LOAD_TOD_REG:
> chiptod->tod_state = tod_running;
> break;
> + case TOD_MOVE_TOD_TO_TB_REG:
> + /*
> + * XXX: it should be a cleaner model to have this drive a SCOM
> + * transaction to the target address, and implement the state machine
> + * in the PnvCore. For now, this hack makes things work.
> + */
> + if (!(val & PPC_BIT(0))) {
> + qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> + " TOD_MOVE_TOD_TO_TB_REG with bad val 0x%016lx\n",
Use PRIx64 please
> + val);
> + } else if (chiptod->slave_pc_target == NULL) {
> + qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> + " TOD_MOVE_TOD_TO_TB_REG with no slave target\n");
> + } else {
> + PowerPCCPU *cpu = chiptod->slave_pc_target->threads[0];
> + CPUPPCState *env = &cpu->env;
> +
> + /*
> + * Moving TOD to TB will set the TB of all threads in a
> + * core, so skiboot only does this once per thread0, so
> + * that is where we keep the timebase state machine.
> + *
> + * It is likely possible for TBST to be driven from other
> + * threads in the core, but for now we only implement it for
> + * thread 0.
> + */
and here, the memop is done and the status of the transaction should be
read in SPR_TFMR. This is not a friendly HW interface !
Thanks,
C.
> + if (env->tb_ready_for_tod) {
> + env->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"
> + " receive TOD\n");
> + }
> + }
> + break;
> case TOD_TX_TTYPE_4_REG:
> if (is_power9) {
> chiptod_power9_send_remotes(chiptod);
On Fri Nov 24, 2023 at 4:34 AM AEST, Cédric Le Goater wrote:
> On 11/23/23 11:30, Nicholas Piggin wrote:
> > One of the functions of the ChipTOD is to transfer TOD to the Core
> > (aka PC - Pervasive Core) timebase facility.
> >
> > The ChipTOD can be programmed with a target address to send the TOD
> > value to. The hardware implementation seems to perform this by
> > sending the TOD value to a SCOM addres.
>
> address
>
> >
> > This implementation grabs the core directly and manipulates the
> > timebase facility state in the core. This is a hack, but it works
> > enough for now. A better implementation would implement the transfer
> > to the PnvCore xscom register and drive the timebase state machine
> > from there.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > include/hw/ppc/pnv_chiptod.h | 3 ++
> > include/hw/ppc/pnv_core.h | 4 ++
> > target/ppc/cpu.h | 7 +++
> > hw/ppc/pnv.c | 33 +++++++++++++
> > hw/ppc/pnv_chiptod.c | 92 ++++++++++++++++++++++++++++++++++++
> > 5 files changed, 139 insertions(+)
> >
> > diff --git a/include/hw/ppc/pnv_chiptod.h b/include/hw/ppc/pnv_chiptod.h
> > index e2765c3bfc..ffcc376e80 100644
> > --- a/include/hw/ppc/pnv_chiptod.h
> > +++ b/include/hw/ppc/pnv_chiptod.h
> > @@ -29,6 +29,8 @@ enum tod_state {
> > tod_stopped = 1,
> > };
> >
> > +typedef struct PnvCore PnvCore;
> > +
> > struct PnvChipTOD {
> > DeviceState xd;
> >
> > @@ -40,6 +42,7 @@ struct PnvChipTOD {
> > enum tod_state tod_state;
> > uint64_t tod_error;
> > uint64_t pss_mss_ctrl_reg;
> > + PnvCore *slave_pc_target;
> > };
> >
> > struct PnvChipTODClass {
> > diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> > index 4db21229a6..5f52ae7dbf 100644
> > --- a/include/hw/ppc/pnv_core.h
> > +++ b/include/hw/ppc/pnv_core.h
> > @@ -85,4 +85,8 @@ struct PnvQuad {
> > MemoryRegion xscom_regs;
> > MemoryRegion xscom_qme_regs;
> > };
> > +
> > +PnvCore *pnv_get_core_by_xscom_base(PnvChip *chip, uint32_t xscom_base);
> > +PnvCore *pnv_get_core_by_id(PnvChip *chip, uint32_t core_id);
> > +
> > #endif /* PPC_PNV_CORE_H */
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index 848e583c2d..8df5626939 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -1258,6 +1258,13 @@ struct CPUArchState {
> > uint32_t tlb_need_flush; /* Delayed flush needed */
> > #define TLB_NEED_LOCAL_FLUSH 0x1
> > #define TLB_NEED_GLOBAL_FLUSH 0x2
> > +
> > +#if defined(TARGET_PPC64)
> > + /* Would be nice to put these into PnvCore */
>
> This is going to be complex to do from the insn implementation.
>
>
> > + /* PowerNV chiptod / timebase facility state. */
> > + 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 */
> > +#endif
> > #endif
> >
> > /* Other registers */
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 546266ae3d..fa0dc26732 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -2032,6 +2032,39 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
> > }
> > }
> >
> > +PnvCore *pnv_get_core_by_xscom_base(PnvChip *chip, uint32_t xscom_base)
>
> please use a pnv_chip_ prefix and move this routine definition in file
> pnv_chiptod.c if possible. We don't want this routine to be used too
> widely ... if not possible, please add a comment saying this is a
> shortcut to avoid complex modeling of HW which is not exposed to the
> software. In any case, add the comment.
Done.
> > +{
> > + PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> > + int i;
> > +
> > + for (i = 0; i < chip->nr_cores; i++) {
> > + PnvCore *pc = chip->cores[i];
> > + CPUCore *cc = CPU_CORE(pc);
> > + int core_hwid = cc->core_id;
> > +
> > + if (pcc->xscom_core_base(chip, core_hwid) == xscom_base) {
> > + return pc;
> > + }
> > + }
> > + return NULL;
> > +}
> > +
> > +PnvCore *pnv_get_core_by_id(PnvChip *chip, uint32_t core_id)
>
> please use a pnv_chip_ prefix and move this routine definition close
> to pnv_chip_find_cpu()
Done.
> > @@ -201,6 +201,62 @@ static void pnv_chiptod_xscom_write(void *opaque, hwaddr addr,
> > chiptod->pss_mss_ctrl_reg = val & PPC_BITMASK(0, 31);
> > break;
> >
> > + case TOD_TX_TTYPE_CTRL_REG:
> > + /*
> > + * This register sets the target of the TOD value transfer initiated
> > + * by TOD_MOVE_TOD_TO_TB. The TOD is able to send the address to
> > + * any target register, though in practice only the PC TOD register
> > + * should be used. ChipTOD has a "SCOM addressing" mode which fully
> > + * specifies the SCOM address, and a core-ID mode which uses the
> > + * core ID to target the PC TOD for a given core.
> > + *
> > + * skiboot uses SCOM for P10 and ID for P9, possibly due to hardware
> > + * weirdness. For this reason, that is all we implement here.
> > + */
> > + if (val & PPC_BIT(35)) { /* SCOM addressing */
> > + uint32_t addr2 = val >> 32;
> > + uint32_t reg = addr2 & 0xfff;
> > +
> > + if (reg != PC_TOD) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: SCOM addressing: "
> > + "unimplemented slave register 0x%" PRIx32 "\n",
> > + reg);
> > + return;
> > + }
> > +
> > + /*
> > + * This may not deal with P10 big-core addressing at the moment.
> > + * The big-core code in skiboot syncs small cores, but it targets
> > + * the even PIR (first small-core) when syncing second small-core.
> > + */
> > + chiptod->slave_pc_target =
> > + pnv_get_core_by_xscom_base(chiptod->chip, addr2 & ~0xfff);
> > + if (!chiptod->slave_pc_target) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> > + " TOD_TX_TTYPE_CTRL_REG val 0x%" PRIx64
> > + " invalid slave XSCOM address\n", val);
> > + }
> So this is preparing the chiptod to initiate a SCOM memop on the
> targetted core.
Yes.
>
> > + } else { /* PIR addressing */
> > + uint32_t core_id;
>
> I suppose "PIR addressing" is the previous way of doing the same.
Yes, I should have written "core ID addresing", and it seems to do the
same thing, but just gives you a shortcut to find the PC_TOD scom
address for that core. The WB basically says they are equivalent AFAIKS.
>
> > +
> > + if (!is_power9) {
>
> Please transform 'is_power9' into a class attribute
>
> bool PnvChipTODClass::pir_addressing
I made the entire transmit operation a class function, which works
okay. There's possibly more than one quirk in HW (based on my trouble
making skiboot work on real HW) so it could be useful to model various
other things too.
> > + } else if (chiptod->slave_pc_target == NULL) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> > + " TOD_MOVE_TOD_TO_TB_REG with no slave target\n");
> > + } else {
> > + PowerPCCPU *cpu = chiptod->slave_pc_target->threads[0];
> > + CPUPPCState *env = &cpu->env;
> > +
> > + /*
> > + * Moving TOD to TB will set the TB of all threads in a
> > + * core, so skiboot only does this once per thread0, so
> > + * that is where we keep the timebase state machine.
> > + *
> > + * It is likely possible for TBST to be driven from other
> > + * threads in the core, but for now we only implement it for
> > + * thread 0.
> > + */
>
>
> and here, the memop is done and the status of the transaction should be
> read in SPR_TFMR. This is not a friendly HW interface !
Yes, workbook says to poll TFMR[18] on the target core after this scom.
It is a bit weird, I guess there's a lot of history (and features I
don't understand) behind it. TFMR can be read via SCOM in the core PC
unit I think, so it could be programmed more naturally that way. Not
that we model SCOM access to PC registers AFAIK...
All other formatting, naming, wording comments I snipped, I agree with
and havechanged.
Thanks,
Nick
© 2016 - 2024 Red Hat, Inc.