arm_smccc_smc as generic smc interface?

arm_smccc_smc as generic smc interface?

[Tony Lindgren]

Hi all,

So it seems that we could make arm_smccc_smc() into a generic kernel
smc interface instead of being limited to optee usage. That is
assuming optee and legacy calls are never be enabled the same time
on a booted system :)

I know arm_smccc_smc() currently assumes a specific register usage
for the optee case, but AFAIK those limitations do not exist for
non-optee cases.

Does anybody see some other issues with making arm_smccc_smc() into
a generic smc call interface?

If there are some more optee specific considerations with making
arm_smccc_smc() into a generic interface, we could just set up
something generic that also arm_smccc_smc() can then call.

The use case I'm familiar with are the old TI smc calls that need
register specific quirks enabled only for the non-optee case,
while with optee enabled, quirks are not needed. There are
probably similar issues with other SoCs too.

Regards,

Tony

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: arm_smccc_smc as generic smc interface?

[Jens Wiklander]

Hi Tony,

On Mon, Dec 9, 2019 at 7:07 PM Tony Lindgren <tony@atomide.com> wrote:
>
> Hi all,
>
> So it seems that we could make arm_smccc_smc() into a generic kernel
> smc interface instead of being limited to optee usage. That is
> assuming optee and legacy calls are never be enabled the same time
> on a booted system :)

arm_smccc_smc() is not limited to OP-TEE only. A quick grep gives
quite a few places of which OP-TEE is just one.

>
> I know arm_smccc_smc() currently assumes a specific register usage
> for the optee case, but AFAIK those limitations do not exist for
> non-optee cases.

arm_smccc_smc() is for SMCs following SMC calling convention, see
http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html

>
> Does anybody see some other issues with making arm_smccc_smc() into
> a generic smc call interface?

I suppose that depends on what you mean with a generic smc call
interface. arm_smccc_smc() is quite generic already as I see it. :-)

>
> If there are some more optee specific considerations with making
> arm_smccc_smc() into a generic interface, we could just set up
> something generic that also arm_smccc_smc() can then call.

OP-TEE is relying on SMC calling convention
http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html

>
> The use case I'm familiar with are the old TI smc calls that need
> register specific quirks enabled only for the non-optee case,
> while with optee enabled, quirks are not needed. There are
> probably similar issues with other SoCs too.

I'm not too familiar with those. There's a few of them in the OP-TEE
code base too, so at least some of them can be handled via the SMC
calling convention.

In <arm-smccc.h> there's already been made room for some Qualcomm
quirks, perhaps it's possible to use or extend it to cover the TI
cases you have in mind.

Thanks,
Jens

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: arm_smccc_smc as generic smc interface?

[Tony Lindgren]

* Jens Wiklander <jens.wiklander@linaro.org> [191210 08:10]:
> Hi Tony,
> 
> On Mon, Dec 9, 2019 at 7:07 PM Tony Lindgren <tony@atomide.com> wrote:
> >
> > Hi all,
> >
> > So it seems that we could make arm_smccc_smc() into a generic kernel
> > smc interface instead of being limited to optee usage. That is
> > assuming optee and legacy calls are never be enabled the same time
> > on a booted system :)
> 
> arm_smccc_smc() is not limited to OP-TEE only. A quick grep gives
> quite a few places of which OP-TEE is just one.

OK good to hear.

> > I know arm_smccc_smc() currently assumes a specific register usage
> > for the optee case, but AFAIK those limitations do not exist for
> > non-optee cases.
> 
> arm_smccc_smc() is for SMCs following SMC calling convention, see
> http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html

Hmm yes that's the part I'm wondering about. For older TI SoCs,
in the non-optee cases, the TI smc calls do not follow the newer
convention at least for r12 usage. For optee cases, TI SoCs follow
the convention AFAIK.

But assuming optee and non-optee are never active the same time,
handling the TI r12 quirk for non-optee cases should not cause
issues that I can think of.

However, if we wanted to have arm_smccc_smc() bail out for non-optee
cases for example, then it probably makes sense to move most of the
arm_smccc_smc() into a more generic function like arm_smc(), and
then have arm_smccc_smc() call arm_smc(). But AFAIK this should
not be needed as the optee code would not be active in the
non-optee case at all.

> > Does anybody see some other issues with making arm_smccc_smc() into
> > a generic smc call interface?
> 
> I suppose that depends on what you mean with a generic smc call
> interface. arm_smccc_smc() is quite generic already as I see it. :-)

Yes it already has nice quirk handling and should work nicely
to replace most of the SoC specific smc calls eventually :)

> > If there are some more optee specific considerations with making
> > arm_smccc_smc() into a generic interface, we could just set up
> > something generic that also arm_smccc_smc() can then call.
> 
> OP-TEE is relying on SMC calling convention
> http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html

Yes and the earlier non-optee smc cases may or may not follow it.

> > The use case I'm familiar with are the old TI smc calls that need
> > register specific quirks enabled only for the non-optee case,
> > while with optee enabled, quirks are not needed. There are
> > probably similar issues with other SoCs too.
> 
> I'm not too familiar with those. There's a few of them in the OP-TEE
> code base too, so at least some of them can be handled via the SMC
> calling convention.
> 
> In <arm-smccc.h> there's already been made room for some Qualcomm
> quirks, perhaps it's possible to use or extend it to cover the TI
> cases you have in mind.

Yeah that's my thinking too as long as there are no issues using
arm_smccc_smc() for non-optee cases.

Regards,

Tony

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: arm_smccc_smc as generic smc interface?

[Mark Rutland]

Hi,

On Tue, Dec 10, 2019 at 07:38:40AM -0800, Tony Lindgren wrote:
> * Jens Wiklander <jens.wiklander@linaro.org> [191210 08:10]:
> > On Mon, Dec 9, 2019 at 7:07 PM Tony Lindgren <tony@atomide.com> wrote:
> > In <arm-smccc.h> there's already been made room for some Qualcomm
> > quirks, perhaps it's possible to use or extend it to cover the TI
> > cases you have in mind.
> 
> Yeah that's my thinking too as long as there are no issues using
> arm_smccc_smc() for non-optee cases.

I don't think that we wan't to turn the SMCCC code itself into a generic
SMC interface, and this boils down to how divergent the TI calling
convention is from SMCCC. What are the differences?

If the conventions differ too much I think having a parallel interface
is preferable. We're going to be extending the SMCCC code for SMCCCv1.2,
and I'd rather not complicat the common code or the 64-bit code for
this.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: arm_smccc_smc as generic smc interface?

[Andrew F. Davis]

On 12/12/19 9:23 AM, Mark Rutland wrote:
> Hi,
> 
> On Tue, Dec 10, 2019 at 07:38:40AM -0800, Tony Lindgren wrote:
>> * Jens Wiklander <jens.wiklander@linaro.org> [191210 08:10]:
>>> On Mon, Dec 9, 2019 at 7:07 PM Tony Lindgren <tony@atomide.com> wrote:
>>> In <arm-smccc.h> there's already been made room for some Qualcomm
>>> quirks, perhaps it's possible to use or extend it to cover the TI
>>> cases you have in mind.
>>
>> Yeah that's my thinking too as long as there are no issues using
>> arm_smccc_smc() for non-optee cases.
> 
> I don't think that we wan't to turn the SMCCC code itself into a generic
> SMC interface, and this boils down to how divergent the TI calling
> convention is from SMCCC. What are the differences?
> 


There are three levels of differences based on the the type of legacy
SMC call we make, first type omap_smc1():

 * r2-r12 are clobbered vs SMCCC only saving r4-r7
 * r12 used as command ID vs SMCCC using r0

omap_smc2(), in addition to the above incompatibilities:

 * r6 must contain #0xff
 * r12 used as command ID again

omap_smc3(), in addition to the above incompatibilities:

 * Uses SMC #1 vs #0

My personal opinion would be to leave arm_smccc_smc() alone and only
allow SMCCC complaint callers to use it (even QCOM's quark should not
have gone in if the plan was not to allow the same exceptions for
others). I believe what Tony might really want is a separate generic
arm_smc() that can handle the above cases based on parameters so that we
don't have to drop into custom assembly in mach-omap2.

Andrew


> If the conventions differ too much I think having a parallel interface
> is preferable. We're going to be extending the SMCCC code for SMCCCv1.2,
> and I'd rather not complicat the common code or the 64-bit code for
> this.
> 
> Thanks,
> Mark.
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: arm_smccc_smc as generic smc interface?

[Mark Rutland]

On Thu, Dec 12, 2019 at 10:23:47AM -0500, Andrew F. Davis wrote:
> On 12/12/19 9:23 AM, Mark Rutland wrote:
> > On Tue, Dec 10, 2019 at 07:38:40AM -0800, Tony Lindgren wrote:
> >> * Jens Wiklander <jens.wiklander@linaro.org> [191210 08:10]:
> >>> On Mon, Dec 9, 2019 at 7:07 PM Tony Lindgren <tony@atomide.com> wrote:
> >>> In <arm-smccc.h> there's already been made room for some Qualcomm
> >>> quirks, perhaps it's possible to use or extend it to cover the TI
> >>> cases you have in mind.
> >>
> >> Yeah that's my thinking too as long as there are no issues using
> >> arm_smccc_smc() for non-optee cases.
> > 
> > I don't think that we wan't to turn the SMCCC code itself into a generic
> > SMC interface, and this boils down to how divergent the TI calling
> > convention is from SMCCC. What are the differences?
> 
> There are three levels of differences based on the the type of legacy
> SMC call we make, first type omap_smc1():
> 
>  * r2-r12 are clobbered vs SMCCC only saving r4-r7
>  * r12 used as command ID vs SMCCC using r0
> 
> omap_smc2(), in addition to the above incompatibilities:
> 
>  * r6 must contain #0xff
>  * r12 used as command ID again
> 
> omap_smc3(), in addition to the above incompatibilities:
> 
>  * Uses SMC #1 vs #0
> 
> My personal opinion would be to leave arm_smccc_smc() alone and only
> allow SMCCC complaint callers to use it (even QCOM's quark should not
> have gone in if the plan was not to allow the same exceptions for
> others). 

I agree. Given the above examples, I'd rather have this separated out.
While I'd have preferred to not have the QCOM quirk, that's simply
something we have to live with now.

> I believe what Tony might really want is a separate generic
> arm_smc() that can handle the above cases based on parameters so that we
> don't have to drop into custom assembly in mach-omap2.

FWIW, I agree that having a separate arm_generic_smc() feels like a
better way of handling the omap cases above. I think that only needs to
exist for arch/arm, too, so some complexity involved with making that
arch-neutral can be avoided.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: arm_smccc_smc as generic smc interface?

[Will Deacon]

On Thu, Dec 12, 2019 at 03:58:12PM +0000, Mark Rutland wrote:
> On Thu, Dec 12, 2019 at 10:23:47AM -0500, Andrew F. Davis wrote:
> > On 12/12/19 9:23 AM, Mark Rutland wrote:
> > > On Tue, Dec 10, 2019 at 07:38:40AM -0800, Tony Lindgren wrote:
> > >> * Jens Wiklander <jens.wiklander@linaro.org> [191210 08:10]:
> > >>> On Mon, Dec 9, 2019 at 7:07 PM Tony Lindgren <tony@atomide.com> wrote:
> > >>> In <arm-smccc.h> there's already been made room for some Qualcomm
> > >>> quirks, perhaps it's possible to use or extend it to cover the TI
> > >>> cases you have in mind.
> > >>
> > >> Yeah that's my thinking too as long as there are no issues using
> > >> arm_smccc_smc() for non-optee cases.
> > > 
> > > I don't think that we wan't to turn the SMCCC code itself into a generic
> > > SMC interface, and this boils down to how divergent the TI calling
> > > convention is from SMCCC. What are the differences?
> > 
> > There are three levels of differences based on the the type of legacy
> > SMC call we make, first type omap_smc1():
> > 
> >  * r2-r12 are clobbered vs SMCCC only saving r4-r7
> >  * r12 used as command ID vs SMCCC using r0
> > 
> > omap_smc2(), in addition to the above incompatibilities:
> > 
> >  * r6 must contain #0xff
> >  * r12 used as command ID again
> > 
> > omap_smc3(), in addition to the above incompatibilities:
> > 
> >  * Uses SMC #1 vs #0
> > 
> > My personal opinion would be to leave arm_smccc_smc() alone and only
> > allow SMCCC complaint callers to use it (even QCOM's quark should not
> > have gone in if the plan was not to allow the same exceptions for
> > others). 
> 
> I agree. Given the above examples, I'd rather have this separated out.
> While I'd have preferred to not have the QCOM quirk, that's simply
> something we have to live with now.

It's not ABI is it, so we could split that out as well? That said, my
recollection of the qcom quirk is that they were mostly following the
SMCCC spec, but just with an extra register to save/restore, which is
a lot less invasive than the stuff above.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: arm_smccc_smc as generic smc interface?

[Andrew F. Davis]

On 12/12/19 12:12 PM, Will Deacon wrote:
> On Thu, Dec 12, 2019 at 03:58:12PM +0000, Mark Rutland wrote:
>> On Thu, Dec 12, 2019 at 10:23:47AM -0500, Andrew F. Davis wrote:
>>> On 12/12/19 9:23 AM, Mark Rutland wrote:
>>>> On Tue, Dec 10, 2019 at 07:38:40AM -0800, Tony Lindgren wrote:
>>>>> * Jens Wiklander <jens.wiklander@linaro.org> [191210 08:10]:
>>>>>> On Mon, Dec 9, 2019 at 7:07 PM Tony Lindgren <tony@atomide.com> wrote:
>>>>>> In <arm-smccc.h> there's already been made room for some Qualcomm
>>>>>> quirks, perhaps it's possible to use or extend it to cover the TI
>>>>>> cases you have in mind.
>>>>>
>>>>> Yeah that's my thinking too as long as there are no issues using
>>>>> arm_smccc_smc() for non-optee cases.
>>>>
>>>> I don't think that we wan't to turn the SMCCC code itself into a generic
>>>> SMC interface, and this boils down to how divergent the TI calling
>>>> convention is from SMCCC. What are the differences?
>>>
>>> There are three levels of differences based on the the type of legacy
>>> SMC call we make, first type omap_smc1():
>>>
>>>  * r2-r12 are clobbered vs SMCCC only saving r4-r7
>>>  * r12 used as command ID vs SMCCC using r0
>>>
>>> omap_smc2(), in addition to the above incompatibilities:
>>>
>>>  * r6 must contain #0xff
>>>  * r12 used as command ID again
>>>
>>> omap_smc3(), in addition to the above incompatibilities:
>>>
>>>  * Uses SMC #1 vs #0
>>>
>>> My personal opinion would be to leave arm_smccc_smc() alone and only
>>> allow SMCCC complaint callers to use it (even QCOM's quark should not
>>> have gone in if the plan was not to allow the same exceptions for
>>> others). 
>>
>> I agree. Given the above examples, I'd rather have this separated out.
>> While I'd have preferred to not have the QCOM quirk, that's simply
>> something we have to live with now.
> 
> It's not ABI is it, so we could split that out as well? That said, my
> recollection of the qcom quirk is that they were mostly following the
> SMCCC spec, but just with an extra register to save/restore, which is
> a lot less invasive than the stuff above.
> 


Two problems with the QCOM quirk.

First is that it was done in just about the worst way: QCOM code that
uses it has to use a new function arm_smccc_smc_quirk(), but then this
calls into the generic arm_smccc_smc() which itself was modified to
handle the QCOM specific quark. That means not only does a different
function need to be called by QCOM code anyway but then also everyone
else not using it has to still do all the quirk check handling, even
when using the generic call!

Second it opens the flood gates for other looking for similar quirk
handling, and if we don't get it we cry foul that QCOM got preferential
treatment.

It's not ABI, so I'd say take out the quirk handling from the base
arm_smccc_smc() and have arm_smccc_smc_quirk() take it's own QCOM
specific path.

Andrew


> Will
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: arm_smccc_smc as generic smc interface?

[Mark Rutland]

On Thu, Dec 12, 2019 at 12:47:37PM -0500, Andrew F. Davis wrote:
> On 12/12/19 12:12 PM, Will Deacon wrote:
> > On Thu, Dec 12, 2019 at 03:58:12PM +0000, Mark Rutland wrote:
> >> On Thu, Dec 12, 2019 at 10:23:47AM -0500, Andrew F. Davis wrote:
> >>> On 12/12/19 9:23 AM, Mark Rutland wrote:
> >>>> On Tue, Dec 10, 2019 at 07:38:40AM -0800, Tony Lindgren wrote:
> >>>>> * Jens Wiklander <jens.wiklander@linaro.org> [191210 08:10]:
> >>>>>> On Mon, Dec 9, 2019 at 7:07 PM Tony Lindgren <tony@atomide.com> wrote:
> >>>>>> In <arm-smccc.h> there's already been made room for some Qualcomm
> >>>>>> quirks, perhaps it's possible to use or extend it to cover the TI
> >>>>>> cases you have in mind.
> >>>>>
> >>>>> Yeah that's my thinking too as long as there are no issues using
> >>>>> arm_smccc_smc() for non-optee cases.
> >>>>
> >>>> I don't think that we wan't to turn the SMCCC code itself into a generic
> >>>> SMC interface, and this boils down to how divergent the TI calling
> >>>> convention is from SMCCC. What are the differences?
> >>>
> >>> There are three levels of differences based on the the type of legacy
> >>> SMC call we make, first type omap_smc1():
> >>>
> >>>  * r2-r12 are clobbered vs SMCCC only saving r4-r7
> >>>  * r12 used as command ID vs SMCCC using r0
> >>>
> >>> omap_smc2(), in addition to the above incompatibilities:
> >>>
> >>>  * r6 must contain #0xff
> >>>  * r12 used as command ID again
> >>>
> >>> omap_smc3(), in addition to the above incompatibilities:
> >>>
> >>>  * Uses SMC #1 vs #0
> >>>
> >>> My personal opinion would be to leave arm_smccc_smc() alone and only
> >>> allow SMCCC complaint callers to use it (even QCOM's quark should not
> >>> have gone in if the plan was not to allow the same exceptions for
> >>> others). 
> >>
> >> I agree. Given the above examples, I'd rather have this separated out.
> >> While I'd have preferred to not have the QCOM quirk, that's simply
> >> something we have to live with now.
> > 
> > It's not ABI is it, so we could split that out as well? That said, my
> > recollection of the qcom quirk is that they were mostly following the
> > SMCCC spec, but just with an extra register to save/restore, which is
> > a lot less invasive than the stuff above.

IIRC they misunderstood the spec rather than having a deliberately
divergent calling convention.

> Two problems with the QCOM quirk.
> 
> First is that it was done in just about the worst way: QCOM code that
> uses it has to use a new function arm_smccc_smc_quirk(), but then this
> calls into the generic arm_smccc_smc() which itself was modified to
> handle the QCOM specific quark. That means not only does a different
> function need to be called by QCOM code anyway but then also everyone
> else not using it has to still do all the quirk check handling, even
> when using the generic call!
>
> Second it opens the flood gates for other looking for similar quirk
> handling, and if we don't get it we cry foul that QCOM got preferential
> treatment.
> 
> It's not ABI, so I'd say take out the quirk handling from the base
> arm_smccc_smc() and have arm_smccc_smc_quirk() take it's own QCOM
> specific path.

I have no problem with splitting that out. I expect that that the QCOM
quirk will end up getting split out as part of adding support for
SMCCCv1.2, which will require refactoring the SMCCC code. To avoid
having to juggle multiple sets of patches fiddling with that, I'd prefer
to leave the SMCCC code as-is for now.

The intent is certainly not to give anyone preferential treatment, and I
hope that no-one on this thread sees things that way. We simply did the
easiest thing at the point we introduced the quirk.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel