| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b161f36b-2142-4d99-bf39-3792f140fa82@linux.intel.com>
Date: Tue, 23 Dec 2025 11:07:51 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Zide Chen <zide.chen@...el.com>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Andi Kleen <ak@...ux.intel.com>, Eranian Stephane <eranian@...gle.com>
Cc: linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
Xudong Hao <xudong.hao@...el.com>, Falcon Thomas <thomas.falcon@...el.com>
Subject: Re: [PATCH 1/7] perf/x86/intel/uncore: Add dual PCI/MSR discovery
support
On 12/13/2025 5:00 AM, Zide Chen wrote:
> On DMR platforms, IMH discovery tables are enumerated via a PCI device,
> while CBB tables are enumerated via an MSR. This differs from prior
> CPUs, which relied exclusively on either PCI or MSR.
>
> Extend intel_uncore_has_discovery_tables() to support platforms that
> require discovery through both PCI and MSR.
>
> DMR CBB uncore uses MSR 0x710 for discovery instead of MSR 0x201e.
> Introduce a discovery_msr field to store platform-specific MSR values.
>
> Similarly, add a discovery_pci field so that DMR can enumerate through
> PCI device 0x9a1 rather than 0x9a7.
>
> In the !x86_match_cpu() fallback path, has_generic_discovery_table()
> is removed because it does not consider multiple possible PCI devices
> and its performance is not important here. Only probe MSR 0x201e to
> keep the code simple.
>
> Co-developed-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
> Signed-off-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
> Signed-off-by: Zide Chen <zide.chen@...el.com>
> ---
> arch/x86/events/intel/uncore.c | 30 ++++++++++++-----
> arch/x86/events/intel/uncore_discovery.c | 42 +++++++-----------------
> arch/x86/events/intel/uncore_discovery.h | 2 +-
> 3 files changed, 34 insertions(+), 40 deletions(-)
>
> diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
> index a762f7f5b161..ecf500470f8e 100644
> --- a/arch/x86/events/intel/uncore.c
> +++ b/arch/x86/events/intel/uncore.c
> @@ -1703,8 +1703,10 @@ struct intel_uncore_init_fun {
> void (*cpu_init)(void);
> int (*pci_init)(void);
> void (*mmio_init)(void);
> - /* Discovery table is required */
> - bool use_discovery;
> + /* MSR carries the Discovery table base address */
> + u32 discovery_msr;
> + /* PCI device carries the Discovery table base address */
> + u32 discovery_pci;
> /* The units in the discovery table should be ignored. */
> int *uncore_units_ignore;
> };
> @@ -1810,7 +1812,7 @@ static const struct intel_uncore_init_fun lnl_uncore_init __initconst = {
> static const struct intel_uncore_init_fun ptl_uncore_init __initconst = {
> .cpu_init = ptl_uncore_cpu_init,
> .mmio_init = ptl_uncore_mmio_init,
> - .use_discovery = true,
> + .discovery_msr = UNCORE_DISCOVERY_MSR,
> };
>
> static const struct intel_uncore_init_fun icx_uncore_init __initconst = {
> @@ -1829,7 +1831,7 @@ static const struct intel_uncore_init_fun spr_uncore_init __initconst = {
> .cpu_init = spr_uncore_cpu_init,
> .pci_init = spr_uncore_pci_init,
> .mmio_init = spr_uncore_mmio_init,
> - .use_discovery = true,
> + .discovery_pci = UNCORE_DISCOVERY_TABLE_DEVICE,
> .uncore_units_ignore = spr_uncore_units_ignore,
> };
>
> @@ -1837,7 +1839,7 @@ static const struct intel_uncore_init_fun gnr_uncore_init __initconst = {
> .cpu_init = gnr_uncore_cpu_init,
> .pci_init = gnr_uncore_pci_init,
> .mmio_init = gnr_uncore_mmio_init,
> - .use_discovery = true,
> + .discovery_pci = UNCORE_DISCOVERY_TABLE_DEVICE,
> .uncore_units_ignore = gnr_uncore_units_ignore,
> };
>
> @@ -1908,6 +1910,11 @@ static const struct x86_cpu_id intel_uncore_match[] __initconst = {
> };
> MODULE_DEVICE_TABLE(x86cpu, intel_uncore_match);
>
> +static bool ucore_use_discovery(struct intel_uncore_init_fun *uncore_init)
> +{
> + return (uncore_init->discovery_pci || uncore_init->discovery_msr);
> +}
> +
> static int __init intel_uncore_init(void)
> {
> const struct x86_cpu_id *id;
> @@ -1922,16 +1929,21 @@ static int __init intel_uncore_init(void)
>
> id = x86_match_cpu(intel_uncore_match);
> if (!id) {
> - if (!uncore_no_discover && intel_uncore_has_discovery_tables(NULL))
> + if (!uncore_no_discover &&
> + intel_uncore_has_discovery_tables(NULL,
> + UNCORE_DISCOVERY_MSR, PCI_ANY_ID))
> uncore_init = (struct intel_uncore_init_fun *)&generic_uncore_init;
> else
> return -ENODEV;
> } else {
> uncore_init = (struct intel_uncore_init_fun *)id->driver_data;
> - if (uncore_no_discover && uncore_init->use_discovery)
> + if (uncore_no_discover && ucore_use_discovery(uncore_init))
> return -ENODEV;
> - if (uncore_init->use_discovery &&
> - !intel_uncore_has_discovery_tables(uncore_init->uncore_units_ignore))
> + if (ucore_use_discovery(uncore_init) &&
> + !intel_uncore_has_discovery_tables(
> + uncore_init->uncore_units_ignore,
> + uncore_init->discovery_msr,
> + uncore_init->discovery_pci))
> return -ENODEV;
> }
>
> diff --git a/arch/x86/events/intel/uncore_discovery.c b/arch/x86/events/intel/uncore_discovery.c
> index 7d57ce706feb..86373b00e966 100644
> --- a/arch/x86/events/intel/uncore_discovery.c
> +++ b/arch/x86/events/intel/uncore_discovery.c
> @@ -12,24 +12,6 @@
> static struct rb_root discovery_tables = RB_ROOT;
> static int num_discovered_types[UNCORE_ACCESS_MAX];
>
> -static bool has_generic_discovery_table(void)
> -{
> - struct pci_dev *dev;
> - int dvsec;
> -
> - dev = pci_get_device(PCI_VENDOR_ID_INTEL, UNCORE_DISCOVERY_TABLE_DEVICE, NULL);
> - if (!dev)
> - return false;
> -
> - /* A discovery table device has the unique capability ID. */
> - dvsec = pci_find_next_ext_capability(dev, 0, UNCORE_EXT_CAP_ID_DISCOVERY);
> - pci_dev_put(dev);
> - if (dvsec)
> - return true;
> -
> - return false;
> -}
> -
> static int logical_die_id;
>
> static int get_device_die_id(struct pci_dev *dev)
> @@ -350,18 +332,13 @@ static int parse_discovery_table(struct pci_dev *dev, int die,
> return __parse_discovery_table(addr, die, parsed, ignore);
> }
>
> -static bool intel_uncore_has_discovery_tables_pci(int *ignore)
> +static bool intel_uncore_has_discovery_tables_pci(int *ignore, u32 device)
> {
> - u32 device, val, entry_id, bar_offset;
> + u32 val, entry_id, bar_offset;
> int die, dvsec = 0, ret = true;
> struct pci_dev *dev = NULL;
> bool parsed = false;
>
> - if (has_generic_discovery_table())
> - device = UNCORE_DISCOVERY_TABLE_DEVICE;
> - else
> - device = PCI_ANY_ID;
> -
> /*
> * Start a new search and iterates through the list of
> * the discovery table devices.
> @@ -399,7 +376,7 @@ static bool intel_uncore_has_discovery_tables_pci(int *ignore)
> return ret;
> }
>
> -static bool intel_uncore_has_discovery_tables_msr(int *ignore)
> +static bool intel_uncore_has_discovery_tables_msr(int *ignore, u32 msr)
> {
> unsigned long *die_mask;
> bool parsed = false;
> @@ -417,7 +394,7 @@ static bool intel_uncore_has_discovery_tables_msr(int *ignore)
> if (__test_and_set_bit(die, die_mask))
> continue;
>
> - if (rdmsrq_safe_on_cpu(cpu, UNCORE_DISCOVERY_MSR, &base))
> + if (rdmsrq_safe_on_cpu(cpu, msr, &base))
> continue;
>
> if (!base)
> @@ -432,10 +409,15 @@ static bool intel_uncore_has_discovery_tables_msr(int *ignore)
> return parsed;
> }
>
> -bool intel_uncore_has_discovery_tables(int *ignore)
> +bool intel_uncore_has_discovery_tables(int *ignore, u32 msr, u32 device)
> {
> - return intel_uncore_has_discovery_tables_msr(ignore) ||
> - intel_uncore_has_discovery_tables_pci(ignore);
> + bool ret = false;
> +
> + if (msr)
> + ret = intel_uncore_has_discovery_tables_msr(ignore, msr);
> + if (device)
> + ret |= intel_uncore_has_discovery_tables_pci(ignore, device);
> + return ret;
> }
>
> void intel_uncore_clear_discovery_tables(void)
> diff --git a/arch/x86/events/intel/uncore_discovery.h b/arch/x86/events/intel/uncore_discovery.h
> index dff75c98e22f..a919b1ac88fe 100644
> --- a/arch/x86/events/intel/uncore_discovery.h
> +++ b/arch/x86/events/intel/uncore_discovery.h
> @@ -136,7 +136,7 @@ struct intel_uncore_discovery_type {
> u16 num_units; /* number of units */
> };
>
> -bool intel_uncore_has_discovery_tables(int *ignore);
> +bool intel_uncore_has_discovery_tables(int *ignore, u32 msr, u32 device);
> void intel_uncore_clear_discovery_tables(void);
> void intel_uncore_generic_uncore_cpu_init(void);
> int intel_uncore_generic_uncore_pci_init(void);
The changes look good to me.
Reviewed-by: Dapeng Mi <dapeng1.mi@...ux.intel.com>
Powered by blists - more mailing lists