qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes
@ 2023-11-23 10:30 Nicholas Piggin
  2023-11-23 10:30 ` [PATCH 1/7] target/ppc: Rename TBL to TB on 64-bit Nicholas Piggin
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-23 10:30 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	Frédéric Barrat, qemu-devel

The chiptod/TFMR/state machine is not really tied to the other
time register fixes, but they touch some of the same code, and
logically same facility.

Changes since v1 of chiptod patches:
- Split hackish ChipTOD<->TFMR/TBST interface into its own patch
- Fix multi-socket addressing on P9 / chip ID mode (P10 works)
- Change chiptod primary/secondary setting to use class properties
- Add more comments to explain TOD overview and timebase state
  machine.
- SMT support for TFMR, some functionality is limited to thread 0.
- FIRMWARE_CONTROL_ERROR bit implemented in TFMR.
- Misc cleanups and bug fixes.

The hacky part, addressing core from chiptod, is still hacky. Is
there strong objection to it?

This successfully runs skiboot chiptod initialisation code with
POWER9 and POWER10 multi-socket, multi-core, SMT. That requires
skiboot 7.1 (not in-tree), otherwise chiptod init is skipped on
QEMU machines.

Thanks,
Nick

Nicholas Piggin (7):
  target/ppc: Rename TBL to TB on 64-bit
  target/ppc: Improve timebase register defines naming
  target/ppc: Fix move-to timebase SPR access permissions
  pnv/chiptod: Add POWER9/10 chiptod model
  pnv/chiptod: Implement the ChipTOD to Core transfer
  target/ppc: Implement core timebase state machine and TFMR
  target/ppc: Add SMT support to time facilities

 include/hw/ppc/pnv_chip.h    |   3 +
 include/hw/ppc/pnv_chiptod.h |  55 ++++
 include/hw/ppc/pnv_core.h    |   4 +
 include/hw/ppc/pnv_xscom.h   |   9 +
 target/ppc/cpu.h             |  50 +++-
 hw/ppc/pnv.c                 |  63 +++++
 hw/ppc/pnv_chiptod.c         | 509 +++++++++++++++++++++++++++++++++++
 target/ppc/helper_regs.c     |  39 ++-
 target/ppc/ppc-qmp-cmds.c    |   4 +
 target/ppc/timebase_helper.c | 309 ++++++++++++++++++++-
 target/ppc/translate.c       |  42 ++-
 hw/ppc/meson.build           |   1 +
 hw/ppc/trace-events          |   4 +
 13 files changed, 1067 insertions(+), 25 deletions(-)
 create mode 100644 include/hw/ppc/pnv_chiptod.h
 create mode 100644 hw/ppc/pnv_chiptod.c

-- 
2.42.0



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/7] target/ppc: Rename TBL to TB on 64-bit
  2023-11-23 10:30 [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin
@ 2023-11-23 10:30 ` Nicholas Piggin
  2023-11-23 10:30 ` [PATCH 2/7] target/ppc: Improve timebase register defines naming Nicholas Piggin
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-23 10:30 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	Frédéric Barrat, qemu-devel

From the earliest PowerPC ISA, TBR (later SPR) 268 has been called TB
and accessed with mftb instruction. The problem is that TB is the name
of the 64-bit register, and 32-bit implementations can only read the
lower half with one instruction, so 268 has also been called TBL and
it does only read TBL on 32-bit.

Change SPR 268 to be called TB on 64-bit implementations.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/helper_regs.c  | 4 ++++
 target/ppc/ppc-qmp-cmds.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index f380342d4d..8c00ed8c06 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -460,7 +460,11 @@ void register_generic_sprs(PowerPCCPU *cpu)
     }
 
     /* Time base */
+#if defined(TARGET_PPC64)
+    spr_register(env, SPR_VTBL,  "TB",
+#else
     spr_register(env, SPR_VTBL,  "TBL",
+#endif
                  &spr_read_tbl, SPR_NOACCESS,
                  &spr_read_tbl, SPR_NOACCESS,
                  0x00000000);
diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c
index f9acc21056..ffaff59e78 100644
--- a/target/ppc/ppc-qmp-cmds.c
+++ b/target/ppc/ppc-qmp-cmds.c
@@ -103,7 +103,11 @@ const MonitorDef monitor_defs[] = {
     { "xer", 0, &monitor_get_xer },
     { "msr", offsetof(CPUPPCState, msr) },
     { "tbu", 0, &monitor_get_tbu, },
+#if defined(TARGET_PPC64)
+    { "tb", 0, &monitor_get_tbl, },
+#else
     { "tbl", 0, &monitor_get_tbl, },
+#endif
     { NULL },
 };
 
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/7] target/ppc: Improve timebase register defines naming
  2023-11-23 10:30 [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin
  2023-11-23 10:30 ` [PATCH 1/7] target/ppc: Rename TBL to TB on 64-bit Nicholas Piggin
@ 2023-11-23 10:30 ` Nicholas Piggin
  2023-11-23 10:30 ` [PATCH 3/7] target/ppc: Fix move-to timebase SPR access permissions Nicholas Piggin
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-23 10:30 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	Frédéric Barrat, qemu-devel

The timebase in ppc started out with the mftb instruction which is like
mfspr but addressed timebase registers (TBRs) rather than SPRs. These
instructions could be used to read TB and TBU at 268 and 269. Timebase
could be written via the TBL and TBU SPRs at 284 and 285.

The ISA changed around v2.03 to bring TB and TBU reads into the SPR
space at 268 and 269 (access via mftb TBR-space is still supported
but will be phased out). Later, VTB was added which is an entirely
different register.

The SPR number defines in QEMU are understandably inconsistently named.
Change SPR 268, 269, 284, 285 to TBL, TBU, WR_TBL, WR_TBU, respectively.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu.h         |  8 ++++----
 target/ppc/helper_regs.c | 10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index f8101ffa29..848e583c2d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1750,8 +1750,8 @@ void ppc_compat_add_property(Object *obj, const char *name,
 #define SPR_USPRG5            (0x105)
 #define SPR_USPRG6            (0x106)
 #define SPR_USPRG7            (0x107)
-#define SPR_VTBL              (0x10C)
-#define SPR_VTBU              (0x10D)
+#define SPR_TBL               (0x10C)
+#define SPR_TBU               (0x10D)
 #define SPR_SPRG0             (0x110)
 #define SPR_SPRG1             (0x111)
 #define SPR_SPRG2             (0x112)
@@ -1764,8 +1764,8 @@ void ppc_compat_add_property(Object *obj, const char *name,
 #define SPR_SPRG7             (0x117)
 #define SPR_ASR               (0x118)
 #define SPR_EAR               (0x11A)
-#define SPR_TBL               (0x11C)
-#define SPR_TBU               (0x11D)
+#define SPR_WR_TBL            (0x11C)
+#define SPR_WR_TBU            (0x11D)
 #define SPR_TBU40             (0x11E)
 #define SPR_SVR               (0x11E)
 #define SPR_BOOKE_PIR         (0x11E)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 8c00ed8c06..6f190ab13b 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -461,22 +461,22 @@ void register_generic_sprs(PowerPCCPU *cpu)
 
     /* Time base */
 #if defined(TARGET_PPC64)
-    spr_register(env, SPR_VTBL,  "TB",
+    spr_register(env, SPR_TBL, "TB",
 #else
-    spr_register(env, SPR_VTBL,  "TBL",
+    spr_register(env, SPR_TBL, "TBL",
 #endif
                  &spr_read_tbl, SPR_NOACCESS,
                  &spr_read_tbl, SPR_NOACCESS,
                  0x00000000);
-    spr_register(env, SPR_TBL,   "TBL",
+    spr_register(env, SPR_WR_TBL, "TBL",
                  &spr_read_tbl, SPR_NOACCESS,
                  &spr_read_tbl, &spr_write_tbl,
                  0x00000000);
-    spr_register(env, SPR_VTBU,  "TBU",
+    spr_register(env, SPR_TBU, "TBU",
                  &spr_read_tbu, SPR_NOACCESS,
                  &spr_read_tbu, SPR_NOACCESS,
                  0x00000000);
-    spr_register(env, SPR_TBU,   "TBU",
+    spr_register(env, SPR_WR_TBU, "TBU",
                  &spr_read_tbu, SPR_NOACCESS,
                  &spr_read_tbu, &spr_write_tbu,
                  0x00000000);
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/7] target/ppc: Fix move-to timebase SPR access permissions
  2023-11-23 10:30 [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin
  2023-11-23 10:30 ` [PATCH 1/7] target/ppc: Rename TBL to TB on 64-bit Nicholas Piggin
  2023-11-23 10:30 ` [PATCH 2/7] target/ppc: Improve timebase register defines naming Nicholas Piggin
@ 2023-11-23 10:30 ` Nicholas Piggin
  2023-11-23 10:30 ` [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model Nicholas Piggin
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-23 10:30 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	Frédéric Barrat, qemu-devel

The move-to timebase registers TBU and TBL can not be read, and they
can not be written in supervisor mode on hypervisor-capable CPUs.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/helper_regs.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 6f190ab13b..f1493ddca0 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -468,18 +468,33 @@ void register_generic_sprs(PowerPCCPU *cpu)
                  &spr_read_tbl, SPR_NOACCESS,
                  &spr_read_tbl, SPR_NOACCESS,
                  0x00000000);
-    spr_register(env, SPR_WR_TBL, "TBL",
-                 &spr_read_tbl, SPR_NOACCESS,
-                 &spr_read_tbl, &spr_write_tbl,
-                 0x00000000);
     spr_register(env, SPR_TBU, "TBU",
                  &spr_read_tbu, SPR_NOACCESS,
                  &spr_read_tbu, SPR_NOACCESS,
                  0x00000000);
-    spr_register(env, SPR_WR_TBU, "TBU",
-                 &spr_read_tbu, SPR_NOACCESS,
-                 &spr_read_tbu, &spr_write_tbu,
-                 0x00000000);
+#ifndef CONFIG_USER_ONLY
+    if (env->has_hv_mode) {
+        spr_register_hv(env, SPR_WR_TBL, "TBL",
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        SPR_NOACCESS, &spr_write_tbl,
+                        0x00000000);
+        spr_register_hv(env, SPR_WR_TBU, "TBU",
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        SPR_NOACCESS, &spr_write_tbu,
+                        0x00000000);
+    } else {
+        spr_register(env, SPR_WR_TBL, "TBL",
+                     SPR_NOACCESS, SPR_NOACCESS,
+                     SPR_NOACCESS, &spr_write_tbl,
+                     0x00000000);
+        spr_register(env, SPR_WR_TBU, "TBU",
+                     SPR_NOACCESS, SPR_NOACCESS,
+                     SPR_NOACCESS, &spr_write_tbu,
+                     0x00000000);
+    }
+#endif
 }
 
 void register_non_embedded_sprs(CPUPPCState *env)
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model
  2023-11-23 10:30 [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin
                   ` (2 preceding siblings ...)
  2023-11-23 10:30 ` [PATCH 3/7] target/ppc: Fix move-to timebase SPR access permissions Nicholas Piggin
@ 2023-11-23 10:30 ` Nicholas Piggin
  2023-11-23 17:49   ` Cédric Le Goater
  2023-11-23 10:30 ` [PATCH 5/7] pnv/chiptod: Implement the ChipTOD to Core transfer Nicholas Piggin
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-23 10:30 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	Frédéric Barrat, qemu-devel

The ChipTOD (for Time-Of-Day) is a pervasive facility that keeps the
clocks on multiple chips consistent which can keep TOD (time-of-day),
synchronise it across multiple chips, and can move that TOD to or from
the core timebase units.

This driver implements basic emulation of chiptod registers sufficient
to successfully run the skiboot chiptod synchronisation procedure
(with the following TFMR and timebase state-machine implementation).

The main way chiptod affects the rest of the system (relevant to the
powernv model) is to interact with the timebase facility in the cores,
influencing the timebase state machine and registers.

The way this chiptod driver implements that interaction is with two
new flags in the CPUPPCState env, one is use for the core timebase to
indicate it is ready to receive a TOD from chiptod, the other used
by chiptod to indicate that it has sent TOD to the core timebase. The
core timebase will be implemented in later changes.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 include/hw/ppc/pnv_chip.h    |   3 +
 include/hw/ppc/pnv_chiptod.h |  52 +++++
 include/hw/ppc/pnv_xscom.h   |   9 +
 hw/ppc/pnv.c                 |  30 +++
 hw/ppc/pnv_chiptod.c         | 417 +++++++++++++++++++++++++++++++++++
 hw/ppc/meson.build           |   1 +
 hw/ppc/trace-events          |   4 +
 7 files changed, 516 insertions(+)
 create mode 100644 include/hw/ppc/pnv_chiptod.h
 create mode 100644 hw/ppc/pnv_chiptod.c

diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
index 0ab5c42308..bfc4772cf3 100644
--- a/include/hw/ppc/pnv_chip.h
+++ b/include/hw/ppc/pnv_chip.h
@@ -2,6 +2,7 @@
 #define PPC_PNV_CHIP_H
 
 #include "hw/pci-host/pnv_phb4.h"
+#include "hw/ppc/pnv_chiptod.h"
 #include "hw/ppc/pnv_core.h"
 #include "hw/ppc/pnv_homer.h"
 #include "hw/ppc/pnv_lpc.h"
@@ -78,6 +79,7 @@ struct Pnv9Chip {
     PnvXive      xive;
     Pnv9Psi      psi;
     PnvLpcController lpc;
+    PnvChipTOD   chiptod;
     PnvOCC       occ;
     PnvSBE       sbe;
     PnvHomer     homer;
@@ -110,6 +112,7 @@ struct Pnv10Chip {
     PnvXive2     xive;
     Pnv9Psi      psi;
     PnvLpcController lpc;
+    PnvChipTOD   chiptod;
     PnvOCC       occ;
     PnvSBE       sbe;
     PnvHomer     homer;
diff --git a/include/hw/ppc/pnv_chiptod.h b/include/hw/ppc/pnv_chiptod.h
new file mode 100644
index 0000000000..e2765c3bfc
--- /dev/null
+++ b/include/hw/ppc/pnv_chiptod.h
@@ -0,0 +1,52 @@
+/*
+ * QEMU PowerPC PowerNV Emulation of some CHIPTOD behaviour
+ *
+ * Copyright (c) 2022-2023, IBM Corporation.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef PPC_PNV_CHIPTOD_H
+#define PPC_PNV_CHIPTOD_H
+
+#include "qom/object.h"
+
+#define TYPE_PNV_CHIPTOD "pnv-chiptod"
+OBJECT_DECLARE_TYPE(PnvChipTOD, PnvChipTODClass, PNV_CHIPTOD)
+#define TYPE_PNV9_CHIPTOD TYPE_PNV_CHIPTOD "-POWER9"
+DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV9_CHIPTOD, TYPE_PNV9_CHIPTOD)
+#define TYPE_PNV10_CHIPTOD TYPE_PNV_CHIPTOD "-POWER10"
+DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV10_CHIPTOD, TYPE_PNV10_CHIPTOD)
+
+enum tod_state {
+    tod_error = 0,
+    tod_not_set = 7,
+    tod_not_set_step = 11,
+    tod_running = 2,
+    tod_running_step = 10,
+    tod_running_sync = 14,
+    tod_wait_for_sync = 13,
+    tod_stopped = 1,
+};
+
+struct PnvChipTOD {
+    DeviceState xd;
+
+    PnvChip *chip;
+    MemoryRegion xscom_regs;
+
+    bool primary;
+    bool secondary;
+    enum tod_state tod_state;
+    uint64_t tod_error;
+    uint64_t pss_mss_ctrl_reg;
+};
+
+struct PnvChipTODClass {
+    DeviceClass parent_class;
+
+    int xscom_size;
+    const MemoryRegionOps *xscom_ops;
+};
+
+#endif /* PPC_PNV_CHIPTOD_H */
diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index f5becbab41..6aa3ac745d 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -64,6 +64,9 @@ struct PnvXScomInterfaceClass {
 #define PNV_XSCOM_PSIHB_BASE      0x2010900
 #define PNV_XSCOM_PSIHB_SIZE      0x20
 
+#define PNV_XSCOM_CHIPTOD_BASE    0x0040000
+#define PNV_XSCOM_CHIPTOD_SIZE    0x31
+
 #define PNV_XSCOM_OCC_BASE        0x0066000
 #define PNV_XSCOM_OCC_SIZE        0x6000
 
@@ -93,6 +96,9 @@ struct PnvXScomInterfaceClass {
 #define PNV9_XSCOM_I2CM_BASE      0xa0000
 #define PNV9_XSCOM_I2CM_SIZE      0x1000
 
+#define PNV9_XSCOM_CHIPTOD_BASE   PNV_XSCOM_CHIPTOD_BASE
+#define PNV9_XSCOM_CHIPTOD_SIZE   PNV_XSCOM_CHIPTOD_SIZE
+
 #define PNV9_XSCOM_OCC_BASE       PNV_XSCOM_OCC_BASE
 #define PNV9_XSCOM_OCC_SIZE       0x8000
 
@@ -155,6 +161,9 @@ struct PnvXScomInterfaceClass {
 #define PNV10_XSCOM_I2CM_BASE      PNV9_XSCOM_I2CM_BASE
 #define PNV10_XSCOM_I2CM_SIZE      PNV9_XSCOM_I2CM_SIZE
 
+#define PNV10_XSCOM_CHIPTOD_BASE   PNV9_XSCOM_CHIPTOD_BASE
+#define PNV10_XSCOM_CHIPTOD_SIZE   PNV9_XSCOM_CHIPTOD_SIZE
+
 #define PNV10_XSCOM_OCC_BASE       PNV9_XSCOM_OCC_BASE
 #define PNV10_XSCOM_OCC_SIZE       PNV9_XSCOM_OCC_SIZE
 
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0297871bdd..546266ae3d 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1419,6 +1419,8 @@ static void pnv_chip_power9_instance_init(Object *obj)
 
     object_initialize_child(obj, "lpc", &chip9->lpc, TYPE_PNV9_LPC);
 
+    object_initialize_child(obj, "chiptod", &chip9->chiptod, TYPE_PNV9_CHIPTOD);
+
     object_initialize_child(obj, "occ", &chip9->occ, TYPE_PNV9_OCC);
 
     object_initialize_child(obj, "sbe", &chip9->sbe, TYPE_PNV9_SBE);
@@ -1565,6 +1567,19 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
                                             (uint64_t) PNV9_LPCM_BASE(chip));
 
+    /* ChipTOD */
+    object_property_set_bool(OBJECT(&chip9->chiptod), "primary",
+                             chip->chip_id == 0, &error_abort);
+    object_property_set_bool(OBJECT(&chip9->chiptod), "secondary",
+                             chip->chip_id == 1, &error_abort);
+    object_property_set_link(OBJECT(&chip9->chiptod), "chip", OBJECT(chip),
+                             &error_abort);
+    if (!qdev_realize(DEVICE(&chip9->chiptod), NULL, errp)) {
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV9_XSCOM_CHIPTOD_BASE,
+                            &chip9->chiptod.xscom_regs);
+
     /* Create the simplified OCC model */
     if (!qdev_realize(DEVICE(&chip9->occ), NULL, errp)) {
         return;
@@ -1677,6 +1692,8 @@ static void pnv_chip_power10_instance_init(Object *obj)
                               "xive-fabric");
     object_initialize_child(obj, "psi", &chip10->psi, TYPE_PNV10_PSI);
     object_initialize_child(obj, "lpc", &chip10->lpc, TYPE_PNV10_LPC);
+    object_initialize_child(obj, "chiptod", &chip10->chiptod,
+                            TYPE_PNV10_CHIPTOD);
     object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
     object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
     object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
@@ -1810,6 +1827,19 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
                                             (uint64_t) PNV10_LPCM_BASE(chip));
 
+    /* ChipTOD */
+    object_property_set_bool(OBJECT(&chip10->chiptod), "primary",
+                             chip->chip_id == 0, &error_abort);
+    object_property_set_bool(OBJECT(&chip10->chiptod), "secondary",
+                             chip->chip_id == 1, &error_abort);
+    object_property_set_link(OBJECT(&chip10->chiptod), "chip", OBJECT(chip),
+                             &error_abort);
+    if (!qdev_realize(DEVICE(&chip10->chiptod), NULL, errp)) {
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV10_XSCOM_CHIPTOD_BASE,
+                            &chip10->chiptod.xscom_regs);
+
     /* Create the simplified OCC model */
     if (!qdev_realize(DEVICE(&chip10->occ), NULL, errp)) {
         return;
diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
new file mode 100644
index 0000000000..a7bfe4298d
--- /dev/null
+++ b/hw/ppc/pnv_chiptod.c
@@ -0,0 +1,417 @@
+/*
+ * QEMU PowerPC PowerNV Emulation of some ChipTOD behaviour
+ *
+ * Copyright (c) 2022-2023, IBM Corporation.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ *
+ * ChipTOD (aka TOD) is a facility implemented in the nest / pervasive. The
+ * purpose is to keep time-of-day across chips and cores.
+ *
+ * There is a master chip TOD, which sends signals to slave chip TODs to
+ * keep them synchronized. There are two sets of configuration registers
+ * called primary and secondary, which can be used fail over.
+ *
+ * The chip TOD also distributes synchronisation signals to the timebase
+ * facility in each of the cores on the chip. In particular there is a
+ * feature that can move the TOD value in the ChipTOD to and from the TB.
+ *
+ * Initialisation typically brings all ChipTOD into sync (see tod_state),
+ * and then brings each core TB into sync with the ChipTODs (see timebase
+ * state and TFMR). This model is a very basic simulation of the init sequence
+ * performed by skiboot.
+ */
+
+#include "qemu/osdep.h"
+#include "target/ppc/cpu.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/ppc/fdt.h"
+#include "hw/ppc/ppc.h"
+#include "hw/ppc/pnv.h"
+#include "hw/ppc/pnv_chip.h"
+#include "hw/ppc/pnv_core.h"
+#include "hw/ppc/pnv_xscom.h"
+#include "hw/ppc/pnv_chiptod.h"
+#include "trace.h"
+
+#include <libfdt.h>
+
+/* TOD chip XSCOM addresses */
+#define TOD_M_PATH_CTRL_REG             0x00000000 /* Master Path ctrl reg */
+#define TOD_PRI_PORT_0_CTRL_REG         0x00000001 /* Primary port0 ctrl reg */
+#define TOD_PRI_PORT_1_CTRL_REG         0x00000002 /* Primary port1 ctrl reg */
+#define TOD_SEC_PORT_0_CTRL_REG         0x00000003 /* Secondary p0 ctrl reg */
+#define TOD_SEC_PORT_1_CTRL_REG         0x00000004 /* Secondary p1 ctrl reg */
+#define TOD_S_PATH_CTRL_REG             0x00000005 /* Slave Path ctrl reg */
+#define TOD_I_PATH_CTRL_REG             0x00000006 /* Internal Path ctrl reg */
+
+/* -- TOD primary/secondary master/slave control register -- */
+#define TOD_PSS_MSS_CTRL_REG            0x00000007
+
+/* -- TOD primary/secondary master/slave status register -- */
+#define TOD_PSS_MSS_STATUS_REG          0x00000008
+
+/* TOD chip XSCOM addresses */
+#define TOD_CHIP_CTRL_REG               0x00000010 /* Chip control reg */
+
+#define TOD_TX_TTYPE_0_REG              0x00000011
+#define TOD_TX_TTYPE_1_REG              0x00000012 /* PSS switch reg */
+#define TOD_TX_TTYPE_2_REG              0x00000013 /* Enable step checkers */
+#define TOD_TX_TTYPE_3_REG              0x00000014 /* Request TOD reg */
+#define TOD_TX_TTYPE_4_REG              0x00000015 /* Send TOD reg */
+#define TOD_TX_TTYPE_5_REG              0x00000016 /* Invalidate TOD reg */
+
+#define TOD_MOVE_TOD_TO_TB_REG          0x00000017
+#define TOD_LOAD_TOD_MOD_REG            0x00000018
+#define TOD_LOAD_TOD_REG                0x00000021
+#define TOD_FSM_REG                     0x00000024
+
+#define TOD_TX_TTYPE_CTRL_REG           0x00000027 /* TX TTYPE Control reg */
+#define   TOD_TX_TTYPE_PIB_SLAVE_ADDR      PPC_BITMASK(26, 31)
+
+/* -- TOD Error interrupt register -- */
+#define TOD_ERROR_REG                   0x00000030
+
+/* PC unit PIB address which recieves the timebase transfer from TOD */
+#define   PC_TOD                        0x4A3
+
+static uint64_t pnv_chiptod_xscom_read(void *opaque, hwaddr addr,
+                                          unsigned size)
+{
+    PnvChipTOD *chiptod = PNV_CHIPTOD(opaque);
+    uint32_t offset = addr >> 3;
+    uint64_t val = 0;
+
+    switch (offset) {
+    case TOD_PSS_MSS_STATUS_REG:
+        /*
+         * ChipTOD does not support configurations other than primary
+         * master, does not support errors, etc.
+         */
+        val |= PPC_BITMASK(6, 10); /* STEP checker validity */
+        val |= PPC_BIT(12); /* Primary config master path select */
+        val |= PPC_BIT(20); /* Is running */
+        val |= PPC_BIT(21); /* Is using primary config */
+        val |= PPC_BIT(26); /* Is using master path select */
+
+        if (chiptod->primary) {
+            val |= PPC_BIT(23); /* Is active master */
+        } else if (chiptod->secondary) {
+            val |= PPC_BIT(24); /* Is backup master */
+        }
+        break;
+    case TOD_PSS_MSS_CTRL_REG:
+        val = chiptod->pss_mss_ctrl_reg;
+        break;
+    case TOD_TX_TTYPE_CTRL_REG:
+        val = 0;
+        break;
+    case TOD_ERROR_REG:
+        val = chiptod->tod_error;
+        break;
+    case TOD_FSM_REG:
+        if (chiptod->tod_state == tod_running) {
+            val |= PPC_BIT(4);
+        }
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: Ox%"
+                      HWADDR_PRIx "\n", addr >> 3);
+    }
+
+    trace_pnv_chiptod_xscom_read(addr >> 3, val);
+
+    return val;
+}
+
+static void chiptod_power9_send_remotes(PnvChipTOD *chiptod)
+{
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
+        if (&chip9->chiptod != chiptod) {
+            chip9->chiptod.tod_state = tod_running;
+        }
+    }
+}
+
+static void chiptod_power10_send_remotes(PnvChipTOD *chiptod)
+{
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
+        if (&chip10->chiptod != chiptod) {
+            chip10->chiptod.tod_state = tod_running;
+        }
+    }
+}
+
+static void chiptod_power9_invalidate_remotes(PnvChipTOD *chiptod)
+{
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
+        if (&chip9->chiptod != chiptod) {
+            chip9->chiptod.tod_state = tod_not_set;
+        }
+    }
+}
+
+static void chiptod_power10_invalidate_remotes(PnvChipTOD *chiptod)
+{
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
+        if (&chip10->chiptod != chiptod) {
+            chip10->chiptod.tod_state = tod_not_set;
+        }
+    }
+}
+
+static void pnv_chiptod_xscom_write(void *opaque, hwaddr addr,
+                                    uint64_t val, unsigned size,
+                                    bool is_power9)
+{
+    PnvChipTOD *chiptod = PNV_CHIPTOD(opaque);
+    uint32_t offset = addr >> 3;
+
+    trace_pnv_chiptod_xscom_write(addr >> 3, val);
+
+    switch (offset) {
+    case TOD_PSS_MSS_CTRL_REG:
+        /* Is this correct? */
+        if (chiptod->primary) {
+            val |= PPC_BIT(1); /* TOD is master */
+        } else {
+            val &= ~PPC_BIT(1);
+        }
+        val |= PPC_BIT(2); /* Drawer is master (don't simulate multi-drawer) */
+        chiptod->pss_mss_ctrl_reg = val & PPC_BITMASK(0, 31);
+        break;
+
+    case TOD_ERROR_REG:
+        chiptod->tod_error &= ~val;
+        break;
+    case TOD_LOAD_TOD_MOD_REG:
+        if (!(val & PPC_BIT(0))) {
+            qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+                          " TOD_LOAD_TOD_MOD_REG with bad val 0x%016lx\n", val);
+        } else {
+            chiptod->tod_state = tod_not_set;
+        }
+        break;
+    case TOD_LOAD_TOD_REG:
+        chiptod->tod_state = tod_running;
+        break;
+    case TOD_TX_TTYPE_4_REG:
+        if (is_power9) {
+            chiptod_power9_send_remotes(chiptod);
+        } else {
+            chiptod_power10_send_remotes(chiptod);
+        }
+        break;
+    case TOD_TX_TTYPE_5_REG:
+        if (is_power9) {
+            chiptod_power9_invalidate_remotes(chiptod);
+        } else {
+            chiptod_power10_invalidate_remotes(chiptod);
+        }
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: Ox%"
+                      HWADDR_PRIx "\n", addr >> 3);
+    }
+}
+
+static void pnv_chiptod_power9_xscom_write(void *opaque, hwaddr addr,
+                                           uint64_t val, unsigned size)
+{
+    pnv_chiptod_xscom_write(opaque, addr, val, size, true);
+}
+
+static const MemoryRegionOps pnv_chiptod_power9_xscom_ops = {
+    .read = pnv_chiptod_xscom_read,
+    .write = pnv_chiptod_power9_xscom_write,
+    .valid.min_access_size = 8,
+    .valid.max_access_size = 8,
+    .impl.min_access_size = 8,
+    .impl.max_access_size = 8,
+    .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static int pnv_chiptod_dt_xscom(PnvXScomInterface *dev, void *fdt,
+                                int xscom_offset,
+                                const char compat[], size_t compat_size)
+{
+    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
+    g_autofree char *name = NULL;
+    int offset;
+    uint32_t lpc_pcba = PNV9_XSCOM_CHIPTOD_BASE;
+    uint32_t reg[] = {
+        cpu_to_be32(lpc_pcba),
+        cpu_to_be32(PNV9_XSCOM_CHIPTOD_SIZE)
+    };
+
+    name = g_strdup_printf("chiptod@%x", lpc_pcba);
+    offset = fdt_add_subnode(fdt, xscom_offset, name);
+    _FDT(offset);
+
+    if (chiptod->primary) {
+        _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
+    } else if (chiptod->secondary) {
+        _FDT((fdt_setprop(fdt, offset, "secondary", NULL, 0)));
+    }
+
+    _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
+    _FDT((fdt_setprop(fdt, offset, "compatible", compat, compat_size)));
+    return 0;
+}
+
+static int pnv_chiptod_power9_dt_xscom(PnvXScomInterface *dev, void *fdt,
+                             int xscom_offset)
+{
+    const char compat[] = "ibm,power-chiptod\0ibm,power9-chiptod";
+
+    return pnv_chiptod_dt_xscom(dev, fdt, xscom_offset, compat, sizeof(compat));
+}
+
+static Property pnv_chiptod_properties[] = {
+    DEFINE_PROP_BOOL("primary", PnvChipTOD, primary, false),
+    DEFINE_PROP_BOOL("secondary", PnvChipTOD, secondary, false),
+    DEFINE_PROP_LINK("chip", PnvChipTOD , chip, TYPE_PNV_CHIP, PnvChip *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pnv_chiptod_power9_class_init(ObjectClass *klass, void *data)
+{
+    PnvChipTODClass *pctc = PNV_CHIPTOD_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
+
+    dc->desc = "PowerNV ChipTOD Controller (POWER9)";
+    device_class_set_props(dc, pnv_chiptod_properties);
+
+    xdc->dt_xscom = pnv_chiptod_power9_dt_xscom;
+
+    pctc->xscom_size = PNV_XSCOM_CHIPTOD_SIZE;
+    pctc->xscom_ops = &pnv_chiptod_power9_xscom_ops;
+}
+
+static const TypeInfo pnv_chiptod_power9_type_info = {
+    .name          = TYPE_PNV9_CHIPTOD,
+    .parent        = TYPE_PNV_CHIPTOD,
+    .instance_size = sizeof(PnvChipTOD),
+    .class_init    = pnv_chiptod_power9_class_init,
+    .interfaces    = (InterfaceInfo[]) {
+        { TYPE_PNV_XSCOM_INTERFACE },
+        { }
+    }
+};
+
+static void pnv_chiptod_power10_xscom_write(void *opaque, hwaddr addr,
+                                           uint64_t val, unsigned size)
+{
+    pnv_chiptod_xscom_write(opaque, addr, val, size, false);
+}
+
+static const MemoryRegionOps pnv_chiptod_power10_xscom_ops = {
+    .read = pnv_chiptod_xscom_read,
+    .write = pnv_chiptod_power10_xscom_write,
+    .valid.min_access_size = 8,
+    .valid.max_access_size = 8,
+    .impl.min_access_size = 8,
+    .impl.max_access_size = 8,
+    .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static int pnv_chiptod_power10_dt_xscom(PnvXScomInterface *dev, void *fdt,
+                             int xscom_offset)
+{
+    const char compat[] = "ibm,power-chiptod\0ibm,power10-chiptod";
+
+    return pnv_chiptod_dt_xscom(dev, fdt, xscom_offset, compat, sizeof(compat));
+}
+
+static void pnv_chiptod_power10_class_init(ObjectClass *klass, void *data)
+{
+    PnvChipTODClass *pctc = PNV_CHIPTOD_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
+
+    dc->desc = "PowerNV ChipTOD Controller (POWER10)";
+    device_class_set_props(dc, pnv_chiptod_properties);
+
+    xdc->dt_xscom = pnv_chiptod_power10_dt_xscom;
+
+    pctc->xscom_size = PNV_XSCOM_CHIPTOD_SIZE;
+    pctc->xscom_ops = &pnv_chiptod_power10_xscom_ops;
+}
+
+static const TypeInfo pnv_chiptod_power10_type_info = {
+    .name          = TYPE_PNV10_CHIPTOD,
+    .parent        = TYPE_PNV_CHIPTOD,
+    .instance_size = sizeof(PnvChipTOD),
+    .class_init    = pnv_chiptod_power10_class_init,
+    .interfaces    = (InterfaceInfo[]) {
+        { TYPE_PNV_XSCOM_INTERFACE },
+        { }
+    }
+};
+
+static void pnv_chiptod_realize(DeviceState *dev, Error **errp)
+{
+    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
+    PnvChipTODClass *pctc = PNV_CHIPTOD_GET_CLASS(chiptod);
+
+    if (chiptod->primary) {
+        chiptod->pss_mss_ctrl_reg |= PPC_BIT(1); /* TOD is master */
+    }
+
+    /* Drawer is master (we do not simulate multi-drawer) */
+    chiptod->pss_mss_ctrl_reg |= PPC_BIT(2);
+    chiptod->tod_state = tod_running;
+
+    /* XScom regions for ChipTOD registers */
+    pnv_xscom_region_init(&chiptod->xscom_regs, OBJECT(dev),
+                          pctc->xscom_ops, chiptod, "xscom-chiptod",
+                          pctc->xscom_size);
+}
+
+static void pnv_chiptod_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = pnv_chiptod_realize;
+    dc->desc = "PowerNV ChipTOD Controller";
+    dc->user_creatable = false;
+}
+
+static const TypeInfo pnv_chiptod_type_info = {
+    .name          = TYPE_PNV_CHIPTOD,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(PnvChipTOD),
+    .class_init    = pnv_chiptod_class_init,
+    .class_size    = sizeof(PnvChipTODClass),
+    .abstract      = true,
+};
+
+static void pnv_chiptod_register_types(void)
+{
+    type_register_static(&pnv_chiptod_type_info);
+    type_register_static(&pnv_chiptod_power9_type_info);
+    type_register_static(&pnv_chiptod_power10_type_info);
+}
+
+type_init(pnv_chiptod_register_types);
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index ea44856d43..b1f4e65d24 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -46,6 +46,7 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
   'pnv_i2c.c',
   'pnv_lpc.c',
   'pnv_psi.c',
+  'pnv_chiptod.c',
   'pnv_occ.c',
   'pnv_sbe.c',
   'pnv_bmc.c',
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index f670e8906c..57c4f265ef 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -95,6 +95,10 @@ vof_write(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] \"%s\""
 vof_avail(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
 vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
 
+# pnv_chiptod.c
+pnv_chiptod_xscom_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
+pnv_chiptod_xscom_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
+
 # pnv_sbe.c
 pnv_sbe_xscom_ctrl_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
 pnv_sbe_xscom_ctrl_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/7] pnv/chiptod: Implement the ChipTOD to Core transfer
  2023-11-23 10:30 [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin
                   ` (3 preceding siblings ...)
  2023-11-23 10:30 ` [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model Nicholas Piggin
@ 2023-11-23 10:30 ` Nicholas Piggin
  2023-11-23 18:34   ` Cédric Le Goater
  2023-11-23 10:30 ` [PATCH 6/7] target/ppc: Implement core timebase state machine and TFMR Nicholas Piggin
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-23 10:30 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	Frédéric Barrat, qemu-devel

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.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/7] target/ppc: Implement core timebase state machine and TFMR
  2023-11-23 10:30 [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin
                   ` (4 preceding siblings ...)
  2023-11-23 10:30 ` [PATCH 5/7] pnv/chiptod: Implement the ChipTOD to Core transfer Nicholas Piggin
@ 2023-11-23 10:30 ` Nicholas Piggin
  2023-11-23 10:30 ` [PATCH 7/7] target/ppc: Add SMT support to time facilities Nicholas Piggin
  2023-11-23 17:09 ` [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Cédric Le Goater
  7 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-23 10:30 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	Frédéric Barrat, qemu-devel

This implements the core timebase state machine, which is the core side
of the time-of-day system in POWER processors. This facility is operated
by control fields in the TFMR register, which also contains status
fields.

The core timebase interacts with the chiptod hardware, primarily to
receive TOD updates, to synchronise timebase with other cores. This
model does not actually update TB values with TOD or updates received
from the chiptod, as timebases are always synchronised. It does step
through the states required to perform the update.

There are several asynchronous state transitions. These are modelled
using using mfTFMR to drive state changes, because it is expected that
firmware poll the register to wait for those states. This is good enough
to test basic firmware behaviour without adding real timers. The values
chosen are arbitrary.

Acked-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu.h             |  35 ++++++
 target/ppc/timebase_helper.c | 210 ++++++++++++++++++++++++++++++++++-
 2 files changed, 242 insertions(+), 3 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 8df5626939..ed4f06f059 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1264,6 +1264,13 @@ struct CPUArchState {
     /* 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 */
+
+    /*
+     * Timers for async events are simulated by mfTFAC because TFAC is to be
+     * polled for event.
+     */
+    int tb_state_timer;
+    int tb_sync_pulse_timer;
 #endif
 #endif
 
@@ -2655,6 +2662,34 @@ enum {
     HMER_XSCOM_STATUS_MASK      = PPC_BITMASK(21, 23),
 };
 
+/* TFMR */
+enum {
+    TFMR_CONTROL_MASK           = PPC_BITMASK(0, 24),
+    TFMR_MASK_HMI               = PPC_BIT(10),
+    TFMR_TB_ECLIPZ              = PPC_BIT(14),
+    TFMR_LOAD_TOD_MOD           = PPC_BIT(16),
+    TFMR_MOVE_CHIP_TOD_TO_TB    = PPC_BIT(18),
+    TFMR_CLEAR_TB_ERRORS        = PPC_BIT(24),
+    TFMR_STATUS_MASK            = PPC_BITMASK(25, 63),
+    TFMR_TBST_ENCODED           = PPC_BITMASK(28, 31), /* TBST = TB State */
+    TFMR_TBST_LAST              = PPC_BITMASK(32, 35), /* Previous TBST */
+    TFMR_TB_ENABLED             = PPC_BIT(40),
+    TFMR_TB_VALID               = PPC_BIT(41),
+    TFMR_TB_SYNC_OCCURED        = PPC_BIT(42),
+    TFMR_FIRMWARE_CONTROL_ERROR = PPC_BIT(46),
+};
+
+/* TFMR TBST (Time Base State Machine). */
+enum {
+    TBST_RESET                  = 0x0,
+    TBST_SEND_TOD_MOD           = 0x1,
+    TBST_NOT_SET                = 0x2,
+    TBST_SYNC_WAIT              = 0x6,
+    TBST_GET_TOD                = 0x7,
+    TBST_TB_RUNNING             = 0x8,
+    TBST_TB_ERROR               = 0x9,
+};
+
 /*****************************************************************************/
 
 #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index 08a6b47ee0..9c77736e77 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -18,6 +18,7 @@
  */
 #include "qemu/osdep.h"
 #include "cpu.h"
+#include "hw/ppc/ppc.h"
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
 #include "qemu/log.h"
@@ -145,15 +146,218 @@ void helper_store_booke_tsr(CPUPPCState *env, target_ulong val)
 }
 
 #if defined(TARGET_PPC64)
-/* POWER processor Timebase Facility */
+/*
+ * POWER processor Timebase Facility
+ */
+
+/*
+ * The TBST is the timebase state machine, which is a per-core machine that
+ * is used to synchronize the core TB with the ChipTOD. States 3,4,5 are
+ * not used in POWER8/9/10.
+ *
+ * The state machine gets driven by writes to TFMR SPR from the core, and
+ * by signals from the ChipTOD. The state machine table for common
+ * transitions is as follows (according to hardware specs, not necessarily
+ * this implementation):
+ *
+ * | Cur            | Event                            | New |
+ * +----------------+----------------------------------+-----+
+ * | 0 RESET        | TFMR |= LOAD_TOD_MOD             | 1   |
+ * | 1 SEND_TOD_MOD | "immediate transition"           | 2   |
+ * | 2 NOT_SET      | mttbu/mttbu40/mttbl              | 2   |
+ * | 2 NOT_SET      | TFMR |= MOVE_CHIP_TOD_TO_TB      | 6   |
+ * | 6 SYNC_WAIT    | "sync pulse from ChipTOD"        | 7   |
+ * | 7 GET_TOD      | ChipTOD xscom MOVE_TOD_TO_TB_REG | 8   |
+ * | 8 TB_RUNNING   | mttbu/mttbu40                    | 8   |
+ * | 8 TB_RUNNING   | TFMR |= LOAD_TOD_MOD             | 1   |
+ * | 8 TB_RUNNING   | mttbl                            | 9   |
+ * | 9 TB_ERROR     | TFMR |= CLEAR_TB_ERRORS          | 0   |
+ *
+ * - LOAD_TOD_MOD will also move states 2,6 to state 1, omitted from table
+ *   because it's not a typical init flow.
+ *
+ * - The ERROR state can be entered from most/all other states on invalid
+ *   states (e.g., if some TFMR control bit is set from a state where it's
+ *   not listed to cause a transition away from), omitted to avoid clutter.
+ *
+ * Note: mttbl causes a timebase error because this inevitably causes
+ * ticks to be lost and TB to become unsynchronized, whereas TB can be
+ * adjusted using mttbu* without losing ticks. mttbl behaviour is not
+ * modelled.
+ *
+ * Note: the TB state machine does not actually cause any real TB adjustment!
+ * TB starts out synchronized across all vCPUs (hardware threads) in
+ * QMEU, so for now the purpose of the TBST and ChipTOD model is simply
+ * to step through firmware initialisation sequences.
+ */
+static unsigned int tfmr_get_tb_state(uint64_t tfmr)
+{
+    return (tfmr & TFMR_TBST_ENCODED) >> (63 - 31);
+}
+
+static uint64_t tfmr_new_tb_state(uint64_t tfmr, unsigned int tbst)
+{
+    tfmr &= ~TFMR_TBST_LAST;
+    tfmr |= (tfmr & TFMR_TBST_ENCODED) >> 4; /* move state to last state */
+    tfmr &= ~TFMR_TBST_ENCODED;
+    tfmr |= (uint64_t)tbst << (63 - 31); /* move new state to state */
+
+    if (tbst == TBST_TB_RUNNING) {
+        tfmr |= TFMR_TB_VALID;
+    } else {
+        tfmr &= ~TFMR_TB_VALID;
+    }
+
+    return tfmr;
+}
+
+static void tb_state_machine_step(CPUPPCState *env)
+{
+    uint64_t tfmr = env->spr[SPR_TFMR];
+    unsigned int tbst = tfmr_get_tb_state(tfmr);
+
+    if (!(tfmr & TFMR_TB_ECLIPZ) || tbst == TBST_TB_ERROR) {
+        return;
+    }
+
+    if (env->tb_sync_pulse_timer) {
+        env->tb_sync_pulse_timer--;
+    } else {
+        tfmr |= TFMR_TB_SYNC_OCCURED;
+        env->spr[SPR_TFMR] = tfmr;
+    }
+
+    if (env->tb_state_timer) {
+        env->tb_state_timer--;
+        return;
+    }
+
+    if (tfmr & TFMR_LOAD_TOD_MOD) {
+        tfmr &= ~TFMR_LOAD_TOD_MOD;
+        if (tbst == TBST_GET_TOD) {
+            tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
+            tfmr |= TFMR_FIRMWARE_CONTROL_ERROR;
+        } else {
+            tfmr = tfmr_new_tb_state(tfmr, TBST_SEND_TOD_MOD);
+            /* State seems to transition immediately */
+            tfmr = tfmr_new_tb_state(tfmr, TBST_NOT_SET);
+        }
+    } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) {
+        if (tbst == TBST_SYNC_WAIT) {
+            tfmr = tfmr_new_tb_state(tfmr, TBST_GET_TOD);
+            env->tb_state_timer = 3;
+        } else if (tbst == TBST_GET_TOD) {
+            if (env->tod_sent_to_tb) {
+                tfmr = tfmr_new_tb_state(tfmr, TBST_TB_RUNNING);
+                tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB;
+                env->tb_ready_for_tod = 0;
+                env->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->tb_ready_for_tod = 0;
+        }
+    }
+
+    env->spr[SPR_TFMR] = tfmr;
+}
+
 target_ulong helper_load_tfmr(CPUPPCState *env)
 {
-    return env->spr[SPR_TFMR];
+    tb_state_machine_step(env);
+
+    return env->spr[SPR_TFMR] | TFMR_TB_ECLIPZ;
 }
 
 void helper_store_tfmr(CPUPPCState *env, target_ulong val)
 {
-    env->spr[SPR_TFMR] = val;
+    uint64_t tfmr = env->spr[SPR_TFMR];
+    uint64_t clear_on_write;
+    unsigned int tbst = tfmr_get_tb_state(tfmr);
+
+    if (!(val & TFMR_TB_ECLIPZ)) {
+        qemu_log_mask(LOG_UNIMP, "TFMR non-ECLIPZ mode not implemented\n");
+        tfmr &= ~TFMR_TBST_ENCODED;
+        tfmr &= ~TFMR_TBST_LAST;
+        goto out;
+    }
+
+    /* Update control bits */
+    tfmr = (tfmr & ~TFMR_CONTROL_MASK) | (val & TFMR_CONTROL_MASK);
+
+    /* Several bits are clear-on-write, only one is implemented so far */
+    clear_on_write = val & TFMR_FIRMWARE_CONTROL_ERROR;
+    tfmr &= ~clear_on_write;
+
+    /*
+     * mtspr always clears this. The sync pulse timer makes it come back
+     * after the second mfspr.
+     */
+    tfmr &= ~TFMR_TB_SYNC_OCCURED;
+    env->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;
+    }
+
+    if (((tfmr | val) & (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) ==
+                        (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: LOAD_TOD_MOD and "
+                                       "MOVE_CHIP_TOD_TO_TB both set\n");
+        tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
+        tfmr |= TFMR_FIRMWARE_CONTROL_ERROR;
+        env->tb_ready_for_tod = 0;
+        goto out;
+    }
+
+    if (tfmr & TFMR_CLEAR_TB_ERRORS) {
+        /*
+         * Workbook says TFMR_CLEAR_TB_ERRORS should be written twice.
+         * This is not simulated/required here.
+         */
+        tfmr = tfmr_new_tb_state(tfmr, TBST_RESET);
+        tfmr &= ~TFMR_CLEAR_TB_ERRORS;
+        tfmr &= ~TFMR_LOAD_TOD_MOD;
+        tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB;
+        tfmr &= ~TFMR_FIRMWARE_CONTROL_ERROR; /* XXX: should this be cleared? */
+        env->tb_ready_for_tod = 0;
+        env->tod_sent_to_tb = 0;
+        goto out;
+    }
+
+    if (tbst == TBST_TB_ERROR) {
+        qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: mtspr TFMR in TB_ERROR"
+                                       " state\n");
+        tfmr |= TFMR_FIRMWARE_CONTROL_ERROR;
+        return;
+    }
+
+    if (tfmr & TFMR_LOAD_TOD_MOD) {
+        /* Wait for an arbitrary 3 mfspr until the next state transition. */
+        env->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->tb_ready_for_tod = 1;
+            env->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->tb_ready_for_tod = 0;
+        }
+    }
+
+out:
+    env->spr[SPR_TFMR] = tfmr;
 }
 #endif
 
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/7] target/ppc: Add SMT support to time facilities
  2023-11-23 10:30 [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin
                   ` (5 preceding siblings ...)
  2023-11-23 10:30 ` [PATCH 6/7] target/ppc: Implement core timebase state machine and TFMR Nicholas Piggin
@ 2023-11-23 10:30 ` Nicholas Piggin
  2023-11-23 17:09 ` [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Cédric Le Goater
  7 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-23 10:30 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	Frédéric Barrat, qemu-devel

The TB, VTB, PURR, HDEC SPRs are per-LPAR registers, and the TFMR is a
per-core register. Add the necessary SMT helpers.

The TFMR can only drive the timebase state machine via thread 0 of the
core, which is almost certainly not right, but it is enough for skiboot
and other proprietary firmware.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/timebase_helper.c | 105 ++++++++++++++++++++++++++++++++---
 target/ppc/translate.c       |  42 +++++++++++++-
 2 files changed, 136 insertions(+), 11 deletions(-)

diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index 9c77736e77..c3fc194b1e 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -60,19 +60,55 @@ target_ulong helper_load_purr(CPUPPCState *env)
 
 void helper_store_purr(CPUPPCState *env, target_ulong val)
 {
-    cpu_ppc_store_purr(env, val);
+    CPUState *cs = env_cpu(env);
+    CPUState *ccs;
+    uint32_t nr_threads = cs->nr_threads;
+
+    if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) {
+        cpu_ppc_store_purr(env, val);
+        return;
+    }
+
+    THREAD_SIBLING_FOREACH(cs, ccs) {
+        CPUPPCState *cenv = &POWERPC_CPU(ccs)->env;
+        cpu_ppc_store_purr(cenv, val);
+    }
 }
 #endif
 
 #if !defined(CONFIG_USER_ONLY)
 void helper_store_tbl(CPUPPCState *env, target_ulong val)
 {
-    cpu_ppc_store_tbl(env, val);
+    CPUState *cs = env_cpu(env);
+    CPUState *ccs;
+    uint32_t nr_threads = cs->nr_threads;
+
+    if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) {
+        cpu_ppc_store_tbl(env, val);
+        return;
+    }
+
+    THREAD_SIBLING_FOREACH(cs, ccs) {
+        CPUPPCState *cenv = &POWERPC_CPU(ccs)->env;
+        cpu_ppc_store_tbl(cenv, val);
+    }
 }
 
 void helper_store_tbu(CPUPPCState *env, target_ulong val)
 {
-    cpu_ppc_store_tbu(env, val);
+    CPUState *cs = env_cpu(env);
+    CPUState *ccs;
+    uint32_t nr_threads = cs->nr_threads;
+
+    if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) {
+        cpu_ppc_store_tbu(env, val);
+        return;
+    }
+
+    THREAD_SIBLING_FOREACH(cs, ccs) {
+        CPUPPCState *cenv = &POWERPC_CPU(ccs)->env;
+        cpu_ppc_store_tbu(cenv, val);
+    }
 }
 
 void helper_store_atbl(CPUPPCState *env, target_ulong val)
@@ -102,17 +138,53 @@ target_ulong helper_load_hdecr(CPUPPCState *env)
 
 void helper_store_hdecr(CPUPPCState *env, target_ulong val)
 {
-    cpu_ppc_store_hdecr(env, val);
+    CPUState *cs = env_cpu(env);
+    CPUState *ccs;
+    uint32_t nr_threads = cs->nr_threads;
+
+    if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) {
+        cpu_ppc_store_hdecr(env, val);
+        return;
+    }
+
+    THREAD_SIBLING_FOREACH(cs, ccs) {
+        CPUPPCState *cenv = &POWERPC_CPU(ccs)->env;
+        cpu_ppc_store_hdecr(cenv, val);
+    }
 }
 
 void helper_store_vtb(CPUPPCState *env, target_ulong val)
 {
-    cpu_ppc_store_vtb(env, val);
+    CPUState *cs = env_cpu(env);
+    CPUState *ccs;
+    uint32_t nr_threads = cs->nr_threads;
+
+    if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) {
+        cpu_ppc_store_vtb(env, val);
+        return;
+    }
+
+    THREAD_SIBLING_FOREACH(cs, ccs) {
+        CPUPPCState *cenv = &POWERPC_CPU(ccs)->env;
+        cpu_ppc_store_vtb(cenv, val);
+    }
 }
 
 void helper_store_tbu40(CPUPPCState *env, target_ulong val)
 {
-    cpu_ppc_store_tbu40(env, val);
+    CPUState *cs = env_cpu(env);
+    CPUState *ccs;
+    uint32_t nr_threads = cs->nr_threads;
+
+    if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) {
+        cpu_ppc_store_tbu40(env, val);
+        return;
+    }
+
+    THREAD_SIBLING_FOREACH(cs, ccs) {
+        CPUPPCState *cenv = &POWERPC_CPU(ccs)->env;
+        cpu_ppc_store_tbu40(cenv, val);
+    }
 }
 
 target_ulong helper_load_40x_pit(CPUPPCState *env)
@@ -211,6 +283,21 @@ static uint64_t tfmr_new_tb_state(uint64_t tfmr, unsigned int tbst)
     return tfmr;
 }
 
+static void write_tfmr(CPUPPCState *env, target_ulong val)
+{
+    CPUState *cs = env_cpu(env);
+
+    if (cs->nr_threads == 1) {
+        env->spr[SPR_TFMR] = val;
+    } else {
+        CPUState *ccs;
+        THREAD_SIBLING_FOREACH(cs, ccs) {
+            CPUPPCState *cenv = &POWERPC_CPU(ccs)->env;
+            cenv->spr[SPR_TFMR] = val;
+        }
+    }
+}
+
 static void tb_state_machine_step(CPUPPCState *env)
 {
     uint64_t tfmr = env->spr[SPR_TFMR];
@@ -224,7 +311,7 @@ static void tb_state_machine_step(CPUPPCState *env)
         env->tb_sync_pulse_timer--;
     } else {
         tfmr |= TFMR_TB_SYNC_OCCURED;
-        env->spr[SPR_TFMR] = tfmr;
+        write_tfmr(env, tfmr);
     }
 
     if (env->tb_state_timer) {
@@ -262,7 +349,7 @@ static void tb_state_machine_step(CPUPPCState *env)
         }
     }
 
-    env->spr[SPR_TFMR] = tfmr;
+    write_tfmr(env, tfmr);
 }
 
 target_ulong helper_load_tfmr(CPUPPCState *env)
@@ -357,7 +444,7 @@ void helper_store_tfmr(CPUPPCState *env, target_ulong val)
     }
 
 out:
-    env->spr[SPR_TFMR] = tfmr;
+    write_tfmr(env, tfmr);
 }
 #endif
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 329da4d518..bd103b1026 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -247,13 +247,24 @@ static inline bool gen_serialize(DisasContext *ctx)
     return true;
 }
 
-#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+#if !defined(CONFIG_USER_ONLY)
+#if defined(TARGET_PPC64)
+static inline bool gen_serialize_core(DisasContext *ctx)
+{
+    if (ctx->flags & POWERPC_FLAG_SMT) {
+        return gen_serialize(ctx);
+    }
+    return true;
+}
+#endif
+
 static inline bool gen_serialize_core_lpar(DisasContext *ctx)
 {
+#if defined(TARGET_PPC64)
     if (ctx->flags & POWERPC_FLAG_SMT_1LPAR) {
         return gen_serialize(ctx);
     }
-
+#endif
     return true;
 }
 #endif
@@ -667,12 +678,20 @@ void spr_read_atbu(DisasContext *ctx, int gprn, int sprn)
 #if !defined(CONFIG_USER_ONLY)
 void spr_write_tbl(DisasContext *ctx, int sprn, int gprn)
 {
+    if (!gen_serialize_core_lpar(ctx)) {
+        return;
+    }
+
     translator_io_start(&ctx->base);
     gen_helper_store_tbl(tcg_env, cpu_gpr[gprn]);
 }
 
 void spr_write_tbu(DisasContext *ctx, int sprn, int gprn)
 {
+    if (!gen_serialize_core_lpar(ctx)) {
+        return;
+    }
+
     translator_io_start(&ctx->base);
     gen_helper_store_tbu(tcg_env, cpu_gpr[gprn]);
 }
@@ -696,6 +715,9 @@ void spr_read_purr(DisasContext *ctx, int gprn, int sprn)
 
 void spr_write_purr(DisasContext *ctx, int sprn, int gprn)
 {
+    if (!gen_serialize_core_lpar(ctx)) {
+        return;
+    }
     translator_io_start(&ctx->base);
     gen_helper_store_purr(tcg_env, cpu_gpr[gprn]);
 }
@@ -709,6 +731,9 @@ void spr_read_hdecr(DisasContext *ctx, int gprn, int sprn)
 
 void spr_write_hdecr(DisasContext *ctx, int sprn, int gprn)
 {
+    if (!gen_serialize_core_lpar(ctx)) {
+        return;
+    }
     translator_io_start(&ctx->base);
     gen_helper_store_hdecr(tcg_env, cpu_gpr[gprn]);
 }
@@ -721,12 +746,18 @@ void spr_read_vtb(DisasContext *ctx, int gprn, int sprn)
 
 void spr_write_vtb(DisasContext *ctx, int sprn, int gprn)
 {
+    if (!gen_serialize_core_lpar(ctx)) {
+        return;
+    }
     translator_io_start(&ctx->base);
     gen_helper_store_vtb(tcg_env, cpu_gpr[gprn]);
 }
 
 void spr_write_tbu40(DisasContext *ctx, int sprn, int gprn)
 {
+    if (!gen_serialize_core_lpar(ctx)) {
+        return;
+    }
     translator_io_start(&ctx->base);
     gen_helper_store_tbu40(tcg_env, cpu_gpr[gprn]);
 }
@@ -1220,11 +1251,18 @@ void spr_write_hmer(DisasContext *ctx, int sprn, int gprn)
 
 void spr_read_tfmr(DisasContext *ctx, int gprn, int sprn)
 {
+    /* Reading TFMR can cause it to be updated, so serialize threads here too */
+    if (!gen_serialize_core(ctx)) {
+        return;
+    }
     gen_helper_load_tfmr(cpu_gpr[gprn], tcg_env);
 }
 
 void spr_write_tfmr(DisasContext *ctx, int sprn, int gprn)
 {
+    if (!gen_serialize_core(ctx)) {
+        return;
+    }
     gen_helper_store_tfmr(tcg_env, cpu_gpr[gprn]);
 }
 
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes
  2023-11-23 10:30 [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin
                   ` (6 preceding siblings ...)
  2023-11-23 10:30 ` [PATCH 7/7] target/ppc: Add SMT support to time facilities Nicholas Piggin
@ 2023-11-23 17:09 ` Cédric Le Goater
  2023-11-24  6:02   ` Nicholas Piggin
  7 siblings, 1 reply; 15+ messages in thread
From: Cédric Le Goater @ 2023-11-23 17:09 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: Daniel Henrique Barboza, Frédéric Barrat, qemu-devel

On 11/23/23 11:30, Nicholas Piggin wrote:
> The chiptod/TFMR/state machine is not really tied to the other
> time register fixes, but they touch some of the same code, and
> logically same facility.
> 
> Changes since v1 of chiptod patches:
> - Split hackish ChipTOD<->TFMR/TBST interface into its own patch
> - Fix multi-socket addressing on P9 / chip ID mode (P10 works)
> - Change chiptod primary/secondary setting to use class properties
> - Add more comments to explain TOD overview and timebase state
>    machine.
> - SMT support for TFMR, some functionality is limited to thread 0.
> - FIRMWARE_CONTROL_ERROR bit implemented in TFMR.
> - Misc cleanups and bug fixes.
> 
> The hacky part, addressing core from chiptod, is still hacky. Is
> there strong objection to it?

Dunno yet :)

> This successfully runs skiboot chiptod initialisation code with
> POWER9 and POWER10 multi-socket, multi-core, SMT. That requires
> skiboot 7.1 (not in-tree), otherwise chiptod init is skipped on
> QEMU machines.

Let's update skiboot at the same time then.

Thanks,

C.

> 
> Thanks,
> Nick
> 
> Nicholas Piggin (7):
>    target/ppc: Rename TBL to TB on 64-bit
>    target/ppc: Improve timebase register defines naming
>    target/ppc: Fix move-to timebase SPR access permissions
>    pnv/chiptod: Add POWER9/10 chiptod model
>    pnv/chiptod: Implement the ChipTOD to Core transfer
>    target/ppc: Implement core timebase state machine and TFMR
>    target/ppc: Add SMT support to time facilities
> 
>   include/hw/ppc/pnv_chip.h    |   3 +
>   include/hw/ppc/pnv_chiptod.h |  55 ++++
>   include/hw/ppc/pnv_core.h    |   4 +
>   include/hw/ppc/pnv_xscom.h   |   9 +
>   target/ppc/cpu.h             |  50 +++-
>   hw/ppc/pnv.c                 |  63 +++++
>   hw/ppc/pnv_chiptod.c         | 509 +++++++++++++++++++++++++++++++++++
>   target/ppc/helper_regs.c     |  39 ++-
>   target/ppc/ppc-qmp-cmds.c    |   4 +
>   target/ppc/timebase_helper.c | 309 ++++++++++++++++++++-
>   target/ppc/translate.c       |  42 ++-
>   hw/ppc/meson.build           |   1 +
>   hw/ppc/trace-events          |   4 +
>   13 files changed, 1067 insertions(+), 25 deletions(-)
>   create mode 100644 include/hw/ppc/pnv_chiptod.h
>   create mode 100644 hw/ppc/pnv_chiptod.c
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model
  2023-11-23 10:30 ` [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model Nicholas Piggin
@ 2023-11-23 17:49   ` Cédric Le Goater
  2023-11-24  6:14     ` Nicholas Piggin
  0 siblings, 1 reply; 15+ messages in thread
From: Cédric Le Goater @ 2023-11-23 17:49 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: Daniel Henrique Barboza, Frédéric Barrat, qemu-devel

On 11/23/23 11:30, Nicholas Piggin wrote:
> The ChipTOD (for Time-Of-Day) is a pervasive facility that keeps the
> clocks on multiple chips consistent which can keep TOD (time-of-day),
> synchronise it across multiple chips, and can move that TOD to or from
> the core timebase units.

May be rephrase a bit the sentence above. I find it difficult to read.

> This driver implements basic emulation of chiptod registers sufficient

it's a model.

> to successfully run the skiboot chiptod synchronisation procedure
> (with the following TFMR and timebase state-machine implementation).
> 
> The main way chiptod affects the rest of the system (relevant to the
> powernv model) is to interact with the timebase facility in the cores,
> influencing the timebase state machine and registers.
> 
> The way this chiptod driver implements that interaction is with two
> new flags in the CPUPPCState env, one is use for the core timebase to
> indicate it is ready to receive a TOD from chiptod, the other used
> by chiptod to indicate that it has sent TOD to the core timebase. The
> core timebase will be implemented in later changes.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

LGTM,

> ---
>   include/hw/ppc/pnv_chip.h    |   3 +
>   include/hw/ppc/pnv_chiptod.h |  52 +++++
>   include/hw/ppc/pnv_xscom.h   |   9 +
>   hw/ppc/pnv.c                 |  30 +++
>   hw/ppc/pnv_chiptod.c         | 417 +++++++++++++++++++++++++++++++++++
>   hw/ppc/meson.build           |   1 +
>   hw/ppc/trace-events          |   4 +
>   7 files changed, 516 insertions(+)
>   create mode 100644 include/hw/ppc/pnv_chiptod.h
>   create mode 100644 hw/ppc/pnv_chiptod.c
> 
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> index 0ab5c42308..bfc4772cf3 100644
> --- a/include/hw/ppc/pnv_chip.h
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -2,6 +2,7 @@
>   #define PPC_PNV_CHIP_H
>   
>   #include "hw/pci-host/pnv_phb4.h"
> +#include "hw/ppc/pnv_chiptod.h"
>   #include "hw/ppc/pnv_core.h"
>   #include "hw/ppc/pnv_homer.h"
>   #include "hw/ppc/pnv_lpc.h"
> @@ -78,6 +79,7 @@ struct Pnv9Chip {
>       PnvXive      xive;
>       Pnv9Psi      psi;
>       PnvLpcController lpc;
> +    PnvChipTOD   chiptod;
>       PnvOCC       occ;
>       PnvSBE       sbe;
>       PnvHomer     homer;
> @@ -110,6 +112,7 @@ struct Pnv10Chip {
>       PnvXive2     xive;
>       Pnv9Psi      psi;
>       PnvLpcController lpc;
> +    PnvChipTOD   chiptod;
>       PnvOCC       occ;
>       PnvSBE       sbe;
>       PnvHomer     homer;
>
> diff --git a/include/hw/ppc/pnv_chiptod.h b/include/hw/ppc/pnv_chiptod.h
> new file mode 100644
> index 0000000000..e2765c3bfc
> --- /dev/null
> +++ b/include/hw/ppc/pnv_chiptod.h
> @@ -0,0 +1,52 @@
> +/*
> + * QEMU PowerPC PowerNV Emulation of some CHIPTOD behaviour
> + *
> + * Copyright (c) 2022-2023, IBM Corporation.
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +
> +#ifndef PPC_PNV_CHIPTOD_H
> +#define PPC_PNV_CHIPTOD_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_PNV_CHIPTOD "pnv-chiptod"
> +OBJECT_DECLARE_TYPE(PnvChipTOD, PnvChipTODClass, PNV_CHIPTOD)
> +#define TYPE_PNV9_CHIPTOD TYPE_PNV_CHIPTOD "-POWER9"
> +DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV9_CHIPTOD, TYPE_PNV9_CHIPTOD)
> +#define TYPE_PNV10_CHIPTOD TYPE_PNV_CHIPTOD "-POWER10"
> +DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV10_CHIPTOD, TYPE_PNV10_CHIPTOD)
> +
> +enum tod_state {
> +    tod_error = 0,
> +    tod_not_set = 7,
> +    tod_not_set_step = 11,
> +    tod_running = 2,
> +    tod_running_step = 10,
> +    tod_running_sync = 14,
> +    tod_wait_for_sync = 13,
> +    tod_stopped = 1,
> +};
> +
> +struct PnvChipTOD {
> +    DeviceState xd;
> +
> +    PnvChip *chip;
> +    MemoryRegion xscom_regs;
> +
> +    bool primary;
> +    bool secondary;
> +    enum tod_state tod_state;
> +    uint64_t tod_error;
> +    uint64_t pss_mss_ctrl_reg;
> +};
> +
> +struct PnvChipTODClass {
> +    DeviceClass parent_class;
> +
> +    int xscom_size;
> +    const MemoryRegionOps *xscom_ops;
> +};
> +
> +#endif /* PPC_PNV_CHIPTOD_H */
> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> index f5becbab41..6aa3ac745d 100644
> --- a/include/hw/ppc/pnv_xscom.h
> +++ b/include/hw/ppc/pnv_xscom.h
> @@ -64,6 +64,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV_XSCOM_PSIHB_BASE      0x2010900
>   #define PNV_XSCOM_PSIHB_SIZE      0x20
>   
> +#define PNV_XSCOM_CHIPTOD_BASE    0x0040000
> +#define PNV_XSCOM_CHIPTOD_SIZE    0x31
> +
>   #define PNV_XSCOM_OCC_BASE        0x0066000
>   #define PNV_XSCOM_OCC_SIZE        0x6000
>   
> @@ -93,6 +96,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV9_XSCOM_I2CM_BASE      0xa0000
>   #define PNV9_XSCOM_I2CM_SIZE      0x1000
>   
> +#define PNV9_XSCOM_CHIPTOD_BASE   PNV_XSCOM_CHIPTOD_BASE
> +#define PNV9_XSCOM_CHIPTOD_SIZE   PNV_XSCOM_CHIPTOD_SIZE
> +
>   #define PNV9_XSCOM_OCC_BASE       PNV_XSCOM_OCC_BASE
>   #define PNV9_XSCOM_OCC_SIZE       0x8000
>   
> @@ -155,6 +161,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV10_XSCOM_I2CM_BASE      PNV9_XSCOM_I2CM_BASE
>   #define PNV10_XSCOM_I2CM_SIZE      PNV9_XSCOM_I2CM_SIZE
>   
> +#define PNV10_XSCOM_CHIPTOD_BASE   PNV9_XSCOM_CHIPTOD_BASE
> +#define PNV10_XSCOM_CHIPTOD_SIZE   PNV9_XSCOM_CHIPTOD_SIZE
> +
>   #define PNV10_XSCOM_OCC_BASE       PNV9_XSCOM_OCC_BASE
>   #define PNV10_XSCOM_OCC_SIZE       PNV9_XSCOM_OCC_SIZE
>   
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 0297871bdd..546266ae3d 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1419,6 +1419,8 @@ static void pnv_chip_power9_instance_init(Object *obj)
>   
>       object_initialize_child(obj, "lpc", &chip9->lpc, TYPE_PNV9_LPC);
>   
> +    object_initialize_child(obj, "chiptod", &chip9->chiptod, TYPE_PNV9_CHIPTOD);
> +
>       object_initialize_child(obj, "occ", &chip9->occ, TYPE_PNV9_OCC);
>   
>       object_initialize_child(obj, "sbe", &chip9->sbe, TYPE_PNV9_SBE);
> @@ -1565,6 +1567,19 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
>                                               (uint64_t) PNV9_LPCM_BASE(chip));
>   
> +    /* ChipTOD */
> +    object_property_set_bool(OBJECT(&chip9->chiptod), "primary",
> +                             chip->chip_id == 0, &error_abort);
> +    object_property_set_bool(OBJECT(&chip9->chiptod), "secondary",
> +                             chip->chip_id == 1, &error_abort);
> +    object_property_set_link(OBJECT(&chip9->chiptod), "chip", OBJECT(chip),
> +                             &error_abort);
> +    if (!qdev_realize(DEVICE(&chip9->chiptod), NULL, errp)) {
> +        return;
> +    }
> +    pnv_xscom_add_subregion(chip, PNV9_XSCOM_CHIPTOD_BASE,
> +                            &chip9->chiptod.xscom_regs);
> +
>       /* Create the simplified OCC model */
>       if (!qdev_realize(DEVICE(&chip9->occ), NULL, errp)) {
>           return;
> @@ -1677,6 +1692,8 @@ static void pnv_chip_power10_instance_init(Object *obj)
>                                 "xive-fabric");
>       object_initialize_child(obj, "psi", &chip10->psi, TYPE_PNV10_PSI);
>       object_initialize_child(obj, "lpc", &chip10->lpc, TYPE_PNV10_LPC);
> +    object_initialize_child(obj, "chiptod", &chip10->chiptod,
> +                            TYPE_PNV10_CHIPTOD);
>       object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
>       object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
>       object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
> @@ -1810,6 +1827,19 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
>                                               (uint64_t) PNV10_LPCM_BASE(chip));
>   
> +    /* ChipTOD */
> +    object_property_set_bool(OBJECT(&chip10->chiptod), "primary",
> +                             chip->chip_id == 0, &error_abort);
> +    object_property_set_bool(OBJECT(&chip10->chiptod), "secondary",
> +                             chip->chip_id == 1, &error_abort);
> +    object_property_set_link(OBJECT(&chip10->chiptod), "chip", OBJECT(chip),
> +                             &error_abort);
> +    if (!qdev_realize(DEVICE(&chip10->chiptod), NULL, errp)) {
> +        return;
> +    }
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_CHIPTOD_BASE,
> +                            &chip10->chiptod.xscom_regs);
> +
>       /* Create the simplified OCC model */
>       if (!qdev_realize(DEVICE(&chip10->occ), NULL, errp)) {
>           return;
> diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
> new file mode 100644
> index 0000000000..a7bfe4298d
> --- /dev/null
> +++ b/hw/ppc/pnv_chiptod.c
> @@ -0,0 +1,417 @@
> +/*
> + * QEMU PowerPC PowerNV Emulation of some ChipTOD behaviour
> + *
> + * Copyright (c) 2022-2023, IBM Corporation.
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + *
> + * ChipTOD (aka TOD) is a facility implemented in the nest / pervasive. The
> + * purpose is to keep time-of-day across chips and cores.
> + *
> + * There is a master chip TOD, which sends signals to slave chip TODs to
> + * keep them synchronized. There are two sets of configuration registers
> + * called primary and secondary, which can be used fail over.
> + *
> + * The chip TOD also distributes synchronisation signals to the timebase
> + * facility in each of the cores on the chip. In particular there is a
> + * feature that can move the TOD value in the ChipTOD to and from the TB.
> + *
> + * Initialisation typically brings all ChipTOD into sync (see tod_state),
> + * and then brings each core TB into sync with the ChipTODs (see timebase
> + * state and TFMR). This model is a very basic simulation of the init sequence
> + * performed by skiboot.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "target/ppc/cpu.h"
> +#include "qapi/error.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "hw/irq.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/ppc/fdt.h"
> +#include "hw/ppc/ppc.h"
> +#include "hw/ppc/pnv.h"
> +#include "hw/ppc/pnv_chip.h"
> +#include "hw/ppc/pnv_core.h"
> +#include "hw/ppc/pnv_xscom.h"
> +#include "hw/ppc/pnv_chiptod.h"
> +#include "trace.h"
> +
> +#include <libfdt.h>
> +
> +/* TOD chip XSCOM addresses */
> +#define TOD_M_PATH_CTRL_REG             0x00000000 /* Master Path ctrl reg */
> +#define TOD_PRI_PORT_0_CTRL_REG         0x00000001 /* Primary port0 ctrl reg */
> +#define TOD_PRI_PORT_1_CTRL_REG         0x00000002 /* Primary port1 ctrl reg */
> +#define TOD_SEC_PORT_0_CTRL_REG         0x00000003 /* Secondary p0 ctrl reg */
> +#define TOD_SEC_PORT_1_CTRL_REG         0x00000004 /* Secondary p1 ctrl reg */
> +#define TOD_S_PATH_CTRL_REG             0x00000005 /* Slave Path ctrl reg */
> +#define TOD_I_PATH_CTRL_REG             0x00000006 /* Internal Path ctrl reg */
> +
> +/* -- TOD primary/secondary master/slave control register -- */
> +#define TOD_PSS_MSS_CTRL_REG            0x00000007
> +
> +/* -- TOD primary/secondary master/slave status register -- */
> +#define TOD_PSS_MSS_STATUS_REG          0x00000008
> +
> +/* TOD chip XSCOM addresses */
> +#define TOD_CHIP_CTRL_REG               0x00000010 /* Chip control reg */
> +
> +#define TOD_TX_TTYPE_0_REG              0x00000011
> +#define TOD_TX_TTYPE_1_REG              0x00000012 /* PSS switch reg */
> +#define TOD_TX_TTYPE_2_REG              0x00000013 /* Enable step checkers */
> +#define TOD_TX_TTYPE_3_REG              0x00000014 /* Request TOD reg */
> +#define TOD_TX_TTYPE_4_REG              0x00000015 /* Send TOD reg */
> +#define TOD_TX_TTYPE_5_REG              0x00000016 /* Invalidate TOD reg */
> +
> +#define TOD_MOVE_TOD_TO_TB_REG          0x00000017
> +#define TOD_LOAD_TOD_MOD_REG            0x00000018
> +#define TOD_LOAD_TOD_REG                0x00000021
> +#define TOD_FSM_REG                     0x00000024
> +
> +#define TOD_TX_TTYPE_CTRL_REG           0x00000027 /* TX TTYPE Control reg */
> +#define   TOD_TX_TTYPE_PIB_SLAVE_ADDR      PPC_BITMASK(26, 31)
> +
> +/* -- TOD Error interrupt register -- */
> +#define TOD_ERROR_REG                   0x00000030
> +
> +/* PC unit PIB address which recieves the timebase transfer from TOD */
> +#define   PC_TOD                        0x4A3
> +
> +static uint64_t pnv_chiptod_xscom_read(void *opaque, hwaddr addr,
> +                                          unsigned size)
> +{
> +    PnvChipTOD *chiptod = PNV_CHIPTOD(opaque);
> +    uint32_t offset = addr >> 3;
> +    uint64_t val = 0;
> +
> +    switch (offset) {
> +    case TOD_PSS_MSS_STATUS_REG:
> +        /*
> +         * ChipTOD does not support configurations other than primary
> +         * master, does not support errors, etc.
> +         */
> +        val |= PPC_BITMASK(6, 10); /* STEP checker validity */
> +        val |= PPC_BIT(12); /* Primary config master path select */
> +        val |= PPC_BIT(20); /* Is running */
> +        val |= PPC_BIT(21); /* Is using primary config */
> +        val |= PPC_BIT(26); /* Is using master path select */
> +
> +        if (chiptod->primary) {
> +            val |= PPC_BIT(23); /* Is active master */
> +        } else if (chiptod->secondary) {
> +            val |= PPC_BIT(24); /* Is backup master */
> +        }
> +        break;
> +    case TOD_PSS_MSS_CTRL_REG:
> +        val = chiptod->pss_mss_ctrl_reg;
> +        break;
> +    case TOD_TX_TTYPE_CTRL_REG:
> +        val = 0;
> +        break;
> +    case TOD_ERROR_REG:
> +        val = chiptod->tod_error;
> +        break;
> +    case TOD_FSM_REG:
> +        if (chiptod->tod_state == tod_running) {
> +            val |= PPC_BIT(4);
> +        }
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: Ox%"
> +                      HWADDR_PRIx "\n", addr >> 3);
> +    }
> +
> +    trace_pnv_chiptod_xscom_read(addr >> 3, val);
> +
> +    return val;
> +}
> +
> +static void chiptod_power9_send_remotes(PnvChipTOD *chiptod)
> +{
> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());

Using qdev_get_machine() in a model is always a bit annoying. There is
work in progress currently to have a single QEMU binary for all arches
and when done, the "machine" would encompass something bigger including
the OCC, BMC, etc.

Do we know how XSCOM propagates the new state to the chiptop on other
chips ? is it some sort of broadcast on the bus ? Could we model it ?
I am only asking, not to be done now.


> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
> +        if (&chip9->chiptod != chiptod) {
> +            chip9->chiptod.tod_state = tod_running;
> +        }
> +    }
> +}
> +
> +static void chiptod_power10_send_remotes(PnvChipTOD *chiptod)
> +{
> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
> +        if (&chip10->chiptod != chiptod) {
> +            chip10->chiptod.tod_state = tod_running;
> +        }
> +    }
> +}
> +
> +static void chiptod_power9_invalidate_remotes(PnvChipTOD *chiptod)
> +{
> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
> +        if (&chip9->chiptod != chiptod) {
> +            chip9->chiptod.tod_state = tod_not_set;
> +        }
> +    }
> +}
> +
> +static void chiptod_power10_invalidate_remotes(PnvChipTOD *chiptod)
> +{
> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
> +        if (&chip10->chiptod != chiptod) {
> +            chip10->chiptod.tod_state = tod_not_set;
> +        }
> +    }
> +}

Could we avoid 4 routines doing the same thing and introduce :

   chiptod_power*_set_tod_state(PnvChipTOD *chiptod, enum tod_state new)

?

We could even introcude a PnvChipClass::get_chiptod handler. Might be
overkill though.


> +
> +static void pnv_chiptod_xscom_write(void *opaque, hwaddr addr,
> +                                    uint64_t val, unsigned size,
> +                                    bool is_power9)
> +{
> +    PnvChipTOD *chiptod = PNV_CHIPTOD(opaque);
> +    uint32_t offset = addr >> 3;
> +
> +    trace_pnv_chiptod_xscom_write(addr >> 3, val);
> +
> +    switch (offset) {
> +    case TOD_PSS_MSS_CTRL_REG:
> +        /* Is this correct? */
> +        if (chiptod->primary) {
> +            val |= PPC_BIT(1); /* TOD is master */
> +        } else {
> +            val &= ~PPC_BIT(1);
> +        }
> +        val |= PPC_BIT(2); /* Drawer is master (don't simulate multi-drawer) */
> +        chiptod->pss_mss_ctrl_reg = val & PPC_BITMASK(0, 31);
> +        break;
> +
> +    case TOD_ERROR_REG:
> +        chiptod->tod_error &= ~val;
> +        break;
> +    case TOD_LOAD_TOD_MOD_REG:
> +        if (!(val & PPC_BIT(0))) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> +                          " TOD_LOAD_TOD_MOD_REG with bad val 0x%016lx\n", val);

Please use PRIx64.

> +        } else {
> +            chiptod->tod_state = tod_not_set;
> +        }
> +        break;
> +    case TOD_LOAD_TOD_REG:
> +        chiptod->tod_state = tod_running;
> +        break;
> +    case TOD_TX_TTYPE_4_REG:
> +        if (is_power9) {
> +            chiptod_power9_send_remotes(chiptod);
> +        } else {
> +            chiptod_power10_send_remotes(chiptod);
> +        }
> +        break;
> +    case TOD_TX_TTYPE_5_REG:
> +        if (is_power9) {
> +            chiptod_power9_invalidate_remotes(chiptod);
> +        } else {
> +            chiptod_power10_invalidate_remotes(chiptod);
> +        }

With PnvChipTODClass::invalidate_remotes and PnvChipTODClass::send_remotes
handlers, or ::set_state, this routine would not need a 'is_power9' parameter
and the model would not need 2 different xscom_ops. Can it be done ?

> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: Ox%"
> +                      HWADDR_PRPnvChipTODClassIx "\n", addr >> 3);
> +    }
> +}
> +
> +static void pnv_chiptod_power9_xscom_write(void *opaque, hwaddr addr,
> +                                           uint64_t val, unsigned size)
> +{
> +    pnv_chiptod_xscom_write(opaque, addr, val, size, true);
> +}
> +
> +static const MemoryRegionOps pnv_chiptod_power9_xscom_ops = {
> +    .read = pnv_chiptod_xscom_read,
> +    .write = pnv_chiptod_power9_xscom_write,
> +    .valid.min_access_size = 8,
> +    .valid.max_access_size = 8,
> +    .impl.min_access_size = 8,
> +    .impl.max_access_size = 8,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static int pnv_chiptod_dt_xscom(PnvXScomInterface *dev, void *fdt,
> +                                int xscom_offset,
> +                                const char compat[], size_t compat_size)
> +{
> +    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
> +    g_autofree char *name = NULL;
> +    int offset;
> +    uint32_t lpc_pcba = PNV9_XSCOM_CHIPTOD_BASE;

chiptod_pcba may be ?

> +    uint32_t reg[] = {
> +        cpu_to_be32(lpc_pcba),
> +        cpu_to_be32(PNV9_XSCOM_CHIPTOD_SIZE)
> +    };
> +
> +    name = g_strdup_printf("chiptod@%x", lpc_pcba);
> +    offset = fdt_add_subnode(fdt, xscom_offset, name);
> +    _FDT(offset);
> +
> +    if (chiptod->primary) {
> +        _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
> +    } else if (chiptod->secondary) {
> +        _FDT((fdt_setprop(fdt, offset, "secondary", NULL, 0)));
> +    }
> +
> +    _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
> +    _FDT((fdt_setprop(fdt, offset, "compatible", compat, compat_size)));
> +    return 0;
> +}
> +
> +static int pnv_chiptod_power9_dt_xscom(PnvXScomInterface *dev, void *fdt,
> +                             int xscom_offset)
> +{
> +    const char compat[] = "ibm,power-chiptod\0ibm,power9-chiptod";
> +
> +    return pnv_chiptod_dt_xscom(dev, fdt, xscom_offset, compat, sizeof(compat));
> +}
> +
> +static Property pnv_chiptod_properties[] = {
> +    DEFINE_PROP_BOOL("primary", PnvChipTOD, primary, false),
> +    DEFINE_PROP_BOOL("secondary", PnvChipTOD, secondary, false),
> +    DEFINE_PROP_LINK("chip", PnvChipTOD , chip, TYPE_PNV_CHIP, PnvChip *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pnv_chiptod_power9_class_init(ObjectClass *klass, void *data)
> +{
> +    PnvChipTODClass *pctc = PNV_CHIPTOD_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
> +
> +    dc->desc = "PowerNV ChipTOD Controller (POWER9)";
> +    device_class_set_props(dc, pnv_chiptod_properties);
> +
> +    xdc->dt_xscom = pnv_chiptod_power9_dt_xscom;
> +
> +    pctc->xscom_size = PNV_XSCOM_CHIPTOD_SIZE;
> +    pctc->xscom_ops = &pnv_chiptod_power9_xscom_ops;
> +}
> +
> +static const TypeInfo pnv_chiptod_power9_type_info = {
> +    .name          = TYPE_PNV9_CHIPTOD,
> +    .parent        = TYPE_PNV_CHIPTOD,
> +    .instance_size = sizeof(PnvChipTOD),
> +    .class_init    = pnv_chiptod_power9_class_init,
> +    .interfaces    = (InterfaceInfo[]) {
> +        { TYPE_PNV_XSCOM_INTERFACE },
> +        { }
> +    }
> +};
> +
> +static void pnv_chiptod_power10_xscom_write(void *opaque, hwaddr addr,
> +                                           uint64_t val, unsigned size)
> +{
> +    pnv_chiptod_xscom_write(opaque, addr, val, size, false);
> +}
> +
> +static const MemoryRegionOps pnv_chiptod_power10_xscom_ops = {
> +    .read = pnv_chiptod_xscom_read,
> +    .write = pnv_chiptod_power10_xscom_write,
> +    .valid.min_access_size = 8,
> +    .valid.max_access_size = 8,
> +    .impl.min_access_size = 8,
> +    .impl.max_access_size = 8,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static int pnv_chiptod_power10_dt_xscom(PnvXScomInterface *dev, void *fdt,
> +                             int xscom_offset)
> +{
> +    const char compat[] = "ibm,power-chiptod\0ibm,power10-chiptod";
> +
> +    return pnv_chiptod_dt_xscom(dev, fdt, xscom_offset, compat, sizeof(compat));
> +}
> +
> +static void pnv_chiptod_power10_class_init(ObjectClass *klass, void *data)
> +{
> +    PnvChipTODClass *pctc = PNV_CHIPTOD_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
> +
> +    dc->desc = "PowerNV ChipTOD Controller (POWER10)";
> +    device_class_set_props(dc, pnv_chiptod_properties);
> +
> +    xdc->dt_xscom = pnv_chiptod_power10_dt_xscom;
> +
> +    pctc->xscom_size = PNV_XSCOM_CHIPTOD_SIZE;
> +    pctc->xscom_ops = &pnv_chiptod_power10_xscom_ops;
> +}
> +
> +static const TypeInfo pnv_chiptod_power10_type_info = {
> +    .name          = TYPE_PNV10_CHIPTOD,
> +    .parent        = TYPE_PNV_CHIPTOD,
> +    .instance_size = sizeof(PnvChipTOD),
> +    .class_init    = pnv_chiptod_power10_class_init,
> +    .interfaces    = (InterfaceInfo[]) {
> +        { TYPE_PNV_XSCOM_INTERFACE },
> +        { }
> +    }
> +};
> +
> +static void pnv_chiptod_realize(DeviceState *dev, Error **errp)
> +{
> +    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
> +    PnvChipTODClass *pctc = PNV_CHIPTOD_GET_CLASS(chiptod);
> +
> +    if (chiptod->primary) {
> +        chiptod->pss_mss_ctrl_reg |= PPC_BIT(1); /* TOD is master */
> +    }
> +
> +    /* Drawer is master (we do not simulate multi-drawer) */
> +    chiptod->pss_mss_ctrl_reg |= PPC_BIT(2);
> +    chiptod->tod_state = tod_running;

Shouldn't these initial values be set in a reset handler ?

> +
> +    /* XScom regions for ChipTOD registers */
> +    pnv_xscom_region_init(&chiptod->xscom_regs, OBJECT(dev),
> +                          pctc->xscom_ops, chiptod, "xscom-chiptod",
> +                          pctc->xscom_size);
> +}
> +
> +static void pnv_chiptod_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = pnv_chiptod_realize;
> +    dc->desc = "PowerNV ChipTOD Controller";
> +    dc->user_creatable = false;
> +}
> +
> +static const TypeInfo pnv_chiptod_type_info = {
> +    .name          = TYPE_PNV_CHIPTOD,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(PnvChipTOD),
> +    .class_init    = pnv_chiptod_class_init,
> +    .class_size    = sizeof(PnvChipTODClass),
> +    .abstract      = true,
> +};
> +
> +static void pnv_chiptod_register_types(void)
> +{
> +    type_register_static(&pnv_chiptod_type_info);
> +    type_register_static(&pnv_chiptod_power9_type_info);
> +    type_register_static(&pnv_chiptod_power10_type_info);
> +}
> +
> +type_init(pnv_chiptod_register_types);
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index ea44856d43..b1f4e65d24 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -46,6 +46,7 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
>     'pnv_i2c.c',
>     'pnv_lpc.c',
>     'pnv_psi.c',
> +  'pnv_chiptod.c',
>     'pnv_occ.c',
>     'pnv_sbe.c',
>     'pnv_bmc.c',
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index f670e8906c..57c4f265ef 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -95,6 +95,10 @@ vof_write(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] \"%s\""
>   vof_avail(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
>   vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
>   
> +# pnv_chiptod.c
> +pnv_chiptod_xscom_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
> +pnv_chiptod_xscom_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
> +
>   # pnv_sbe.c
>   pnv_sbe_xscom_ctrl_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
>   pnv_sbe_xscom_ctrl_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/7] pnv/chiptod: Implement the ChipTOD to Core transfer
  2023-11-23 10:30 ` [PATCH 5/7] pnv/chiptod: Implement the ChipTOD to Core transfer Nicholas Piggin
@ 2023-11-23 18:34   ` Cédric Le Goater
  2023-11-24  6:33     ` Nicholas Piggin
  0 siblings, 1 reply; 15+ messages in thread
From: Cédric Le Goater @ 2023-11-23 18:34 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: Daniel Henrique Barboza, Frédéric Barrat, qemu-devel

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.

> +{
> +    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);
  


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes
  2023-11-23 17:09 ` [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Cédric Le Goater
@ 2023-11-24  6:02   ` Nicholas Piggin
  0 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-24  6:02 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: Daniel Henrique Barboza, Frédéric Barrat, qemu-devel

On Fri Nov 24, 2023 at 3:09 AM AEST, Cédric Le Goater wrote:
> On 11/23/23 11:30, Nicholas Piggin wrote:
> > The chiptod/TFMR/state machine is not really tied to the other
> > time register fixes, but they touch some of the same code, and
> > logically same facility.
> > 
> > Changes since v1 of chiptod patches:
> > - Split hackish ChipTOD<->TFMR/TBST interface into its own patch
> > - Fix multi-socket addressing on P9 / chip ID mode (P10 works)
> > - Change chiptod primary/secondary setting to use class properties
> > - Add more comments to explain TOD overview and timebase state
> >    machine.
> > - SMT support for TFMR, some functionality is limited to thread 0.
> > - FIRMWARE_CONTROL_ERROR bit implemented in TFMR.
> > - Misc cleanups and bug fixes.
> > 
> > The hacky part, addressing core from chiptod, is still hacky. Is
> > there strong objection to it?
>
> Dunno yet :)

Thanks for the nice review!

> > This successfully runs skiboot chiptod initialisation code with
> > POWER9 and POWER10 multi-socket, multi-core, SMT. That requires
> > skiboot 7.1 (not in-tree), otherwise chiptod init is skipped on
> > QEMU machines.
>
> Let's update skiboot at the same time then.

Yeah, I'll update skiboot ahead of adding merging this.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model
  2023-11-23 17:49   ` Cédric Le Goater
@ 2023-11-24  6:14     ` Nicholas Piggin
  0 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-24  6:14 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: Daniel Henrique Barboza, Frédéric Barrat, qemu-devel

On Fri Nov 24, 2023 at 3:49 AM AEST, Cédric Le Goater wrote:
> On 11/23/23 11:30, Nicholas Piggin wrote:
> > The ChipTOD (for Time-Of-Day) is a pervasive facility that keeps the
> > clocks on multiple chips consistent which can keep TOD (time-of-day),
> > synchronise it across multiple chips, and can move that TOD to or from
> > the core timebase units.
>
> May be rephrase a bit the sentence above. I find it difficult to read.
>
> > This driver implements basic emulation of chiptod registers sufficient
>
> it's a model.

+1

> > +static void chiptod_power9_send_remotes(PnvChipTOD *chiptod)
> > +{
> > +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>
> Using qdev_get_machine() in a model is always a bit annoying. There is
> work in progress currently to have a single QEMU binary for all arches
> and when done, the "machine" would encompass something bigger including
> the OCC, BMC, etc.

We need a way to get from one chip to another, fundamentally. I
didn't see a better way, I suppose we could build a PnvHost container
that includes all PnvChips or something.

> Do we know how XSCOM propagates the new state to the chiptop on other
> chips ? is it some sort of broadcast on the bus ? Could we model it ?
> I am only asking, not to be done now.

It's a bit hard to work out. Real ChipTOD is vastly more complicated
than this model :P

It seems that ChipTOD can master PIB scoms to other chips, but also
this TTYPE transactions look like they have a command that goes
on the powerbus via ADU.


> > +static void chiptod_power10_invalidate_remotes(PnvChipTOD *chiptod)
> > +{
> > +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> > +    int i;
> > +
> > +    for (i = 0; i < pnv->num_chips; i++) {
> > +        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
> > +        if (&chip10->chiptod != chiptod) {
> > +            chip10->chiptod.tod_state = tod_not_set;
> > +        }
> > +    }
> > +}
>
> Could we avoid 4 routines doing the same thing and introduce :
>
>    chiptod_power*_set_tod_state(PnvChipTOD *chiptod, enum tod_state new)
>
> ?
>
> We could even introcude a PnvChipClass::get_chiptod handler. Might be
> overkill though.

Good idea...

> > +    case TOD_TX_TTYPE_5_REG:
> > +        if (is_power9) {
> > +            chiptod_power9_invalidate_remotes(chiptod);
> > +        } else {
> > +            chiptod_power10_invalidate_remotes(chiptod);
> > +        }
>
> With PnvChipTODClass::invalidate_remotes and PnvChipTODClass::send_remotes
> handlers, or ::set_state, this routine would not need a 'is_power9' parameter
> and the model would not need 2 different xscom_ops. Can it be done ?

... yes! ChipTODClass seems to be a good place for it.

> > +static void pnv_chiptod_realize(DeviceState *dev, Error **errp)
> > +{
> > +    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
> > +    PnvChipTODClass *pctc = PNV_CHIPTOD_GET_CLASS(chiptod);
> > +
> > +    if (chiptod->primary) {
> > +        chiptod->pss_mss_ctrl_reg |= PPC_BIT(1); /* TOD is master */
> > +    }
> > +
> > +    /* Drawer is master (we do not simulate multi-drawer) */
> > +    chiptod->pss_mss_ctrl_reg |= PPC_BIT(2);
> > +    chiptod->tod_state = tod_running;
>
> Shouldn't these initial values be set in a reset handler ?

Yes, and actually reset state is tod_error so fixed that too.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/7] pnv/chiptod: Implement the ChipTOD to Core transfer
  2023-11-23 18:34   ` Cédric Le Goater
@ 2023-11-24  6:33     ` Nicholas Piggin
  0 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-11-24  6:33 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: Daniel Henrique Barboza, Frédéric Barrat, qemu-devel

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes
  2023-12-01 12:24 [PATCH] target/ppc: Re-name registers to match ISA Nicholas Piggin
@ 2023-12-01 12:24 ` Nicholas Piggin
  0 siblings, 0 replies; 15+ messages in thread
From: Nicholas Piggin @ 2023-12-01 12:24 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	qemu-devel, Frédéric Barrat

The chiptod/TFMR/state machine is not really tied to the other
time register fixes, but they touch some of the same code, and
logically same facility.

Changes since v1 of chiptod patches:
- Split hackish ChipTOD<->TFMR/TBST interface into its own patch
- Fix multi-socket addressing on P9 / chip ID mode (P10 works)
- Change chiptod primary/secondary setting to use class properties
- Add more comments to explain TOD overview and timebase state
  machine.
- SMT support for TFMR, some functionality is limited to thread 0.
- FIRMWARE_CONTROL_ERROR bit implemented in TFMR.
- Misc cleanups and bug fixes.

The hacky part, addressing core from chiptod, is still hacky. Is
there strong objection to it?

This successfully runs skiboot chiptod initialisation code with
POWER9 and POWER10 multi-socket, multi-core, SMT. That requires
skiboot 7.1 (not in-tree), otherwise chiptod init is skipped on
QEMU machines.

Thanks,
Nick

Nicholas Piggin (7):
  target/ppc: Rename TBL to TB on 64-bit
  target/ppc: Improve timebase register defines naming
  target/ppc: Fix move-to timebase SPR access permissions
  pnv/chiptod: Add POWER9/10 chiptod model
  pnv/chiptod: Implement the ChipTOD to Core transfer
  target/ppc: Implement core timebase state machine and TFMR
  target/ppc: Add SMT support to time facilities

 include/hw/ppc/pnv_chip.h    |   3 +
 include/hw/ppc/pnv_chiptod.h |  55 ++++
 include/hw/ppc/pnv_core.h    |   4 +
 include/hw/ppc/pnv_xscom.h   |   9 +
 target/ppc/cpu.h             |  50 +++-
 hw/ppc/pnv.c                 |  63 +++++
 hw/ppc/pnv_chiptod.c         | 509 +++++++++++++++++++++++++++++++++++
 target/ppc/helper_regs.c     |  39 ++-
 target/ppc/ppc-qmp-cmds.c    |   4 +
 target/ppc/timebase_helper.c | 309 ++++++++++++++++++++-
 target/ppc/translate.c       |  42 ++-
 hw/ppc/meson.build           |   1 +
 hw/ppc/trace-events          |   4 +
 13 files changed, 1067 insertions(+), 25 deletions(-)
 create mode 100644 include/hw/ppc/pnv_chiptod.h
 create mode 100644 hw/ppc/pnv_chiptod.c

-- 
2.42.0



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-12-01 12:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-23 10:30 [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin
2023-11-23 10:30 ` [PATCH 1/7] target/ppc: Rename TBL to TB on 64-bit Nicholas Piggin
2023-11-23 10:30 ` [PATCH 2/7] target/ppc: Improve timebase register defines naming Nicholas Piggin
2023-11-23 10:30 ` [PATCH 3/7] target/ppc: Fix move-to timebase SPR access permissions Nicholas Piggin
2023-11-23 10:30 ` [PATCH 4/7] pnv/chiptod: Add POWER9/10 chiptod model Nicholas Piggin
2023-11-23 17:49   ` Cédric Le Goater
2023-11-24  6:14     ` Nicholas Piggin
2023-11-23 10:30 ` [PATCH 5/7] pnv/chiptod: Implement the ChipTOD to Core transfer Nicholas Piggin
2023-11-23 18:34   ` Cédric Le Goater
2023-11-24  6:33     ` Nicholas Piggin
2023-11-23 10:30 ` [PATCH 6/7] target/ppc: Implement core timebase state machine and TFMR Nicholas Piggin
2023-11-23 10:30 ` [PATCH 7/7] target/ppc: Add SMT support to time facilities Nicholas Piggin
2023-11-23 17:09 ` [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Cédric Le Goater
2023-11-24  6:02   ` Nicholas Piggin
  -- strict thread matches above, loose matches on Subject: below --
2023-12-01 12:24 [PATCH] target/ppc: Re-name registers to match ISA Nicholas Piggin
2023-12-01 12:24 ` [PATCH 0/7] ppc: pnv ChipTOD and various timebase fixes Nicholas Piggin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).