On Tue, Nov 15, 2022 at 11:03:51AM +0200, Ilias Apalodimas wrote:
Hi Abdellatif
On Mon, Nov 07, 2022 at 07:20:54PM +0000, Abdellatif El Khlifi wrote:
Add MM communication support using FF-A transport
This feature allows accessing MM partitions services through EFI MM communication protocol. MM partitions such as StandAlonneMM or smm-gateway secure partitions which reside in secure world.
An MM shared buffer and a door bell event are used to exchange the data.
The data is used by EFI services such as GetVariable()/SetVariable() and copied from the communication buffer to the MM shared buffer.
config EFI_MM_COMM_TEE
- bool "UEFI variables storage service via OP-TEE"
- depends on OPTEE
- bool "UEFI variables storage service via the trusted world"
- depends on OPTEE && ARM_FFA_TRANSPORT
This shouldn't rely on both. It's either OP-TEE or FF-A
Thanks. Addressed in v8 patchset.
+#if (IS_ENABLED(CONFIG_OPTEE))
This separation is a bit problematic. A user can configure bot OP-TEE and FF-A. Those are not mutually exclusive, but for the EFI variables case they are. We need a better way to isolate the compilation choices. Why don't we make ffa_bus_discover() return -1 if FF-A isn't compiled in?
I agree, this is addressed in v8 patchset.
/**
- get_connection() - Retrieve OP-TEE session for a specific UUID.
@@ -143,13 +176,229 @@ static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
return ret; } +#endif
+#if (IS_ENABLED(CONFIG_ARM_FFA_TRANSPORT))
+/**
- ffa_notify_mm_sp() - Announce there is data in the shared buffer
- Notifies the MM partition in the trusted world that
- data is available in the shared buffer.
- This is a blocking call during which trusted world has exclusive access
- to the MM shared buffer.
- Return:
- 0 on success
- */
+static int ffa_notify_mm_sp(void) +{
- struct ffa_send_direct_data msg = {0};
- int ret;
- int sp_event_ret = -1;
- if (!ffa_bus_ops_get())
return -EINVAL;- msg.data0 = FFA_SHARED_MM_BUFFER_OFFSET; /* x3 */
- ret = ffa_bus_ops_get()->sync_send_receive(mm_sp_id, &msg, 1);
- if (ret != 0)
return ret;- sp_event_ret = msg.data0; /* x3 */
- if (sp_event_ret == MM_SUCCESS)
return 0;- /*
* Failure to notify the MM SP*/- return -EACCES;
+}
+/**
- ffa_discover_mm_sp_id() - Query the MM partition ID
- Use the FF-A driver to get the MM partition ID.
- If multiple partitions are found, use the first one.
- This is a boot time function.
- Return:
- 0 on success
- */
+static int ffa_discover_mm_sp_id(void) +{
- u32 count = 0, size = 0;
- int ret;
- struct ffa_partition_info *parts_info;
- if (!ffa_bus_ops_get())
return -EINVAL;- /*
* get from the driver the count of the SPs matching the UUID*/- ret = ffa_bus_ops_get()->partition_info_get(mm_sp_svc_uuid, &count, NULL);
- if (ret != 0) {
log_err("EFI: Failure in querying partitions count (error code: %d)\n", ret);return ret;- }
- if (!count) {
log_info("EFI: No MM partition found\n");return ret;- }
- /*
* pre-allocate a buffer to be filled by the driver* with ffa_partition_info structs*/- log_info("EFI: Pre-allocating %d partition(s) info structures\n", count);
- parts_info = calloc(count, sizeof(struct ffa_partition_info));
I prefer sizeof(*parts_info). Same goes for all sizeof() calls.
Addressed in v8 patchset.
- if (!parts_info)
return -ENOMEM;- size = count * sizeof(struct ffa_partition_info);
- /*
* ask the driver to fill the* buffer with the SPs info*/- ret = ffa_bus_ops_get()->partition_info_get(mm_sp_svc_uuid, &size, parts_info);
- if (ret != 0) {
if (!ret)
Addressed in v8 patchset.
[...]
Thanks /Ilias