diff mbox series

[v1] arm64: dts: qcom: sc7280: Make ADSP a secure fastrpc domain

Message ID 20241113050042.181028-1-quic_ekangupt@quicinc.com (mailing list archive)
State New
Headers show
Series [v1] arm64: dts: qcom: sc7280: Make ADSP a secure fastrpc domain | expand

Commit Message

Ekansh Gupta Nov. 13, 2024, 5 a.m. UTC
FastRPC framework treats ADSP as a secure domain on sc7280 SoC
which means that only secure fastrpc device node should be
created for ADSP remoteproc. Remove the non-secure-domain
property from ADSP fastrpc node.

Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 -
 1 file changed, 1 deletion(-)

Comments

Dmitry Baryshkov Nov. 13, 2024, 5:43 a.m. UTC | #1
On Wed, Nov 13, 2024 at 10:30:42AM +0530, Ekansh Gupta wrote:
> FastRPC framework treats ADSP as a secure domain on sc7280 SoC
> which means that only secure fastrpc device node should be
> created for ADSP remoteproc. Remove the non-secure-domain
> property from ADSP fastrpc node.

If this prevents the non-secure devices from being created, isn't that a
regression from the userspace point of view?

> 
> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 3d8410683402..c633926c0f33 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3852,7 +3852,6 @@ fastrpc {
>  					compatible = "qcom,fastrpc";
>  					qcom,glink-channels = "fastrpcglink-apps-dsp";
>  					label = "adsp";
> -					qcom,non-secure-domain;
>  					#address-cells = <1>;
>  					#size-cells = <0>;
>  
> -- 
> 2.34.1
>
Ekansh Gupta Nov. 13, 2024, 6:17 a.m. UTC | #2
On 11/13/2024 11:13 AM, Dmitry Baryshkov wrote:
> On Wed, Nov 13, 2024 at 10:30:42AM +0530, Ekansh Gupta wrote:
>> FastRPC framework treats ADSP as a secure domain on sc7280 SoC
>> which means that only secure fastrpc device node should be
>> created for ADSP remoteproc. Remove the non-secure-domain
>> property from ADSP fastrpc node.
> If this prevents the non-secure devices from being created, isn't that a
> regression from the userspace point of view?
The actual intention of having secure and non-secure domains is to utilize signed(high privilege)
and unsigned(low privilege) DSP processes properly.

Non-secure device node is intended to be used by untrusted/generic applications which needs to
offload tasks to DSP as unsignedPD. Only unsigned PD is expected to be allowed if the process is
using non-secure node.

Secure device is intended to be used by trusted processes like daemons or any application
which needs to offload as signed PD to DSP.

The ideal expectation from userspace is to first try to open secure device node and fall back to
non-secure node if the secure node is not accessible or absent.

I understand your concerns, can you please suggest how this can be improved/corrected?

--ekansh
>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 3d8410683402..c633926c0f33 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -3852,7 +3852,6 @@ fastrpc {
>>  					compatible = "qcom,fastrpc";
>>  					qcom,glink-channels = "fastrpcglink-apps-dsp";
>>  					label = "adsp";
>> -					qcom,non-secure-domain;
>>  					#address-cells = <1>;
>>  					#size-cells = <0>;
>>  
>> -- 
>> 2.34.1
>>
Dmitry Baryshkov Nov. 13, 2024, 11:50 a.m. UTC | #3
On Wed, 13 Nov 2024 at 08:18, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
>
>
>
> On 11/13/2024 11:13 AM, Dmitry Baryshkov wrote:
> > On Wed, Nov 13, 2024 at 10:30:42AM +0530, Ekansh Gupta wrote:
> >> FastRPC framework treats ADSP as a secure domain on sc7280 SoC
> >> which means that only secure fastrpc device node should be
> >> created for ADSP remoteproc. Remove the non-secure-domain
> >> property from ADSP fastrpc node.
> > If this prevents the non-secure devices from being created, isn't that a
> > regression from the userspace point of view?
> The actual intention of having secure and non-secure domains is to utilize signed(high privilege)
> and unsigned(low privilege) DSP processes properly.
>
> Non-secure device node is intended to be used by untrusted/generic applications which needs to
> offload tasks to DSP as unsignedPD. Only unsigned PD is expected to be allowed if the process is
> using non-secure node.
>
> Secure device is intended to be used by trusted processes like daemons or any application
> which needs to offload as signed PD to DSP.
>
> The ideal expectation from userspace is to first try to open secure device node and fall back to
> non-secure node if the secure node is not accessible or absent.
>
> I understand your concerns, can you please suggest how this can be improved/corrected?

Thank you for the explanation, and thanks for the description of the
expected behaviour, but the question is different.
Currently (with the property being present in DT) the driver creates a
non-secure fastrpc device for the ADSP.
Can it actually be used? Note: no mentioning of a particular userspace
implementation or the (un)expected usage.
If it could not and an attempt to use it resulted in some kind of an
error, then the patch is a fix and it should be decribed accordingly.
If it could be used and now you are removing this possibility, then it
is a regression. Again, this must be clearly documented, but generally
this is not allowed.

>
> --ekansh
> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> >> ---
> >>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> index 3d8410683402..c633926c0f33 100644
> >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> @@ -3852,7 +3852,6 @@ fastrpc {
> >>                                      compatible = "qcom,fastrpc";
> >>                                      qcom,glink-channels = "fastrpcglink-apps-dsp";
> >>                                      label = "adsp";
> >> -                                    qcom,non-secure-domain;
> >>                                      #address-cells = <1>;
> >>                                      #size-cells = <0>;
> >>
> >> --
> >> 2.34.1
> >>
>
Ekansh Gupta Nov. 14, 2024, 5:19 a.m. UTC | #4
On 11/13/2024 5:20 PM, Dmitry Baryshkov wrote:
> On Wed, 13 Nov 2024 at 08:18, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
>>
>>
>> On 11/13/2024 11:13 AM, Dmitry Baryshkov wrote:
>>> On Wed, Nov 13, 2024 at 10:30:42AM +0530, Ekansh Gupta wrote:
>>>> FastRPC framework treats ADSP as a secure domain on sc7280 SoC
>>>> which means that only secure fastrpc device node should be
>>>> created for ADSP remoteproc. Remove the non-secure-domain
>>>> property from ADSP fastrpc node.
>>> If this prevents the non-secure devices from being created, isn't that a
>>> regression from the userspace point of view?
>> The actual intention of having secure and non-secure domains is to utilize signed(high privilege)
>> and unsigned(low privilege) DSP processes properly.
>>
>> Non-secure device node is intended to be used by untrusted/generic applications which needs to
>> offload tasks to DSP as unsignedPD. Only unsigned PD is expected to be allowed if the process is
>> using non-secure node.
>>
>> Secure device is intended to be used by trusted processes like daemons or any application
>> which needs to offload as signed PD to DSP.
>>
>> The ideal expectation from userspace is to first try to open secure device node and fall back to
>> non-secure node if the secure node is not accessible or absent.
>>
>> I understand your concerns, can you please suggest how this can be improved/corrected?
> Thank you for the explanation, and thanks for the description of the
> expected behaviour, but the question is different.
> Currently (with the property being present in DT) the driver creates a
> non-secure fastrpc device for the ADSP.
> Can it actually be used? Note: no mentioning of a particular userspace
> implementation or the (un)expected usage.
> If it could not and an attempt to use it resulted in some kind of an
> error, then the patch is a fix and it should be decribed accordingly.
> If it could be used and now you are removing this possibility, then it
> is a regression. Again, this must be clearly documented, but generally
> this is not allowed.
Thanks for the clarification, Dmitry.

As of today, if the property is present in DT, non-secure fastrpc device will be created
for ADSP and as there are no checks to restrict daemons to use only secure node, there
will not be any failures observed. So there is no error if non-secure property is added
for ADSP and your 2nd point holds here.

Problems with the current design are(you can look into below points independent of the change):

1. This creates a security concern as any process that can open non-secure device
can replicate daemon to attach to DSP root PD and cause troubles there which is not
a good thing. So basically any trusted process(maybe same group) should only use secure
device node and any process using non-secure node should only offload to unsigned PD.

2. Having this property well defined also help in scaling fastrpc driver for new domains(like CDSP1
was recently introduced) as driver can only rely on the "label" and "non-secure-domain" property
for device creation. Say, only secure device is create if property is not defined and both device nodes
are created if non-secure-domain is define. This way, the dependency on domain_id can be removed
from fastrpc_rpmsg_probe[1] and create either only fastrpc-xdsp-secure or both(secure and non-secure).

This however is a regression as you have mentioned, but it it helps address multiple problems.

Should I discuss further on documentation or is any more design clarification should be done here?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/misc/fastrpc.c#n2327

--ekansh
>> --ekansh
>>>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>>> index 3d8410683402..c633926c0f33 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>>> @@ -3852,7 +3852,6 @@ fastrpc {
>>>>                                      compatible = "qcom,fastrpc";
>>>>                                      qcom,glink-channels = "fastrpcglink-apps-dsp";
>>>>                                      label = "adsp";
>>>> -                                    qcom,non-secure-domain;
>>>>                                      #address-cells = <1>;
>>>>                                      #size-cells = <0>;
>>>>
>>>> --
>>>> 2.34.1
>>>>
>
Dmitry Baryshkov Nov. 14, 2024, noon UTC | #5
On Thu, Nov 14, 2024 at 10:49:52AM +0530, Ekansh Gupta wrote:
> 
> 
> On 11/13/2024 5:20 PM, Dmitry Baryshkov wrote:
> > On Wed, 13 Nov 2024 at 08:18, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
> >>
> >>
> >> On 11/13/2024 11:13 AM, Dmitry Baryshkov wrote:
> >>> On Wed, Nov 13, 2024 at 10:30:42AM +0530, Ekansh Gupta wrote:
> >>>> FastRPC framework treats ADSP as a secure domain on sc7280 SoC
> >>>> which means that only secure fastrpc device node should be
> >>>> created for ADSP remoteproc. Remove the non-secure-domain
> >>>> property from ADSP fastrpc node.
> >>> If this prevents the non-secure devices from being created, isn't that a
> >>> regression from the userspace point of view?
> >> The actual intention of having secure and non-secure domains is to utilize signed(high privilege)
> >> and unsigned(low privilege) DSP processes properly.
> >>
> >> Non-secure device node is intended to be used by untrusted/generic applications which needs to
> >> offload tasks to DSP as unsignedPD. Only unsigned PD is expected to be allowed if the process is
> >> using non-secure node.
> >>
> >> Secure device is intended to be used by trusted processes like daemons or any application
> >> which needs to offload as signed PD to DSP.
> >>
> >> The ideal expectation from userspace is to first try to open secure device node and fall back to
> >> non-secure node if the secure node is not accessible or absent.
> >>
> >> I understand your concerns, can you please suggest how this can be improved/corrected?
> > Thank you for the explanation, and thanks for the description of the
> > expected behaviour, but the question is different.
> > Currently (with the property being present in DT) the driver creates a
> > non-secure fastrpc device for the ADSP.
> > Can it actually be used? Note: no mentioning of a particular userspace
> > implementation or the (un)expected usage.
> > If it could not and an attempt to use it resulted in some kind of an
> > error, then the patch is a fix and it should be decribed accordingly.
> > If it could be used and now you are removing this possibility, then it
> > is a regression. Again, this must be clearly documented, but generally
> > this is not allowed.
> Thanks for the clarification, Dmitry.
> 
> As of today, if the property is present in DT, non-secure fastrpc device will be created
> for ADSP and as there are no checks to restrict daemons to use only secure node, there
> will not be any failures observed. So there is no error if non-secure property is added
> for ADSP and your 2nd point holds here.
> 
> Problems with the current design are(you can look into below points independent of the change):
> 
> 1. This creates a security concern as any process that can open non-secure device
> can replicate daemon to attach to DSP root PD and cause troubles there which is not
> a good thing. So basically any trusted process(maybe same group) should only use secure
> device node and any process using non-secure node should only offload to unsigned PD.

Again, you are describing expected behaviour. Other userspace clients
can deviate from this.

> 
> 2. Having this property well defined also help in scaling fastrpc driver for new domains(like CDSP1
> was recently introduced) as driver can only rely on the "label" and "non-secure-domain" property
> for device creation. Say, only secure device is create if property is not defined and both device nodes
> are created if non-secure-domain is define. This way, the dependency on domain_id can be removed
> from fastrpc_rpmsg_probe[1] and create either only fastrpc-xdsp-secure or both(secure and non-secure).

Well, I don't think I follow this point. The property is already
well-defined.

> 
> This however is a regression as you have mentioned, but it it helps address multiple problems.
> 
> Should I discuss further on documentation or is any more design clarification should be done here?

At least you must explicitly specify that this causes changes to
userspace, and all the reasons to do that. So that everybody else
doesn't have to read between the lines.

> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/misc/fastrpc.c#n2327
> 
> --ekansh
> >> --ekansh
> >>>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 -
> >>>>  1 file changed, 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >>>> index 3d8410683402..c633926c0f33 100644
> >>>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >>>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >>>> @@ -3852,7 +3852,6 @@ fastrpc {
> >>>>                                      compatible = "qcom,fastrpc";
> >>>>                                      qcom,glink-channels = "fastrpcglink-apps-dsp";
> >>>>                                      label = "adsp";
> >>>> -                                    qcom,non-secure-domain;

- Are there other platforms which have this flag set for ADSP?

- Granted that sc7280 was targeting ChromeOS devices, might it be that
  there is a CrOS-specific userspace for that?

> >>>>                                      #address-cells = <1>;
> >>>>                                      #size-cells = <0>;
> >>>>
> >>>> --
> >>>> 2.34.1
> >>>>
> >
>
Ekansh Gupta Nov. 15, 2024, 12:10 p.m. UTC | #6
On 11/14/2024 5:30 PM, Dmitry Baryshkov wrote:
> On Thu, Nov 14, 2024 at 10:49:52AM +0530, Ekansh Gupta wrote:
>>
>> On 11/13/2024 5:20 PM, Dmitry Baryshkov wrote:
>>> On Wed, 13 Nov 2024 at 08:18, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
>>>>
>>>> On 11/13/2024 11:13 AM, Dmitry Baryshkov wrote:
>>>>> On Wed, Nov 13, 2024 at 10:30:42AM +0530, Ekansh Gupta wrote:
>>>>>> FastRPC framework treats ADSP as a secure domain on sc7280 SoC
>>>>>> which means that only secure fastrpc device node should be
>>>>>> created for ADSP remoteproc. Remove the non-secure-domain
>>>>>> property from ADSP fastrpc node.
>>>>> If this prevents the non-secure devices from being created, isn't that a
>>>>> regression from the userspace point of view?
>>>> The actual intention of having secure and non-secure domains is to utilize signed(high privilege)
>>>> and unsigned(low privilege) DSP processes properly.
>>>>
>>>> Non-secure device node is intended to be used by untrusted/generic applications which needs to
>>>> offload tasks to DSP as unsignedPD. Only unsigned PD is expected to be allowed if the process is
>>>> using non-secure node.
>>>>
>>>> Secure device is intended to be used by trusted processes like daemons or any application
>>>> which needs to offload as signed PD to DSP.
>>>>
>>>> The ideal expectation from userspace is to first try to open secure device node and fall back to
>>>> non-secure node if the secure node is not accessible or absent.
>>>>
>>>> I understand your concerns, can you please suggest how this can be improved/corrected?
>>> Thank you for the explanation, and thanks for the description of the
>>> expected behaviour, but the question is different.
>>> Currently (with the property being present in DT) the driver creates a
>>> non-secure fastrpc device for the ADSP.
>>> Can it actually be used? Note: no mentioning of a particular userspace
>>> implementation or the (un)expected usage.
>>> If it could not and an attempt to use it resulted in some kind of an
>>> error, then the patch is a fix and it should be decribed accordingly.
>>> If it could be used and now you are removing this possibility, then it
>>> is a regression. Again, this must be clearly documented, but generally
>>> this is not allowed.
>> Thanks for the clarification, Dmitry.
>>
>> As of today, if the property is present in DT, non-secure fastrpc device will be created
>> for ADSP and as there are no checks to restrict daemons to use only secure node, there
>> will not be any failures observed. So there is no error if non-secure property is added
>> for ADSP and your 2nd point holds here.
>>
>> Problems with the current design are(you can look into below points independent of the change):
>>
>> 1. This creates a security concern as any process that can open non-secure device
>> can replicate daemon to attach to DSP root PD and cause troubles there which is not
>> a good thing. So basically any trusted process(maybe same group) should only use secure
>> device node and any process using non-secure node should only offload to unsigned PD.
> Again, you are describing expected behaviour. Other userspace clients
> can deviate from this.
Okay, understood.
>
>> 2. Having this property well defined also help in scaling fastrpc driver for new domains(like CDSP1
>> was recently introduced) as driver can only rely on the "label" and "non-secure-domain" property
>> for device creation. Say, only secure device is create if property is not defined and both device nodes
>> are created if non-secure-domain is define. This way, the dependency on domain_id can be removed
>> from fastrpc_rpmsg_probe[1] and create either only fastrpc-xdsp-secure or both(secure and non-secure).
> Well, I don't think I follow this point. The property is already
> well-defined.
By well-defined I meant there isn't a proper documentation of what is meant by non-secure-domain.
>> This however is a regression as you have mentioned, but it it helps address multiple problems.
>>
>> Should I discuss further on documentation or is any more design clarification should be done here?
> At least you must explicitly specify that this causes changes to
> userspace, and all the reasons to do that. So that everybody else
> doesn't have to read between the lines.
Ack.
>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/misc/fastrpc.c#n2327
>>
>> --ekansh
>>>> --ekansh
>>>>>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>>>>>> ---
>>>>>>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 1 -
>>>>>>  1 file changed, 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>>>>> index 3d8410683402..c633926c0f33 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>>>>> @@ -3852,7 +3852,6 @@ fastrpc {
>>>>>>                                      compatible = "qcom,fastrpc";
>>>>>>                                      qcom,glink-channels = "fastrpcglink-apps-dsp";
>>>>>>                                      label = "adsp";
>>>>>> -                                    qcom,non-secure-domain;
> - Are there other platforms which have this flag set for ADSP?
Yes, there are a few platforms where this property is added for ADSP.
> - Granted that sc7280 was targeting ChromeOS devices, might it be that
>   there is a CrOS-specific userspace for that?
FastRPC nodes were recently added to this devicetree recently. Looks like this property is just getting copied.
It might be that fastrpc was recently tried on ChromeOS device or it might be added to support some other devices
that uses fastrpc(qcm6490-idp etc.).

--ekansh

>>>>>>                                      #address-cells = <1>;
>>>>>>                                      #size-cells = <0>;
>>>>>>
>>>>>> --
>>>>>> 2.34.1
>>>>>>
Dmitry Baryshkov Nov. 15, 2024, 2:45 p.m. UTC | #7
On Fri, Nov 15, 2024 at 05:40:23PM +0530, Ekansh Gupta wrote:
> 
> 
> On 11/14/2024 5:30 PM, Dmitry Baryshkov wrote:
> > On Thu, Nov 14, 2024 at 10:49:52AM +0530, Ekansh Gupta wrote:
> >>
> >> On 11/13/2024 5:20 PM, Dmitry Baryshkov wrote:
> >>> On Wed, 13 Nov 2024 at 08:18, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
> >>>>
> >>>> On 11/13/2024 11:13 AM, Dmitry Baryshkov wrote:
> >>>>> On Wed, Nov 13, 2024 at 10:30:42AM +0530, Ekansh Gupta wrote:

[...]

> >>>>>>
> >>>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >>>>>> index 3d8410683402..c633926c0f33 100644
> >>>>>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >>>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >>>>>> @@ -3852,7 +3852,6 @@ fastrpc {
> >>>>>>                                      compatible = "qcom,fastrpc";
> >>>>>>                                      qcom,glink-channels = "fastrpcglink-apps-dsp";
> >>>>>>                                      label = "adsp";
> >>>>>> -                                    qcom,non-secure-domain;
> > - Are there other platforms which have this flag set for ADSP?
> Yes, there are a few platforms where this property is added for ADSP.

Please clean up all of them to reduce a possible chance of different behaviour or further c&p errors.

> > - Granted that sc7280 was targeting ChromeOS devices, might it be that
> >   there is a CrOS-specific userspace for that?
> FastRPC nodes were recently added to this devicetree recently. Looks like this property is just getting copied.
> It might be that fastrpc was recently tried on ChromeOS device or it might be added to support some other devices
> that uses fastrpc(qcm6490-idp etc.).

Indeed.

Luca, could you possibly comment, as you've added ADSP / FastRPC nodes?
Luca Weiss Nov. 15, 2024, 3:06 p.m. UTC | #8
Hi Dmitry,

On Fri Nov 15, 2024 at 3:45 PM CET, Dmitry Baryshkov wrote:
> On Fri, Nov 15, 2024 at 05:40:23PM +0530, Ekansh Gupta wrote:
> > 
> > 
> > On 11/14/2024 5:30 PM, Dmitry Baryshkov wrote:
> > > On Thu, Nov 14, 2024 at 10:49:52AM +0530, Ekansh Gupta wrote:
> > >>
> > >> On 11/13/2024 5:20 PM, Dmitry Baryshkov wrote:
> > >>> On Wed, 13 Nov 2024 at 08:18, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
> > >>>>
> > >>>> On 11/13/2024 11:13 AM, Dmitry Baryshkov wrote:
> > >>>>> On Wed, Nov 13, 2024 at 10:30:42AM +0530, Ekansh Gupta wrote:
>
> [...]
>
> > >>>>>>
> > >>>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > >>>>>> index 3d8410683402..c633926c0f33 100644
> > >>>>>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > >>>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > >>>>>> @@ -3852,7 +3852,6 @@ fastrpc {
> > >>>>>>                                      compatible = "qcom,fastrpc";
> > >>>>>>                                      qcom,glink-channels = "fastrpcglink-apps-dsp";
> > >>>>>>                                      label = "adsp";
> > >>>>>> -                                    qcom,non-secure-domain;
> > > - Are there other platforms which have this flag set for ADSP?
> > Yes, there are a few platforms where this property is added for ADSP.
>
> Please clean up all of them to reduce a possible chance of different behaviour or further c&p errors.
>
> > > - Granted that sc7280 was targeting ChromeOS devices, might it be that
> > >   there is a CrOS-specific userspace for that?
> > FastRPC nodes were recently added to this devicetree recently. Looks like this property is just getting copied.
> > It might be that fastrpc was recently tried on ChromeOS device or it might be added to support some other devices
> > that uses fastrpc(qcm6490-idp etc.).
>
> Indeed.
>
> Luca, could you possibly comment, as you've added ADSP / FastRPC nodes?

I've just followed other platforms, I have little clue about FastRPC
myself apart from it being used for hexagonrpcd for interfacing with
sensors. There's not much (any?) docs out there.

Regards
Luca
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 3d8410683402..c633926c0f33 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3852,7 +3852,6 @@  fastrpc {
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "adsp";
-					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;