Re: [PATCH 5/7] perf/x86/intel/uncore: Support IIO free-running counters on DMR

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

On 12/22/2025 9:18 PM, Mi, Dapeng wrote:
> 
> On 12/13/2025 5:00 AM, Zide Chen wrote:
>> The free-running counters for IIO uncore blocks on Diamond Rapids are
>> similar to Sapphire Rapids IMC freecounters, with the following
>> differences:
>>
>> - The counters are MMIO based.
>> - Only a subset of IP blocks implement free-running counters:
>>   HIOP0 (IP Base Addr: 2E7000h)
>>   HIOP1 (IP Base Addr: 2EF000h)
>>   HIOP3 (IP Base Addr: 2FF000h)
>>   HIOP4 (IP Base Addr: 307000h)
>> - IMH2 (Secondary IMH) does not provide free-running counters.
>>
>> Signed-off-by: Zide Chen <zide.chen@intel.com>
>> ---
>>  arch/x86/events/intel/uncore_snbep.c | 120 +++++++++++++++++++++++++--
>>  1 file changed, 115 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
>> index 56c6ac86f28e..21cca1b28075 100644
>> --- a/arch/x86/events/intel/uncore_snbep.c
>> +++ b/arch/x86/events/intel/uncore_snbep.c
>> @@ -472,10 +472,14 @@
>>  #define SPR_C0_MSR_PMON_BOX_FILTER0		0x200e
>>  
>>  /* DMR */
>> +#define DMR_IMH1_HIOP_MMIO_BASE			0x1ffff6ae7000
>> +#define DMR_HIOP_MMIO_SIZE			0x8000
>>  #define DMR_CXLCM_EVENT_MASK_EXT		0xf
>>  #define DMR_HAMVF_EVENT_MASK_EXT		0xffffffff
>>  #define DMR_PCIE4_EVENT_MASK_EXT		0xffffff
>>  
>> +#define UNCORE_DMR_ITC				0x30
>> +
>>  #define DMR_IMC_PMON_FIXED_CTR			0x18
>>  #define DMR_IMC_PMON_FIXED_CTL			0x10
>>  
>> @@ -6442,7 +6446,11 @@ static int uncore_type_max_boxes(struct intel_uncore_type **types,
>>  	for (node = rb_first(type->boxes); node; node = rb_next(node)) {
>>  		unit = rb_entry(node, struct intel_uncore_discovery_unit, node);
>>  
>> -		if (unit->id > max)
>> +		/*
>> +		 * on DMR IMH2, the unit id starts from 0x8000,
>> +		 * and we don't need to count it.
>> +		 */
>> +		if ((unit->id > max) && (unit->id < 0x8000))
>>  			max = unit->id;
>>  	}
>>  	return max + 1;
>> @@ -6925,6 +6933,103 @@ int dmr_uncore_units_ignore[] = {
>>  	UNCORE_IGNORE_END
>>  };
>>  
>> +static unsigned int dmr_iio_freerunning_box_offsets[] = {
>> +	0x0, 0x8000, 0x18000, 0x20000
>> +};
>> +
>> +static void dmr_uncore_freerunning_init_box(struct intel_uncore_box *box)
>> +{
>> +	struct intel_uncore_type *type = box->pmu->type;
>> +	u64 mmio_base;
>> +
>> +	if (box->pmu->pmu_idx >= type->num_boxes) {
>> +		pr_warn("perf uncore: Failed to ioremap for %s.\n", type->name);
> 
> The warning message is not quite matched with the error case, please update it.


My mistake. This is only a sanity check to prevent out-of-bounds access
to dmr_iio_freerunning_box_offsets[] and should never be triggered in
practice. I think it is safe to remove the pr_warn().


> Other part looks good to me.