* [PATCH v5 0/9] Xen FF-A mediator
@ 2022-08-18 10:55 Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers Jens Wiklander
` (8 more replies)
0 siblings, 9 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:55 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander
Hi,
This patch sets add a FF-A [1] mediator modeled after the TEE mediator
already present in Xen. The FF-A mediator implements the subset of the FF-A
1.1 specification needed to communicate with OP-TEE using FF-A as transport
mechanism instead of SMC/HVC as with the TEE mediator. It allows a similar
design in OP-TEE as with the TEE mediator where OP-TEE presents one virtual
partition of itself to each guest in Xen.
The FF-A mediator is generic in the sense it has nothing OP-TEE specific
except that only the subset needed for OP-TEE is implemented so far. The
hooks needed to inform OP-TEE that a guest is created or destroyed is part
of the FF-A specification.
It should be possible to extend the FF-A mediator to implement a larger
portion of the FF-A 1.1 specification without breaking with the way OP-TEE
is communicated with here. So it should be possible to support any TEE or
Secure Partition using FF-A as transport with this mediator.
[1] https://developer.arm.com/documentation/den0077/latest
Thanks,
Jens
V4->v5:
* Added "xen/arm: move regpair_to_uint64() and uint64_to_regpair() to regs.h"
* Added documentation for the "ffa_enabled" guest config flag
* Changed to GPL license for xen/arch/arm/ffa.c
* Added __read_mostly and const where applicable
* Added more describing comments in the code
* Moved list of shared memory object ("ffa_mem_list") into the guest context
as they are guest specific
* Simplified a few of the simple wrapper functions for SMC to SPMC
* Added a BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE) since the mediator
currently depends on the page size to be same as FFA_PAGE_SIZE (4k).
* Added max number of shared memory object per guest and max number of
size of each shared memory object
* Added helper macros to calculate offsets of different FF-A data structures
in the communication buffer instead of relying on pointer arithmetic
* Addressed style issues and other comments
* Broke the commit "xen/arm: add FF-A mediator" into multiple parts, trying
to add a few features at a time as requested
* Added a missing call to rxtx_unmap() in ffa_relinquish_resources()
* Assignment of "ffa_enabled" is kept as is until I have something definitive
on the type etc.
* Tested with CONFIG_DEBUG=y
v3->v4:
* Missed v3 and sent a v4 instead by mistake.
v2->v3:
* Generates offsets into struct arm_smccc_1_2_regs with asm-offsets.c in
order to avoid hard coded offsets in the assembly function
arm_smccc_1_2_smc()
* Adds an entry in SUPPORT.md on the FF-A status
* Adds a configuration variable "ffa_enabled" to tell if FF-A should be
enabled for a particular domu guest
* Moves the ffa_frag_list for fragmented memory share requests into
struct ffa_ctx instead to keep it per guest in order to avoid mixups
and simplify locking
* Adds a spinlock to struct ffa_ctx for per guest locking
* Addressing style issues and suggestions
* Uses FFA_FEATURES to check that all the needed features are available
before initializing the mediator
* Rebased on staging as of 2022-06-20
v1->v2:
* Rebased on staging to resolve some merge conflicts as requested
Jens Wiklander (9):
xen/arm: smccc: add support for SMCCCv1.2 extended input/output
registers
xen/arm: move regpair_to_uint64() and uint64_to_regpair() to regs.h
xen/arm: add a primitive FF-A mediator
xen/arm: ffa: add direct request support
xen/arm: ffa: map SPMC rx/tx buffers
xen/arm: ffa: send guest events to Secure Partitions
xen/arm: ffa: support mapping guest RX/TX buffers
xen/arm: ffa: support guest FFA_PARTITION_INFO_GET
xen/arm: ffa: support sharing memory
SUPPORT.md | 7 +
docs/man/xl.cfg.5.pod.in | 15 +
tools/include/libxl.h | 6 +
tools/libs/light/libxl_arm.c | 3 +
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 3 +
xen/arch/arm/Kconfig | 11 +
xen/arch/arm/Makefile | 1 +
xen/arch/arm/arm64/asm-offsets.c | 9 +
xen/arch/arm/arm64/smc.S | 43 +
xen/arch/arm/domain.c | 10 +
xen/arch/arm/domain_build.c | 1 +
xen/arch/arm/ffa.c | 1805 +++++++++++++++++++++++++++++
xen/arch/arm/include/asm/domain.h | 4 +
xen/arch/arm/include/asm/ffa.h | 71 ++
xen/arch/arm/include/asm/regs.h | 12 +
xen/arch/arm/include/asm/smccc.h | 40 +
xen/arch/arm/tee/optee.c | 11 -
xen/arch/arm/vsmc.c | 19 +-
xen/include/public/arch-arm.h | 2 +
20 files changed, 2059 insertions(+), 15 deletions(-)
create mode 100644 xen/arch/arm/ffa.c
create mode 100644 xen/arch/arm/include/asm/ffa.h
--
2.31.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
@ 2022-08-18 10:55 ` Jens Wiklander
2022-08-18 13:44 ` Bertrand Marquis
2022-08-23 6:31 ` Jiamei Xie
2022-08-18 10:55 ` [PATCH v5 2/9] xen/arm: move regpair_to_uint64() and uint64_to_regpair() to regs.h Jens Wiklander
` (7 subsequent siblings)
8 siblings, 2 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:55 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander, Luca Fancellu
SMCCC v1.2 [1] AArch64 allows x0-x17 to be used as both parameter
registers and result registers for the SMC and HVC instructions.
Arm Firmware Framework for Armv8-A specification makes use of x0-x7 as
parameter and result registers.
Let us add new interface to support this extended set of input/output
registers.
This is based on 3fdc0cb59d97 ("arm64: smccc: Add support for SMCCCv1.2
extended input/output registers") by Sudeep Holla from the Linux kernel
The SMCCC version reported to the VM is bumped to 1.2 in order to support
handling FF-A messages.
[1] https://developer.arm.com/documentation/den0028/c/?lang=en
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
xen/arch/arm/arm64/asm-offsets.c | 9 +++++++
xen/arch/arm/arm64/smc.S | 43 ++++++++++++++++++++++++++++++++
xen/arch/arm/include/asm/smccc.h | 40 +++++++++++++++++++++++++++++
xen/arch/arm/vsmc.c | 2 +-
4 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
index 280ddb55bfd4..1721e1ed26e1 100644
--- a/xen/arch/arm/arm64/asm-offsets.c
+++ b/xen/arch/arm/arm64/asm-offsets.c
@@ -56,6 +56,15 @@ void __dummy__(void)
BLANK();
OFFSET(SMCCC_RES_a0, struct arm_smccc_res, a0);
OFFSET(SMCCC_RES_a2, struct arm_smccc_res, a2);
+ OFFSET(ARM_SMCCC_1_2_REGS_X0_OFFS, struct arm_smccc_1_2_regs, a0);
+ OFFSET(ARM_SMCCC_1_2_REGS_X2_OFFS, struct arm_smccc_1_2_regs, a2);
+ OFFSET(ARM_SMCCC_1_2_REGS_X4_OFFS, struct arm_smccc_1_2_regs, a4);
+ OFFSET(ARM_SMCCC_1_2_REGS_X6_OFFS, struct arm_smccc_1_2_regs, a6);
+ OFFSET(ARM_SMCCC_1_2_REGS_X8_OFFS, struct arm_smccc_1_2_regs, a8);
+ OFFSET(ARM_SMCCC_1_2_REGS_X10_OFFS, struct arm_smccc_1_2_regs, a10);
+ OFFSET(ARM_SMCCC_1_2_REGS_X12_OFFS, struct arm_smccc_1_2_regs, a12);
+ OFFSET(ARM_SMCCC_1_2_REGS_X14_OFFS, struct arm_smccc_1_2_regs, a14);
+ OFFSET(ARM_SMCCC_1_2_REGS_X16_OFFS, struct arm_smccc_1_2_regs, a16);
}
/*
diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
index 91bae62dd4d2..c546192e7f2d 100644
--- a/xen/arch/arm/arm64/smc.S
+++ b/xen/arch/arm/arm64/smc.S
@@ -27,3 +27,46 @@ ENTRY(__arm_smccc_1_0_smc)
stp x2, x3, [x4, #SMCCC_RES_a2]
1:
ret
+
+
+/*
+ * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
+ * struct arm_smccc_1_2_regs *res)
+ */
+ENTRY(arm_smccc_1_2_smc)
+ /* Save `res` and free a GPR that won't be clobbered */
+ stp x1, x19, [sp, #-16]!
+
+ /* Ensure `args` won't be clobbered while loading regs in next step */
+ mov x19, x0
+
+ /* Load the registers x0 - x17 from the struct arm_smccc_1_2_regs */
+ ldp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
+ ldp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
+ ldp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
+ ldp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
+ ldp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
+ ldp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
+ ldp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
+ ldp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
+ ldp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
+
+ smc #0
+
+ /* Load the `res` from the stack */
+ ldr x19, [sp]
+
+ /* Store the registers x0 - x17 into the result structure */
+ stp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
+ stp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
+ stp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
+ stp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
+ stp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
+ stp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
+ stp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
+ stp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
+ stp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
+
+ /* Restore original x19 */
+ ldp xzr, x19, [sp], #16
+ ret
diff --git a/xen/arch/arm/include/asm/smccc.h b/xen/arch/arm/include/asm/smccc.h
index b3dbeecc90ad..b5e3f67eb34e 100644
--- a/xen/arch/arm/include/asm/smccc.h
+++ b/xen/arch/arm/include/asm/smccc.h
@@ -33,6 +33,7 @@
#define ARM_SMCCC_VERSION_1_0 SMCCC_VERSION(1, 0)
#define ARM_SMCCC_VERSION_1_1 SMCCC_VERSION(1, 1)
+#define ARM_SMCCC_VERSION_1_2 SMCCC_VERSION(1, 2)
/*
* This file provides common defines for ARM SMC Calling Convention as
@@ -265,6 +266,45 @@ void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2,
else \
arm_smccc_1_0_smc(__VA_ARGS__); \
} while ( 0 )
+
+/**
+ * struct arm_smccc_1_2_regs - Arguments for or Results from SMC call
+ * @a0-a17 argument values from registers 0 to 17
+ */
+struct arm_smccc_1_2_regs {
+ unsigned long a0;
+ unsigned long a1;
+ unsigned long a2;
+ unsigned long a3;
+ unsigned long a4;
+ unsigned long a5;
+ unsigned long a6;
+ unsigned long a7;
+ unsigned long a8;
+ unsigned long a9;
+ unsigned long a10;
+ unsigned long a11;
+ unsigned long a12;
+ unsigned long a13;
+ unsigned long a14;
+ unsigned long a15;
+ unsigned long a16;
+ unsigned long a17;
+};
+
+/**
+ * arm_smccc_1_2_smc() - make SMC calls
+ * @args: arguments passed via struct arm_smccc_1_2_regs
+ * @res: result values via struct arm_smccc_1_2_regs
+ *
+ * This function is used to make SMC calls following SMC Calling Convention
+ * v1.2 or above. The content of the supplied param are copied from the
+ * structure to registers prior to the SMC instruction. The return values
+ * are updated with the content from registers on return from the SMC
+ * instruction.
+ */
+void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
+ struct arm_smccc_1_2_regs *res);
#endif /* CONFIG_ARM_64 */
#endif /* __ASSEMBLY__ */
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 676740ef1520..6f90c08a6304 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -93,7 +93,7 @@ static bool handle_arch(struct cpu_user_regs *regs)
switch ( fid )
{
case ARM_SMCCC_VERSION_FID:
- set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_1);
+ set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_2);
return true;
case ARM_SMCCC_ARCH_FEATURES_FID:
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 2/9] xen/arm: move regpair_to_uint64() and uint64_to_regpair() to regs.h
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers Jens Wiklander
@ 2022-08-18 10:55 ` Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator Jens Wiklander
` (6 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:55 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander
Moves the two helper functions regpair_to_uint64() and
uint64_to_regpair() from xen/arch/arm/tee/optee.c to the common arm
specific regs.h.
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
xen/arch/arm/include/asm/regs.h | 12 ++++++++++++
xen/arch/arm/tee/optee.c | 11 -----------
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/include/asm/regs.h b/xen/arch/arm/include/asm/regs.h
index 794721a103a4..977fc3c07f4a 100644
--- a/xen/arch/arm/include/asm/regs.h
+++ b/xen/arch/arm/include/asm/regs.h
@@ -60,6 +60,18 @@ static inline bool guest_mode(const struct cpu_user_regs *r)
register_t get_user_reg(struct cpu_user_regs *regs, int reg);
void set_user_reg(struct cpu_user_regs *regs, int reg, register_t val);
+static inline uint64_t regpair_to_uint64(register_t reg0, register_t reg1)
+{
+ return ((uint64_t)reg0 << 32) | (uint32_t)reg1;
+}
+
+static inline void uint64_to_regpair(register_t *reg0, register_t *reg1,
+ uint64_t val)
+{
+ *reg0 = val >> 32;
+ *reg1 = (uint32_t)val;
+}
+
#endif
#endif /* __ARM_REGS_H__ */
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c
index 9cb9f16d43cb..47027ecef47c 100644
--- a/xen/arch/arm/tee/optee.c
+++ b/xen/arch/arm/tee/optee.c
@@ -268,17 +268,6 @@ static int optee_domain_init(struct domain *d)
return 0;
}
-static uint64_t regpair_to_uint64(register_t reg0, register_t reg1)
-{
- return ((uint64_t)reg0 << 32) | (uint32_t)reg1;
-}
-
-static void uint64_to_regpair(register_t *reg0, register_t *reg1, uint64_t val)
-{
- *reg0 = val >> 32;
- *reg1 = (uint32_t)val;
-}
-
static struct page_info *get_domain_ram_page(gfn_t gfn)
{
struct page_info *page;
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 2/9] xen/arm: move regpair_to_uint64() and uint64_to_regpair() to regs.h Jens Wiklander
@ 2022-08-18 10:55 ` Jens Wiklander
2022-09-05 22:17 ` Julien Grall
` (2 more replies)
2022-08-18 10:55 ` [PATCH v5 4/9] xen/arm: ffa: add direct request support Jens Wiklander
` (5 subsequent siblings)
8 siblings, 3 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:55 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander
Adds a FF-A version 1.1 [1] mediator to communicate with a Secure
Partition in secure world.
This commit brings in only the parts needed to negotiate FF-A version
number with guest and SPMC.
A guest configuration variable "ffa_enabled" is used to indicate if a guest
is trusted to use FF-A.
This is loosely based on the TEE mediator framework and the OP-TEE
mediator.
[1] https://developer.arm.com/documentation/den0077/latest
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
SUPPORT.md | 7 +
docs/man/xl.cfg.5.pod.in | 15 ++
tools/include/libxl.h | 6 +
tools/libs/light/libxl_arm.c | 3 +
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 3 +
xen/arch/arm/Kconfig | 11 +
xen/arch/arm/Makefile | 1 +
xen/arch/arm/domain.c | 10 +
xen/arch/arm/domain_build.c | 1 +
xen/arch/arm/ffa.c | 354 ++++++++++++++++++++++++++++++
xen/arch/arm/include/asm/domain.h | 4 +
xen/arch/arm/include/asm/ffa.h | 71 ++++++
xen/arch/arm/vsmc.c | 17 +-
xen/include/public/arch-arm.h | 2 +
15 files changed, 503 insertions(+), 3 deletions(-)
create mode 100644 xen/arch/arm/ffa.c
create mode 100644 xen/arch/arm/include/asm/ffa.h
diff --git a/SUPPORT.md b/SUPPORT.md
index 70e98964cbc0..215bb3c9043b 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -785,6 +785,13 @@ that covers the DMA of the device to be passed through.
No support for QEMU backends in a 16K or 64K domain.
+### ARM: Firmware Framework for Arm A-profile (FF-A) Mediator
+
+ Status, Arm64: Tech Preview
+
+There are still some code paths where a vCPU may hog a pCPU longer than
+necessary. The FF-A mediator is not yet implemented for Arm32.
+
### ARM: Guest Device Tree support
Status: Supported
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index b98d1613987e..234c036aecb1 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -1616,6 +1616,21 @@ This feature is a B<technology preview>.
=back
+=item B<ffa_enabled=BOOLEAN>
+
+B<Arm only.> Allow a guest to communicate via FF-A with Secure Partitions
+(SP), default false.
+
+Currently is only a small subset of the FF-A specification supported. Just
+enough to communicate with OP-TEE. In general all the basic things and
+sharing memory with one SP. More advanced use cases where memory might be
+shared or donated to multple SPs is not supported.
+
+See L<https://developer.arm.com/documentation/den0077/latest> for more
+informantion about FF-A.
+
+This feature is a B<technology preview>.
+
=head2 Paravirtualised (PV) Guest Specific Options
The following options apply only to Paravirtual (PV) guests.
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 7ce978e83c9a..4ab5a7b044d6 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -278,6 +278,12 @@
*/
#define LIBXL_HAVE_BUILDINFO_ARCH_ARM_TEE 1
+/*
+ * LIBXL_HAVE_BUILDINFO_ARM_FFA_ENABLED indicates that
+ * libxl_domain_build_info has the arm.ffa_enabled field.
+ */
+#define LIBXL_HAVE_BUILDINFO_ARM_FFA_ENABLED 1
+
/*
* LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing
* 'soft reset' for domains and there is 'soft_reset' shutdown reason
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index eef1de093914..a985609861c7 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -101,6 +101,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
return ERROR_FAIL;
}
+ config->arch.ffa_enabled =
+ libxl_defbool_val(d_config->b_info.arch_arm.ffa_enabled);
+
return 0;
}
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 2a42da2f7d78..bf4544bef399 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -646,6 +646,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
("vuart", libxl_vuart_type),
+ ("ffa_enabled", libxl_defbool),
])),
("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
])),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index b98c0de378b6..e0e99ed8d2b1 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2746,6 +2746,9 @@ skip_usbdev:
exit(-ERROR_FAIL);
}
}
+ libxl_defbool_setdefault(&b_info->arch_arm.ffa_enabled, false);
+ xlu_cfg_get_defbool(config, "ffa_enabled",
+ &b_info->arch_arm.ffa_enabled, 0);
parse_vkb_list(config, d_config);
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index be9eff014120..e57e1d3757e2 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -139,6 +139,17 @@ config TEE
source "arch/arm/tee/Kconfig"
+config FFA
+ bool "Enable FF-A mediator support" if EXPERT
+ default n
+ depends on ARM_64
+ help
+ This option enables a minimal FF-A mediator. The mediator is
+ generic as it follows the FF-A specification [1], but it only
+ implements a small subset of the specification.
+
+ [1] https://developer.arm.com/documentation/den0077/latest
+
endmenu
menu "ARM errata workaround via the alternative framework"
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index bb7a6151c13c..af0c69f793d4 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -20,6 +20,7 @@ obj-y += domain_build.init.o
obj-y += domctl.o
obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
obj-y += efi/
+obj-$(CONFIG_FFA) += ffa.o
obj-y += gic.o
obj-y += gic-v2.o
obj-$(CONFIG_GICV3) += gic-v3.o
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 8110c1df8638..a3f00e7e234d 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -27,6 +27,7 @@
#include <asm/cpufeature.h>
#include <asm/current.h>
#include <asm/event.h>
+#include <asm/ffa.h>
#include <asm/gic.h>
#include <asm/guest_atomics.h>
#include <asm/irq.h>
@@ -756,6 +757,9 @@ int arch_domain_create(struct domain *d,
if ( (rc = tee_domain_init(d, config->arch.tee_type)) != 0 )
goto fail;
+ if ( (rc = ffa_domain_init(d, config->arch.ffa_enabled)) != 0 )
+ goto fail;
+
update_domain_wallclock_time(d);
/*
@@ -998,6 +1002,7 @@ static int relinquish_memory(struct domain *d, struct page_list_head *list)
enum {
PROG_pci = 1,
PROG_tee,
+ PROG_ffa,
PROG_xen,
PROG_page,
PROG_mapping,
@@ -1043,6 +1048,11 @@ int domain_relinquish_resources(struct domain *d)
PROGRESS(tee):
ret = tee_relinquish_resources(d);
+ if ( ret )
+ return ret;
+
+ PROGRESS(ffa):
+ ret = ffa_relinquish_resources(d);
if (ret )
return ret;
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 7ddd16c26da5..d708f76356f7 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3450,6 +3450,7 @@ void __init create_dom0(void)
if ( gic_number_lines() > 992 )
printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
dom0_cfg.arch.tee_type = tee_get_type();
+ dom0_cfg.arch.ffa_enabled = true;
dom0_cfg.max_vcpus = dom0_max_vcpus();
if ( iommu_enabled )
diff --git a/xen/arch/arm/ffa.c b/xen/arch/arm/ffa.c
new file mode 100644
index 000000000000..b85c492928d2
--- /dev/null
+++ b/xen/arch/arm/ffa.c
@@ -0,0 +1,354 @@
+/*
+ * xen/arch/arm/ffa.c
+ *
+ * Arm Firmware Framework for ARMv8-A (FF-A) mediator
+ *
+ * Copyright (C) 2022 Linaro Limited
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions 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.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/domain_page.h>
+#include <xen/errno.h>
+#include <xen/init.h>
+#include <xen/lib.h>
+#include <xen/sched.h>
+#include <xen/types.h>
+#include <xen/sizes.h>
+#include <xen/bitops.h>
+
+#include <asm/smccc.h>
+#include <asm/event.h>
+#include <asm/ffa.h>
+#include <asm/regs.h>
+
+/* Error codes */
+#define FFA_RET_OK 0
+#define FFA_RET_NOT_SUPPORTED -1
+#define FFA_RET_INVALID_PARAMETERS -2
+#define FFA_RET_NO_MEMORY -3
+#define FFA_RET_BUSY -4
+#define FFA_RET_INTERRUPTED -5
+#define FFA_RET_DENIED -6
+#define FFA_RET_RETRY -7
+#define FFA_RET_ABORTED -8
+
+/* FFA_VERSION helpers */
+#define FFA_VERSION_MAJOR_SHIFT 16U
+#define FFA_VERSION_MAJOR_MASK 0x7FFFU
+#define FFA_VERSION_MINOR_SHIFT 0U
+#define FFA_VERSION_MINOR_MASK 0xFFFFU
+#define MAKE_FFA_VERSION(major, minor) \
+ ((((major) & FFA_VERSION_MAJOR_MASK) << FFA_VERSION_MAJOR_SHIFT) | \
+ ((minor) & FFA_VERSION_MINOR_MASK))
+
+#define FFA_MIN_VERSION MAKE_FFA_VERSION(1, 0)
+#define FFA_VERSION_1_0 MAKE_FFA_VERSION(1, 0)
+#define FFA_VERSION_1_1 MAKE_FFA_VERSION(1, 1)
+
+/*
+ * This is the version we want to use in communication with guests and SPs.
+ * During negotiation with a guest or a SP we may need to lower it for
+ * that particular guest or SP.
+ */
+#define FFA_MY_VERSION_MAJOR 1U
+#define FFA_MY_VERSION_MINOR 1U
+#define FFA_MY_VERSION MAKE_FFA_VERSION(FFA_MY_VERSION_MAJOR, \
+ FFA_MY_VERSION_MINOR)
+
+#define FFA_PAGE_SIZE SZ_4K
+
+/*
+ * Limit for shared buffer size. Please note that this define limits
+ * number of pages. But user buffer can be not aligned to a page
+ * boundary. So it is possible that user would not be able to share
+ * exactly MAX_SHM_BUFFER_PG * PAGE_SIZE bytes.
+ *
+ * FF-A doesn't have any direct requirments on GlobalPlatform or vice
+ * versa, but an implementation can very well use FF-A in order to provide
+ * a GlobalPlatform interface on top.
+ *
+ * Global Platform specification for TEE requires that any TEE
+ * implementation should allow to share buffers with size of at least
+ * 512KB, which equals to 128 4kB pages. Due to align issue mentioned
+ * above, we need to increase this value to 129.
+ */
+#define FFA_MAX_SHM_PAGE_COUNT 129
+
+/*
+ * Limits the number of shared buffers that guest can have at once. This
+ * is to prevent case, when guests tricks XEN into exhausting its own
+ * memory by allocating many small buffers. This value has been chosen
+ * arbitrary.
+ */
+#define FFA_MAX_SHM_COUNT 32
+
+#define FFA_HANDLE_HYP_FLAG BIT(63, ULL)
+
+/* Memory attributes: Normal memory, Write-Back cacheable, Inner shareable */
+#define FFA_NORMAL_MEM_REG_ATTR 0x2fU
+
+/* Memory access permissions: Read-write */
+#define FFA_MEM_ACC_RW 0x2U
+
+/* Clear memory before mapping in receiver */
+#define FFA_MEMORY_REGION_FLAG_CLEAR BIT(0, U)
+/* Relayer may time slice this operation */
+#define FFA_MEMORY_REGION_FLAG_TIME_SLICE BIT(1, U)
+/* Clear memory after receiver relinquishes it */
+#define FFA_MEMORY_REGION_FLAG_CLEAR_RELINQUISH BIT(2, U)
+
+/* Share memory transaction */
+#define FFA_MEMORY_REGION_TRANSACTION_TYPE_SHARE (1U << 3)
+
+#define FFA_HANDLE_INVALID 0xffffffffffffffffULL
+
+/* Framework direct request/response */
+#define FFA_MSG_FLAG_FRAMEWORK BIT(31, U)
+#define FFA_MSG_TYPE_MASK 0xFFU;
+#define FFA_MSG_PSCI 0x0U
+#define FFA_MSG_SEND_VM_CREATED 0x4U
+#define FFA_MSG_RESP_VM_CREATED 0x5U
+#define FFA_MSG_SEND_VM_DESTROYED 0x6U
+#define FFA_MSG_RESP_VM_DESTROYED 0x7U
+
+/*
+ * Flags used for the FFA_PARTITION_INFO_GET return message:
+ * BIT(0): Supports receipt of direct requests
+ * BIT(1): Can send direct requests
+ * BIT(2): Can send and receive indirect messages
+ * BIT(3): Supports receipt of notifications
+ * BIT(4-5): Partition ID is a PE endpoint ID
+ */
+#define FFA_PART_PROP_DIRECT_REQ_RECV BIT(0, U)
+#define FFA_PART_PROP_DIRECT_REQ_SEND BIT(1, U)
+#define FFA_PART_PROP_INDIRECT_MSGS BIT(2, U)
+#define FFA_PART_PROP_RECV_NOTIF BIT(3, U)
+#define FFA_PART_PROP_IS_PE_ID (0U << 4)
+#define FFA_PART_PROP_IS_SEPID_INDEP (1U << 4)
+#define FFA_PART_PROP_IS_SEPID_DEP (2U << 4)
+#define FFA_PART_PROP_IS_AUX_ID (3U << 4)
+#define FFA_PART_PROP_NOTIF_CREATED BIT(6, U)
+#define FFA_PART_PROP_NOTIF_DESTROYED BIT(7, U)
+#define FFA_PART_PROP_AARCH64_STATE BIT(8, U)
+
+/*
+ * Flag used as parameter to FFA_PARTITION_INFO_GET to return partition
+ * count only.
+ */
+#define FFA_PARTITION_INFO_GET_COUNT_FLAG BIT(0, U)
+
+/* Function IDs */
+#define FFA_ERROR 0x84000060U
+#define FFA_SUCCESS_32 0x84000061U
+#define FFA_SUCCESS_64 0xC4000061U
+#define FFA_INTERRUPT 0x84000062U
+#define FFA_VERSION 0x84000063U
+#define FFA_FEATURES 0x84000064U
+#define FFA_RX_ACQUIRE 0x84000084U
+#define FFA_RX_RELEASE 0x84000065U
+#define FFA_RXTX_MAP_32 0x84000066U
+#define FFA_RXTX_MAP_64 0xC4000066U
+#define FFA_RXTX_UNMAP 0x84000067U
+#define FFA_PARTITION_INFO_GET 0x84000068U
+#define FFA_ID_GET 0x84000069U
+#define FFA_SPM_ID_GET 0x84000085U
+#define FFA_MSG_WAIT 0x8400006BU
+#define FFA_MSG_YIELD 0x8400006CU
+#define FFA_MSG_RUN 0x8400006DU
+#define FFA_MSG_SEND2 0x84000086U
+#define FFA_MSG_SEND_DIRECT_REQ_32 0x8400006FU
+#define FFA_MSG_SEND_DIRECT_REQ_64 0xC400006FU
+#define FFA_MSG_SEND_DIRECT_RESP_32 0x84000070U
+#define FFA_MSG_SEND_DIRECT_RESP_64 0xC4000070U
+#define FFA_MEM_DONATE_32 0x84000071U
+#define FFA_MEM_DONATE_64 0xC4000071U
+#define FFA_MEM_LEND_32 0x84000072U
+#define FFA_MEM_LEND_64 0xC4000072U
+#define FFA_MEM_SHARE_32 0x84000073U
+#define FFA_MEM_SHARE_64 0xC4000073U
+#define FFA_MEM_RETRIEVE_REQ_32 0x84000074U
+#define FFA_MEM_RETRIEVE_REQ_64 0xC4000074U
+#define FFA_MEM_RETRIEVE_RESP 0x84000075U
+#define FFA_MEM_RELINQUISH 0x84000076U
+#define FFA_MEM_RECLAIM 0x84000077U
+#define FFA_MEM_FRAG_RX 0x8400007AU
+#define FFA_MEM_FRAG_TX 0x8400007BU
+#define FFA_MSG_SEND 0x8400006EU
+#define FFA_MSG_POLL 0x8400006AU
+
+struct ffa_ctx {
+ uint32_t guest_vers;
+};
+
+/* Negotiated FF-A version to use with the SPMC */
+static uint32_t ffa_version __read_mostly;
+
+static bool ffa_get_version(uint32_t *vers)
+{
+ const struct arm_smccc_1_2_regs arg = {
+ .a0 = FFA_VERSION,
+ .a1 = FFA_MY_VERSION,
+ };
+ struct arm_smccc_1_2_regs resp;
+
+ arm_smccc_1_2_smc(&arg, &resp);
+ if ( resp.a0 == FFA_RET_NOT_SUPPORTED )
+ {
+ printk(XENLOG_ERR "ffa: FFA_VERSION returned not supported\n");
+ return false;
+ }
+
+ *vers = resp.a0;
+
+ return true;
+}
+
+static u16 get_vm_id(const struct domain *d)
+{
+ /* +1 since 0 is reserved for the hypervisor in FF-A */
+ return d->domain_id + 1;
+}
+
+static void set_regs(struct cpu_user_regs *regs, register_t v0, register_t v1,
+ register_t v2, register_t v3, register_t v4, register_t v5,
+ register_t v6, register_t v7)
+{
+ set_user_reg(regs, 0, v0);
+ set_user_reg(regs, 1, v1);
+ set_user_reg(regs, 2, v2);
+ set_user_reg(regs, 3, v3);
+ set_user_reg(regs, 4, v4);
+ set_user_reg(regs, 5, v5);
+ set_user_reg(regs, 6, v6);
+ set_user_reg(regs, 7, v7);
+}
+
+static void set_regs_success(struct cpu_user_regs *regs, uint32_t w2,
+ uint32_t w3)
+{
+ set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, 0, 0, 0, 0);
+}
+
+static void handle_version(struct cpu_user_regs *regs)
+{
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+ uint32_t vers = get_user_reg(regs, 1);
+
+ if ( vers < FFA_VERSION_1_1 )
+ vers = FFA_VERSION_1_0;
+ else
+ vers = FFA_VERSION_1_1;
+
+ ctx->guest_vers = vers;
+ set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
+}
+
+bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
+{
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+
+ if ( !ctx )
+ return false;
+
+ switch ( fid )
+ {
+ case FFA_VERSION:
+ handle_version(regs);
+ return true;
+ case FFA_ID_GET:
+ set_regs_success(regs, get_vm_id(d), 0);
+ return true;
+
+ default:
+ printk(XENLOG_ERR "ffa: unhandled fid 0x%x\n", fid);
+ return false;
+ }
+}
+
+int ffa_domain_init(struct domain *d, bool ffa_enabled)
+{
+ struct ffa_ctx *ctx;
+
+ if ( !ffa_version || !ffa_enabled )
+ return 0;
+
+ ctx = xzalloc(struct ffa_ctx);
+ if ( !ctx )
+ return -ENOMEM;
+
+ d->arch.ffa = ctx;
+
+ return 0;
+}
+
+int ffa_relinquish_resources(struct domain *d)
+{
+ struct ffa_ctx *ctx = d->arch.ffa;
+
+ if ( !ctx )
+ return 0;
+
+ XFREE(d->arch.ffa);
+
+ return 0;
+}
+
+static int __init ffa_init(void)
+{
+ uint32_t vers;
+ unsigned int major_vers;
+ unsigned int minor_vers;
+
+ /*
+ * FFA_PAGE_SIZE is defined to 4k and we're currently depending on
+ * using that page size.
+ */
+ BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
+
+ /*
+ * psci_init_smccc() updates this value with what's reported by EL-3
+ * or secure world.
+ */
+ if ( smccc_ver < ARM_SMCCC_VERSION_1_2 )
+ {
+ printk(XENLOG_ERR
+ "ffa: unsupported SMCCC version %#x (need at least %#x)\n",
+ smccc_ver, ARM_SMCCC_VERSION_1_2);
+ return 0;
+ }
+
+ if ( !ffa_get_version(&vers) )
+ return 0;
+
+ if ( vers < FFA_MIN_VERSION || vers > FFA_MY_VERSION )
+ {
+ printk(XENLOG_ERR "ffa: Incompatible version %#x found\n", vers);
+ return 0;
+ }
+
+ major_vers = (vers >> FFA_VERSION_MAJOR_SHIFT) & FFA_VERSION_MAJOR_MASK;
+ minor_vers = vers & FFA_VERSION_MINOR_MASK;
+ printk(XENLOG_INFO "ARM FF-A Mediator version %u.%u\n",
+ FFA_MY_VERSION_MAJOR, FFA_MY_VERSION_MINOR);
+ printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
+ major_vers, minor_vers);
+
+ ffa_version = vers;
+
+ return 0;
+}
+
+__initcall(ffa_init);
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index ed63c2b6f91f..b3dee269bced 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -103,6 +103,10 @@ struct arch_domain
void *tee;
#endif
+#ifdef CONFIG_FFA
+ void *ffa;
+#endif
+
bool directmap;
} __cacheline_aligned;
diff --git a/xen/arch/arm/include/asm/ffa.h b/xen/arch/arm/include/asm/ffa.h
new file mode 100644
index 000000000000..4f4a739345bd
--- /dev/null
+++ b/xen/arch/arm/include/asm/ffa.h
@@ -0,0 +1,71 @@
+/*
+ * xen/arch/arm/ffa.c
+ *
+ * Arm Firmware Framework for ARMv8-A(FFA) mediator
+ *
+ * Copyright (C) 2021 Linaro Limited
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without restriction,
+ * including without limitation the rights to use, copy, modify, merge,
+ * publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+ * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+ * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __ASM_ARM_FFA_H__
+#define __ASM_ARM_FFA_H__
+
+#include <xen/const.h>
+
+#include <asm/smccc.h>
+#include <asm/types.h>
+
+#define FFA_FNUM_MIN_VALUE _AC(0x60,U)
+#define FFA_FNUM_MAX_VALUE _AC(0x86,U)
+
+static inline bool is_ffa_fid(uint32_t fid)
+{
+ uint32_t fn = fid & ARM_SMCCC_FUNC_MASK;
+
+ return fn >= FFA_FNUM_MIN_VALUE && fn <= FFA_FNUM_MAX_VALUE;
+}
+
+#ifdef CONFIG_FFA
+#define FFA_NR_FUNCS 11
+
+bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid);
+int ffa_domain_init(struct domain *d, bool ffa_enabled);
+int ffa_relinquish_resources(struct domain *d);
+#else
+#define FFA_NR_FUNCS 0
+
+static inline bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
+{
+ return false;
+}
+
+static inline int ffa_domain_init(struct domain *d, bool ffa_enabled)
+{
+ return 0;
+}
+
+static inline int ffa_relinquish_resources(struct domain *d)
+{
+ return 0;
+}
+#endif
+
+#endif /*__ASM_ARM_FFA_H__*/
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 6f90c08a6304..34586025eff8 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -20,6 +20,7 @@
#include <public/arch-arm/smccc.h>
#include <asm/cpuerrata.h>
#include <asm/cpufeature.h>
+#include <asm/ffa.h>
#include <asm/monitor.h>
#include <asm/regs.h>
#include <asm/smccc.h>
@@ -32,7 +33,7 @@
#define XEN_SMCCC_FUNCTION_COUNT 3
/* Number of functions currently supported by Standard Service Service Calls. */
-#define SSSC_SMCCC_FUNCTION_COUNT (3 + VPSCI_NR_FUNCS)
+#define SSSC_SMCCC_FUNCTION_COUNT (3 + VPSCI_NR_FUNCS + FFA_NR_FUNCS)
static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid)
{
@@ -196,13 +197,23 @@ static bool handle_existing_apis(struct cpu_user_regs *regs)
return do_vpsci_0_1_call(regs, fid);
}
+static bool is_psci_fid(uint32_t fid)
+{
+ uint32_t fn = fid & ARM_SMCCC_FUNC_MASK;
+
+ return fn >= 0 && fn <= 0x1fU;
+}
+
/* PSCI 0.2 interface and other Standard Secure Calls */
static bool handle_sssc(struct cpu_user_regs *regs)
{
uint32_t fid = (uint32_t)get_user_reg(regs, 0);
- if ( do_vpsci_0_2_call(regs, fid) )
- return true;
+ if ( is_psci_fid(fid) )
+ return do_vpsci_0_2_call(regs, fid);
+
+ if ( is_ffa_fid(fid) )
+ return ffa_handle_call(regs, fid);
switch ( fid )
{
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index ab05fe12b0de..53f8d44a6a8e 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -318,6 +318,8 @@ struct xen_arch_domainconfig {
/* IN/OUT */
uint8_t gic_version;
/* IN */
+ uint8_t ffa_enabled;
+ /* IN */
uint16_t tee_type;
/* IN */
uint32_t nr_spis;
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 4/9] xen/arm: ffa: add direct request support
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
` (2 preceding siblings ...)
2022-08-18 10:55 ` [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator Jens Wiklander
@ 2022-08-18 10:55 ` Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 5/9] xen/arm: ffa: map SPMC rx/tx buffers Jens Wiklander
` (4 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:55 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander
Adds support for sending a FF-A direct request.
[1] https://developer.arm.com/documentation/den0077/latest
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
xen/arch/arm/ffa.c | 119 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 119 insertions(+)
diff --git a/xen/arch/arm/ffa.c b/xen/arch/arm/ffa.c
index b85c492928d2..cd5eefb437f8 100644
--- a/xen/arch/arm/ffa.c
+++ b/xen/arch/arm/ffa.c
@@ -189,6 +189,7 @@
struct ffa_ctx {
uint32_t guest_vers;
+ bool interrupted;
};
/* Negotiated FF-A version to use with the SPMC */
@@ -214,6 +215,55 @@ static bool ffa_get_version(uint32_t *vers)
return true;
}
+static int32_t get_ffa_ret_code(const struct arm_smccc_1_2_regs *resp)
+{
+ switch ( resp->a0 )
+ {
+ case FFA_ERROR:
+ if ( resp->a2 )
+ return resp->a2;
+ else
+ return FFA_RET_NOT_SUPPORTED;
+ case FFA_SUCCESS_32:
+ case FFA_SUCCESS_64:
+ return FFA_RET_OK;
+ default:
+ return FFA_RET_NOT_SUPPORTED;
+ }
+}
+
+static int32_t ffa_simple_call(uint32_t fid, register_t a1, register_t a2,
+ register_t a3, register_t a4)
+{
+ const struct arm_smccc_1_2_regs arg = {
+ .a0 = fid,
+ .a1 = a1,
+ .a2 = a2,
+ .a3 = a3,
+ .a4 = a4,
+ };
+ struct arm_smccc_1_2_regs resp;
+
+ arm_smccc_1_2_smc(&arg, &resp);
+
+ return get_ffa_ret_code(&resp);
+}
+
+static int32_t ffa_features(uint32_t id)
+{
+ return ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
+}
+
+static bool check_mandatory_feature(uint32_t id)
+{
+ uint32_t ret = ffa_features(id);
+
+ if (ret)
+ printk(XENLOG_ERR "ffa: mandatory feature id %#x missing\n", id);
+
+ return !ret;
+}
+
static u16 get_vm_id(const struct domain *d)
{
/* +1 since 0 is reserved for the hypervisor in FF-A */
@@ -255,6 +305,66 @@ static void handle_version(struct cpu_user_regs *regs)
set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
}
+static void handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid)
+{
+ struct arm_smccc_1_2_regs arg = { .a0 = fid, };
+ struct arm_smccc_1_2_regs resp = { };
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+ uint32_t src_dst;
+ uint64_t mask;
+
+ if ( smccc_is_conv_64(fid) )
+ mask = GENMASK_ULL(63, 0);
+ else
+ mask = GENMASK_ULL(31, 0);
+
+ src_dst = get_user_reg(regs, 1);
+ if ( (src_dst >> 16) != get_vm_id(d) )
+ {
+ resp.a0 = FFA_ERROR;
+ resp.a2 = FFA_RET_INVALID_PARAMETERS;
+ goto out;
+ }
+
+ arg.a1 = src_dst;
+ arg.a2 = get_user_reg(regs, 2) & mask;
+ arg.a3 = get_user_reg(regs, 3) & mask;
+ arg.a4 = get_user_reg(regs, 4) & mask;
+ arg.a5 = get_user_reg(regs, 5) & mask;
+ arg.a6 = get_user_reg(regs, 6) & mask;
+ arg.a7 = get_user_reg(regs, 7) & mask;
+
+ while ( true )
+ {
+ arm_smccc_1_2_smc(&arg, &resp);
+
+ switch ( resp.a0 )
+ {
+ case FFA_INTERRUPT:
+ ctx->interrupted = true;
+ goto out;
+ case FFA_ERROR:
+ case FFA_SUCCESS_32:
+ case FFA_SUCCESS_64:
+ case FFA_MSG_SEND_DIRECT_RESP_32:
+ case FFA_MSG_SEND_DIRECT_RESP_64:
+ goto out;
+ default:
+ /* Bad fid, report back. */
+ memset(&arg, 0, sizeof(arg));
+ arg.a0 = FFA_ERROR;
+ arg.a1 = src_dst;
+ arg.a2 = FFA_RET_NOT_SUPPORTED;
+ continue;
+ }
+ }
+
+out:
+ set_regs(regs, resp.a0, resp.a1 & mask, resp.a2 & mask, resp.a3 & mask,
+ resp.a4 & mask, resp.a5 & mask, resp.a6 & mask, resp.a7 & mask);
+}
+
bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
{
struct domain *d = current->domain;
@@ -271,6 +381,12 @@ bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
case FFA_ID_GET:
set_regs_success(regs, get_vm_id(d), 0);
return true;
+ case FFA_MSG_SEND_DIRECT_REQ_32:
+#ifdef CONFIG_ARM_64
+ case FFA_MSG_SEND_DIRECT_REQ_64:
+#endif
+ handle_msg_send_direct_req(regs, fid);
+ return true;
default:
printk(XENLOG_ERR "ffa: unhandled fid 0x%x\n", fid);
@@ -346,6 +462,9 @@ static int __init ffa_init(void)
printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
major_vers, minor_vers);
+ if ( !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
+ return 0;
+
ffa_version = vers;
return 0;
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 5/9] xen/arm: ffa: map SPMC rx/tx buffers
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
` (3 preceding siblings ...)
2022-08-18 10:55 ` [PATCH v5 4/9] xen/arm: ffa: add direct request support Jens Wiklander
@ 2022-08-18 10:55 ` Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 6/9] xen/arm: ffa: send guest events to Secure Partitions Jens Wiklander
` (3 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:55 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander
When initializing the FF-A mediator map the RX and TX buffers shared with
the SPMC.
These buffer are later used to to transmit data that cannot be passed in
registers only.
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
xen/arch/arm/ffa.c | 57 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/ffa.c b/xen/arch/arm/ffa.c
index cd5eefb437f8..b1bf5538b111 100644
--- a/xen/arch/arm/ffa.c
+++ b/xen/arch/arm/ffa.c
@@ -195,6 +195,15 @@ struct ffa_ctx {
/* Negotiated FF-A version to use with the SPMC */
static uint32_t ffa_version __read_mostly;
+/*
+ * Our rx/tx buffers shared with the SPMC.
+ *
+ * ffa_page_count is the number of pages used in each of these buffers.
+ */
+static void *ffa_rx __read_mostly;
+static void *ffa_tx __read_mostly;
+static unsigned int ffa_page_count __read_mostly;
+
static bool ffa_get_version(uint32_t *vers)
{
const struct arm_smccc_1_2_regs arg = {
@@ -264,6 +273,17 @@ static bool check_mandatory_feature(uint32_t id)
return !ret;
}
+static int32_t ffa_rxtx_map(register_t tx_addr, register_t rx_addr,
+ uint32_t page_count)
+{
+ uint32_t fid = FFA_RXTX_MAP_32;
+
+ if ( IS_ENABLED(CONFIG_ARM_64) )
+ fid = FFA_RXTX_MAP_64;
+
+ return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
+}
+
static u16 get_vm_id(const struct domain *d)
{
/* +1 since 0 is reserved for the hypervisor in FF-A */
@@ -425,6 +445,7 @@ int ffa_relinquish_resources(struct domain *d)
static int __init ffa_init(void)
{
uint32_t vers;
+ int e;
unsigned int major_vers;
unsigned int minor_vers;
@@ -462,11 +483,45 @@ static int __init ffa_init(void)
printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
major_vers, minor_vers);
- if ( !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
+ if (
+#ifdef CONFIG_ARM_64
+ !check_mandatory_feature(FFA_RXTX_MAP_64) ||
+#endif
+#ifdef CONFIG_ARM_32
+ !check_mandatory_feature(FFA_RXTX_MAP_32) ||
+#endif
+ !check_mandatory_feature(FFA_RXTX_UNMAP) ||
+ !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
+ return 0;
+
+ ffa_rx = alloc_xenheap_pages(0, 0);
+ if ( !ffa_rx )
return 0;
+ ffa_tx = alloc_xenheap_pages(0, 0);
+ if ( !ffa_tx )
+ goto err_free_ffa_rx;
+
+ e = ffa_rxtx_map(__pa(ffa_tx), __pa(ffa_rx), 1);
+ if ( e )
+ {
+ printk(XENLOG_ERR "ffa: Failed to map rxtx: error %d\n", e);
+ goto err_free_ffa_tx;
+ }
+ ffa_page_count = 1;
ffa_version = vers;
+ return 0;
+
+err_free_ffa_tx:
+ free_xenheap_pages(ffa_tx, 0);
+ ffa_tx = NULL;
+err_free_ffa_rx:
+ free_xenheap_pages(ffa_rx, 0);
+ ffa_rx = NULL;
+ ffa_page_count = 0;
+ ffa_version = 0;
+
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 6/9] xen/arm: ffa: send guest events to Secure Partitions
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
` (4 preceding siblings ...)
2022-08-18 10:55 ` [PATCH v5 5/9] xen/arm: ffa: map SPMC rx/tx buffers Jens Wiklander
@ 2022-08-18 10:55 ` Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 7/9] xen/arm: ffa: support mapping guest RX/TX buffers Jens Wiklander
` (2 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:55 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander
The FF-A specification defines framework messages sent as direct
requests when certain events occurs. For instance when a VM (guest) is
created or destroyed. Only SPs which have subscribed to these events
will receive them. An SP can subscribe to these messages in its
partition properties.
The partition properties of each SP is retrieved with
FFA_PARTITION_INFO_GET which returns the information in our RX buffer.
Using FFA_PARTITION_INFO_GET changes the owner of the RX buffer to the
caller (us), so once we're done with the buffer it must be released
using FFA_RX_RELEASE before another call can be made.
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
xen/arch/arm/ffa.c | 192 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 191 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/ffa.c b/xen/arch/arm/ffa.c
index b1bf5538b111..a829379ad176 100644
--- a/xen/arch/arm/ffa.c
+++ b/xen/arch/arm/ffa.c
@@ -187,6 +187,14 @@
#define FFA_MSG_SEND 0x8400006EU
#define FFA_MSG_POLL 0x8400006AU
+/* Partition information descriptor */
+struct ffa_partition_info_1_1 {
+ uint16_t id;
+ uint16_t execution_context;
+ uint32_t partition_properties;
+ uint8_t uuid[16];
+};
+
struct ffa_ctx {
uint32_t guest_vers;
bool interrupted;
@@ -195,6 +203,12 @@ struct ffa_ctx {
/* Negotiated FF-A version to use with the SPMC */
static uint32_t ffa_version __read_mostly;
+/* SPs subscribing to VM_CREATE and VM_DESTROYED events */
+static uint16_t *subscr_vm_created __read_mostly;
+static unsigned int subscr_vm_created_count __read_mostly;
+static uint16_t *subscr_vm_destroyed __read_mostly;
+static unsigned int subscr_vm_destroyed_count __read_mostly;
+
/*
* Our rx/tx buffers shared with the SPMC.
*
@@ -284,6 +298,72 @@ static int32_t ffa_rxtx_map(register_t tx_addr, register_t rx_addr,
return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
}
+static int32_t ffa_partition_info_get(uint32_t w1, uint32_t w2, uint32_t w3,
+ uint32_t w4, uint32_t w5,
+ uint32_t *count)
+{
+ const struct arm_smccc_1_2_regs arg = {
+ .a0 = FFA_PARTITION_INFO_GET,
+ .a1 = w1,
+ .a2 = w2,
+ .a3 = w3,
+ .a4 = w4,
+ .a5 = w5,
+ };
+ struct arm_smccc_1_2_regs resp;
+ uint32_t ret;
+
+ arm_smccc_1_2_smc(&arg, &resp);
+
+ ret = get_ffa_ret_code(&resp);
+ if ( !ret )
+ *count = resp.a2;
+
+ return ret;
+}
+
+static int32_t ffa_rx_release(void)
+{
+ return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
+}
+
+static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
+ uint8_t msg)
+{
+ uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
+ int32_t res;
+
+ if ( msg == FFA_MSG_SEND_VM_CREATED )
+ exp_resp |= FFA_MSG_RESP_VM_CREATED;
+ else if ( msg == FFA_MSG_SEND_VM_DESTROYED )
+ exp_resp |= FFA_MSG_RESP_VM_DESTROYED;
+ else
+ return FFA_RET_INVALID_PARAMETERS;
+
+ do {
+ const struct arm_smccc_1_2_regs arg = {
+ .a0 = FFA_MSG_SEND_DIRECT_REQ_32,
+ .a1 = sp_id,
+ .a2 = FFA_MSG_FLAG_FRAMEWORK | msg,
+ .a5 = vm_id,
+ };
+ struct arm_smccc_1_2_regs resp;
+
+ arm_smccc_1_2_smc(&arg, &resp);
+ if ( resp.a0 != FFA_MSG_SEND_DIRECT_RESP_32 || resp.a2 != exp_resp )
+ {
+ /*
+ * This is an invalid response, likely due to some error in the
+ * implementation of the ABI.
+ */
+ return FFA_RET_INVALID_PARAMETERS;
+ }
+ res = resp.a3;
+ } while ( res == FFA_RET_INTERRUPTED || res == FFA_RET_RETRY );
+
+ return res;
+}
+
static u16 get_vm_id(const struct domain *d)
{
/* +1 since 0 is reserved for the hypervisor in FF-A */
@@ -417,6 +497,10 @@ bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
int ffa_domain_init(struct domain *d, bool ffa_enabled)
{
struct ffa_ctx *ctx;
+ unsigned int n;
+ unsigned int m;
+ unsigned int c_pos;
+ int32_t res;
if ( !ffa_version || !ffa_enabled )
return 0;
@@ -425,23 +509,120 @@ int ffa_domain_init(struct domain *d, bool ffa_enabled)
if ( !ctx )
return -ENOMEM;
+ for ( n = 0; n < subscr_vm_created_count; n++ )
+ {
+ res = ffa_direct_req_send_vm(subscr_vm_created[n], get_vm_id(d),
+ FFA_MSG_SEND_VM_CREATED);
+ if ( res )
+ {
+ printk(XENLOG_ERR "ffa: Failed to report creation of vm_id %u to %u: res %d\n",
+ get_vm_id(d), subscr_vm_created[n], res);
+ c_pos = n;
+ goto err;
+ }
+ }
+
d->arch.ffa = ctx;
return 0;
+
+err:
+ /* Undo any already sent vm created messaged */
+ for ( n = 0; n < c_pos; n++ )
+ for ( m = 0; m < subscr_vm_destroyed_count; m++ )
+ if ( subscr_vm_destroyed[m] == subscr_vm_created[n] )
+ ffa_direct_req_send_vm(subscr_vm_destroyed[n], get_vm_id(d),
+ FFA_MSG_SEND_VM_DESTROYED);
+
+ return -ENOMEM;
}
int ffa_relinquish_resources(struct domain *d)
{
struct ffa_ctx *ctx = d->arch.ffa;
+ unsigned int n;
+ int32_t res;
if ( !ctx )
return 0;
+ for ( n = 0; n < subscr_vm_destroyed_count; n++ )
+ {
+ res = ffa_direct_req_send_vm(subscr_vm_destroyed[n], get_vm_id(d),
+ FFA_MSG_SEND_VM_DESTROYED);
+
+ if ( res )
+ printk(XENLOG_ERR "ffa: Failed to report destruction of vm_id %u to %u: res %d\n",
+ get_vm_id(d), subscr_vm_destroyed[n], res);
+ }
+
XFREE(d->arch.ffa);
return 0;
}
+static bool __init init_subscribers(void)
+{
+ struct ffa_partition_info_1_1 *fpi;
+ bool ret = false;
+ uint32_t count;
+ int e;
+ uint32_t n;
+ uint32_t c_pos;
+ uint32_t d_pos;
+
+ if ( ffa_version < FFA_VERSION_1_1 )
+ return true;
+
+ e = ffa_partition_info_get(0, 0, 0, 0, 0, &count);
+ if ( e )
+ {
+ printk(XENLOG_ERR "ffa: Failed to get list of SPs: %d\n", e);
+ goto out;
+ }
+
+ fpi = ffa_rx;
+ subscr_vm_created_count = 0;
+ subscr_vm_destroyed_count = 0;
+ for ( n = 0; n < count; n++ )
+ {
+ if (fpi[n].partition_properties & FFA_PART_PROP_NOTIF_CREATED)
+ subscr_vm_created_count++;
+ if (fpi[n].partition_properties & FFA_PART_PROP_NOTIF_DESTROYED)
+ subscr_vm_destroyed_count++;
+ }
+
+ if ( subscr_vm_created_count )
+ subscr_vm_created = xzalloc_array(uint16_t, subscr_vm_created_count);
+ if ( subscr_vm_destroyed_count )
+ subscr_vm_destroyed = xzalloc_array(uint16_t,
+ subscr_vm_destroyed_count);
+ if ( (subscr_vm_created_count && !subscr_vm_created) ||
+ (subscr_vm_destroyed_count && !subscr_vm_destroyed) )
+ {
+ printk(XENLOG_ERR "ffa: Failed to allocate subscription lists\n");
+ subscr_vm_created_count = 0;
+ subscr_vm_destroyed_count = 0;
+ XFREE(subscr_vm_created);
+ XFREE(subscr_vm_destroyed);
+ goto out;
+ }
+
+ for ( c_pos = 0, d_pos = 0, n = 0; n < count; n++ )
+ {
+ if ( fpi[n].partition_properties & FFA_PART_PROP_NOTIF_CREATED )
+ subscr_vm_created[c_pos++] = fpi[n].id;
+ if ( fpi[n].partition_properties & FFA_PART_PROP_NOTIF_DESTROYED )
+ subscr_vm_destroyed[d_pos++] = fpi[n].id;
+ }
+
+ ret = true;
+out:
+ ffa_rx_release();
+
+ return ret;
+}
+
static int __init ffa_init(void)
{
uint32_t vers;
@@ -483,9 +664,11 @@ static int __init ffa_init(void)
printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
major_vers, minor_vers);
- if (
+ if ( !check_mandatory_feature(FFA_PARTITION_INFO_GET) ||
+ !check_mandatory_feature(FFA_RX_RELEASE) ||
#ifdef CONFIG_ARM_64
!check_mandatory_feature(FFA_RXTX_MAP_64) ||
+ !check_mandatory_feature(FFA_MEM_SHARE_64) ||
#endif
#ifdef CONFIG_ARM_32
!check_mandatory_feature(FFA_RXTX_MAP_32) ||
@@ -511,6 +694,9 @@ static int __init ffa_init(void)
ffa_page_count = 1;
ffa_version = vers;
+ if ( !init_subscribers() )
+ goto err_free_ffa_tx;
+
return 0;
err_free_ffa_tx:
@@ -521,6 +707,10 @@ err_free_ffa_rx:
ffa_rx = NULL;
ffa_page_count = 0;
ffa_version = 0;
+ XFREE(subscr_vm_created);
+ subscr_vm_created_count = 0;
+ XFREE(subscr_vm_destroyed);
+ subscr_vm_destroyed_count = 0;
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 7/9] xen/arm: ffa: support mapping guest RX/TX buffers
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
` (5 preceding siblings ...)
2022-08-18 10:55 ` [PATCH v5 6/9] xen/arm: ffa: send guest events to Secure Partitions Jens Wiklander
@ 2022-08-18 10:55 ` Jens Wiklander
2022-08-18 10:56 ` [PATCH v5 8/9] xen/arm: ffa: support guest FFA_PARTITION_INFO_GET Jens Wiklander
2022-08-18 10:56 ` [PATCH v5 9/9] xen/arm: ffa: support sharing memory Jens Wiklander
8 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:55 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander
Adds support in the mediator to map and unmap the RX and TX buffers
provided by the guest using the two FF-A functions FFA_RXTX_MAP and
FFA_RXTX_UNMAP.
These buffer are later used to to transmit data that cannot be passed in
registers only.
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
xen/arch/arm/ffa.c | 127 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 127 insertions(+)
diff --git a/xen/arch/arm/ffa.c b/xen/arch/arm/ffa.c
index a829379ad176..5e7211f0b4f6 100644
--- a/xen/arch/arm/ffa.c
+++ b/xen/arch/arm/ffa.c
@@ -196,10 +196,17 @@ struct ffa_partition_info_1_1 {
};
struct ffa_ctx {
+ void *rx;
+ const void *tx;
+ struct page_info *rx_pg;
+ struct page_info *tx_pg;
+ unsigned int page_count;
uint32_t guest_vers;
+ bool tx_is_mine;
bool interrupted;
};
+
/* Negotiated FF-A version to use with the SPMC */
static uint32_t ffa_version __read_mostly;
@@ -384,6 +391,11 @@ static void set_regs(struct cpu_user_regs *regs, register_t v0, register_t v1,
set_user_reg(regs, 7, v7);
}
+static void set_regs_error(struct cpu_user_regs *regs, uint32_t error_code)
+{
+ set_regs(regs, FFA_ERROR, 0, error_code, 0, 0, 0, 0, 0);
+}
+
static void set_regs_success(struct cpu_user_regs *regs, uint32_t w2,
uint32_t w3)
{
@@ -405,6 +417,99 @@ static void handle_version(struct cpu_user_regs *regs)
set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
}
+static uint32_t handle_rxtx_map(uint32_t fid, register_t tx_addr,
+ register_t rx_addr, uint32_t page_count)
+{
+ uint32_t ret = FFA_RET_INVALID_PARAMETERS;
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+ struct page_info *tx_pg;
+ struct page_info *rx_pg;
+ p2m_type_t t;
+ void *rx;
+ void *tx;
+
+ if ( !smccc_is_conv_64(fid) )
+ {
+ tx_addr &= UINT32_MAX;
+ rx_addr &= UINT32_MAX;
+ }
+
+ /* For now to keep things simple, only deal with a single page */
+ if ( page_count != 1 )
+ return FFA_RET_NOT_SUPPORTED;
+
+ /* Already mapped */
+ if ( ctx->rx )
+ return FFA_RET_DENIED;
+
+ tx_pg = get_page_from_gfn(d, gfn_x(gaddr_to_gfn(tx_addr)), &t, P2M_ALLOC);
+ if ( !tx_pg )
+ return FFA_RET_INVALID_PARAMETERS;
+ /* Only normal RAM for now */
+ if ( !p2m_is_ram(t) )
+ goto err_put_tx_pg;
+
+ rx_pg = get_page_from_gfn(d, gfn_x(gaddr_to_gfn(rx_addr)), &t, P2M_ALLOC);
+ if ( !tx_pg )
+ goto err_put_tx_pg;
+ /* Only normal RAM for now */
+ if ( !p2m_is_ram(t) )
+ goto err_put_rx_pg;
+
+ tx = __map_domain_page_global(tx_pg);
+ if ( !tx )
+ goto err_put_rx_pg;
+
+ rx = __map_domain_page_global(rx_pg);
+ if ( !rx )
+ goto err_unmap_tx;
+
+ ctx->rx = rx;
+ ctx->tx = tx;
+ ctx->rx_pg = rx_pg;
+ ctx->tx_pg = tx_pg;
+ ctx->page_count = 1;
+ ctx->tx_is_mine = true;
+ return FFA_RET_OK;
+
+err_unmap_tx:
+ unmap_domain_page_global(tx);
+err_put_rx_pg:
+ put_page(rx_pg);
+err_put_tx_pg:
+ put_page(tx_pg);
+
+ return ret;
+}
+
+static void rxtx_unmap(struct ffa_ctx *ctx)
+{
+ unmap_domain_page_global(ctx->rx);
+ unmap_domain_page_global(ctx->tx);
+ put_page(ctx->rx_pg);
+ put_page(ctx->tx_pg);
+ ctx->rx = NULL;
+ ctx->tx = NULL;
+ ctx->rx_pg = NULL;
+ ctx->tx_pg = NULL;
+ ctx->page_count = 0;
+ ctx->tx_is_mine = false;
+}
+
+static uint32_t handle_rxtx_unmap(void)
+{
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+
+ if ( !ctx->rx )
+ return FFA_RET_INVALID_PARAMETERS;
+
+ rxtx_unmap(ctx);
+
+ return FFA_RET_OK;
+}
+
static void handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid)
{
struct arm_smccc_1_2_regs arg = { .a0 = fid, };
@@ -469,6 +574,7 @@ bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
{
struct domain *d = current->domain;
struct ffa_ctx *ctx = d->arch.ffa;
+ int e;
if ( !ctx )
return false;
@@ -481,6 +587,24 @@ bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
case FFA_ID_GET:
set_regs_success(regs, get_vm_id(d), 0);
return true;
+ case FFA_RXTX_MAP_32:
+#ifdef CONFIG_ARM_64
+ case FFA_RXTX_MAP_64:
+#endif
+ e = handle_rxtx_map(fid, get_user_reg(regs, 1), get_user_reg(regs, 2),
+ get_user_reg(regs, 3));
+ if ( e )
+ set_regs_error(regs, e);
+ else
+ set_regs_success(regs, 0, 0);
+ return true;
+ case FFA_RXTX_UNMAP:
+ e = handle_rxtx_unmap();
+ if ( e )
+ set_regs_error(regs, e);
+ else
+ set_regs_success(regs, 0, 0);
+ return true;
case FFA_MSG_SEND_DIRECT_REQ_32:
#ifdef CONFIG_ARM_64
case FFA_MSG_SEND_DIRECT_REQ_64:
@@ -556,6 +680,9 @@ int ffa_relinquish_resources(struct domain *d)
get_vm_id(d), subscr_vm_destroyed[n], res);
}
+ if ( ctx->rx )
+ rxtx_unmap(ctx);
+
XFREE(d->arch.ffa);
return 0;
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 8/9] xen/arm: ffa: support guest FFA_PARTITION_INFO_GET
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
` (6 preceding siblings ...)
2022-08-18 10:55 ` [PATCH v5 7/9] xen/arm: ffa: support mapping guest RX/TX buffers Jens Wiklander
@ 2022-08-18 10:56 ` Jens Wiklander
2022-08-18 10:56 ` [PATCH v5 9/9] xen/arm: ffa: support sharing memory Jens Wiklander
8 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:56 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander
Adds support in the mediator to handle FFA_PARTITION_INFO_GET requests
from a guest. The requests are forwarded to the SPMC and the response is
translated according to the FF-A version in use by the guest.
Using FFA_PARTITION_INFO_GET changes the owner of the RX buffer to the
caller (the guest in this case), so once it is done with the buffer it
must be released using FFA_RX_RELEASE before another call can be made.
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
xen/arch/arm/ffa.c | 126 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 124 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/ffa.c b/xen/arch/arm/ffa.c
index 5e7211f0b4f6..d2872acc3474 100644
--- a/xen/arch/arm/ffa.c
+++ b/xen/arch/arm/ffa.c
@@ -188,6 +188,12 @@
#define FFA_MSG_POLL 0x8400006AU
/* Partition information descriptor */
+struct ffa_partition_info_1_0 {
+ uint16_t id;
+ uint16_t execution_context;
+ uint32_t partition_properties;
+};
+
struct ffa_partition_info_1_1 {
uint16_t id;
uint16_t execution_context;
@@ -204,9 +210,8 @@ struct ffa_ctx {
uint32_t guest_vers;
bool tx_is_mine;
bool interrupted;
+ spinlock_t lock;
};
-
-
/* Negotiated FF-A version to use with the SPMC */
static uint32_t ffa_version __read_mostly;
@@ -220,10 +225,16 @@ static unsigned int subscr_vm_destroyed_count __read_mostly;
* Our rx/tx buffers shared with the SPMC.
*
* ffa_page_count is the number of pages used in each of these buffers.
+ *
+ * The RX buffer is protected from concurrent usage with ffa_rx_buffer_lock.
+ * Note that the SPMC is also tracking the ownership of our RX buffer so
+ * for calls which uses our RX buffer to deliver a result we must call
+ * ffa_rx_release() to let the SPMC know that we're done with the buffer.
*/
static void *ffa_rx __read_mostly;
static void *ffa_tx __read_mostly;
static unsigned int ffa_page_count __read_mostly;
+static DEFINE_SPINLOCK(ffa_rx_buffer_lock);
static bool ffa_get_version(uint32_t *vers)
{
@@ -510,6 +521,98 @@ static uint32_t handle_rxtx_unmap(void)
return FFA_RET_OK;
}
+static uint32_t handle_partition_info_get(uint32_t w1, uint32_t w2, uint32_t w3,
+ uint32_t w4, uint32_t w5,
+ uint32_t *count)
+{
+ bool query_count_only = w5 & FFA_PARTITION_INFO_GET_COUNT_FLAG;
+ uint32_t w5_mask = 0;
+ uint32_t ret = FFA_RET_DENIED;
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+
+ /*
+ * FF-A v1.0 has w5 MBZ while v1.1 allows
+ * FFA_PARTITION_INFO_GET_COUNT_FLAG to be non-zero.
+ */
+ if ( ctx->guest_vers == FFA_VERSION_1_1 )
+ w5_mask = FFA_PARTITION_INFO_GET_COUNT_FLAG;
+ if ( w5 & ~w5_mask )
+ return FFA_RET_INVALID_PARAMETERS;
+
+ if ( query_count_only )
+ return ffa_partition_info_get(w1, w2, w3, w4, w5, count);
+
+ if ( !ffa_page_count )
+ return FFA_RET_DENIED;
+
+ spin_lock(&ctx->lock);
+ spin_lock(&ffa_rx_buffer_lock);
+ if ( !ctx->page_count || !ctx->tx_is_mine )
+ goto out;
+ ret = ffa_partition_info_get(w1, w2, w3, w4, w5, count);
+ if ( ret )
+ goto out;
+
+ if ( ctx->guest_vers == FFA_VERSION_1_0 )
+ {
+ size_t n;
+ struct ffa_partition_info_1_1 *src = ffa_rx;
+ struct ffa_partition_info_1_0 *dst = ctx->rx;
+
+ if ( ctx->page_count * FFA_PAGE_SIZE < *count * sizeof(*dst) )
+ {
+ ret = FFA_RET_NO_MEMORY;
+ goto out_rx_release;
+ }
+
+ for ( n = 0; n < *count; n++ )
+ {
+ dst[n].id = src[n].id;
+ dst[n].execution_context = src[n].execution_context;
+ dst[n].partition_properties = src[n].partition_properties;
+ }
+ }
+ else
+ {
+ size_t sz = *count * sizeof(struct ffa_partition_info_1_1);
+
+ if ( ctx->page_count * FFA_PAGE_SIZE < sz )
+ {
+ ret = FFA_RET_NO_MEMORY;
+ goto out_rx_release;
+ }
+
+
+ memcpy(ctx->rx, ffa_rx, sz);
+ }
+ ctx->tx_is_mine = false;
+out_rx_release:
+ ffa_rx_release();
+out:
+ spin_unlock(&ffa_rx_buffer_lock);
+ spin_unlock(&ctx->lock);
+
+ return ret;
+}
+
+static uint32_t handle_rx_release(void)
+{
+ uint32_t ret = FFA_RET_DENIED;
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+
+ spin_lock(&ctx->lock);
+ if ( !ctx->page_count || ctx->tx_is_mine )
+ goto out;
+ ret = FFA_RET_OK;
+ ctx->tx_is_mine = true;
+out:
+ spin_unlock(&ctx->lock);
+
+ return ret;
+}
+
static void handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t fid)
{
struct arm_smccc_1_2_regs arg = { .a0 = fid, };
@@ -574,6 +677,7 @@ bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
{
struct domain *d = current->domain;
struct ffa_ctx *ctx = d->arch.ffa;
+ uint32_t count;
int e;
if ( !ctx )
@@ -605,6 +709,24 @@ bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
else
set_regs_success(regs, 0, 0);
return true;
+ case FFA_PARTITION_INFO_GET:
+ e = handle_partition_info_get(get_user_reg(regs, 1),
+ get_user_reg(regs, 2),
+ get_user_reg(regs, 3),
+ get_user_reg(regs, 4),
+ get_user_reg(regs, 5), &count);
+ if ( e )
+ set_regs_error(regs, e);
+ else
+ set_regs_success(regs, count, 0);
+ return true;
+ case FFA_RX_RELEASE:
+ e = handle_rx_release();
+ if ( e )
+ set_regs_error(regs, e);
+ else
+ set_regs_success(regs, 0, 0);
+ return true;
case FFA_MSG_SEND_DIRECT_REQ_32:
#ifdef CONFIG_ARM_64
case FFA_MSG_SEND_DIRECT_REQ_64:
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 9/9] xen/arm: ffa: support sharing memory
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
` (7 preceding siblings ...)
2022-08-18 10:56 ` [PATCH v5 8/9] xen/arm: ffa: support guest FFA_PARTITION_INFO_GET Jens Wiklander
@ 2022-08-18 10:56 ` Jens Wiklander
8 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-18 10:56 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Jens Wiklander
Adds support for a guest to share memory with an SP using FFA_MEM_SHARE,
FFA_MEM_RECLAIM and FFA_MEM_FRAG_TX. Small memory regions can be shared
using FFA_MEM_SHARE, but larger memory regions may need to be
transmitted in fragments with FFA_MEM_FRAG_TX. A memory region that
doesn't need to be shared any longer can be reclaimed with
FFA_MEM_RECLAIM once the SP doesn't use it any longer. This is checked
by the SPMC and not in control of the mediator.
With this commit we have a FF-A version 1.1 [1] mediator able to
communicate with a Secure Partition in secure world. The secure world
must use FF-A version 1.1, but the guest is free to use version 1.0 or
version 1.1.
The implementation is the bare minimum to be able to communicate with
OP-TEE running as an SPMC at S-EL1.
[1] https://developer.arm.com/documentation/den0077/latest
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
xen/arch/arm/ffa.c | 838 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 838 insertions(+)
diff --git a/xen/arch/arm/ffa.c b/xen/arch/arm/ffa.c
index d2872acc3474..d2ad49ae6de2 100644
--- a/xen/arch/arm/ffa.c
+++ b/xen/arch/arm/ffa.c
@@ -201,6 +201,107 @@ struct ffa_partition_info_1_1 {
uint8_t uuid[16];
};
+/* Constituent memory region descriptor */
+struct ffa_address_range {
+ uint64_t address;
+ uint32_t page_count;
+ uint32_t reserved;
+};
+
+/* Composite memory region descriptor */
+struct ffa_mem_region {
+ uint32_t total_page_count;
+ uint32_t address_range_count;
+ uint64_t reserved;
+ struct ffa_address_range address_range_array[];
+};
+
+/* Memory access permissions descriptor */
+struct ffa_mem_access_perm {
+ uint16_t endpoint_id;
+ uint8_t perm;
+ uint8_t flags;
+};
+
+/* Endpoint memory access descriptor */
+struct ffa_mem_access {
+ struct ffa_mem_access_perm access_perm;
+ uint32_t region_offs;
+ uint64_t reserved;
+};
+
+/* Lend, donate or share memory transaction descriptor */
+struct ffa_mem_transaction_1_0 {
+ uint16_t sender_id;
+ uint8_t mem_reg_attr;
+ uint8_t reserved0;
+ uint32_t flags;
+ uint64_t global_handle;
+ uint64_t tag;
+ uint32_t reserved1;
+ uint32_t mem_access_count;
+ struct ffa_mem_access mem_access_array[];
+};
+
+struct ffa_mem_transaction_1_1 {
+ uint16_t sender_id;
+ uint16_t mem_reg_attr;
+ uint32_t flags;
+ uint64_t global_handle;
+ uint64_t tag;
+ uint32_t mem_access_size;
+ uint32_t mem_access_count;
+ uint32_t mem_access_offs;
+ uint8_t reserved[12];
+};
+
+/* Calculate offset of struct ffa_mem_access from start of buffer */
+#define MEM_ACCESS_OFFSET(access_idx) \
+ ( sizeof(struct ffa_mem_transaction_1_1) + \
+ ( access_idx ) * sizeof(struct ffa_mem_access) )
+
+/* Calculate offset of struct ffa_mem_region from start of buffer */
+#define REGION_OFFSET(access_count, region_idx) \
+ ( MEM_ACCESS_OFFSET(access_count) + \
+ ( region_idx ) * sizeof(struct ffa_mem_region) )
+
+/* Calculate offset of struct ffa_address_range from start of buffer */
+#define ADDR_RANGE_OFFSET(access_count, region_count, range_idx) \
+ ( REGION_OFFSET(access_count, region_count) + \
+ ( range_idx ) * sizeof(struct ffa_address_range) )
+
+/*
+ * The parts needed from struct ffa_mem_transaction_1_0 or struct
+ * ffa_mem_transaction_1_1, used to provide an abstraction of difference in
+ * data structures between version 1.0 and 1.1. This is just an internal
+ * interface and can be changed without changing any ABI.
+ */
+struct ffa_mem_transaction_x {
+ uint16_t sender_id;
+ uint8_t mem_reg_attr;
+ uint8_t flags;
+ uint8_t mem_access_size;
+ uint8_t mem_access_count;
+ uint16_t mem_access_offs;
+ uint64_t global_handle;
+ uint64_t tag;
+};
+
+/* Endpoint RX/TX descriptor */
+struct ffa_endpoint_rxtx_descriptor_1_0 {
+ uint16_t sender_id;
+ uint16_t reserved;
+ uint32_t rx_range_count;
+ uint32_t tx_range_count;
+};
+
+struct ffa_endpoint_rxtx_descriptor_1_1 {
+ uint16_t sender_id;
+ uint16_t reserved;
+ uint32_t rx_region_offs;
+ uint32_t tx_region_offs;
+};
+
struct ffa_ctx {
void *rx;
const void *tx;
@@ -210,8 +311,33 @@ struct ffa_ctx {
uint32_t guest_vers;
bool tx_is_mine;
bool interrupted;
+ struct list_head frag_list;
+ struct list_head shm_list;
+ unsigned int shm_count;
spinlock_t lock;
};
+
+struct ffa_shm_mem {
+ struct list_head list;
+ uint16_t sender_id;
+ uint16_t ep_id; /* endpoint, the one lending */
+ uint64_t handle; /* FFA_HANDLE_INVALID if not set yet */
+ unsigned int page_count;
+ struct page_info *pages[];
+};
+
+struct mem_frag_state {
+ struct list_head list;
+ struct ffa_shm_mem *shm;
+ uint32_t range_count;
+ unsigned int current_page_idx;
+ unsigned int frag_offset;
+ unsigned int range_offset;
+ const uint8_t *buf;
+ unsigned int buf_size;
+ struct ffa_address_range range;
+};
+
/* Negotiated FF-A version to use with the SPMC */
static uint32_t ffa_version __read_mostly;
@@ -226,6 +352,8 @@ static unsigned int subscr_vm_destroyed_count __read_mostly;
*
* ffa_page_count is the number of pages used in each of these buffers.
*
+ * The TX buffer is protected from concurrent usage with ffa_tx_buffer_lock.
+ *
* The RX buffer is protected from concurrent usage with ffa_rx_buffer_lock.
* Note that the SPMC is also tracking the ownership of our RX buffer so
* for calls which uses our RX buffer to deliver a result we must call
@@ -235,6 +363,7 @@ static void *ffa_rx __read_mostly;
static void *ffa_tx __read_mostly;
static unsigned int ffa_page_count __read_mostly;
static DEFINE_SPINLOCK(ffa_rx_buffer_lock);
+static DEFINE_SPINLOCK(ffa_tx_buffer_lock);
static bool ffa_get_version(uint32_t *vers)
{
@@ -345,6 +474,78 @@ static int32_t ffa_rx_release(void)
return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
}
+static int32_t ffa_mem_share(uint32_t tot_len, uint32_t frag_len,
+ register_t addr, uint32_t pg_count,
+ uint64_t *handle)
+{
+ struct arm_smccc_1_2_regs arg = {
+ .a0 = FFA_MEM_SHARE_32,
+ .a1 = tot_len,
+ .a2 = frag_len,
+ .a3 = addr,
+ .a4 = pg_count,
+ };
+ struct arm_smccc_1_2_regs resp;
+
+ if ( IS_ENABLED(CONFIG_ARM_64) )
+ arg.a0 = FFA_MEM_SHARE_64;
+
+ arm_smccc_1_2_smc(&arg, &resp);
+
+ switch ( resp.a0 )
+ {
+ case FFA_ERROR:
+ if ( resp.a2 )
+ return resp.a2;
+ else
+ return FFA_RET_NOT_SUPPORTED;
+ case FFA_SUCCESS_32:
+ *handle = regpair_to_uint64(resp.a3, resp.a2);
+ return FFA_RET_OK;
+ case FFA_MEM_FRAG_RX:
+ *handle = regpair_to_uint64(resp.a2, resp.a1);
+ return resp.a3;
+ default:
+ return FFA_RET_NOT_SUPPORTED;
+ }
+}
+
+static int32_t ffa_mem_frag_tx(uint64_t handle, uint32_t frag_len,
+ uint16_t sender_id)
+{
+ struct arm_smccc_1_2_regs arg = {
+ .a0 = FFA_MEM_FRAG_TX,
+ .a1 = handle & UINT32_MAX,
+ .a2 = handle >> 32,
+ .a3 = frag_len,
+ .a4 = (uint32_t)sender_id << 16,
+ };
+ struct arm_smccc_1_2_regs resp;
+
+ arm_smccc_1_2_smc(&arg, &resp);
+
+ switch ( resp.a0 )
+ {
+ case FFA_ERROR:
+ if ( resp.a2 )
+ return resp.a2;
+ else
+ return FFA_RET_NOT_SUPPORTED;
+ case FFA_SUCCESS_32:
+ return FFA_RET_OK;
+ case FFA_MEM_FRAG_RX:
+ return resp.a3;
+ default:
+ return FFA_RET_NOT_SUPPORTED;
+ }
+}
+
+static int32_t ffa_mem_reclaim(uint32_t handle_lo, uint32_t handle_hi,
+ uint32_t flags)
+{
+ return ffa_simple_call(FFA_MEM_RECLAIM, handle_lo, handle_hi, flags, 0);
+}
+
static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
uint8_t msg)
{
@@ -413,6 +614,14 @@ static void set_regs_success(struct cpu_user_regs *regs, uint32_t w2,
set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, 0, 0, 0, 0);
}
+static void set_regs_frag_rx(struct cpu_user_regs *regs, uint32_t handle_lo,
+ uint32_t handle_hi, uint32_t frag_offset,
+ uint16_t sender_id)
+{
+ set_regs(regs, FFA_MEM_FRAG_RX, handle_lo, handle_hi, frag_offset,
+ (uint32_t)sender_id << 16, 0, 0, 0);
+}
+
static void handle_version(struct cpu_user_regs *regs)
{
struct domain *d = current->domain;
@@ -673,6 +882,611 @@ out:
resp.a4 & mask, resp.a5 & mask, resp.a6 & mask, resp.a7 & mask);
}
+/*
+ * Gets all page and assigns them to the supplied shared memory object. If
+ * this function fails then the caller is still expected to call
+ * put_shm_pages() as a cleanup.
+ */
+static int get_shm_pages(struct domain *d, struct ffa_shm_mem *shm,
+ const struct ffa_address_range *range,
+ uint32_t range_count, unsigned int start_page_idx,
+ unsigned int *last_page_idx)
+{
+ unsigned int pg_idx = start_page_idx;
+ gfn_t gfn;
+ unsigned int n;
+ unsigned int m;
+ p2m_type_t t;
+ uint64_t addr;
+
+ for ( n = 0; n < range_count; n++ )
+ {
+ for ( m = 0; m < range[n].page_count; m++ )
+ {
+ if ( pg_idx >= shm->page_count )
+ return FFA_RET_INVALID_PARAMETERS;
+
+ addr = read_atomic(&range[n].address);
+ gfn = gaddr_to_gfn(addr + m * FFA_PAGE_SIZE);
+ shm->pages[pg_idx] = get_page_from_gfn(d, gfn_x(gfn), &t,
+ P2M_ALLOC);
+ if ( !shm->pages[pg_idx] )
+ return FFA_RET_DENIED;
+ pg_idx++;
+ /* Only normal RAM for now */
+ if ( !p2m_is_ram(t) )
+ return FFA_RET_DENIED;
+ }
+ }
+
+ *last_page_idx = pg_idx;
+
+ return FFA_RET_OK;
+}
+
+static void put_shm_pages(struct ffa_shm_mem *shm)
+{
+ unsigned int n;
+
+ for ( n = 0; n < shm->page_count && shm->pages[n]; n++ )
+ {
+ put_page(shm->pages[n]);
+ shm->pages[n] = NULL;
+ }
+}
+
+static struct ffa_shm_mem *alloc_ffa_shm_mem(struct ffa_ctx *ctx,
+ unsigned int page_count)
+{
+ struct ffa_shm_mem *shm;
+
+ if ( page_count >= FFA_MAX_SHM_PAGE_COUNT ||
+ ctx->shm_count >= FFA_MAX_SHM_COUNT )
+ return NULL;
+
+ shm = xzalloc_flex_struct(struct ffa_shm_mem, pages, page_count);
+ if ( shm )
+ {
+ ctx->shm_count++;
+ shm->page_count = page_count;
+ }
+
+ return shm;
+}
+
+static void free_ffa_shm_mem(struct ffa_ctx *ctx, struct ffa_shm_mem *shm)
+{
+ if ( shm ) {
+ ASSERT(ctx->shm_count > 0);
+ ctx->shm_count--;
+ put_shm_pages(shm);
+ xfree(shm);
+ }
+}
+
+static void init_range(struct ffa_address_range *addr_range,
+ paddr_t pa)
+{
+ memset(addr_range, 0, sizeof(*addr_range));
+ addr_range->address = pa;
+ addr_range->page_count = 1;
+}
+
+/*
+ * This function uses the ffa_tx buffer to transmit the memory transaction
+ * descriptor. The function depends ffa_tx_buffer_lock to be used to guard
+ * the buffer from concurent use.
+ */
+static int share_shm(struct ffa_shm_mem *shm)
+{
+ const uint32_t max_frag_len = ffa_page_count * FFA_PAGE_SIZE;
+ struct ffa_mem_access *mem_access_array;
+ struct ffa_mem_transaction_1_1 *descr;
+ struct ffa_address_range *addr_range;
+ struct ffa_mem_region *region_descr;
+ const unsigned int region_count = 1;
+ void *buf = ffa_tx;
+ uint32_t frag_len;
+ uint32_t tot_len;
+ paddr_t last_pa;
+ unsigned int n;
+ paddr_t pa;
+ bool first;
+ int ret;
+
+ ASSERT(spin_is_locked(&ffa_tx_buffer_lock));
+ if ( !shm->page_count )
+ {
+ ASSERT_UNREACHABLE();
+ return FFA_RET_INVALID_PARAMETERS;
+ }
+
+ descr = buf;
+ memset(descr, 0, sizeof(*descr));
+ descr->sender_id = shm->sender_id;
+ descr->global_handle = shm->handle;
+ descr->mem_reg_attr = FFA_NORMAL_MEM_REG_ATTR;
+ descr->mem_access_count = 1;
+ descr->mem_access_size = sizeof(*mem_access_array);
+ descr->mem_access_offs = MEM_ACCESS_OFFSET(0);
+
+ mem_access_array = buf + descr->mem_access_offs;
+ memset(mem_access_array, 0, sizeof(*mem_access_array));
+ mem_access_array[0].access_perm.endpoint_id = shm->ep_id;
+ mem_access_array[0].access_perm.perm = FFA_MEM_ACC_RW;
+ mem_access_array[0].region_offs = REGION_OFFSET(descr->mem_access_count, 0);
+
+ region_descr = buf + mem_access_array[0].region_offs;
+ memset(region_descr, 0, sizeof(*region_descr));
+ region_descr->total_page_count = shm->page_count;
+
+ region_descr->address_range_count = 1;
+ last_pa = page_to_maddr(shm->pages[0]);
+ for ( n = 1; n < shm->page_count; last_pa = pa, n++ )
+ {
+ pa = page_to_maddr(shm->pages[n]);
+ if ( last_pa + FFA_PAGE_SIZE == pa )
+ continue;
+ region_descr->address_range_count++;
+ }
+
+ tot_len = ADDR_RANGE_OFFSET(descr->mem_access_count, region_count,
+ region_descr->address_range_count);
+
+ /*
+ * Sharing memory with secure world may have to be done with multiple
+ * calls depending on how many address ranges will be needed. If we're
+ * sharing physically contiguous memory we will only need one range but
+ * we will also need to deal with the worst case where all physical
+ * pages are non-contiguous. For the first batch of address ranges we
+ * call ffa_mem_share() and for all that follows ffa_mem_frag_tx().
+ *
+ * We use frag_len to keep track of how far into the transmit buffer we
+ * have gone.
+ */
+ addr_range = region_descr->address_range_array;
+ frag_len = ADDR_RANGE_OFFSET(descr->mem_access_count, region_count, 1);
+ last_pa = page_to_maddr(shm->pages[0]);
+ init_range(addr_range, last_pa);
+ first = true;
+ for ( n = 1; n < shm->page_count; last_pa = pa, n++ )
+ {
+ pa = page_to_maddr(shm->pages[n]);
+ if ( last_pa + FFA_PAGE_SIZE == pa )
+ {
+ addr_range->page_count++;
+ continue;
+ }
+
+ if ( frag_len == max_frag_len )
+ {
+ if ( first )
+ {
+ ret = ffa_mem_share(tot_len, frag_len, 0, 0, &shm->handle);
+ first = false;
+ }
+ else
+ {
+ ret = ffa_mem_frag_tx(shm->handle, frag_len, shm->sender_id);
+ }
+ if ( ret <= 0 )
+ return ret;
+ frag_len = sizeof(*addr_range);
+ addr_range = buf;
+ }
+ else
+ {
+ frag_len += sizeof(*addr_range);
+ addr_range++;
+ }
+ init_range(addr_range, pa);
+ }
+
+ if ( first )
+ return ffa_mem_share(tot_len, frag_len, 0, 0, &shm->handle);
+ else
+ return ffa_mem_frag_tx(shm->handle, frag_len, shm->sender_id);
+}
+
+static int read_mem_transaction(uint32_t ffa_vers, const void *buf, size_t blen,
+ struct ffa_mem_transaction_x *trans)
+{
+ uint16_t mem_reg_attr;
+ uint32_t flags;
+ uint32_t count;
+ uint32_t offs;
+ uint32_t size;
+
+ if ( ffa_vers >= FFA_VERSION_1_1 )
+ {
+ const struct ffa_mem_transaction_1_1 *descr;
+
+ if ( blen < sizeof(*descr) )
+ return FFA_RET_INVALID_PARAMETERS;
+
+ descr = buf;
+ trans->sender_id = descr->sender_id;
+ mem_reg_attr = descr->mem_reg_attr;
+ flags = descr->flags;
+ trans->global_handle = descr->global_handle;
+ trans->tag = descr->tag;
+
+ count = descr->mem_access_count;
+ size = descr->mem_access_size;
+ offs = descr->mem_access_offs;
+ }
+ else
+ {
+ const struct ffa_mem_transaction_1_0 *descr;
+
+ if ( blen < sizeof(*descr) )
+ return FFA_RET_INVALID_PARAMETERS;
+
+ descr = buf;
+ trans->sender_id = descr->sender_id;
+ mem_reg_attr = descr->mem_reg_attr;
+ flags = descr->flags;
+ trans->global_handle = descr->global_handle;
+ trans->tag = descr->tag;
+
+ count = descr->mem_access_count;
+ size = sizeof(struct ffa_mem_access);
+ offs = offsetof(struct ffa_mem_transaction_1_0, mem_access_array);
+ }
+ /*
+ * Make sure that "descr" which is shared with the guest isn't accessed
+ * again after this point.
+ */
+ barrier();
+
+ /*
+ * We're doing a rough check to see that no information is lost when
+ * tranfering the values into a struct ffa_mem_transaction_x below. The
+ * fields in struct ffa_mem_transaction_x are wide enough to hold any
+ * valid value so being out of range means that something is wrong.
+ */
+ if ( mem_reg_attr > UINT8_MAX || flags > UINT8_MAX || size > UINT8_MAX ||
+ count > UINT8_MAX || offs > UINT16_MAX )
+ return FFA_RET_INVALID_PARAMETERS;
+
+ /* Check that the endpoint memory access descriptor array fits */
+ if ( size * count + offs > blen )
+ return FFA_RET_INVALID_PARAMETERS;
+
+ trans->mem_reg_attr = mem_reg_attr;
+ trans->flags = flags;
+ trans->mem_access_size = size;
+ trans->mem_access_count = count;
+ trans->mem_access_offs = offs;
+
+ return 0;
+}
+
+static int add_mem_share_frag(struct mem_frag_state *s, unsigned int offs,
+ unsigned int frag_len)
+{
+ struct domain *d = current->domain;
+ unsigned int o = offs;
+ unsigned int l;
+ int ret;
+
+ if ( frag_len < o )
+ return FFA_RET_INVALID_PARAMETERS;
+
+ /* Fill up the first struct ffa_address_range */
+ l = min_t(unsigned int, frag_len - o, sizeof(s->range) - s->range_offset);
+ memcpy((uint8_t *)&s->range + s->range_offset, s->buf + o, l);
+ s->range_offset += l;
+ o += l;
+ if ( s->range_offset != sizeof(s->range) )
+ goto out;
+ s->range_offset = 0;
+
+ while ( true )
+ {
+ ret = get_shm_pages(d, s->shm, &s->range, 1, s->current_page_idx,
+ &s->current_page_idx);
+ if ( ret )
+ return ret;
+ if ( s->range_count == 1 )
+ return 0;
+ s->range_count--;
+ if ( frag_len - o < sizeof(s->range) )
+ break;
+ memcpy(&s->range, s->buf + o, sizeof(s->range));
+ o += sizeof(s->range);
+ }
+
+ /* Collect any remaining bytes for the next struct ffa_address_range */
+ s->range_offset = frag_len - o;
+ memcpy(&s->range, s->buf + o, frag_len - o);
+out:
+ s->frag_offset += frag_len;
+
+ return s->frag_offset;
+}
+
+static void handle_mem_share(struct cpu_user_regs *regs)
+{
+ static uint64_t next_handle = FFA_HANDLE_HYP_FLAG;
+ uint32_t tot_len = get_user_reg(regs, 1);
+ uint32_t frag_len = get_user_reg(regs, 2);
+ uint64_t addr = get_user_reg(regs, 3);
+ uint32_t page_count = get_user_reg(regs, 4);
+ const struct ffa_mem_region *region_descr;
+ const struct ffa_mem_access *mem_access;
+ struct ffa_mem_transaction_x trans;
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+ struct ffa_shm_mem *shm = NULL;
+ unsigned int last_page_idx = 0;
+ register_t handle_hi = 0;
+ register_t handle_lo = 0;
+ int ret = FFA_RET_DENIED;
+ uint32_t range_count;
+ uint32_t region_offs;
+
+ /*
+ * We're only accepting memory transaction descriptors via the rx/tx
+ * buffer.
+ */
+ if ( addr )
+ {
+ ret = FFA_RET_NOT_SUPPORTED;
+ goto out_unlock;
+ }
+
+ /* Check that fragment length doesn't exceed total length */
+ if ( frag_len > tot_len )
+ {
+ ret = FFA_RET_INVALID_PARAMETERS;
+ goto out_unlock;
+ }
+
+ spin_lock(&ctx->lock);
+
+ if ( frag_len > ctx->page_count * FFA_PAGE_SIZE )
+ goto out_unlock;
+
+ if ( !ffa_page_count )
+ {
+ ret = FFA_RET_NO_MEMORY;
+ goto out_unlock;
+ }
+
+ ret = read_mem_transaction(ctx->guest_vers, ctx->tx, frag_len, &trans);
+ if ( ret )
+ goto out_unlock;
+
+ if ( trans.mem_reg_attr != FFA_NORMAL_MEM_REG_ATTR )
+ {
+ ret = FFA_RET_NOT_SUPPORTED;
+ goto out;
+ }
+
+ /* Only supports sharing it with one SP for now */
+ if ( trans.mem_access_count != 1 )
+ {
+ ret = FFA_RET_NOT_SUPPORTED;
+ goto out_unlock;
+ }
+
+ if ( trans.sender_id != get_vm_id(d) )
+ {
+ ret = FFA_RET_INVALID_PARAMETERS;
+ goto out_unlock;
+ }
+
+ /* Check that it fits in the supplied data */
+ if ( trans.mem_access_offs + trans.mem_access_size > frag_len )
+ goto out_unlock;
+
+ mem_access = ctx->tx + trans.mem_access_offs;
+ if ( read_atomic(&mem_access->access_perm.perm) != FFA_MEM_ACC_RW )
+ {
+ ret = FFA_RET_NOT_SUPPORTED;
+ goto out_unlock;
+ }
+
+ region_offs = read_atomic(&mem_access->region_offs);
+ if ( sizeof(*region_descr) + region_offs > frag_len )
+ {
+ ret = FFA_RET_NOT_SUPPORTED;
+ goto out_unlock;
+ }
+
+ region_descr = ctx->tx + region_offs;
+ range_count = read_atomic(®ion_descr->address_range_count);
+ page_count = read_atomic(®ion_descr->total_page_count);
+
+ shm = alloc_ffa_shm_mem(ctx, page_count);
+ if ( !shm )
+ {
+ ret = FFA_RET_NO_MEMORY;
+ goto out;
+ }
+ shm->sender_id = trans.sender_id;
+ shm->ep_id = read_atomic(&mem_access->access_perm.endpoint_id);
+
+ if ( frag_len != tot_len )
+ {
+ struct mem_frag_state *s = xzalloc(struct mem_frag_state);
+
+ if ( !s )
+ {
+ ret = FFA_RET_NO_MEMORY;
+ goto out;
+ }
+ s->shm = shm;
+ s->range_count = range_count;
+ s->buf = ctx->tx;
+ s->buf_size = ffa_page_count * FFA_PAGE_SIZE;
+ ret = add_mem_share_frag(s, sizeof(*region_descr) + region_offs,
+ frag_len);
+ if ( ret <= 0 )
+ {
+ xfree(s);
+ if ( ret < 0 )
+ goto out;
+ }
+ else
+ {
+ shm->handle = next_handle++;
+ uint64_to_regpair(&handle_hi, &handle_lo, shm->handle);
+ list_add_tail(&s->list, &ctx->frag_list);
+ }
+ goto out_unlock;
+ }
+
+ /*
+ * Check that the Composite memory region descriptor fits.
+ */
+ if ( sizeof(*region_descr) + region_offs +
+ range_count * sizeof(struct ffa_address_range) > frag_len )
+ {
+ ret = FFA_RET_INVALID_PARAMETERS;
+ goto out;
+ }
+
+ ret = get_shm_pages(d, shm, region_descr->address_range_array, range_count,
+ 0, &last_page_idx);
+ if ( ret )
+ goto out;
+ if ( last_page_idx != shm->page_count )
+ {
+ ret = FFA_RET_INVALID_PARAMETERS;
+ goto out;
+ }
+
+ /* Note that share_shm() uses our tx buffer */
+ spin_lock(&ffa_tx_buffer_lock);
+ ret = share_shm(shm);
+ spin_unlock(&ffa_tx_buffer_lock);
+ if ( ret )
+ goto out;
+
+ list_add_tail(&shm->list, &ctx->shm_list);
+
+ uint64_to_regpair(&handle_hi, &handle_lo, shm->handle);
+
+out:
+ if ( ret )
+ free_ffa_shm_mem(ctx, shm);
+out_unlock:
+ spin_unlock(&ctx->lock);
+
+ if ( ret > 0 )
+ set_regs_frag_rx(regs, handle_lo, handle_hi, ret, trans.sender_id);
+ else if ( ret == 0)
+ set_regs_success(regs, handle_lo, handle_hi);
+ else
+ set_regs_error(regs, ret);
+}
+
+static struct mem_frag_state *find_frag_state(struct ffa_ctx *ctx,
+ uint64_t handle)
+{
+ struct mem_frag_state *s;
+
+ list_for_each_entry(s, &ctx->frag_list, list)
+ if ( s->shm->handle == handle )
+ return s;
+
+ return NULL;
+}
+
+static void handle_mem_frag_tx(struct cpu_user_regs *regs)
+{
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+ uint32_t frag_len = get_user_reg(regs, 3);
+ uint32_t handle_lo = get_user_reg(regs, 1);
+ uint32_t handle_hi = get_user_reg(regs, 2);
+ uint64_t handle = regpair_to_uint64(handle_hi, handle_lo);
+ struct mem_frag_state *s;
+ uint16_t sender_id = 0;
+ int ret;
+
+ spin_lock(&ctx->lock);
+ s = find_frag_state(ctx, handle);
+ if ( !s )
+ {
+ ret = FFA_RET_INVALID_PARAMETERS;
+ goto out;
+ }
+ sender_id = s->shm->sender_id;
+
+ if ( frag_len > s->buf_size )
+ {
+ ret = FFA_RET_INVALID_PARAMETERS;
+ goto out;
+ }
+
+ ret = add_mem_share_frag(s, 0, frag_len);
+ if ( ret == 0 )
+ {
+ /* Note that share_shm() uses our tx buffer */
+ spin_lock(&ffa_tx_buffer_lock);
+ ret = share_shm(s->shm);
+ spin_unlock(&ffa_tx_buffer_lock);
+ if ( ret == 0 )
+ list_add_tail(&s->shm->list, &ctx->shm_list);
+ else
+ free_ffa_shm_mem(ctx, s->shm);
+ }
+ else if ( ret < 0 )
+ free_ffa_shm_mem(ctx, s->shm);
+ list_del(&s->list);
+ xfree(s);
+out:
+ spin_unlock(&ctx->lock);
+
+ if ( ret > 0 )
+ set_regs_frag_rx(regs, handle_lo, handle_hi, ret, sender_id);
+ else if ( ret == 0)
+ set_regs_success(regs, handle_lo, handle_hi);
+ else
+ set_regs_error(regs, ret);
+}
+
+static int handle_mem_reclaim(uint64_t handle, uint32_t flags)
+{
+ struct domain *d = current->domain;
+ struct ffa_ctx *ctx = d->arch.ffa;
+ struct ffa_shm_mem *shm;
+ register_t handle_hi;
+ register_t handle_lo;
+ int ret;
+
+ spin_lock(&ctx->lock);
+ list_for_each_entry(shm, &ctx->shm_list, list)
+ {
+ if ( shm->handle == handle )
+ goto found_it;
+ }
+ shm = NULL;
+ ret = FFA_RET_INVALID_PARAMETERS;
+ goto out;
+found_it:
+
+ uint64_to_regpair(&handle_hi, &handle_lo, handle);
+ ret = ffa_mem_reclaim(handle_lo, handle_hi, flags);
+ if ( ret )
+ {
+ shm = NULL;
+ goto out;
+ }
+
+ list_del(&shm->list);
+
+out:
+ free_ffa_shm_mem(ctx, shm);
+ spin_unlock(&ctx->lock);
+
+
+ return ret;
+}
+
bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
{
struct domain *d = current->domain;
@@ -733,6 +1547,24 @@ bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
#endif
handle_msg_send_direct_req(regs, fid);
return true;
+ case FFA_MEM_SHARE_32:
+#ifdef CONFIG_ARM_64
+ case FFA_MEM_SHARE_64:
+#endif
+ handle_mem_share(regs);
+ return true;
+ case FFA_MEM_RECLAIM:
+ e = handle_mem_reclaim(regpair_to_uint64(get_user_reg(regs, 2),
+ get_user_reg(regs, 1)),
+ get_user_reg(regs, 3));
+ if ( e )
+ set_regs_error(regs, e);
+ else
+ set_regs_success(regs, 0, 0);
+ return true;
+ case FFA_MEM_FRAG_TX:
+ handle_mem_frag_tx(regs);
+ return true;
default:
printk(XENLOG_ERR "ffa: unhandled fid 0x%x\n", fid);
@@ -768,6 +1600,9 @@ int ffa_domain_init(struct domain *d, bool ffa_enabled)
}
}
+ INIT_LIST_HEAD(&ctx->frag_list);
+ INIT_LIST_HEAD(&ctx->shm_list);
+
d->arch.ffa = ctx;
return 0;
@@ -923,6 +1758,9 @@ static int __init ffa_init(void)
!check_mandatory_feature(FFA_RXTX_MAP_32) ||
#endif
!check_mandatory_feature(FFA_RXTX_UNMAP) ||
+ !check_mandatory_feature(FFA_MEM_SHARE_32) ||
+ !check_mandatory_feature(FFA_MEM_FRAG_TX) ||
+ !check_mandatory_feature(FFA_MEM_RECLAIM) ||
!check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
return 0;
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
2022-08-18 10:55 ` [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers Jens Wiklander
@ 2022-08-18 13:44 ` Bertrand Marquis
2022-08-18 14:31 ` Julien Grall
2022-08-24 8:04 ` Jens Wiklander
2022-08-23 6:31 ` Jiamei Xie
1 sibling, 2 replies; 22+ messages in thread
From: Bertrand Marquis @ 2022-08-18 13:44 UTC (permalink / raw)
To: Jens Wiklander
Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Anthony PERARD, Juergen Gross, Wei Liu, Luca Fancellu
Hi Jens,
> On 18 Aug 2022, at 11:55, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> SMCCC v1.2 [1] AArch64 allows x0-x17 to be used as both parameter
> registers and result registers for the SMC and HVC instructions.
>
> Arm Firmware Framework for Armv8-A specification makes use of x0-x7 as
> parameter and result registers.
>
> Let us add new interface to support this extended set of input/output
> registers.
>
> This is based on 3fdc0cb59d97 ("arm64: smccc: Add support for SMCCCv1.2
> extended input/output registers") by Sudeep Holla from the Linux kernel
>
> The SMCCC version reported to the VM is bumped to 1.2 in order to support
> handling FF-A messages.
With this patch, you add something so that you could call SMCCCv1.2 but in practice you are not using it anywhere.
I do not think this patch should bump the version we present to guests.
>
> [1] https://developer.arm.com/documentation/den0028/c/?lang=en
>
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
> xen/arch/arm/arm64/asm-offsets.c | 9 +++++++
> xen/arch/arm/arm64/smc.S | 43 ++++++++++++++++++++++++++++++++
> xen/arch/arm/include/asm/smccc.h | 40 +++++++++++++++++++++++++++++
> xen/arch/arm/vsmc.c | 2 +-
> 4 files changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
> index 280ddb55bfd4..1721e1ed26e1 100644
> --- a/xen/arch/arm/arm64/asm-offsets.c
> +++ b/xen/arch/arm/arm64/asm-offsets.c
> @@ -56,6 +56,15 @@ void __dummy__(void)
> BLANK();
> OFFSET(SMCCC_RES_a0, struct arm_smccc_res, a0);
> OFFSET(SMCCC_RES_a2, struct arm_smccc_res, a2);
> + OFFSET(ARM_SMCCC_1_2_REGS_X0_OFFS, struct arm_smccc_1_2_regs, a0);
> + OFFSET(ARM_SMCCC_1_2_REGS_X2_OFFS, struct arm_smccc_1_2_regs, a2);
> + OFFSET(ARM_SMCCC_1_2_REGS_X4_OFFS, struct arm_smccc_1_2_regs, a4);
> + OFFSET(ARM_SMCCC_1_2_REGS_X6_OFFS, struct arm_smccc_1_2_regs, a6);
> + OFFSET(ARM_SMCCC_1_2_REGS_X8_OFFS, struct arm_smccc_1_2_regs, a8);
> + OFFSET(ARM_SMCCC_1_2_REGS_X10_OFFS, struct arm_smccc_1_2_regs, a10);
> + OFFSET(ARM_SMCCC_1_2_REGS_X12_OFFS, struct arm_smccc_1_2_regs, a12);
> + OFFSET(ARM_SMCCC_1_2_REGS_X14_OFFS, struct arm_smccc_1_2_regs, a14);
> + OFFSET(ARM_SMCCC_1_2_REGS_X16_OFFS, struct arm_smccc_1_2_regs, a16);
> }
>
> /*
> diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
> index 91bae62dd4d2..c546192e7f2d 100644
> --- a/xen/arch/arm/arm64/smc.S
> +++ b/xen/arch/arm/arm64/smc.S
> @@ -27,3 +27,46 @@ ENTRY(__arm_smccc_1_0_smc)
> stp x2, x3, [x4, #SMCCC_RES_a2]
> 1:
> ret
> +
> +
Please only add one line only here
> +/*
> + * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> + * struct arm_smccc_1_2_regs *res)
> + */
> +ENTRY(arm_smccc_1_2_smc)
> + /* Save `res` and free a GPR that won't be clobbered */
The comment here should be fixed, you are clobbering x19 hence you need to save it.
> + stp x1, x19, [sp, #-16]!
> +
> + /* Ensure `args` won't be clobbered while loading regs in next step */
> + mov x19, x0
You do not need to save args (and no code is restoring it).
> +
> + /* Load the registers x0 - x17 from the struct arm_smccc_1_2_regs */
> + ldp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> + ldp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> + ldp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> + ldp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> + ldp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> + ldp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> + ldp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> + ldp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
> + ldp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
> +
> + smc #0
> +
> + /* Load the `res` from the stack */
> + ldr x19, [sp]
> +
> + /* Store the registers x0 - x17 into the result structure */
> + stp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> + stp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> + stp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> + stp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> + stp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> + stp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> + stp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> + stp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
> + stp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
> +
> + /* Restore original x19 */
> + ldp xzr, x19, [sp], #16
You should use ldr and just load x19 value here.
> + ret
> diff --git a/xen/arch/arm/include/asm/smccc.h b/xen/arch/arm/include/asm/smccc.h
> index b3dbeecc90ad..b5e3f67eb34e 100644
> --- a/xen/arch/arm/include/asm/smccc.h
> +++ b/xen/arch/arm/include/asm/smccc.h
> @@ -33,6 +33,7 @@
>
> #define ARM_SMCCC_VERSION_1_0 SMCCC_VERSION(1, 0)
> #define ARM_SMCCC_VERSION_1_1 SMCCC_VERSION(1, 1)
> +#define ARM_SMCCC_VERSION_1_2 SMCCC_VERSION(1, 2)
>
> /*
> * This file provides common defines for ARM SMC Calling Convention as
> @@ -265,6 +266,45 @@ void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2,
> else \
> arm_smccc_1_0_smc(__VA_ARGS__); \
> } while ( 0 )
> +
> +/**
> + * struct arm_smccc_1_2_regs - Arguments for or Results from SMC call
> + * @a0-a17 argument values from registers 0 to 17
> + */
> +struct arm_smccc_1_2_regs {
> + unsigned long a0;
> + unsigned long a1;
> + unsigned long a2;
> + unsigned long a3;
> + unsigned long a4;
> + unsigned long a5;
> + unsigned long a6;
> + unsigned long a7;
> + unsigned long a8;
> + unsigned long a9;
> + unsigned long a10;
> + unsigned long a11;
> + unsigned long a12;
> + unsigned long a13;
> + unsigned long a14;
> + unsigned long a15;
> + unsigned long a16;
> + unsigned long a17;
> +};
> +
> +/**
> + * arm_smccc_1_2_smc() - make SMC calls
> + * @args: arguments passed via struct arm_smccc_1_2_regs
> + * @res: result values via struct arm_smccc_1_2_regs
> + *
> + * This function is used to make SMC calls following SMC Calling Convention
> + * v1.2 or above. The content of the supplied param are copied from the
> + * structure to registers prior to the SMC instruction. The return values
> + * are updated with the content from registers on return from the SMC
> + * instruction.
> + */
> +void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> + struct arm_smccc_1_2_regs *res);
> #endif /* CONFIG_ARM_64 */
>
> #endif /* __ASSEMBLY__ */
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index 676740ef1520..6f90c08a6304 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -93,7 +93,7 @@ static bool handle_arch(struct cpu_user_regs *regs)
> switch ( fid )
> {
> case ARM_SMCCC_VERSION_FID:
> - set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_1);
> + set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_2);
As said for the commit message, I do not see what changes are making Xen providing 1.2 interface at this stage.
Regards
Bertrand
> return true;
>
> case ARM_SMCCC_ARCH_FEATURES_FID:
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
2022-08-18 13:44 ` Bertrand Marquis
@ 2022-08-18 14:31 ` Julien Grall
2022-08-18 15:55 ` Bertrand Marquis
2022-08-24 8:04 ` Jens Wiklander
1 sibling, 1 reply; 22+ messages in thread
From: Julien Grall @ 2022-08-18 14:31 UTC (permalink / raw)
To: Bertrand Marquis, Jens Wiklander
Cc: xen-devel, Stefano Stabellini, Volodymyr Babchuk, Anthony PERARD,
Juergen Gross, Wei Liu, Luca Fancellu
Hi Bertrand,
On 18/08/2022 14:44, Bertrand Marquis wrote:
>> On 18 Aug 2022, at 11:55, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>>
>> SMCCC v1.2 [1] AArch64 allows x0-x17 to be used as both parameter
>> registers and result registers for the SMC and HVC instructions.
>>
>> Arm Firmware Framework for Armv8-A specification makes use of x0-x7 as
>> parameter and result registers.
>>
>> Let us add new interface to support this extended set of input/output
>> registers.
>>
>> This is based on 3fdc0cb59d97 ("arm64: smccc: Add support for SMCCCv1.2
>> extended input/output registers") by Sudeep Holla from the Linux kernel
>>
>> The SMCCC version reported to the VM is bumped to 1.2 in order to support
>> handling FF-A messages.
>
> With this patch, you add something so that you could call SMCCCv1.2 but in practice you are not using it anywhere.
> I do not think this patch should bump the version we present to guests.
IMHO, this is better to add it here rather than in a FFA specific patch.
Otherwise, one could raise the question of why we are adding wrapper
when they are not used?
>
>>
>> [1] https://developer.arm.com/documentation/den0028/c/?lang=en
>>
>> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
>> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
>> ---
>> xen/arch/arm/arm64/asm-offsets.c | 9 +++++++
>> xen/arch/arm/arm64/smc.S | 43 ++++++++++++++++++++++++++++++++
>> xen/arch/arm/include/asm/smccc.h | 40 +++++++++++++++++++++++++++++
>> xen/arch/arm/vsmc.c | 2 +-
>> 4 files changed, 93 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
>> index 280ddb55bfd4..1721e1ed26e1 100644
>> --- a/xen/arch/arm/arm64/asm-offsets.c
>> +++ b/xen/arch/arm/arm64/asm-offsets.c
>> @@ -56,6 +56,15 @@ void __dummy__(void)
>> BLANK();
>> OFFSET(SMCCC_RES_a0, struct arm_smccc_res, a0);
>> OFFSET(SMCCC_RES_a2, struct arm_smccc_res, a2);
>> + OFFSET(ARM_SMCCC_1_2_REGS_X0_OFFS, struct arm_smccc_1_2_regs, a0);
>> + OFFSET(ARM_SMCCC_1_2_REGS_X2_OFFS, struct arm_smccc_1_2_regs, a2);
>> + OFFSET(ARM_SMCCC_1_2_REGS_X4_OFFS, struct arm_smccc_1_2_regs, a4);
>> + OFFSET(ARM_SMCCC_1_2_REGS_X6_OFFS, struct arm_smccc_1_2_regs, a6);
>> + OFFSET(ARM_SMCCC_1_2_REGS_X8_OFFS, struct arm_smccc_1_2_regs, a8);
>> + OFFSET(ARM_SMCCC_1_2_REGS_X10_OFFS, struct arm_smccc_1_2_regs, a10);
>> + OFFSET(ARM_SMCCC_1_2_REGS_X12_OFFS, struct arm_smccc_1_2_regs, a12);
>> + OFFSET(ARM_SMCCC_1_2_REGS_X14_OFFS, struct arm_smccc_1_2_regs, a14);
>> + OFFSET(ARM_SMCCC_1_2_REGS_X16_OFFS, struct arm_smccc_1_2_regs, a16);
>> }
>>
>> /*
>> diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
>> index 91bae62dd4d2..c546192e7f2d 100644
>> --- a/xen/arch/arm/arm64/smc.S
>> +++ b/xen/arch/arm/arm64/smc.S
>> @@ -27,3 +27,46 @@ ENTRY(__arm_smccc_1_0_smc)
>> stp x2, x3, [x4, #SMCCC_RES_a2]
>> 1:
>> ret
>> +
>> +
>
> Please only add one line only here
>
>> +/*
>> + * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
>> + * struct arm_smccc_1_2_regs *res)
>> + */
>> +ENTRY(arm_smccc_1_2_smc)
>> + /* Save `res` and free a GPR that won't be clobbered */
>
> The comment here should be fixed, you are clobbering x19 hence you need to save it.
The comment is correct. x19 is one of the few registers that will not be
clobbered by the SMC call. But we still need a register below to store
'args', so we need to free it (what you call clobber).
>
>> + stp x1, x19, [sp, #-16]!
>> +
>> + /* Ensure `args` won't be clobbered while loading regs in next step */
>> + mov x19, x0
>
> You do not need to save args (and no code is restoring it).
The next instruction will overwrite x0. So if you don't save 'x0' to
'x19' then you will not be able to load the rest of the registers.
>
>> +
>> + /* Load the registers x0 - x17 from the struct arm_smccc_1_2_regs */
>> + ldp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
>> + ldp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
>> + ldp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
>> + ldp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
>> + ldp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
>> + ldp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
>> + ldp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
>> + ldp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
>> + ldp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
>> +
>> + smc #0
>> +
>> + /* Load the `res` from the stack */
>> + ldr x19, [sp]
>> +
>> + /* Store the registers x0 - x17 into the result structure */
>> + stp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
>> + stp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
>> + stp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
>> + stp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
>> + stp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
>> + stp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
>> + stp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
>> + stp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
>> + stp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
>> +
>> + /* Restore original x19 */
>> + ldp xzr, x19, [sp], #16
>
> You should use ldr and just load x19 value here.
AFAIU, this would mean an extra instruction to increment sp by 8
(covering the xzr register).
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
2022-08-18 14:31 ` Julien Grall
@ 2022-08-18 15:55 ` Bertrand Marquis
2022-08-18 17:31 ` Julien Grall
0 siblings, 1 reply; 22+ messages in thread
From: Bertrand Marquis @ 2022-08-18 15:55 UTC (permalink / raw)
To: Julien Grall
Cc: Jens Wiklander, xen-devel, Stefano Stabellini, Volodymyr Babchuk,
Anthony PERARD, Juergen Gross, Wei Liu, Luca Fancellu
Hi Julien,
> On 18 Aug 2022, at 15:31, Julien Grall <julien@xen.org> wrote:
>
> Hi Bertrand,
>
> On 18/08/2022 14:44, Bertrand Marquis wrote:
>>> On 18 Aug 2022, at 11:55, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>>>
>>> SMCCC v1.2 [1] AArch64 allows x0-x17 to be used as both parameter
>>> registers and result registers for the SMC and HVC instructions.
>>>
>>> Arm Firmware Framework for Armv8-A specification makes use of x0-x7 as
>>> parameter and result registers.
>>>
>>> Let us add new interface to support this extended set of input/output
>>> registers.
>>>
>>> This is based on 3fdc0cb59d97 ("arm64: smccc: Add support for SMCCCv1.2
>>> extended input/output registers") by Sudeep Holla from the Linux kernel
>>>
>>> The SMCCC version reported to the VM is bumped to 1.2 in order to support
>>> handling FF-A messages.
>> With this patch, you add something so that you could call SMCCCv1.2 but in practice you are not using it anywhere.
>> I do not think this patch should bump the version we present to guests.
>
> IMHO, this is better to add it here rather than in a FFA specific patch. Otherwise, one could raise the question of why we are adding wrapper when they are not used?
I was more thinking of pushing this until we have something compatible with 1.2 but I get the argument so ok.
>
>>>
>>> [1] https://developer.arm.com/documentation/den0028/c/?lang=en
>>>
>>> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
>>> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
>>> ---
>>> xen/arch/arm/arm64/asm-offsets.c | 9 +++++++
>>> xen/arch/arm/arm64/smc.S | 43 ++++++++++++++++++++++++++++++++
>>> xen/arch/arm/include/asm/smccc.h | 40 +++++++++++++++++++++++++++++
>>> xen/arch/arm/vsmc.c | 2 +-
>>> 4 files changed, 93 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
>>> index 280ddb55bfd4..1721e1ed26e1 100644
>>> --- a/xen/arch/arm/arm64/asm-offsets.c
>>> +++ b/xen/arch/arm/arm64/asm-offsets.c
>>> @@ -56,6 +56,15 @@ void __dummy__(void)
>>> BLANK();
>>> OFFSET(SMCCC_RES_a0, struct arm_smccc_res, a0);
>>> OFFSET(SMCCC_RES_a2, struct arm_smccc_res, a2);
>>> + OFFSET(ARM_SMCCC_1_2_REGS_X0_OFFS, struct arm_smccc_1_2_regs, a0);
>>> + OFFSET(ARM_SMCCC_1_2_REGS_X2_OFFS, struct arm_smccc_1_2_regs, a2);
>>> + OFFSET(ARM_SMCCC_1_2_REGS_X4_OFFS, struct arm_smccc_1_2_regs, a4);
>>> + OFFSET(ARM_SMCCC_1_2_REGS_X6_OFFS, struct arm_smccc_1_2_regs, a6);
>>> + OFFSET(ARM_SMCCC_1_2_REGS_X8_OFFS, struct arm_smccc_1_2_regs, a8);
>>> + OFFSET(ARM_SMCCC_1_2_REGS_X10_OFFS, struct arm_smccc_1_2_regs, a10);
>>> + OFFSET(ARM_SMCCC_1_2_REGS_X12_OFFS, struct arm_smccc_1_2_regs, a12);
>>> + OFFSET(ARM_SMCCC_1_2_REGS_X14_OFFS, struct arm_smccc_1_2_regs, a14);
>>> + OFFSET(ARM_SMCCC_1_2_REGS_X16_OFFS, struct arm_smccc_1_2_regs, a16);
>>> }
>>>
>>> /*
>>> diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
>>> index 91bae62dd4d2..c546192e7f2d 100644
>>> --- a/xen/arch/arm/arm64/smc.S
>>> +++ b/xen/arch/arm/arm64/smc.S
>>> @@ -27,3 +27,46 @@ ENTRY(__arm_smccc_1_0_smc)
>>> stp x2, x3, [x4, #SMCCC_RES_a2]
>>> 1:
>>> ret
>>> +
>>> +
>> Please only add one line only here
>>> +/*
>>> + * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
>>> + * struct arm_smccc_1_2_regs *res)
>>> + */
>>> +ENTRY(arm_smccc_1_2_smc)
>>> + /* Save `res` and free a GPR that won't be clobbered */
>> The comment here should be fixed, you are clobbering x19 hence you need to save it.
>
> The comment is correct. x19 is one of the few registers that will not be clobbered by the SMC call. But we still need a register below to store 'args', so we need to free it (what you call clobber).
Adding “by SMC call" would make this more clear
>
>>> + stp x1, x19, [sp, #-16]!
>>> +
>>> + /* Ensure `args` won't be clobbered while loading regs in next step */
>>> + mov x19, x0
>> You do not need to save args (and no code is restoring it).
>
> The next instruction will overwrite x0. So if you don't save 'x0' to 'x19' then you will not be able to load the rest of the registers.
>
Right
>>> +
>>> + /* Load the registers x0 - x17 from the struct arm_smccc_1_2_regs */
>>> + ldp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
>>> + ldp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
>>> + ldp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
>>> + ldp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
>>> + ldp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
>>> + ldp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
>>> + ldp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
>>> + ldp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
>>> + ldp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
>>> +
>>> + smc #0
>>> +
>>> + /* Load the `res` from the stack */
>>> + ldr x19, [sp]
>>> +
>>> + /* Store the registers x0 - x17 into the result structure */
>>> + stp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
>>> + stp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
>>> + stp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
>>> + stp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
>>> + stp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
>>> + stp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
>>> + stp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
>>> + stp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
>>> + stp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
>>> +
>>> + /* Restore original x19 */
>>> + ldp xzr, x19, [sp], #16
>> You should use ldr and just load x19 value here.
>
> AFAIU, this would mean an extra instruction to increment sp by 8 (covering the xzr register).
Right instruction is also incrementing sp, my bad.
Cheers
Bertrand
>
> Cheers,
>
> --
> Julien Grall
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
2022-08-18 15:55 ` Bertrand Marquis
@ 2022-08-18 17:31 ` Julien Grall
2022-08-24 8:11 ` Jens Wiklander
0 siblings, 1 reply; 22+ messages in thread
From: Julien Grall @ 2022-08-18 17:31 UTC (permalink / raw)
To: Bertrand Marquis
Cc: Jens Wiklander, xen-devel, Stefano Stabellini, Volodymyr Babchuk,
Anthony PERARD, Juergen Gross, Wei Liu, Luca Fancellu
Hi Bertrand,
On 18/08/2022 16:55, Bertrand Marquis wrote:
>> On 18 Aug 2022, at 15:31, Julien Grall <julien@xen.org> wrote:
>>>> +/*
>>>> + * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
>>>> + * struct arm_smccc_1_2_regs *res)
>>>> + */
>>>> +ENTRY(arm_smccc_1_2_smc)
>>>> + /* Save `res` and free a GPR that won't be clobbered */
>>> The comment here should be fixed, you are clobbering x19 hence you need to save it.
>>
>> The comment is correct. x19 is one of the few registers that will not be clobbered by the SMC call. But we still need a register below to store 'args', so we need to free it (what you call clobber).
>
> Adding “by SMC call" would make this more clear
I would be fine with that.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
2022-08-18 10:55 ` [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers Jens Wiklander
2022-08-18 13:44 ` Bertrand Marquis
@ 2022-08-23 6:31 ` Jiamei Xie
1 sibling, 0 replies; 22+ messages in thread
From: Jiamei Xie @ 2022-08-23 6:31 UTC (permalink / raw)
To: Jens Wiklander, xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Luca Fancellu
Hi
I build and run it on armv8a, and start dom0 with two cpus. Cpu off and on tests passed. It seems it don't break current cpu basic functions.
Best wishes
Jiamei Xie
> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> Jens Wiklander
> Sent: Thursday, August 18, 2022 6:56 PM
> To: xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall <julien@xen.org>;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Bertrand Marquis
> <Bertrand.Marquis@arm.com>; Anthony PERARD
> <anthony.perard@citrix.com>; Juergen Gross <jgross@suse.com>; Wei Liu
> <wl@xen.org>; Jens Wiklander <jens.wiklander@linaro.org>; Luca Fancellu
> <Luca.Fancellu@arm.com>
> Subject: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2
> extended input/output registers
>
> SMCCC v1.2 [1] AArch64 allows x0-x17 to be used as both parameter
> registers and result registers for the SMC and HVC instructions.
>
> Arm Firmware Framework for Armv8-A specification makes use of x0-x7 as
> parameter and result registers.
>
> Let us add new interface to support this extended set of input/output
> registers.
>
> This is based on 3fdc0cb59d97 ("arm64: smccc: Add support for SMCCCv1.2
> extended input/output registers") by Sudeep Holla from the Linux kernel
>
> The SMCCC version reported to the VM is bumped to 1.2 in order to support
> handling FF-A messages.
>
> [1] https://developer.arm.com/documentation/den0028/c/?lang=en
>
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
> xen/arch/arm/arm64/asm-offsets.c | 9 +++++++
> xen/arch/arm/arm64/smc.S | 43
> ++++++++++++++++++++++++++++++++
> xen/arch/arm/include/asm/smccc.h | 40
> +++++++++++++++++++++++++++++
> xen/arch/arm/vsmc.c | 2 +-
> 4 files changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-
> offsets.c
> index 280ddb55bfd4..1721e1ed26e1 100644
> --- a/xen/arch/arm/arm64/asm-offsets.c
> +++ b/xen/arch/arm/arm64/asm-offsets.c
> @@ -56,6 +56,15 @@ void __dummy__(void)
> BLANK();
> OFFSET(SMCCC_RES_a0, struct arm_smccc_res, a0);
> OFFSET(SMCCC_RES_a2, struct arm_smccc_res, a2);
> + OFFSET(ARM_SMCCC_1_2_REGS_X0_OFFS, struct arm_smccc_1_2_regs,
> a0);
> + OFFSET(ARM_SMCCC_1_2_REGS_X2_OFFS, struct arm_smccc_1_2_regs,
> a2);
> + OFFSET(ARM_SMCCC_1_2_REGS_X4_OFFS, struct arm_smccc_1_2_regs,
> a4);
> + OFFSET(ARM_SMCCC_1_2_REGS_X6_OFFS, struct arm_smccc_1_2_regs,
> a6);
> + OFFSET(ARM_SMCCC_1_2_REGS_X8_OFFS, struct arm_smccc_1_2_regs,
> a8);
> + OFFSET(ARM_SMCCC_1_2_REGS_X10_OFFS, struct arm_smccc_1_2_regs,
> a10);
> + OFFSET(ARM_SMCCC_1_2_REGS_X12_OFFS, struct arm_smccc_1_2_regs,
> a12);
> + OFFSET(ARM_SMCCC_1_2_REGS_X14_OFFS, struct arm_smccc_1_2_regs,
> a14);
> + OFFSET(ARM_SMCCC_1_2_REGS_X16_OFFS, struct arm_smccc_1_2_regs,
> a16);
> }
>
> /*
> diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
> index 91bae62dd4d2..c546192e7f2d 100644
> --- a/xen/arch/arm/arm64/smc.S
> +++ b/xen/arch/arm/arm64/smc.S
> @@ -27,3 +27,46 @@ ENTRY(__arm_smccc_1_0_smc)
> stp x2, x3, [x4, #SMCCC_RES_a2]
> 1:
> ret
> +
> +
> +/*
> + * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> + * struct arm_smccc_1_2_regs *res)
> + */
> +ENTRY(arm_smccc_1_2_smc)
> + /* Save `res` and free a GPR that won't be clobbered */
> + stp x1, x19, [sp, #-16]!
> +
> + /* Ensure `args` won't be clobbered while loading regs in next step */
> + mov x19, x0
> +
> + /* Load the registers x0 - x17 from the struct arm_smccc_1_2_regs */
> + ldp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> + ldp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> + ldp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> + ldp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> + ldp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> + ldp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> + ldp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> + ldp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
> + ldp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
> +
> + smc #0
> +
> + /* Load the `res` from the stack */
> + ldr x19, [sp]
> +
> + /* Store the registers x0 - x17 into the result structure */
> + stp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> + stp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> + stp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> + stp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> + stp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> + stp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> + stp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> + stp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
> + stp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
> +
> + /* Restore original x19 */
> + ldp xzr, x19, [sp], #16
> + ret
> diff --git a/xen/arch/arm/include/asm/smccc.h
> b/xen/arch/arm/include/asm/smccc.h
> index b3dbeecc90ad..b5e3f67eb34e 100644
> --- a/xen/arch/arm/include/asm/smccc.h
> +++ b/xen/arch/arm/include/asm/smccc.h
> @@ -33,6 +33,7 @@
>
> #define ARM_SMCCC_VERSION_1_0 SMCCC_VERSION(1, 0)
> #define ARM_SMCCC_VERSION_1_1 SMCCC_VERSION(1, 1)
> +#define ARM_SMCCC_VERSION_1_2 SMCCC_VERSION(1, 2)
>
> /*
> * This file provides common defines for ARM SMC Calling Convention as
> @@ -265,6 +266,45 @@ void __arm_smccc_1_0_smc(register_t a0,
> register_t a1, register_t a2,
> else \
> arm_smccc_1_0_smc(__VA_ARGS__); \
> } while ( 0 )
> +
> +/**
> + * struct arm_smccc_1_2_regs - Arguments for or Results from SMC call
> + * @a0-a17 argument values from registers 0 to 17
> + */
> +struct arm_smccc_1_2_regs {
> + unsigned long a0;
> + unsigned long a1;
> + unsigned long a2;
> + unsigned long a3;
> + unsigned long a4;
> + unsigned long a5;
> + unsigned long a6;
> + unsigned long a7;
> + unsigned long a8;
> + unsigned long a9;
> + unsigned long a10;
> + unsigned long a11;
> + unsigned long a12;
> + unsigned long a13;
> + unsigned long a14;
> + unsigned long a15;
> + unsigned long a16;
> + unsigned long a17;
> +};
> +
> +/**
> + * arm_smccc_1_2_smc() - make SMC calls
> + * @args: arguments passed via struct arm_smccc_1_2_regs
> + * @res: result values via struct arm_smccc_1_2_regs
> + *
> + * This function is used to make SMC calls following SMC Calling Convention
> + * v1.2 or above. The content of the supplied param are copied from the
> + * structure to registers prior to the SMC instruction. The return values
> + * are updated with the content from registers on return from the SMC
> + * instruction.
> + */
> +void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> + struct arm_smccc_1_2_regs *res);
> #endif /* CONFIG_ARM_64 */
>
> #endif /* __ASSEMBLY__ */
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index 676740ef1520..6f90c08a6304 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -93,7 +93,7 @@ static bool handle_arch(struct cpu_user_regs *regs)
> switch ( fid )
> {
> case ARM_SMCCC_VERSION_FID:
> - set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_1);
> + set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_2);
> return true;
>
> case ARM_SMCCC_ARCH_FEATURES_FID:
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
2022-08-18 13:44 ` Bertrand Marquis
2022-08-18 14:31 ` Julien Grall
@ 2022-08-24 8:04 ` Jens Wiklander
1 sibling, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-24 8:04 UTC (permalink / raw)
To: Bertrand Marquis
Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Anthony PERARD, Juergen Gross, Wei Liu, Luca Fancellu
On Thu, Aug 18, 2022 at 3:45 PM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
> Hi Jens,
>
> > On 18 Aug 2022, at 11:55, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > SMCCC v1.2 [1] AArch64 allows x0-x17 to be used as both parameter
> > registers and result registers for the SMC and HVC instructions.
> >
> > Arm Firmware Framework for Armv8-A specification makes use of x0-x7 as
> > parameter and result registers.
> >
> > Let us add new interface to support this extended set of input/output
> > registers.
> >
> > This is based on 3fdc0cb59d97 ("arm64: smccc: Add support for SMCCCv1.2
> > extended input/output registers") by Sudeep Holla from the Linux kernel
> >
> > The SMCCC version reported to the VM is bumped to 1.2 in order to support
> > handling FF-A messages.
>
> With this patch, you add something so that you could call SMCCCv1.2 but in practice you are not using it anywhere.
> I do not think this patch should bump the version we present to guests.
>
> >
> > [1] https://developer.arm.com/documentation/den0028/c/?lang=en
> >
> > Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > ---
> > xen/arch/arm/arm64/asm-offsets.c | 9 +++++++
> > xen/arch/arm/arm64/smc.S | 43 ++++++++++++++++++++++++++++++++
> > xen/arch/arm/include/asm/smccc.h | 40 +++++++++++++++++++++++++++++
> > xen/arch/arm/vsmc.c | 2 +-
> > 4 files changed, 93 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
> > index 280ddb55bfd4..1721e1ed26e1 100644
> > --- a/xen/arch/arm/arm64/asm-offsets.c
> > +++ b/xen/arch/arm/arm64/asm-offsets.c
> > @@ -56,6 +56,15 @@ void __dummy__(void)
> > BLANK();
> > OFFSET(SMCCC_RES_a0, struct arm_smccc_res, a0);
> > OFFSET(SMCCC_RES_a2, struct arm_smccc_res, a2);
> > + OFFSET(ARM_SMCCC_1_2_REGS_X0_OFFS, struct arm_smccc_1_2_regs, a0);
> > + OFFSET(ARM_SMCCC_1_2_REGS_X2_OFFS, struct arm_smccc_1_2_regs, a2);
> > + OFFSET(ARM_SMCCC_1_2_REGS_X4_OFFS, struct arm_smccc_1_2_regs, a4);
> > + OFFSET(ARM_SMCCC_1_2_REGS_X6_OFFS, struct arm_smccc_1_2_regs, a6);
> > + OFFSET(ARM_SMCCC_1_2_REGS_X8_OFFS, struct arm_smccc_1_2_regs, a8);
> > + OFFSET(ARM_SMCCC_1_2_REGS_X10_OFFS, struct arm_smccc_1_2_regs, a10);
> > + OFFSET(ARM_SMCCC_1_2_REGS_X12_OFFS, struct arm_smccc_1_2_regs, a12);
> > + OFFSET(ARM_SMCCC_1_2_REGS_X14_OFFS, struct arm_smccc_1_2_regs, a14);
> > + OFFSET(ARM_SMCCC_1_2_REGS_X16_OFFS, struct arm_smccc_1_2_regs, a16);
> > }
> >
> > /*
> > diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
> > index 91bae62dd4d2..c546192e7f2d 100644
> > --- a/xen/arch/arm/arm64/smc.S
> > +++ b/xen/arch/arm/arm64/smc.S
> > @@ -27,3 +27,46 @@ ENTRY(__arm_smccc_1_0_smc)
> > stp x2, x3, [x4, #SMCCC_RES_a2]
> > 1:
> > ret
> > +
> > +
>
> Please only add one line only here
OK, I'll fix.
Thanks,
Jens
>
> > +/*
> > + * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> > + * struct arm_smccc_1_2_regs *res)
> > + */
> > +ENTRY(arm_smccc_1_2_smc)
> > + /* Save `res` and free a GPR that won't be clobbered */
>
> The comment here should be fixed, you are clobbering x19 hence you need to save it.
>
> > + stp x1, x19, [sp, #-16]!
> > +
> > + /* Ensure `args` won't be clobbered while loading regs in next step */
> > + mov x19, x0
>
> You do not need to save args (and no code is restoring it).
>
> > +
> > + /* Load the registers x0 - x17 from the struct arm_smccc_1_2_regs */
> > + ldp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> > + ldp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> > + ldp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> > + ldp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> > + ldp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> > + ldp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> > + ldp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> > + ldp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
> > + ldp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
> > +
> > + smc #0
> > +
> > + /* Load the `res` from the stack */
> > + ldr x19, [sp]
> > +
> > + /* Store the registers x0 - x17 into the result structure */
> > + stp x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> > + stp x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> > + stp x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> > + stp x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> > + stp x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> > + stp x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> > + stp x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> > + stp x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
> > + stp x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
> > +
> > + /* Restore original x19 */
> > + ldp xzr, x19, [sp], #16
>
> You should use ldr and just load x19 value here.
>
> > + ret
> > diff --git a/xen/arch/arm/include/asm/smccc.h b/xen/arch/arm/include/asm/smccc.h
> > index b3dbeecc90ad..b5e3f67eb34e 100644
> > --- a/xen/arch/arm/include/asm/smccc.h
> > +++ b/xen/arch/arm/include/asm/smccc.h
> > @@ -33,6 +33,7 @@
> >
> > #define ARM_SMCCC_VERSION_1_0 SMCCC_VERSION(1, 0)
> > #define ARM_SMCCC_VERSION_1_1 SMCCC_VERSION(1, 1)
> > +#define ARM_SMCCC_VERSION_1_2 SMCCC_VERSION(1, 2)
> >
> > /*
> > * This file provides common defines for ARM SMC Calling Convention as
> > @@ -265,6 +266,45 @@ void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2,
> > else \
> > arm_smccc_1_0_smc(__VA_ARGS__); \
> > } while ( 0 )
> > +
> > +/**
> > + * struct arm_smccc_1_2_regs - Arguments for or Results from SMC call
> > + * @a0-a17 argument values from registers 0 to 17
> > + */
> > +struct arm_smccc_1_2_regs {
> > + unsigned long a0;
> > + unsigned long a1;
> > + unsigned long a2;
> > + unsigned long a3;
> > + unsigned long a4;
> > + unsigned long a5;
> > + unsigned long a6;
> > + unsigned long a7;
> > + unsigned long a8;
> > + unsigned long a9;
> > + unsigned long a10;
> > + unsigned long a11;
> > + unsigned long a12;
> > + unsigned long a13;
> > + unsigned long a14;
> > + unsigned long a15;
> > + unsigned long a16;
> > + unsigned long a17;
> > +};
> > +
> > +/**
> > + * arm_smccc_1_2_smc() - make SMC calls
> > + * @args: arguments passed via struct arm_smccc_1_2_regs
> > + * @res: result values via struct arm_smccc_1_2_regs
> > + *
> > + * This function is used to make SMC calls following SMC Calling Convention
> > + * v1.2 or above. The content of the supplied param are copied from the
> > + * structure to registers prior to the SMC instruction. The return values
> > + * are updated with the content from registers on return from the SMC
> > + * instruction.
> > + */
> > +void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> > + struct arm_smccc_1_2_regs *res);
> > #endif /* CONFIG_ARM_64 */
> >
> > #endif /* __ASSEMBLY__ */
> > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> > index 676740ef1520..6f90c08a6304 100644
> > --- a/xen/arch/arm/vsmc.c
> > +++ b/xen/arch/arm/vsmc.c
> > @@ -93,7 +93,7 @@ static bool handle_arch(struct cpu_user_regs *regs)
> > switch ( fid )
> > {
> > case ARM_SMCCC_VERSION_FID:
> > - set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_1);
> > + set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_2);
>
> As said for the commit message, I do not see what changes are making Xen providing 1.2 interface at this stage.
>
> Regards
> Bertrand
>
> > return true;
> >
> > case ARM_SMCCC_ARCH_FEATURES_FID:
> > --
> > 2.31.1
> >
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers
2022-08-18 17:31 ` Julien Grall
@ 2022-08-24 8:11 ` Jens Wiklander
0 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-08-24 8:11 UTC (permalink / raw)
To: Julien Grall
Cc: Bertrand Marquis, xen-devel, Stefano Stabellini,
Volodymyr Babchuk, Anthony PERARD, Juergen Gross, Wei Liu,
Luca Fancellu
Hi,
On Thu, Aug 18, 2022 at 7:31 PM Julien Grall <julien@xen.org> wrote:
>
> Hi Bertrand,
>
> On 18/08/2022 16:55, Bertrand Marquis wrote:
> >> On 18 Aug 2022, at 15:31, Julien Grall <julien@xen.org> wrote:
> >>>> +/*
> >>>> + * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> >>>> + * struct arm_smccc_1_2_regs *res)
> >>>> + */
> >>>> +ENTRY(arm_smccc_1_2_smc)
> >>>> + /* Save `res` and free a GPR that won't be clobbered */
> >>> The comment here should be fixed, you are clobbering x19 hence you need to save it.
> >>
> >> The comment is correct. x19 is one of the few registers that will not be clobbered by the SMC call. But we still need a register below to store 'args', so we need to free it (what you call clobber).
> >
> > Adding “by SMC call" would make this more clear
>
> I would be fine with that.
I'll update the comment.
Thanks,
Jens
>
> Cheers,
>
> --
> Julien Grall
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator
2022-08-18 10:55 ` [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator Jens Wiklander
@ 2022-09-05 22:17 ` Julien Grall
2022-09-06 14:57 ` Jens Wiklander
2022-09-05 22:25 ` Julien Grall
2022-09-06 9:44 ` Anthony PERARD
2 siblings, 1 reply; 22+ messages in thread
From: Julien Grall @ 2022-09-05 22:17 UTC (permalink / raw)
To: Jens Wiklander, xen-devel
Cc: Stefano Stabellini, Volodymyr Babchuk, Bertrand.Marquis,
Anthony PERARD, Juergen Gross, Wei Liu, Andrew Cooper
Hi Jens,
On 18/08/2022 11:55, Jens Wiklander wrote:
> Adds a FF-A version 1.1 [1] mediator to communicate with a Secure
> Partition in secure world.
>
> This commit brings in only the parts needed to negotiate FF-A version
> number with guest and SPMC.
>
> A guest configuration variable "ffa_enabled" is used to indicate if a guest
> is trusted to use FF-A.
>
> This is loosely based on the TEE mediator framework and the OP-TEE
> mediator.
>
> [1] https://developer.arm.com/documentation/den0077/latest
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
> SUPPORT.md | 7 +
> docs/man/xl.cfg.5.pod.in | 15 ++
> tools/include/libxl.h | 6 +
> tools/libs/light/libxl_arm.c | 3 +
> tools/libs/light/libxl_types.idl | 1 +
> tools/xl/xl_parse.c | 3 +
> xen/arch/arm/Kconfig | 11 +
> xen/arch/arm/Makefile | 1 +
> xen/arch/arm/domain.c | 10 +
> xen/arch/arm/domain_build.c | 1 +
> xen/arch/arm/ffa.c | 354 ++++++++++++++++++++++++++++++
> xen/arch/arm/include/asm/domain.h | 4 +
> xen/arch/arm/include/asm/ffa.h | 71 ++++++
> xen/arch/arm/vsmc.c | 17 +-
> xen/include/public/arch-arm.h | 2 +
> 15 files changed, 503 insertions(+), 3 deletions(-)
> create mode 100644 xen/arch/arm/ffa.c
> create mode 100644 xen/arch/arm/include/asm/ffa.h
>
> diff --git a/SUPPORT.md b/SUPPORT.md
> index 70e98964cbc0..215bb3c9043b 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -785,6 +785,13 @@ that covers the DMA of the device to be passed through.
>
> No support for QEMU backends in a 16K or 64K domain.
>
> +### ARM: Firmware Framework for Arm A-profile (FF-A) Mediator
> +
> + Status, Arm64: Tech Preview
> +
> +There are still some code paths where a vCPU may hog a pCPU longer than
> +necessary. The FF-A mediator is not yet implemented for Arm32.
> +
> ### ARM: Guest Device Tree support
>
> Status: Supported
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index b98d1613987e..234c036aecb1 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -1616,6 +1616,21 @@ This feature is a B<technology preview>.
>
> =back
>
> +=item B<ffa_enabled=BOOLEAN>
> +
> +B<Arm only.> Allow a guest to communicate via FF-A with Secure Partitions
> +(SP), default false.
> +
> +Currently is only a small subset of the FF-A specification supported. Just
> +enough to communicate with OP-TEE. In general all the basic things and
> +sharing memory with one SP. More advanced use cases where memory might be
> +shared or donated to multple SPs is not supported.
> +
> +See L<https://developer.arm.com/documentation/den0077/latest> for more
> +informantion about FF-A.
> +
> +This feature is a B<technology preview>.
> +
> =head2 Paravirtualised (PV) Guest Specific Options
>
> The following options apply only to Paravirtual (PV) guests.
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 7ce978e83c9a..4ab5a7b044d6 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -278,6 +278,12 @@
> */
> #define LIBXL_HAVE_BUILDINFO_ARCH_ARM_TEE 1
>
> +/*
> + * LIBXL_HAVE_BUILDINFO_ARM_FFA_ENABLED indicates that
> + * libxl_domain_build_info has the arm.ffa_enabled field.
> + */
> +#define LIBXL_HAVE_BUILDINFO_ARM_FFA_ENABLED 1
> +
> /*
> * LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing
> * 'soft reset' for domains and there is 'soft_reset' shutdown reason
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index eef1de093914..a985609861c7 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -101,6 +101,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> return ERROR_FAIL;
> }
>
> + config->arch.ffa_enabled =
> + libxl_defbool_val(d_config->b_info.arch_arm.ffa_enabled);
> +
> return 0;
> }
>
> diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
> index 2a42da2f7d78..bf4544bef399 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -646,6 +646,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>
> ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
> ("vuart", libxl_vuart_type),
> + ("ffa_enabled", libxl_defbool),
> ])),
> ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
> ])),
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index b98c0de378b6..e0e99ed8d2b1 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -2746,6 +2746,9 @@ skip_usbdev:
> exit(-ERROR_FAIL);
> }
> }
> + libxl_defbool_setdefault(&b_info->arch_arm.ffa_enabled, false);
> + xlu_cfg_get_defbool(config, "ffa_enabled",
> + &b_info->arch_arm.ffa_enabled, 0);
>
> parse_vkb_list(config, d_config);
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index be9eff014120..e57e1d3757e2 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -139,6 +139,17 @@ config TEE
>
> source "arch/arm/tee/Kconfig"
>
> +config FFA
> + bool "Enable FF-A mediator support" if EXPERT
> + default n
> + depends on ARM_64
> + help
> + This option enables a minimal FF-A mediator. The mediator is
> + generic as it follows the FF-A specification [1], but it only
> + implements a small subset of the specification.
> +
> + [1] https://developer.arm.com/documentation/den0077/latest
> +
> endmenu
>
> menu "ARM errata workaround via the alternative framework"
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index bb7a6151c13c..af0c69f793d4 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -20,6 +20,7 @@ obj-y += domain_build.init.o
> obj-y += domctl.o
> obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> obj-y += efi/
> +obj-$(CONFIG_FFA) += ffa.o
> obj-y += gic.o
> obj-y += gic-v2.o
> obj-$(CONFIG_GICV3) += gic-v3.o
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 8110c1df8638..a3f00e7e234d 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -27,6 +27,7 @@
> #include <asm/cpufeature.h>
> #include <asm/current.h>
> #include <asm/event.h>
> +#include <asm/ffa.h>
> #include <asm/gic.h>
> #include <asm/guest_atomics.h>
> #include <asm/irq.h>
> @@ -756,6 +757,9 @@ int arch_domain_create(struct domain *d,
> if ( (rc = tee_domain_init(d, config->arch.tee_type)) != 0 )
> goto fail;
>
> + if ( (rc = ffa_domain_init(d, config->arch.ffa_enabled)) != 0 )
> + goto fail;
> +
> update_domain_wallclock_time(d);
>
> /*
> @@ -998,6 +1002,7 @@ static int relinquish_memory(struct domain *d, struct page_list_head *list)
> enum {
> PROG_pci = 1,
> PROG_tee,
> + PROG_ffa,
> PROG_xen,
> PROG_page,
> PROG_mapping,
> @@ -1043,6 +1048,11 @@ int domain_relinquish_resources(struct domain *d)
>
> PROGRESS(tee):
> ret = tee_relinquish_resources(d);
> + if ( ret )
> + return ret;
> +
> + PROGRESS(ffa):
> + ret = ffa_relinquish_resources(d);
> if (ret )
> return ret;
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 7ddd16c26da5..d708f76356f7 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3450,6 +3450,7 @@ void __init create_dom0(void)
> if ( gic_number_lines() > 992 )
> printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
> dom0_cfg.arch.tee_type = tee_get_type();
> + dom0_cfg.arch.ffa_enabled = true;
> dom0_cfg.max_vcpus = dom0_max_vcpus();
>
> if ( iommu_enabled )
> diff --git a/xen/arch/arm/ffa.c b/xen/arch/arm/ffa.c
> new file mode 100644
> index 000000000000..b85c492928d2
> --- /dev/null
> +++ b/xen/arch/arm/ffa.c
> @@ -0,0 +1,354 @@
> +/*
> + * xen/arch/arm/ffa.c
> + *
> + * Arm Firmware Framework for ARMv8-A (FF-A) mediator
> + *
> + * Copyright (C) 2022 Linaro Limited
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms and conditions 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.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/domain_page.h>
> +#include <xen/errno.h>
> +#include <xen/init.h>
> +#include <xen/lib.h>
> +#include <xen/sched.h>
> +#include <xen/types.h>
> +#include <xen/sizes.h>
> +#include <xen/bitops.h>
> +
> +#include <asm/smccc.h>
> +#include <asm/event.h>
> +#include <asm/ffa.h>
> +#include <asm/regs.h>
> +
> +/* Error codes */
> +#define FFA_RET_OK 0
> +#define FFA_RET_NOT_SUPPORTED -1
> +#define FFA_RET_INVALID_PARAMETERS -2
> +#define FFA_RET_NO_MEMORY -3
> +#define FFA_RET_BUSY -4
> +#define FFA_RET_INTERRUPTED -5
> +#define FFA_RET_DENIED -6
> +#define FFA_RET_RETRY -7
> +#define FFA_RET_ABORTED -8
> +
> +/* FFA_VERSION helpers */
> +#define FFA_VERSION_MAJOR_SHIFT 16U
> +#define FFA_VERSION_MAJOR_MASK 0x7FFFU
> +#define FFA_VERSION_MINOR_SHIFT 0U
> +#define FFA_VERSION_MINOR_MASK 0xFFFFU
> +#define MAKE_FFA_VERSION(major, minor) \
> + ((((major) & FFA_VERSION_MAJOR_MASK) << FFA_VERSION_MAJOR_SHIFT) | \
> + ((minor) & FFA_VERSION_MINOR_MASK))
> +
> +#define FFA_MIN_VERSION MAKE_FFA_VERSION(1, 0)
> +#define FFA_VERSION_1_0 MAKE_FFA_VERSION(1, 0)
> +#define FFA_VERSION_1_1 MAKE_FFA_VERSION(1, 1)
> +
> +/*
> + * This is the version we want to use in communication with guests and SPs.
> + * During negotiation with a guest or a SP we may need to lower it for
> + * that particular guest or SP.
> + */
> +#define FFA_MY_VERSION_MAJOR 1U
> +#define FFA_MY_VERSION_MINOR 1U
> +#define FFA_MY_VERSION MAKE_FFA_VERSION(FFA_MY_VERSION_MAJOR, \
> + FFA_MY_VERSION_MINOR)
> +
> +#define FFA_PAGE_SIZE SZ_4K
> +
> +/*
> + * Limit for shared buffer size. Please note that this define limits
> + * number of pages. But user buffer can be not aligned to a page
> + * boundary. So it is possible that user would not be able to share
> + * exactly MAX_SHM_BUFFER_PG * PAGE_SIZE bytes.
> + *
> + * FF-A doesn't have any direct requirments on GlobalPlatform or vice
> + * versa, but an implementation can very well use FF-A in order to provide
> + * a GlobalPlatform interface on top.
> + *
> + * Global Platform specification for TEE requires that any TEE
> + * implementation should allow to share buffers with size of at least
> + * 512KB, which equals to 128 4kB pages. Due to align issue mentioned
> + * above, we need to increase this value to 129.
> + */
> +#define FFA_MAX_SHM_PAGE_COUNT 129
> +
> +/*
> + * Limits the number of shared buffers that guest can have at once. This
> + * is to prevent case, when guests tricks XEN into exhausting its own
> + * memory by allocating many small buffers. This value has been chosen
> + * arbitrary.
> + */
> +#define FFA_MAX_SHM_COUNT 32
> +
> +#define FFA_HANDLE_HYP_FLAG BIT(63, ULL)
> +
> +/* Memory attributes: Normal memory, Write-Back cacheable, Inner shareable */
> +#define FFA_NORMAL_MEM_REG_ATTR 0x2fU
> +
> +/* Memory access permissions: Read-write */
> +#define FFA_MEM_ACC_RW 0x2U
> +
> +/* Clear memory before mapping in receiver */
> +#define FFA_MEMORY_REGION_FLAG_CLEAR BIT(0, U)
> +/* Relayer may time slice this operation */
> +#define FFA_MEMORY_REGION_FLAG_TIME_SLICE BIT(1, U)
> +/* Clear memory after receiver relinquishes it */
> +#define FFA_MEMORY_REGION_FLAG_CLEAR_RELINQUISH BIT(2, U)
> +
> +/* Share memory transaction */
> +#define FFA_MEMORY_REGION_TRANSACTION_TYPE_SHARE (1U << 3)
> +
> +#define FFA_HANDLE_INVALID 0xffffffffffffffffULL
> +
> +/* Framework direct request/response */
> +#define FFA_MSG_FLAG_FRAMEWORK BIT(31, U)
> +#define FFA_MSG_TYPE_MASK 0xFFU;
> +#define FFA_MSG_PSCI 0x0U
> +#define FFA_MSG_SEND_VM_CREATED 0x4U
> +#define FFA_MSG_RESP_VM_CREATED 0x5U
> +#define FFA_MSG_SEND_VM_DESTROYED 0x6U
> +#define FFA_MSG_RESP_VM_DESTROYED 0x7U
> +
> +/*
> + * Flags used for the FFA_PARTITION_INFO_GET return message:
> + * BIT(0): Supports receipt of direct requests
> + * BIT(1): Can send direct requests
> + * BIT(2): Can send and receive indirect messages
> + * BIT(3): Supports receipt of notifications
> + * BIT(4-5): Partition ID is a PE endpoint ID
> + */
> +#define FFA_PART_PROP_DIRECT_REQ_RECV BIT(0, U)
> +#define FFA_PART_PROP_DIRECT_REQ_SEND BIT(1, U)
> +#define FFA_PART_PROP_INDIRECT_MSGS BIT(2, U)
> +#define FFA_PART_PROP_RECV_NOTIF BIT(3, U)
> +#define FFA_PART_PROP_IS_PE_ID (0U << 4)
> +#define FFA_PART_PROP_IS_SEPID_INDEP (1U << 4)
> +#define FFA_PART_PROP_IS_SEPID_DEP (2U << 4)
> +#define FFA_PART_PROP_IS_AUX_ID (3U << 4)
> +#define FFA_PART_PROP_NOTIF_CREATED BIT(6, U)
> +#define FFA_PART_PROP_NOTIF_DESTROYED BIT(7, U)
> +#define FFA_PART_PROP_AARCH64_STATE BIT(8, U)
> +
> +/*
> + * Flag used as parameter to FFA_PARTITION_INFO_GET to return partition
> + * count only.
> + */
> +#define FFA_PARTITION_INFO_GET_COUNT_FLAG BIT(0, U)
> +
> +/* Function IDs */
> +#define FFA_ERROR 0x84000060U
> +#define FFA_SUCCESS_32 0x84000061U
> +#define FFA_SUCCESS_64 0xC4000061U
> +#define FFA_INTERRUPT 0x84000062U
> +#define FFA_VERSION 0x84000063U
> +#define FFA_FEATURES 0x84000064U
> +#define FFA_RX_ACQUIRE 0x84000084U
> +#define FFA_RX_RELEASE 0x84000065U
> +#define FFA_RXTX_MAP_32 0x84000066U
> +#define FFA_RXTX_MAP_64 0xC4000066U
> +#define FFA_RXTX_UNMAP 0x84000067U
> +#define FFA_PARTITION_INFO_GET 0x84000068U
> +#define FFA_ID_GET 0x84000069U
> +#define FFA_SPM_ID_GET 0x84000085U
> +#define FFA_MSG_WAIT 0x8400006BU
> +#define FFA_MSG_YIELD 0x8400006CU
> +#define FFA_MSG_RUN 0x8400006DU
> +#define FFA_MSG_SEND2 0x84000086U
> +#define FFA_MSG_SEND_DIRECT_REQ_32 0x8400006FU
> +#define FFA_MSG_SEND_DIRECT_REQ_64 0xC400006FU
> +#define FFA_MSG_SEND_DIRECT_RESP_32 0x84000070U
> +#define FFA_MSG_SEND_DIRECT_RESP_64 0xC4000070U
> +#define FFA_MEM_DONATE_32 0x84000071U
> +#define FFA_MEM_DONATE_64 0xC4000071U
> +#define FFA_MEM_LEND_32 0x84000072U
> +#define FFA_MEM_LEND_64 0xC4000072U
> +#define FFA_MEM_SHARE_32 0x84000073U
> +#define FFA_MEM_SHARE_64 0xC4000073U
> +#define FFA_MEM_RETRIEVE_REQ_32 0x84000074U
> +#define FFA_MEM_RETRIEVE_REQ_64 0xC4000074U
> +#define FFA_MEM_RETRIEVE_RESP 0x84000075U
> +#define FFA_MEM_RELINQUISH 0x84000076U
> +#define FFA_MEM_RECLAIM 0x84000077U
> +#define FFA_MEM_FRAG_RX 0x8400007AU
> +#define FFA_MEM_FRAG_TX 0x8400007BU
> +#define FFA_MSG_SEND 0x8400006EU
> +#define FFA_MSG_POLL 0x8400006AU
> +
> +struct ffa_ctx {
> + uint32_t guest_vers;
> +};
> +
> +/* Negotiated FF-A version to use with the SPMC */
> +static uint32_t ffa_version __read_mostly;
> +
> +static bool ffa_get_version(uint32_t *vers)
> +{
> + const struct arm_smccc_1_2_regs arg = {
> + .a0 = FFA_VERSION,
> + .a1 = FFA_MY_VERSION,
> + };
> + struct arm_smccc_1_2_regs resp;
> +
> + arm_smccc_1_2_smc(&arg, &resp);
> + if ( resp.a0 == FFA_RET_NOT_SUPPORTED )
> + {
> + printk(XENLOG_ERR "ffa: FFA_VERSION returned not supported\n");
> + return false;
> + }
> +
> + *vers = resp.a0;
> +
> + return true;
> +}
> +
> +static u16 get_vm_id(const struct domain *d)
> +{
> + /* +1 since 0 is reserved for the hypervisor in FF-A */
> + return d->domain_id + 1;
> +}
> +
> +static void set_regs(struct cpu_user_regs *regs, register_t v0, register_t v1,
> + register_t v2, register_t v3, register_t v4, register_t v5,
> + register_t v6, register_t v7)
> +{
> + set_user_reg(regs, 0, v0);
> + set_user_reg(regs, 1, v1);
> + set_user_reg(regs, 2, v2);
> + set_user_reg(regs, 3, v3);
> + set_user_reg(regs, 4, v4);
> + set_user_reg(regs, 5, v5);
> + set_user_reg(regs, 6, v6);
> + set_user_reg(regs, 7, v7);
> +}
> +
> +static void set_regs_success(struct cpu_user_regs *regs, uint32_t w2,
> + uint32_t w3)
> +{
> + set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, 0, 0, 0, 0);
> +}
> +
> +static void handle_version(struct cpu_user_regs *regs)
> +{
> + struct domain *d = current->domain;
> + struct ffa_ctx *ctx = d->arch.ffa;
> + uint32_t vers = get_user_reg(regs, 1);
> +
> + if ( vers < FFA_VERSION_1_1 )
> + vers = FFA_VERSION_1_0;
> + else
> + vers = FFA_VERSION_1_1;
> +
> + ctx->guest_vers = vers;
> + set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
> +}
> +
> +bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
> +{
> + struct domain *d = current->domain;
> + struct ffa_ctx *ctx = d->arch.ffa;
> +
> + if ( !ctx )
> + return false;
> +
> + switch ( fid )
> + {
> + case FFA_VERSION:
> + handle_version(regs);
> + return true;
> + case FFA_ID_GET:
> + set_regs_success(regs, get_vm_id(d), 0);
> + return true;
> +
> + default:
> + printk(XENLOG_ERR "ffa: unhandled fid 0x%x\n", fid);
> + return false;
> + }
> +}
> +
> +int ffa_domain_init(struct domain *d, bool ffa_enabled)
> +{
> + struct ffa_ctx *ctx;
> +
> + if ( !ffa_version || !ffa_enabled )
AFAIU, this check means that even if the admin requested to enable FFA
for the domain, it may not be honored.
If that's the case, then I am afraid this is not a desired approach
because this issue will only get noticed after the OS has booted.
We should return an error if the domain has requested an unavaible feature.
> + return 0;
> +
> + ctx = xzalloc(struct ffa_ctx);
> + if ( !ctx )
> + return -ENOMEM;
> +
> + d->arch.ffa = ctx;
> +
> + return 0;
> +}
> +
> +int ffa_relinquish_resources(struct domain *d)
This is called from domain_relinquish_resources(). However...
> +{
> + struct ffa_ctx *ctx = d->arch.ffa;
> +
> + if ( !ctx )
> + return 0;
> +
> + XFREE(d->arch.ffa);
... the allocation of FFA is happening in arch_domain_create(). In case
of early failure, the relinquish helper will not be called.
You want to split this function in two parts:
1) ffa_domain_destroy() -> This will be freeing anything that was
allocated by ffa_domain_init() and will be called from
arch_domain_destroy().
2) ffa_relinquish_resources() -> This will be freeing any resources
allocated afterwards.
The former will not preemptible while the latter will. Which should be
fine because arch_domain_init() and therefore arch_domain_free() should
never contain long running operations.
> +
> + return 0;
> +}
> +
> +static int __init ffa_init(void)
> +{
> + uint32_t vers;
> + unsigned int major_vers;
> + unsigned int minor_vers;
> +
> + /*
> + * FFA_PAGE_SIZE is defined to 4k and we're currently depending on
> + * using that page size.
> + */
> + BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
> +
> + /*
> + * psci_init_smccc() updates this value with what's reported by EL-3
> + * or secure world.
> + */
> + if ( smccc_ver < ARM_SMCCC_VERSION_1_2 )
> + {
> + printk(XENLOG_ERR
> + "ffa: unsupported SMCCC version %#x (need at least %#x)\n",
> + smccc_ver, ARM_SMCCC_VERSION_1_2);
> + return 0;
> + }
> +
> + if ( !ffa_get_version(&vers) )
> + return 0;
> +
> + if ( vers < FFA_MIN_VERSION || vers > FFA_MY_VERSION )
> + {
> + printk(XENLOG_ERR "ffa: Incompatible version %#x found\n", vers);
> + return 0;
> + }
> +
> + major_vers = (vers >> FFA_VERSION_MAJOR_SHIFT) & FFA_VERSION_MAJOR_MASK;
> + minor_vers = vers & FFA_VERSION_MINOR_MASK;
> + printk(XENLOG_INFO "ARM FF-A Mediator version %u.%u\n",
> + FFA_MY_VERSION_MAJOR, FFA_MY_VERSION_MINOR);
> + printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
> + major_vers, minor_vers);
> +
> + ffa_version = vers;
> +
> + return 0;
> +}
> +
> +__initcall(ffa_init);
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index ed63c2b6f91f..b3dee269bced 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -103,6 +103,10 @@ struct arch_domain
> void *tee;
> #endif
>
> +#ifdef CONFIG_FFA
> + void *ffa;
> +#endif
> +
> bool directmap;
> } __cacheline_aligned;
>
> diff --git a/xen/arch/arm/include/asm/ffa.h b/xen/arch/arm/include/asm/ffa.h
> new file mode 100644
> index 000000000000..4f4a739345bd
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/ffa.h
> @@ -0,0 +1,71 @@
> +/*
> + * xen/arch/arm/ffa.c
> + *
> + * Arm Firmware Framework for ARMv8-A(FFA) mediator
> + *
> + * Copyright (C) 2021 Linaro Limited
> + *
> + * Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without restriction,
> + * including without limitation the rights to use, copy, modify, merge,
> + * publish, distribute, sublicense, and/or sell copies of the Software,
> + * and to permit persons to whom the Software is furnished to do so,
> + * subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
> + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#ifndef __ASM_ARM_FFA_H__
> +#define __ASM_ARM_FFA_H__
> +
> +#include <xen/const.h>
> +
> +#include <asm/smccc.h>
> +#include <asm/types.h>
> +
> +#define FFA_FNUM_MIN_VALUE _AC(0x60,U)
> +#define FFA_FNUM_MAX_VALUE _AC(0x86,U)
> +
> +static inline bool is_ffa_fid(uint32_t fid)
> +{
> + uint32_t fn = fid & ARM_SMCCC_FUNC_MASK;
> +
> + return fn >= FFA_FNUM_MIN_VALUE && fn <= FFA_FNUM_MAX_VALUE;
> +}
> +
> +#ifdef CONFIG_FFA
> +#define FFA_NR_FUNCS 11
You wrote 11 here, but you seem only expose 2 in this patch. However,
AFAICT the call count is deprected in SMCCC v1.2. So do we need to
update it?
But if it is not deprecated then...
> +
> +bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid);
> +int ffa_domain_init(struct domain *d, bool ffa_enabled);
> +int ffa_relinquish_resources(struct domain *d);
> +#else
> +#define FFA_NR_FUNCS 0
> +
> +static inline bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
> +{
> + return false;
> +}
> +
> +static inline int ffa_domain_init(struct domain *d, bool ffa_enabled)
> +{
> + return 0;
For the same reason as above, this should return -ENODEV if ffa_enabled
is true.
> +}
> +
> +static inline int ffa_relinquish_resources(struct domain *d)
> +{
> + return 0;
> +}
> +#endif
> +
> +#endif /*__ASM_ARM_FFA_H__*/
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index 6f90c08a6304..34586025eff8 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -20,6 +20,7 @@
> #include <public/arch-arm/smccc.h>
> #include <asm/cpuerrata.h>
> #include <asm/cpufeature.h>
> +#include <asm/ffa.h>
> #include <asm/monitor.h>
> #include <asm/regs.h>
> #include <asm/smccc.h>
> @@ -32,7 +33,7 @@
> #define XEN_SMCCC_FUNCTION_COUNT 3
>
> /* Number of functions currently supported by Standard Service Service Calls. */
> -#define SSSC_SMCCC_FUNCTION_COUNT (3 + VPSCI_NR_FUNCS)
> +#define SSSC_SMCCC_FUNCTION_COUNT (3 + VPSCI_NR_FUNCS + FFA_NR_FUNCS)
... it seems incorrect to me add FFA_NR_FUNCS unconditionally because
while the hypervisor may support FFA, the guest may not have access to it.
>
> static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid)
> {
> @@ -196,13 +197,23 @@ static bool handle_existing_apis(struct cpu_user_regs *regs)
> return do_vpsci_0_1_call(regs, fid);
> }
>
> +static bool is_psci_fid(uint32_t fid)
> +{
> + uint32_t fn = fid & ARM_SMCCC_FUNC_MASK;
> +
> + return fn >= 0 && fn <= 0x1fU;
> +}
> +
> /* PSCI 0.2 interface and other Standard Secure Calls */
> static bool handle_sssc(struct cpu_user_regs *regs)
> {
> uint32_t fid = (uint32_t)get_user_reg(regs, 0);
>
> - if ( do_vpsci_0_2_call(regs, fid) )
> - return true;
> + if ( is_psci_fid(fid) )
> + return do_vpsci_0_2_call(regs, fid);
> +
> + if ( is_ffa_fid(fid) )
> + return ffa_handle_call(regs, fid);
>
> switch ( fid )
> {
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index ab05fe12b0de..53f8d44a6a8e 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -318,6 +318,8 @@ struct xen_arch_domainconfig {
> /* IN/OUT */
> uint8_t gic_version;
> /* IN */
> + uint8_t ffa_enabled;
AFAIU, this will be 0/1. We have a 'flags' in common but not in arch.
I don't think we should use the common one (I don't think FFA will be
ever used on x86?). So I would introduce a new field flag in
xen-arch_domainconfig.
Also AFAICT, XEN_DOMCTL_INTERFACE_VERSION has already been bumped for
4.17. So we should not need to do it in this patch. However, if this is
not going to merged in 4.17, then you will need to bump the domctl
version (assuming this is the first patch touch the domctl after the
release).
> + /* IN */
> uint16_t tee_type;
> /* IN */
> uint32_t nr_spis;
--
Julien Grall
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator
2022-08-18 10:55 ` [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator Jens Wiklander
2022-09-05 22:17 ` Julien Grall
@ 2022-09-05 22:25 ` Julien Grall
2022-09-06 15:19 ` Jens Wiklander
2022-09-06 9:44 ` Anthony PERARD
2 siblings, 1 reply; 22+ messages in thread
From: Julien Grall @ 2022-09-05 22:25 UTC (permalink / raw)
To: Jens Wiklander, xen-devel
Cc: Stefano Stabellini, Volodymyr Babchuk, Bertrand.Marquis,
Anthony PERARD, Juergen Gross, Wei Liu
Hi Jens,
More remarks.
On 18/08/2022 11:55, Jens Wiklander wrote:
> +/* Negotiated FF-A version to use with the SPMC */
> +static uint32_t ffa_version __read_mostly;
NIT: if this is not meant to be modified after boot, then I would
suggest to use __ro_after_init. This was introduced recently and will
prevent the variable to be modified after boot.
> +
> +static bool ffa_get_version(uint32_t *vers)
This is not __init. Is this going to be called at runtime by a domain?
If yes...
> +{
> + const struct arm_smccc_1_2_regs arg = {
> + .a0 = FFA_VERSION,
> + .a1 = FFA_MY_VERSION,
> + };
> + struct arm_smccc_1_2_regs resp;
> +
> + arm_smccc_1_2_smc(&arg, &resp);
> + if ( resp.a0 == FFA_RET_NOT_SUPPORTED )
> + {
> + printk(XENLOG_ERR "ffa: FFA_VERSION returned not supported\n");
... this wants to be a XENLOG_G_ERR to rate limited it. XENLOG_ERR is
not by default and will allow a domain to spam Xen console.
A rule of thumb is any code reachable for a domain (other than dom0)
should use XENLOG_G_* when printing or gprintk(XENLOG_*, ) if you want
to print the domain ID and ratelimit. Note that the latter doesn't
require the G_* becauce it will add it automatically.
> + return false;
> + }
> +
> + *vers = resp.a0;
> +
> + return true;
> +}
> +
> +static u16 get_vm_id(const struct domain *d)
> +{
> + /* +1 since 0 is reserved for the hypervisor in FF-A */
> + return d->domain_id + 1;
> +}
> +
> +static void set_regs(struct cpu_user_regs *regs, register_t v0, register_t v1,
> + register_t v2, register_t v3, register_t v4, register_t v5,
> + register_t v6, register_t v7)
> +{
> + set_user_reg(regs, 0, v0);
> + set_user_reg(regs, 1, v1);
> + set_user_reg(regs, 2, v2);
> + set_user_reg(regs, 3, v3);
> + set_user_reg(regs, 4, v4);
> + set_user_reg(regs, 5, v5);
> + set_user_reg(regs, 6, v6);
> + set_user_reg(regs, 7, v7);
> +}
> +
> +static void set_regs_success(struct cpu_user_regs *regs, uint32_t w2,
> + uint32_t w3)
> +{
> + set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, 0, 0, 0, 0);
> +}
> +
> +static void handle_version(struct cpu_user_regs *regs)
> +{
> + struct domain *d = current->domain;
> + struct ffa_ctx *ctx = d->arch.ffa;
> + uint32_t vers = get_user_reg(regs, 1);
> +
> + if ( vers < FFA_VERSION_1_1 )
> + vers = FFA_VERSION_1_0;
> + else
> + vers = FFA_VERSION_1_1;
> +
> + ctx->guest_vers = vers;
> + set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
> +}
> +
> +bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
> +{
> + struct domain *d = current->domain;
> + struct ffa_ctx *ctx = d->arch.ffa;
> +
> + if ( !ctx )
> + return false;
> +
> + switch ( fid )
> + {
> + case FFA_VERSION:
> + handle_version(regs);
> + return true;
> + case FFA_ID_GET:
> + set_regs_success(regs, get_vm_id(d), 0);
> + return true;
> +
> + default:
> + printk(XENLOG_ERR "ffa: unhandled fid 0x%x\n", fid);
This one definitely want to be a XENLOG_G_ERR. But I would use
gprintk(XENLOG_ERR, ).
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator
2022-08-18 10:55 ` [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator Jens Wiklander
2022-09-05 22:17 ` Julien Grall
2022-09-05 22:25 ` Julien Grall
@ 2022-09-06 9:44 ` Anthony PERARD
2 siblings, 0 replies; 22+ messages in thread
From: Anthony PERARD @ 2022-09-06 9:44 UTC (permalink / raw)
To: Jens Wiklander
Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Bertrand.Marquis, Juergen Gross, Wei Liu
Hi Jens,
On Thu, Aug 18, 2022 at 12:55:55PM +0200, Jens Wiklander wrote:
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index eef1de093914..a985609861c7 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -101,6 +101,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> return ERROR_FAIL;
> }
>
> + config->arch.ffa_enabled =
> + libxl_defbool_val(d_config->b_info.arch_arm.ffa_enabled);
There seems to be missing a call to libxl_defbool_setdefault() before
this. This could result in an abort when creating a guest for Arm.
A call to libxl_defbool_setdefault() probably want to be done in
libxl__arch_domain_build_info_setdefault() in libxl_arm.c.
> return 0;
> }
>
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index b98c0de378b6..e0e99ed8d2b1 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -2746,6 +2746,9 @@ skip_usbdev:
> exit(-ERROR_FAIL);
> }
> }
> + libxl_defbool_setdefault(&b_info->arch_arm.ffa_enabled, false);
This should be done in libxl (as pointed out above) instead of xl.
> + xlu_cfg_get_defbool(config, "ffa_enabled",
> + &b_info->arch_arm.ffa_enabled, 0);
>
> parse_vkb_list(config, d_config);
>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator
2022-09-05 22:17 ` Julien Grall
@ 2022-09-06 14:57 ` Jens Wiklander
0 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-09-06 14:57 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Stefano Stabellini, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu,
Andrew Cooper
Hi Julien,
On Tue, Sep 6, 2022 at 12:17 AM Julien Grall <julien@xen.org> wrote:
>
> Hi Jens,
>
> On 18/08/2022 11:55, Jens Wiklander wrote:
> > Adds a FF-A version 1.1 [1] mediator to communicate with a Secure
> > Partition in secure world.
> >
> > This commit brings in only the parts needed to negotiate FF-A version
> > number with guest and SPMC.
> >
> > A guest configuration variable "ffa_enabled" is used to indicate if a guest
> > is trusted to use FF-A.
> >
> > This is loosely based on the TEE mediator framework and the OP-TEE
> > mediator.
> >
> > [1] https://developer.arm.com/documentation/den0077/latest
> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > ---
> > SUPPORT.md | 7 +
> > docs/man/xl.cfg.5.pod.in | 15 ++
> > tools/include/libxl.h | 6 +
> > tools/libs/light/libxl_arm.c | 3 +
> > tools/libs/light/libxl_types.idl | 1 +
> > tools/xl/xl_parse.c | 3 +
> > xen/arch/arm/Kconfig | 11 +
> > xen/arch/arm/Makefile | 1 +
> > xen/arch/arm/domain.c | 10 +
> > xen/arch/arm/domain_build.c | 1 +
> > xen/arch/arm/ffa.c | 354 ++++++++++++++++++++++++++++++
> > xen/arch/arm/include/asm/domain.h | 4 +
> > xen/arch/arm/include/asm/ffa.h | 71 ++++++
> > xen/arch/arm/vsmc.c | 17 +-
> > xen/include/public/arch-arm.h | 2 +
> > 15 files changed, 503 insertions(+), 3 deletions(-)
> > create mode 100644 xen/arch/arm/ffa.c
> > create mode 100644 xen/arch/arm/include/asm/ffa.h
> >
> > diff --git a/SUPPORT.md b/SUPPORT.md
> > index 70e98964cbc0..215bb3c9043b 100644
> > --- a/SUPPORT.md
> > +++ b/SUPPORT.md
> > @@ -785,6 +785,13 @@ that covers the DMA of the device to be passed through.
> >
> > No support for QEMU backends in a 16K or 64K domain.
> >
> > +### ARM: Firmware Framework for Arm A-profile (FF-A) Mediator
> > +
> > + Status, Arm64: Tech Preview
> > +
> > +There are still some code paths where a vCPU may hog a pCPU longer than
> > +necessary. The FF-A mediator is not yet implemented for Arm32.
> > +
> > ### ARM: Guest Device Tree support
> >
> > Status: Supported
> > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> > index b98d1613987e..234c036aecb1 100644
> > --- a/docs/man/xl.cfg.5.pod.in
> > +++ b/docs/man/xl.cfg.5.pod.in
> > @@ -1616,6 +1616,21 @@ This feature is a B<technology preview>.
> >
> > =back
> >
> > +=item B<ffa_enabled=BOOLEAN>
> > +
> > +B<Arm only.> Allow a guest to communicate via FF-A with Secure Partitions
> > +(SP), default false.
> > +
> > +Currently is only a small subset of the FF-A specification supported. Just
> > +enough to communicate with OP-TEE. In general all the basic things and
> > +sharing memory with one SP. More advanced use cases where memory might be
> > +shared or donated to multple SPs is not supported.
> > +
> > +See L<https://developer.arm.com/documentation/den0077/latest> for more
> > +informantion about FF-A.
> > +
> > +This feature is a B<technology preview>.
> > +
> > =head2 Paravirtualised (PV) Guest Specific Options
> >
> > The following options apply only to Paravirtual (PV) guests.
> > diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> > index 7ce978e83c9a..4ab5a7b044d6 100644
> > --- a/tools/include/libxl.h
> > +++ b/tools/include/libxl.h
> > @@ -278,6 +278,12 @@
> > */
> > #define LIBXL_HAVE_BUILDINFO_ARCH_ARM_TEE 1
> >
> > +/*
> > + * LIBXL_HAVE_BUILDINFO_ARM_FFA_ENABLED indicates that
> > + * libxl_domain_build_info has the arm.ffa_enabled field.
> > + */
> > +#define LIBXL_HAVE_BUILDINFO_ARM_FFA_ENABLED 1
> > +
> > /*
> > * LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing
> > * 'soft reset' for domains and there is 'soft_reset' shutdown reason
> > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> > index eef1de093914..a985609861c7 100644
> > --- a/tools/libs/light/libxl_arm.c
> > +++ b/tools/libs/light/libxl_arm.c
> > @@ -101,6 +101,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> > return ERROR_FAIL;
> > }
> >
> > + config->arch.ffa_enabled =
> > + libxl_defbool_val(d_config->b_info.arch_arm.ffa_enabled);
> > +
> > return 0;
> > }
> >
> > diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
> > index 2a42da2f7d78..bf4544bef399 100644
> > --- a/tools/libs/light/libxl_types.idl
> > +++ b/tools/libs/light/libxl_types.idl
> > @@ -646,6 +646,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> >
> > ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
> > ("vuart", libxl_vuart_type),
> > + ("ffa_enabled", libxl_defbool),
> > ])),
> > ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
> > ])),
> > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> > index b98c0de378b6..e0e99ed8d2b1 100644
> > --- a/tools/xl/xl_parse.c
> > +++ b/tools/xl/xl_parse.c
> > @@ -2746,6 +2746,9 @@ skip_usbdev:
> > exit(-ERROR_FAIL);
> > }
> > }
> > + libxl_defbool_setdefault(&b_info->arch_arm.ffa_enabled, false);
> > + xlu_cfg_get_defbool(config, "ffa_enabled",
> > + &b_info->arch_arm.ffa_enabled, 0);
> >
> > parse_vkb_list(config, d_config);
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index be9eff014120..e57e1d3757e2 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -139,6 +139,17 @@ config TEE
> >
> > source "arch/arm/tee/Kconfig"
> >
> > +config FFA
> > + bool "Enable FF-A mediator support" if EXPERT
> > + default n
> > + depends on ARM_64
> > + help
> > + This option enables a minimal FF-A mediator. The mediator is
> > + generic as it follows the FF-A specification [1], but it only
> > + implements a small subset of the specification.
> > +
> > + [1] https://developer.arm.com/documentation/den0077/latest
> > +
> > endmenu
> >
> > menu "ARM errata workaround via the alternative framework"
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > index bb7a6151c13c..af0c69f793d4 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -20,6 +20,7 @@ obj-y += domain_build.init.o
> > obj-y += domctl.o
> > obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> > obj-y += efi/
> > +obj-$(CONFIG_FFA) += ffa.o
> > obj-y += gic.o
> > obj-y += gic-v2.o
> > obj-$(CONFIG_GICV3) += gic-v3.o
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index 8110c1df8638..a3f00e7e234d 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -27,6 +27,7 @@
> > #include <asm/cpufeature.h>
> > #include <asm/current.h>
> > #include <asm/event.h>
> > +#include <asm/ffa.h>
> > #include <asm/gic.h>
> > #include <asm/guest_atomics.h>
> > #include <asm/irq.h>
> > @@ -756,6 +757,9 @@ int arch_domain_create(struct domain *d,
> > if ( (rc = tee_domain_init(d, config->arch.tee_type)) != 0 )
> > goto fail;
> >
> > + if ( (rc = ffa_domain_init(d, config->arch.ffa_enabled)) != 0 )
> > + goto fail;
> > +
> > update_domain_wallclock_time(d);
> >
> > /*
> > @@ -998,6 +1002,7 @@ static int relinquish_memory(struct domain *d, struct page_list_head *list)
> > enum {
> > PROG_pci = 1,
> > PROG_tee,
> > + PROG_ffa,
> > PROG_xen,
> > PROG_page,
> > PROG_mapping,
> > @@ -1043,6 +1048,11 @@ int domain_relinquish_resources(struct domain *d)
> >
> > PROGRESS(tee):
> > ret = tee_relinquish_resources(d);
> > + if ( ret )
> > + return ret;
> > +
> > + PROGRESS(ffa):
> > + ret = ffa_relinquish_resources(d);
> > if (ret )
> > return ret;
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 7ddd16c26da5..d708f76356f7 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -3450,6 +3450,7 @@ void __init create_dom0(void)
> > if ( gic_number_lines() > 992 )
> > printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
> > dom0_cfg.arch.tee_type = tee_get_type();
> > + dom0_cfg.arch.ffa_enabled = true;
> > dom0_cfg.max_vcpus = dom0_max_vcpus();
> >
> > if ( iommu_enabled )
> > diff --git a/xen/arch/arm/ffa.c b/xen/arch/arm/ffa.c
> > new file mode 100644
> > index 000000000000..b85c492928d2
> > --- /dev/null
> > +++ b/xen/arch/arm/ffa.c
> > @@ -0,0 +1,354 @@
> > +/*
> > + * xen/arch/arm/ffa.c
> > + *
> > + * Arm Firmware Framework for ARMv8-A (FF-A) mediator
> > + *
> > + * Copyright (C) 2022 Linaro Limited
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms and conditions 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.
> > + *
> > + * You should have received a copy of the GNU General Public
> > + * License along with this program; If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <xen/domain_page.h>
> > +#include <xen/errno.h>
> > +#include <xen/init.h>
> > +#include <xen/lib.h>
> > +#include <xen/sched.h>
> > +#include <xen/types.h>
> > +#include <xen/sizes.h>
> > +#include <xen/bitops.h>
> > +
> > +#include <asm/smccc.h>
> > +#include <asm/event.h>
> > +#include <asm/ffa.h>
> > +#include <asm/regs.h>
> > +
> > +/* Error codes */
> > +#define FFA_RET_OK 0
> > +#define FFA_RET_NOT_SUPPORTED -1
> > +#define FFA_RET_INVALID_PARAMETERS -2
> > +#define FFA_RET_NO_MEMORY -3
> > +#define FFA_RET_BUSY -4
> > +#define FFA_RET_INTERRUPTED -5
> > +#define FFA_RET_DENIED -6
> > +#define FFA_RET_RETRY -7
> > +#define FFA_RET_ABORTED -8
> > +
> > +/* FFA_VERSION helpers */
> > +#define FFA_VERSION_MAJOR_SHIFT 16U
> > +#define FFA_VERSION_MAJOR_MASK 0x7FFFU
> > +#define FFA_VERSION_MINOR_SHIFT 0U
> > +#define FFA_VERSION_MINOR_MASK 0xFFFFU
> > +#define MAKE_FFA_VERSION(major, minor) \
> > + ((((major) & FFA_VERSION_MAJOR_MASK) << FFA_VERSION_MAJOR_SHIFT) | \
> > + ((minor) & FFA_VERSION_MINOR_MASK))
> > +
> > +#define FFA_MIN_VERSION MAKE_FFA_VERSION(1, 0)
> > +#define FFA_VERSION_1_0 MAKE_FFA_VERSION(1, 0)
> > +#define FFA_VERSION_1_1 MAKE_FFA_VERSION(1, 1)
> > +
> > +/*
> > + * This is the version we want to use in communication with guests and SPs.
> > + * During negotiation with a guest or a SP we may need to lower it for
> > + * that particular guest or SP.
> > + */
> > +#define FFA_MY_VERSION_MAJOR 1U
> > +#define FFA_MY_VERSION_MINOR 1U
> > +#define FFA_MY_VERSION MAKE_FFA_VERSION(FFA_MY_VERSION_MAJOR, \
> > + FFA_MY_VERSION_MINOR)
> > +
> > +#define FFA_PAGE_SIZE SZ_4K
> > +
> > +/*
> > + * Limit for shared buffer size. Please note that this define limits
> > + * number of pages. But user buffer can be not aligned to a page
> > + * boundary. So it is possible that user would not be able to share
> > + * exactly MAX_SHM_BUFFER_PG * PAGE_SIZE bytes.
> > + *
> > + * FF-A doesn't have any direct requirments on GlobalPlatform or vice
> > + * versa, but an implementation can very well use FF-A in order to provide
> > + * a GlobalPlatform interface on top.
> > + *
> > + * Global Platform specification for TEE requires that any TEE
> > + * implementation should allow to share buffers with size of at least
> > + * 512KB, which equals to 128 4kB pages. Due to align issue mentioned
> > + * above, we need to increase this value to 129.
> > + */
> > +#define FFA_MAX_SHM_PAGE_COUNT 129
> > +
> > +/*
> > + * Limits the number of shared buffers that guest can have at once. This
> > + * is to prevent case, when guests tricks XEN into exhausting its own
> > + * memory by allocating many small buffers. This value has been chosen
> > + * arbitrary.
> > + */
> > +#define FFA_MAX_SHM_COUNT 32
> > +
> > +#define FFA_HANDLE_HYP_FLAG BIT(63, ULL)
> > +
> > +/* Memory attributes: Normal memory, Write-Back cacheable, Inner shareable */
> > +#define FFA_NORMAL_MEM_REG_ATTR 0x2fU
> > +
> > +/* Memory access permissions: Read-write */
> > +#define FFA_MEM_ACC_RW 0x2U
> > +
> > +/* Clear memory before mapping in receiver */
> > +#define FFA_MEMORY_REGION_FLAG_CLEAR BIT(0, U)
> > +/* Relayer may time slice this operation */
> > +#define FFA_MEMORY_REGION_FLAG_TIME_SLICE BIT(1, U)
> > +/* Clear memory after receiver relinquishes it */
> > +#define FFA_MEMORY_REGION_FLAG_CLEAR_RELINQUISH BIT(2, U)
> > +
> > +/* Share memory transaction */
> > +#define FFA_MEMORY_REGION_TRANSACTION_TYPE_SHARE (1U << 3)
> > +
> > +#define FFA_HANDLE_INVALID 0xffffffffffffffffULL
> > +
> > +/* Framework direct request/response */
> > +#define FFA_MSG_FLAG_FRAMEWORK BIT(31, U)
> > +#define FFA_MSG_TYPE_MASK 0xFFU;
> > +#define FFA_MSG_PSCI 0x0U
> > +#define FFA_MSG_SEND_VM_CREATED 0x4U
> > +#define FFA_MSG_RESP_VM_CREATED 0x5U
> > +#define FFA_MSG_SEND_VM_DESTROYED 0x6U
> > +#define FFA_MSG_RESP_VM_DESTROYED 0x7U
> > +
> > +/*
> > + * Flags used for the FFA_PARTITION_INFO_GET return message:
> > + * BIT(0): Supports receipt of direct requests
> > + * BIT(1): Can send direct requests
> > + * BIT(2): Can send and receive indirect messages
> > + * BIT(3): Supports receipt of notifications
> > + * BIT(4-5): Partition ID is a PE endpoint ID
> > + */
> > +#define FFA_PART_PROP_DIRECT_REQ_RECV BIT(0, U)
> > +#define FFA_PART_PROP_DIRECT_REQ_SEND BIT(1, U)
> > +#define FFA_PART_PROP_INDIRECT_MSGS BIT(2, U)
> > +#define FFA_PART_PROP_RECV_NOTIF BIT(3, U)
> > +#define FFA_PART_PROP_IS_PE_ID (0U << 4)
> > +#define FFA_PART_PROP_IS_SEPID_INDEP (1U << 4)
> > +#define FFA_PART_PROP_IS_SEPID_DEP (2U << 4)
> > +#define FFA_PART_PROP_IS_AUX_ID (3U << 4)
> > +#define FFA_PART_PROP_NOTIF_CREATED BIT(6, U)
> > +#define FFA_PART_PROP_NOTIF_DESTROYED BIT(7, U)
> > +#define FFA_PART_PROP_AARCH64_STATE BIT(8, U)
> > +
> > +/*
> > + * Flag used as parameter to FFA_PARTITION_INFO_GET to return partition
> > + * count only.
> > + */
> > +#define FFA_PARTITION_INFO_GET_COUNT_FLAG BIT(0, U)
> > +
> > +/* Function IDs */
> > +#define FFA_ERROR 0x84000060U
> > +#define FFA_SUCCESS_32 0x84000061U
> > +#define FFA_SUCCESS_64 0xC4000061U
> > +#define FFA_INTERRUPT 0x84000062U
> > +#define FFA_VERSION 0x84000063U
> > +#define FFA_FEATURES 0x84000064U
> > +#define FFA_RX_ACQUIRE 0x84000084U
> > +#define FFA_RX_RELEASE 0x84000065U
> > +#define FFA_RXTX_MAP_32 0x84000066U
> > +#define FFA_RXTX_MAP_64 0xC4000066U
> > +#define FFA_RXTX_UNMAP 0x84000067U
> > +#define FFA_PARTITION_INFO_GET 0x84000068U
> > +#define FFA_ID_GET 0x84000069U
> > +#define FFA_SPM_ID_GET 0x84000085U
> > +#define FFA_MSG_WAIT 0x8400006BU
> > +#define FFA_MSG_YIELD 0x8400006CU
> > +#define FFA_MSG_RUN 0x8400006DU
> > +#define FFA_MSG_SEND2 0x84000086U
> > +#define FFA_MSG_SEND_DIRECT_REQ_32 0x8400006FU
> > +#define FFA_MSG_SEND_DIRECT_REQ_64 0xC400006FU
> > +#define FFA_MSG_SEND_DIRECT_RESP_32 0x84000070U
> > +#define FFA_MSG_SEND_DIRECT_RESP_64 0xC4000070U
> > +#define FFA_MEM_DONATE_32 0x84000071U
> > +#define FFA_MEM_DONATE_64 0xC4000071U
> > +#define FFA_MEM_LEND_32 0x84000072U
> > +#define FFA_MEM_LEND_64 0xC4000072U
> > +#define FFA_MEM_SHARE_32 0x84000073U
> > +#define FFA_MEM_SHARE_64 0xC4000073U
> > +#define FFA_MEM_RETRIEVE_REQ_32 0x84000074U
> > +#define FFA_MEM_RETRIEVE_REQ_64 0xC4000074U
> > +#define FFA_MEM_RETRIEVE_RESP 0x84000075U
> > +#define FFA_MEM_RELINQUISH 0x84000076U
> > +#define FFA_MEM_RECLAIM 0x84000077U
> > +#define FFA_MEM_FRAG_RX 0x8400007AU
> > +#define FFA_MEM_FRAG_TX 0x8400007BU
> > +#define FFA_MSG_SEND 0x8400006EU
> > +#define FFA_MSG_POLL 0x8400006AU
> > +
> > +struct ffa_ctx {
> > + uint32_t guest_vers;
> > +};
> > +
> > +/* Negotiated FF-A version to use with the SPMC */
> > +static uint32_t ffa_version __read_mostly;
> > +
> > +static bool ffa_get_version(uint32_t *vers)
> > +{
> > + const struct arm_smccc_1_2_regs arg = {
> > + .a0 = FFA_VERSION,
> > + .a1 = FFA_MY_VERSION,
> > + };
> > + struct arm_smccc_1_2_regs resp;
> > +
> > + arm_smccc_1_2_smc(&arg, &resp);
> > + if ( resp.a0 == FFA_RET_NOT_SUPPORTED )
> > + {
> > + printk(XENLOG_ERR "ffa: FFA_VERSION returned not supported\n");
> > + return false;
> > + }
> > +
> > + *vers = resp.a0;
> > +
> > + return true;
> > +}
> > +
> > +static u16 get_vm_id(const struct domain *d)
> > +{
> > + /* +1 since 0 is reserved for the hypervisor in FF-A */
> > + return d->domain_id + 1;
> > +}
> > +
> > +static void set_regs(struct cpu_user_regs *regs, register_t v0, register_t v1,
> > + register_t v2, register_t v3, register_t v4, register_t v5,
> > + register_t v6, register_t v7)
> > +{
> > + set_user_reg(regs, 0, v0);
> > + set_user_reg(regs, 1, v1);
> > + set_user_reg(regs, 2, v2);
> > + set_user_reg(regs, 3, v3);
> > + set_user_reg(regs, 4, v4);
> > + set_user_reg(regs, 5, v5);
> > + set_user_reg(regs, 6, v6);
> > + set_user_reg(regs, 7, v7);
> > +}
> > +
> > +static void set_regs_success(struct cpu_user_regs *regs, uint32_t w2,
> > + uint32_t w3)
> > +{
> > + set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, 0, 0, 0, 0);
> > +}
> > +
> > +static void handle_version(struct cpu_user_regs *regs)
> > +{
> > + struct domain *d = current->domain;
> > + struct ffa_ctx *ctx = d->arch.ffa;
> > + uint32_t vers = get_user_reg(regs, 1);
> > +
> > + if ( vers < FFA_VERSION_1_1 )
> > + vers = FFA_VERSION_1_0;
> > + else
> > + vers = FFA_VERSION_1_1;
> > +
> > + ctx->guest_vers = vers;
> > + set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
> > +}
> > +
> > +bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
> > +{
> > + struct domain *d = current->domain;
> > + struct ffa_ctx *ctx = d->arch.ffa;
> > +
> > + if ( !ctx )
> > + return false;
> > +
> > + switch ( fid )
> > + {
> > + case FFA_VERSION:
> > + handle_version(regs);
> > + return true;
> > + case FFA_ID_GET:
> > + set_regs_success(regs, get_vm_id(d), 0);
> > + return true;
> > +
> > + default:
> > + printk(XENLOG_ERR "ffa: unhandled fid 0x%x\n", fid);
> > + return false;
> > + }
> > +}
> > +
> > +int ffa_domain_init(struct domain *d, bool ffa_enabled)
> > +{
> > + struct ffa_ctx *ctx;
> > +
> > + if ( !ffa_version || !ffa_enabled )
>
> AFAIU, this check means that even if the admin requested to enable FFA
> for the domain, it may not be honored.
>
> If that's the case, then I am afraid this is not a desired approach
> because this issue will only get noticed after the OS has booted.
>
> We should return an error if the domain has requested an unavaible feature.
OK, I'll return an error.
>
> > + return 0;
> > +
> > + ctx = xzalloc(struct ffa_ctx);
> > + if ( !ctx )
> > + return -ENOMEM;
> > +
> > + d->arch.ffa = ctx;
> > +
> > + return 0;
> > +}
> > +
> > +int ffa_relinquish_resources(struct domain *d)
>
> This is called from domain_relinquish_resources(). However...
>
>
> > +{
> > + struct ffa_ctx *ctx = d->arch.ffa;
> > +
> > + if ( !ctx )
> > + return 0;
> > +
> > + XFREE(d->arch.ffa);
>
> ... the allocation of FFA is happening in arch_domain_create(). In case
> of early failure, the relinquish helper will not be called.
>
> You want to split this function in two parts:
> 1) ffa_domain_destroy() -> This will be freeing anything that was
> allocated by ffa_domain_init() and will be called from
> arch_domain_destroy().
> 2) ffa_relinquish_resources() -> This will be freeing any resources
> allocated afterwards.
>
> The former will not preemptible while the latter will. Which should be
> fine because arch_domain_init() and therefore arch_domain_free() should
> never contain long running operations.
Thanks, I'll fix this.
>
> > +
> > + return 0;
> > +}
> > +
> > +static int __init ffa_init(void)
> > +{
> > + uint32_t vers;
> > + unsigned int major_vers;
> > + unsigned int minor_vers;
> > +
> > + /*
> > + * FFA_PAGE_SIZE is defined to 4k and we're currently depending on
> > + * using that page size.
> > + */
> > + BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
> > +
> > + /*
> > + * psci_init_smccc() updates this value with what's reported by EL-3
> > + * or secure world.
> > + */
> > + if ( smccc_ver < ARM_SMCCC_VERSION_1_2 )
> > + {
> > + printk(XENLOG_ERR
> > + "ffa: unsupported SMCCC version %#x (need at least %#x)\n",
> > + smccc_ver, ARM_SMCCC_VERSION_1_2);
> > + return 0;
> > + }
> > +
> > + if ( !ffa_get_version(&vers) )
> > + return 0;
> > +
> > + if ( vers < FFA_MIN_VERSION || vers > FFA_MY_VERSION )
> > + {
> > + printk(XENLOG_ERR "ffa: Incompatible version %#x found\n", vers);
> > + return 0;
> > + }
> > +
> > + major_vers = (vers >> FFA_VERSION_MAJOR_SHIFT) & FFA_VERSION_MAJOR_MASK;
> > + minor_vers = vers & FFA_VERSION_MINOR_MASK;
> > + printk(XENLOG_INFO "ARM FF-A Mediator version %u.%u\n",
> > + FFA_MY_VERSION_MAJOR, FFA_MY_VERSION_MINOR);
> > + printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
> > + major_vers, minor_vers);
> > +
> > + ffa_version = vers;
> > +
> > + return 0;
> > +}
> > +
> > +__initcall(ffa_init);
> > diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> > index ed63c2b6f91f..b3dee269bced 100644
> > --- a/xen/arch/arm/include/asm/domain.h
> > +++ b/xen/arch/arm/include/asm/domain.h
> > @@ -103,6 +103,10 @@ struct arch_domain
> > void *tee;
> > #endif
> >
> > +#ifdef CONFIG_FFA
> > + void *ffa;
> > +#endif
> > +
> > bool directmap;
> > } __cacheline_aligned;
> >
> > diff --git a/xen/arch/arm/include/asm/ffa.h b/xen/arch/arm/include/asm/ffa.h
> > new file mode 100644
> > index 000000000000..4f4a739345bd
> > --- /dev/null
> > +++ b/xen/arch/arm/include/asm/ffa.h
> > @@ -0,0 +1,71 @@
> > +/*
> > + * xen/arch/arm/ffa.c
> > + *
> > + * Arm Firmware Framework for ARMv8-A(FFA) mediator
> > + *
> > + * Copyright (C) 2021 Linaro Limited
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > + * obtaining a copy of this software and associated documentation
> > + * files (the "Software"), to deal in the Software without restriction,
> > + * including without limitation the rights to use, copy, modify, merge,
> > + * publish, distribute, sublicense, and/or sell copies of the Software,
> > + * and to permit persons to whom the Software is furnished to do so,
> > + * subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be
> > + * included in all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
> > + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
> > + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> > + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> > + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +#ifndef __ASM_ARM_FFA_H__
> > +#define __ASM_ARM_FFA_H__
> > +
> > +#include <xen/const.h>
> > +
> > +#include <asm/smccc.h>
> > +#include <asm/types.h>
> > +
> > +#define FFA_FNUM_MIN_VALUE _AC(0x60,U)
> > +#define FFA_FNUM_MAX_VALUE _AC(0x86,U)
> > +
> > +static inline bool is_ffa_fid(uint32_t fid)
> > +{
> > + uint32_t fn = fid & ARM_SMCCC_FUNC_MASK;
> > +
> > + return fn >= FFA_FNUM_MIN_VALUE && fn <= FFA_FNUM_MAX_VALUE;
> > +}
> > +
> > +#ifdef CONFIG_FFA
> > +#define FFA_NR_FUNCS 11
>
> You wrote 11 here, but you seem only expose 2 in this patch. However,
> AFAICT the call count is deprected in SMCCC v1.2. So do we need to
> update it?
>
> But if it is not deprecated then...
I suppose that as long as we do support it we should try to return an
accurate number. This value is used in the macro
SSSC_SMCCC_FUNCTION_COUNT together with values from other services
which is then returned for ARM_SMCCC_CALL_COUNT_FID(STANDARD).
>
> > +
> > +bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid);
> > +int ffa_domain_init(struct domain *d, bool ffa_enabled);
> > +int ffa_relinquish_resources(struct domain *d);
> > +#else
> > +#define FFA_NR_FUNCS 0
> > +
> > +static inline bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
> > +{
> > + return false;
> > +}
> > +
> > +static inline int ffa_domain_init(struct domain *d, bool ffa_enabled)
> > +{
> > + return 0;
>
> For the same reason as above, this should return -ENODEV if ffa_enabled
> is true.
Thanks, I'll fix.
>
> > +}
> > +
> > +static inline int ffa_relinquish_resources(struct domain *d)
> > +{
> > + return 0;
> > +}
> > +#endif
> > +
> > +#endif /*__ASM_ARM_FFA_H__*/
> > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> > index 6f90c08a6304..34586025eff8 100644
> > --- a/xen/arch/arm/vsmc.c
> > +++ b/xen/arch/arm/vsmc.c
> > @@ -20,6 +20,7 @@
> > #include <public/arch-arm/smccc.h>
> > #include <asm/cpuerrata.h>
> > #include <asm/cpufeature.h>
> > +#include <asm/ffa.h>
> > #include <asm/monitor.h>
> > #include <asm/regs.h>
> > #include <asm/smccc.h>
> > @@ -32,7 +33,7 @@
> > #define XEN_SMCCC_FUNCTION_COUNT 3
> >
> > /* Number of functions currently supported by Standard Service Service Calls. */
> > -#define SSSC_SMCCC_FUNCTION_COUNT (3 + VPSCI_NR_FUNCS)
> > +#define SSSC_SMCCC_FUNCTION_COUNT (3 + VPSCI_NR_FUNCS + FFA_NR_FUNCS)
>
> ... it seems incorrect to me add FFA_NR_FUNCS unconditionally because
> while the hypervisor may support FFA, the guest may not have access to it.
OK, I'll add a runtime check.
>
> >
> > static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid)
> > {
> > @@ -196,13 +197,23 @@ static bool handle_existing_apis(struct cpu_user_regs *regs)
> > return do_vpsci_0_1_call(regs, fid);
> > }
> >
> > +static bool is_psci_fid(uint32_t fid)
> > +{
> > + uint32_t fn = fid & ARM_SMCCC_FUNC_MASK;
> > +
> > + return fn >= 0 && fn <= 0x1fU;
> > +}
> > +
> > /* PSCI 0.2 interface and other Standard Secure Calls */
> > static bool handle_sssc(struct cpu_user_regs *regs)
> > {
> > uint32_t fid = (uint32_t)get_user_reg(regs, 0);
> >
> > - if ( do_vpsci_0_2_call(regs, fid) )
> > - return true;
> > + if ( is_psci_fid(fid) )
> > + return do_vpsci_0_2_call(regs, fid);
> > +
> > + if ( is_ffa_fid(fid) )
> > + return ffa_handle_call(regs, fid);
> >
> > switch ( fid )
> > {
> > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> > index ab05fe12b0de..53f8d44a6a8e 100644
> > --- a/xen/include/public/arch-arm.h
> > +++ b/xen/include/public/arch-arm.h
> > @@ -318,6 +318,8 @@ struct xen_arch_domainconfig {
> > /* IN/OUT */
> > uint8_t gic_version;
> > /* IN */
> > + uint8_t ffa_enabled;
>
> AFAIU, this will be 0/1. We have a 'flags' in common but not in arch.
>
> I don't think we should use the common one (I don't think FFA will be
> ever used on x86?). So I would introduce a new field flag in
> xen-arch_domainconfig.
>
> Also AFAICT, XEN_DOMCTL_INTERFACE_VERSION has already been bumped for
> 4.17. So we should not need to do it in this patch. However, if this is
> not going to merged in 4.17, then you will need to bump the domctl
> version (assuming this is the first patch touch the domctl after the
> release).
OK, I'll add a flags field. Let's see who will have to bump
XEN_DOMCTL_INTERFACE_VERSION.
Thanks for the review.
Cheers,
Jens
>
> > + /* IN */
> > uint16_t tee_type;
> > /* IN */
> > uint32_t nr_spis;
>
> --
> Julien Grall
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator
2022-09-05 22:25 ` Julien Grall
@ 2022-09-06 15:19 ` Jens Wiklander
0 siblings, 0 replies; 22+ messages in thread
From: Jens Wiklander @ 2022-09-06 15:19 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Stefano Stabellini, Volodymyr Babchuk,
Bertrand.Marquis, Anthony PERARD, Juergen Gross, Wei Liu
Hi Julien,
On Tue, Sep 6, 2022 at 12:25 AM Julien Grall <julien@xen.org> wrote:
>
> Hi Jens,
>
> More remarks.
>
> On 18/08/2022 11:55, Jens Wiklander wrote:
> > +/* Negotiated FF-A version to use with the SPMC */
> > +static uint32_t ffa_version __read_mostly;
>
> NIT: if this is not meant to be modified after boot, then I would
> suggest to use __ro_after_init. This was introduced recently and will
> prevent the variable to be modified after boot.
Thanks, I'll update
> > +
> > +static bool ffa_get_version(uint32_t *vers)
>
> This is not __init. Is this going to be called at runtime by a domain?
> If yes...
Correct.
>
> > +{
> > + const struct arm_smccc_1_2_regs arg = {
> > + .a0 = FFA_VERSION,
> > + .a1 = FFA_MY_VERSION,
> > + };
> > + struct arm_smccc_1_2_regs resp;
> > +
> > + arm_smccc_1_2_smc(&arg, &resp);
> > + if ( resp.a0 == FFA_RET_NOT_SUPPORTED )
> > + {
> > + printk(XENLOG_ERR "ffa: FFA_VERSION returned not supported\n");
>
> ... this wants to be a XENLOG_G_ERR to rate limited it. XENLOG_ERR is
> not by default and will allow a domain to spam Xen console.
>
> A rule of thumb is any code reachable for a domain (other than dom0)
> should use XENLOG_G_* when printing or gprintk(XENLOG_*, ) if you want
> to print the domain ID and ratelimit. Note that the latter doesn't
> require the G_* becauce it will add it automatically.
Thanks for the explanation, I'll update accordingly.
>
> > + return false;
> > + }
> > +
> > + *vers = resp.a0;
> > +
> > + return true;
> > +}
> > +
> > +static u16 get_vm_id(const struct domain *d)
> > +{
> > + /* +1 since 0 is reserved for the hypervisor in FF-A */
> > + return d->domain_id + 1;
> > +}
> > +
> > +static void set_regs(struct cpu_user_regs *regs, register_t v0, register_t v1,
> > + register_t v2, register_t v3, register_t v4, register_t v5,
> > + register_t v6, register_t v7)
> > +{
> > + set_user_reg(regs, 0, v0);
> > + set_user_reg(regs, 1, v1);
> > + set_user_reg(regs, 2, v2);
> > + set_user_reg(regs, 3, v3);
> > + set_user_reg(regs, 4, v4);
> > + set_user_reg(regs, 5, v5);
> > + set_user_reg(regs, 6, v6);
> > + set_user_reg(regs, 7, v7);
> > +}
> > +
> > +static void set_regs_success(struct cpu_user_regs *regs, uint32_t w2,
> > + uint32_t w3)
> > +{
> > + set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, 0, 0, 0, 0);
> > +}
> > +
> > +static void handle_version(struct cpu_user_regs *regs)
> > +{
> > + struct domain *d = current->domain;
> > + struct ffa_ctx *ctx = d->arch.ffa;
> > + uint32_t vers = get_user_reg(regs, 1);
> > +
> > + if ( vers < FFA_VERSION_1_1 )
> > + vers = FFA_VERSION_1_0;
> > + else
> > + vers = FFA_VERSION_1_1;
> > +
> > + ctx->guest_vers = vers;
> > + set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
> > +}
> > +
> > +bool ffa_handle_call(struct cpu_user_regs *regs, uint32_t fid)
> > +{
> > + struct domain *d = current->domain;
> > + struct ffa_ctx *ctx = d->arch.ffa;
> > +
> > + if ( !ctx )
> > + return false;
> > +
> > + switch ( fid )
> > + {
> > + case FFA_VERSION:
> > + handle_version(regs);
> > + return true;
> > + case FFA_ID_GET:
> > + set_regs_success(regs, get_vm_id(d), 0);
> > + return true;
> > +
> > + default:
> > + printk(XENLOG_ERR "ffa: unhandled fid 0x%x\n", fid);
>
> This one definitely want to be a XENLOG_G_ERR. But I would use
> gprintk(XENLOG_ERR, ).
I'll update.
Again, thanks for the review.
Cheers,
Jens
>
> Cheers,
>
> --
> Julien Grall
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2022-09-06 21:39 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-18 10:55 [PATCH v5 0/9] Xen FF-A mediator Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers Jens Wiklander
2022-08-18 13:44 ` Bertrand Marquis
2022-08-18 14:31 ` Julien Grall
2022-08-18 15:55 ` Bertrand Marquis
2022-08-18 17:31 ` Julien Grall
2022-08-24 8:11 ` Jens Wiklander
2022-08-24 8:04 ` Jens Wiklander
2022-08-23 6:31 ` Jiamei Xie
2022-08-18 10:55 ` [PATCH v5 2/9] xen/arm: move regpair_to_uint64() and uint64_to_regpair() to regs.h Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 3/9] xen/arm: add a primitive FF-A mediator Jens Wiklander
2022-09-05 22:17 ` Julien Grall
2022-09-06 14:57 ` Jens Wiklander
2022-09-05 22:25 ` Julien Grall
2022-09-06 15:19 ` Jens Wiklander
2022-09-06 9:44 ` Anthony PERARD
2022-08-18 10:55 ` [PATCH v5 4/9] xen/arm: ffa: add direct request support Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 5/9] xen/arm: ffa: map SPMC rx/tx buffers Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 6/9] xen/arm: ffa: send guest events to Secure Partitions Jens Wiklander
2022-08-18 10:55 ` [PATCH v5 7/9] xen/arm: ffa: support mapping guest RX/TX buffers Jens Wiklander
2022-08-18 10:56 ` [PATCH v5 8/9] xen/arm: ffa: support guest FFA_PARTITION_INFO_GET Jens Wiklander
2022-08-18 10:56 ` [PATCH v5 9/9] xen/arm: ffa: support sharing memory Jens Wiklander
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.