Re: [PATCH v2 14/19] ppc/pnv: Add POWER10 ChipTOD quirk for big-core

[Cédric Le Goater]

On 7/12/24 14:02, Nicholas Piggin wrote:
> POWER10 has a quirk in its ChipTOD addressing that requires the even
> small-core to be selected even when programming the odd small-core.
> This allows skiboot chiptod init to run in big-core mode.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Looks good, some more proposals below,

> ---
>   include/hw/ppc/pnv.h         | 1 +
>   include/hw/ppc/pnv_core.h    | 7 +++++++
>   hw/ppc/pnv.c                 | 7 ++++++-
>   target/ppc/timebase_helper.c | 9 +++++++++
>   4 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 283ddd50e7..c56d152889 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -77,6 +77,7 @@ struct PnvMachineClass {
>       const char *compat;
>       int compat_size;
>       int max_smt_threads;
> +    bool quirk_tb_big_core;
>   
>       void (*dt_power_mgt)(PnvMachineState *pnv, void *fdt);
>       void (*i2c_init)(PnvMachineState *pnv);
> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> index 50164e9e1f..c8784777a4 100644
> --- a/include/hw/ppc/pnv_core.h
> +++ b/include/hw/ppc/pnv_core.h
> @@ -27,6 +27,13 @@
>   
>   /* Per-core ChipTOD / TimeBase state */
>   typedef struct PnvCoreTODState {
> +    /*
> +     * POWER10 DD2.0 - big core TFMR drives the state machine on the even
> +     * small core. Skiboot has a workaround that targets the even small core
> +     * for CHIPTOD_TO_TB ops.
> +     */
> +    bool big_core_quirk;
> +
>       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 */
>   
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index fdf66323b8..24f7c007ce 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -2288,11 +2288,12 @@ static void pnv_chip_core_sanitize(PnvMachineState 
> *pnv, PnvChip *chip,
>   
>   static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
>   {
> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    PnvMachineClass *pmc = PNV_MACHINE_GET_CLASS(pnv);
>       Error *error = NULL;
>       PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>       const char *typename = pnv_chip_core_typename(chip);
>       int i, core_hwid;
> -    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>   
>       if (!object_class_by_name(typename)) {
>           error_setg(errp, "Unable to find PowerNV CPU Core '%s'", typename);
> @@ -2335,6 +2336,9 @@ static void pnv_chip_core_realize(PnvChip *chip, Error 
> **errp)
>                                   &error_fatal);
>           object_property_set_link(OBJECT(pnv_core), "chip", OBJECT(chip),
>                                    &error_abort);
> +
> +        pnv_core->tod_state.big_core_quirk = pmc->quirk_tb_big_core;
> +

instead :

@@ -2363,6 +2364,9 @@ static void pnv_chip_core_realize(PnvChi
                                  &error_abort);
         object_property_set_bool(OBJECT(pnv_core), "big-core", chip->big_core,
                                  &error_abort);
+        object_property_set_bool(OBJECT(pnv_core), "big-core-quirk",
+                                 pmc->quirk_tb_big_core, &error_abort);
+
         qdev_realize(DEVICE(pnv_core), NULL, &error_fatal);
 
         /* Each core has an XSCOM MMIO region */

and in hw/ppc/pnv_core.c :


@@ -365,6 +365,7 @@ static Property pnv_core_properties[] =
     DEFINE_PROP_UINT64("hrmor", PnvCore, hrmor, 0),
     DEFINE_PROP_LINK("chip", PnvCore, chip, TYPE_PNV_CHIP, PnvChip *),
     DEFINE_PROP_BOOL("big-core", PnvCore, big_core, false),
+    DEFINE_PROP_BOOL("big-core-quirk", PnvCore, tod_state.big_core_quirk, 
false),
     DEFINE_PROP_END_OF_LIST(),
 };
 

thanks,

C.


>           qdev_realize(DEVICE(pnv_core), NULL, &error_fatal);
>   
>           /* Each core has an XSCOM MMIO region */
> @@ -2647,6 +2651,7 @@ static void pnv_machine_p10_common_class_init(ObjectClass 
> *oc, void *data)
>       pmc->compat = compat;
>       pmc->compat_size = sizeof(compat);
>       pmc->max_smt_threads = 4;
> +    pmc->quirk_tb_big_core = true;
>       pmc->dt_power_mgt = pnv_dt_power_mgt;
>   
>       xfc->match_nvt = pnv10_xive_match_nvt;
> diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
> index 44cacf065e..019b8ee41f 100644
> --- a/target/ppc/timebase_helper.c
> +++ b/target/ppc/timebase_helper.c
> @@ -20,6 +20,7 @@
>   #include "cpu.h"
>   #include "hw/ppc/ppc.h"
>   #include "hw/ppc/pnv_core.h"
> +#include "hw/ppc/pnv_chip.h"
>   #include "exec/helper-proto.h"
>   #include "exec/exec-all.h"
>   #include "qemu/log.h"
> @@ -297,6 +298,14 @@ static PnvCoreTODState *cpu_get_tbst(PowerPCCPU *cpu)
>   {
>       PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
>   
> +    if (pc->big_core && pc->tod_state.big_core_quirk) {
> +        /* Must operate on the even small core */
> +        int core_id = CPU_CORE(pc)->core_id;
> +        if (core_id & 1) {
> +            pc = pc->chip->cores[core_id & ~1];
> +        }
> +    }
> +
>       return &pc->tod_state;
>   }
>