diff mbox

[v2,4/7] ppc/pnv: add a core mask to PnvChip

Message ID 1472661255-20160-5-git-send-email-clg@kaod.org
State New
Headers show

Commit Message

Cédric Le Goater Aug. 31, 2016, 4:34 p.m. UTC 
This will be used to build real HW ids for the cores and enforce some
limits on the available cores per chip.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv.c         | 27 +++++++++++++++++++++++++++
 include/hw/ppc/pnv.h |  2 ++
 2 files changed, 29 insertions(+)

Comments

Cédric Le Goater Sept. 2, 2016, 8:03 a.m. UTC | #1 
On 08/31/2016 06:34 PM, Cédric Le Goater wrote:
> This will be used to build real HW ids for the cores and enforce some
> limits on the available cores per chip.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/pnv.c         | 27 +++++++++++++++++++++++++++
>  include/hw/ppc/pnv.h |  2 ++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index a6e7f66b2c0a..b6efb5e3ef07 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -236,6 +236,27 @@ static void ppc_powernv_init(MachineState *machine)
>      g_free(chip_typename);
>  }
>  
> +/* Allowed core identifiers on a POWER8 Processor Chip :
> + *
> + * <EX0 reserved>
> + *  EX1  - Venice only
> + *  EX2  - Venice only
> + *  EX3  - Venice only
> + *  EX4
> + *  EX5
> + *  EX6
> + * <EX7,8 reserved> <reserved>
> + *  EX9  - Venice only
> + *  EX10 - Venice only
> + *  EX11 - Venice only
> + *  EX12
> + *  EX13
> + *  EX14
> + * <EX15 reserved>
> + */
> +#define POWER8E_CORE_MASK  (~0xffff8f8f)
> +#define POWER8_CORE_MASK   (~0xffff8181)
> +
>  static void pnv_chip_power8nvl_realize(PnvChip *chip, Error **errp)
>  {
>      ;
> @@ -250,6 +271,8 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
>      k->cpu_model = "POWER8NVL";
>      k->chip_type = PNV_CHIP_P8NVL;
>      k->chip_f000f = 0x120d304980000000ull;
> +    k->cores_max = 12;
> +    k->cores_mask = POWER8_CORE_MASK;
>      dc->desc = "PowerNV Chip POWER8NVL";
>  }
>  
> @@ -274,6 +297,8 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
>      k->cpu_model = "POWER8";
>      k->chip_type = PNV_CHIP_P8;
>      k->chip_f000f = 0x220ea04980000000ull;
> +    k->cores_max = 12;
> +    k->cores_mask = POWER8_CORE_MASK;
>      dc->desc = "PowerNV Chip POWER8";
>  }
>  
> @@ -298,6 +323,8 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
>      k->cpu_model = "POWER8E";
>      k->chip_type = PNV_CHIP_P8E;
>      k->chip_f000f = 0x221ef04980000000ull;
> +    k->cores_max = 6;
> +    k->cores_mask = POWER8E_CORE_MASK;
>      dc->desc = "PowerNV Chip POWER8E";
>  }
>  
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index bc6e1f80096b..987bc70245a7 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -49,6 +49,8 @@ typedef struct PnvChipClass {
>      /*< private >*/
>      SysBusDeviceClass parent_class;
>      /*< public >*/
> +    uint32_t   cores_max;
> +    uint32_t   cores_mask;

This 'cores_mask' attribute needs to be a uint64_t for POWER9. The 
core chiplet ids are in the range : [ 0x20 - 0x37 ]

I will change that in the next version and include a PnvChipPower9
class as the cpu_model was merged in David's branch.

Cheers,
C.
David Gibson Sept. 5, 2016, 3:42 a.m. UTC | #2 
On Wed, Aug 31, 2016 at 06:34:12PM +0200, Cédric Le Goater wrote:
> This will be used to build real HW ids for the cores and enforce some
> limits on the available cores per chip.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/pnv.c         | 27 +++++++++++++++++++++++++++
>  include/hw/ppc/pnv.h |  2 ++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index a6e7f66b2c0a..b6efb5e3ef07 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -236,6 +236,27 @@ static void ppc_powernv_init(MachineState *machine)
>      g_free(chip_typename);
>  }
>  
> +/* Allowed core identifiers on a POWER8 Processor Chip :
> + *
> + * <EX0 reserved>
> + *  EX1  - Venice only
> + *  EX2  - Venice only
> + *  EX3  - Venice only
> + *  EX4
> + *  EX5
> + *  EX6
> + * <EX7,8 reserved> <reserved>
> + *  EX9  - Venice only
> + *  EX10 - Venice only
> + *  EX11 - Venice only
> + *  EX12
> + *  EX13
> + *  EX14
> + * <EX15 reserved>
> + */
> +#define POWER8E_CORE_MASK  (~0xffff8f8f)
> +#define POWER8_CORE_MASK   (~0xffff8181)
> +
>  static void pnv_chip_power8nvl_realize(PnvChip *chip, Error **errp)
>  {
>      ;
> @@ -250,6 +271,8 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
>      k->cpu_model = "POWER8NVL";
>      k->chip_type = PNV_CHIP_P8NVL;
>      k->chip_f000f = 0x120d304980000000ull;
> +    k->cores_max = 12;

Is it worth having this cores_max field, since AFAICT you can
calculate it as hweight(~cores_mask)?

> +    k->cores_mask = POWER8_CORE_MASK;
>      dc->desc = "PowerNV Chip POWER8NVL";
>  }
>  
> @@ -274,6 +297,8 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
>      k->cpu_model = "POWER8";
>      k->chip_type = PNV_CHIP_P8;
>      k->chip_f000f = 0x220ea04980000000ull;
> +    k->cores_max = 12;
> +    k->cores_mask = POWER8_CORE_MASK;
>      dc->desc = "PowerNV Chip POWER8";
>  }
>  
> @@ -298,6 +323,8 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
>      k->cpu_model = "POWER8E";
>      k->chip_type = PNV_CHIP_P8E;
>      k->chip_f000f = 0x221ef04980000000ull;
> +    k->cores_max = 6;
> +    k->cores_mask = POWER8E_CORE_MASK;
>      dc->desc = "PowerNV Chip POWER8E";
>  }
>  
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index bc6e1f80096b..987bc70245a7 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -49,6 +49,8 @@ typedef struct PnvChipClass {
>      /*< private >*/
>      SysBusDeviceClass parent_class;
>      /*< public >*/
> +    uint32_t   cores_max;
> +    uint32_t   cores_mask;
>      const char *cpu_model;
>      PnvChipType  chip_type;
>      uint64_t     chip_f000f;
Cédric Le Goater Sept. 5, 2016, 11:13 a.m. UTC | #3 
On 09/05/2016 05:42 AM, David Gibson wrote:
> On Wed, Aug 31, 2016 at 06:34:12PM +0200, Cédric Le Goater wrote:
>> This will be used to build real HW ids for the cores and enforce some
>> limits on the available cores per chip.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/ppc/pnv.c         | 27 +++++++++++++++++++++++++++
>>  include/hw/ppc/pnv.h |  2 ++
>>  2 files changed, 29 insertions(+)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index a6e7f66b2c0a..b6efb5e3ef07 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -236,6 +236,27 @@ static void ppc_powernv_init(MachineState *machine)
>>      g_free(chip_typename);
>>  }
>>  
>> +/* Allowed core identifiers on a POWER8 Processor Chip :
>> + *
>> + * <EX0 reserved>
>> + *  EX1  - Venice only
>> + *  EX2  - Venice only
>> + *  EX3  - Venice only
>> + *  EX4
>> + *  EX5
>> + *  EX6
>> + * <EX7,8 reserved> <reserved>
>> + *  EX9  - Venice only
>> + *  EX10 - Venice only
>> + *  EX11 - Venice only
>> + *  EX12
>> + *  EX13
>> + *  EX14
>> + * <EX15 reserved>
>> + */
>> +#define POWER8E_CORE_MASK  (~0xffff8f8f)
>> +#define POWER8_CORE_MASK   (~0xffff8181)
>> +
>>  static void pnv_chip_power8nvl_realize(PnvChip *chip, Error **errp)
>>  {
>>      ;
>> @@ -250,6 +271,8 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
>>      k->cpu_model = "POWER8NVL";
>>      k->chip_type = PNV_CHIP_P8NVL;
>>      k->chip_f000f = 0x120d304980000000ull;
>> +    k->cores_max = 12;
> 
> Is it worth having this cores_max field, since AFAICT you can
> calculate it as hweight(~cores_mask)?

Sure. I looked for it and missed it. 

Here is a slightly improved version from Brian Kernighan :

	unsigned int v; // count the number of bits set in v
	unsigned int c; // c accumulates the total bits set in v

	for (c = 0; v; c++)
	{
		v &= v - 1; // clear the least significant bit set
	}

Thanks,

C. 

>> +    k->cores_mask = POWER8_CORE_MASK;
>>      dc->desc = "PowerNV Chip POWER8NVL";
>>  }
>>  
>> @@ -274,6 +297,8 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
>>      k->cpu_model = "POWER8";
>>      k->chip_type = PNV_CHIP_P8;
>>      k->chip_f000f = 0x220ea04980000000ull;
>> +    k->cores_max = 12;
>> +    k->cores_mask = POWER8_CORE_MASK;
>>      dc->desc = "PowerNV Chip POWER8";
>>  }
>>  
>> @@ -298,6 +323,8 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
>>      k->cpu_model = "POWER8E";
>>      k->chip_type = PNV_CHIP_P8E;
>>      k->chip_f000f = 0x221ef04980000000ull;
>> +    k->cores_max = 6;
>> +    k->cores_mask = POWER8E_CORE_MASK;
>>      dc->desc = "PowerNV Chip POWER8E";
>>  }
>>  
>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
>> index bc6e1f80096b..987bc70245a7 100644
>> --- a/include/hw/ppc/pnv.h
>> +++ b/include/hw/ppc/pnv.h
>> @@ -49,6 +49,8 @@ typedef struct PnvChipClass {
>>      /*< private >*/
>>      SysBusDeviceClass parent_class;
>>      /*< public >*/
>> +    uint32_t   cores_max;
>> +    uint32_t   cores_mask;
>>      const char *cpu_model;
>>      PnvChipType  chip_type;
>>      uint64_t     chip_f000f;
>
Benjamin Herrenschmidt Sept. 5, 2016, 11:30 a.m. UTC | #4 
On Mon, 2016-09-05 at 13:13 +0200, Cédric Le Goater wrote:
> > Is it worth having this cores_max field, since AFAICT you can

> > calculate it as hweight(~cores_mask)?

> 

> Sure. I looked for it and missed it. 

> 

> Here is a slightly improved version from Brian Kernighan :

> 

>         unsigned int v; // count the number of bits set in v

>         unsigned int c; // c accumulates the total bits set in v

> 

>         for (c = 0; v; c++)

>         {

>                 v &= v - 1; // clear the least significant bit set

>         }


Use the accessor (ie, hweight). The compiler might turn it into a
popcnt instruction of some sort if the CPU has one :-)

Ben.
diff mbox

Patch

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index a6e7f66b2c0a..b6efb5e3ef07 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -236,6 +236,27 @@  static void ppc_powernv_init(MachineState *machine)
     g_free(chip_typename);
 }
 
+/* Allowed core identifiers on a POWER8 Processor Chip :
+ *
+ * <EX0 reserved>
+ *  EX1  - Venice only
+ *  EX2  - Venice only
+ *  EX3  - Venice only
+ *  EX4
+ *  EX5
+ *  EX6
+ * <EX7,8 reserved> <reserved>
+ *  EX9  - Venice only
+ *  EX10 - Venice only
+ *  EX11 - Venice only
+ *  EX12
+ *  EX13
+ *  EX14
+ * <EX15 reserved>
+ */
+#define POWER8E_CORE_MASK  (~0xffff8f8f)
+#define POWER8_CORE_MASK   (~0xffff8181)
+
 static void pnv_chip_power8nvl_realize(PnvChip *chip, Error **errp)
 {
     ;
@@ -250,6 +271,8 @@  static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
     k->cpu_model = "POWER8NVL";
     k->chip_type = PNV_CHIP_P8NVL;
     k->chip_f000f = 0x120d304980000000ull;
+    k->cores_max = 12;
+    k->cores_mask = POWER8_CORE_MASK;
     dc->desc = "PowerNV Chip POWER8NVL";
 }
 
@@ -274,6 +297,8 @@  static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
     k->cpu_model = "POWER8";
     k->chip_type = PNV_CHIP_P8;
     k->chip_f000f = 0x220ea04980000000ull;
+    k->cores_max = 12;
+    k->cores_mask = POWER8_CORE_MASK;
     dc->desc = "PowerNV Chip POWER8";
 }
 
@@ -298,6 +323,8 @@  static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
     k->cpu_model = "POWER8E";
     k->chip_type = PNV_CHIP_P8E;
     k->chip_f000f = 0x221ef04980000000ull;
+    k->cores_max = 6;
+    k->cores_mask = POWER8E_CORE_MASK;
     dc->desc = "PowerNV Chip POWER8E";
 }
 
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index bc6e1f80096b..987bc70245a7 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -49,6 +49,8 @@  typedef struct PnvChipClass {
     /*< private >*/
     SysBusDeviceClass parent_class;
     /*< public >*/
+    uint32_t   cores_max;
+    uint32_t   cores_mask;
     const char *cpu_model;
     PnvChipType  chip_type;
     uint64_t     chip_f000f;