Message ID | 20240710173153.4060457-1-cristian.marussi@arm.com (mailing list archive) |
---|---|
Headers | show |
Series | Make SCMI transport as standalone drivers | expand |
> Subject: [PATCH 0/8] Make SCMI transport as standalone drivers You may need use V2 here :) > > Hi all, > > Till now the SCMI transport layer was being built embedded into in the > core SCMI stack. > > Some of these transports, despite being currently part of the main > SCMI module, are indeed also registered with different subsystems like > optee or virtio, and actively probed also by those: this led to a few > awkward and convoluted tricks to properly handle such interactions at > boot time in the SCMI stack. > > Moreover some partner expressed the desire to be able to fully > modularize the transports components. > > This series aim to make all such transports as standalone drivers that > can be optionally loaded as modules. > > In order to do this, at first some new mechanism is introduced to > support this new capability while maintaining, in parallel, the old > legacy embedded transports; then each transport, one by one, is > transitioned to be a standalone driver and finally the old legacy > support for embedded transport is removed. > > Patch [1/8] is a mostly unrelated (but much needed) clean-up from > Peng, which I included in this series to avoid conflicts at merge. > > Patch [2/8] simply collects the existing datagram manipulation helpers > in a pair of function pointers structures, in preparation for later reworks. > > Patch [3/8] adds the bulk of the new logic to the core SCMI stack and > then each existing transport is transitioned to be a standalone driver in > patches 4,5,6,7 while shuffling around the compatibles. (no DT change > is needed of curse for backward compatibility) While doing this I kept > the module authorship in line with the main > author(S) as spitted out by git-blame. > > Finally patch [8/8] removes all the legacy dead code from the core > SCMI stack. > > No new symbol EXPORT has been added. > > The new transport drivers have been tested, as built-in and LKM, as > follows: > > - mailbox on JUNO > - virtio on emulation > - optee on QEMU/optee using Linaro setup > > Exercised using the regular SCMI drivers stack and the SCMI ACS suite, > testing commands, replies, delayed responses and notification. > > Multiple virtual SCMI instances support has been tested too. > > SMC has NOT been tested/exercised at run-time, only compile-tested. > (due to lack of hardware) > > Note that in this new setup, all the probe deferral and retries between > the SCMI core stack and the transports has been removed, since no > more needed. > > Moreover the new drivers have been tested also with a fully > modularized SCMI stack, i.e.: > > scmi-core.ko + scmi-module.ko + scmi_transport_*.ko [ + vendor > modules ] > > ToBeDone: > - completely remove any dependency at build time at the Kconfig level > between > the SCMI core and the transport drivers: so that the transports will be > dependent only on the related subsystems (optee/virtio/mailbox/smc) > (easy to be done but maybe it is not worth...) > - integrate per-platform transport configuration capabilities > (max_rx_timeout_ms & friends..) > > Based on sudeep/for-next/scmi/updates. > > Any feedback, and especially testing (:D) is welcome. > For the v2 patchset: Tested-by: Peng Fan <peng.fan@nxp.com> #i.MX95-19x19-EVK Regards, Peng. > Thanks, > Cristian > > --- > v1 --> v2 > - fixed setup_shmem_iomap to address also SMC needs (QC/nikunj) > (silencing also warnings by kernel test robot <lkp@intel.com>) > - using __free OF cleanup.h magic in setup_shmme_iomap > - properly handle platform_driver_register() failures (Dan) > - fixed a few commit message style > - added a few missing static in scmi_desc > (addresses warnings by kernel test robot <lkp@intel.com>) > > Cristian Marussi (7): > firmware: arm_scmi: Introduce packet handling helpers > firmware: arm_scmi: Add support for standalone transport drivers > firmware: arm_scmi: Make MBOX transport a standalone driver > firmware: arm_scmi: Make SMC transport a standalone driver > firmware: arm_scmi: Make OPTEE transport a standalone driver > firmware: arm_scmi: Make VirtIO transport a standalone driver > firmware: arm_scmi: Remove legacy transport-layer code > > Peng Fan (1): > firmware: arm_scmi: Introduce setup_shmem_iomap > > drivers/firmware/arm_scmi/Kconfig | 20 +- > drivers/firmware/arm_scmi/Makefile | 9 +- > drivers/firmware/arm_scmi/common.h | 184 +++++++++++++---- > - > drivers/firmware/arm_scmi/driver.c | 140 +++++-------- > drivers/firmware/arm_scmi/msg.c | 34 +++- > .../{mailbox.c => scmi_transport_mailbox.c} | 69 ++++--- > .../{optee.c => scmi_transport_optee.c} | 124 +++++------- > .../arm_scmi/{smc.c => scmi_transport_smc.c} | 58 +++--- > .../{virtio.c => scmi_transport_virtio.c} | 103 +++++----- > drivers/firmware/arm_scmi/shmem.c | 85 ++++++-- > 10 files changed, 468 insertions(+), 358 deletions(-) rename > drivers/firmware/arm_scmi/{mailbox.c => scmi_transport_mailbox.c} > (87%) rename drivers/firmware/arm_scmi/{optee.c => > scmi_transport_optee.c} (89%) rename > drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (87%) > rename drivers/firmware/arm_scmi/{virtio.c => scmi_transport_virtio.c} > (94%) > > -- > 2.45.2 >
On Thu, Jul 11, 2024 at 01:26:16PM +0000, Peng Fan wrote: > > Subject: [PATCH 0/8] Make SCMI transport as standalone drivers > > You may need use V2 here :) ...oh damn :P > > > > Hi all, > > > > Till now the SCMI transport layer was being built embedded into in the > > core SCMI stack. > > > > Some of these transports, despite being currently part of the main > > SCMI module, are indeed also registered with different subsystems like > > optee or virtio, and actively probed also by those: this led to a few > > awkward and convoluted tricks to properly handle such interactions at > > boot time in the SCMI stack. > > > > Moreover some partner expressed the desire to be able to fully > > modularize the transports components. > > > > This series aim to make all such transports as standalone drivers that > > can be optionally loaded as modules. > > > > In order to do this, at first some new mechanism is introduced to > > support this new capability while maintaining, in parallel, the old > > legacy embedded transports; then each transport, one by one, is > > transitioned to be a standalone driver and finally the old legacy > > support for embedded transport is removed. > > > > Patch [1/8] is a mostly unrelated (but much needed) clean-up from > > Peng, which I included in this series to avoid conflicts at merge. > > > > Patch [2/8] simply collects the existing datagram manipulation helpers > > in a pair of function pointers structures, in preparation for later reworks. > > > > Patch [3/8] adds the bulk of the new logic to the core SCMI stack and > > then each existing transport is transitioned to be a standalone driver in > > patches 4,5,6,7 while shuffling around the compatibles. (no DT change > > is needed of curse for backward compatibility) While doing this I kept > > the module authorship in line with the main > > author(S) as spitted out by git-blame. > > > > Finally patch [8/8] removes all the legacy dead code from the core > > SCMI stack. > > > > No new symbol EXPORT has been added. > > > > The new transport drivers have been tested, as built-in and LKM, as > > follows: > > > > - mailbox on JUNO > > - virtio on emulation > > - optee on QEMU/optee using Linaro setup > > > > Exercised using the regular SCMI drivers stack and the SCMI ACS suite, > > testing commands, replies, delayed responses and notification. > > > > Multiple virtual SCMI instances support has been tested too. > > > > SMC has NOT been tested/exercised at run-time, only compile-tested. > > (due to lack of hardware) > > > > Note that in this new setup, all the probe deferral and retries between > > the SCMI core stack and the transports has been removed, since no > > more needed. > > > > Moreover the new drivers have been tested also with a fully > > modularized SCMI stack, i.e.: > > > > scmi-core.ko + scmi-module.ko + scmi_transport_*.ko [ + vendor > > modules ] > > > > ToBeDone: > > - completely remove any dependency at build time at the Kconfig level > > between > > the SCMI core and the transport drivers: so that the transports will be > > dependent only on the related subsystems (optee/virtio/mailbox/smc) > > (easy to be done but maybe it is not worth...) > > - integrate per-platform transport configuration capabilities > > (max_rx_timeout_ms & friends..) > > > > Based on sudeep/for-next/scmi/updates. > > > > Any feedback, and especially testing (:D) is welcome. > > > > For the v2 patchset: > Tested-by: Peng Fan <peng.fan@nxp.com> #i.MX95-19x19-EVK > Thanks a lot for the review and testing, Cristian
On 7/10/24 10:31, Cristian Marussi wrote: > Hi all, > > Till now the SCMI transport layer was being built embedded into in the core > SCMI stack. > > Some of these transports, despite being currently part of the main SCMI > module, are indeed also registered with different subsystems like optee or > virtio, and actively probed also by those: this led to a few awkward and > convoluted tricks to properly handle such interactions at boot time in the > SCMI stack. > > Moreover some partner expressed the desire to be able to fully modularize > the transports components. > > This series aim to make all such transports as standalone drivers that can > be optionally loaded as modules. > > In order to do this, at first some new mechanism is introduced to support > this new capability while maintaining, in parallel, the old legacy embedded > transports; then each transport, one by one, is transitioned to be a > standalone driver and finally the old legacy support for embedded transport > is removed. > > Patch [1/8] is a mostly unrelated (but much needed) clean-up from Peng, > which I included in this series to avoid conflicts at merge. > > Patch [2/8] simply collects the existing datagram manipulation helpers in a > pair of function pointers structures, in preparation for later reworks. > > Patch [3/8] adds the bulk of the new logic to the core SCMI stack and then > each existing transport is transitioned to be a standalone driver in > patches 4,5,6,7 while shuffling around the compatibles. (no DT change is > needed of curse for backward compatibility) > While doing this I kept the module authorship in line with the main > author(S) as spitted out by git-blame. > > Finally patch [8/8] removes all the legacy dead code from the core SCMI > stack. > > No new symbol EXPORT has been added. > > The new transport drivers have been tested, as built-in and LKM, as > follows: > > - mailbox on JUNO > - virtio on emulation > - optee on QEMU/optee using Linaro setup > > Exercised using the regular SCMI drivers stack and the SCMI ACS suite, > testing commands, replies, delayed responses and notification. > > Multiple virtual SCMI instances support has been tested too. > > SMC has NOT been tested/exercised at run-time, only compile-tested. > (due to lack of hardware) > > Note that in this new setup, all the probe deferral and retries between the > SCMI core stack and the transports has been removed, since no more needed. > > Moreover the new drivers have been tested also with a fully modularized > SCMI stack, i.e.: > > scmi-core.ko + scmi-module.ko + scmi_transport_*.ko [ + vendor modules ] > > ToBeDone: > - completely remove any dependency at build time at the Kconfig level between > the SCMI core and the transport drivers: so that the transports will be > dependent only on the related subsystems (optee/virtio/mailbox/smc) > (easy to be done but maybe it is not worth...) > - integrate per-platform transport configuration capabilities > (max_rx_timeout_ms & friends..) > > Based on sudeep/for-next/scmi/updates. > > Any feedback, and especially testing (:D) is welcome. Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>
Hi Cristian, Peng, On Thursday, July 11, 2024, Cristian Marussi worte: > On Thu, Jul 11, 2024 at 01:26:16PM +0000, Peng Fan wrote: > > > Subject: [PATCH 0/8] Make SCMI transport as standalone drivers > > > > You may need use V2 here :) > > ...oh damn :P > > > > > > > Hi all, > > > > > > Till now the SCMI transport layer was being built embedded into in the > > > core SCMI stack. > > > > > > Some of these transports, despite being currently part of the main > > > SCMI module, are indeed also registered with different subsystems like > > > optee or virtio, and actively probed also by those: this led to a few > > > awkward and convoluted tricks to properly handle such interactions at > > > boot time in the SCMI stack. > > > > > > Moreover some partner expressed the desire to be able to fully > > > modularize the transports components. > > > > > > This series aim to make all such transports as standalone drivers that > > > can be optionally loaded as modules. > > > > > > In order to do this, at first some new mechanism is introduced to > > > support this new capability while maintaining, in parallel, the old > > > legacy embedded transports; then each transport, one by one, is > > > transitioned to be a standalone driver and finally the old legacy > > > support for embedded transport is removed. > > > > > > Patch [1/8] is a mostly unrelated (but much needed) clean-up from > > > Peng, which I included in this series to avoid conflicts at merge. > > > > > > Patch [2/8] simply collects the existing datagram manipulation helpers > > > in a pair of function pointers structures, in preparation for later reworks. > > > > > > Patch [3/8] adds the bulk of the new logic to the core SCMI stack and > > > then each existing transport is transitioned to be a standalone driver in > > > patches 4,5,6,7 while shuffling around the compatibles. (no DT change > > > is needed of curse for backward compatibility) While doing this I kept > > > the module authorship in line with the main > > > author(S) as spitted out by git-blame. > > > > > > Finally patch [8/8] removes all the legacy dead code from the core > > > SCMI stack. > > > > > > No new symbol EXPORT has been added. > > > > > > The new transport drivers have been tested, as built-in and LKM, as > > > follows: > > > > > > - mailbox on JUNO > > > - virtio on emulation > > > - optee on QEMU/optee using Linaro setup > > > > > > Exercised using the regular SCMI drivers stack and the SCMI ACS suite, > > > testing commands, replies, delayed responses and notification. > > > > > > Multiple virtual SCMI instances support has been tested too. > > > > > > SMC has NOT been tested/exercised at run-time, only compile-tested. > > > (due to lack of hardware) > > > > > > Note that in this new setup, all the probe deferral and retries between > > > the SCMI core stack and the transports has been removed, since no > > > more needed. > > > > > > Moreover the new drivers have been tested also with a fully > > > modularized SCMI stack, i.e.: > > > > > > scmi-core.ko + scmi-module.ko + scmi_transport_*.ko [ + vendor > > > modules ] > > > > > > ToBeDone: > > > - completely remove any dependency at build time at the Kconfig level > > > between > > > the SCMI core and the transport drivers: so that the transports will be > > > dependent only on the related subsystems (optee/virtio/mailbox/smc) > > > (easy to be done but maybe it is not worth...) > > > - integrate per-platform transport configuration capabilities > > > (max_rx_timeout_ms & friends..) > > > > > > Based on sudeep/for-next/scmi/updates. > > > > > > Any feedback, and especially testing (:D) is welcome. > > > > > > > For the v2 patchset: > > Tested-by: Peng Fan <peng.fan@nxp.com> #i.MX95-19x19-EVK > > > > Thanks a lot for the review and testing, > > Cristian I've tested this v2 on stm32mp157c-scmi.dts. Using built-in modules works perfectly. I've tweaked my platform setup to test the .ko and modprobe part. It works ok for the probe part but I faced kernel oops when unloading scmi-module after transport is loaded, used, then unoaded. The issue I saw is around calls to info->desc->ops->chan_free in scmi_cleanup_channels(). I wonder if there are some ops that were not unregistered when transport driver is unloaded. Aside this, I'll sent few minor comments on few patches of your series. Best regards, Etienne
On Tue, Jul 23, 2024 at 01:36:55PM +0000, Etienne CARRIERE wrote: > Hi Cristian, Peng, > Hi Etienne, Thanks for giving this a go on your setup. > On Thursday, July 11, 2024, Cristian Marussi worte: > > On Thu, Jul 11, 2024 at 01:26:16PM +0000, Peng Fan wrote: > > > > Subject: [PATCH 0/8] Make SCMI transport as standalone drivers > > > > > > You may need use V2 here :) > > > > ...oh damn :P > > > > > > > > > > Hi all, > > > > > > > > Till now the SCMI transport layer was being built embedded into in the > > > > core SCMI stack. > > > > > > > > Some of these transports, despite being currently part of the main > > > > SCMI module, are indeed also registered with different subsystems like > > > > optee or virtio, and actively probed also by those: this led to a few > > > > awkward and convoluted tricks to properly handle such interactions at > > > > boot time in the SCMI stack. > > > > > > > > Moreover some partner expressed the desire to be able to fully > > > > modularize the transports components. > > > > > > > > This series aim to make all such transports as standalone drivers that > > > > can be optionally loaded as modules. > > > > > > > > In order to do this, at first some new mechanism is introduced to > > > > support this new capability while maintaining, in parallel, the old > > > > legacy embedded transports; then each transport, one by one, is > > > > transitioned to be a standalone driver and finally the old legacy > > > > support for embedded transport is removed. > > > > > > > > Patch [1/8] is a mostly unrelated (but much needed) clean-up from > > > > Peng, which I included in this series to avoid conflicts at merge. > > > > > > > > Patch [2/8] simply collects the existing datagram manipulation helpers > > > > in a pair of function pointers structures, in preparation for later reworks. > > > > > > > > Patch [3/8] adds the bulk of the new logic to the core SCMI stack and > > > > then each existing transport is transitioned to be a standalone driver in > > > > patches 4,5,6,7 while shuffling around the compatibles. (no DT change > > > > is needed of curse for backward compatibility) While doing this I kept > > > > the module authorship in line with the main > > > > author(S) as spitted out by git-blame. > > > > > > > > Finally patch [8/8] removes all the legacy dead code from the core > > > > SCMI stack. > > > > > > > > No new symbol EXPORT has been added. > > > > > > > > The new transport drivers have been tested, as built-in and LKM, as > > > > follows: > > > > > > > > - mailbox on JUNO > > > > - virtio on emulation > > > > - optee on QEMU/optee using Linaro setup > > > > > > > > Exercised using the regular SCMI drivers stack and the SCMI ACS suite, > > > > testing commands, replies, delayed responses and notification. > > > > > > > > Multiple virtual SCMI instances support has been tested too. > > > > > > > > SMC has NOT been tested/exercised at run-time, only compile-tested. > > > > (due to lack of hardware) > > > > > > > > Note that in this new setup, all the probe deferral and retries between > > > > the SCMI core stack and the transports has been removed, since no > > > > more needed. > > > > > > > > Moreover the new drivers have been tested also with a fully > > > > modularized SCMI stack, i.e.: > > > > > > > > scmi-core.ko + scmi-module.ko + scmi_transport_*.ko [ + vendor > > > > modules ] > > > > > > > > ToBeDone: > > > > - completely remove any dependency at build time at the Kconfig level > > > > between > > > > the SCMI core and the transport drivers: so that the transports will be > > > > dependent only on the related subsystems (optee/virtio/mailbox/smc) > > > > (easy to be done but maybe it is not worth...) > > > > - integrate per-platform transport configuration capabilities > > > > (max_rx_timeout_ms & friends..) > > > > > > > > Based on sudeep/for-next/scmi/updates. > > > > > > > > Any feedback, and especially testing (:D) is welcome. > > > > > > > > > > For the v2 patchset: > > > Tested-by: Peng Fan <peng.fan@nxp.com> #i.MX95-19x19-EVK > > > > > > > Thanks a lot for the review and testing, > > > > Cristian > > > I've tested this v2 on stm32mp157c-scmi.dts. Using built-in modules > works perfectly. I've tweaked my platform setup to test the .ko and > modprobe part. It works ok for the probe part but I faced kernel oops > when unloading scmi-module after transport is loaded, used, then unoaded. > The issue I saw is around calls to info->desc->ops->chan_free in > scmi_cleanup_channels(). I wonder if there are some ops that were not > unregistered when transport driver is unloaded. > You are right, I could reproduce your oops in my QEMU/optee setup. There was a bug in chan_free for optee that pre-dated this series....it is exposed when unloading the scmi-module....I'll post a fix for this as the initial patch of this series V3. Moreover even once that was fixed, there was another bug in the optee_remove of this new transport driver since I was calling the platform_driver_unregister() too late (after the check for channel empty)...as a result when you unload the scmi_transport_optee BEFORE the scmi-module (which is another valid unload sequence option) the core SCMI stack was NOT unbound like for the other transports. Last but not least, I spotted another issue for all of these transport drivers (and related WARN) when finally unloading the scmi-core module (the last one to go) due to a missing device_release...this was easily fixed just by using other platform drivers core helpers...so I refactored more the DEFINE_SCMI_TRANSPORT_DRIVER macros internals... Next week, on top of -rc1, I'll post a v3 with all the fixes I mentioned. Thanks, Cristian
On Fri, Jul 12, 2024 at 02:02:32PM -0700, Florian Fainelli wrote: > On 7/10/24 10:31, Cristian Marussi wrote: > > Hi all, > > > > Till now the SCMI transport layer was being built embedded into in the core > > SCMI stack. > > > > Some of these transports, despite being currently part of the main SCMI > > module, are indeed also registered with different subsystems like optee or > > virtio, and actively probed also by those: this led to a few awkward and > > convoluted tricks to properly handle such interactions at boot time in the > > SCMI stack. > > > > Moreover some partner expressed the desire to be able to fully modularize > > the transports components. > > > > This series aim to make all such transports as standalone drivers that can > > be optionally loaded as modules. > > > > In order to do this, at first some new mechanism is introduced to support > > this new capability while maintaining, in parallel, the old legacy embedded > > transports; then each transport, one by one, is transitioned to be a > > standalone driver and finally the old legacy support for embedded transport > > is removed. > > > > Patch [1/8] is a mostly unrelated (but much needed) clean-up from Peng, > > which I included in this series to avoid conflicts at merge. > > > > Patch [2/8] simply collects the existing datagram manipulation helpers in a > > pair of function pointers structures, in preparation for later reworks. > > > > Patch [3/8] adds the bulk of the new logic to the core SCMI stack and then > > each existing transport is transitioned to be a standalone driver in > > patches 4,5,6,7 while shuffling around the compatibles. (no DT change is > > needed of curse for backward compatibility) > > While doing this I kept the module authorship in line with the main > > author(S) as spitted out by git-blame. > > > > Finally patch [8/8] removes all the legacy dead code from the core SCMI > > stack. > > > > No new symbol EXPORT has been added. > > > > The new transport drivers have been tested, as built-in and LKM, as > > follows: > > > > - mailbox on JUNO > > - virtio on emulation > > - optee on QEMU/optee using Linaro setup > > > > Exercised using the regular SCMI drivers stack and the SCMI ACS suite, > > testing commands, replies, delayed responses and notification. > > > > Multiple virtual SCMI instances support has been tested too. > > > > SMC has NOT been tested/exercised at run-time, only compile-tested. > > (due to lack of hardware) > > > > Note that in this new setup, all the probe deferral and retries between the > > SCMI core stack and the transports has been removed, since no more needed. > > > > Moreover the new drivers have been tested also with a fully modularized > > SCMI stack, i.e.: > > > > scmi-core.ko + scmi-module.ko + scmi_transport_*.ko [ + vendor modules ] > > > > ToBeDone: > > - completely remove any dependency at build time at the Kconfig level between > > the SCMI core and the transport drivers: so that the transports will be > > dependent only on the related subsystems (optee/virtio/mailbox/smc) > > (easy to be done but maybe it is not worth...) > > - integrate per-platform transport configuration capabilities > > (max_rx_timeout_ms & friends..) > > > > Based on sudeep/for-next/scmi/updates. > > > > Any feedback, and especially testing (:D) is welcome. > > Tested-by: Florian Fainelli <florian.fainelli@broadcom.com> Thanks for testing this, Florian. Cristian