Re: [PATCH V2 7/7] perf/x86/intel/uncore: Implement global init callback for GNR uncore

["Chen, Zide" <zide.chen@intel.com>]

On 5/28/2026 1:46 AM, Mi, Dapeng wrote:
> 
> On 5/27/2026 11:11 PM, Zide Chen wrote:
>> On Sierra Forest and Clearwater Forest, the FRZ_ALL bit in the global
>> control register defaults to 0 at boot, but UBOX PMON units do not
>> work until the global control register is explicitly written with 0
>> to trigger hardware initialization properly.
>>
>> Implement the generic uncore_msr_global_init() callback and add it to
>> gnr_uncore_init[], which is shared by GNR, GRR, SRF, and CWF.
> 
> Need a "Fixes" tag?
No Fixes tag needed. This is a hardware initialization workaround rather
than a fix for a software bug. The register defaults to 0, but the
hardware requires an explicit write to trigger PMON functionality.

> Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> 
> 
>> Signed-off-by: Zide Chen <zide.chen@intel.com>
>> ---
>> V2:
>> - Propagate return value of wrmsrq_on_cpu() to global_init().
>> ---
>>  arch/x86/events/intel/uncore.c           | 13 ++++++++++++-
>>  arch/x86/events/intel/uncore.h           |  2 +-
>>  arch/x86/events/intel/uncore_discovery.c |  2 +-
>>  3 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
>> index 4b3a1fa5b41b..7857959c6e82 100644
>> --- a/arch/x86/events/intel/uncore.c
>> +++ b/arch/x86/events/intel/uncore.c
>> @@ -1716,7 +1716,7 @@ static int __init uncore_mmio_init(void)
>>  	return ret;
>>  }
>>  
>> -static int uncore_mmio_global_init(u64 ctl)
>> +static int uncore_mmio_global_init(int die, u64 ctl)
>>  {
>>  	void __iomem *io_addr;
>>  
>> @@ -1731,6 +1731,16 @@ static int uncore_mmio_global_init(u64 ctl)
>>  	return 0;
>>  }
>>  
>> +static int uncore_msr_global_init(int die, u64 msr)
>> +{
>> +	int cpu = uncore_die_to_cpu(die);
>> +
>> +	if (cpu == -1)
>> +		return -ENODEV;
>> +
>> +	return wrmsrq_on_cpu(cpu, msr, 0);
>> +}
>> +
>>  static const struct uncore_plat_init nhm_uncore_init __initconst = {
>>  	.cpu_init = nhm_uncore_cpu_init,
>>  };
>> @@ -1871,6 +1881,7 @@ static const struct uncore_plat_init gnr_uncore_init __initconst = {
>>  	.domain[0].base_is_pci = true,
>>  	.domain[0].discovery_base = UNCORE_DISCOVERY_TABLE_DEVICE,
>>  	.domain[0].units_ignore = gnr_uncore_units_ignore,
>> +	.domain[0].global_init = uncore_msr_global_init,
>>  };
>>  
>>  static const struct uncore_plat_init dmr_uncore_init __initconst = {
>> diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h
>> index 94c68e3417b6..c2e5ccb1d72c 100644
>> --- a/arch/x86/events/intel/uncore.h
>> +++ b/arch/x86/events/intel/uncore.h
>> @@ -53,7 +53,7 @@ struct uncore_discovery_domain {
>>  	/* MSR address or PCI device used as the discovery base */
>>  	u32	discovery_base;
>>  	bool	base_is_pci;
>> -	int	(*global_init)(u64 ctl);
>> +	int	(*global_init)(int die, u64 ctl);
>>  
>>  	/* The units in the discovery table should be ignored. */
>>  	int	*units_ignore;
>> diff --git a/arch/x86/events/intel/uncore_discovery.c b/arch/x86/events/intel/uncore_discovery.c
>> index af2217b44a81..e36613d934b1 100644
>> --- a/arch/x86/events/intel/uncore_discovery.c
>> +++ b/arch/x86/events/intel/uncore_discovery.c
>> @@ -287,7 +287,7 @@ static int __parse_discovery_table(struct uncore_discovery_domain *domain,
>>  	if (!io_addr)
>>  		return -ENOMEM;
>>  
>> -	if (domain->global_init && domain->global_init(global.ctl)) {
>> +	if (domain->global_init && domain->global_init(die, global.ctl)) {
>>  		ret = -ENODEV;
>>  		goto out;
>>  	}