2023-11-28 18:56:56

by Srinivas Pandruvada

[permalink] [raw]
Subject: [PATCH 1/6] platform/x86/intel/tpmi: Add additional TPMI header fields

TPMI information header added additional fields in version 2. Some of the
reserved fields in version 1 are used to define new fields.
Parse new fields and export as part of platform data. These fields include:
- PCI segment ID
- Partition ID of the package, useful when more than one Intel VSEC PCI
device per package
- cdie_mask: Mask of all compute dies in this partition

Signed-off-by: Srinivas Pandruvada <[email protected]>
---
drivers/platform/x86/intel/tpmi.c | 11 ++++++++++-
include/linux/intel_tpmi.h | 6 ++++++
2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
index 311abcac894a..c89aa4d14bea 100644
--- a/drivers/platform/x86/intel/tpmi.c
+++ b/drivers/platform/x86/intel/tpmi.c
@@ -128,6 +128,9 @@ struct intel_tpmi_info {
* @dev: PCI device number
* @bus: PCI bus number
* @pkg: CPU Package id
+ * @segment: PCI segment id
+ * @partition: Package Partition id
+ * @cdie_mask: Bitmap of compute dies in the current partition
* @reserved: Reserved for future use
* @lock: When set to 1 the register is locked and becomes read-only
* until next reset. Not for use by the OS driver.
@@ -139,7 +142,10 @@ struct tpmi_info_header {
u64 dev:5;
u64 bus:8;
u64 pkg:8;
- u64 reserved:39;
+ u64 segment:8;
+ u64 partition:2;
+ u64 cdie_mask:16;
+ u64 reserved:13;
u64 lock:1;
} __packed;

@@ -684,6 +690,9 @@ static int tpmi_process_info(struct intel_tpmi_info *tpmi_info,
tpmi_info->plat_info.bus_number = header.bus;
tpmi_info->plat_info.device_number = header.dev;
tpmi_info->plat_info.function_number = header.fn;
+ tpmi_info->plat_info.cdie_mask = header.cdie_mask;
+ tpmi_info->plat_info.partition = header.partition;
+ tpmi_info->plat_info.segment = header.segment;

iounmap(info_mem);

diff --git a/include/linux/intel_tpmi.h b/include/linux/intel_tpmi.h
index ee07393445f9..939663bb095f 100644
--- a/include/linux/intel_tpmi.h
+++ b/include/linux/intel_tpmi.h
@@ -14,7 +14,10 @@

/**
* struct intel_tpmi_plat_info - Platform information for a TPMI device instance
+ * @cdie_mask: Mask of all compute dies in the partition
* @package_id: CPU Package id
+ * @partition: Package partition id when multiple VSEC PCI devices per package
+ * @segment: PCI segment ID
* @bus_number: PCI bus number
* @device_number: PCI device number
* @function_number: PCI function number
@@ -23,7 +26,10 @@
* struct is used to return data via tpmi_get_platform_data().
*/
struct intel_tpmi_plat_info {
+ u16 cdie_mask;
u8 package_id;
+ u8 partition;
+ u8 segment;
u8 bus_number;
u8 device_number;
u8 function_number;
--
2.41.0


2023-11-30 12:33:39

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH 1/6] platform/x86/intel/tpmi: Add additional TPMI header fields

On Tue, 28 Nov 2023, Srinivas Pandruvada wrote:

> TPMI information header added additional fields in version 2. Some of the
> reserved fields in version 1 are used to define new fields.
> Parse new fields and export as part of platform data. These fields include:
> - PCI segment ID
> - Partition ID of the package, useful when more than one Intel VSEC PCI
> device per package
> - cdie_mask: Mask of all compute dies in this partition
>
> Signed-off-by: Srinivas Pandruvada <[email protected]>
> ---
> drivers/platform/x86/intel/tpmi.c | 11 ++++++++++-
> include/linux/intel_tpmi.h | 6 ++++++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
> index 311abcac894a..c89aa4d14bea 100644
> --- a/drivers/platform/x86/intel/tpmi.c
> +++ b/drivers/platform/x86/intel/tpmi.c
> @@ -128,6 +128,9 @@ struct intel_tpmi_info {
> * @dev: PCI device number
> * @bus: PCI bus number
> * @pkg: CPU Package id
> + * @segment: PCI segment id
> + * @partition: Package Partition id
> + * @cdie_mask: Bitmap of compute dies in the current partition
> * @reserved: Reserved for future use
> * @lock: When set to 1 the register is locked and becomes read-only
> * until next reset. Not for use by the OS driver.
> @@ -139,7 +142,10 @@ struct tpmi_info_header {
> u64 dev:5;
> u64 bus:8;
> u64 pkg:8;
> - u64 reserved:39;
> + u64 segment:8;
> + u64 partition:2;
> + u64 cdie_mask:16;
> + u64 reserved:13;
> u64 lock:1;
> } __packed;
> @@ -684,6 +690,9 @@ static int tpmi_process_info(struct intel_tpmi_info *tpmi_info,
> tpmi_info->plat_info.bus_number = header.bus;
> tpmi_info->plat_info.device_number = header.dev;
> tpmi_info->plat_info.function_number = header.fn;
> + tpmi_info->plat_info.cdie_mask = header.cdie_mask;
> + tpmi_info->plat_info.partition = header.partition;
> + tpmi_info->plat_info.segment = header.segment;
>
> iounmap(info_mem);
>
> diff --git a/include/linux/intel_tpmi.h b/include/linux/intel_tpmi.h
> index ee07393445f9..939663bb095f 100644
> --- a/include/linux/intel_tpmi.h
> +++ b/include/linux/intel_tpmi.h
> @@ -14,7 +14,10 @@
>
> /**
> * struct intel_tpmi_plat_info - Platform information for a TPMI device instance
> + * @cdie_mask: Mask of all compute dies in the partition
> * @package_id: CPU Package id
> + * @partition: Package partition id when multiple VSEC PCI devices per package
> + * @segment: PCI segment ID
> * @bus_number: PCI bus number
> * @device_number: PCI device number
> * @function_number: PCI function number
> @@ -23,7 +26,10 @@
> * struct is used to return data via tpmi_get_platform_data().
> */
> struct intel_tpmi_plat_info {
> + u16 cdie_mask;
> u8 package_id;
> + u8 partition;
> + u8 segment;
> u8 bus_number;
> u8 device_number;
> u8 function_number;

I've a number of questions about this patch...

- There no version check anywhere, yet commit message talks about v2?

- What will those fields have in v1?

- Entirely unrelated to the rest of this serie? So no users for these?
Why not send this along with the patches containing the actual users
so it'd have been easier to find the answers from the patches?

--
i.

2023-11-30 14:10:42

by Srinivas Pandruvada

[permalink] [raw]
Subject: Re: [PATCH 1/6] platform/x86/intel/tpmi: Add additional TPMI header fields

On Thu, 2023-11-30 at 14:33 +0200, Ilpo Järvinen wrote:
> On Tue, 28 Nov 2023, Srinivas Pandruvada wrote:
>
> > TPMI information header added additional fields in version 2. Some
> > of the
> > reserved fields in version 1 are used to define new fields.
> > Parse new fields and export as part of platform data. These fields
> > include:
> > - PCI segment ID
> > - Partition ID of the package, useful when more than one Intel VSEC
> > PCI
> > device per package
> > - cdie_mask: Mask of all compute dies in this partition
> >
> > Signed-off-by: Srinivas Pandruvada
> > <[email protected]>
> > ---
> >  drivers/platform/x86/intel/tpmi.c | 11 ++++++++++-
> >  include/linux/intel_tpmi.h        |  6 ++++++
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/intel/tpmi.c
> > b/drivers/platform/x86/intel/tpmi.c
> > index 311abcac894a..c89aa4d14bea 100644
> > --- a/drivers/platform/x86/intel/tpmi.c
> > +++ b/drivers/platform/x86/intel/tpmi.c
> > @@ -128,6 +128,9 @@ struct intel_tpmi_info {
> >   * @dev:       PCI device number
> >   * @bus:       PCI bus number
> >   * @pkg:       CPU Package id
> > + * @segment: PCI segment id
> > + * @partition: Package Partition id
> > + * @cdie_mask: Bitmap of compute dies in the current partition
> >   * @reserved:  Reserved for future use
> >   * @lock:      When set to 1 the register is locked and becomes
> > read-only
> >   *             until next reset. Not for use by the OS driver.
> > @@ -139,7 +142,10 @@ struct tpmi_info_header {
> >         u64 dev:5;
> >         u64 bus:8;
> >         u64 pkg:8;
> > -       u64 reserved:39;
> > +       u64 segment:8;
> > +       u64 partition:2;
> > +       u64 cdie_mask:16;
> > +       u64 reserved:13;
> >         u64 lock:1;
> >  } __packed;
> > @@ -684,6 +690,9 @@ static int tpmi_process_info(struct
> > intel_tpmi_info *tpmi_info,
> >         tpmi_info->plat_info.bus_number = header.bus;
> >         tpmi_info->plat_info.device_number = header.dev;
> >         tpmi_info->plat_info.function_number = header.fn;
> > +       tpmi_info->plat_info.cdie_mask = header.cdie_mask;
> > +       tpmi_info->plat_info.partition = header.partition;
> > +       tpmi_info->plat_info.segment = header.segment;
> >  
> >         iounmap(info_mem);
> >  
> > diff --git a/include/linux/intel_tpmi.h
> > b/include/linux/intel_tpmi.h
> > index ee07393445f9..939663bb095f 100644
> > --- a/include/linux/intel_tpmi.h
> > +++ b/include/linux/intel_tpmi.h
> > @@ -14,7 +14,10 @@
> >  
> >  /**
> >   * struct intel_tpmi_plat_info - Platform information for a TPMI
> > device instance
> > + * @cdie_mask: Mask of all compute dies in the partition
> >   * @package_id:        CPU Package id
> > + * @partition:  Package partition id when multiple VSEC PCI
> > devices per package
> > + * @segment: PCI segment ID
> >   * @bus_number:        PCI bus number
> >   * @device_number: PCI device number
> >   * @function_number: PCI function number
> > @@ -23,7 +26,10 @@
> >   * struct is used to return data via tpmi_get_platform_data().
> >   */
> >  struct intel_tpmi_plat_info {
> > +       u16 cdie_mask;
> >         u8 package_id;
> > +       u8 partition;
> > +       u8 segment;
> >         u8 bus_number;
> >         u8 device_number;
> >         u8 function_number;
>
> I've a number of questions about this patch...
>
> - There no version check anywhere, yet commit message talks about v2?
>
No need to check, as the other fields will be 0s in v1.

> - What will those fields have in v1?
They are 0s as they were reserved for future use.

>
> - Entirely unrelated to the rest of this serie? So no users for
> these?
>   Why not send this along with the patches containing the actual
> users
>   so it'd have been easier to find the answers from the patches?
>
This is the core changes, so submitted as changes done in specs.
But fine to bundle with next series. I limit number of changes per
kernel release in the order of importance in the current products.

Thanks,
Srinivas