* Re: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4
[not found] <046bf689-9a2b-4993-b8ca-927d7d2a0cc5@stanley.mountain>
@ 2025-03-03 14:52 ` Borislav Petkov
2025-03-11 2:22 ` David Thompson
0 siblings, 1 reply; 6+ messages in thread
From: Borislav Petkov @ 2025-03-03 14:52 UTC (permalink / raw)
To: Dan Carpenter, David Thompson; +Cc: Shravan Kumar Ramani, linux-edac, lkml
On Thu, Oct 24, 2024 at 11:20:45AM +0300, Dan Carpenter wrote:
> Hello Shravan Kumar Ramani,
>
> Commit 82413e562ea6 ("EDAC, mellanox: Add ECC support for BlueField
> DDR4") from Jun 25, 2019 (linux-next), leads to the following Smatch
> static checker warning:
>
> drivers/edac/bluefield_edac.c:205 bluefield_gather_report_ecc() error: uninitialized symbol 'dram_syndrom'.
> drivers/edac/bluefield_edac.c:219 bluefield_gather_report_ecc() error: uninitialized symbol 'dram_additional_info'.
> drivers/edac/bluefield_edac.c:231 bluefield_gather_report_ecc() error: uninitialized symbol 'edea0'.
> drivers/edac/bluefield_edac.c:231 bluefield_gather_report_ecc() error: uninitialized symbol 'edea1'.
> drivers/edac/bluefield_edac.c:256 bluefield_edac_check() error: uninitialized symbol 'ecc_count'.
>
> drivers/edac/bluefield_edac.c
> 173 static void bluefield_gather_report_ecc(struct mem_ctl_info *mci,
> 174 int error_cnt,
> 175 int is_single_ecc)
> 176 {
> 177 struct bluefield_edac_priv *priv = mci->pvt_info;
> 178 u32 dram_additional_info, err_prank, edea0, edea1;
> 179 u32 ecc_latch_select, dram_syndrom, serr, derr, syndrom;
> 180 enum hw_event_mc_err_type ecc_type;
> 181 u64 ecc_dimm_addr;
> 182 int ecc_dimm, err;
> 183
> 184 ecc_type = is_single_ecc ? HW_EVENT_ERR_CORRECTED :
> 185 HW_EVENT_ERR_UNCORRECTED;
> 186
> 187 /*
> 188 * Tell the External Memory Interface to populate the relevant
> 189 * registers with information about the last ECC error occurrence.
> 190 */
> 191 ecc_latch_select = MLXBF_ECC_LATCH_SEL__START;
> 192 err = bluefield_edac_writel(priv, MLXBF_ECC_LATCH_SEL, ecc_latch_select);
> 193 if (err)
> 194 dev_err(priv->dev, "ECC latch select write failed.\n");
> 195
> 196 /*
> 197 * Verify that the ECC reported info in the registers is of the
> 198 * same type as the one asked to report. If not, just report the
> 199 * error without the detailed information.
> 200 */
> 201 err = bluefield_edac_readl(priv, MLXBF_SYNDROM, &dram_syndrom);
> 202 if (err)
> 203 dev_err(priv->dev, "DRAM syndrom read failed.\n");
>
> If bluefield_edac_readl() fails then dram_syndrom is uninitialized.
>
> 204
> --> 205 serr = FIELD_GET(MLXBF_SYNDROM__SERR, dram_syndrom);
> 206 derr = FIELD_GET(MLXBF_SYNDROM__DERR, dram_syndrom);
> 207 syndrom = FIELD_GET(MLXBF_SYNDROM__SYN, dram_syndrom);
>
This looks forgotten.
I'm thinking of:
diff --git a/MAINTAINERS b/MAINTAINERS
index 8e0736dc2ee0..061149ade8c0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8222,8 +8222,7 @@ F: Documentation/devicetree/bindings/edac/aspeed-sdram-edac.txt
F: drivers/edac/aspeed_edac.c
EDAC-BLUEFIELD
-M: Shravan Kumar Ramani <shravankr@nvidia.com>
-S: Supported
+S: Orphan
F: drivers/edac/bluefield_edac.c
EDAC-CALXEDA
but lemme Cc people who have touched this recently first.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4
2025-03-03 14:52 ` [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4 Borislav Petkov
@ 2025-03-11 2:22 ` David Thompson
2025-03-11 8:12 ` Dan Carpenter
2025-03-11 10:31 ` Borislav Petkov
0 siblings, 2 replies; 6+ messages in thread
From: David Thompson @ 2025-03-11 2:22 UTC (permalink / raw)
To: Borislav Petkov, Dan Carpenter; +Cc: Shravan Ramani, linux-edac, lkml
> -----Original Message-----
> From: Borislav Petkov <bp@alien8.de>
> Sent: Monday, March 3, 2025 9:52 AM
> To: Dan Carpenter <dan.carpenter@linaro.org>; David Thompson
> <davthompson@nvidia.com>
> Cc: Shravan Ramani <sramani@mellanox.com>; linux-edac@vger.kernel.org; lkml
> <linux-kernel@vger.kernel.org>
> Subject: Re: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4
>
> On Thu, Oct 24, 2024 at 11:20:45AM +0300, Dan Carpenter wrote:
> > Hello Shravan Kumar Ramani,
> >
> > Commit 82413e562ea6 ("EDAC, mellanox: Add ECC support for BlueField
> > DDR4") from Jun 25, 2019 (linux-next), leads to the following Smatch
> > static checker warning:
> >
> > drivers/edac/bluefield_edac.c:205 bluefield_gather_report_ecc() error:
> uninitialized symbol 'dram_syndrom'.
> > drivers/edac/bluefield_edac.c:219 bluefield_gather_report_ecc() error:
> uninitialized symbol 'dram_additional_info'.
> > drivers/edac/bluefield_edac.c:231 bluefield_gather_report_ecc() error:
> uninitialized symbol 'edea0'.
> > drivers/edac/bluefield_edac.c:231 bluefield_gather_report_ecc() error:
> uninitialized symbol 'edea1'.
> > drivers/edac/bluefield_edac.c:256 bluefield_edac_check() error: uninitialized
> symbol 'ecc_count'.
> >
> > drivers/edac/bluefield_edac.c
> > 173 static void bluefield_gather_report_ecc(struct mem_ctl_info *mci,
> > 174 int error_cnt,
> > 175 int is_single_ecc)
> > 176 {
> > 177 struct bluefield_edac_priv *priv = mci->pvt_info;
> > 178 u32 dram_additional_info, err_prank, edea0, edea1;
> > 179 u32 ecc_latch_select, dram_syndrom, serr, derr, syndrom;
> > 180 enum hw_event_mc_err_type ecc_type;
> > 181 u64 ecc_dimm_addr;
> > 182 int ecc_dimm, err;
> > 183
> > 184 ecc_type = is_single_ecc ? HW_EVENT_ERR_CORRECTED :
> > 185 HW_EVENT_ERR_UNCORRECTED;
> > 186
> > 187 /*
> > 188 * Tell the External Memory Interface to populate the relevant
> > 189 * registers with information about the last ECC error occurrence.
> > 190 */
> > 191 ecc_latch_select = MLXBF_ECC_LATCH_SEL__START;
> > 192 err = bluefield_edac_writel(priv, MLXBF_ECC_LATCH_SEL,
> ecc_latch_select);
> > 193 if (err)
> > 194 dev_err(priv->dev, "ECC latch select write failed.\n");
> > 195
> > 196 /*
> > 197 * Verify that the ECC reported info in the registers is of the
> > 198 * same type as the one asked to report. If not, just report the
> > 199 * error without the detailed information.
> > 200 */
> > 201 err = bluefield_edac_readl(priv, MLXBF_SYNDROM,
> &dram_syndrom);
> > 202 if (err)
> > 203 dev_err(priv->dev, "DRAM syndrom read failed.\n");
> >
> > If bluefield_edac_readl() fails then dram_syndrom is uninitialized.
> >
> > 204
> > --> 205 serr = FIELD_GET(MLXBF_SYNDROM__SERR, dram_syndrom);
> > 206 derr = FIELD_GET(MLXBF_SYNDROM__DERR, dram_syndrom);
> > 207 syndrom = FIELD_GET(MLXBF_SYNDROM__SYN, dram_syndrom);
> >
>
> This looks forgotten.
>
> I'm thinking of:
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8e0736dc2ee0..061149ade8c0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8222,8 +8222,7 @@ F:
> Documentation/devicetree/bindings/edac/aspeed-sdram-edac.txt
> F: drivers/edac/aspeed_edac.c
>
> EDAC-BLUEFIELD
> -M: Shravan Kumar Ramani <shravankr@nvidia.com>
> -S: Supported
> +S: Orphan
> F: drivers/edac/bluefield_edac.c
>
> EDAC-CALXEDA
>
> but lemme Cc people who have touched this recently first.
>
> Thx.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
Shravan and I will investigate solving this 'smatch' warning.
I setup a recent linux-next repo, installed the sparse package, and downloaded + built the smatch tool.
I execute the following command to run 'smatch' over the entire linux kernel, including bluefield_edac.c:
$ make CHECK="/path/to/smatch -p=kernel --file-output" C=1 -j64
I don't see any warnings related to the bluefield_edac.c module. That is, the generated file
"drivers/edac/bluefield_edac.c.smatch" has size 0, and thus no content.
What is the recommended command line options to pass to Linux kernel build?
Is there something I am missing here?
- Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4
2025-03-11 2:22 ` David Thompson
@ 2025-03-11 8:12 ` Dan Carpenter
2025-03-11 10:31 ` Borislav Petkov
1 sibling, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2025-03-11 8:12 UTC (permalink / raw)
To: David Thompson; +Cc: Borislav Petkov, Shravan Ramani, linux-edac, lkml
On Tue, Mar 11, 2025 at 02:22:20AM +0000, David Thompson wrote:
> > -----Original Message-----
> > From: Borislav Petkov <bp@alien8.de>
> > Sent: Monday, March 3, 2025 9:52 AM
> > To: Dan Carpenter <dan.carpenter@linaro.org>; David Thompson
> > <davthompson@nvidia.com>
> > Cc: Shravan Ramani <sramani@mellanox.com>; linux-edac@vger.kernel.org; lkml
> > <linux-kernel@vger.kernel.org>
> > Subject: Re: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4
> >
> > On Thu, Oct 24, 2024 at 11:20:45AM +0300, Dan Carpenter wrote:
> > > Hello Shravan Kumar Ramani,
> > >
> > > Commit 82413e562ea6 ("EDAC, mellanox: Add ECC support for BlueField
> > > DDR4") from Jun 25, 2019 (linux-next), leads to the following Smatch
> > > static checker warning:
> > >
> > > drivers/edac/bluefield_edac.c:205 bluefield_gather_report_ecc() error:
> > uninitialized symbol 'dram_syndrom'.
> > > drivers/edac/bluefield_edac.c:219 bluefield_gather_report_ecc() error:
> > uninitialized symbol 'dram_additional_info'.
> > > drivers/edac/bluefield_edac.c:231 bluefield_gather_report_ecc() error:
> > uninitialized symbol 'edea0'.
> > > drivers/edac/bluefield_edac.c:231 bluefield_gather_report_ecc() error:
> > uninitialized symbol 'edea1'.
> > > drivers/edac/bluefield_edac.c:256 bluefield_edac_check() error: uninitialized
> > symbol 'ecc_count'.
> > >
> > > drivers/edac/bluefield_edac.c
> > > 173 static void bluefield_gather_report_ecc(struct mem_ctl_info *mci,
> > > 174 int error_cnt,
> > > 175 int is_single_ecc)
> > > 176 {
> > > 177 struct bluefield_edac_priv *priv = mci->pvt_info;
> > > 178 u32 dram_additional_info, err_prank, edea0, edea1;
> > > 179 u32 ecc_latch_select, dram_syndrom, serr, derr, syndrom;
> > > 180 enum hw_event_mc_err_type ecc_type;
> > > 181 u64 ecc_dimm_addr;
> > > 182 int ecc_dimm, err;
> > > 183
> > > 184 ecc_type = is_single_ecc ? HW_EVENT_ERR_CORRECTED :
> > > 185 HW_EVENT_ERR_UNCORRECTED;
> > > 186
> > > 187 /*
> > > 188 * Tell the External Memory Interface to populate the relevant
> > > 189 * registers with information about the last ECC error occurrence.
> > > 190 */
> > > 191 ecc_latch_select = MLXBF_ECC_LATCH_SEL__START;
> > > 192 err = bluefield_edac_writel(priv, MLXBF_ECC_LATCH_SEL,
> > ecc_latch_select);
> > > 193 if (err)
> > > 194 dev_err(priv->dev, "ECC latch select write failed.\n");
> > > 195
> > > 196 /*
> > > 197 * Verify that the ECC reported info in the registers is of the
> > > 198 * same type as the one asked to report. If not, just report the
> > > 199 * error without the detailed information.
> > > 200 */
> > > 201 err = bluefield_edac_readl(priv, MLXBF_SYNDROM,
> > &dram_syndrom);
> > > 202 if (err)
> > > 203 dev_err(priv->dev, "DRAM syndrom read failed.\n");
> > >
> > > If bluefield_edac_readl() fails then dram_syndrom is uninitialized.
> > >
> > > 204
> > > --> 205 serr = FIELD_GET(MLXBF_SYNDROM__SERR, dram_syndrom);
> > > 206 derr = FIELD_GET(MLXBF_SYNDROM__DERR, dram_syndrom);
> > > 207 syndrom = FIELD_GET(MLXBF_SYNDROM__SYN, dram_syndrom);
> > >
> >
> > This looks forgotten.
> >
> > I'm thinking of:
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 8e0736dc2ee0..061149ade8c0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -8222,8 +8222,7 @@ F:
> > Documentation/devicetree/bindings/edac/aspeed-sdram-edac.txt
> > F: drivers/edac/aspeed_edac.c
> >
> > EDAC-BLUEFIELD
> > -M: Shravan Kumar Ramani <shravankr@nvidia.com>
> > -S: Supported
> > +S: Orphan
> > F: drivers/edac/bluefield_edac.c
> >
> > EDAC-CALXEDA
> >
> > but lemme Cc people who have touched this recently first.
> >
> > Thx.
> >
> > --
> > Regards/Gruss,
> > Boris.
> >
> > https://people.kernel.org/tglx/notes-about-netiquette
>
> Shravan and I will investigate solving this 'smatch' warning.
>
> I setup a recent linux-next repo, installed the sparse package, and downloaded + built the smatch tool.
> I execute the following command to run 'smatch' over the entire linux kernel, including bluefield_edac.c:
> $ make CHECK="/path/to/smatch -p=kernel --file-output" C=1 -j64
>
> I don't see any warnings related to the bluefield_edac.c module. That is, the generated file
> "drivers/edac/bluefield_edac.c.smatch" has size 0, and thus no content.
>
> What is the recommended command line options to pass to Linux kernel build?
> Is there something I am missing here?
The actual issue is the first "return -1" in secure_readl().
You need build the cross function database to see the warning in Smatch.
In fact you'd need to build it twice because it's two functions away.
It's not hard to build the database but it takes a few hours.
~/progs/smatch/release/smatch_scripts/build_kernel_data.sh
I wouldn't bother trying to silence the warning. We don't worry about
false positives. The only thing to consider is the kernel code.
Probably bluefield_edac_readl() can't actually fail in real life can
it? There are a lot of functions like that in the kernel. If reading
from the PCI stops working, there is nothing you can do in the software
to recover from that, you need to buy new hardware. If you want I can
just add bluefield_edac_readl() to the list of functions which don't
actually fail.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4
2025-03-11 2:22 ` David Thompson
2025-03-11 8:12 ` Dan Carpenter
@ 2025-03-11 10:31 ` Borislav Petkov
2025-03-17 21:34 ` David Thompson
1 sibling, 1 reply; 6+ messages in thread
From: Borislav Petkov @ 2025-03-11 10:31 UTC (permalink / raw)
To: David Thompson; +Cc: Dan Carpenter, Shravan Ramani, linux-edac, lkml
On Tue, Mar 11, 2025 at 02:22:20AM +0000, David Thompson wrote:
> Shravan and I will investigate solving this 'smatch' warning.
Thanks, sounds like you're still interested in this driver.
Which means, I'd need either of you to be responsive on the ML and review when
patches like that get sent. Otherwise, I'll simply orphan the thing and it'll
bitrot until it gets removed completely.
So, if interested, which email address should be in MAINTAINERS so that it
gets CCed? Shravan's or yours or both or some ML you guys want to be CCed?
> I setup a recent linux-next repo, installed the sparse package, and
> downloaded + built the smatch tool.
Btw, you don't really need linux-next - you can follow EDAC development by
fetching the edac-for-next branch from this repo here:
https://web.git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git
That's what goes into linux-next too and EDAC patches should be based on that
branch.
Holler if there are questions.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4
2025-03-11 10:31 ` Borislav Petkov
@ 2025-03-17 21:34 ` David Thompson
2025-03-18 23:42 ` Borislav Petkov
0 siblings, 1 reply; 6+ messages in thread
From: David Thompson @ 2025-03-17 21:34 UTC (permalink / raw)
To: Borislav Petkov; +Cc: Dan Carpenter, Shravan Ramani, linux-edac, lkml
> -----Original Message-----
> From: Borislav Petkov <bp@alien8.de>
> Sent: Tuesday, March 11, 2025 6:31 AM
> To: David Thompson <davthompson@nvidia.com>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>; Shravan Ramani
> <shravankr@nvidia.com>; linux-edac@vger.kernel.org; lkml <linux-
> kernel@vger.kernel.org>
> Subject: Re: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4
>
> On Tue, Mar 11, 2025 at 02:22:20AM +0000, David Thompson wrote:
> > Shravan and I will investigate solving this 'smatch' warning.
>
> Thanks, sounds like you're still interested in this driver.
>
> Which means, I'd need either of you to be responsive on the ML and review when
> patches like that get sent. Otherwise, I'll simply orphan the thing and it'll bitrot
> until it gets removed completely.
>
> So, if interested, which email address should be in MAINTAINERS so that it gets
> CCed? Shravan's or yours or both or some ML you guys want to be CCed?
>
Shravan's email is currently listed in MAINTAINERS for bluefield_edac.c
My email address can be added there as well. Shall I post a patch to do this?
> > I setup a recent linux-next repo, installed the sparse package, and
> > downloaded + built the smatch tool.
>
> Btw, you don't really need linux-next - you can follow EDAC development by
> fetching the edac-for-next branch from this repo here:
>
> https://web.git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git
>
> That's what goes into linux-next too and EDAC patches should be based on that
> branch.
Ok, will do future work out of the "ras" repo, edac-for-next branch.
>
> Holler if there are questions.
>
> Thx.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4
2025-03-17 21:34 ` David Thompson
@ 2025-03-18 23:42 ` Borislav Petkov
0 siblings, 0 replies; 6+ messages in thread
From: Borislav Petkov @ 2025-03-18 23:42 UTC (permalink / raw)
To: David Thompson; +Cc: Dan Carpenter, Shravan Ramani, linux-edac, lkml
On Mon, Mar 17, 2025 at 09:34:05PM +0000, David Thompson wrote:
> Shravan's email is currently listed in MAINTAINERS for bluefield_edac.c
> My email address can be added there as well. Shall I post a patch to do this?
Depends on you guys, whether you will maintain it and be responsive on patches.
Two people are fine too as long as the mails don't disappear into a black
hole.
> Ok, will do future work out of the "ras" repo, edac-for-next branch.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-18 23:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <046bf689-9a2b-4993-b8ca-927d7d2a0cc5@stanley.mountain>
2025-03-03 14:52 ` [bug report] EDAC, mellanox: Add ECC support for BlueField DDR4 Borislav Petkov
2025-03-11 2:22 ` David Thompson
2025-03-11 8:12 ` Dan Carpenter
2025-03-11 10:31 ` Borislav Petkov
2025-03-17 21:34 ` David Thompson
2025-03-18 23:42 ` Borislav Petkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®