On 2/22/2023 2:21 AM, Srinivas Kandagatla wrote:
On 21/02/2023 21:22, Elliot Berman wrote:
On 2/21/2023 6:51 AM, Srinivas Kandagatla wrote:
On 14/02/2023 21:24, Elliot Berman wrote:
[snip]
+
+static struct gunyah_rm_platform_ops *rm_platform_ops;
+static DECLARE_RWSEM(rm_platform_ops_lock);
Why do we need this read/write lock or this global rm_platform_ops
here, AFAIU, there will be only one instance of platform_ops per
platform.
This should be a core part of the gunyah and its driver early setup,
that should give us pretty much lock less behaviour.
We should be able to determine by Hypervisor UUID that its on
Qualcomm platform or not, during early gunyah setup which should help
us setup the platfrom ops accordingly.
This should also help cleanup some of the gunyah code that was added
futher down in this patchset.
I'm guessing the direction to take is:
config GUNYAH
select QCOM_SCM if ARCH_QCOM
This is how other kernel drivers use SCM.
and have vm_mgr call directly into qcom_scm driver if the UID matches?
Yes that is the plan, we could have these callbacks as part key data
structure like struct gh_rm and update it at very early in setup stage
based on UUID match.
We have an Android requirement to enable CONFIG_GUNYAH=y and
CONFIG_QCOM_SCM=m, but it wouldn't be possible with this design. The
Am not sure how this will work, if gunyah for QCOM Platform is depended
on SCM then there is no way that gunyah could be a inbuilt and make scm
a module. >
On the other hand with the existing design gunyah will not be functional
until scm driver is loaded and platform hooks are registered. This
runtime dependency design does not express the dependency correctly and
the only way to know if gunyah is functional is keep trying which can
only work after scm driver is probed.
This also raises the design question on how much of platform hooks
dependency is captured at gunyah core and api level, with state of
current code /dev/gunyah will be created even without platform hooks and
let the userspace use it which then only fail at hyp call level.
Other issue with current design is, scm module can be unloaded under the
hood leaving gunyah with NULL pointers to those platform hook functions.
This is not possible because SCM module can't be unloaded (except with
CONFIG_MODULE_FORCE_UNLOAD). I can also increase refcount of qcom_scm.ko
module to be more correct.
This is the kind of issues we could see if the dependency is not
expressed from bottom up. >
The current design is not really capturing the depended components
accurately.
Considering platform hooks as a core resource to gunyah on Qualcomm
platform is something that needs attention. If we can fix that then it
might be doable to have QCOM_SCM=m and CONFIG_GUNYAH=y.
I'm open to ideas. I don't see this as being a real-world issue because
default defconfig has QCOM_SCM=y and all Qualcomm platforms enable
QCOM_SCM at least as =m.
Thanks,
Elliot
--srini
platform hooks implementation allows GUNYAH and QCOM_SCM to be enabled
without setting lower bound of the other.
- Elliot
