Re: [PATCH 08/13] ppc/pnv: Introduce PnvChipClass::xscom_core_base() method

[Cédric Le Goater]

On 13/12/2019 13:00, Greg Kurz wrote:
> The pnv_chip_core_realize() function configures the XSCOM MMIO subregion
> for each core of a single chip. The base address of the subregion depends
> on the CPU type. Its computation is currently open-code using the
> pnv_chip_is_powerXX() helpers. This can be achieved with QOM. Introduce
> a method for this in the base chip class and implement it in child classes.

OK. We might need to introduce a PnvXscom model one day but this is fine
for now.

> Signed-off-by: Greg Kurz <address@hidden>

Reviewed-by: Cédric Le Goater <address@hidden>

> ---
>  hw/ppc/pnv.c         |   31 ++++++++++++++++++++++++-------
>  include/hw/ppc/pnv.h |    1 +
>  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 2a53e99bda2e..88efa755e611 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -616,6 +616,24 @@ static void pnv_chip_power9_pic_print_info(PnvChip 
> *chip, Monitor *mon)
>      pnv_psi_pic_print_info(&chip9->psi, mon);
>  }
>  
> +static uint64_t pnv_chip_power8_xscom_core_base(PnvChip *chip,
> +                                                uint32_t core_id)
> +{
> +    return PNV_XSCOM_EX_BASE(core_id);
> +}
> +
> +static uint64_t pnv_chip_power9_xscom_core_base(PnvChip *chip,
> +                                                uint32_t core_id)
> +{
> +    return PNV9_XSCOM_EC_BASE(core_id);
> +}
> +
> +static uint64_t pnv_chip_power10_xscom_core_base(PnvChip *chip,
> +                                                 uint32_t core_id)
> +{
> +    return PNV10_XSCOM_EC_BASE(core_id);
> +}
> +
>  static bool pnv_match_cpu(const char *default_type, const char *cpu_type)
>  {
>      PowerPCCPUClass *ppc_default =
> @@ -1107,6 +1125,7 @@ static void pnv_chip_power8e_class_init(ObjectClass 
> *klass, void *data)
>      k->isa_create = pnv_chip_power8_isa_create;
>      k->dt_populate = pnv_chip_power8_dt_populate;
>      k->pic_print_info = pnv_chip_power8_pic_print_info;
> +    k->xscom_core_base = pnv_chip_power8_xscom_core_base;
>      dc->desc = "PowerNV Chip POWER8E";
>  
>      device_class_set_parent_realize(dc, pnv_chip_power8_realize,
> @@ -1129,6 +1148,7 @@ static void pnv_chip_power8_class_init(ObjectClass 
> *klass, void *data)
>      k->isa_create = pnv_chip_power8_isa_create;
>      k->dt_populate = pnv_chip_power8_dt_populate;
>      k->pic_print_info = pnv_chip_power8_pic_print_info;
> +    k->xscom_core_base = pnv_chip_power8_xscom_core_base;
>      dc->desc = "PowerNV Chip POWER8";
>  
>      device_class_set_parent_realize(dc, pnv_chip_power8_realize,
> @@ -1151,6 +1171,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass 
> *klass, void *data)
>      k->isa_create = pnv_chip_power8nvl_isa_create;
>      k->dt_populate = pnv_chip_power8_dt_populate;
>      k->pic_print_info = pnv_chip_power8_pic_print_info;
> +    k->xscom_core_base = pnv_chip_power8_xscom_core_base;
>      dc->desc = "PowerNV Chip POWER8NVL";
>  
>      device_class_set_parent_realize(dc, pnv_chip_power8_realize,
> @@ -1323,6 +1344,7 @@ static void pnv_chip_power9_class_init(ObjectClass 
> *klass, void *data)
>      k->isa_create = pnv_chip_power9_isa_create;
>      k->dt_populate = pnv_chip_power9_dt_populate;
>      k->pic_print_info = pnv_chip_power9_pic_print_info;
> +    k->xscom_core_base = pnv_chip_power9_xscom_core_base;
>      dc->desc = "PowerNV Chip POWER9";
>  
>      device_class_set_parent_realize(dc, pnv_chip_power9_realize,
> @@ -1404,6 +1426,7 @@ static void pnv_chip_power10_class_init(ObjectClass 
> *klass, void *data)
>      k->isa_create = pnv_chip_power10_isa_create;
>      k->dt_populate = pnv_chip_power10_dt_populate;
>      k->pic_print_info = pnv_chip_power10_pic_print_info;
> +    k->xscom_core_base = pnv_chip_power10_xscom_core_base;
>      dc->desc = "PowerNV Chip POWER10";
>  
>      device_class_set_parent_realize(dc, pnv_chip_power10_realize,
> @@ -1491,13 +1514,7 @@ static void pnv_chip_core_realize(PnvChip *chip, Error 
> **errp)
>                                   &error_fatal);
>  
>          /* Each core has an XSCOM MMIO region */
> -        if (pnv_chip_is_power10(chip)) {
> -            xscom_core_base = PNV10_XSCOM_EC_BASE(core_hwid);
> -        } else if (pnv_chip_is_power9(chip)) {
> -            xscom_core_base = PNV9_XSCOM_EC_BASE(core_hwid);
> -        } else {
> -            xscom_core_base = PNV_XSCOM_EX_BASE(core_hwid);
> -        }
> +        xscom_core_base = pcc->xscom_core_base(chip, core_hwid);
>  
>          pnv_xscom_add_subregion(chip, xscom_core_base,
>                                  &pnv_core->xscom_regs);
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 7d2402784d4b..17ca9a14ac8f 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -137,6 +137,7 @@ typedef struct PnvChipClass {
>      ISABus *(*isa_create)(PnvChip *chip, Error **errp);
>      void (*dt_populate)(PnvChip *chip, void *fdt);
>      void (*pic_print_info)(PnvChip *chip, Monitor *mon);
> +    uint64_t (*xscom_core_base)(PnvChip *chip, uint32_t core_id);
>  } PnvChipClass;
>  
>  #define PNV_CHIP_TYPE_SUFFIX "-" TYPE_PNV_CHIP
>