Re: [Qemu-devel] [PATCH 5/6] ppc/pnv: introduce a new intc_create() operation to the chip model

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6814 bytes --]

On Thu, Jun 14, 2018 at 04:00:42PM +0200, Cédric Le Goater wrote:
> On Power9, the thread interrupt presenter has a different type and is
> linked to the chip owning the cores.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

(but not applied for now, since it depends on earlier patches I had
comments on)

> ---
>  include/hw/ppc/pnv.h |  1 +
>  hw/ppc/pnv.c         | 21 +++++++++++++++++++--
>  hw/ppc/pnv_core.c    | 18 +++++++++---------
>  3 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 90759240a7b1..e934e84f555e 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -76,6 +76,7 @@ typedef struct PnvChipClass {
>      hwaddr       xscom_base;
>  
>      uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
> +    Object *(*intc_create)(PnvChip *chip, Object *child, Error **errp);
>  } PnvChipClass;
>  
>  #define PNV_CHIP_TYPE_SUFFIX "-" TYPE_PNV_CHIP
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index b3b0dd44582f..7d99366daf90 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -641,6 +641,13 @@ static uint32_t pnv_chip_core_pir_p8(PnvChip *chip, uint32_t core_id)
>      return (chip->chip_id << 7) | (core_id << 3);
>  }
>  
> +static Object *pnv_chip_power8_intc_create(PnvChip *chip, Object *child,
> +                                           Error **errp)
> +{
> +    return icp_create(child, TYPE_PNV_ICP, XICS_FABRIC(qdev_get_machine()),
> +                      errp);
> +}
> +
>  /*
>   *    0:48  Reserved - Read as zeroes
>   *   49:52  Node ID
> @@ -656,6 +663,12 @@ static uint32_t pnv_chip_core_pir_p9(PnvChip *chip, uint32_t core_id)
>      return (chip->chip_id << 8) | (core_id << 2);
>  }
>  
> +static Object *pnv_chip_power9_intc_create(PnvChip *chip, Object *child,
> +                                           Error **errp)
> +{
> +    return NULL;
> +}
> +
>  /* Allowed core identifiers on a POWER8 Processor Chip :
>   *
>   * <EX0 reserved>
> @@ -691,6 +704,7 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
>      k->chip_cfam_id = 0x221ef04980000000ull;  /* P8 Murano DD2.1 */
>      k->cores_mask = POWER8E_CORE_MASK;
>      k->core_pir = pnv_chip_core_pir_p8;
> +    k->intc_create = pnv_chip_power8_intc_create;
>      k->xscom_base = 0x003fc0000000000ull;
>      dc->desc = "PowerNV Chip POWER8E";
>  }
> @@ -704,6 +718,7 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
>      k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
>      k->cores_mask = POWER8_CORE_MASK;
>      k->core_pir = pnv_chip_core_pir_p8;
> +    k->intc_create = pnv_chip_power8_intc_create;
>      k->xscom_base = 0x003fc0000000000ull;
>      dc->desc = "PowerNV Chip POWER8";
>  }
> @@ -717,6 +732,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
>      k->chip_cfam_id = 0x120d304980000000ull;  /* P8 Naples DD1.0 */
>      k->cores_mask = POWER8_CORE_MASK;
>      k->core_pir = pnv_chip_core_pir_p8;
> +    k->intc_create = pnv_chip_power8_intc_create;
>      k->xscom_base = 0x003fc0000000000ull;
>      dc->desc = "PowerNV Chip POWER8NVL";
>  }
> @@ -730,6 +746,7 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
>      k->chip_cfam_id = 0x220d104900008000ull; /* P9 Nimbus DD2.0 */
>      k->cores_mask = POWER9_CORE_MASK;
>      k->core_pir = pnv_chip_core_pir_p9;
> +    k->intc_create = pnv_chip_power9_intc_create;
>      k->xscom_base = 0x00603fc00000000ull;
>      dc->desc = "PowerNV Chip POWER9";
>  }
> @@ -865,8 +882,8 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
>          object_property_set_int(OBJECT(pnv_core),
>                                  pcc->core_pir(chip, core_hwid),
>                                  "pir", &error_fatal);
> -        object_property_add_const_link(OBJECT(pnv_core), "xics",
> -                                       qdev_get_machine(), &error_fatal);
> +        object_property_add_const_link(OBJECT(pnv_core), "chip",
> +                                       OBJECT(chip), &error_fatal);
>          object_property_set_bool(OBJECT(pnv_core), true, "realized",
>                                   &error_fatal);
>          object_unref(OBJECT(pnv_core));
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 13ad7d9e0470..5805bcd10abf 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -121,11 +121,12 @@ static const MemoryRegionOps pnv_core_xscom_ops = {
>      .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> -static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp)
> +static void pnv_core_realize_child(Object *child, PnvChip *chip, Error **errp)
>  {
>      Error *local_err = NULL;
>      CPUState *cs = CPU(child);
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>  
>      object_property_set_bool(child, true, "realized", &local_err);
>      if (local_err) {
> @@ -133,7 +134,7 @@ static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp)
>          return;
>      }
>  
> -    cpu->intc = icp_create(child, TYPE_PNV_ICP, xi, &local_err);
> +    cpu->intc = pcc->intc_create(chip, child, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
> @@ -156,13 +157,12 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
>      void *obj;
>      int i, j;
>      char name[32];
> -    Object *xi;
> +    Object *chip;
>  
> -    xi = object_property_get_link(OBJECT(dev), "xics", &local_err);
> -    if (!xi) {
> -        error_setg(errp, "%s: required link 'xics' not found: %s",
> -                   __func__, error_get_pretty(local_err));
> -        return;
> +    chip = object_property_get_link(OBJECT(dev), "chip", &local_err);
> +    if (!chip) {
> +        error_propagate(errp, local_err);
> +        error_prepend(errp, "required link 'chip' not found: ");
>      }
>  
>      pc->threads = g_malloc0(size * cc->nr_threads);
> @@ -184,7 +184,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
>      for (j = 0; j < cc->nr_threads; j++) {
>          obj = pc->threads + j * size;
>  
> -        pnv_core_realize_child(obj, XICS_FABRIC(xi), &local_err);
> +        pnv_core_realize_child(obj, PNV_CHIP(chip), &local_err);
>          if (local_err) {
>              goto err;
>          }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]