Re: [PATCH] ARM: OMAP: Use ARM SMC Calling Convention when OP-TEE is available

[Andrew F. Davis]

On 11/19/19 2:59 PM, Andrew F. Davis wrote:
> On 11/19/19 2:44 PM, Tony Lindgren wrote:
>> * Andrew F. Davis <afd@xxxxxx> [191119 19:36]:
>>> On 11/19/19 2:20 PM, Tony Lindgren wrote:
>>>> * Andrew F. Davis <afd@xxxxxx> [191119 19:13]:
>>>>> On 11/19/19 2:07 PM, Tony Lindgren wrote:
>>>>>> * Andrew F. Davis <afd@xxxxxx> [191119 18:51]:
>>>>>>> On 11/19/19 1:32 PM, Tony Lindgren wrote:
>>>>>>>> It would allow us to completely change over to using
>>>>>>>> arm_smccc_smc() and forget the custom calls.
>>>>>>>
>>>>>>> We would need more than just the r12 quirk to replace all our custom SMC
>>>>>>> handlers, we would need quirks for omap_smc2 which puts process ID in r1
>>>>>>> and puts #0xff in r6, and omap_smc3 that uses smc #1. All of our legacy
>>>>>>> SMC calls also trash r4-r11, that is very non SMCCC complaint as only
>>>>>>> r4-r7 need be caller saved. I don't see arm_smccc_smc() working with
>>>>>>> legacy ROM no matter how much we hack at it :(
>>>>>>
>>>>>> We would just have omap_smc2() call arm_smccc_smc() and in that
>>>>>> case. And omap_smc2() would still deal with saving and restoring
>>>>>> the registers.
>>>>>
>>>>> Then why call arm_smccc_smc()? omap_smc2() is already an assembly
>>>>> function, all it needs to do after loading the registers and saving the
>>>>> right ones is issue an "smc #0" instruction, why would we want to
>>>>> instead call into some other function to re-save registers and issue the
>>>>> exact same instruction?
>>>>
>>>> To use Linux generic API for smc calls where possible.
>>>
>>> But we are not using generic API calls, we are using omap_smcx() which
>>> cannot call into arm_smccc_smc(). For all the above reasons plus
>>> arm_smccc_smc() uses r12 to save the stack pointer, our ROM expects r12
>>> to store the function ID.
>>
>> Saving and restoring r12 could be handled by the arm_smccc_smc() quirk
>> for the non-optee case.
>>
>> Then we could get rid of omap_smc1() and arm_smccc_smc() should work
>> for the optee case and non-optee case, right.
>>
> 
> 
> Yes, we could have both cases working if we could get the quirk in.
> 
> 
>>>>>> Certainly the wrapper functions calling arm_smccc_smc() can deal
>>>>>> with r12 too if the r12-quirk version and the plain version are
>>>>>> never needed the same time on a booted SoC.
>>>>>>
>>>>>> Are they ever needed the same time on a booted SoC or not?
>>
>>> They should not be needed at the same time, either OP-TEE is on the
>>> secure side or ROM is there.
>>
>> OK thanks. So we could just modify the code dynamically on boot
>> based on if optee is found or not. The quirk could be done along
>> the lines of the qcom quirk but only for the non-optee case:
>>
> 
> 
> We wouldn't have to patch anything if we could get the quirk in. One has
> to state they wish to use the quirk version in a structure passed into
> arm_smccc_smc_quirk(), in which case for all legacy user we just fill
> out this quirk struct. OP-TEE uses the same arm_smccc_smc() but without
> the quirk struct and so it uses the compliant call.
> 
> The issue is still the same, I tried adding this, I got NAKd, if you
> want to convince Mark to change his mind and allow us the quirk then we
> can go down this path. Otherwise this will remain a dead end.
> 


Hi Tony,

Looks like the TI quirk idea is not moving forward, even the QCOM quirk
looks like it may get kicked out. arm_smccc_smc() will remain only for
SMCCC compliant calls, but it looks like a generic arm_smc() wouldn't be
too opposed upstream.

Either way this patch would still be valid as when OP-TEE is present
then arm_smccc_smc() will be the right call to make, how we handle the
legacy calls can be sorted out later if a generic SMC call is implemented.

Thanks,
Andrew


> Andrew
> 
> 
>> $ git grep -C10 ARM_SMCCC_QUIRK_QCOM_A6
>>
>> Regards,
>>
>> Tony
>>