Re: [Qemu-devel] [PATCH v4 04/20] ppc/pnv: add a PIR handler to PnvChip

[David Gibson]

On Mon, Oct 03, 2016 at 09:24:40AM +0200, Cédric Le Goater wrote:
> The Processor Identification Register (PIR) is a register that holds a
> processor identifier which is used for bus transactions (XSCOM) and
> for processor differentiation in multiprocessor systems. It also used
> in the interrupt vector entries (IVE) to identify the thread serving
> the interrupts.
> 
> P9 and P8 have some differences in the CPU PIR encoding.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>

Reviewed-by: David Gibson <address@hidden>

Looks fine, although it's a bit hard to be sure since I haven't read
the patches which actually use this yet.

> ---
> 
>  Changes since v3 :
> 
>  - added a couple more comments on the bits definition
>   
>  hw/ppc/pnv.c         | 30 ++++++++++++++++++++++++++++++
>  include/hw/ppc/pnv.h |  2 ++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index fc930be94f53..758c849702a0 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -239,6 +239,32 @@ static void ppc_powernv_init(MachineState *machine)
>      g_free(chip_typename);
>  }
>  
> +/*
> + *    0:21  Reserved - Read as zeros
> + *   22:24  Chip ID
> + *   25:28  Core number
> + *   29:31  Thread ID
> + */
> +static uint32_t pnv_chip_core_pir_p8(PnvChip *chip, uint32_t core_id)
> +{
> +    return (chip->chip_id << 7) | (core_id << 3);
> +}
> +
> +/*
> + *    0:48  Reserved - Read as zeroes
> + *   49:52  Node ID
> + *   53:55  Chip ID
> + *   56     Reserved - Read as zero
> + *   57:61  Core number
> + *   62:63  Thread ID
> + *
> + * We only care about the lower bits. uint32_t is fine for the moment.
> + */
> +static uint32_t pnv_chip_core_pir_p9(PnvChip *chip, uint32_t core_id)
> +{
> +    return (chip->chip_id << 8) | (core_id << 2);
> +}
> +
>  /* Allowed core identifiers on a POWER8 Processor Chip :
>   *
>   * <EX0 reserved>
> @@ -274,6 +300,7 @@ static void pnv_chip_power8e_class_init(ObjectClass 
> *klass, void *data)
>      k->chip_type = PNV_CHIP_POWER8E;
>      k->chip_cfam_id = 0x221ef04980000000ull;  /* P8 Murano DD2.1 */
>      k->cores_mask = POWER8E_CORE_MASK;
> +    k->core_pir = pnv_chip_core_pir_p8;
>      dc->desc = "PowerNV Chip POWER8E";
>  }
>  
> @@ -293,6 +320,7 @@ static void pnv_chip_power8_class_init(ObjectClass 
> *klass, void *data)
>      k->chip_type = PNV_CHIP_POWER8;
>      k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
>      k->cores_mask = POWER8_CORE_MASK;
> +    k->core_pir = pnv_chip_core_pir_p8;
>      dc->desc = "PowerNV Chip POWER8";
>  }
>  
> @@ -312,6 +340,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass 
> *klass, void *data)
>      k->chip_type = PNV_CHIP_POWER8NVL;
>      k->chip_cfam_id = 0x120d304980000000ull;  /* P8 Naples DD1.0 */
>      k->cores_mask = POWER8_CORE_MASK;
> +    k->core_pir = pnv_chip_core_pir_p8;
>      dc->desc = "PowerNV Chip POWER8NVL";
>  }
>  
> @@ -331,6 +360,7 @@ static void pnv_chip_power9_class_init(ObjectClass 
> *klass, void *data)
>      k->chip_type = PNV_CHIP_POWER9;
>      k->chip_cfam_id = 0x100d104980000000ull; /* P9 Nimbus DD1.0 */
>      k->cores_mask = POWER9_CORE_MASK;
> +    k->core_pir = pnv_chip_core_pir_p9;
>      dc->desc = "PowerNV Chip POWER9";
>  }
>  
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 2c225c928974..c676f800e28e 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -56,6 +56,8 @@ typedef struct PnvChipClass {
>      PnvChipType  chip_type;
>      uint64_t     chip_cfam_id;
>      uint64_t     cores_mask;
> +
> +    uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
>  } PnvChipClass;
>  
>  #define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E"

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

signature.asc
Description: PGP signature