diff mbox series

[05/12] platform/x86: ISST: Add support for MSR 0x54

Message ID	20230211063257.311746-6-srinivas.pandruvada@linux.intel.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> To: hdegoede@redhat.com, markgross@kernel.org Cc: platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Subject: [PATCH 05/12] platform/x86: ISST: Add support for MSR 0x54 Date: Fri, 10 Feb 2023 22:32:50 -0800 Message-Id: <20230211063257.311746-6-srinivas.pandruvada@linux.intel.com> In-Reply-To: <20230211063257.311746-1-srinivas.pandruvada@linux.intel.com> References: <20230211063257.311746-1-srinivas.pandruvada@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	platform/x86: ISST: Use TPMI interface \| [00/12] platform/x86: ISST: Use TPMI interface [01/12] platform/x86: ISST: Fix kernel documentation warnings [02/12] platform/x86: ISST: Add TPMI target [03/12] platform/x86: ISST: Add IOCTL default callback [04/12] platform/x86: ISST: Add API version of the target [05/12] platform/x86: ISST: Add support for MSR 0x54 [06/12] platform/x86: ISST: Enumerate TPMI SST and create framework [07/12] platform/x86: ISST: Parse SST MMIO and update instance [08/12] platform/x86: ISST: Add SST-CP support via TPMI [09/12] platform/x86: ISST: Add SST-PP support via TPMI [10/12] platform/x86: ISST: Add SST-BF support via TPMI [11/12] platform/x86: ISST: Add SST-TF support via TPMI [12/12] platform/x86: ISST: Add suspend/resume callbacks

Commit Message

srinivas pandruvada Feb. 11, 2023, 6:32 a.m. UTC

  To map Linux CPU numbering scheme to hardware CPU numbering scheme
MSR 0x53 is getting used. But for new generation of CPUs, this MSR
is not valid. Since this is model specific MSR, this is possible.

A new MSR 0x54 is defined. Use this MSR and convert the IOCTL format
to match existing MSR 0x53, in this case user spaces don't need to
be aware of this change.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../intel/speed_select_if/isst_if_common.c    | 51 +++++++++++++++++++
 1 file changed, 51 insertions(+)

Comments

Hans de Goede March 1, 2023, 2:30 p.m. UTC | #1

Hi,

On 2/11/23 07:32, Srinivas Pandruvada wrote:
> To map Linux CPU numbering scheme to hardware CPU numbering scheme
> MSR 0x53 is getting used. But for new generation of CPUs, this MSR
> is not valid. Since this is model specific MSR, this is possible.
> 
> A new MSR 0x54 is defined. Use this MSR and convert the IOCTL format
> to match existing MSR 0x53, in this case user spaces don't need to
> be aware of this change.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

I am not a fan of this. I expect that users of these new CPUs will
very likely also need a new intel-speed-select userspace tool regardless
of doing this MSR munging/shuffling in the kernel. So why not fix
the tool to teach it about the MSR instead ?

If you have good arguments for doing this in the kernel please
add them the commit message for the next version, but my initial
reaction to this is that it is wrong to do this in the kernel
and that the tool should be fixed instead. So my preference
would be for this patch to be dropped from the next version of
the patch-set.

Regards,

Hans







> ---
>  .../intel/speed_select_if/isst_if_common.c    | 51 +++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> index 60e58b0b3835..97d1b4566535 100644
> --- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> +++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> @@ -19,9 +19,13 @@
>  #include <linux/uaccess.h>
>  #include <uapi/linux/isst_if.h>
>  
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
> +
>  #include "isst_if_common.h"
>  
>  #define MSR_THREAD_ID_INFO	0x53
> +#define MSR_PM_LOGICAL_ID	0x54
>  #define MSR_CPU_BUS_NUMBER	0x128
>  
>  static struct isst_if_cmd_cb punit_callbacks[ISST_IF_DEV_MAX];
> @@ -31,6 +35,7 @@ static int punit_msr_white_list[] = {
>  	MSR_CONFIG_TDP_CONTROL,
>  	MSR_TURBO_RATIO_LIMIT1,
>  	MSR_TURBO_RATIO_LIMIT2,
> +	MSR_PM_LOGICAL_ID,
>  };
>  
>  struct isst_valid_cmd_ranges {
> @@ -73,6 +78,8 @@ struct isst_cmd {
>  	u32 param;
>  };
>  
> +static bool isst_hpm_support;
> +
>  static DECLARE_HASHTABLE(isst_hash, 8);
>  static DEFINE_MUTEX(isst_hash_lock);
>  
> @@ -411,11 +418,43 @@ static int isst_if_cpu_online(unsigned int cpu)
>  		isst_cpu_info[cpu].pci_dev[1] = _isst_if_get_pci_dev(cpu, 1, 30, 1);
>  	}
>  
> +	if (isst_hpm_support) {
> +		u64 raw_data;
> +
> +		ret = rdmsrl_safe(MSR_PM_LOGICAL_ID, &raw_data);
> +		if (!ret) {
> +			/*
> +			 * Use the same format as MSR 53, for user space harmony
> +			 *  Format
> +			 *	Bit 0 – thread ID
> +			 *	Bit 8:1 – core ID
> +			 *	Bit 13:9 – Compute domain ID (aka die ID)
> +			 * From the MSR 0x54 format
> +			 *	[15:11] PM_DOMAIN_ID
> +			 *	[10:3] MODULE_ID (aka IDI_AGENT_ID)
> +			 *	[2:0] LP_ID (We don't care about these bits we only
> +			 *			care die and core id
> +			 *	For Atom:
> +			 *		[2] Always 0
> +			 *		[1:0] core ID within module
> +			 *	For Core
> +			 *		[2:1] Always 0
> +			 *		[0] thread ID
> +			 */
> +			data = (raw_data >> 11) & 0x1f;
> +			data <<= 9;
> +			data |= (((raw_data >> 3) & 0xff) << 1);
> +			goto set_punit_id;
> +		}
> +	}
> +
>  	ret = rdmsrl_safe(MSR_THREAD_ID_INFO, &data);
>  	if (ret) {
>  		isst_cpu_info[cpu].punit_cpu_id = -1;
>  		return ret;
>  	}
> +
> +set_punit_id:
>  	isst_cpu_info[cpu].punit_cpu_id = data;
>  
>  	isst_restore_msr_local(cpu);
> @@ -704,6 +743,12 @@ static struct miscdevice isst_if_char_driver = {
>  	.fops		= &isst_if_char_driver_ops,
>  };
>  
> +static const struct x86_cpu_id hpm_cpu_ids[] = {
> +	X86_MATCH_INTEL_FAM6_MODEL(GRANITERAPIDS_X,	NULL),
> +	X86_MATCH_INTEL_FAM6_MODEL(SIERRAFOREST_X,	NULL),
> +	{}
> +};
> +
>  static int isst_misc_reg(void)
>  {
>  	mutex_lock(&punit_misc_dev_reg_lock);
> @@ -711,6 +756,12 @@ static int isst_misc_reg(void)
>  		goto unlock_exit;
>  
>  	if (!misc_usage_count) {
> +		const struct x86_cpu_id *id;
> +
> +		id = x86_match_cpu(hpm_cpu_ids);
> +		if (id)
> +			isst_hpm_support = true;
> +
>  		misc_device_ret = isst_if_cpu_info_init();
>  		if (misc_device_ret)
>  			goto unlock_exit;

srinivas pandruvada March 1, 2023, 2:41 p.m. UTC | #2

On Wed, 2023-03-01 at 15:30 +0100, Hans de Goede wrote:
> Hi,
> 
> On 2/11/23 07:32, Srinivas Pandruvada wrote:
> > To map Linux CPU numbering scheme to hardware CPU numbering scheme
> > MSR 0x53 is getting used. But for new generation of CPUs, this MSR
> > is not valid. Since this is model specific MSR, this is possible.
> > 
> > A new MSR 0x54 is defined. Use this MSR and convert the IOCTL
> > format
> > to match existing MSR 0x53, in this case user spaces don't need to
> > be aware of this change.
> > 
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com>
> 
> I am not a fan of this. I expect that users of these new CPUs will
> very likely also need a new intel-speed-select userspace tool
> regardless
> of doing this MSR munging/shuffling in the kernel. So why not fix
> the tool to teach it about the MSR instead ?

Sure.

I can remove the format conversion in the kernel, so that user space
tool will do that.

I think that's what you mean.



> 
> If you have good arguments for doing this in the kernel please
> add them the commit message for the next version, but my initial
> reaction to this is that it is wrong to do this in the kernel
> and that the tool should be fixed instead. So my preference
> would be for this patch to be dropped from the next version of
> the patch-set.

Since we can't read MSR from user space, this patch is still required
to read only MSR 0x54. Just it will not do any format conversion. So
format conversion will happen in user space tool.

Thanks,
Srinivas


> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> 
> 
> 
> > ---
> >  .../intel/speed_select_if/isst_if_common.c    | 51
> > +++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> > 
> > diff --git
> > a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> > b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> > index 60e58b0b3835..97d1b4566535 100644
> > --- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> > +++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> > @@ -19,9 +19,13 @@
> >  #include <linux/uaccess.h>
> >  #include <uapi/linux/isst_if.h>
> >  
> > +#include <asm/cpu_device_id.h>
> > +#include <asm/intel-family.h>
> > +
> >  #include "isst_if_common.h"
> >  
> >  #define MSR_THREAD_ID_INFO     0x53
> > +#define MSR_PM_LOGICAL_ID      0x54
> >  #define MSR_CPU_BUS_NUMBER     0x128
> >  
> >  static struct isst_if_cmd_cb punit_callbacks[ISST_IF_DEV_MAX];
> > @@ -31,6 +35,7 @@ static int punit_msr_white_list[] = {
> >         MSR_CONFIG_TDP_CONTROL,
> >         MSR_TURBO_RATIO_LIMIT1,
> >         MSR_TURBO_RATIO_LIMIT2,
> > +       MSR_PM_LOGICAL_ID,
> >  };
> >  
> >  struct isst_valid_cmd_ranges {
> > @@ -73,6 +78,8 @@ struct isst_cmd {
> >         u32 param;
> >  };
> >  
> > +static bool isst_hpm_support;
> > +
> >  static DECLARE_HASHTABLE(isst_hash, 8);
> >  static DEFINE_MUTEX(isst_hash_lock);
> >  
> > @@ -411,11 +418,43 @@ static int isst_if_cpu_online(unsigned int
> > cpu)
> >                 isst_cpu_info[cpu].pci_dev[1] =
> > _isst_if_get_pci_dev(cpu, 1, 30, 1);
> >         }
> >  
> > +       if (isst_hpm_support) {
> > +               u64 raw_data;
> > +
> > +               ret = rdmsrl_safe(MSR_PM_LOGICAL_ID, &raw_data);
> > +               if (!ret) {
> > +                       /*
> > +                        * Use the same format as MSR 53, for user
> > space harmony
> > +                        *  Format
> > +                        *      Bit 0 – thread ID
> > +                        *      Bit 8:1 – core ID
> > +                        *      Bit 13:9 – Compute domain ID (aka
> > die ID)
> > +                        * From the MSR 0x54 format
> > +                        *      [15:11] PM_DOMAIN_ID
> > +                        *      [10:3] MODULE_ID (aka IDI_AGENT_ID)
> > +                        *      [2:0] LP_ID (We don't care about
> > these bits we only
> > +                        *                      care die and core
> > id
> > +                        *      For Atom:
> > +                        *              [2] Always 0
> > +                        *              [1:0] core ID within module
> > +                        *      For Core
> > +                        *              [2:1] Always 0
> > +                        *              [0] thread ID
> > +                        */
> > +                       data = (raw_data >> 11) & 0x1f;
> > +                       data <<= 9;
> > +                       data |= (((raw_data >> 3) & 0xff) << 1);
> > +                       goto set_punit_id;
> > +               }
> > +       }
> > +
> >         ret = rdmsrl_safe(MSR_THREAD_ID_INFO, &data);
> >         if (ret) {
> >                 isst_cpu_info[cpu].punit_cpu_id = -1;
> >                 return ret;
> >         }
> > +
> > +set_punit_id:
> >         isst_cpu_info[cpu].punit_cpu_id = data;
> >  
> >         isst_restore_msr_local(cpu);
> > @@ -704,6 +743,12 @@ static struct miscdevice isst_if_char_driver =
> > {
> >         .fops           = &isst_if_char_driver_ops,
> >  };
> >  
> > +static const struct x86_cpu_id hpm_cpu_ids[] = {
> > +       X86_MATCH_INTEL_FAM6_MODEL(GRANITERAPIDS_X,     NULL),
> > +       X86_MATCH_INTEL_FAM6_MODEL(SIERRAFOREST_X,      NULL),
> > +       {}
> > +};
> > +
> >  static int isst_misc_reg(void)
> >  {
> >         mutex_lock(&punit_misc_dev_reg_lock);
> > @@ -711,6 +756,12 @@ static int isst_misc_reg(void)
> >                 goto unlock_exit;
> >  
> >         if (!misc_usage_count) {
> > +               const struct x86_cpu_id *id;
> > +
> > +               id = x86_match_cpu(hpm_cpu_ids);
> > +               if (id)
> > +                       isst_hpm_support = true;
> > +
> >                 misc_device_ret = isst_if_cpu_info_init();
> >                 if (misc_device_ret)
> >                         goto unlock_exit;
>

Hans de Goede March 1, 2023, 2:46 p.m. UTC | #3

Hi,

On 3/1/23 15:41, srinivas pandruvada wrote:
> On Wed, 2023-03-01 at 15:30 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 2/11/23 07:32, Srinivas Pandruvada wrote:
>>> To map Linux CPU numbering scheme to hardware CPU numbering scheme
>>> MSR 0x53 is getting used. But for new generation of CPUs, this MSR
>>> is not valid. Since this is model specific MSR, this is possible.
>>>
>>> A new MSR 0x54 is defined. Use this MSR and convert the IOCTL
>>> format
>>> to match existing MSR 0x53, in this case user spaces don't need to
>>> be aware of this change.
>>>
>>> Signed-off-by: Srinivas Pandruvada
>>> <srinivas.pandruvada@linux.intel.com>
>>
>> I am not a fan of this. I expect that users of these new CPUs will
>> very likely also need a new intel-speed-select userspace tool
>> regardless
>> of doing this MSR munging/shuffling in the kernel. So why not fix
>> the tool to teach it about the MSR instead ?
> 
> Sure.
> 
> I can remove the format conversion in the kernel, so that user space
> tool will do that.
> 
> I think that's what you mean.

Yes that is what I mean.

>> If you have good arguments for doing this in the kernel please
>> add them the commit message for the next version, but my initial
>> reaction to this is that it is wrong to do this in the kernel
>> and that the tool should be fixed instead. So my preference
>> would be for this patch to be dropped from the next version of
>> the patch-set.
> 
> Since we can't read MSR from user space, this patch is still required
> to read only MSR 0x54. Just it will not do any format conversion. So
> format conversion will happen in user space tool.

Yes that would be great, having the kernel read the MSR is fine
(of course).

Regards,

Hans



>>> ---
>>>  .../intel/speed_select_if/isst_if_common.c    | 51
>>> +++++++++++++++++++
>>>  1 file changed, 51 insertions(+)
>>>
>>> diff --git
>>> a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
>>> b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
>>> index 60e58b0b3835..97d1b4566535 100644
>>> --- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
>>> +++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
>>> @@ -19,9 +19,13 @@
>>>  #include <linux/uaccess.h>
>>>  #include <uapi/linux/isst_if.h>
>>>  
>>> +#include <asm/cpu_device_id.h>
>>> +#include <asm/intel-family.h>
>>> +
>>>  #include "isst_if_common.h"
>>>  
>>>  #define MSR_THREAD_ID_INFO     0x53
>>> +#define MSR_PM_LOGICAL_ID      0x54
>>>  #define MSR_CPU_BUS_NUMBER     0x128
>>>  
>>>  static struct isst_if_cmd_cb punit_callbacks[ISST_IF_DEV_MAX];
>>> @@ -31,6 +35,7 @@ static int punit_msr_white_list[] = {
>>>         MSR_CONFIG_TDP_CONTROL,
>>>         MSR_TURBO_RATIO_LIMIT1,
>>>         MSR_TURBO_RATIO_LIMIT2,
>>> +       MSR_PM_LOGICAL_ID,
>>>  };
>>>  
>>>  struct isst_valid_cmd_ranges {
>>> @@ -73,6 +78,8 @@ struct isst_cmd {
>>>         u32 param;
>>>  };
>>>  
>>> +static bool isst_hpm_support;
>>> +
>>>  static DECLARE_HASHTABLE(isst_hash, 8);
>>>  static DEFINE_MUTEX(isst_hash_lock);
>>>  
>>> @@ -411,11 +418,43 @@ static int isst_if_cpu_online(unsigned int
>>> cpu)
>>>                 isst_cpu_info[cpu].pci_dev[1] =
>>> _isst_if_get_pci_dev(cpu, 1, 30, 1);
>>>         }
>>>  
>>> +       if (isst_hpm_support) {
>>> +               u64 raw_data;
>>> +
>>> +               ret = rdmsrl_safe(MSR_PM_LOGICAL_ID, &raw_data);
>>> +               if (!ret) {
>>> +                       /*
>>> +                        * Use the same format as MSR 53, for user
>>> space harmony
>>> +                        *  Format
>>> +                        *      Bit 0 – thread ID
>>> +                        *      Bit 8:1 – core ID
>>> +                        *      Bit 13:9 – Compute domain ID (aka
>>> die ID)
>>> +                        * From the MSR 0x54 format
>>> +                        *      [15:11] PM_DOMAIN_ID
>>> +                        *      [10:3] MODULE_ID (aka IDI_AGENT_ID)
>>> +                        *      [2:0] LP_ID (We don't care about
>>> these bits we only
>>> +                        *                      care die and core
>>> id
>>> +                        *      For Atom:
>>> +                        *              [2] Always 0
>>> +                        *              [1:0] core ID within module
>>> +                        *      For Core
>>> +                        *              [2:1] Always 0
>>> +                        *              [0] thread ID
>>> +                        */
>>> +                       data = (raw_data >> 11) & 0x1f;
>>> +                       data <<= 9;
>>> +                       data |= (((raw_data >> 3) & 0xff) << 1);
>>> +                       goto set_punit_id;
>>> +               }
>>> +       }
>>> +
>>>         ret = rdmsrl_safe(MSR_THREAD_ID_INFO, &data);
>>>         if (ret) {
>>>                 isst_cpu_info[cpu].punit_cpu_id = -1;
>>>                 return ret;
>>>         }
>>> +
>>> +set_punit_id:
>>>         isst_cpu_info[cpu].punit_cpu_id = data;
>>>  
>>>         isst_restore_msr_local(cpu);
>>> @@ -704,6 +743,12 @@ static struct miscdevice isst_if_char_driver =
>>> {
>>>         .fops           = &isst_if_char_driver_ops,
>>>  };
>>>  
>>> +static const struct x86_cpu_id hpm_cpu_ids[] = {
>>> +       X86_MATCH_INTEL_FAM6_MODEL(GRANITERAPIDS_X,     NULL),
>>> +       X86_MATCH_INTEL_FAM6_MODEL(SIERRAFOREST_X,      NULL),
>>> +       {}
>>> +};
>>> +
>>>  static int isst_misc_reg(void)
>>>  {
>>>         mutex_lock(&punit_misc_dev_reg_lock);
>>> @@ -711,6 +756,12 @@ static int isst_misc_reg(void)
>>>                 goto unlock_exit;
>>>  
>>>         if (!misc_usage_count) {
>>> +               const struct x86_cpu_id *id;
>>> +
>>> +               id = x86_match_cpu(hpm_cpu_ids);
>>> +               if (id)
>>> +                       isst_hpm_support = true;
>>> +
>>>                 misc_device_ret = isst_if_cpu_info_init();
>>>                 if (misc_device_ret)
>>>                         goto unlock_exit;
>>
>

diff mbox series

Patch

diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
index 60e58b0b3835..97d1b4566535 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
@@ -19,9 +19,13 @@ 
 #include <linux/uaccess.h>
 #include <uapi/linux/isst_if.h>
 
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
+
 #include "isst_if_common.h"
 
 #define MSR_THREAD_ID_INFO	0x53
+#define MSR_PM_LOGICAL_ID	0x54
 #define MSR_CPU_BUS_NUMBER	0x128
 
 static struct isst_if_cmd_cb punit_callbacks[ISST_IF_DEV_MAX];
@@ -31,6 +35,7 @@  static int punit_msr_white_list[] = {
 	MSR_CONFIG_TDP_CONTROL,
 	MSR_TURBO_RATIO_LIMIT1,
 	MSR_TURBO_RATIO_LIMIT2,
+	MSR_PM_LOGICAL_ID,
 };
 
 struct isst_valid_cmd_ranges {
@@ -73,6 +78,8 @@  struct isst_cmd {
 	u32 param;
 };
 
+static bool isst_hpm_support;
+
 static DECLARE_HASHTABLE(isst_hash, 8);
 static DEFINE_MUTEX(isst_hash_lock);
 
@@ -411,11 +418,43 @@  static int isst_if_cpu_online(unsigned int cpu)
 		isst_cpu_info[cpu].pci_dev[1] = _isst_if_get_pci_dev(cpu, 1, 30, 1);
 	}
 
+	if (isst_hpm_support) {
+		u64 raw_data;
+
+		ret = rdmsrl_safe(MSR_PM_LOGICAL_ID, &raw_data);
+		if (!ret) {
+			/*
+			 * Use the same format as MSR 53, for user space harmony
+			 *  Format
+			 *	Bit 0 – thread ID
+			 *	Bit 8:1 – core ID
+			 *	Bit 13:9 – Compute domain ID (aka die ID)
+			 * From the MSR 0x54 format
+			 *	[15:11] PM_DOMAIN_ID
+			 *	[10:3] MODULE_ID (aka IDI_AGENT_ID)
+			 *	[2:0] LP_ID (We don't care about these bits we only
+			 *			care die and core id
+			 *	For Atom:
+			 *		[2] Always 0
+			 *		[1:0] core ID within module
+			 *	For Core
+			 *		[2:1] Always 0
+			 *		[0] thread ID
+			 */
+			data = (raw_data >> 11) & 0x1f;
+			data <<= 9;
+			data |= (((raw_data >> 3) & 0xff) << 1);
+			goto set_punit_id;
+		}
+	}
+
 	ret = rdmsrl_safe(MSR_THREAD_ID_INFO, &data);
 	if (ret) {
 		isst_cpu_info[cpu].punit_cpu_id = -1;
 		return ret;
 	}
+
+set_punit_id:
 	isst_cpu_info[cpu].punit_cpu_id = data;
 
 	isst_restore_msr_local(cpu);
@@ -704,6 +743,12 @@  static struct miscdevice isst_if_char_driver = {
 	.fops		= &isst_if_char_driver_ops,
 };
 
+static const struct x86_cpu_id hpm_cpu_ids[] = {
+	X86_MATCH_INTEL_FAM6_MODEL(GRANITERAPIDS_X,	NULL),
+	X86_MATCH_INTEL_FAM6_MODEL(SIERRAFOREST_X,	NULL),
+	{}
+};
+
 static int isst_misc_reg(void)
 {
 	mutex_lock(&punit_misc_dev_reg_lock);
@@ -711,6 +756,12 @@  static int isst_misc_reg(void)
 		goto unlock_exit;
 
 	if (!misc_usage_count) {
+		const struct x86_cpu_id *id;
+
+		id = x86_match_cpu(hpm_cpu_ids);
+		if (id)
+			isst_hpm_support = true;
+
 		misc_device_ret = isst_if_cpu_info_init();
 		if (misc_device_ret)
 			goto unlock_exit;