Re: [PATCH V3 1/4] perf/x86/intel/uncore: Support MSR portal for discovery tables

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 7/4/2025 12:56 AM, kan.liang@linux.intel.com wrote:
> From: Kan Liang <kan.liang@linux.intel.com>
>
> Starting from the Panther Lake, the discovery table mechanism is also
> supported in client platforms. The difference is that the portal of the
> global discovery table is retrieved from an MSR.
>
> The layout of discovery tables are the same as the server platforms.
> Factor out __parse_discovery_table() to parse discover tables.
>
> The uncore PMON is Die scope. Need to parse the discovery tables for
> each die.
>
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> ---
>  arch/x86/events/intel/uncore_discovery.c | 87 ++++++++++++++++++------
>  arch/x86/events/intel/uncore_discovery.h |  3 +
>  2 files changed, 70 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/events/intel/uncore_discovery.c b/arch/x86/events/intel/uncore_discovery.c
> index 571e44b49691..8680f66c3e34 100644
> --- a/arch/x86/events/intel/uncore_discovery.c
> +++ b/arch/x86/events/intel/uncore_discovery.c
> @@ -273,32 +273,15 @@ uncore_ignore_unit(struct uncore_unit_discovery *unit, int *ignore)
>  	return false;
>  }
>  
> -static int parse_discovery_table(struct pci_dev *dev, int die,
> -				 u32 bar_offset, bool *parsed,
> -				 int *ignore)
> +static int __parse_discovery_table(resource_size_t addr, int die,
> +				   bool *parsed, int *ignore)
>  {
>  	struct uncore_global_discovery global;
>  	struct uncore_unit_discovery unit;
>  	void __iomem *io_addr;
> -	resource_size_t addr;
>  	unsigned long size;
> -	u32 val;
>  	int i;
>  
> -	pci_read_config_dword(dev, bar_offset, &val);
> -
> -	if (val & ~PCI_BASE_ADDRESS_MEM_MASK & ~PCI_BASE_ADDRESS_MEM_TYPE_64)
> -		return -EINVAL;
> -
> -	addr = (resource_size_t)(val & PCI_BASE_ADDRESS_MEM_MASK);
> -#ifdef CONFIG_PHYS_ADDR_T_64BIT
> -	if ((val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64) {
> -		u32 val2;
> -
> -		pci_read_config_dword(dev, bar_offset + 4, &val2);
> -		addr |= ((resource_size_t)val2) << 32;
> -	}
> -#endif
>  	size = UNCORE_DISCOVERY_GLOBAL_MAP_SIZE;
>  	io_addr = ioremap(addr, size);
>  	if (!io_addr)
> @@ -341,7 +324,32 @@ static int parse_discovery_table(struct pci_dev *dev, int die,
>  	return 0;
>  }
>  
> -bool intel_uncore_has_discovery_tables(int *ignore)
> +static int parse_discovery_table(struct pci_dev *dev, int die,
> +				 u32 bar_offset, bool *parsed,
> +				 int *ignore)
> +{
> +	resource_size_t addr;
> +	u32 val;
> +
> +	pci_read_config_dword(dev, bar_offset, &val);
> +
> +	if (val & ~PCI_BASE_ADDRESS_MEM_MASK & ~PCI_BASE_ADDRESS_MEM_TYPE_64)
> +		return -EINVAL;
> +
> +	addr = (resource_size_t)(val & PCI_BASE_ADDRESS_MEM_MASK);
> +#ifdef CONFIG_PHYS_ADDR_T_64BIT
> +	if ((val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) == PCI_BASE_ADDRESS_MEM_TYPE_64) {
> +		u32 val2;
> +
> +		pci_read_config_dword(dev, bar_offset + 4, &val2);
> +		addr |= ((resource_size_t)val2) << 32;
> +	}
> +#endif
> +
> +	return __parse_discovery_table(addr, die, parsed, ignore);
> +}
> +
> +static bool intel_uncore_has_discovery_tables_pci(int *ignore)
>  {
>  	u32 device, val, entry_id, bar_offset;
>  	int die, dvsec = 0, ret = true;
> @@ -390,6 +398,45 @@ bool intel_uncore_has_discovery_tables(int *ignore)
>  	return ret;
>  }
>  
> +static bool intel_uncore_has_discovery_tables_msr(int *ignore)
> +{
> +	unsigned long *die_mask;
> +	bool parsed = false;
> +	int cpu, die;
> +	u64 base;
> +
> +	die_mask = kcalloc(BITS_TO_LONGS(__uncore_max_dies),

Better use uncore_max_dies() instead of __uncore_max_dies.


> +			   sizeof(unsigned long), GFP_KERNEL);
> +	if (!die_mask)
> +		return false;
> +
> +	cpus_read_lock();
> +	for_each_online_cpu(cpu) {
> +		die = topology_logical_die_id(cpu);
> +		if (__test_and_set_bit(die, die_mask))
> +			continue;
> +
> +		if (rdmsrq_safe_on_cpu(cpu, UNCORE_DISCOVERY_MSR, &base))
> +			continue;
> +
> +		if (!base)
> +			continue;
> +
> +		__parse_discovery_table(base, die, &parsed, ignore);
> +	}
> +
> +	cpus_read_unlock();
> +
> +	kfree(die_mask);
> +	return parsed;
> +}
> +
> +bool intel_uncore_has_discovery_tables(int *ignore)
> +{
> +	return intel_uncore_has_discovery_tables_msr(ignore) ||
> +	       intel_uncore_has_discovery_tables_pci(ignore);
> +}
> +
>  void intel_uncore_clear_discovery_tables(void)
>  {
>  	struct intel_uncore_discovery_type *type, *next;
> diff --git a/arch/x86/events/intel/uncore_discovery.h b/arch/x86/events/intel/uncore_discovery.h
> index 0e94aa7db8e7..690f737e6837 100644
> --- a/arch/x86/events/intel/uncore_discovery.h
> +++ b/arch/x86/events/intel/uncore_discovery.h
> @@ -1,5 +1,8 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
>  
> +/* Store the full address of the global discovery table */
> +#define UNCORE_DISCOVERY_MSR			0x201e
> +
>  /* Generic device ID of a discovery table device */
>  #define UNCORE_DISCOVERY_TABLE_DEVICE		0x09a7
>  /* Capability ID for a discovery table device */