Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Linus Walleij]

On Tue, Sep 10, 2013 at 3:04 PM, Will Deacon <will.deacon@xxxxxxx> wrote:
> On Mon, Sep 09, 2013 at 07:15:31AM +0100, Alexandre Courbot wrote:

>> I don't have any information about the future of TF unfortunately,
>> excepted that it should remain backward-compatible. What is this SMC
>> calling convention doc your are talking about btw? Is there a standard
>> calling convention defined by ARM?
>
> SMC calling convention:
> https://silver.arm.com/download/download.tm?pv=1435186

According to the SMC calling convention r0 should be
the SMC function identifier.

+static void __naked tf_generic_smc(u32 type, u32 subtype, u32 arg)
+{
+       asm volatile(
+               ".arch_extension        sec\n\t"
+               "stmfd  sp!, {r4 - r11, lr}\n\t"
+               __asmeq("%0", "r0")
+               __asmeq("%1", "r1")
+               __asmeq("%2", "r2")
+               "mov    r3, #0\n\t"
+               "mov    r4, #0\n\t"
+               "smc    #0\n\t"
+               "ldmfd  sp!, {r4 - r11, pc}"
+               :
+               : "r" (type), "r" (subtype), "r" (arg)
+               : "memory");
+}

So type == function identifier? Or are these two totally
different things, such that trusted foundations are already
a different beast that what the SMC calling convention
specifies?

Anyway:
r0,r1,r2 = type/subtype/arg.

+#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
(...)
+static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
+{
+       tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, boot_addr, 0);
+
+       return 0;
+}

What kind of a "subtype" is the boot address? I could have
accepted it if the *last* thing, the argument contained the boot
address. With the type/subtype/arg convention it would be
more natural if the "subtype" was something like 0.

Should the SMC function rather have this signature:

tf_generic_smc(u32 type, u32 arg1, u32 arg2)

?

Then *sometimes* arg1 is a subtype, if the "type" is
such that it takes a subtype?

(That's a rough guess.)

So to begin with the arguments to tf_generic_smc() are either
confusingly named or tf_set_cpu_boot_addr() begins the
journey with violating the function signature.

I also wonder what kind if "type" starts its enumerator on
0xfffff200? Wouldn't it be more natural if it was e.g. 1?
It looks like the TF_SET_CPU_BOOT_ADDR_SMC
reflects some bit-wise encoding scheme, so some details
here wouldn't hurt?

The main thing is that the patch has to say that this
is an API toward trusted foundations, that it has nothing
to do with the SMC calling conventions, and only pertain
to devices that use trusted foundations, so as to avoid
any confusion.

Then I'm game :-)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/