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

[Ilpo Järvinen]

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 <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> ---
>  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.