lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20221213161709.k7r4rdsfwe4pxlid@halaney-x13s> Date: Tue, 13 Dec 2022 10:17:09 -0600 From: Andrew Halaney <ahalaney@...hat.com> To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org> Cc: andersson@...nel.org, robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org, bp@...en8.de, tony.luck@...el.com, quic_saipraka@...cinc.com, konrad.dybcio@...aro.org, linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, james.morse@....com, mchehab@...nel.org, rric@...nel.org, linux-edac@...r.kernel.org, quic_ppareek@...cinc.com, luca.weiss@...rphone.com Subject: Re: [PATCH v2 00/13] Qcom: LLCC/EDAC: Fix base address used for LLCC banks On Tue, Dec 13, 2022 at 10:58:02AM +0530, Manivannan Sadhasivam wrote: > On Mon, Dec 12, 2022 at 01:23:40PM -0600, Andrew Halaney wrote: > > On Mon, Dec 12, 2022 at 06:02:58PM +0530, Manivannan Sadhasivam wrote: > > > The Qualcomm LLCC/EDAC drivers were using a fixed register stride for > > > accessing the (Control and Status Regsiters) CSRs of each LLCC bank. > > > This offset only works for some SoCs like SDM845 for which driver support > > > was initially added. > > > > > > But the later SoCs use different register stride that vary between the > > > banks with holes in-between. So it is not possible to use a single register > > > stride for accessing the CSRs of each bank. By doing so could result in a > > > crash with the current drivers. So far this crash is not reported since > > > EDAC_QCOM driver is not enabled in ARM64 defconfig and no one tested the > > > driver extensively by triggering the EDAC IRQ (that's where each bank > > > CSRs are accessed). > > > > > > For fixing this issue, let's obtain the base address of each LLCC bank from > > > devicetree and get rid of the fixed stride. > > > > > > This series affects multiple platforms but I have only tested this on > > > SM8250 and SM8450. Testing on other platforms is welcomed. > > > > > > > Tested-by: Andrew Halaney <ahalaney@...hat.com> # sa8540p-ride > > > > Thanks! > > > I took this for a quick spin on the qdrive3 I've got access to without > > any issue: > > > > [root@...alhost ~]# modprobe qcom_edac > > [root@...alhost ~]# dmesg | grep -i edac > > [ 0.620723] EDAC MC: Ver: 3.0.0 > > [ 1.165417] ghes_edac: GHES probing device list is empty > > [ 594.688103] EDAC DEVICE0: Giving out device to module qcom_llcc_edac controller llcc: DEV qcom_llcc_edac (INTERRUPT) > > [root@...alhost ~]# cat /proc/interrupts | grep ecc > > 174: 0 0 0 0 0 0 0 0 GICv3 614 Level llcc_ecc > > [root@...alhost ~]# > > > > Potentially stupid question, but are users expected to manually load the > > driver as I did? I don't see how it would be loaded automatically in the > > current state, but thought it was funny that I needed to modprobe > > myself. > > > > Please let me know if you want me to do any more further testing! > > > > Well, I always ended up using the driver as a built-in. I do make it module for > build test but never really used it as a module, so didn't catch this issue. > > This is due to the module alias not exported by the qcom_edac driver. Below > diff allows kernel to autoload it: > > diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c > index f7afb5375293..13919d01c22d 100644 > --- a/drivers/edac/qcom_edac.c > +++ b/drivers/edac/qcom_edac.c > @@ -419,3 +419,4 @@ module_platform_driver(qcom_llcc_edac_driver); > > MODULE_DESCRIPTION("QCOM EDAC driver"); > MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:qcom_llcc_edac"); > > Please test and let me know. I will add this as a new patch in next version. > Thanks Mani, that gets things working for me. For that patch: Reviewed-by: Andrew Halaney <ahalaney@...hat.com> Tested-by: Andrew Halaney <ahalaney@...hat.com> My personal opinion, but that probably deserves a Fixes: tag too!
Powered by blists - more mailing lists