[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/2] xen/arm: add FF-A mediator
Hi,
On 26/07/2022 07:17, Jens Wiklander wrote:
On Fri, Jul 8, 2022 at 3:41 PM Julien Grall <julien@xxxxxxx> wrote:
Hi Jens,
I haven't checked whether the FFA driver is complaint with the spec. I
mainly checked whether the code makes sense from Xen PoV.
This is a fairly long patch to review. So I will split my review in
multiple session/e-mail.
On 22/06/2022 14:42, Jens Wiklander wrote:
Adds a FF-A version 1.1 [1] mediator to communicate with a Secure
Partition in secure world.
The implementation is the bare minimum to be able to communicate with
OP-TEE running as an SPMC at S-EL1.
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@xxxxxxxxxx>
---
SUPPORT.md | 7 +
tools/libs/light/libxl_arm.c | 3 +
tools/libs/light/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 3 +
These changes would need an ack from the toolstack maintainers.
OK, I'll keep them in CC.
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 | 1683 +++++++++++++++++++++++++++++
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 +
13 files changed, 1811 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/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),
This needs to be accompagnied with a define LIBXL_HAVE_* in
tools/include/libxl.h. Please see the examples in the header.
OK, I'll add something. I'm not entirely sure how this is used so I'm
afraid it will be a bit of Cargo Cult programming from my side.
The LIBXL_HAVE* by toolstack built on top of libxl (like virtio) to know
whether a future is supported by the current library.
[...]
+
+static inline uint64_t reg_pair_to_64(uint32_t reg0, uint32_t reg1)
+{
+ return (uint64_t)reg0 << 32 | reg1;
+}
+
+static void inline reg_pair_from_64(uint32_t *reg0, uint32_t *reg1,
+ uint64_t val)
+{
+ *reg0 = val >> 32;
+ *reg1 = val;
+}
Those two functions are the same as optee.c but with a different. I
would rather prefer if we avoid the duplication and provide generic
helpers in a pre-requisite.
These functions are trivial but at the same time for a special purpose
which happens to coincide with the usage in optee.c. I can't find a
suitable common .h file and creating a new one seems a bit much.
I would implement it in regs.h.
[...]
+ .a4 = pg_count,
+ };
+ struct arm_smccc_1_2_regs resp;
+
+ /*
+ * For arm64 we must use 64-bit calling convention if the buffer isn't
+ * passed in our tx buffer.
+ */
Can you explain why we would want to use the 32-bit calling convention
if addr is 0?
I was trying to avoid the 64-bit calling convention where possible,
OOI, why are you trying to avoid the 64-bit calling convention?
[...]
--
Julien Grall
|
