| Message ID | 20241119-qcom-scm-missing-barriers-and-all-sort-of-srap-v1-0-7056127007a7@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | firmware: qcom: scm: Fixes for concurrency | expand |
On Tue, Nov 19, 2024 at 07:33:16PM +0100, Krzysztof Kozlowski wrote: > SCM driver looks messy in terms of handling concurrency of probe. The > driver exports interface which is guarded by global '__scm' variable > but: > 1. Lacks proper read barrier (commit adding write barriers mixed up > READ_ONCE with a read barrier). > 2. Lacks barriers or checks for '__scm' in multiple places. > 3. Lacks probe error cleanup. > > I fixed here few visible things, but this was not tested extensively. I > tried only SM8450. > > ARM32 and SC8280xp/X1E platforms would be useful for testing as well. ARM32 devices are present in the lab. > > All the issues here are non-urgent, IOW, they were here for some time > (v6.10-rc1 and earlier). > > Best regards, > Krzysztof > > --- > Krzysztof Kozlowski (6): > firmware: qcom: scm: Fix missing read barrier in qcom_scm_is_available() > firmware: qcom: scm: Fix missing read barrier in qcom_scm_get_tzmem_pool() > firmware: qcom: scm: Handle various probe ordering for qcom_scm_assign_mem() > [RFC/RFT] firmware: qcom: scm: Cleanup global '__scm' on probe failures > firmware: qcom: scm: smc: Handle missing SCM device > firmware: qcom: scm: smc: Narrow 'mempool' variable scope > > drivers/firmware/qcom/qcom_scm-smc.c | 6 +++- > drivers/firmware/qcom/qcom_scm.c | 55 +++++++++++++++++++++++++----------- > 2 files changed, 44 insertions(+), 17 deletions(-) > --- > base-commit: 414c97c966b69e4a6ea7b32970fa166b2f9b9ef0 > change-id: 20241119-qcom-scm-missing-barriers-and-all-sort-of-srap-a25d59074882 > > Best regards, > -- > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >
On 20/11/2024 12:13, Dmitry Baryshkov wrote: > On Tue, Nov 19, 2024 at 07:33:16PM +0100, Krzysztof Kozlowski wrote: >> SCM driver looks messy in terms of handling concurrency of probe. The >> driver exports interface which is guarded by global '__scm' variable >> but: >> 1. Lacks proper read barrier (commit adding write barriers mixed up >> READ_ONCE with a read barrier). >> 2. Lacks barriers or checks for '__scm' in multiple places. >> 3. Lacks probe error cleanup. >> >> I fixed here few visible things, but this was not tested extensively. I >> tried only SM8450. >> >> ARM32 and SC8280xp/X1E platforms would be useful for testing as well. > > ARM32 devices are present in the lab. I passed the patchset on our devices, and no regressions observed: arm32: https://git.codelinaro.org/linaro/qcomlt/ci/staging/cdba-tester/-/pipelines/116195 arm64(including x1e): https://git.codelinaro.org/linaro/qcomlt/ci/staging/cdba-tester/-/pipelines/116201 Neil > >> >> All the issues here are non-urgent, IOW, they were here for some time >> (v6.10-rc1 and earlier). >> >> Best regards, >> Krzysztof >> >> --- >> Krzysztof Kozlowski (6): >> firmware: qcom: scm: Fix missing read barrier in qcom_scm_is_available() >> firmware: qcom: scm: Fix missing read barrier in qcom_scm_get_tzmem_pool() >> firmware: qcom: scm: Handle various probe ordering for qcom_scm_assign_mem() >> [RFC/RFT] firmware: qcom: scm: Cleanup global '__scm' on probe failures >> firmware: qcom: scm: smc: Handle missing SCM device >> firmware: qcom: scm: smc: Narrow 'mempool' variable scope >> >> drivers/firmware/qcom/qcom_scm-smc.c | 6 +++- >> drivers/firmware/qcom/qcom_scm.c | 55 +++++++++++++++++++++++++----------- >> 2 files changed, 44 insertions(+), 17 deletions(-) >> --- >> base-commit: 414c97c966b69e4a6ea7b32970fa166b2f9b9ef0 >> change-id: 20241119-qcom-scm-missing-barriers-and-all-sort-of-srap-a25d59074882 >> >> Best regards, >> -- >> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >
SCM driver looks messy in terms of handling concurrency of probe. The driver exports interface which is guarded by global '__scm' variable but: 1. Lacks proper read barrier (commit adding write barriers mixed up READ_ONCE with a read barrier). 2. Lacks barriers or checks for '__scm' in multiple places. 3. Lacks probe error cleanup. I fixed here few visible things, but this was not tested extensively. I tried only SM8450. ARM32 and SC8280xp/X1E platforms would be useful for testing as well. All the issues here are non-urgent, IOW, they were here for some time (v6.10-rc1 and earlier). Best regards, Krzysztof --- Krzysztof Kozlowski (6): firmware: qcom: scm: Fix missing read barrier in qcom_scm_is_available() firmware: qcom: scm: Fix missing read barrier in qcom_scm_get_tzmem_pool() firmware: qcom: scm: Handle various probe ordering for qcom_scm_assign_mem() [RFC/RFT] firmware: qcom: scm: Cleanup global '__scm' on probe failures firmware: qcom: scm: smc: Handle missing SCM device firmware: qcom: scm: smc: Narrow 'mempool' variable scope drivers/firmware/qcom/qcom_scm-smc.c | 6 +++- drivers/firmware/qcom/qcom_scm.c | 55 +++++++++++++++++++++++++----------- 2 files changed, 44 insertions(+), 17 deletions(-) --- base-commit: 414c97c966b69e4a6ea7b32970fa166b2f9b9ef0 change-id: 20241119-qcom-scm-missing-barriers-and-all-sort-of-srap-a25d59074882 Best regards,