Hi Jens,
Mainly reviewing this patch from a Xen PoV. I will leave the others to
review the patch from a spec-compliance PoV.
On 13/04/2023 08:14, Jens Wiklander wrote:
+struct ffa_ctx {
+ /* FF-A version used by the guest */
+ uint32_t guest_vers;
+};
+
+/* Negotiated FF-A version to use with the SPMC */
+static uint32_t ffa_version __ro_after_init;
Coding style: We tend to add attributes just after the type. E.g.:
static uint32_t __ro_after_init ffa_version;
I can deal with this one on commit if there is nothing else to change.
[...]
+static int ffa_domain_init(struct domain *d)
+{
+ struct ffa_ctx *ctx;
+
+ if ( !ffa_version )
+ return -ENODEV;
+
+ ctx = xzalloc(struct ffa_ctx);
+ if ( !ctx )
+ return -ENOMEM;
+
+ d->arch.tee = ctx;
+
+ return 0;
+}
+
+/* This function is supposed to undo what ffa_domain_init() has done */
I think there is a problem in the TEE framework. The callback
.relinquish_resources() will not be called if domain_create() failed. So
this will result to a memory leak.
We also can't call .relinquish_resources() on early domain creation
failure because relinquishing resources can take time and therefore
needs to be preemptible.
So I think we need to introduce a new callback domain_free() that will
be called arch_domain_destroy(). Is this something you can look at?
+static int ffa_relinquish_resources(struct domain *d)
+{
+ struct ffa_ctx *ctx = d->arch.tee;
+
+ if ( !ctx )
+ return 0;
+
+ XFREE(d->arch.tee);
+
+ return 0;
+}
Cheers,
--
Julien Grall
