diff mbox

[v5,1/5] arm/arm64: add smccc ARCH32

Message ID 1439973629-19505-2-git-send-email-jens.wiklander@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jens Wiklander Aug. 19, 2015, 8:40 a.m. UTC
Adds helpers to do SMC based on ARM SMC Calling Convention.
CONFIG_HAVE_SMCCC is enabled for architectures that may support
the SMC instruction. It's the responsibility of the caller to
know if the SMC instruction is supported by the platform.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 arch/arm/Kconfig               |  4 +++
 arch/arm/kernel/Makefile       |  2 ++
 arch/arm/kernel/smccc-call.S   | 26 ++++++++++++++
 arch/arm/kernel/smccc.c        | 17 +++++++++
 arch/arm64/Kconfig             |  4 +++
 arch/arm64/kernel/Makefile     |  1 +
 arch/arm64/kernel/smccc-call.S | 34 ++++++++++++++++++
 arch/arm64/kernel/smccc.c      | 17 +++++++++
 include/linux/arm-smccc.h      | 79 ++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 184 insertions(+)
 create mode 100644 arch/arm/kernel/smccc-call.S
 create mode 100644 arch/arm/kernel/smccc.c
 create mode 100644 arch/arm64/kernel/smccc-call.S
 create mode 100644 arch/arm64/kernel/smccc.c
 create mode 100644 include/linux/arm-smccc.h

Comments

Yury Norov Aug. 19, 2015, 10:56 a.m. UTC | #1
On 19.08.2015 11:40, Jens Wiklander wrote:
 >
 > Adds helpers to do SMC based on ARM SMC Calling Convention.
 > CONFIG_HAVE_SMCCC is enabled for architectures that may support
 > the SMC instruction. It's the responsibility of the caller to
 > know if the SMC instruction is supported by the platform.
 >
 > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
 > ---
 >  arch/arm/Kconfig               |  4 +++
 >  arch/arm/kernel/Makefile       |  2 ++
 >  arch/arm/kernel/smccc-call.S   | 26 ++++++++++++++
 >  arch/arm/kernel/smccc.c        | 17 +++++++++
 >  arch/arm64/Kconfig             |  4 +++
 >  arch/arm64/kernel/Makefile     |  1 +
 >  arch/arm64/kernel/smccc-call.S | 34 ++++++++++++++++++
 >  arch/arm64/kernel/smccc.c      | 17 +++++++++
 >  include/linux/arm-smccc.h      | 79 
++++++++++++++++++++++++++++++++++++++++++
 >  9 files changed, 184 insertions(+)
 >  create mode 100644 arch/arm/kernel/smccc-call.S
 >  create mode 100644 arch/arm/kernel/smccc.c
 >  create mode 100644 arch/arm64/kernel/smccc-call.S
 >  create mode 100644 arch/arm64/kernel/smccc.c
 >  create mode 100644 include/linux/arm-smccc.h
 >
 > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
 > index 45df48b..75e4da3 100644
 > --- a/arch/arm/Kconfig
 > +++ b/arch/arm/Kconfig
 > @@ -221,6 +221,9 @@ config NEED_RET_TO_USER
 >  config ARCH_MTD_XIP
 >  	bool
 >
 > +config HAVE_SMCCC
 > +	bool
 > +
 >  config VECTORS_BASE
 >  	hex
 >  	default 0xffff0000 if MMU || CPU_HIGH_VECTOR
 > @@ -324,6 +327,7 @@ config ARCH_MULTIPLATFORM
 >  	select CLKSRC_OF
 >  	select COMMON_CLK
 >  	select GENERIC_CLOCKEVENTS
 > +	select HAVE_SMCCC if CPU_V7
 >  	select MIGHT_HAVE_PCI
 >  	select MULTI_IRQ_HANDLER
 >  	select SPARSE_IRQ
 > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
 > index 752725d..8cdd25b 100644
 > --- a/arch/arm/kernel/Makefile
 > +++ b/arch/arm/kernel/Makefile
 > @@ -90,4 +90,6 @@ obj-y				+= psci.o psci-call.o
 >  obj-$(CONFIG_SMP)		+= psci_smp.o
 >  endif
 >
 > +obj-$(CONFIG_HAVE_SMCCC)	+= smccc-call.o smccc.o
 > +
 >  extra-y := $(head-y) vmlinux.lds
 > diff --git a/arch/arm/kernel/smccc-call.S b/arch/arm/kernel/smccc-call.S
 > new file mode 100644
 > index 0000000..05bc554
 > --- /dev/null
 > +++ b/arch/arm/kernel/smccc-call.S
 > @@ -0,0 +1,26 @@
 > +/*
 > + * Copyright (c) 2015, Linaro Limited
 > + *
 > + * This software is licensed under the terms of the GNU General Public
 > + * License version 2, as published by the Free Software Foundation, and
 > + * may be copied, distributed, and modified under those terms.
 > + *
 > + * This program is distributed in the hope that it will be useful,
 > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 > + * GNU General Public License for more details.
 > + *
 > + */
 > +#include <linux/linkage.h>
 > +
 > +#include <asm/opcodes-sec.h>
 > +
 > +/* void smccc_call32(struct smccc_param32 *param) */
 > +ENTRY(smccc_call32)
 > +	push	{r4-r8, lr}
 > +	mov	r8, r0
 > +	ldm	r8, {r0-r7}
 > +	__SMC(0)
 > +	stm	r8, {r0-r7}
 > +	pop	{r4-r8, pc}
 > +ENDPROC(smccc_call32)
 > diff --git a/arch/arm/kernel/smccc.c b/arch/arm/kernel/smccc.c
 > new file mode 100644
 > index 0000000..ba4039e
 > --- /dev/null
 > +++ b/arch/arm/kernel/smccc.c
 > @@ -0,0 +1,17 @@
 > +/*
 > + * Copyright (c) 2015, Linaro Limited
 > + *
 > + * This software is licensed under the terms of the GNU General Public
 > + * License version 2, as published by the Free Software Foundation, and
 > + * may be copied, distributed, and modified under those terms.
 > + *
 > + * This program is distributed in the hope that it will be useful,
 > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 > + * GNU General Public License for more details.
 > + *
 > + */
 > +#include <linux/export.h>
 > +#include <linux/arm-smccc.h>
 > +
 > +EXPORT_SYMBOL_GPL(smccc_call32);
 > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
 > index 7796af4..b3ea778 100644
 > --- a/arch/arm64/Kconfig
 > +++ b/arch/arm64/Kconfig
 > @@ -83,6 +83,7 @@ config ARM64
 >  	select SPARSE_IRQ
 >  	select SYSCTL_EXCEPTION_TRACE
 >  	select HAVE_CONTEXT_TRACKING
 > +	select HAVE_SMCCC
 >  	help
 >  	  ARM 64-bit (AArch64) Linux support.
 >
 > @@ -146,6 +147,9 @@ config KERNEL_MODE_NEON
 >  config FIX_EARLYCON_MEM
 >  	def_bool y
 >
 > +config HAVE_SMCCC
 > +	bool
 > +
 >  config PGTABLE_LEVELS
 >  	int
 >  	default 2 if ARM64_64K_PAGES && ARM64_VA_BITS_42
 > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
 > index 426d076..f7804f7 100644
 > --- a/arch/arm64/kernel/Makefile
 > +++ b/arch/arm64/kernel/Makefile
 > @@ -36,6 +36,7 @@ arm64-obj-$(CONFIG_EFI)			+= efi.o efi-stub.o 
efi-entry.o
 >  arm64-obj-$(CONFIG_PCI)			+= pci.o
 >  arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 >  arm64-obj-$(CONFIG_ACPI)		+= acpi.o
 > +arm64-obj-$(CONFIG_HAVE_SMCCC)		+= smccc-call.o smccc.o
 >
 >  obj-y					+= $(arm64-obj-y) vdso/
 >  obj-m					+= $(arm64-obj-m)
 > diff --git a/arch/arm64/kernel/smccc-call.S 
b/arch/arm64/kernel/smccc-call.S
 > new file mode 100644
 > index 0000000..3ce7fe8
 > --- /dev/null
 > +++ b/arch/arm64/kernel/smccc-call.S
 > @@ -0,0 +1,34 @@
 > +/*
 > + * Copyright (c) 2015, Linaro Limited
 > + *
 > + * This program is free software; you can redistribute it and/or modify
 > + * it under the terms of the GNU General Public License Version 2 as
 > + * published by the Free Software Foundation.
 > + *
 > + * This program is distributed in the hope that it will be useful,
 > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 > + * GNU General Public License for more details.
 > + *
 > + */
 > +#include <linux/linkage.h>
 > +
 > +#define SMC_PARAM_W0_OFFS	0
 > +#define SMC_PARAM_W2_OFFS	8
 > +#define SMC_PARAM_W4_OFFS	16
 > +#define SMC_PARAM_W6_OFFS	24
 > +
 > +/* void smccc_call32(struct smccc_param32 *param) */
 > +ENTRY(smccc_call32)
 > +	stp	x28, x30, [sp, #-16]!
 > +	mov	x28, x0
 > +	ldp	w0, w1, [x28, #SMC_PARAM_W0_OFFS]
 > +	ldp	w2, w3, [x28, #SMC_PARAM_W2_OFFS]
 > +	ldp	w4, w5, [x28, #SMC_PARAM_W4_OFFS]
 > +	ldp	w6, w7, [x28, #SMC_PARAM_W6_OFFS]
 > +	smc	#0
 > +	stp	w0, w1, [x28, #SMC_PARAM_W0_OFFS]
 > +	stp	w2, w3, [x28, #SMC_PARAM_W2_OFFS]
 > +	ldp	x28, x30, [sp], #16
 > +	ret
 > +ENDPROC(smccc_call32)
 > diff --git a/arch/arm64/kernel/smccc.c b/arch/arm64/kernel/smccc.c
 > new file mode 100644
 > index 0000000..ba4039e
 > --- /dev/null
 > +++ b/arch/arm64/kernel/smccc.c
 > @@ -0,0 +1,17 @@
 > +/*
 > + * Copyright (c) 2015, Linaro Limited
 > + *
 > + * This software is licensed under the terms of the GNU General Public
 > + * License version 2, as published by the Free Software Foundation, and
 > + * may be copied, distributed, and modified under those terms.
 > + *
 > + * This program is distributed in the hope that it will be useful,
 > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 > + * GNU General Public License for more details.
 > + *
 > + */
 > +#include <linux/export.h>
 > +#include <linux/arm-smccc.h>
 > +
 > +EXPORT_SYMBOL_GPL(smccc_call32);
 > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
 > new file mode 100644
 > index 0000000..9b8d775
 > --- /dev/null
 > +++ b/include/linux/arm-smccc.h
 > @@ -0,0 +1,79 @@
 > +/*
 > + * Copyright (c) 2015, Linaro Limited
 > + *
 > + * This software is licensed under the terms of the GNU General Public
 > + * License version 2, as published by the Free Software Foundation, and
 > + * may be copied, distributed, and modified under those terms.
 > + *
 > + * This program is distributed in the hope that it will be useful,
 > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 > + * GNU General Public License for more details.
 > + *
 > + */
 > +#ifndef __LINUX_ARM_SMCCC_H
 > +#define __LINUX_ARM_SMCCC_H
 > +
 > +#include <linux/types.h>
 > +
 > +/*
 > + * This file provideds defines common defines for ARM SMC Calling

typos here?

 > + * Convention as specified in
 > + * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
 > + */
 > +
 > +#define SMCCC_SMC_32			(0 << 30)

0 << 30 is just 0, and so meaningless.
It's better to introduce SMCCC_IS_32() macro instead.

 > +#define SMCCC_SMC_64			(1 << 30)
 > +#define SMCCC_FAST_CALL			(1 << 31)
 > +#define SMCCC_STD_CALL			(0 << 31)

The same

 > +
 > +#define SMCCC_OWNER_MASK		0x3F
 > +#define SMCCC_OWNER_SHIFT		24
 > +
 > +#define SMCCC_FUNC_MASK			0xFFFF
 > +
 > +#define SMCCC_IS_FAST_CALL(smc_val)	((smc_val) & SMCCC_FAST_CALL)
 > +#define SMCCC_IS_64(smc_val)		((smc_val) & SMCCC_SMC_64)
 > +#define SMCCC_FUNC_NUM(smc_val)		((smc_val) & SMCCC_FUNC_MASK)
 > +#define SMCCC_OWNER_NUM(smc_val) \
 > +	(((smc_val) >> SMCCC_OWNER_SHIFT) & SMCCC_OWNER_MASK)
 > +
 > +#define SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
 > +			((type) | (calling_convention) | \
 > +			(((owner) & SMCCC_OWNER_MASK) << SMCCC_OWNER_SHIFT) | \
 > +			((func_num) & SMCCC_FUNC_MASK))
 > +
 > +#define SMCCC_OWNER_ARCH		0
 > +#define SMCCC_OWNER_CPU			1
 > +#define SMCCC_OWNER_SIP			2
 > +#define SMCCC_OWNER_OEM			3
 > +#define SMCCC_OWNER_STANDARD		4
 > +#define SMCCC_OWNER_TRUSTED_APP		48
 > +#define SMCCC_OWNER_TRUSTED_APP_END	49
 > +#define SMCCC_OWNER_TRUSTED_OS		50
 > +#define SMCCC_OWNER_TRUSTED_OS_END	63
 > +
 > +struct smccc_param32 {
 > +	u32 a0;
 > +	u32 a1;
 > +	u32 a2;
 > +	u32 a3;
 > +	u32 a4;
 > +	u32 a5;
 > +	u32 a6;
 > +	u32 a7;
 > +};
 > +
 > +/**
 > + * smccc_call32() - make ARCH32 SMC calls
 > + * @param: values to pass in registers 0 to 7
 > + *
 > + * This function is used to make SMC calls following SMC Calling 
Convention
 > + * for ARCH32 calls. The content of the supplied param are copied to
 > + * registers 0 to 7 prior to the SMC instruction. Values a0..a3 are 
updated
 > + * with the content from register 0 to 3 on return from the SMC
 > + * instruction.
 > + */
 > +void smccc_call32(struct smccc_param32 *param);
 > +
 > +#endif /*__LINUX_ARM_SMCCC_H*/
 > --
 > 1.9.1
 >
 >
 > _______________________________________________
 > linux-arm-kernel mailing list
 > linux-arm-kernel@lists.infradead.org
 > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Will Deacon Aug. 19, 2015, 4:50 p.m. UTC | #2
On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
> Adds helpers to do SMC based on ARM SMC Calling Convention.
> CONFIG_HAVE_SMCCC is enabled for architectures that may support
> the SMC instruction. It's the responsibility of the caller to
> know if the SMC instruction is supported by the platform.

[...]
> diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> new file mode 100644
> index 0000000..3ce7fe8
> --- /dev/null
> +++ b/arch/arm64/kernel/smccc-call.S
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright (c) 2015, Linaro Limited
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License Version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include <linux/linkage.h>
> +
> +#define SMC_PARAM_W0_OFFS      0
> +#define SMC_PARAM_W2_OFFS      8
> +#define SMC_PARAM_W4_OFFS      16
> +#define SMC_PARAM_W6_OFFS      24
> +
> +/* void smccc_call32(struct smccc_param32 *param) */
> +ENTRY(smccc_call32)
> +       stp     x28, x30, [sp, #-16]!

Why are you saving lr?

> +       mov     x28, x0
> +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
> +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
> +       smc     #0
> +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> +       ldp     x28, x30, [sp], #16
> +       ret
> +ENDPROC(smccc_call32)

Could we deal with this like we do for PSCI instead? (see
__invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
and stick this in there too.

Will
Jens Wiklander Aug. 20, 2015, 7:23 a.m. UTC | #3
On Wed, Aug 19, 2015 at 01:56:39PM +0300, Yury wrote:
[...]
> > +++ b/include/linux/arm-smccc.h
> > @@ -0,0 +1,79 @@
> > +/*
> > + * Copyright (c) 2015, Linaro Limited
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +#ifndef __LINUX_ARM_SMCCC_H
> > +#define __LINUX_ARM_SMCCC_H
> > +
> > +#include <linux/types.h>
> > +
> > +/*
> > + * This file provideds defines common defines for ARM SMC Calling
> 
> typos here?

Thanks

> 
> > + * Convention as specified in
> > + * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
> > + */
> > +
> > +#define SMCCC_SMC_32			(0 << 30)
> 
> 0 << 30 is just 0, and so meaningless.
> It's better to introduce SMCCC_IS_32() macro instead.

SMCCC_SMC_32 is used as an argument for SMCCC_CALL_VAL (example in
"[PATCH v5 4/5] tee: add OP-TEE driver" drivers/tee/optee/optee_smc.h).
Shifting 0 here is still 0 as you point out, but it connects it with the
other define SMCCC_SMC_64.

> 
> > +#define SMCCC_SMC_64			(1 << 30)
> > +#define SMCCC_FAST_CALL			(1 << 31)
> > +#define SMCCC_STD_CALL			(0 << 31)
> 
> The same
> 
> > +
> > +#define SMCCC_OWNER_MASK		0x3F
> > +#define SMCCC_OWNER_SHIFT		24
> > +
> > +#define SMCCC_FUNC_MASK			0xFFFF
> > +
> > +#define SMCCC_IS_FAST_CALL(smc_val)	((smc_val) & SMCCC_FAST_CALL)
> > +#define SMCCC_IS_64(smc_val)		((smc_val) & SMCCC_SMC_64)
> > +#define SMCCC_FUNC_NUM(smc_val)		((smc_val) & SMCCC_FUNC_MASK)
> > +#define SMCCC_OWNER_NUM(smc_val) \
> > +	(((smc_val) >> SMCCC_OWNER_SHIFT) & SMCCC_OWNER_MASK)
> > +
> > +#define SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
> > +			((type) | (calling_convention) | \
> > +			(((owner) & SMCCC_OWNER_MASK) << SMCCC_OWNER_SHIFT) | \
> > +			((func_num) & SMCCC_FUNC_MASK))
> > +
> > +#define SMCCC_OWNER_ARCH		0
> > +#define SMCCC_OWNER_CPU			1
> > +#define SMCCC_OWNER_SIP			2
> > +#define SMCCC_OWNER_OEM			3
> > +#define SMCCC_OWNER_STANDARD		4
> > +#define SMCCC_OWNER_TRUSTED_APP		48
> > +#define SMCCC_OWNER_TRUSTED_APP_END	49
> > +#define SMCCC_OWNER_TRUSTED_OS		50
> > +#define SMCCC_OWNER_TRUSTED_OS_END	63
> > +
> > +struct smccc_param32 {
> > +	u32 a0;
> > +	u32 a1;
> > +	u32 a2;
> > +	u32 a3;
> > +	u32 a4;
> > +	u32 a5;
> > +	u32 a6;
> > +	u32 a7;
> > +};
> > +
> > +/**
> > + * smccc_call32() - make ARCH32 SMC calls
> > + * @param: values to pass in registers 0 to 7
> > + *
> > + * This function is used to make SMC calls following SMC Calling
> Convention
> > + * for ARCH32 calls. The content of the supplied param are copied to
> > + * registers 0 to 7 prior to the SMC instruction. Values a0..a3
> are updated
> > + * with the content from register 0 to 3 on return from the SMC
> > + * instruction.
> > + */
> > +void smccc_call32(struct smccc_param32 *param);
> > +
> > +#endif /*__LINUX_ARM_SMCCC_H*/
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
Thanks,
Jens
Michal Simek Aug. 20, 2015, 11:35 a.m. UTC | #4
On 08/19/2015 06:50 PM, Will Deacon wrote:
> On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
>> Adds helpers to do SMC based on ARM SMC Calling Convention.
>> CONFIG_HAVE_SMCCC is enabled for architectures that may support
>> the SMC instruction. It's the responsibility of the caller to
>> know if the SMC instruction is supported by the platform.
> 
> [...]
>> diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
>> new file mode 100644
>> index 0000000..3ce7fe8
>> --- /dev/null
>> +++ b/arch/arm64/kernel/smccc-call.S
>> @@ -0,0 +1,34 @@
>> +/*
>> + * Copyright (c) 2015, Linaro Limited
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License Version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +#include <linux/linkage.h>
>> +
>> +#define SMC_PARAM_W0_OFFS      0
>> +#define SMC_PARAM_W2_OFFS      8
>> +#define SMC_PARAM_W4_OFFS      16
>> +#define SMC_PARAM_W6_OFFS      24
>> +
>> +/* void smccc_call32(struct smccc_param32 *param) */
>> +ENTRY(smccc_call32)
>> +       stp     x28, x30, [sp, #-16]!
> 
> Why are you saving lr?
> 
>> +       mov     x28, x0
>> +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
>> +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
>> +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
>> +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
>> +       smc     #0
>> +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
>> +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
>> +       ldp     x28, x30, [sp], #16
>> +       ret
>> +ENDPROC(smccc_call32)
> 
> Could we deal with this like we do for PSCI instead? (see
> __invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
> and stick this in there too.

I think that make sense to make smc, hvc calling more generic. Remove
psci name from __invoke_psci_fn_smc and just use it in other drivers.

Thanks,
Michal
Jens Wiklander Aug. 20, 2015, 11:37 a.m. UTC | #5
On Wed, Aug 19, 2015 at 05:50:09PM +0100, Will Deacon wrote:
> On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
> > Adds helpers to do SMC based on ARM SMC Calling Convention.
> > CONFIG_HAVE_SMCCC is enabled for architectures that may support
> > the SMC instruction. It's the responsibility of the caller to
> > know if the SMC instruction is supported by the platform.
> 
> [...]
> > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> > new file mode 100644
> > index 0000000..3ce7fe8
> > --- /dev/null
> > +++ b/arch/arm64/kernel/smccc-call.S
> > @@ -0,0 +1,34 @@
> > +/*
> > + * Copyright (c) 2015, Linaro Limited
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License Version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +#include <linux/linkage.h>
> > +
> > +#define SMC_PARAM_W0_OFFS      0
> > +#define SMC_PARAM_W2_OFFS      8
> > +#define SMC_PARAM_W4_OFFS      16
> > +#define SMC_PARAM_W6_OFFS      24
> > +
> > +/* void smccc_call32(struct smccc_param32 *param) */
> > +ENTRY(smccc_call32)
> > +       stp     x28, x30, [sp, #-16]!
> 
> Why are you saving lr?

Agree, no point in saving lr, but I still need to decrease sp with 16 to
maintain correct alignment. I'll do it with an str instruction instead.

> 
> > +       mov     x28, x0
> > +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
> > +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
> > +       smc     #0
> > +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > +       ldp     x28, x30, [sp], #16
> > +       ret
> > +ENDPROC(smccc_call32)
> 
> Could we deal with this like we do for PSCI instead? (see
> __invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
> and stick this in there too.

I assume you're referring to when to use "hvc" and "smc".

I would rather consider smccc_call32() a primitive function that a
driver may use if it's configured to do so, for instance via DT. When an
"hvc" should be used instead of an "smc" up to the driver to decide
based how it's configured.

When merging psci-call.S and smccc-call.S into fw-call.S, what should I
do about smccc.c? I need to export the smccc_call32 function somewhere
as it could be used from a loadable module.

--
Thanks,
Jens
Mark Rutland Aug. 20, 2015, 9:30 p.m. UTC | #6
On Thu, Aug 20, 2015 at 12:37:29PM +0100, Jens Wiklander wrote:
> On Wed, Aug 19, 2015 at 05:50:09PM +0100, Will Deacon wrote:
> > On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
> > > Adds helpers to do SMC based on ARM SMC Calling Convention.
> > > CONFIG_HAVE_SMCCC is enabled for architectures that may support
> > > the SMC instruction. It's the responsibility of the caller to
> > > know if the SMC instruction is supported by the platform.

[...]

> > > +       mov     x28, x0
> > > +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
> > > +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
> > > +       smc     #0
> > > +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > +       ldp     x28, x30, [sp], #16
> > > +       ret
> > > +ENDPROC(smccc_call32)
> > 
> > Could we deal with this like we do for PSCI instead? (see
> > __invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
> > and stick this in there too.
> 
> I assume you're referring to when to use "hvc" and "smc".

I assume he's on about passing the values in registers rather than a struct.

From the looks of the SMC Calling Convention documentation, it's valid to have
return values in registers r0-r3, which necessitates the use of a struct (at
least for the return values).

Thanks,
Mark.
Will Deacon Aug. 21, 2015, 9:24 a.m. UTC | #7
On Thu, Aug 20, 2015 at 12:37:29PM +0100, Jens Wiklander wrote:
> On Wed, Aug 19, 2015 at 05:50:09PM +0100, Will Deacon wrote:
> > On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
> > > Adds helpers to do SMC based on ARM SMC Calling Convention.
> > > CONFIG_HAVE_SMCCC is enabled for architectures that may support
> > > the SMC instruction. It's the responsibility of the caller to
> > > know if the SMC instruction is supported by the platform.
> > 
> > [...]
> > > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> > > new file mode 100644
> > > index 0000000..3ce7fe8
> > > --- /dev/null
> > > +++ b/arch/arm64/kernel/smccc-call.S
> > > @@ -0,0 +1,34 @@
> > > +/*
> > > + * Copyright (c) 2015, Linaro Limited
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License Version 2 as
> > > + * published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU General Public License for more details.
> > > + *
> > > + */
> > > +#include <linux/linkage.h>
> > > +
> > > +#define SMC_PARAM_W0_OFFS      0
> > > +#define SMC_PARAM_W2_OFFS      8
> > > +#define SMC_PARAM_W4_OFFS      16
> > > +#define SMC_PARAM_W6_OFFS      24
> > > +
> > > +/* void smccc_call32(struct smccc_param32 *param) */
> > > +ENTRY(smccc_call32)
> > > +       stp     x28, x30, [sp, #-16]!
> > 
> > Why are you saving lr?
> 
> Agree, no point in saving lr, but I still need to decrease sp with 16 to
> maintain correct alignment. I'll do it with an str instruction instead.

That or pad out with xzr

> > 
> > > +       mov     x28, x0
> > > +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
> > > +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
> > > +       smc     #0
> > > +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > +       ldp     x28, x30, [sp], #16
> > > +       ret
> > > +ENDPROC(smccc_call32)
> > 
> > Could we deal with this like we do for PSCI instead? (see
> > __invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
> > and stick this in there too.
> 
> I assume you're referring to when to use "hvc" and "smc".

No, I mean use a C prototype to avoid marshalling the parameters in assembly
like this. As Rutland pointed out, the return value is a bit messy, but
the arguments align nicely with the PCS afaict.

Will
Jens Wiklander Aug. 21, 2015, 11:43 a.m. UTC | #8
On Fri, Aug 21, 2015 at 10:24:30AM +0100, Will Deacon wrote:
> On Thu, Aug 20, 2015 at 12:37:29PM +0100, Jens Wiklander wrote:
> > On Wed, Aug 19, 2015 at 05:50:09PM +0100, Will Deacon wrote:
> > > On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
> > > > Adds helpers to do SMC based on ARM SMC Calling Convention.
> > > > CONFIG_HAVE_SMCCC is enabled for architectures that may support
> > > > the SMC instruction. It's the responsibility of the caller to
> > > > know if the SMC instruction is supported by the platform.
> > > 
> > > [...]
> > > > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> > > > new file mode 100644
> > > > index 0000000..3ce7fe8
> > > > --- /dev/null
> > > > +++ b/arch/arm64/kernel/smccc-call.S
> > > > @@ -0,0 +1,34 @@
> > > > +/*
> > > > + * Copyright (c) 2015, Linaro Limited
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License Version 2 as
> > > > + * published by the Free Software Foundation.
> > > > + *
> > > > + * This program is distributed in the hope that it will be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > + * GNU General Public License for more details.
> > > > + *
> > > > + */
> > > > +#include <linux/linkage.h>
> > > > +
> > > > +#define SMC_PARAM_W0_OFFS      0
> > > > +#define SMC_PARAM_W2_OFFS      8
> > > > +#define SMC_PARAM_W4_OFFS      16
> > > > +#define SMC_PARAM_W6_OFFS      24
> > > > +
> > > > +/* void smccc_call32(struct smccc_param32 *param) */
> > > > +ENTRY(smccc_call32)
> > > > +       stp     x28, x30, [sp, #-16]!
> > > 
> > > Why are you saving lr?
> > 
> > Agree, no point in saving lr, but I still need to decrease sp with 16 to
> > maintain correct alignment. I'll do it with an str instruction instead.
> 
> That or pad out with xzr
> 
> > > 
> > > > +       mov     x28, x0
> > > > +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
> > > > +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
> > > > +       smc     #0
> > > > +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > +       ldp     x28, x30, [sp], #16
> > > > +       ret
> > > > +ENDPROC(smccc_call32)
> > > 
> > > Could we deal with this like we do for PSCI instead? (see
> > > __invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
> > > and stick this in there too.
> > 
> > I assume you're referring to when to use "hvc" and "smc".
> 
> No, I mean use a C prototype to avoid marshalling the parameters in assembly
> like this. As Rutland pointed out, the return value is a bit messy, but
> the arguments align nicely with the PCS afaict.

If possible I'd like the function to have the same prototype for both
arm and arm64. For arm it's not possible to supply more than 4
parameters. To fully support SMC Calling Convention we need to be able
to pass 8 parameters and have 4 return values. The OP-TEE driver in this
patch set depends on this. I don't see how we can avoid the marshalling
here.

We could have two versions of the SMCCC functions, one simplified which
only uses registers and one complete like this one with marshalling.

Thanks,
Jens
Jens Wiklander Sept. 14, 2015, 8:30 a.m. UTC | #9
On Fri, Aug 21, 2015 at 01:43:31PM +0200, Jens Wiklander wrote:
> On Fri, Aug 21, 2015 at 10:24:30AM +0100, Will Deacon wrote:
> > On Thu, Aug 20, 2015 at 12:37:29PM +0100, Jens Wiklander wrote:
> > > On Wed, Aug 19, 2015 at 05:50:09PM +0100, Will Deacon wrote:
> > > > On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
> > > > > Adds helpers to do SMC based on ARM SMC Calling Convention.
> > > > > CONFIG_HAVE_SMCCC is enabled for architectures that may support
> > > > > the SMC instruction. It's the responsibility of the caller to
> > > > > know if the SMC instruction is supported by the platform.
> > > > 
> > > > [...]
> > > > > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> > > > > new file mode 100644
> > > > > index 0000000..3ce7fe8
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/kernel/smccc-call.S
> > > > > @@ -0,0 +1,34 @@
> > > > > +/*
> > > > > + * Copyright (c) 2015, Linaro Limited
> > > > > + *
> > > > > + * This program is free software; you can redistribute it and/or modify
> > > > > + * it under the terms of the GNU General Public License Version 2 as
> > > > > + * published by the Free Software Foundation.
> > > > > + *
> > > > > + * This program is distributed in the hope that it will be useful,
> > > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > > > + * GNU General Public License for more details.
> > > > > + *
> > > > > + */
> > > > > +#include <linux/linkage.h>
> > > > > +
> > > > > +#define SMC_PARAM_W0_OFFS      0
> > > > > +#define SMC_PARAM_W2_OFFS      8
> > > > > +#define SMC_PARAM_W4_OFFS      16
> > > > > +#define SMC_PARAM_W6_OFFS      24
> > > > > +
> > > > > +/* void smccc_call32(struct smccc_param32 *param) */
> > > > > +ENTRY(smccc_call32)
> > > > > +       stp     x28, x30, [sp, #-16]!
> > > > 
> > > > Why are you saving lr?
> > > 
> > > Agree, no point in saving lr, but I still need to decrease sp with 16 to
> > > maintain correct alignment. I'll do it with an str instruction instead.
> > 
> > That or pad out with xzr
> > 
> > > > 
> > > > > +       mov     x28, x0
> > > > > +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > > +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > > +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
> > > > > +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
> > > > > +       smc     #0
> > > > > +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > > +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > > +       ldp     x28, x30, [sp], #16
> > > > > +       ret
> > > > > +ENDPROC(smccc_call32)
> > > > 
> > > > Could we deal with this like we do for PSCI instead? (see
> > > > __invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
> > > > and stick this in there too.
> > > 
> > > I assume you're referring to when to use "hvc" and "smc".
> > 
> > No, I mean use a C prototype to avoid marshalling the parameters in assembly
> > like this. As Rutland pointed out, the return value is a bit messy, but
> > the arguments align nicely with the PCS afaict.
> 
> If possible I'd like the function to have the same prototype for both
> arm and arm64. For arm it's not possible to supply more than 4
> parameters. To fully support SMC Calling Convention we need to be able
> to pass 8 parameters and have 4 return values. The OP-TEE driver in this
> patch set depends on this. I don't see how we can avoid the marshalling
> here.
> 
> We could have two versions of the SMCCC functions, one simplified which
> only uses registers and one complete like this one with marshalling.

Will, what do think about this?

Thanks,
Jens
Will Deacon Sept. 15, 2015, 6:26 p.m. UTC | #10
On Mon, Sep 14, 2015 at 09:30:30AM +0100, Jens Wiklander wrote:
> On Fri, Aug 21, 2015 at 01:43:31PM +0200, Jens Wiklander wrote:
> > On Fri, Aug 21, 2015 at 10:24:30AM +0100, Will Deacon wrote:
> > > On Thu, Aug 20, 2015 at 12:37:29PM +0100, Jens Wiklander wrote:
> > > > On Wed, Aug 19, 2015 at 05:50:09PM +0100, Will Deacon wrote:
> > > > > On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
> > > > > > +       mov     x28, x0
> > > > > > +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > > > +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > > > +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
> > > > > > +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
> > > > > > +       smc     #0
> > > > > > +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > > > +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > > > +       ldp     x28, x30, [sp], #16
> > > > > > +       ret
> > > > > > +ENDPROC(smccc_call32)
> > > > > 
> > > > > Could we deal with this like we do for PSCI instead? (see
> > > > > __invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
> > > > > and stick this in there too.
> > > > 
> > > > I assume you're referring to when to use "hvc" and "smc".
> > > 
> > > No, I mean use a C prototype to avoid marshalling the parameters in assembly
> > > like this. As Rutland pointed out, the return value is a bit messy, but
> > > the arguments align nicely with the PCS afaict.
> > 
> > If possible I'd like the function to have the same prototype for both
> > arm and arm64. For arm it's not possible to supply more than 4
> > parameters. To fully support SMC Calling Convention we need to be able
> > to pass 8 parameters and have 4 return values. The OP-TEE driver in this
> > patch set depends on this. I don't see how we can avoid the marshalling
> > here.
> > 
> > We could have two versions of the SMCCC functions, one simplified which
> > only uses registers and one complete like this one with marshalling.
> 
> Will, what do think about this?

I still think you should make use of a C prototype to avoid explicit
parameter marshalling in assembly. If you want to maintain a compatible
API between arm and arm64, then you can easily have an intermediate
function in arm64 that sits between the API entry point and the assembly.

Will
Jens Wiklander Sept. 15, 2015, 9:05 p.m. UTC | #11
On Tue, Sep 15, 2015 at 07:26:45PM +0100, Will Deacon wrote:
> On Mon, Sep 14, 2015 at 09:30:30AM +0100, Jens Wiklander wrote:
> > On Fri, Aug 21, 2015 at 01:43:31PM +0200, Jens Wiklander wrote:
> > > On Fri, Aug 21, 2015 at 10:24:30AM +0100, Will Deacon wrote:
> > > > On Thu, Aug 20, 2015 at 12:37:29PM +0100, Jens Wiklander wrote:
> > > > > On Wed, Aug 19, 2015 at 05:50:09PM +0100, Will Deacon wrote:
> > > > > > On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
> > > > > > > +       mov     x28, x0
> > > > > > > +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > > > > +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > > > > +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
> > > > > > > +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
> > > > > > > +       smc     #0
> > > > > > > +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > > > > +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > > > > +       ldp     x28, x30, [sp], #16
> > > > > > > +       ret
> > > > > > > +ENDPROC(smccc_call32)
> > > > > > 
> > > > > > Could we deal with this like we do for PSCI instead? (see
> > > > > > __invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
> > > > > > and stick this in there too.
> > > > > 
> > > > > I assume you're referring to when to use "hvc" and "smc".
> > > > 
> > > > No, I mean use a C prototype to avoid marshalling the parameters in assembly
> > > > like this. As Rutland pointed out, the return value is a bit messy, but
> > > > the arguments align nicely with the PCS afaict.
> > > 
> > > If possible I'd like the function to have the same prototype for both
> > > arm and arm64. For arm it's not possible to supply more than 4
> > > parameters. To fully support SMC Calling Convention we need to be able
> > > to pass 8 parameters and have 4 return values. The OP-TEE driver in this
> > > patch set depends on this. I don't see how we can avoid the marshalling
> > > here.
> > > 
> > > We could have two versions of the SMCCC functions, one simplified which
> > > only uses registers and one complete like this one with marshalling.
> > 
> > Will, what do think about this?
> 
> I still think you should make use of a C prototype to avoid explicit
> parameter marshalling in assembly. If you want to maintain a compatible
> API between arm and arm64, then you can easily have an intermediate
> function in arm64 that sits between the API entry point and the assembly.

Yes, I see how that's convenient for passing argument values in
registers, but that doesn't help with storing the returned values in
x0..x3 into something accessible in C. Am I missing something?

Thanks,
Jens
Will Deacon Sept. 16, 2015, 5:18 p.m. UTC | #12
On Tue, Sep 15, 2015 at 10:05:59PM +0100, Jens Wiklander wrote:
> On Tue, Sep 15, 2015 at 07:26:45PM +0100, Will Deacon wrote:
> > On Mon, Sep 14, 2015 at 09:30:30AM +0100, Jens Wiklander wrote:
> > > On Fri, Aug 21, 2015 at 01:43:31PM +0200, Jens Wiklander wrote:
> > > > On Fri, Aug 21, 2015 at 10:24:30AM +0100, Will Deacon wrote:
> > > > > On Thu, Aug 20, 2015 at 12:37:29PM +0100, Jens Wiklander wrote:
> > > > > > On Wed, Aug 19, 2015 at 05:50:09PM +0100, Will Deacon wrote:
> > > > > > > On Wed, Aug 19, 2015 at 09:40:25AM +0100, Jens Wiklander wrote:
> > > > > > > > +       mov     x28, x0
> > > > > > > > +       ldp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > > > > > +       ldp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > > > > > +       ldp     w4, w5, [x28, #SMC_PARAM_W4_OFFS]
> > > > > > > > +       ldp     w6, w7, [x28, #SMC_PARAM_W6_OFFS]
> > > > > > > > +       smc     #0
> > > > > > > > +       stp     w0, w1, [x28, #SMC_PARAM_W0_OFFS]
> > > > > > > > +       stp     w2, w3, [x28, #SMC_PARAM_W2_OFFS]
> > > > > > > > +       ldp     x28, x30, [sp], #16
> > > > > > > > +       ret
> > > > > > > > +ENDPROC(smccc_call32)
> > > > > > > 
> > > > > > > Could we deal with this like we do for PSCI instead? (see
> > > > > > > __invoke_psci_fn_smc). We could also then rename psci-call.S to fw-call.S
> > > > > > > and stick this in there too.
> > > > > > 
> > > > > > I assume you're referring to when to use "hvc" and "smc".
> > > > > 
> > > > > No, I mean use a C prototype to avoid marshalling the parameters in assembly
> > > > > like this. As Rutland pointed out, the return value is a bit messy, but
> > > > > the arguments align nicely with the PCS afaict.
> > > > 
> > > > If possible I'd like the function to have the same prototype for both
> > > > arm and arm64. For arm it's not possible to supply more than 4
> > > > parameters. To fully support SMC Calling Convention we need to be able
> > > > to pass 8 parameters and have 4 return values. The OP-TEE driver in this
> > > > patch set depends on this. I don't see how we can avoid the marshalling
> > > > here.
> > > > 
> > > > We could have two versions of the SMCCC functions, one simplified which
> > > > only uses registers and one complete like this one with marshalling.
> > > 
> > > Will, what do think about this?
> > 
> > I still think you should make use of a C prototype to avoid explicit
> > parameter marshalling in assembly. If you want to maintain a compatible
> > API between arm and arm64, then you can easily have an intermediate
> > function in arm64 that sits between the API entry point and the assembly.
> 
> Yes, I see how that's convenient for passing argument values in
> registers, but that doesn't help with storing the returned values in
> x0..x3 into something accessible in C. Am I missing something?

Your approach using a struct looked fine to me, just place it after the
parameters and use it only for the return values. Then __invoke_psci*
can wrap this and unpack r0/x0 from the structure.

Will
diff mbox

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 45df48b..75e4da3 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -221,6 +221,9 @@  config NEED_RET_TO_USER
 config ARCH_MTD_XIP
 	bool
 
+config HAVE_SMCCC
+	bool
+
 config VECTORS_BASE
 	hex
 	default 0xffff0000 if MMU || CPU_HIGH_VECTOR
@@ -324,6 +327,7 @@  config ARCH_MULTIPLATFORM
 	select CLKSRC_OF
 	select COMMON_CLK
 	select GENERIC_CLOCKEVENTS
+	select HAVE_SMCCC if CPU_V7
 	select MIGHT_HAVE_PCI
 	select MULTI_IRQ_HANDLER
 	select SPARSE_IRQ
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 752725d..8cdd25b 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -90,4 +90,6 @@  obj-y				+= psci.o psci-call.o
 obj-$(CONFIG_SMP)		+= psci_smp.o
 endif
 
+obj-$(CONFIG_HAVE_SMCCC)	+= smccc-call.o smccc.o
+
 extra-y := $(head-y) vmlinux.lds
diff --git a/arch/arm/kernel/smccc-call.S b/arch/arm/kernel/smccc-call.S
new file mode 100644
index 0000000..05bc554
--- /dev/null
+++ b/arch/arm/kernel/smccc-call.S
@@ -0,0 +1,26 @@ 
+/*
+ * Copyright (c) 2015, Linaro Limited
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <linux/linkage.h>
+
+#include <asm/opcodes-sec.h>
+
+/* void smccc_call32(struct smccc_param32 *param) */
+ENTRY(smccc_call32)
+	push	{r4-r8, lr}
+	mov	r8, r0
+	ldm	r8, {r0-r7}
+	__SMC(0)
+	stm	r8, {r0-r7}
+	pop	{r4-r8, pc}
+ENDPROC(smccc_call32)
diff --git a/arch/arm/kernel/smccc.c b/arch/arm/kernel/smccc.c
new file mode 100644
index 0000000..ba4039e
--- /dev/null
+++ b/arch/arm/kernel/smccc.c
@@ -0,0 +1,17 @@ 
+/*
+ * Copyright (c) 2015, Linaro Limited
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <linux/export.h>
+#include <linux/arm-smccc.h>
+
+EXPORT_SYMBOL_GPL(smccc_call32);
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7796af4..b3ea778 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -83,6 +83,7 @@  config ARM64
 	select SPARSE_IRQ
 	select SYSCTL_EXCEPTION_TRACE
 	select HAVE_CONTEXT_TRACKING
+	select HAVE_SMCCC
 	help
 	  ARM 64-bit (AArch64) Linux support.
 
@@ -146,6 +147,9 @@  config KERNEL_MODE_NEON
 config FIX_EARLYCON_MEM
 	def_bool y
 
+config HAVE_SMCCC
+	bool
+
 config PGTABLE_LEVELS
 	int
 	default 2 if ARM64_64K_PAGES && ARM64_VA_BITS_42
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 426d076..f7804f7 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -36,6 +36,7 @@  arm64-obj-$(CONFIG_EFI)			+= efi.o efi-stub.o efi-entry.o
 arm64-obj-$(CONFIG_PCI)			+= pci.o
 arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 arm64-obj-$(CONFIG_ACPI)		+= acpi.o
+arm64-obj-$(CONFIG_HAVE_SMCCC)		+= smccc-call.o smccc.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)
diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
new file mode 100644
index 0000000..3ce7fe8
--- /dev/null
+++ b/arch/arm64/kernel/smccc-call.S
@@ -0,0 +1,34 @@ 
+/*
+ * Copyright (c) 2015, Linaro Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License Version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <linux/linkage.h>
+
+#define SMC_PARAM_W0_OFFS	0
+#define SMC_PARAM_W2_OFFS	8
+#define SMC_PARAM_W4_OFFS	16
+#define SMC_PARAM_W6_OFFS	24
+
+/* void smccc_call32(struct smccc_param32 *param) */
+ENTRY(smccc_call32)
+	stp	x28, x30, [sp, #-16]!
+	mov	x28, x0
+	ldp	w0, w1, [x28, #SMC_PARAM_W0_OFFS]
+	ldp	w2, w3, [x28, #SMC_PARAM_W2_OFFS]
+	ldp	w4, w5, [x28, #SMC_PARAM_W4_OFFS]
+	ldp	w6, w7, [x28, #SMC_PARAM_W6_OFFS]
+	smc	#0
+	stp	w0, w1, [x28, #SMC_PARAM_W0_OFFS]
+	stp	w2, w3, [x28, #SMC_PARAM_W2_OFFS]
+	ldp	x28, x30, [sp], #16
+	ret
+ENDPROC(smccc_call32)
diff --git a/arch/arm64/kernel/smccc.c b/arch/arm64/kernel/smccc.c
new file mode 100644
index 0000000..ba4039e
--- /dev/null
+++ b/arch/arm64/kernel/smccc.c
@@ -0,0 +1,17 @@ 
+/*
+ * Copyright (c) 2015, Linaro Limited
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <linux/export.h>
+#include <linux/arm-smccc.h>
+
+EXPORT_SYMBOL_GPL(smccc_call32);
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
new file mode 100644
index 0000000..9b8d775
--- /dev/null
+++ b/include/linux/arm-smccc.h
@@ -0,0 +1,79 @@ 
+/*
+ * Copyright (c) 2015, Linaro Limited
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __LINUX_ARM_SMCCC_H
+#define __LINUX_ARM_SMCCC_H
+
+#include <linux/types.h>
+
+/*
+ * This file provideds defines common defines for ARM SMC Calling
+ * Convention as specified in
+ * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
+ */
+
+#define SMCCC_SMC_32			(0 << 30)
+#define SMCCC_SMC_64			(1 << 30)
+#define SMCCC_FAST_CALL			(1 << 31)
+#define SMCCC_STD_CALL			(0 << 31)
+
+#define SMCCC_OWNER_MASK		0x3F
+#define SMCCC_OWNER_SHIFT		24
+
+#define SMCCC_FUNC_MASK			0xFFFF
+
+#define SMCCC_IS_FAST_CALL(smc_val)	((smc_val) & SMCCC_FAST_CALL)
+#define SMCCC_IS_64(smc_val)		((smc_val) & SMCCC_SMC_64)
+#define SMCCC_FUNC_NUM(smc_val)		((smc_val) & SMCCC_FUNC_MASK)
+#define SMCCC_OWNER_NUM(smc_val) \
+	(((smc_val) >> SMCCC_OWNER_SHIFT) & SMCCC_OWNER_MASK)
+
+#define SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
+			((type) | (calling_convention) | \
+			(((owner) & SMCCC_OWNER_MASK) << SMCCC_OWNER_SHIFT) | \
+			((func_num) & SMCCC_FUNC_MASK))
+
+#define SMCCC_OWNER_ARCH		0
+#define SMCCC_OWNER_CPU			1
+#define SMCCC_OWNER_SIP			2
+#define SMCCC_OWNER_OEM			3
+#define SMCCC_OWNER_STANDARD		4
+#define SMCCC_OWNER_TRUSTED_APP		48
+#define SMCCC_OWNER_TRUSTED_APP_END	49
+#define SMCCC_OWNER_TRUSTED_OS		50
+#define SMCCC_OWNER_TRUSTED_OS_END	63
+
+struct smccc_param32 {
+	u32 a0;
+	u32 a1;
+	u32 a2;
+	u32 a3;
+	u32 a4;
+	u32 a5;
+	u32 a6;
+	u32 a7;
+};
+
+/**
+ * smccc_call32() - make ARCH32 SMC calls
+ * @param: values to pass in registers 0 to 7
+ *
+ * This function is used to make SMC calls following SMC Calling Convention
+ * for ARCH32 calls. The content of the supplied param are copied to
+ * registers 0 to 7 prior to the SMC instruction. Values a0..a3 are updated
+ * with the content from register 0 to 3 on return from the SMC
+ * instruction.
+ */
+void smccc_call32(struct smccc_param32 *param);
+
+#endif /*__LINUX_ARM_SMCCC_H*/