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

[Mi, Dapeng]

On 5/29/2026 2:14 AM, Chen, Zide wrote:
>
> 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.

Zide, per my understanding, some uncore PMUs can't work for SRF and CWF
without this change. Is it right? If so, we need to add a "Fixes" tag to
ensure this patch is merged to the corresponding stable branches. Thanks.


>
>> Reviewed-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
>>
>>
>>> Signed-off-by: Zide Chen <zide.chen@xxxxxxxxx>
>>> ---
>>> 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;
>>>  	}
>