ARM: smccc-call: Use r12 to route secure monitor calls on TI platforms

On TI platforms OP-TEE must be called using a modified SMC call, allow the selection of this though DT.

Signed-off-by Andrew F. Davis <[email protected]>

---
 Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt | 2 ++
 drivers/tee/optee/core.c                                           | 2 ++
 2 files changed, 4 insertions(+)

Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt +0 -0

index d38834c67dff..a3275ecdf186 100644

Quoted 17 lines · Andrew F. Davis

On TI platforms OP-TEE must be called using a modified SMC call, allow the selection of this though DT.

Signed-off-by Andrew F. Davis <[email protected]>

---
 Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt | 2 ++
 drivers/tee/optee/core.c                                           | 2 ++
 2 files changed, 4 insertions(+)

Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt +0 -0

index d38834c67dff..a3275ecdf186 100644
@@ -20,6 +20,8 @@ the reference implementation maintained by Linaro.
                    "hvc" : HVC #0, with the register assignments specified
                           in drivers/tee/optee/optee_smc.h
 

Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt the reference implementation maintained by Linaro. · line 24

Quoted 2 lines

Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt +2 -0

+                   "ti-smc" : Similar to "smc" with TI specific register
+                             adjustments

Sigh, really? IMO, this should be determined from the compatible string. Then the next TI (or any vendor) specific thing can be handled without a DT change.

Maybe some day we'll figure out that not just h/w needs to be probe-able, but all the firmware pieces do too.

Rob

Quoted 18 lines · Rob Herring

Quoted 17 lines · Andrew F. Davis

On TI platforms OP-TEE must be called using a modified SMC call, allow the selection of this though DT.

Signed-off-by Andrew F. Davis <[email protected]>

---
 Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt | 2 ++
 drivers/tee/optee/core.c                                           | 2 ++
 2 files changed, 4 insertions(+)

Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt +0 -0

index d38834c67dff..a3275ecdf186 100644
@@ -20,6 +20,8 @@ the reference implementation maintained by Linaro.
                    "hvc" : HVC #0, with the register assignments specified
                           in drivers/tee/optee/optee_smc.h
 

Quoted 7 lines

Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt the reference implementation maintained by Linaro. · line 24

Quoted 2 lines

Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt +2 -0

+                   "ti-smc" : Similar to "smc" with TI specific register
+                             adjustments

Sigh, really? IMO, this should be determined from the compatible string. Then the next TI (or any vendor) specific thing can be handled without a DT change.

Which compatible string, do you mean the OP-TEE driver check the top level platform compatible string?

On TI platforms OP-TEE must be called using a modified SMC call, allow the selection of this though DT.

Signed-off-by Andrew F. Davis <[email protected]>

---
 Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt | 2 ++
 drivers/tee/optee/core.c                                           | 2 ++
 2 files changed, 4 insertions(+)

Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt +0 -0

index d38834c67dff..a3275ecdf186 100644

Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt +0 -0

 Example:

drivers/tee/optee/core.c +2 -0

index 7952357df9c8..dfa9de590d98 100644
@@ -441,6 +441,8 @@ static optee_invoke_fn *get_invoke_func(struct device_node *np)
 		return optee_smccc_hvc;
 	else if (!strcmp("smc", method))
 		return optee_smccc_smc;
+	else if (!strcmp("ti-smc", method))
+		return arm_ti_smccc_smc;
 
 	pr_warn("invalid \"method\" property: %s\n", method);
 	return ERR_PTR(-EINVAL);

Signature · 1 line

2.14.1

Our ROM Secure Monitor(SM) uses the value in r12 to determine which service is being requested by an SMC call. This inline with the ARM recommended SMC Calling Convention(SMCCC), which partitions the values in R0 for this task, OP-TEE's SM follows the ARM recommended convention.

We need a way to signal that a call is for the OP-TEE SM and not for the ROM SM in a way that is safe for the ROM SM, in case it is still present. We do this by putting a value of 0x200 in r12 when the call is for OP-TEE or any other ARM by modifying the SMCCC caller function.

There are four combinations of events:

If the ROM SM is present and we make a legacy style SMC call, as we do in early boot, the call will not have r12 set to 0x200 as these calls go through existing mach-omap2/ SMC handlers, so all is well.

If the ROM SM is present and we make an SMCCC style call, r12 will be set to 0x200 and ROM SM will see this as an invalid service call and safely return to the normal world.

If OP-TEE is present and we make a legacy style SMC call, r12 will not be set to 0x200, and OP-TEE will emulate the functionality that the call is requesting.

Quoted 4 lines · Andrew F. Davis

Our ROM Secure Monitor(SM) uses the value in r12 to determine which service is being requested by an SMC call. This inline with the ARM recommended SMC Calling Convention(SMCCC), which partitions the values in R0 for this task, OP-TEE's SM follows the ARM recommended convention.

I can't parse this last sentence. What exactly is inline with the SMCCC?

AFAICT, the SMCCC says r12 is "The Intra-Procedure-call scratch register", which is not a paramter, service ID, etc.

Quoted 4 lines

We need a way to signal that a call is for the OP-TEE SM and not for the ROM SM in a way that is safe for the ROM SM, in case it is still present. We do this by putting a value of 0x200 in r12 when the call is for OP-TEE or any other ARM by modifying the SMCCC caller function.

So IIUC, you have a secure monitor which is not SMCCC compliant, but you want to use it at the same time as OP-TEE, which is SMCCC compliant.

It would be much better to have a new version of that monitor that was SMCCC compliant, and have to update the code for that, than to have to move OP-TEE away from standards compliance.

Quoted 6 lines

There are four combinations of events:

If the ROM SM is present and we make a legacy style SMC call, as we do in early boot, the call will not have r12 set to 0x200 as these calls go through existing mach-omap2/ SMC handlers, so all is well.

Quoted 3 lines

If the ROM SM is present and we make an SMCCC style call, r12 will be set to 0x200 and ROM SM will see this as an invalid service call and safely return to the normal world.

So you're going to describe OP-TEE as present even when it's not!?

That's simply wrong.

Quoted 3 lines

If OP-TEE is present and we make a legacy style SMC call, r12 will not be set to 0x200, and OP-TEE will emulate the functionality that the call is requesting.

AFAICT this means that OP-TEE completely replaces your existing monitor.

Please fix this properly. Implement SMCCC-compliant versions of those calls you wish to retain, and come up with a new binding for those. Describe that in the DT for systems which have OP-TEE providing these services.

Systems with the old monitor should have that descrbied in their DT, and not OP-TEE.

That completely removes the need to come up with a new OP-TEE binding extension, and aligns your FW for forward compatibility with future services.

Thanks,
Mark

Quoted 8 lines · Mark Rutland

Quoted 4 lines · Andrew F. Davis

We need a way to signal that a call is for the OP-TEE SM and not for the ROM SM in a way that is safe for the ROM SM, in case it is still present. We do this by putting a value of 0x200 in r12 when the call is for OP-TEE or any other ARM by modifying the SMCCC caller function.

So IIUC, you have a secure monitor which is not SMCCC compliant, but you want to use it at the same time as OP-TEE, which is SMCCC compliant.

Not at the same time, our old secure monitor is in ROM, it will be present anytime OP-TEE is not installed. OP-TEE is not mandatory, and so any call to the secure side must be safe for both secure monitors.

Quoted 3 lines

It would be much better to have a new version of that monitor that was SMCCC compliant, and have to update the code for that, than to have to move OP-TEE away from standards compliance.

The current monitor is part of ROM and dates back to OMAP3 days, so looks to pre-date ARM standardizing the SMCCC by several years, so I hope our lack of compliance here can be forgiven here..

Luckily, OP-TEE does not have to move away from compliance to support our platforms, it only needs to check for this sentinel value in r12, which it already has support upstream for:

https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/plat-ti/sm_platform_handler_a15.c#L34

Quoted 5 lines

Quoted 3 lines

If OP-TEE is present and we make a legacy style SMC call, r12 will not be set to 0x200, and OP-TEE will emulate the functionality that the call is requesting.

AFAICT this means that OP-TEE completely replaces your existing monitor.

Correct.

Quoted 4 lines

Please fix this properly. Implement SMCCC-compliant versions of those calls you wish to retain, and come up with a new binding for those. Describe that in the DT for systems which have OP-TEE providing these services.

This might not be possible, these old SMCs are made very early and are used all over our existing platform code:

git grep "smc" -- arch/arm/mach-omap2

Quoted 2 lines

Systems with the old monitor should have that descrbied in their DT, and not OP-TEE.

All systems have the old monitor by default, OP-TEE is the new and optional case, I think its node should be the entity modified when adding something OP-TEE specific like this.