* [PATCH] EDAC/ie31200: work around false positive build warning
@ 2025-01-22 6:50 Arnd Bergmann
2025-01-22 14:18 ` Zhuo, Qiuxu
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2025-01-22 6:50 UTC (permalink / raw)
To: Jason Baron, Borislav Petkov, Tony Luck
Cc: Arnd Bergmann, James Morse, Mauro Carvalho Chehab, Robert Richter,
James Ye, linux-edac, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
gcc-14 produces a bogus warning in some configurations:
drivers/edac/ie31200_edac.c: In function 'ie31200_probe1.isra':
drivers/edac/ie31200_edac.c:412:26: error: 'dimm_info' is used uninitialized [-Werror=uninitialized]
412 | struct dimm_data dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL];
| ^~~~~~~~~
drivers/edac/ie31200_edac.c:412:26: note: 'dimm_info' declared here
412 | struct dimm_data dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL];
| ^~~~~~~~~
I don't see any way the unintialized access could really happen here,
but I can see why the compiler gets confused by the two loops.
Instead, rework the two nested loops to only read the addr_decode
registers and then keep only one instance of the dimm info structure.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/edac/ie31200_edac.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c
index 4fc16922dc1a..b0ccc54dc66a 100644
--- a/drivers/edac/ie31200_edac.c
+++ b/drivers/edac/ie31200_edac.c
@@ -409,10 +409,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int dev_idx)
int i, j, ret;
struct mem_ctl_info *mci = NULL;
struct edac_mc_layer layers[2];
- struct dimm_data dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL];
void __iomem *window;
struct ie31200_priv *priv;
- u32 addr_decode, mad_offset;
+ u32 addr_decode[IE31200_CHANNELS], mad_offset;
/*
* Kaby Lake, Coffee Lake seem to work like Skylake. Please re-visit
@@ -472,17 +471,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int dev_idx)
/* populate DIMM info */
for (i = 0; i < IE31200_CHANNELS; i++) {
- addr_decode = readl(window + mad_offset +
+ addr_decode[i] = readl(window + mad_offset +
(i * 4));
- edac_dbg(0, "addr_decode: 0x%x\n", addr_decode);
- for (j = 0; j < IE31200_DIMMS_PER_CHANNEL; j++) {
- populate_dimm_info(&dimm_info[i][j], addr_decode, j,
- skl);
- edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n",
- dimm_info[i][j].size,
- dimm_info[i][j].dual_rank,
- dimm_info[i][j].x16_width);
- }
+ edac_dbg(0, "addr_decode: 0x%x\n", addr_decode[i]);
}
/*
@@ -493,14 +484,22 @@ static int ie31200_probe1(struct pci_dev *pdev, int dev_idx)
*/
for (i = 0; i < IE31200_DIMMS_PER_CHANNEL; i++) {
for (j = 0; j < IE31200_CHANNELS; j++) {
+ struct dimm_data dimm_info;
struct dimm_info *dimm;
unsigned long nr_pages;
- nr_pages = IE31200_PAGES(dimm_info[j][i].size, skl);
+ populate_dimm_info(&dimm_info, addr_decode[j], i,
+ skl);
+ edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n",
+ dimm_info.size,
+ dimm_info.dual_rank,
+ dimm_info.x16_width);
+
+ nr_pages = IE31200_PAGES(dimm_info.size, skl);
if (nr_pages == 0)
continue;
- if (dimm_info[j][i].dual_rank) {
+ if (dimm_info.dual_rank) {
nr_pages = nr_pages / 2;
dimm = edac_get_dimm(mci, (i * 2) + 1, j, 0);
dimm->nr_pages = nr_pages;
--
2.39.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: [PATCH] EDAC/ie31200: work around false positive build warning
2025-01-22 6:50 [PATCH] EDAC/ie31200: work around false positive build warning Arnd Bergmann
@ 2025-01-22 14:18 ` Zhuo, Qiuxu
2025-01-24 19:27 ` Jason Baron
0 siblings, 1 reply; 4+ messages in thread
From: Zhuo, Qiuxu @ 2025-01-22 14:18 UTC (permalink / raw)
To: Arnd Bergmann, Jason Baron, Borislav Petkov, Luck, Tony
Cc: Arnd Bergmann, James Morse, Mauro Carvalho Chehab, Robert Richter,
James Ye, linux-edac@vger.kernel.org,
linux-kernel@vger.kernel.org
> From: Arnd Bergmann <arnd@kernel.org>
> [...]
> Subject: [PATCH] EDAC/ie31200: work around false positive build warning
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> gcc-14 produces a bogus warning in some configurations:
>
> drivers/edac/ie31200_edac.c: In function 'ie31200_probe1.isra':
> drivers/edac/ie31200_edac.c:412:26: error: 'dimm_info' is used uninitialized
> [-Werror=uninitialized]
> 412 | struct dimm_data
> dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL];
> | ^~~~~~~~~
> drivers/edac/ie31200_edac.c:412:26: note: 'dimm_info' declared here
> 412 | struct dimm_data
> dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL];
> | ^~~~~~~~~
>
> I don't see any way the unintialized access could really happen here, but I can
It looks like gcc-14 isn't smart enough to recognize that this is a false positive build warning :-).
Actually, the dim_info[][] array is always initialized by populate_dimm_info().
> see why the compiler gets confused by the two loops.
>
> Instead, rework the two nested loops to only read the addr_decode registers
> and then keep only one instance of the dimm info structure.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/edac/ie31200_edac.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c index
> 4fc16922dc1a..b0ccc54dc66a 100644
> --- a/drivers/edac/ie31200_edac.c
> +++ b/drivers/edac/ie31200_edac.c
> @@ -409,10 +409,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int
> dev_idx)
> int i, j, ret;
> struct mem_ctl_info *mci = NULL;
> struct edac_mc_layer layers[2];
> - struct dimm_data
> dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL];
> void __iomem *window;
> struct ie31200_priv *priv;
> - u32 addr_decode, mad_offset;
> + u32 addr_decode[IE31200_CHANNELS], mad_offset;
>
> /*
> * Kaby Lake, Coffee Lake seem to work like Skylake. Please re-visit
> @@ -472,17 +471,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int
> dev_idx)
>
> /* populate DIMM info */
This comment also needs to be moved to just above the new location of populate_dimm_info().
> for (i = 0; i < IE31200_CHANNELS; i++) {
> - addr_decode = readl(window + mad_offset +
> + addr_decode[i] = readl(window + mad_offset +
> (i * 4));
> - edac_dbg(0, "addr_decode: 0x%x\n", addr_decode);
> - for (j = 0; j < IE31200_DIMMS_PER_CHANNEL; j++) {
> - populate_dimm_info(&dimm_info[i][j], addr_decode,
> j,
> - skl);
> - edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n",
> - dimm_info[i][j].size,
> - dimm_info[i][j].dual_rank,
> - dimm_info[i][j].x16_width);
> - }
> + edac_dbg(0, "addr_decode: 0x%x\n", addr_decode[i]);
> }
>
> /*
> @@ -493,14 +484,22 @@ static int ie31200_probe1(struct pci_dev *pdev, int
> dev_idx)
> */
> for (i = 0; i < IE31200_DIMMS_PER_CHANNEL; i++) {
> for (j = 0; j < IE31200_CHANNELS; j++) {
> + struct dimm_data dimm_info;
> struct dimm_info *dimm;
> unsigned long nr_pages;
>
> - nr_pages = IE31200_PAGES(dimm_info[j][i].size, skl);
Move the comment here.
> + populate_dimm_info(&dimm_info, addr_decode[j], i,
> + skl);
> + edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n",
> + dimm_info.size,
> + dimm_info.dual_rank,
> + dimm_info.x16_width);
> +
> + nr_pages = IE31200_PAGES(dimm_info.size, skl);
> if (nr_pages == 0)
> continue;
>
> - if (dimm_info[j][i].dual_rank) {
> + if (dimm_info.dual_rank) {
> nr_pages = nr_pages / 2;
> dimm = edac_get_dimm(mci, (i * 2) + 1, j, 0);
> dimm->nr_pages = nr_pages;
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] EDAC/ie31200: work around false positive build warning
2025-01-22 14:18 ` Zhuo, Qiuxu
@ 2025-01-24 19:27 ` Jason Baron
2025-02-21 1:13 ` Luck, Tony
0 siblings, 1 reply; 4+ messages in thread
From: Jason Baron @ 2025-01-24 19:27 UTC (permalink / raw)
To: Zhuo, Qiuxu, Arnd Bergmann, Borislav Petkov, Luck, Tony
Cc: Arnd Bergmann, James Morse, Mauro Carvalho Chehab, Robert Richter,
James Ye, linux-edac@vger.kernel.org,
linux-kernel@vger.kernel.org
On 1/22/25 9:18 AM, Zhuo, Qiuxu wrote:
> !-------------------------------------------------------------------|
> This Message Is From an External Sender
> This message came from outside your organization.
> |-------------------------------------------------------------------!
>
>> From: Arnd Bergmann <arnd@kernel.org>
>> [...]
>> Subject: [PATCH] EDAC/ie31200: work around false positive build warning
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> gcc-14 produces a bogus warning in some configurations:
>>
>> drivers/edac/ie31200_edac.c: In function 'ie31200_probe1.isra':
>> drivers/edac/ie31200_edac.c:412:26: error: 'dimm_info' is used uninitialized
>> [-Werror=uninitialized]
>> 412 | struct dimm_data
>> dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL];
>> | ^~~~~~~~~
>> drivers/edac/ie31200_edac.c:412:26: note: 'dimm_info' declared here
>> 412 | struct dimm_data
>> dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL];
>> | ^~~~~~~~~
>>
>> I don't see any way the unintialized access could really happen here, but I can
>
> It looks like gcc-14 isn't smart enough to recognize that this is a false positive build warning :-).
> Actually, the dim_info[][] array is always initialized by populate_dimm_info().
>
>> see why the compiler gets confused by the two loops.
>>
>> Instead, rework the two nested loops to only read the addr_decode registers
>> and then keep only one instance of the dimm info structure.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Seems fine with the comment adjustment.
Thanks.
Acked-by: Jason Baron <jbaron@akamai.com>
>> ---
>> drivers/edac/ie31200_edac.c | 27 +++++++++++++--------------
>> 1 file changed, 13 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c index
>> 4fc16922dc1a..b0ccc54dc66a 100644
>> --- a/drivers/edac/ie31200_edac.c
>> +++ b/drivers/edac/ie31200_edac.c
>> @@ -409,10 +409,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int
>> dev_idx)
>> int i, j, ret;
>> struct mem_ctl_info *mci = NULL;
>> struct edac_mc_layer layers[2];
>> - struct dimm_data
>> dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL];
>> void __iomem *window;
>> struct ie31200_priv *priv;
>> - u32 addr_decode, mad_offset;
>> + u32 addr_decode[IE31200_CHANNELS], mad_offset;
>>
>> /*
>> * Kaby Lake, Coffee Lake seem to work like Skylake. Please re-visit
>> @@ -472,17 +471,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int
>> dev_idx)
>>
>> /* populate DIMM info */
>
> This comment also needs to be moved to just above the new location of populate_dimm_info().
>
>> for (i = 0; i < IE31200_CHANNELS; i++) {
>> - addr_decode = readl(window + mad_offset +
>> + addr_decode[i] = readl(window + mad_offset +
>> (i * 4));
>> - edac_dbg(0, "addr_decode: 0x%x\n", addr_decode);
>> - for (j = 0; j < IE31200_DIMMS_PER_CHANNEL; j++) {
>> - populate_dimm_info(&dimm_info[i][j], addr_decode,
>> j,
>> - skl);
>> - edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n",
>> - dimm_info[i][j].size,
>> - dimm_info[i][j].dual_rank,
>> - dimm_info[i][j].x16_width);
>> - }
>> + edac_dbg(0, "addr_decode: 0x%x\n", addr_decode[i]);
>> }
>>
>> /*
>> @@ -493,14 +484,22 @@ static int ie31200_probe1(struct pci_dev *pdev, int
>> dev_idx)
>> */
>> for (i = 0; i < IE31200_DIMMS_PER_CHANNEL; i++) {
>> for (j = 0; j < IE31200_CHANNELS; j++) {
>> + struct dimm_data dimm_info;
>> struct dimm_info *dimm;
>> unsigned long nr_pages;
>>
>> - nr_pages = IE31200_PAGES(dimm_info[j][i].size, skl);
>
> Move the comment here.
>
>> + populate_dimm_info(&dimm_info, addr_decode[j], i,
>> + skl);
>> + edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n",
>> + dimm_info.size,
>> + dimm_info.dual_rank,
>> + dimm_info.x16_width);
>> +
>> + nr_pages = IE31200_PAGES(dimm_info.size, skl);
>> if (nr_pages == 0)
>> continue;
>>
>> - if (dimm_info[j][i].dual_rank) {
>> + if (dimm_info.dual_rank) {
>> nr_pages = nr_pages / 2;
>> dimm = edac_get_dimm(mci, (i * 2) + 1, j, 0);
>> dimm->nr_pages = nr_pages;
>> --
>> 2.39.5
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] EDAC/ie31200: work around false positive build warning
2025-01-24 19:27 ` Jason Baron
@ 2025-02-21 1:13 ` Luck, Tony
0 siblings, 0 replies; 4+ messages in thread
From: Luck, Tony @ 2025-02-21 1:13 UTC (permalink / raw)
To: Jason Baron
Cc: Zhuo, Qiuxu, Arnd Bergmann, Borislav Petkov, Arnd Bergmann,
James Morse, Mauro Carvalho Chehab, Robert Richter, James Ye,
linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org
> > It looks like gcc-14 isn't smart enough to recognize that this is a false positive build warning :-).
> > Actually, the dim_info[][] array is always initialized by populate_dimm_info().
> >
> > > see why the compiler gets confused by the two loops.
> > >
> > > Instead, rework the two nested loops to only read the addr_decode registers
> > > and then keep only one instance of the dimm info structure.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Seems fine with the comment adjustment.
>
> Thanks.
>
> Acked-by: Jason Baron <jbaron@akamai.com>
Applied (with comment adjustment) to RAS tree edac-drivers branch
Thanks
-Tony
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-02-21 1:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-22 6:50 [PATCH] EDAC/ie31200: work around false positive build warning Arnd Bergmann
2025-01-22 14:18 ` Zhuo, Qiuxu
2025-01-24 19:27 ` Jason Baron
2025-02-21 1:13 ` Luck, Tony
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox