On 2020-10-30 01:02, John Stultz wrote:
On Wed, Oct 28, 2020 at 7:51 AM Robin Murphy <robin.murphy@xxxxxxx> wrote:
Hmm, perhaps I'm missing something here, but even if the config options
*do* line up, what prevents arm-smmu probing before qcom-scm and
dereferencing NULL in qcom_scm_qsmmu500_wait_safe_toggle() before __scm
is initialised?
Oh man, this spun me on a "wait, but how does it all work!" trip. :)
So in the non-module case, the qcom_scm driver is a subsys_initcall
and the arm-smmu is a module_platform_driver, so the ordering works
out.
In the module case, the arm-smmu code isn't loaded until the qcom_scm
driver finishes probing due to the symbol dependency handling.
My point is that module load != driver probe. AFAICS you could disable
drivers_autoprobe, load both modules, bind the SMMU to its driver first,
and boom!
To double check this, I added a big msleep at the top of the
qcom_scm_probe to try to open the race window you described, but the
arm_smmu_device_probe() doesn't run until after qcom_scm_probe
completes.
I don't think asynchronous probing is enabled by default, so indeed I
would expect that to still happen to work ;)
So at least as a built in / built in, or a module/module case its ok.
And in the case where arm-smmu is a module and qcom_scm is built in
that's ok too.
In the built-in case, I'm sure it happens to work out similarly because
the order of nodes in the DTB tends to be the order in which devices are
autoprobed. Again, async probe might be enough to break things; manually
binding drivers definitely should; moving the firmware node to the end
of the DTB probably would as well.
Its just the case my patch is trying to prevent is where arm-smmu is
built in, but qcom_scm is a module that it can't work (due to build
errors in missing symbols, or if we tried to use function pointers to
plug in the qcom_scm - the lack of initialization ordering).
Hopefully that addresses your concern? Let me know if I'm still
missing something.
What I was dancing around is that the SCM API (and/or its users) appears
to need a general way to tell whether SCM is ready or not, because the
initialisation ordering problem is there anyway. Neither Kconfig nor the
module loader can solve that.
One possible self-contained workaround would be to see if an SCM DT node
exists, see if a corresponding device exists, and see if that device has
a driver bound. It's a little ugly, and strictly it still doesn't tell
you that the _right_ driver is bound, but at least it's a lot more
robust than implicitly relying on DT order, default probing behaviours,
and hope.
Robin.
