linux-edac.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v2] EDAC, sb_edac: Add support for systems with segmented PCI buses
@ 2018-07-26  0:27 Masayoshi Mizuma
  0 siblings, 0 replies; 3+ messages in thread
From: Masayoshi Mizuma @ 2018-07-26  0:27 UTC (permalink / raw)
  To: bp; +Cc: mchehab, linux-edac, linux-kernel, tony.luck, m.mizuma

Hi Boris,

On 07/25/2018 05:22 AM, Borislav Petkov wrote:
> On Tue, Jul 24, 2018 at 03:02:13PM -0400, Masayoshi Mizuma wrote:
>> [*] KASAN report is as follows.
> 
> That KASAN report is an arbitrary side-effect from the missing segmented
> support so I ripped it out from the commit message and ended up
> committing this:

Thank you so much!

- Masa

> 
> ---
> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> Date: Tue, 24 Jul 2018 15:02:13 -0400
> Subject: [PATCH] EDAC, sb_edac: Add support for systems with segmented PCI buses
> 
> Extend the driver to check whether segment number and bus number matches
> when deciding how to group memory controller PCI devices to CPU sockets.
> 
> Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-edac <linux-edac@vger.kernel.org>
> Link: http://lkml.kernel.org/r/20180724190213.26359-1-msys.mizuma@gmail.com
> [ Cleanup commit message. ]
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  drivers/edac/sb_edac.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
> index 4a89c8093307..07726fb00321 100644
> --- a/drivers/edac/sb_edac.c
> +++ b/drivers/edac/sb_edac.c
> @@ -352,6 +352,7 @@ struct pci_id_table {
>  
>  struct sbridge_dev {
>  	struct list_head	list;
> +	int			seg;
>  	u8			bus, mc;
>  	u8			node_id, source_id;
>  	struct pci_dev		**pdev;
> @@ -729,7 +730,8 @@ static inline int numcol(u32 mtr)
>  	return 1 << cols;
>  }
>  
> -static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bus,
> +static struct sbridge_dev *get_sbridge_dev(int seg, u8 bus, enum domain dom,
> +					   int multi_bus,
>  					   struct sbridge_dev *prev)
>  {
>  	struct sbridge_dev *sbridge_dev;
> @@ -747,14 +749,15 @@ static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bu
>  				      : sbridge_edac_list.next, struct sbridge_dev, list);
>  
>  	list_for_each_entry_from(sbridge_dev, &sbridge_edac_list, list) {
> -		if (sbridge_dev->bus == bus && (dom == SOCK || dom == sbridge_dev->dom))
> +		if ((sbridge_dev->seg == seg) && (sbridge_dev->bus == bus) &&
> +				(dom == SOCK || dom == sbridge_dev->dom))
>  			return sbridge_dev;
>  	}
>  
>  	return NULL;
>  }
>  
> -static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom,
> +static struct sbridge_dev *alloc_sbridge_dev(int seg, u8 bus, enum domain dom,
>  					     const struct pci_id_table *table)
>  {
>  	struct sbridge_dev *sbridge_dev;
> @@ -771,6 +774,7 @@ static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom,
>  		return NULL;
>  	}
>  
> +	sbridge_dev->seg = seg;
>  	sbridge_dev->bus = bus;
>  	sbridge_dev->dom = dom;
>  	sbridge_dev->n_devs = table->n_devs_per_imc;
> @@ -2246,6 +2250,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
>  	struct sbridge_dev *sbridge_dev = NULL;
>  	const struct pci_id_descr *dev_descr = &table->descr[devno];
>  	struct pci_dev *pdev = NULL;
> +	int seg = 0;
>  	u8 bus = 0;
>  	int i = 0;
>  
> @@ -2276,10 +2281,12 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
>  		/* End of list, leave */
>  		return -ENODEV;
>  	}
> +	seg = pci_domain_nr(pdev->bus);
>  	bus = pdev->bus->number;
>  
>  next_imc:
> -	sbridge_dev = get_sbridge_dev(bus, dev_descr->dom, multi_bus, sbridge_dev);
> +	sbridge_dev = get_sbridge_dev(seg, bus, dev_descr->dom,
> +				      multi_bus, sbridge_dev);
>  	if (!sbridge_dev) {
>  		/* If the HA1 wasn't found, don't create EDAC second memory controller */
>  		if (dev_descr->dom == IMC1 && devno != 1) {
> @@ -2292,7 +2299,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
>  		if (dev_descr->dom == SOCK)
>  			goto out_imc;
>  
> -		sbridge_dev = alloc_sbridge_dev(bus, dev_descr->dom, table);
> +		sbridge_dev = alloc_sbridge_dev(seg, bus, dev_descr->dom, table);
>  		if (!sbridge_dev) {
>  			pci_dev_put(pdev);
>  			return -ENOMEM;
>
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [v2] EDAC, sb_edac: Add support for systems with segmented PCI buses
@ 2018-07-25  9:22 Borislav Petkov
  0 siblings, 0 replies; 3+ messages in thread
From: Borislav Petkov @ 2018-07-25  9:22 UTC (permalink / raw)
  To: Masayoshi Mizuma
  Cc: Mauro Carvalho Chehab, linux-edac, linux-kernel, Tony Luck,
	Masayoshi Mizuma

On Tue, Jul 24, 2018 at 03:02:13PM -0400, Masayoshi Mizuma wrote:
> [*] KASAN report is as follows.

That KASAN report is an arbitrary side-effect from the missing segmented
support so I ripped it out from the commit message and ended up
committing this:
---
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Date: Tue, 24 Jul 2018 15:02:13 -0400
Subject: [PATCH] EDAC, sb_edac: Add support for systems with segmented PCI buses

Extend the driver to check whether segment number and bus number matches
when deciding how to group memory controller PCI devices to CPU sockets.

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-edac <linux-edac@vger.kernel.org>
Link: http://lkml.kernel.org/r/20180724190213.26359-1-msys.mizuma@gmail.com
[ Cleanup commit message. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/edac/sb_edac.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 4a89c8093307..07726fb00321 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -352,6 +352,7 @@ struct pci_id_table {
 
 struct sbridge_dev {
 	struct list_head	list;
+	int			seg;
 	u8			bus, mc;
 	u8			node_id, source_id;
 	struct pci_dev		**pdev;
@@ -729,7 +730,8 @@ static inline int numcol(u32 mtr)
 	return 1 << cols;
 }
 
-static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bus,
+static struct sbridge_dev *get_sbridge_dev(int seg, u8 bus, enum domain dom,
+					   int multi_bus,
 					   struct sbridge_dev *prev)
 {
 	struct sbridge_dev *sbridge_dev;
@@ -747,14 +749,15 @@ static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bu
 				      : sbridge_edac_list.next, struct sbridge_dev, list);
 
 	list_for_each_entry_from(sbridge_dev, &sbridge_edac_list, list) {
-		if (sbridge_dev->bus == bus && (dom == SOCK || dom == sbridge_dev->dom))
+		if ((sbridge_dev->seg == seg) && (sbridge_dev->bus == bus) &&
+				(dom == SOCK || dom == sbridge_dev->dom))
 			return sbridge_dev;
 	}
 
 	return NULL;
 }
 
-static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom,
+static struct sbridge_dev *alloc_sbridge_dev(int seg, u8 bus, enum domain dom,
 					     const struct pci_id_table *table)
 {
 	struct sbridge_dev *sbridge_dev;
@@ -771,6 +774,7 @@ static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom,
 		return NULL;
 	}
 
+	sbridge_dev->seg = seg;
 	sbridge_dev->bus = bus;
 	sbridge_dev->dom = dom;
 	sbridge_dev->n_devs = table->n_devs_per_imc;
@@ -2246,6 +2250,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 	struct sbridge_dev *sbridge_dev = NULL;
 	const struct pci_id_descr *dev_descr = &table->descr[devno];
 	struct pci_dev *pdev = NULL;
+	int seg = 0;
 	u8 bus = 0;
 	int i = 0;
 
@@ -2276,10 +2281,12 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 		/* End of list, leave */
 		return -ENODEV;
 	}
+	seg = pci_domain_nr(pdev->bus);
 	bus = pdev->bus->number;
 
 next_imc:
-	sbridge_dev = get_sbridge_dev(bus, dev_descr->dom, multi_bus, sbridge_dev);
+	sbridge_dev = get_sbridge_dev(seg, bus, dev_descr->dom,
+				      multi_bus, sbridge_dev);
 	if (!sbridge_dev) {
 		/* If the HA1 wasn't found, don't create EDAC second memory controller */
 		if (dev_descr->dom == IMC1 && devno != 1) {
@@ -2292,7 +2299,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 		if (dev_descr->dom == SOCK)
 			goto out_imc;
 
-		sbridge_dev = alloc_sbridge_dev(bus, dev_descr->dom, table);
+		sbridge_dev = alloc_sbridge_dev(seg, bus, dev_descr->dom, table);
 		if (!sbridge_dev) {
 			pci_dev_put(pdev);
 			return -ENOMEM;

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [v2] EDAC, sb_edac: Add support for systems with segmented PCI buses
@ 2018-07-24 19:02 Masayoshi Mizuma
  0 siblings, 0 replies; 3+ messages in thread
From: Masayoshi Mizuma @ 2018-07-24 19:02 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Borislav Petkov, linux-edac
  Cc: Masayoshi Mizuma, linux-kernel, Tony Luck, Masayoshi Mizuma

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Currently, sb_edac driver does not support systems which has
multi PCI segments. If the driver is loaded to such system,
a slab-out-bounds happens [*].

This patch extends sb_edac driver to check whether segment number
and bus number matches when deciding how to group memory controller
PCI devices to CPU sockets. 

[*] KASAN report is as follows.

==================================================================
BUG: KASAN: slab-out-of-bounds in sbridge_get_all_devices.constprop.14+0x75f/0x96a [sb_edac]
Read of size 8 at addr ffff8c0d44dfe850 by task modprobe/4221

CPU: 19 PID: 4221 Comm: modprobe Not tainted 4.18.0-rc5 #2
Call Trace:
 dump_stack+0xc2/0x16b
 ? show_regs_print_info+0x5/0x5
 ? kmsg_dump_rewind_nolock+0xd9/0xd9
 ? pci_get_dev_by_id+0x57/0x70
 ? pci_get_device+0x155/0x210
 print_address_description+0x6a/0x270
 kasan_report+0x258/0x380
 ? sbridge_get_all_devices.constprop.14+0x75f/0x96a [sb_edac]
 sbridge_get_all_devices.constprop.14+0x75f/0x96a [sb_edac]
...
==================================================================

Fixes: e2f747b1f42a ("EDAC, sb_edac: Assign EDAC memory controller per h/w controller")

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 drivers/edac/sb_edac.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 4a89c80..07726fb 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -352,6 +352,7 @@ struct pci_id_table {
 
 struct sbridge_dev {
 	struct list_head	list;
+	int			seg;
 	u8			bus, mc;
 	u8			node_id, source_id;
 	struct pci_dev		**pdev;
@@ -729,7 +730,8 @@ static inline int numcol(u32 mtr)
 	return 1 << cols;
 }
 
-static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bus,
+static struct sbridge_dev *get_sbridge_dev(int seg, u8 bus, enum domain dom,
+					   int multi_bus,
 					   struct sbridge_dev *prev)
 {
 	struct sbridge_dev *sbridge_dev;
@@ -747,14 +749,15 @@ static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bu
 				      : sbridge_edac_list.next, struct sbridge_dev, list);
 
 	list_for_each_entry_from(sbridge_dev, &sbridge_edac_list, list) {
-		if (sbridge_dev->bus == bus && (dom == SOCK || dom == sbridge_dev->dom))
+		if ((sbridge_dev->seg == seg) && (sbridge_dev->bus == bus) &&
+				(dom == SOCK || dom == sbridge_dev->dom))
 			return sbridge_dev;
 	}
 
 	return NULL;
 }
 
-static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom,
+static struct sbridge_dev *alloc_sbridge_dev(int seg, u8 bus, enum domain dom,
 					     const struct pci_id_table *table)
 {
 	struct sbridge_dev *sbridge_dev;
@@ -771,6 +774,7 @@ static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom,
 		return NULL;
 	}
 
+	sbridge_dev->seg = seg;
 	sbridge_dev->bus = bus;
 	sbridge_dev->dom = dom;
 	sbridge_dev->n_devs = table->n_devs_per_imc;
@@ -2246,6 +2250,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 	struct sbridge_dev *sbridge_dev = NULL;
 	const struct pci_id_descr *dev_descr = &table->descr[devno];
 	struct pci_dev *pdev = NULL;
+	int seg = 0;
 	u8 bus = 0;
 	int i = 0;
 
@@ -2276,10 +2281,12 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 		/* End of list, leave */
 		return -ENODEV;
 	}
+	seg = pci_domain_nr(pdev->bus);
 	bus = pdev->bus->number;
 
 next_imc:
-	sbridge_dev = get_sbridge_dev(bus, dev_descr->dom, multi_bus, sbridge_dev);
+	sbridge_dev = get_sbridge_dev(seg, bus, dev_descr->dom,
+				      multi_bus, sbridge_dev);
 	if (!sbridge_dev) {
 		/* If the HA1 wasn't found, don't create EDAC second memory controller */
 		if (dev_descr->dom == IMC1 && devno != 1) {
@@ -2292,7 +2299,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 		if (dev_descr->dom == SOCK)
 			goto out_imc;
 
-		sbridge_dev = alloc_sbridge_dev(bus, dev_descr->dom, table);
+		sbridge_dev = alloc_sbridge_dev(seg, bus, dev_descr->dom, table);
 		if (!sbridge_dev) {
 			pci_dev_put(pdev);
 			return -ENOMEM;

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-07-26  0:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-26  0:27 [v2] EDAC, sb_edac: Add support for systems with segmented PCI buses Masayoshi Mizuma
  -- strict thread matches above, loose matches on Subject: below --
2018-07-25  9:22 Borislav Petkov
2018-07-24 19:02 Masayoshi Mizuma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).