[PATCH] platform/x86/intel: power-domains validate domain in tpmi_cpu_online()

[PATCH] platform/x86/intel: power-domains validate domain in tpmi_cpu_online()

[David Arcari]

Although tpmi_get_power_domain_mask() calls tpmi_domain_is_valid()
prior to indexing tpmi_power_domain_mask[], tpmi_cpu_online() does
not. In the case where a VM creates non-contiguous package ids the
result can be memory corruption. This can be prevented by adding
the same validation in tpmi_cpu_online().

Fixes: 17ca2780458c ("platform/x86/intel: TPMI domain id and CPU mapping")

Cc: Hans de Goede <hansg@kernel.org>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: David Arcari <darcari@redhat.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Tero Kristo <tero.kristo@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: David Arcari <darcari@redhat.com>
---
 drivers/platform/x86/intel/tpmi_power_domains.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/platform/x86/intel/tpmi_power_domains.c b/drivers/platform/x86/intel/tpmi_power_domains.c
index 9d8247bb9cfa..ae5b58679e29 100644
--- a/drivers/platform/x86/intel/tpmi_power_domains.c
+++ b/drivers/platform/x86/intel/tpmi_power_domains.c
@@ -194,6 +194,9 @@ static int tpmi_cpu_online(unsigned int cpu)
 	if (ret)
 		return 0;
 
+	if (!tpmi_domain_is_valid(info))
+		return 0;
+
 	index = info->pkg_id * MAX_POWER_DOMAINS + info->punit_domain_id;
 
 	guard(mutex)(&tpmi_lock);
-- 
2.50.0

Re: [PATCH] platform/x86/intel: power-domains validate domain in tpmi_cpu_online()

[Andy Shevchenko]

Tue, Aug 26, 2025 at 12:43:28PM -0400, David Arcari kirjoitti:
> Although tpmi_get_power_domain_mask() calls tpmi_domain_is_valid()
> prior to indexing tpmi_power_domain_mask[], tpmi_cpu_online() does
> not. In the case where a VM creates non-contiguous package ids the
> result can be memory corruption. This can be prevented by adding
> the same validation in tpmi_cpu_online().
> 
> Fixes: 17ca2780458c ("platform/x86/intel: TPMI domain id and CPU mapping")

> 

Shouldn't be blank lines in the tag block.

> Cc: Hans de Goede <hansg@kernel.org>
> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: David Arcari <darcari@redhat.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Tero Kristo <tero.kristo@linux.intel.com>
> Cc: linux-kernel@vger.kernel.org

Can you keep Cc list below '---' line? It will have the same effect on emails,
but reduce unneeded noise in the commit message. If one wants to retrieve this
it is available on lore archive.

> Signed-off-by: David Arcari <darcari@redhat.com>
> ---

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] platform/x86/intel: power-domains validate domain in tpmi_cpu_online()

[srinivas pandruvada]

Hi David,

On Tue, 2025-08-26 at 12:43 -0400, David Arcari wrote:
> Although tpmi_get_power_domain_mask() calls tpmi_domain_is_valid()
> prior to indexing tpmi_power_domain_mask[],
Because this an API call so that caller parameter needs to be
sanitized.

>  tpmi_cpu_online() does
> not.
This is hotplug callback, which should have correct topology
information.

>  In the case where a VM creates non-contiguous package ids the
> result can be memory corruption. This can be prevented by adding
> the same validation in tpmi_cpu_online().
> 

This driver is getting loaded means MSR 0x54 is virtualised otherwise
this driver will not load.
Not sure this is an upstream kernel or not.

Some comments below.

> Fixes: 17ca2780458c ("platform/x86/intel: TPMI domain id and CPU
> mapping")
> 
Andy already pointed about new line here.

> Cc: Hans de Goede <hansg@kernel.org>
> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: David Arcari <darcari@redhat.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Tero Kristo <tero.kristo@linux.intel.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: David Arcari <darcari@redhat.com>
> ---
>  drivers/platform/x86/intel/tpmi_power_domains.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/tpmi_power_domains.c
> b/drivers/platform/x86/intel/tpmi_power_domains.c
> index 9d8247bb9cfa..ae5b58679e29 100644
> --- a/drivers/platform/x86/intel/tpmi_power_domains.c
> +++ b/drivers/platform/x86/intel/tpmi_power_domains.c
> @@ -194,6 +194,9 @@ static int tpmi_cpu_online(unsigned int cpu)
>  	if (ret)
>  		return 0;
>  
Need some more information.

The only case this check is required if
topology_physical_package_id(cpu) is returning greater or equal to
topology_max_packages(). If this true in this case, please check the
value of info->pkg_id. If this is bad then then some other places also
this may have issue. info->punit_domain_id is already checked for valid
value in tpmi_get_logical_id().


Thanks,
Srinivas

> +	if (!tpmi_domain_is_valid(info))
> +		return 0;
> +
>  	index = info->pkg_id * MAX_POWER_DOMAINS + info-
> >punit_domain_id;
>  
>  	guard(mutex)(&tpmi_lock);

Re: [PATCH] platform/x86/intel: power-domains validate domain in tpmi_cpu_online()

[David Arcari]

Hi Srinivas,

On 8/26/25 4:26 PM, srinivas pandruvada wrote:
> Hi David,
> 
> On Tue, 2025-08-26 at 12:43 -0400, David Arcari wrote:
>> Although tpmi_get_power_domain_mask() calls tpmi_domain_is_valid()
>> prior to indexing tpmi_power_domain_mask[],
> Because this an API call so that caller parameter needs to be
> sanitized.
> 
>>   tpmi_cpu_online() does
>> not.
> This is hotplug callback, which should have correct topology
> information.
> 
>>   In the case where a VM creates non-contiguous package ids the
>> result can be memory corruption. This can be prevented by adding
>> the same validation in tpmi_cpu_online().
>>
> 
> This driver is getting loaded means MSR 0x54 is virtualised otherwise
> this driver will not load.

I don't have direct access to the system, but this appears to be the 
case.  The driver is reading MSR 0x54:

drivers/platform/x86/intel/tpmi_power_domains.c:#define 
MSR_PM_LOGICAL_ID       0x54
drivers/platform/x86/intel/tpmi_power_domains.c:        ret = 
rdmsrl_safe(MSR_PM_LOGICAL_ID, &data);
drivers/platform/x86/intel/tpmi_power_domains.c:        ret = 
rdmsrl_safe(MSR_PM_LOGICAL_ID, &data);


> Not sure this is an upstream kernel or not.

This was not an upstream kernel, but I don't see anything in the 
upstream driver that would have prevented the access that is occurring.

> 
> Some comments below.
> 
>> Fixes: 17ca2780458c ("platform/x86/intel: TPMI domain id and CPU
>> mapping")
>>
> Andy already pointed about new line here.
> 
>> Cc: Hans de Goede <hansg@kernel.org>
>> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
>> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Dan Carpenter <dan.carpenter@linaro.org>
>> Cc: David Arcari <darcari@redhat.com>
>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Cc: Tero Kristo <tero.kristo@linux.intel.com>
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: David Arcari <darcari@redhat.com>
>> ---
>>   drivers/platform/x86/intel/tpmi_power_domains.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/tpmi_power_domains.c
>> b/drivers/platform/x86/intel/tpmi_power_domains.c
>> index 9d8247bb9cfa..ae5b58679e29 100644
>> --- a/drivers/platform/x86/intel/tpmi_power_domains.c
>> +++ b/drivers/platform/x86/intel/tpmi_power_domains.c
>> @@ -194,6 +194,9 @@ static int tpmi_cpu_online(unsigned int cpu)
>>   	if (ret)
>>   		return 0;
>>   
> Need some more information.
> 
> The only case this check is required if
> topology_physical_package_id(cpu) is returning greater or equal to
> topology_max_packages(). If this true in this case, please check the
> value of info->pkg_id. If this is bad then then some other places also
> this may have issue. info->punit_domain_id is already checked for valid
> value in tpmi_get_logical_id().

That is correct.  In the case of the crash we have:

crash> p/x __max_logical_packages
$1 = 0x2

static inline unsigned int topology_max_packages(void)
{
	return __max_logical_packages;
}


$2 = {
   hnode = {
     next = 0xffff9651bbc37010,
     pprev = 0xffffffffc0b7a640 <tpmi_cpu_hash>
   },
   linux_cpu = 1,
   pkg_id = 2 '\002',
   punit_thread_id = 0 '\000',
   punit_core_id = 0 '\000',
   punit_domain_id = 0 '\000'
}

The pkg_id of 2 leads to the bad reference.

FWIW this change has been tested and resolves the issue.

Let me know if there is any other information I can provide.  I will be 
out of the office on Wednesday, so response may be delayed.

Best,
-DA

> 
> Thanks,
> Srinivas
> 
>> +	if (!tpmi_domain_is_valid(info))
>> +		return 0;
>> +
>>   	index = info->pkg_id * MAX_POWER_DOMAINS + info-
>>> punit_domain_id;
>>   
>>   	guard(mutex)(&tpmi_lock);
> 
> 

Re: [PATCH] platform/x86/intel: power-domains validate domain in tpmi_cpu_online()

[srinivas pandruvada]

On Tue, 2025-08-26 at 21:39 -0400, David Arcari wrote:
> 
> Hi Srinivas,
> 
> On 8/26/25 4:26 PM, srinivas pandruvada wrote:
> > Hi David,
> > 
> > On Tue, 2025-08-26 at 12:43 -0400, David Arcari wrote:
> > > Although tpmi_get_power_domain_mask() calls
> > > tpmi_domain_is_valid()
> > > prior to indexing tpmi_power_domain_mask[],
> > Because this an API call so that caller parameter needs to be
> > sanitized.
> > 
> > >   tpmi_cpu_online() does
> > > not.
> > This is hotplug callback, which should have correct topology
> > information.
> > 
> > >   In the case where a VM creates non-contiguous package ids the
> > > result can be memory corruption. This can be prevented by adding
> > > the same validation in tpmi_cpu_online().
> > > 
> > 
> > This driver is getting loaded means MSR 0x54 is virtualised
> > otherwise
> > this driver will not load.
> 
> I don't have direct access to the system, but this appears to be the 
> case.  The driver is reading MSR 0x54:
> 
> drivers/platform/x86/intel/tpmi_power_domains.c:#define 
> MSR_PM_LOGICAL_ID       0x54
> drivers/platform/x86/intel/tpmi_power_domains.c:        ret = 
> rdmsrl_safe(MSR_PM_LOGICAL_ID, &data);
> drivers/platform/x86/intel/tpmi_power_domains.c:        ret = 
> rdmsrl_safe(MSR_PM_LOGICAL_ID, &data);
> 
> 
> > Not sure this is an upstream kernel or not.
> 
> This was not an upstream kernel, but I don't see anything in the 
> upstream driver that would have prevented the access that is
> occurring.
> 
The issue here the topology_max_packages() is 2 but cpu 1 package ID is
also 2. So everywhere topology_max_packages() is used there may be
issue as you have to verify the package ID is fine.


Repost the patch by adding the above root cause in the description, so
we know why we need this change.

Thanks,
Srinivas

> > 
> > Some comments below.
> > 
> > > Fixes: 17ca2780458c ("platform/x86/intel: TPMI domain id and CPU
> > > mapping")
> > > 
> > Andy already pointed about new line here.
> > 
> > > Cc: Hans de Goede <hansg@kernel.org>
> > > Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> > > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Ingo Molnar <mingo@kernel.org>
> > > Cc: Dan Carpenter <dan.carpenter@linaro.org>
> > > Cc: David Arcari <darcari@redhat.com>
> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Cc: Tero Kristo <tero.kristo@linux.intel.com>
> > > Cc: linux-kernel@vger.kernel.org
> > > Signed-off-by: David Arcari <darcari@redhat.com>
> > > ---
> > >   drivers/platform/x86/intel/tpmi_power_domains.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/platform/x86/intel/tpmi_power_domains.c
> > > b/drivers/platform/x86/intel/tpmi_power_domains.c
> > > index 9d8247bb9cfa..ae5b58679e29 100644
> > > --- a/drivers/platform/x86/intel/tpmi_power_domains.c
> > > +++ b/drivers/platform/x86/intel/tpmi_power_domains.c
> > > @@ -194,6 +194,9 @@ static int tpmi_cpu_online(unsigned int cpu)
> > >   	if (ret)
> > >   		return 0;
> > >   
> > Need some more information.
> > 
> > The only case this check is required if
> > topology_physical_package_id(cpu) is returning greater or equal to
> > topology_max_packages(). If this true in this case, please check
> > the
> > value of info->pkg_id. If this is bad then then some other places
> > also
> > this may have issue. info->punit_domain_id is already checked for
> > valid
> > value in tpmi_get_logical_id().
> 
> That is correct.  In the case of the crash we have:
> 
> crash> p/x __max_logical_packages
> $1 = 0x2
> 
> static inline unsigned int topology_max_packages(void)
> {
> 	return __max_logical_packages;
> }
> 
> 
> $2 = {
>    hnode = {
>      next = 0xffff9651bbc37010,
>      pprev = 0xffffffffc0b7a640 <tpmi_cpu_hash>
>    },
>    linux_cpu = 1,
>    pkg_id = 2 '\002',
>    punit_thread_id = 0 '\000',
>    punit_core_id = 0 '\000',
>    punit_domain_id = 0 '\000'
> }
> 
> The pkg_id of 2 leads to the bad reference.
> 
> FWIW this change has been tested and resolves the issue.
> 
> Let me know if there is any other information I can provide.  I will
> be 
> out of the office on Wednesday, so response may be delayed.
> 
> Best,
> -DA
> 
> > 
> > Thanks,
> > Srinivas
> > 
> > > +	if (!tpmi_domain_is_valid(info))
> > > +		return 0;
> > > +
> > >   	index = info->pkg_id * MAX_POWER_DOMAINS + info-
> > > > punit_domain_id;
> > >   
> > >   	guard(mutex)(&tpmi_lock);
> > 
> > 
> 

Re: [PATCH] platform/x86/intel: power-domains validate domain in tpmi_cpu_online()

[David Arcari]

On 8/27/25 7:49 AM, srinivas pandruvada wrote:
> On Tue, 2025-08-26 at 21:39 -0400, David Arcari wrote:
>>
>> Hi Srinivas,
>>
>> On 8/26/25 4:26 PM, srinivas pandruvada wrote:
>>> Hi David,
>>>
>>> On Tue, 2025-08-26 at 12:43 -0400, David Arcari wrote:
>>>> Although tpmi_get_power_domain_mask() calls
>>>> tpmi_domain_is_valid()
>>>> prior to indexing tpmi_power_domain_mask[],
>>> Because this an API call so that caller parameter needs to be
>>> sanitized.
>>>
>>>>    tpmi_cpu_online() does
>>>> not.
>>> This is hotplug callback, which should have correct topology
>>> information.
>>>
>>>>    In the case where a VM creates non-contiguous package ids the
>>>> result can be memory corruption. This can be prevented by adding
>>>> the same validation in tpmi_cpu_online().
>>>>
>>>
>>> This driver is getting loaded means MSR 0x54 is virtualised
>>> otherwise
>>> this driver will not load.
>>
>> I don't have direct access to the system, but this appears to be the
>> case.  The driver is reading MSR 0x54:
>>
>> drivers/platform/x86/intel/tpmi_power_domains.c:#define
>> MSR_PM_LOGICAL_ID       0x54
>> drivers/platform/x86/intel/tpmi_power_domains.c:        ret =
>> rdmsrl_safe(MSR_PM_LOGICAL_ID, &data);
>> drivers/platform/x86/intel/tpmi_power_domains.c:        ret =
>> rdmsrl_safe(MSR_PM_LOGICAL_ID, &data);
>>
>>
>>> Not sure this is an upstream kernel or not.
>>
>> This was not an upstream kernel, but I don't see anything in the
>> upstream driver that would have prevented the access that is
>> occurring.
>>
> The issue here the topology_max_packages() is 2 but cpu 1 package ID is
> also 2. So everywhere topology_max_packages() is used there may be
> issue as you have to verify the package ID is fine.

Correct.  There are two logical packages on this system, but they are 0 
and 2.

> 
> 
> Repost the patch by adding the above root cause in the description, so
> we know why we need this change.

I will do that.

Best,
-DA

> 
> Thanks,
> Srinivas
> 
>>>
>>> Some comments below.
>>>
>>>> Fixes: 17ca2780458c ("platform/x86/intel: TPMI domain id and CPU
>>>> mapping")
>>>>
>>> Andy already pointed about new line here.
>>>
>>>> Cc: Hans de Goede <hansg@kernel.org>
>>>> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
>>>> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>>> Cc: Ingo Molnar <mingo@kernel.org>
>>>> Cc: Dan Carpenter <dan.carpenter@linaro.org>
>>>> Cc: David Arcari <darcari@redhat.com>
>>>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>> Cc: Tero Kristo <tero.kristo@linux.intel.com>
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: David Arcari <darcari@redhat.com>
>>>> ---
>>>>    drivers/platform/x86/intel/tpmi_power_domains.c | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/platform/x86/intel/tpmi_power_domains.c
>>>> b/drivers/platform/x86/intel/tpmi_power_domains.c
>>>> index 9d8247bb9cfa..ae5b58679e29 100644
>>>> --- a/drivers/platform/x86/intel/tpmi_power_domains.c
>>>> +++ b/drivers/platform/x86/intel/tpmi_power_domains.c
>>>> @@ -194,6 +194,9 @@ static int tpmi_cpu_online(unsigned int cpu)
>>>>    	if (ret)
>>>>    		return 0;
>>>>    
>>> Need some more information.
>>>
>>> The only case this check is required if
>>> topology_physical_package_id(cpu) is returning greater or equal to
>>> topology_max_packages(). If this true in this case, please check
>>> the
>>> value of info->pkg_id. If this is bad then then some other places
>>> also
>>> this may have issue. info->punit_domain_id is already checked for
>>> valid
>>> value in tpmi_get_logical_id().
>>
>> That is correct.  In the case of the crash we have:
>>
>> crash> p/x __max_logical_packages
>> $1 = 0x2
>>
>> static inline unsigned int topology_max_packages(void)
>> {
>> 	return __max_logical_packages;
>> }
>>
>>
>> $2 = {
>>     hnode = {
>>       next = 0xffff9651bbc37010,
>>       pprev = 0xffffffffc0b7a640 <tpmi_cpu_hash>
>>     },
>>     linux_cpu = 1,
>>     pkg_id = 2 '\002',
>>     punit_thread_id = 0 '\000',
>>     punit_core_id = 0 '\000',
>>     punit_domain_id = 0 '\000'
>> }
>>
>> The pkg_id of 2 leads to the bad reference.
>>
>> FWIW this change has been tested and resolves the issue.
>>
>> Let me know if there is any other information I can provide.  I will
>> be
>> out of the office on Wednesday, so response may be delayed.
>>
>> Best,
>> -DA
>>
>>>
>>> Thanks,
>>> Srinivas
>>>
>>>> +	if (!tpmi_domain_is_valid(info))
>>>> +		return 0;
>>>> +
>>>>    	index = info->pkg_id * MAX_POWER_DOMAINS + info-
>>>>> punit_domain_id;
>>>>    
>>>>    	guard(mutex)(&tpmi_lock);
>>>
>>>
>>
> 

Re: [PATCH] platform/x86/intel: power-domains validate domain in tpmi_cpu_online()

[srinivas pandruvada]

On Thu, 2025-08-28 at 07:27 -0400, David Arcari wrote:
> 
> 
> On 8/27/25 7:49 AM, srinivas pandruvada wrote:
> > On Tue, 2025-08-26 at 21:39 -0400, David Arcari wrote:
> > > 
> > > Hi Srinivas,
> > > 
> > > On 8/26/25 4:26 PM, srinivas pandruvada wrote:
> > > > Hi David,
> > > > 
> > > > On Tue, 2025-08-26 at 12:43 -0400, David Arcari wrote:
> > > > > Although tpmi_get_power_domain_mask() calls
> > > > > tpmi_domain_is_valid()
> > > > > prior to indexing tpmi_power_domain_mask[],
> > > > Because this an API call so that caller parameter needs to be
> > > > sanitized.
> > > > 
> > > > >    tpmi_cpu_online() does
> > > > > not.
> > > > This is hotplug callback, which should have correct topology
> > > > information.
> > > > 
> > > > >    In the case where a VM creates non-contiguous package ids
> > > > > the
> > > > > result can be memory corruption. This can be prevented by
> > > > > adding
> > > > > the same validation in tpmi_cpu_online().
> > > > > 
> > > > 
> > > > This driver is getting loaded means MSR 0x54 is virtualised
> > > > otherwise
> > > > this driver will not load.
> > > 
> > > I don't have direct access to the system, but this appears to be
> > > the
> > > case.  The driver is reading MSR 0x54:
> > > 
> > > drivers/platform/x86/intel/tpmi_power_domains.c:#define
> > > MSR_PM_LOGICAL_ID       0x54
> > > drivers/platform/x86/intel/tpmi_power_domains.c:        ret =
> > > rdmsrl_safe(MSR_PM_LOGICAL_ID, &data);
> > > drivers/platform/x86/intel/tpmi_power_domains.c:        ret =
> > > rdmsrl_safe(MSR_PM_LOGICAL_ID, &data);
> > > 
> > > 
> > > > Not sure this is an upstream kernel or not.
> > > 
> > > This was not an upstream kernel, but I don't see anything in the
> > > upstream driver that would have prevented the access that is
> > > occurring.
> > > 
> > The issue here the topology_max_packages() is 2 but cpu 1 package
> > ID is
> > also 2. So everywhere topology_max_packages() is used there may be
> > issue as you have to verify the package ID is fine.
> 
> Correct.  There are two logical packages on this system, but they are
> 0 
> and 2.
> 
> > 
> > 
> > Repost the patch by adding the above root cause in the description,
> > so
> > we know why we need this change.
> 
> I will do that.
> 
Also this customer contacted me. In their case they don't GP fault on
the MSR 0x54 so driver is loaded.

Thanks,
Srinivas

> Best,
> -DA
> 
> > 
> > Thanks,
> > Srinivas
> > 
> > > > 
> > > > Some comments below.
> > > > 
> > > > > Fixes: 17ca2780458c ("platform/x86/intel: TPMI domain id and
> > > > > CPU
> > > > > mapping")
> > > > > 
> > > > Andy already pointed about new line here.
> > > > 
> > > > > Cc: Hans de Goede <hansg@kernel.org>
> > > > > Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> > > > > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > > > Cc: Ingo Molnar <mingo@kernel.org>
> > > > > Cc: Dan Carpenter <dan.carpenter@linaro.org>
> > > > > Cc: David Arcari <darcari@redhat.com>
> > > > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > Cc: Tero Kristo <tero.kristo@linux.intel.com>
> > > > > Cc: linux-kernel@vger.kernel.org
> > > > > Signed-off-by: David Arcari <darcari@redhat.com>
> > > > > ---
> > > > >    drivers/platform/x86/intel/tpmi_power_domains.c | 3 +++
> > > > >    1 file changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/platform/x86/intel/tpmi_power_domains.c
> > > > > b/drivers/platform/x86/intel/tpmi_power_domains.c
> > > > > index 9d8247bb9cfa..ae5b58679e29 100644
> > > > > --- a/drivers/platform/x86/intel/tpmi_power_domains.c
> > > > > +++ b/drivers/platform/x86/intel/tpmi_power_domains.c
> > > > > @@ -194,6 +194,9 @@ static int tpmi_cpu_online(unsigned int
> > > > > cpu)
> > > > >    	if (ret)
> > > > >    		return 0;
> > > > >    
> > > > Need some more information.
> > > > 
> > > > The only case this check is required if
> > > > topology_physical_package_id(cpu) is returning greater or equal
> > > > to
> > > > topology_max_packages(). If this true in this case, please
> > > > check
> > > > the
> > > > value of info->pkg_id. If this is bad then then some other
> > > > places
> > > > also
> > > > this may have issue. info->punit_domain_id is already checked
> > > > for
> > > > valid
> > > > value in tpmi_get_logical_id().
> > > 
> > > That is correct.  In the case of the crash we have:
> > > 
> > > crash> p/x __max_logical_packages
> > > $1 = 0x2
> > > 
> > > static inline unsigned int topology_max_packages(void)
> > > {
> > > 	return __max_logical_packages;
> > > }
> > > 
> > > 
> > > $2 = {
> > >     hnode = {
> > >       next = 0xffff9651bbc37010,
> > >       pprev = 0xffffffffc0b7a640 <tpmi_cpu_hash>
> > >     },
> > >     linux_cpu = 1,
> > >     pkg_id = 2 '\002',
> > >     punit_thread_id = 0 '\000',
> > >     punit_core_id = 0 '\000',
> > >     punit_domain_id = 0 '\000'
> > > }
> > > 
> > > The pkg_id of 2 leads to the bad reference.
> > > 
> > > FWIW this change has been tested and resolves the issue.
> > > 
> > > Let me know if there is any other information I can provide.  I
> > > will
> > > be
> > > out of the office on Wednesday, so response may be delayed.
> > > 
> > > Best,
> > > -DA
> > > 
> > > > 
> > > > Thanks,
> > > > Srinivas
> > > > 
> > > > > +	if (!tpmi_domain_is_valid(info))
> > > > > +		return 0;
> > > > > +
> > > > >    	index = info->pkg_id * MAX_POWER_DOMAINS + info-
> > > > > > punit_domain_id;
> > > > >    
> > > > >    	guard(mutex)(&tpmi_lock);
> > > > 
> > > > 
> > > 
> > 
> 

[PATCH v2] platform/x86/intel: power-domains: Use topology_logical_package_id() for package ID

[David Arcari]

Currently, tpmi_get_logical_id() calls topology_physical_package_id()
to set the pkg_id of the info structure. Since some VM hosts assign non
contiguous package IDs, topology_physical_package_id() can return a
larger value than topology_max_packages(). This will result in an
invalid reference into tpmi_power_domain_mask[] as that is allocatead
based on topology_max_packages() as the maximum package ID.

Fixes: 17ca2780458c ("platform/x86/intel: TPMI domain id and CPU mapping")
Signed-off-by: David Arcari <darcari@redhat.com>
---
v2: fixed underlying issue in tpmi_get_logcal_id() instead of
    preventing access in tpmi_cpu_online().

Cc: Hans de Goede <hansg@kernel.org>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: David Arcari <darcari@redhat.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Tero Kristo <tero.kristo@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
 drivers/platform/x86/intel/tpmi_power_domains.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/tpmi_power_domains.c b/drivers/platform/x86/intel/tpmi_power_domains.c
index 9d8247bb9cfa..8641353b2e06 100644
--- a/drivers/platform/x86/intel/tpmi_power_domains.c
+++ b/drivers/platform/x86/intel/tpmi_power_domains.c
@@ -178,7 +178,7 @@ static int tpmi_get_logical_id(unsigned int cpu, struct tpmi_cpu_info *info)
 
 	info->punit_thread_id = FIELD_GET(LP_ID_MASK, data);
 	info->punit_core_id = FIELD_GET(MODULE_ID_MASK, data);
-	info->pkg_id = topology_physical_package_id(cpu);
+	info->pkg_id = topology_logical_package_id(cpu);
 	info->linux_cpu = cpu;
 
 	return 0;
-- 
2.50.0

Re: [PATCH v2] platform/x86/intel: power-domains: Use topology_logical_package_id() for package ID

[srinivas pandruvada]

On Fri, 2025-08-29 at 07:38 -0400, David Arcari wrote:
> Currently, tpmi_get_logical_id() calls topology_physical_package_id()
> to set the pkg_id of the info structure. Since some VM hosts assign
> non
> contiguous package IDs, topology_physical_package_id() can return a
> larger value than topology_max_packages(). This will result in an
> invalid reference into tpmi_power_domain_mask[] as that is allocatead
> based on topology_max_packages() as the maximum package ID.
> 
> Fixes: 17ca2780458c ("platform/x86/intel: TPMI domain id and CPU
> mapping")
> Signed-off-by: David Arcari <darcari@redhat.com>
    Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> ---
> v2: fixed underlying issue in tpmi_get_logcal_id() instead of
>     preventing access in tpmi_cpu_online().
> 
> Cc: Hans de Goede <hansg@kernel.org>
> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: David Arcari <darcari@redhat.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Tero Kristo <tero.kristo@linux.intel.com>
> Cc: linux-kernel@vger.kernel.org
>  drivers/platform/x86/intel/tpmi_power_domains.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/tpmi_power_domains.c
> b/drivers/platform/x86/intel/tpmi_power_domains.c
> index 9d8247bb9cfa..8641353b2e06 100644
> --- a/drivers/platform/x86/intel/tpmi_power_domains.c
> +++ b/drivers/platform/x86/intel/tpmi_power_domains.c
> @@ -178,7 +178,7 @@ static int tpmi_get_logical_id(unsigned int cpu,
> struct tpmi_cpu_info *info)
>  
>  	info->punit_thread_id = FIELD_GET(LP_ID_MASK, data);
>  	info->punit_core_id = FIELD_GET(MODULE_ID_MASK, data);
> -	info->pkg_id = topology_physical_package_id(cpu);
> +	info->pkg_id = topology_logical_package_id(cpu);
>  	info->linux_cpu = cpu;
>  
>  	return 0;

Re: [PATCH v2] platform/x86/intel: power-domains: Use topology_logical_package_id() for package ID

[Ilpo Järvinen]

On Fri, 29 Aug 2025 07:38:59 -0400, David Arcari wrote:

> Currently, tpmi_get_logical_id() calls topology_physical_package_id()
> to set the pkg_id of the info structure. Since some VM hosts assign non
> contiguous package IDs, topology_physical_package_id() can return a
> larger value than topology_max_packages(). This will result in an
> invalid reference into tpmi_power_domain_mask[] as that is allocatead
> based on topology_max_packages() as the maximum package ID.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-fixes branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-fixes branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/1] platform/x86/intel: power-domains: Use topology_logical_package_id() for package ID
      commit: aa28991fd5dc4c01a40caab2bd9af8c5e06f9899

--
 i.