2022-09-07 15:42:08

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH v3 00/10] firmware: arm_ffa: Refactoring and initial/minor v1.1 update

Hi All,

This series is just some refactoring in preparation to add FF-A v1.1
support. It doesn't have any memory layout or notification changes
supported in v1.1 yet.

Regards,
Sudeep

v2[2]->v3:
- Fixed the logic to set 32bit mode which was wrong.
- Ensured that we advance each partition info size by returned
size even if the size is greater than the partition_info structure,
we will still just copy the right size.

v1[1]->v2[2]:
- Merged dropping of ffa_ops in optee_ffa structure and using
ffa_dev->ops into single patch
- Added separate patch(didn't fit any patch strictly to fit in)
to rename ffa_dev_ops as ffa_ops as suggested by Sumit
- Fixed some minor comments, handling size > structure size in
partition_info_get and added extra parameter to ffa_features
to get both possible output/interface properties.

[1] https://lore.kernel.org/all/[email protected]
[2] https://lore.kernel.org/all/[email protected]

Sudeep Holla (10):
firmware: arm_ffa: Add pointer to the ffa_dev_ops in struct ffa_dev
tee: optee: Drop ffa_ops in optee_ffa structure using ffa_dev->ops directly
firmware: arm_ffa: Remove ffa_dev_ops_get()
firmware: arm_ffa: Add support for querying FF-A features
firmware: arm_ffa: Use FFA_FEATURES to detect if native versions are supported
firmware: arm_ffa: Make memory apis ffa_device independent
firmware: arm_ffa: Rename ffa_dev_ops as ffa_ops
firmware: arm_ffa: Add v1.1 get_partition_info support
firmware: arm_ffa: Set up 32bit execution mode flag using partiion property
firmware: arm_ffa: Split up ffa_ops into info, message and memory operations

drivers/firmware/arm_ffa/bus.c | 4 +-
drivers/firmware/arm_ffa/driver.c | 131 +++++++++++++++++++++++-------
drivers/tee/optee/ffa_abi.c | 46 +++++------
drivers/tee/optee/optee_private.h | 1 -
include/linux/arm_ffa.h | 36 +++++---
5 files changed, 151 insertions(+), 67 deletions(-)

--
2.37.3


2022-09-07 15:43:02

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH v3 01/10] firmware: arm_ffa: Add pointer to the ffa_dev_ops in struct ffa_dev

Currently ffa_dev_ops_get() is the way to fetch the ffa_dev_ops pointer.
It checks if the ffa_dev structure pointer is valid before returning the
ffa_dev_ops pointer.

Instead, the pointer can be made part of the ffa_dev structure and since
the core driver is incharge of creating ffa_device for each identified
partition, there is no need to check for the validity explicitly if the
pointer is embedded in the structure.

Add the pointer to the ffa_dev_ops in the ffa_dev structure itself and
initialise the same as part of creation of the device.

Reviewed-by: Jens Wiklander <[email protected]>
Signed-off-by: Sudeep Holla <[email protected]>
---
drivers/firmware/arm_ffa/bus.c | 4 +++-
drivers/firmware/arm_ffa/driver.c | 2 +-
include/linux/arm_ffa.h | 7 +++++--
3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
index 641a91819088..69328041fbc3 100644
--- a/drivers/firmware/arm_ffa/bus.c
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -167,7 +167,8 @@ bool ffa_device_is_valid(struct ffa_device *ffa_dev)
return valid;
}

-struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id)
+struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id,
+ const struct ffa_dev_ops *ops)
{
int ret;
struct device *dev;
@@ -183,6 +184,7 @@ struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id)
dev_set_name(&ffa_dev->dev, "arm-ffa-%04x", vm_id);

ffa_dev->vm_id = vm_id;
+ ffa_dev->ops = ops;
uuid_copy(&ffa_dev->uuid, uuid);

ret = device_register(&ffa_dev->dev);
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index ec731e9e942b..213665e5ad0e 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -688,7 +688,7 @@ static void ffa_setup_partitions(void)
* as part of the discovery API, we need to pass the
* discovered UUID here instead.
*/
- ffa_dev = ffa_device_register(&uuid_null, tpbuf->id);
+ ffa_dev = ffa_device_register(&uuid_null, tpbuf->id, &ffa_ops);
if (!ffa_dev) {
pr_err("%s: failed to register partition ID 0x%x\n",
__func__, tpbuf->id);
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index e5c76c1ef9ed..91b47e42b73d 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -17,6 +17,7 @@ struct ffa_device {
bool mode_32bit;
uuid_t uuid;
struct device dev;
+ const struct ffa_dev_ops *ops;
};

#define to_ffa_dev(d) container_of(d, struct ffa_device, dev)
@@ -47,7 +48,8 @@ static inline void *ffa_dev_get_drvdata(struct ffa_device *fdev)
}

#if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
-struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id);
+struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id,
+ const struct ffa_dev_ops *ops);
void ffa_device_unregister(struct ffa_device *ffa_dev);
int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
const char *mod_name);
@@ -57,7 +59,8 @@ const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev);

#else
static inline
-struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id)
+struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id,
+ const struct ffa_dev_ops *ops)
{
return NULL;
}
--
2.37.3

2022-09-07 15:44:27

by Sudeep Holla

[permalink] [raw]
Subject: [PATCH v3 02/10] tee: optee: Drop ffa_ops in optee_ffa structure using ffa_dev->ops directly

Now that the ffa_device structure holds the pointer to ffa_dev_ops,
there is no need to obtain the same through ffa_dev_ops_get().

Just use the ffa_dev->ops directly. Since the ffa_device itself carries
ffa_dev_ops now, there is no need to keep a copy in optee_ffa structure.

Drop ffa_ops in the optee_ffa structure as it is not needed anymore.

Reviewed-by: Jens Wiklander <[email protected]>
Reviewed-by: Sumit Garg <[email protected]>
Signed-off-by: Sudeep Holla <[email protected]>
---
drivers/tee/optee/ffa_abi.c | 15 +++++----------
drivers/tee/optee/optee_private.h | 1 -
2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
index 7ab31740cff8..3d4079575ccd 100644
--- a/drivers/tee/optee/ffa_abi.c
+++ b/drivers/tee/optee/ffa_abi.c
@@ -271,8 +271,8 @@ static int optee_ffa_shm_register(struct tee_context *ctx, struct tee_shm *shm,
unsigned long start)
{
struct optee *optee = tee_get_drvdata(ctx->teedev);
- const struct ffa_dev_ops *ffa_ops = optee->ffa.ffa_ops;
struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
+ const struct ffa_dev_ops *ffa_ops = ffa_dev->ops;
struct ffa_mem_region_attributes mem_attr = {
.receiver = ffa_dev->vm_id,
.attrs = FFA_MEM_RW,
@@ -314,8 +314,8 @@ static int optee_ffa_shm_unregister(struct tee_context *ctx,
struct tee_shm *shm)
{
struct optee *optee = tee_get_drvdata(ctx->teedev);
- const struct ffa_dev_ops *ffa_ops = optee->ffa.ffa_ops;
struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
+ const struct ffa_dev_ops *ffa_ops = ffa_dev->ops;
u64 global_handle = shm->sec_world_id;
struct ffa_send_direct_data data = {
.data0 = OPTEE_FFA_UNREGISTER_SHM,
@@ -342,7 +342,7 @@ static int optee_ffa_shm_unregister_supp(struct tee_context *ctx,
struct tee_shm *shm)
{
struct optee *optee = tee_get_drvdata(ctx->teedev);
- const struct ffa_dev_ops *ffa_ops = optee->ffa.ffa_ops;
+ const struct ffa_dev_ops *ffa_ops = optee->ffa.ffa_dev->ops;
u64 global_handle = shm->sec_world_id;
int rc;

@@ -529,8 +529,8 @@ static int optee_ffa_yielding_call(struct tee_context *ctx,
struct optee_msg_arg *rpc_arg)
{
struct optee *optee = tee_get_drvdata(ctx->teedev);
- const struct ffa_dev_ops *ffa_ops = optee->ffa.ffa_ops;
struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
+ const struct ffa_dev_ops *ffa_ops = ffa_dev->ops;
struct optee_call_waiter w;
u32 cmd = data->data0;
u32 w4 = data->data1;
@@ -793,11 +793,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
u32 sec_caps;
int rc;

- ffa_ops = ffa_dev_ops_get(ffa_dev);
- if (!ffa_ops) {
- pr_warn("failed \"method\" init: ffa\n");
- return -ENOENT;
- }
+ ffa_ops = ffa_dev->ops;

if (!optee_ffa_api_is_compatbile(ffa_dev, ffa_ops))
return -EINVAL;
@@ -821,7 +817,6 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)

optee->ops = &optee_ffa_ops;
optee->ffa.ffa_dev = ffa_dev;
- optee->ffa.ffa_ops = ffa_ops;
optee->rpc_param_count = rpc_param_count;

teedev = tee_device_alloc(&optee_ffa_clnt_desc, NULL, optee->pool,
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index a33d98d17cfd..04ae58892608 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -111,7 +111,6 @@ struct optee_smc {
*/
struct optee_ffa {
struct ffa_device *ffa_dev;
- const struct ffa_dev_ops *ffa_ops;
/* Serializes access to @global_ids */
struct mutex mutex;
struct rhashtable global_ids;
--
2.37.3

2022-09-08 12:09:29

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH v3 00/10] firmware: arm_ffa: Refactoring and initial/minor v1.1 update

On Wed, 7 Sep 2022 15:52:30 +0100, Sudeep Holla wrote:
> This series is just some refactoring in preparation to add FF-A v1.1
> support. It doesn't have any memory layout or notification changes
> supported in v1.1 yet.
>
> Regards,
> Sudeep
>
> [...]

Applied to sudeep.holla/linux (for-next/ffa), thanks!

[01/10] firmware: arm_ffa: Add pointer to the ffa_dev_ops in struct ffa_dev
https://git.kernel.org/sudeep.holla/c/d01387fc1642
[02/10] tee: optee: Drop ffa_ops in optee_ffa structure using ffa_dev->ops directly
https://git.kernel.org/sudeep.holla/c/320c3fa38c51
[03/10] firmware: arm_ffa: Remove ffa_dev_ops_get()
https://git.kernel.org/sudeep.holla/c/55bf84fd0a76
[04/10] firmware: arm_ffa: Add support for querying FF-A features
https://git.kernel.org/sudeep.holla/c/cb1f4c2c15bb
[05/10] firmware: arm_ffa: Use FFA_FEATURES to detect if native versions are supported
https://git.kernel.org/sudeep.holla/c/e57fba9105fa
[06/10] firmware: arm_ffa: Make memory apis ffa_device independent
https://git.kernel.org/sudeep.holla/c/8c3812c8f74f
[07/10] firmware: arm_ffa: Rename ffa_dev_ops as ffa_ops
https://git.kernel.org/sudeep.holla/c/7aa7a9798955
[08/10] firmware: arm_ffa: Add v1.1 get_partition_info support
https://git.kernel.org/sudeep.holla/c/bb1be7498500
[09/10] firmware: arm_ffa: Set up 32bit execution mode flag using partiion property
(with the additional version check as discussed on the list)
https://git.kernel.org/sudeep.holla/c/106b11b1ccd5
[10/10] firmware: arm_ffa: Split up ffa_ops into info, message and memory operations
https://git.kernel.org/sudeep.holla/c/5b0c6328e47d

--
Regards,
Sudeep