| Message ID | 20221222131656.49584-1-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Qcom: LLCC/EDAC: Fix base address used for LLCC banks | expand |
On Thu, Dec 22, 2022 at 06:46:40PM +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, SM8450, and SM6350. Testing on other platforms is welcomed. > > Thanks, > Mani Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride With this series applied: 1. The module loads automatically 2. I get the edac driver setup in interrupt mode 3. unloading/loading the module repeatedly works without issue Thanks Mani! > > Changes in v4: > > * Added a patch that fixes the use-after-free bug in qcom_edac driver > > Changes in v3: > > * Brought back reg-names property for compatibility (Krzysztof) > * Removed Fixes tag and stable list as backporting the drivers/binding/dts > patches alone would break (Krzysztof) > * Fixed the uninitialized variable issue (Kbot) > * Added a patch to make use of driver supplied polling interval (Luca) > * Added a patch for module autoloading (Andrew) > * Didn't collect Review tags from Sai as the dts patches were changed. > > Changes in v2: > > * Removed reg-names property and used index of reg property to parse LLCC > bank base address (Bjorn) > * Collected Ack from Sai for binding > * Added a new patch for polling mode (Luca) > * Renamed subject of patches targeting SC7180 and SM6350 > > Manivannan Sadhasivam (16): > dt-bindings: arm: msm: Update the maintainers for LLCC > dt-bindings: arm: msm: Fix register regions used for LLCC banks > arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks > arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks > arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks > arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks > EDAC/device: Make use of poll_msec value in edac_device_ctl_info > struct > EDAC/qcom: Add platform_device_id table for module autoloading > EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's > pvt_info > qcom: llcc/edac: Fix the base address used for accessing LLCC banks > qcom: llcc/edac: Support polling mode for ECC handling > > .../bindings/arm/msm/qcom,llcc.yaml | 128 ++++++++++++++++-- > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 +- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm6350.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm8350.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm8450.dtsi | 7 +- > drivers/edac/edac_device.c | 2 +- > drivers/edac/qcom_edac.c | 63 +++++---- > drivers/soc/qcom/llcc-qcom.c | 85 ++++++------ > include/linux/soc/qcom/llcc-qcom.h | 6 +- > 14 files changed, 243 insertions(+), 95 deletions(-) > > -- > 2.25.1 >
On Thu, Dec 22, 2022 at 7:17 AM Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> 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, SM8450, and SM6350. Testing on other platforms is welcomed. > > Thanks, > Mani > > Changes in v4: > > * Added a patch that fixes the use-after-free bug in qcom_edac driver > > Changes in v3: > > * Brought back reg-names property for compatibility (Krzysztof) > * Removed Fixes tag and stable list as backporting the drivers/binding/dts > patches alone would break (Krzysztof) > * Fixed the uninitialized variable issue (Kbot) > * Added a patch to make use of driver supplied polling interval (Luca) > * Added a patch for module autoloading (Andrew) > * Didn't collect Review tags from Sai as the dts patches were changed. > > Changes in v2: > > * Removed reg-names property and used index of reg property to parse LLCC > bank base address (Bjorn) > * Collected Ack from Sai for binding > * Added a new patch for polling mode (Luca) > * Renamed subject of patches targeting SC7180 and SM6350 > > Manivannan Sadhasivam (16): > dt-bindings: arm: msm: Update the maintainers for LLCC > dt-bindings: arm: msm: Fix register regions used for LLCC banks > arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks > arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks > arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks > arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks > EDAC/device: Make use of poll_msec value in edac_device_ctl_info > struct > EDAC/qcom: Add platform_device_id table for module autoloading > EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's > pvt_info > qcom: llcc/edac: Fix the base address used for accessing LLCC banks > qcom: llcc/edac: Support polling mode for ECC handling > > .../bindings/arm/msm/qcom,llcc.yaml | 128 ++++++++++++++++-- > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 +- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm6350.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm8350.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm8450.dtsi | 7 +- > drivers/edac/edac_device.c | 2 +- > drivers/edac/qcom_edac.c | 63 +++++---- > drivers/soc/qcom/llcc-qcom.c | 85 ++++++------ > include/linux/soc/qcom/llcc-qcom.h | 6 +- > 14 files changed, 243 insertions(+), 95 deletions(-) > > -- > 2.25.1 > Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s Like Andrew, tested starting, restarting, and stopping the edac service multiple times. edac-ctl --mainboard reports "edac-ctl: mainboard: LENOVO 21BX0015US" Thanks Mani! -- steev
On Thu, Dec 22, 2022 at 06:46:40PM +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, SM8450, and SM6350. Testing on other platforms is welcomed. > > Thanks, > Mani > > Changes in v4: > > * Added a patch that fixes the use-after-free bug in qcom_edac driver > > Changes in v3: > > * Brought back reg-names property for compatibility (Krzysztof) > * Removed Fixes tag and stable list as backporting the drivers/binding/dts > patches alone would break (Krzysztof) > * Fixed the uninitialized variable issue (Kbot) > * Added a patch to make use of driver supplied polling interval (Luca) > * Added a patch for module autoloading (Andrew) > * Didn't collect Review tags from Sai as the dts patches were changed. > > Changes in v2: > > * Removed reg-names property and used index of reg property to parse LLCC > bank base address (Bjorn) > * Collected Ack from Sai for binding > * Added a new patch for polling mode (Luca) > * Renamed subject of patches targeting SC7180 and SM6350 > > Manivannan Sadhasivam (16): > dt-bindings: arm: msm: Update the maintainers for LLCC > dt-bindings: arm: msm: Fix register regions used for LLCC banks > arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks > arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks > arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks > arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks > arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks > EDAC/device: Make use of poll_msec value in edac_device_ctl_info > struct > EDAC/qcom: Add platform_device_id table for module autoloading > EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's > pvt_info Can you clarify if these patches needs to be applied in the specific order, or if the EDAC changes can go in independently of the llcc driver changes? Thanks, Bjorn > qcom: llcc/edac: Fix the base address used for accessing LLCC banks > qcom: llcc/edac: Support polling mode for ECC handling > > .../bindings/arm/msm/qcom,llcc.yaml | 128 ++++++++++++++++-- > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 +- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm6350.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sm8150.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm8350.dtsi | 7 +- > arch/arm64/boot/dts/qcom/sm8450.dtsi | 7 +- > drivers/edac/edac_device.c | 2 +- > drivers/edac/qcom_edac.c | 63 +++++---- > drivers/soc/qcom/llcc-qcom.c | 85 ++++++------ > include/linux/soc/qcom/llcc-qcom.h | 6 +- > 14 files changed, 243 insertions(+), 95 deletions(-) > > -- > 2.25.1 >
On Tue, Dec 27, 2022 at 10:31:38PM -0600, Bjorn Andersson wrote: > On Thu, Dec 22, 2022 at 06:46:40PM +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, SM8450, and SM6350. Testing on other platforms is welcomed. > > > > Thanks, > > Mani > > > > Changes in v4: > > > > * Added a patch that fixes the use-after-free bug in qcom_edac driver > > > > Changes in v3: > > > > * Brought back reg-names property for compatibility (Krzysztof) > > * Removed Fixes tag and stable list as backporting the drivers/binding/dts > > patches alone would break (Krzysztof) > > * Fixed the uninitialized variable issue (Kbot) > > * Added a patch to make use of driver supplied polling interval (Luca) > > * Added a patch for module autoloading (Andrew) > > * Didn't collect Review tags from Sai as the dts patches were changed. > > > > Changes in v2: > > > > * Removed reg-names property and used index of reg property to parse LLCC > > bank base address (Bjorn) > > * Collected Ack from Sai for binding > > * Added a new patch for polling mode (Luca) > > * Renamed subject of patches targeting SC7180 and SM6350 > > > > Manivannan Sadhasivam (16): > > dt-bindings: arm: msm: Update the maintainers for LLCC > > dt-bindings: arm: msm: Fix register regions used for LLCC banks > > arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks > > arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks > > arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks > > arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks > > arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks > > arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks > > arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks > > arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks > > arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks > > EDAC/device: Make use of poll_msec value in edac_device_ctl_info > > struct > > EDAC/qcom: Add platform_device_id table for module autoloading > > EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's > > pvt_info > > Can you clarify if these patches needs to be applied in the specific > order, or if the EDAC changes can go in independently of the llcc driver > changes? > The EDAC/qcom and EDAC/device patches can go independently of the rest. All other patches needs to go through qcom tree. Steev spotted an issue on C630, so I'm going to respin the series one more time. Thanks, Mani > Thanks, > Bjorn > > > qcom: llcc/edac: Fix the base address used for accessing LLCC banks > > qcom: llcc/edac: Support polling mode for ECC handling > > > > .../bindings/arm/msm/qcom,llcc.yaml | 128 ++++++++++++++++-- > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +- > > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 +- > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 7 +- > > arch/arm64/boot/dts/qcom/sm6350.dtsi | 2 +- > > arch/arm64/boot/dts/qcom/sm8150.dtsi | 7 +- > > arch/arm64/boot/dts/qcom/sm8250.dtsi | 7 +- > > arch/arm64/boot/dts/qcom/sm8350.dtsi | 7 +- > > arch/arm64/boot/dts/qcom/sm8450.dtsi | 7 +- > > drivers/edac/edac_device.c | 2 +- > > drivers/edac/qcom_edac.c | 63 +++++---- > > drivers/soc/qcom/llcc-qcom.c | 85 ++++++------ > > include/linux/soc/qcom/llcc-qcom.h | 6 +- > > 14 files changed, 243 insertions(+), 95 deletions(-) > > > > -- > > 2.25.1 > >
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, SM8450, and SM6350. Testing on other platforms is welcomed. Thanks, Mani Changes in v4: * Added a patch that fixes the use-after-free bug in qcom_edac driver Changes in v3: * Brought back reg-names property for compatibility (Krzysztof) * Removed Fixes tag and stable list as backporting the drivers/binding/dts patches alone would break (Krzysztof) * Fixed the uninitialized variable issue (Kbot) * Added a patch to make use of driver supplied polling interval (Luca) * Added a patch for module autoloading (Andrew) * Didn't collect Review tags from Sai as the dts patches were changed. Changes in v2: * Removed reg-names property and used index of reg property to parse LLCC bank base address (Bjorn) * Collected Ack from Sai for binding * Added a new patch for polling mode (Luca) * Renamed subject of patches targeting SC7180 and SM6350 Manivannan Sadhasivam (16): dt-bindings: arm: msm: Update the maintainers for LLCC dt-bindings: arm: msm: Fix register regions used for LLCC banks arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks EDAC/device: Make use of poll_msec value in edac_device_ctl_info struct EDAC/qcom: Add platform_device_id table for module autoloading EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's pvt_info qcom: llcc/edac: Fix the base address used for accessing LLCC banks qcom: llcc/edac: Support polling mode for ECC handling .../bindings/arm/msm/qcom,llcc.yaml | 128 ++++++++++++++++-- arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +- arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 +- arch/arm64/boot/dts/qcom/sdm845.dtsi | 7 +- arch/arm64/boot/dts/qcom/sm6350.dtsi | 2 +- arch/arm64/boot/dts/qcom/sm8150.dtsi | 7 +- arch/arm64/boot/dts/qcom/sm8250.dtsi | 7 +- arch/arm64/boot/dts/qcom/sm8350.dtsi | 7 +- arch/arm64/boot/dts/qcom/sm8450.dtsi | 7 +- drivers/edac/edac_device.c | 2 +- drivers/edac/qcom_edac.c | 63 +++++---- drivers/soc/qcom/llcc-qcom.c | 85 ++++++------ include/linux/soc/qcom/llcc-qcom.h | 6 +- 14 files changed, 243 insertions(+), 95 deletions(-)