Fix UPI uncore issue on SPR
Subsystems (2)
cover Fix UPI uncore issue on SPR
From: Kan Liang <[email protected]>
The discovery table of UPI on SPR MCC is broken. The patch series is to mitigate the issue by providing a hardcode pre-defined table.
The broken discovery table can trigger a kernel warning message, which is overkilled. The patch series also refine the error handling code.
Kan Liang (5):
perf/x86/uncore: Factor out uncore_device_to_die() perf/x86/uncore: Fix potential NULL pointer in uncore_get_alias_name perf/x86/uncore: Ignore broken units in discovery table perf/x86/uncore: Add a quirk for UPI on SPR perf/x86/uncore: Don't WARN_ON_ONCE() for a broken discovery table
arch/x86/events/intel/uncore.c | 34 ++++- arch/x86/events/intel/uncore.h | 4 + arch/x86/events/intel/uncore_discovery.c | 60 ++++++--- arch/x86/events/intel/uncore_discovery.h | 14 +- arch/x86/events/intel/uncore_snbep.c | 158 ++++++++++++++++++----- 5 files changed, 210 insertions(+), 60 deletions(-)
Signature · 1 line
2.35.1
PATCH RESEND 1/5 perf/x86/uncore: Factor out uncore_device_to_die()
From: Kan Liang <[email protected]>
The same code is used to retrieve the logical die ID with a given PCI device in both the discovery code and the code that supports a system with > 8 nodes.
Factor out uncore_device_to_die() to replace the duplicate code.
No functional change.
--- arch/x86/events/intel/uncore.c | 15 +++++++++++++++ arch/x86/events/intel/uncore.h | 1 + arch/x86/events/intel/uncore_discovery.c | 16 ++-------------- arch/x86/events/intel/uncore_snbep.c | 12 +----------- 4 files changed, 19 insertions(+), 25 deletions(-)
arch/x86/events/intel/uncore.c +15 -0
@@ -65,6 +65,21 @@ int uncore_die_to_segment(int die)return bus ?pci_domain_nr (bus) : -EINVAL ; } +int uncore_device_to_die (struct pci_dev *dev) +{ +int node =pcibus_to_node (dev->bus); +int cpu; + +for_each_cpu (cpu,cpumask_of_pcibus (dev->bus)) { +struct cpuinfo_x86 *c = &cpu_data (cpu); + +if (c->initialized &&cpu_to_node (cpu) == node) +return c->logical_die_id; + } + +return -1 ; +} +static void uncore_free_pcibus_map (void ) {struct pci2phy_map *map, *tmp;
arch/x86/events/intel/uncore.h +1 -0
@@ -208,6 +208,7 @@ struct pci2phy_map {struct pci2phy_map *__find_pci2phy_map (int segment);int uncore_pcibus_to_dieid (struct pci_bus *bus);int uncore_die_to_segment (int die); +int uncore_device_to_die (struct pci_dev *dev);ssize_t uncore_event_show (struct device *dev,struct device_attribute *attr,char *buf);
arch/x86/events/intel/uncore_discovery.c +2 -14
@@ -33,7 +33,7 @@ static int logical_die_id;static int get_device_die_id (struct pci_dev *dev) { -int cpu, node =pcibus_to_node (dev->bus); +int node =pcibus_to_node (dev->bus);/* * If the NUMA info is not available, assume that the logical die id is
@@ -43,19 +43,7 @@ static int get_device_die_id(struct pci_dev *dev)if (node <0 )return logical_die_id++; -for_each_cpu (cpu,cpumask_of_node (node)) { -struct cpuinfo_x86 *c = &cpu_data (cpu); - -if (c->initialized &&cpu_to_node (cpu) == node) -return c->logical_die_id; - } - -/* -* All CPUs of a node may be offlined. For this case, -* the PCI and MMIO type of uncore blocks which are -* enumerated by the device will be unavailable. -*/ -return -1 ; +return uncore_device_to_die (dev); }#define __node_2_type (cur) \
arch/x86/events/intel/uncore_snbep.c +1 -11
@@ -1453,9 +1453,6 @@ static int snbep_pci2phy_map_init(int devid, int nodeid_loc, int idmap_loc, bool }raw_spin_unlock (&pci2phy_map_lock); }else { -int node =pcibus_to_node (ubox_dev->bus); -int cpu; - segment =pci_domain_nr (ubox_dev->bus);raw_spin_lock (&pci2phy_map_lock); map =__find_pci2phy_map (segment);
@@ -1465,15 +1462,8 @@ static int snbep_pci2phy_map_init(int devid, int nodeid_loc, int idmap_loc, boolbreak ; } - die_id =-1 ; -for_each_cpu (cpu,cpumask_of_pcibus (ubox_dev->bus)) { -struct cpuinfo_x86 *c = &cpu_data (cpu); + map->pbus_to_dieid[bus] = die_id =uncore_device_to_die (ubox_dev); -if (c->initialized &&cpu_to_node (cpu) == node) { - map->pbus_to_dieid[bus] = die_id = c->logical_die_id; -break ; - } - }raw_spin_unlock (&pci2phy_map_lock);if (WARN_ON_ONCE (die_id ==-1 )) {
Signature · 1 line
2.35.1
[tip: perf/core] perf/x86/uncore: Factor out uncore_device_to_die()
The following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: dbf061b26221fa1a99e6489dd61f5b4ee97a24e8 Gitweb: https://git.kernel.org/tip/dbf061b26221fa1a99e6489dd61f5b4ee97a24e8 Author: Kan Liang <[email protected]> AuthorDate: Thu, 12 Jan 2023 12:01:01 -08:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Sat, 21 Jan 2023 00:06:11 +01:00
perf/x86/uncore: Factor out uncore_device_to_die()
The same code is used to retrieve the logical die ID with a given PCI device in both the discovery code and the code that supports a system with > 8 nodes.
Factor out uncore_device_to_die() to replace the duplicate code.
No functional change.
--- arch/x86/events/intel/uncore.c | 15 +++++++++++++++ arch/x86/events/intel/uncore.h | 1 + arch/x86/events/intel/uncore_discovery.c | 16 ++-------------- arch/x86/events/intel/uncore_snbep.c | 12 +----------- 4 files changed, 19 insertions(+), 25 deletions(-)
arch/x86/events/intel/uncore.c +15 -0
@@ -65,6 +65,21 @@ int uncore_die_to_segment(int die)return bus ?pci_domain_nr (bus) : -EINVAL ; } +int uncore_device_to_die (struct pci_dev *dev) +{ +int node =pcibus_to_node (dev->bus); +int cpu; + +for_each_cpu (cpu,cpumask_of_pcibus (dev->bus)) { +struct cpuinfo_x86 *c = &cpu_data (cpu); + +if (c->initialized &&cpu_to_node (cpu) == node) +return c->logical_die_id; + } + +return -1 ; +} +static void uncore_free_pcibus_map (void ) {struct pci2phy_map *map, *tmp;
arch/x86/events/intel/uncore.h +1 -0
@@ -208,6 +208,7 @@ struct pci2phy_map {struct pci2phy_map *__find_pci2phy_map (int segment);int uncore_pcibus_to_dieid (struct pci_bus *bus);int uncore_die_to_segment (int die); +int uncore_device_to_die (struct pci_dev *dev);ssize_t uncore_event_show (struct device *dev,struct device_attribute *attr,char *buf);
arch/x86/events/intel/uncore_discovery.c +2 -14
@@ -33,7 +33,7 @@ static int logical_die_id;static int get_device_die_id (struct pci_dev *dev) { -int cpu, node =pcibus_to_node (dev->bus); +int node =pcibus_to_node (dev->bus);/* * If the NUMA info is not available, assume that the logical die id is
@@ -43,19 +43,7 @@ static int get_device_die_id(struct pci_dev *dev)if (node <0 )return logical_die_id++; -for_each_cpu (cpu,cpumask_of_node (node)) { -struct cpuinfo_x86 *c = &cpu_data (cpu); - -if (c->initialized &&cpu_to_node (cpu) == node) -return c->logical_die_id; - } - -/* -* All CPUs of a node may be offlined. For this case, -* the PCI and MMIO type of uncore blocks which are -* enumerated by the device will be unavailable. -*/ -return -1 ; +return uncore_device_to_die (dev); }#define __node_2_type (cur) \
arch/x86/events/intel/uncore_snbep.c +1 -11
@@ -1453,9 +1453,6 @@ static int snbep_pci2phy_map_init(int devid, int nodeid_loc, int idmap_loc, bool }raw_spin_unlock (&pci2phy_map_lock); }else { -int node =pcibus_to_node (ubox_dev->bus); -int cpu; - segment =pci_domain_nr (ubox_dev->bus);raw_spin_lock (&pci2phy_map_lock); map =__find_pci2phy_map (segment);
@@ -1465,15 +1462,8 @@ static int snbep_pci2phy_map_init(int devid, int nodeid_loc, int idmap_loc, boolbreak ; } - die_id =-1 ; -for_each_cpu (cpu,cpumask_of_pcibus (ubox_dev->bus)) { -struct cpuinfo_x86 *c = &cpu_data (cpu); + map->pbus_to_dieid[bus] = die_id =uncore_device_to_die (ubox_dev); -if (c->initialized &&cpu_to_node (cpu) == node) { - map->pbus_to_dieid[bus] = die_id = c->logical_die_id; -break ; - } - }raw_spin_unlock (&pci2phy_map_lock);if (WARN_ON_ONCE (die_id ==-1 )) {
PATCH RESEND 2/5 perf/x86/uncore: Fix potential NULL pointer in uncore_get_alias_name
In reply to [email protected]From: Kan Liang <[email protected]>
The current code assumes that the discovery table provides valid box_ids for the normal units. It's not the case anymore since some units in the discovery table are broken on some SPR variants.
Factor out uncore_get_box_id(). Check the existence of the type->box_ids before using it. If it's not available, use pmu_idx.
--- arch/x86/events/intel/uncore.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
arch/x86/events/intel/uncore.c +8 -2
@@ -857,6 +857,12 @@ static const struct attribute_group uncore_pmu_attr_group = { .attrs = uncore_pmu_attrs, }; +static inline int uncore_get_box_id (struct intel_uncore_type *type, +struct intel_uncore_pmu *pmu) +{ +return type->box_ids ? type->box_ids[pmu->pmu_idx] : pmu->pmu_idx; +} +void uncore_get_alias_name (char *pmu_name,struct intel_uncore_pmu *pmu) {struct intel_uncore_type *type = pmu->type;
@@ -865,7 +871,7 @@ void uncore_get_alias_name(char *pmu_name, struct intel_uncore_pmu *pmu)sprintf (pmu_name,"uncore_type_%u" , type->type_id); else {sprintf (pmu_name,"uncore_type_%u_%d" , - type->type_id, type->box_ids[pmu->pmu_idx]); + type->type_id,uncore_get_box_id (type, pmu)); } }
@@ -892,7 +898,7 @@ static void uncore_get_pmu_name(struct intel_uncore_pmu *pmu)* Use the box ID from the discovery table if applicable. */ sprintf (pmu->name,"uncore_%s_%d" , type->name, - type->box_ids ? type->box_ids[pmu->pmu_idx] : pmu->pmu_idx); +uncore_get_box_id (type, pmu)); } }
Signature · 1 line
2.35.1
[tip: perf/core] perf/x86/uncore: Fix potential NULL pointer in uncore_get_alias_name
The following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 3af548f2361077cd53762c88d62343d4e8ea1efb Gitweb: https://git.kernel.org/tip/3af548f2361077cd53762c88d62343d4e8ea1efb Author: Kan Liang <[email protected]> AuthorDate: Thu, 12 Jan 2023 12:01:02 -08:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Sat, 21 Jan 2023 00:06:12 +01:00
perf/x86/uncore: Fix potential NULL pointer in uncore_get_alias_name
The current code assumes that the discovery table provides valid box_ids for the normal units. It's not the case anymore since some units in the discovery table are broken on some SPR variants.
Factor out uncore_get_box_id(). Check the existence of the type->box_ids before using it. If it's not available, use pmu_idx.
--- arch/x86/events/intel/uncore.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
arch/x86/events/intel/uncore.c +8 -2
@@ -857,6 +857,12 @@ static const struct attribute_group uncore_pmu_attr_group = { .attrs = uncore_pmu_attrs, }; +static inline int uncore_get_box_id (struct intel_uncore_type *type, +struct intel_uncore_pmu *pmu) +{ +return type->box_ids ? type->box_ids[pmu->pmu_idx] : pmu->pmu_idx; +} +void uncore_get_alias_name (char *pmu_name,struct intel_uncore_pmu *pmu) {struct intel_uncore_type *type = pmu->type;
@@ -865,7 +871,7 @@ void uncore_get_alias_name(char *pmu_name, struct intel_uncore_pmu *pmu)sprintf (pmu_name,"uncore_type_%u" , type->type_id); else {sprintf (pmu_name,"uncore_type_%u_%d" , - type->type_id, type->box_ids[pmu->pmu_idx]); + type->type_id,uncore_get_box_id (type, pmu)); } }
@@ -892,7 +898,7 @@ static void uncore_get_pmu_name(struct intel_uncore_pmu *pmu)* Use the box ID from the discovery table if applicable. */ sprintf (pmu->name,"uncore_%s_%d" , type->name, - type->box_ids ? type->box_ids[pmu->pmu_idx] : pmu->pmu_idx); +uncore_get_box_id (type, pmu)); } }
PATCH RESEND 3/5 perf/x86/uncore: Ignore broken units in discovery table
In reply to [email protected]From: Kan Liang <[email protected]>
Some units in a discovery table may be broken, e.g., UPI of SPR MCC. A generic method is required to ignore the broken units.
Add uncore_units_ignore in the struct intel_uncore_init_fun, which indicates the type ID of broken units. It will be assigned by the platform-specific code later when the platform has a broken discovery table.
--- arch/x86/events/intel/uncore.c | 8 ++++++-- arch/x86/events/intel/uncore.h | 2 ++ arch/x86/events/intel/uncore_discovery.c | 26 +++++++++++++++++++++--- arch/x86/events/intel/uncore_discovery.h | 2 +- 4 files changed, 32 insertions(+), 6 deletions(-)
arch/x86/events/intel/uncore.c +6 -2
@@ -1695,7 +1695,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; +/* The units in the discovery table should be ignored. */ +int *uncore_units_ignore; };static const struct intel_uncore_init_fun nhm_uncore_init__initconst = {
@@ -1873,7 +1876,7 @@ 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 ()) +if (!uncore_no_discover &&intel_uncore_has_discovery_tables (NULL )) uncore_init = (struct intel_uncore_init_fun *)&generic_uncore_init;else return -ENODEV ;
@@ -1881,7 +1884,8 @@ static int __init intel_uncore_init(void) uncore_init = (struct intel_uncore_init_fun *)id->driver_data;if (uncore_no_discover && uncore_init->use_discovery)return -ENODEV ; -if (uncore_init->use_discovery && !intel_uncore_has_discovery_tables ()) +if (uncore_init->use_discovery && + !intel_uncore_has_discovery_tables (uncore_init->uncore_units_ignore))return -ENODEV ; }
arch/x86/events/intel/uncore.h +2 -0
@@ -34,6 +34,8 @@#define UNCORE_EVENT_CONSTRAINT (c, n) EVENT_CONSTRAINT(c, n, 0xff) +#define UNCORE_IGNORE_END -1 +struct pci_extra_dev {struct pci_dev *dev[UNCORE_EXTRA_PCI_DEV_MAX ]; };
arch/x86/events/intel/uncore_discovery.c +23 -3
@@ -190,8 +190,25 @@ uncore_insert_box_info(struct uncore_unit_discovery *unit, } +static bool +uncore_ignore_unit (struct uncore_unit_discovery *unit,int *ignore) +{ +int i; + +if (!ignore) +return false; + +for (i =0 ; ignore[i] !=UNCORE_IGNORE_END ; i++) { +if (unit->box_type == ignore[i]) +return true; + } + +return false; +} +static int parse_discovery_table (struct pci_dev *dev,int die, -u32 bar_offset,bool *parsed) +u32 bar_offset,bool *parsed, +int *ignore) {struct uncore_global_discovery global;struct uncore_unit_discovery unit;
@@ -246,6 +263,9 @@ static int parse_discovery_table(struct pci_dev *dev, int die,if (unit.access_type >=UNCORE_ACCESS_MAX )continue ; +if (uncore_ignore_unit (&unit, ignore)) +continue ; +uncore_insert_box_info (&unit, die, *parsed); }
@@ -254,7 +274,7 @@ static int parse_discovery_table(struct pci_dev *dev, int die,return 0 ; } -bool intel_uncore_has_discovery_tables (void ) +bool intel_uncore_has_discovery_tables (int *ignore) {u32 device, val, entry_id, bar_offset;int die, dvsec =0 , ret = true;
@@ -290,7 +310,7 @@ bool intel_uncore_has_discovery_tables(void)if (die <0 )continue ; -parse_discovery_table (dev, die, bar_offset, &parsed); +parse_discovery_table (dev, die, bar_offset, &parsed, ignore); } }
arch/x86/events/intel/uncore_discovery.h +1 -1
@@ -122,7 +122,7 @@ struct intel_uncore_discovery_type {unsigned int *box_offset;/* Box offset */ }; -bool intel_uncore_has_discovery_tables (void ); +bool intel_uncore_has_discovery_tables (int *ignore);void intel_uncore_clear_discovery_tables (void );void intel_uncore_generic_uncore_cpu_init (void );int intel_uncore_generic_uncore_pci_init (void );
Signature · 1 line
2.35.1
[tip: perf/core] perf/x86/uncore: Ignore broken units in discovery table
The following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: bd9514a4d5ec25b29728720ca8b3a9ac4e3137d7 Gitweb: https://git.kernel.org/tip/bd9514a4d5ec25b29728720ca8b3a9ac4e3137d7 Author: Kan Liang <[email protected]> AuthorDate: Thu, 12 Jan 2023 12:01:03 -08:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Sat, 21 Jan 2023 00:06:12 +01:00
perf/x86/uncore: Ignore broken units in discovery table
Some units in a discovery table may be broken, e.g., UPI of SPR MCC. A generic method is required to ignore the broken units.
Add uncore_units_ignore in the struct intel_uncore_init_fun, which indicates the type ID of broken units. It will be assigned by the platform-specific code later when the platform has a broken discovery table.
--- arch/x86/events/intel/uncore.c | 8 +++++-- arch/x86/events/intel/uncore.h | 2 ++- arch/x86/events/intel/uncore_discovery.c | 26 ++++++++++++++++++++--- arch/x86/events/intel/uncore_discovery.h | 2 +- 4 files changed, 32 insertions(+), 6 deletions(-)
arch/x86/events/intel/uncore.c +6 -2
@@ -1695,7 +1695,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; +/* The units in the discovery table should be ignored. */ +int *uncore_units_ignore; };static const struct intel_uncore_init_fun nhm_uncore_init__initconst = {
@@ -1874,7 +1877,7 @@ 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 ()) +if (!uncore_no_discover &&intel_uncore_has_discovery_tables (NULL )) uncore_init = (struct intel_uncore_init_fun *)&generic_uncore_init;else return -ENODEV ;
@@ -1882,7 +1885,8 @@ static int __init intel_uncore_init(void) uncore_init = (struct intel_uncore_init_fun *)id->driver_data;if (uncore_no_discover && uncore_init->use_discovery)return -ENODEV ; -if (uncore_init->use_discovery && !intel_uncore_has_discovery_tables ()) +if (uncore_init->use_discovery && + !intel_uncore_has_discovery_tables (uncore_init->uncore_units_ignore))return -ENODEV ; }
arch/x86/events/intel/uncore.h +2 -0
@@ -34,6 +34,8 @@#define UNCORE_EVENT_CONSTRAINT (c, n) EVENT_CONSTRAINT(c, n, 0xff) +#define UNCORE_IGNORE_END -1 +struct pci_extra_dev {struct pci_dev *dev[UNCORE_EXTRA_PCI_DEV_MAX ]; };
arch/x86/events/intel/uncore_discovery.c +23 -3
@@ -190,8 +190,25 @@ free_box_offset: } +static bool +uncore_ignore_unit (struct uncore_unit_discovery *unit,int *ignore) +{ +int i; + +if (!ignore) +return false; + +for (i =0 ; ignore[i] !=UNCORE_IGNORE_END ; i++) { +if (unit->box_type == ignore[i]) +return true; + } + +return false; +} +static int parse_discovery_table (struct pci_dev *dev,int die, -u32 bar_offset,bool *parsed) +u32 bar_offset,bool *parsed, +int *ignore) {struct uncore_global_discovery global;struct uncore_unit_discovery unit;
@@ -246,6 +263,9 @@ static int parse_discovery_table(struct pci_dev *dev, int die,if (unit.access_type >=UNCORE_ACCESS_MAX )continue ; +if (uncore_ignore_unit (&unit, ignore)) +continue ; +uncore_insert_box_info (&unit, die, *parsed); }
@@ -254,7 +274,7 @@ static int parse_discovery_table(struct pci_dev *dev, int die,return 0 ; } -bool intel_uncore_has_discovery_tables (void ) +bool intel_uncore_has_discovery_tables (int *ignore) {u32 device, val, entry_id, bar_offset;int die, dvsec =0 , ret = true;
@@ -290,7 +310,7 @@ bool intel_uncore_has_discovery_tables(void)if (die <0 )continue ; -parse_discovery_table (dev, die, bar_offset, &parsed); +parse_discovery_table (dev, die, bar_offset, &parsed, ignore); } }
arch/x86/events/intel/uncore_discovery.h +1 -1
@@ -122,7 +122,7 @@ struct intel_uncore_discovery_type {unsigned int *box_offset;/* Box offset */ }; -bool intel_uncore_has_discovery_tables (void ); +bool intel_uncore_has_discovery_tables (int *ignore);void intel_uncore_clear_discovery_tables (void );void intel_uncore_generic_uncore_cpu_init (void );int intel_uncore_generic_uncore_pci_init (void );
PATCH RESEND 4/5 perf/x86/uncore: Add a quirk for UPI on SPR
In reply to [email protected]From: Kan Liang <[email protected]>
The discovery table of UPI on some SPR variants, e.g., MCC, is broken. The third UPI table may includes a wrong address which points to a non-exists device. The bug impacts both UPI and M3UPI uncore PMON.
Use a pre-defined UPI and M3UPI table to replace the broken table.
Different BIOS may populate a device into a different domain or a different BUS. The accurate location can only be retrieved at load time. Add spr_update_device_location() to update the location of the UPI and M3UPI in the pre-defined table.
--- arch/x86/events/intel/uncore.c | 1 + arch/x86/events/intel/uncore.h | 1 + arch/x86/events/intel/uncore_discovery.h | 12 +- arch/x86/events/intel/uncore_snbep.c | 146 +++++++++++++++++++---- 4 files changed, 136 insertions(+), 24 deletions(-)
arch/x86/events/intel/uncore.c +1 -0
@@ -1806,6 +1806,7 @@ static const struct intel_uncore_init_fun spr_uncore_init __initconst = { .pci_init = spr_uncore_pci_init, .mmio_init = spr_uncore_mmio_init, .use_discovery = true, + .uncore_units_ignore = spr_uncore_units_ignore, };static const struct intel_uncore_init_fun generic_uncore_init__initconst = {
arch/x86/events/intel/uncore.h +1 -0
@@ -592,6 +592,7 @@ extern raw_spinlock_t pci2phy_map_lock;extern struct list_head pci2phy_map_head;extern struct pci_extra_dev *uncore_extra_pci_dev;extern struct event_constraint uncore_constraint_empty; +extern int spr_uncore_units_ignore[];/* uncore_snb.c */ int snb_uncore_pci_init (void );
arch/x86/events/intel/uncore_discovery.h +9 -3
@@ -21,9 +21,15 @@/* Global discovery table size */ #define UNCORE_DISCOVERY_GLOBAL_MAP_SIZE 0x20 -#define UNCORE_DISCOVERY_PCI_DOMAIN (data) ((data >> 28) & 0x7) -#define UNCORE_DISCOVERY_PCI_BUS (data) ((data >> 20) & 0xff) -#define UNCORE_DISCOVERY_PCI_DEVFN (data) ((data >> 12) & 0xff) +#define UNCORE_DISCOVERY_PCI_DOMAIN_OFFSET 28 +#define UNCORE_DISCOVERY_PCI_DOMAIN (data) \ + ((data >> UNCORE_DISCOVERY_PCI_DOMAIN_OFFSET) & 0x7) +#define UNCORE_DISCOVERY_PCI_BUS_OFFSET 20 +#define UNCORE_DISCOVERY_PCI_BUS (data) \ + ((data >> UNCORE_DISCOVERY_PCI_BUS_OFFSET) & 0xff) +#define UNCORE_DISCOVERY_PCI_DEVFN_OFFSET 12 +#define UNCORE_DISCOVERY_PCI_DEVFN (data) \ + ((data >> UNCORE_DISCOVERY_PCI_DEVFN_OFFSET) & 0xff)#define UNCORE_DISCOVERY_PCI_BOX_CTRL (data) (data & 0xfff)
arch/x86/events/intel/uncore_snbep.c +125 -21
189 lines not on this page · raw message
Signature · 1 line
2.35.1
[tip: perf/core] perf/x86/uncore: Add a quirk for UPI on SPR
The following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 65248a9a9ee1ea2e6aad8f35253ffd9096ab81b7 Gitweb: https://git.kernel.org/tip/65248a9a9ee1ea2e6aad8f35253ffd9096ab81b7 Author: Kan Liang <[email protected]> AuthorDate: Thu, 12 Jan 2023 12:01:04 -08:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Sat, 21 Jan 2023 00:06:13 +01:00
perf/x86/uncore: Add a quirk for UPI on SPR
The discovery table of UPI on some SPR variants, e.g., MCC, is broken. The third UPI table may includes a wrong address which points to a non-exists device. The bug impacts both UPI and M3UPI uncore PMON.
Use a pre-defined UPI and M3UPI table to replace the broken table.
Different BIOS may populate a device into a different domain or a different BUS. The accurate location can only be retrieved at load time. Add spr_update_device_location() to update the location of the UPI and M3UPI in the pre-defined table.
--- arch/x86/events/intel/uncore.c | 1 +- arch/x86/events/intel/uncore.h | 1 +- arch/x86/events/intel/uncore_discovery.h | 12 +- arch/x86/events/intel/uncore_snbep.c | 146 ++++++++++++++++++---- 4 files changed, 136 insertions(+), 24 deletions(-)
arch/x86/events/intel/uncore.c +1 -0
@@ -1806,6 +1806,7 @@ static const struct intel_uncore_init_fun spr_uncore_init __initconst = { .pci_init = spr_uncore_pci_init, .mmio_init = spr_uncore_mmio_init, .use_discovery = true, + .uncore_units_ignore = spr_uncore_units_ignore, };static const struct intel_uncore_init_fun generic_uncore_init__initconst = {
arch/x86/events/intel/uncore.h +1 -0
@@ -592,6 +592,7 @@ extern raw_spinlock_t pci2phy_map_lock;extern struct list_head pci2phy_map_head;extern struct pci_extra_dev *uncore_extra_pci_dev;extern struct event_constraint uncore_constraint_empty; +extern int spr_uncore_units_ignore[];/* uncore_snb.c */ int snb_uncore_pci_init (void );
arch/x86/events/intel/uncore_discovery.h +9 -3
@@ -21,9 +21,15 @@/* Global discovery table size */ #define UNCORE_DISCOVERY_GLOBAL_MAP_SIZE 0x20 -#define UNCORE_DISCOVERY_PCI_DOMAIN (data) ((data >> 28) & 0x7) -#define UNCORE_DISCOVERY_PCI_BUS (data) ((data >> 20) & 0xff) -#define UNCORE_DISCOVERY_PCI_DEVFN (data) ((data >> 12) & 0xff) +#define UNCORE_DISCOVERY_PCI_DOMAIN_OFFSET 28 +#define UNCORE_DISCOVERY_PCI_DOMAIN (data) \ + ((data >> UNCORE_DISCOVERY_PCI_DOMAIN_OFFSET) & 0x7) +#define UNCORE_DISCOVERY_PCI_BUS_OFFSET 20 +#define UNCORE_DISCOVERY_PCI_BUS (data) \ + ((data >> UNCORE_DISCOVERY_PCI_BUS_OFFSET) & 0xff) +#define UNCORE_DISCOVERY_PCI_DEVFN_OFFSET 12 +#define UNCORE_DISCOVERY_PCI_DEVFN (data) \ + ((data >> UNCORE_DISCOVERY_PCI_DEVFN_OFFSET) & 0xff)#define UNCORE_DISCOVERY_PCI_BOX_CTRL (data) (data & 0xfff)
arch/x86/events/intel/uncore_snbep.c +125 -21
189 lines not on this page · raw message
PATCH RESEND 5/5 perf/x86/uncore: Don't WARN_ON_ONCE() for a broken discovery table
In reply to [email protected]From: Kan Liang <[email protected]>
The kernel warning message is triggered, when SPR MCC is used.
[ 17.945331] ------------[ cut here ]------------ [ 17.946305] WARNING: CPU: 65 PID: 1 at arch/x86/events/intel/uncore_discovery.c:184 intel_uncore_has_discovery_tables+0x4c0/0x65c [ 17.946305] Modules linked in: [ 17.946305] CPU: 65 PID: 1 Comm: swapper/0 Not tainted
5.4.17-2136.313.1-X10-2c+ #4
It's caused by the broken discovery table of UPI.
The discovery tables are from hardware. Except for dropping the broken information, there is nothing Linux can do. Using WARN_ON_ONCE() is overkilled.
Use the pr_info() to replace WARN_ON_ONCE(), and specify what uncore unit is dropped and the reason.
--- arch/x86/events/intel/uncore_discovery.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
arch/x86/events/intel/uncore_discovery.c +15 -3
@@ -128,13 +128,21 @@ uncore_insert_box_info(struct uncore_unit_discovery *unit,unsigned int *box_offset, *ids;int i; -if (WARN_ON_ONCE (!unit->ctl || !unit->ctl_offset || !unit->ctr_offset)) +if (!unit->ctl || !unit->ctl_offset || !unit->ctr_offset) { +pr_info ("Invalid address is detected for uncore type %d box %d, " +"Disable the uncore unit.\n" , + unit->box_type, unit->box_id);return ; + }if (parsed) { type =search_uncore_discovery_type (unit->box_type); -if (WARN_ON_ONCE (!type)) +if (!type) { +pr_info ("A spurious uncore type %d is detected, " +"Disable the uncore type.\n" , + unit->box_type);return ; + }/* Store the first box of each die */ if (!type->box_ctrl_die[die]) type->box_ctrl_die[die] = unit->ctl;
@@ -169,8 +177,12 @@ uncore_insert_box_info(struct uncore_unit_discovery *unit, ids[i] = type->ids[i]; box_offset[i] = type->box_offset[i]; -if (WARN_ON_ONCE (unit->box_id == ids[i])) +if (unit->box_id == ids[i]) { +pr_info ("Duplicate uncore type %d box ID %d is detected, " +"Drop the duplicate uncore unit.\n" , + unit->box_type, unit->box_id); goto free_ids; + } } ids[i] = unit->box_id; box_offset[i] = unit->ctl - type->box_ctrl;
Signature · 1 line
2.35.1
[tip: perf/core] perf/x86/uncore: Don't WARN_ON_ONCE() for a broken discovery table
The following commit has been merged into the perf/core branch of tip:
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 5d515ee40cb57ea5331998f27df7946a69f14dc3 Gitweb: https://git.kernel.org/tip/5d515ee40cb57ea5331998f27df7946a69f14dc3 Author: Kan Liang <[email protected]> AuthorDate: Thu, 12 Jan 2023 12:01:05 -08:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Sat, 21 Jan 2023 00:06:13 +01:00
perf/x86/uncore: Don't WARN_ON_ONCE() for a broken discovery table
The kernel warning message is triggered, when SPR MCC is used.
[ 17.945331] ------------[ cut here ]------------ [ 17.946305] WARNING: CPU: 65 PID: 1 at arch/x86/events/intel/uncore_discovery.c:184 intel_uncore_has_discovery_tables+0x4c0/0x65c [ 17.946305] Modules linked in: [ 17.946305] CPU: 65 PID: 1 Comm: swapper/0 Not tainted
5.4.17-2136.313.1-X10-2c+ #4
It's caused by the broken discovery table of UPI.
The discovery tables are from hardware. Except for dropping the broken information, there is nothing Linux can do. Using WARN_ON_ONCE() is overkilled.
Use the pr_info() to replace WARN_ON_ONCE(), and specify what uncore unit is dropped and the reason.
--- arch/x86/events/intel/uncore_discovery.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
arch/x86/events/intel/uncore_discovery.c +15 -3
@@ -128,13 +128,21 @@ uncore_insert_box_info(struct uncore_unit_discovery *unit,unsigned int *box_offset, *ids;int i; -if (WARN_ON_ONCE (!unit->ctl || !unit->ctl_offset || !unit->ctr_offset)) +if (!unit->ctl || !unit->ctl_offset || !unit->ctr_offset) { +pr_info ("Invalid address is detected for uncore type %d box %d, " +"Disable the uncore unit.\n" , + unit->box_type, unit->box_id);return ; + }if (parsed) { type =search_uncore_discovery_type (unit->box_type); -if (WARN_ON_ONCE (!type)) +if (!type) { +pr_info ("A spurious uncore type %d is detected, " +"Disable the uncore type.\n" , + unit->box_type);return ; + }/* Store the first box of each die */ if (!type->box_ctrl_die[die]) type->box_ctrl_die[die] = unit->ctl;
@@ -169,8 +177,12 @@ uncore_insert_box_info(struct uncore_unit_discovery *unit, ids[i] = type->ids[i]; box_offset[i] = type->box_offset[i]; -if (WARN_ON_ONCE (unit->box_id == ids[i])) +if (unit->box_id == ids[i]) { +pr_info ("Duplicate uncore type %d box ID %d is detected, " +"Drop the duplicate uncore unit.\n" , + unit->box_type, unit->box_id); goto free_ids; + } } ids[i] = unit->box_id; box_offset[i] = unit->ctl - type->box_ctrl;
Hi Peter & Ingo,
Gentle Ping. Please let me know if you have any comments on the patch set.
Thanks
Kan
Quoted 22 lines · On 2023-01-12 3:01 p.m., [email protected] wrote:
From: Kan Liang <[email protected]>
The discovery table of UPI on SPR MCC is broken. The patch series is to mitigate the issue by providing a hardcode pre-defined table.
The broken discovery table can trigger a kernel warning message, which is overkilled. The patch series also refine the error handling code.
Kan Liang (5):
perf/x86/uncore: Factor outuncore_device_to_die()perf/x86/uncore: Fix potential NULL pointer inuncore_get_alias_nameperf/x86/uncore: Ignore broken units in discovery table perf/x86/uncore: Add a quirk for UPI on SPR perf/x86/uncore: Don'tWARN_ON_ONCE()for a broken discovery tablearch/x86/events/intel/uncore.c | 34 ++++- arch/x86/events/intel/uncore.h | 4 + arch/x86/events/intel/uncore_discovery.c | 60 ++++++--- arch/x86/events/intel/uncore_discovery.h | 14 +- arch/x86/events/intel/uncore_snbep.c | 158 ++++++++++++++++++----- 5 files changed, 210 insertions(+), 60 deletions(-)
Michael Petlan
Tested-byQuoted 22 lines · Kan
Hi Peter & Ingo,
Gentle Ping. Please let me know if you have any comments on the patch set.
Thanks
KanQuoted 14 lines · On 2023-01-12 3:01 p.m., [email protected] wrote:
From: Kan Liang <[email protected]>
The discovery table of UPI on SPR MCC is broken. The patch series is to mitigate the issue by providing a hardcode pre-defined table.
The broken discovery table can trigger a kernel warning message, which is overkilled. The patch series also refine the error handling code.
Kan Liang (5):
perf/x86/uncore: Factor outuncore_device_to_die()perf/x86/uncore: Fix potential NULL pointer inuncore_get_alias_nameperf/x86/uncore: Ignore broken units in discovery table perf/x86/uncore: Add a quirk for UPI on SPR perf/x86/uncore: Don'tWARN_ON_ONCE()for a broken discovery table
For the series,
Quoted 10 lines
Quoted 8 lines
arch/x86/events/intel/uncore.c | 34 ++++- arch/x86/events/intel/uncore.h | 4 + arch/x86/events/intel/uncore_discovery.c | 60 ++++++--- arch/x86/events/intel/uncore_discovery.h | 14 +- arch/x86/events/intel/uncore_snbep.c | 158 ++++++++++++++++++----- 5 files changed, 210 insertions(+), 60 deletions(-)
Hello all,
gentle ping #2... How does it look with the patchset acceptance? Is everything OK? Does it need any additional testing/etc.? When could the patches be expected to land in Linus' tree? Is it within v6.2 scope?
Thank you.
Michael
Quoted 32 lines · Kan
Hi Peter & Ingo,
Gentle Ping. Please let me know if you have any comments on the patch set.
Thanks
KanQuoted 22 lines · On 2023-01-12 3:01 p.m., [email protected] wrote:
From: Kan Liang <[email protected]>
The discovery table of UPI on SPR MCC is broken. The patch series is to mitigate the issue by providing a hardcode pre-defined table.
The broken discovery table can trigger a kernel warning message, which is overkilled. The patch series also refine the error handling code.
Kan Liang (5):
perf/x86/uncore: Factor outuncore_device_to_die()perf/x86/uncore: Fix potential NULL pointer inuncore_get_alias_nameperf/x86/uncore: Ignore broken units in discovery table perf/x86/uncore: Add a quirk for UPI on SPR perf/x86/uncore: Don'tWARN_ON_ONCE()for a broken discovery tablearch/x86/events/intel/uncore.c | 34 ++++- arch/x86/events/intel/uncore.h | 4 + arch/x86/events/intel/uncore_discovery.c | 60 ++++++--- arch/x86/events/intel/uncore_discovery.h | 14 +- arch/x86/events/intel/uncore_snbep.c | 158 ++++++++++++++++++----- 5 files changed, 210 insertions(+), 60 deletions(-)
I reviewed this patch series, applied it to a kernel tree, and tested it on two larger (12+ socket) systems, did not notice any adverse affects. So I believe it's appropriate to add both of these tags:
--> Steve
Quoted 25 lines · On Thu, Jan 12, 2023 at 12:01:00PM -0800, [email protected] wrote:
From: Kan Liang <[email protected]>
The discovery table of UPI on SPR MCC is broken. The patch series is to mitigate the issue by providing a hardcode pre-defined table.
The broken discovery table can trigger a kernel warning message, which is overkilled. The patch series also refine the error handling code.
Kan Liang (5):
perf/x86/uncore: Factor outuncore_device_to_die()perf/x86/uncore: Fix potential NULL pointer inuncore_get_alias_nameperf/x86/uncore: Ignore broken units in discovery table perf/x86/uncore: Add a quirk for UPI on SPR perf/x86/uncore: Don'tWARN_ON_ONCE()for a broken discovery tablearch/x86/events/intel/uncore.c | 34 ++++- arch/x86/events/intel/uncore.h | 4 + arch/x86/events/intel/uncore_discovery.c | 60 ++++++--- arch/x86/events/intel/uncore_discovery.h | 14 +- arch/x86/events/intel/uncore_snbep.c | 158 ++++++++++++++++++----- 5 files changed, 210 insertions(+), 60 deletions(-)Signature · 1 line
2.35.1
Signature · 1 line
Steve Wahl, Hewlett Packard Enterprise