| Message ID | 20230519160420.2588475-1-steve.wahl@hpe.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: ISST: Remove 8 socket limit | expand |
On Fri, 19 May 2023, Steve Wahl wrote: > Stop restricting the PCI search to a range of PCI domains fed to > pci_get_domain_bus_and_slot(). Instead, use for_each_pci_dev() and > look at all PCI domains in one pass. > > On systems with more than 8 sockets, this avoids error messages like > "Information: Invalid level, Can't get TDP control information at > specified levels on cpu 480" from the intel speed select utility. > > Fixes: aa2ddd242572 ("platform/x86: ISST: Use numa node id for cpu pci dev mapping") > Signed-off-by: Steve Wahl <steve.wahl@hpe.com> > --- > .../x86/intel/speed_select_if/isst_if_common.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c > index e0572a29212e..02fe360a59c7 100644 > --- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c > +++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c > @@ -304,14 +304,13 @@ struct isst_if_pkg_info { > static struct isst_if_cpu_info *isst_cpu_info; > static struct isst_if_pkg_info *isst_pkg_info; > > -#define ISST_MAX_PCI_DOMAINS 8 > - > static struct pci_dev *_isst_if_get_pci_dev(int cpu, int bus_no, int dev, int fn) > { > struct pci_dev *matched_pci_dev = NULL; > struct pci_dev *pci_dev = NULL; > + struct pci_dev *_pci_dev = NULL; > int no_matches = 0, pkg_id; > - int i, bus_number; > + int bus_number; > > if (bus_no < 0 || bus_no >= ISST_MAX_BUS_NUMBER || cpu < 0 || > cpu >= nr_cpu_ids || cpu >= num_possible_cpus()) > @@ -323,12 +322,11 @@ static struct pci_dev *_isst_if_get_pci_dev(int cpu, int bus_no, int dev, int fn > if (bus_number < 0) > return NULL; > > - for (i = 0; i < ISST_MAX_PCI_DOMAINS; ++i) { > - struct pci_dev *_pci_dev; > + for_each_pci_dev(_pci_dev) { > int node; > > - _pci_dev = pci_get_domain_bus_and_slot(i, bus_number, PCI_DEVFN(dev, fn)); > - if (!_pci_dev) > + if (_pci_dev->bus->number != bus_number || > + _pci_dev->devfn != PCI_DEVFN(dev, fn)) > continue; > > ++no_matches; Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> With the note that _pci_dev is not a good variable name (but the rename would make this fix larger than it needs to be).
Hi, On 5/19/23 18:04, Steve Wahl wrote: > Stop restricting the PCI search to a range of PCI domains fed to > pci_get_domain_bus_and_slot(). Instead, use for_each_pci_dev() and > look at all PCI domains in one pass. > > On systems with more than 8 sockets, this avoids error messages like > "Information: Invalid level, Can't get TDP control information at > specified levels on cpu 480" from the intel speed select utility. > > Fixes: aa2ddd242572 ("platform/x86: ISST: Use numa node id for cpu pci dev mapping") > Signed-off-by: Steve Wahl <steve.wahl@hpe.com> Thank you for your patch, I've applied this patch to my fixes branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes Note it will show up in my fixes branch once I've pushed my local branch there, which might take a while. I will include this patch in my next fixes pull-req to Linus for the current kernel development cycle. Regards, Hans > --- > .../x86/intel/speed_select_if/isst_if_common.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c > index e0572a29212e..02fe360a59c7 100644 > --- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c > +++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c > @@ -304,14 +304,13 @@ struct isst_if_pkg_info { > static struct isst_if_cpu_info *isst_cpu_info; > static struct isst_if_pkg_info *isst_pkg_info; > > -#define ISST_MAX_PCI_DOMAINS 8 > - > static struct pci_dev *_isst_if_get_pci_dev(int cpu, int bus_no, int dev, int fn) > { > struct pci_dev *matched_pci_dev = NULL; > struct pci_dev *pci_dev = NULL; > + struct pci_dev *_pci_dev = NULL; > int no_matches = 0, pkg_id; > - int i, bus_number; > + int bus_number; > > if (bus_no < 0 || bus_no >= ISST_MAX_BUS_NUMBER || cpu < 0 || > cpu >= nr_cpu_ids || cpu >= num_possible_cpus()) > @@ -323,12 +322,11 @@ static struct pci_dev *_isst_if_get_pci_dev(int cpu, int bus_no, int dev, int fn > if (bus_number < 0) > return NULL; > > - for (i = 0; i < ISST_MAX_PCI_DOMAINS; ++i) { > - struct pci_dev *_pci_dev; > + for_each_pci_dev(_pci_dev) { > int node; > > - _pci_dev = pci_get_domain_bus_and_slot(i, bus_number, PCI_DEVFN(dev, fn)); > - if (!_pci_dev) > + if (_pci_dev->bus->number != bus_number || > + _pci_dev->devfn != PCI_DEVFN(dev, fn)) > continue; > > ++no_matches;
On Tue, May 23, 2023 at 12:27:08PM +0300, Ilpo Järvinen wrote: > On Fri, 19 May 2023, Steve Wahl wrote: > > > Stop restricting the PCI search to a range of PCI domains fed to > > pci_get_domain_bus_and_slot(). Instead, use for_each_pci_dev() and > > look at all PCI domains in one pass. > > > > On systems with more than 8 sockets, this avoids error messages like > > "Information: Invalid level, Can't get TDP control information at > > specified levels on cpu 480" from the intel speed select utility. > > > > Fixes: aa2ddd242572 ("platform/x86: ISST: Use numa node id for cpu pci dev mapping") > > Signed-off-by: Steve Wahl <steve.wahl@hpe.com> > > --- > > .../x86/intel/speed_select_if/isst_if_common.c | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c > > index e0572a29212e..02fe360a59c7 100644 > > --- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c > > +++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c > > @@ -304,14 +304,13 @@ struct isst_if_pkg_info { > > static struct isst_if_cpu_info *isst_cpu_info; > > static struct isst_if_pkg_info *isst_pkg_info; > > > > -#define ISST_MAX_PCI_DOMAINS 8 > > - > > static struct pci_dev *_isst_if_get_pci_dev(int cpu, int bus_no, int dev, int fn) > > { > > struct pci_dev *matched_pci_dev = NULL; > > struct pci_dev *pci_dev = NULL; > > + struct pci_dev *_pci_dev = NULL; > > int no_matches = 0, pkg_id; > > - int i, bus_number; > > + int bus_number; > > > > if (bus_no < 0 || bus_no >= ISST_MAX_BUS_NUMBER || cpu < 0 || > > cpu >= nr_cpu_ids || cpu >= num_possible_cpus()) > > @@ -323,12 +322,11 @@ static struct pci_dev *_isst_if_get_pci_dev(int cpu, int bus_no, int dev, int fn > > if (bus_number < 0) > > return NULL; > > > > - for (i = 0; i < ISST_MAX_PCI_DOMAINS; ++i) { > > - struct pci_dev *_pci_dev; > > + for_each_pci_dev(_pci_dev) { > > int node; > > > > - _pci_dev = pci_get_domain_bus_and_slot(i, bus_number, PCI_DEVFN(dev, fn)); > > - if (!_pci_dev) > > + if (_pci_dev->bus->number != bus_number || > > + _pci_dev->devfn != PCI_DEVFN(dev, fn)) > > continue; > > > > ++no_matches; > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > With the note that _pci_dev is not a good variable name (but the rename > would make this fix larger than it needs to be). I did look askance at that name for the briefest moment, but came to the same conclusion. Thank you for your time reviewing! --> Steve
diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c index e0572a29212e..02fe360a59c7 100644 --- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c +++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c @@ -304,14 +304,13 @@ struct isst_if_pkg_info { static struct isst_if_cpu_info *isst_cpu_info; static struct isst_if_pkg_info *isst_pkg_info; -#define ISST_MAX_PCI_DOMAINS 8 - static struct pci_dev *_isst_if_get_pci_dev(int cpu, int bus_no, int dev, int fn) { struct pci_dev *matched_pci_dev = NULL; struct pci_dev *pci_dev = NULL; + struct pci_dev *_pci_dev = NULL; int no_matches = 0, pkg_id; - int i, bus_number; + int bus_number; if (bus_no < 0 || bus_no >= ISST_MAX_BUS_NUMBER || cpu < 0 || cpu >= nr_cpu_ids || cpu >= num_possible_cpus()) @@ -323,12 +322,11 @@ static struct pci_dev *_isst_if_get_pci_dev(int cpu, int bus_no, int dev, int fn if (bus_number < 0) return NULL; - for (i = 0; i < ISST_MAX_PCI_DOMAINS; ++i) { - struct pci_dev *_pci_dev; + for_each_pci_dev(_pci_dev) { int node; - _pci_dev = pci_get_domain_bus_and_slot(i, bus_number, PCI_DEVFN(dev, fn)); - if (!_pci_dev) + if (_pci_dev->bus->number != bus_number || + _pci_dev->devfn != PCI_DEVFN(dev, fn)) continue; ++no_matches;
Stop restricting the PCI search to a range of PCI domains fed to pci_get_domain_bus_and_slot(). Instead, use for_each_pci_dev() and look at all PCI domains in one pass. On systems with more than 8 sockets, this avoids error messages like "Information: Invalid level, Can't get TDP control information at specified levels on cpu 480" from the intel speed select utility. Fixes: aa2ddd242572 ("platform/x86: ISST: Use numa node id for cpu pci dev mapping") Signed-off-by: Steve Wahl <steve.wahl@hpe.com> --- .../x86/intel/speed_select_if/isst_if_common.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)