[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <74c19e48-481a-7be9-e287-ca1f7a659c21@quicinc.com>
Date: Wed, 22 Feb 2023 17:55:17 -0800
From: Elliot Berman <quic_eberman@...cinc.com>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Alex Elder <elder@...aro.org>,
Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>
CC: Murali Nalajala <quic_mnalajal@...cinc.com>,
Trilok Soni <quic_tsoni@...cinc.com>,
Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>,
Carl van Schaik <quic_cvanscha@...cinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
"Konrad Dybcio" <konrad.dybcio@...aro.org>,
Arnd Bergmann <arnd@...db.de>,
"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Jonathan Corbet <corbet@....net>,
Bagas Sanjaya <bagasdotme@...il.com>,
Catalin Marinas <catalin.marinas@....com>,
Jassi Brar <jassisinghbrar@...il.com>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v10 15/26] gunyah: rsc_mgr: Add platform ops on
mem_lend/mem_reclaim
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
Powered by blists - more mailing lists