Hi Volodymyr,
Only few NITs in this patch. See below:
On 07/03/2019 21:04, Volodymyr Babchuk wrote:
+/*
+ * Default maximal number concurrent threads that OP-TEE supports.
+ * This limits number of standard calls that guest can have.
+ */
+#define DEF_MAX_OPTEE_THREADS 16
+
#define OPTEE_KNOWN_NSEC_CAPS OPTEE_SMC_NSEC_CAP_UNIPROCESSOR
#define OPTEE_KNOWN_SEC_CAPS (OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM | \
OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM | \
OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
+static unsigned int max_optee_threads = DEF_MAX_OPTEE_THREADS;
NIT: This could be __read_mostly.
+
+/*
+ * Call context. OP-TEE can issue multiple RPC returns during one call.
+ * We need to preserve context during them.
+ */
+struct optee_std_call {
+ struct list_head list;
+ /* Page where shadowed copy of call arguments is stored */
+ struct page_info *xen_arg_pg;
+ /* Above page mapped into XEN */
+ struct optee_msg_arg *xen_arg;
+ /* Address of original call arguments */
+ paddr_t guest_arg_ipa;
+ int optee_thread_id;
+ int rpc_op;
+ bool in_flight;
+};
+
+/* Domain context */
+struct optee_domain {
+ struct list_head call_list;
+ atomic_t call_count;
+ spinlock_t lock;
+};
+
static bool optee_probe(void)
{
struct dt_device_node *node;
@@ -48,12 +82,25 @@ static bool optee_probe(void)
(uint32_t)resp.a3 != OPTEE_MSG_UID_3 )
return false;
+ /* Read number of threads */
+ arm_smccc_smc(OPTEE_SMC_GET_CONFIG, OPTEE_SMC_CONFIG_NUM_THREADS, &resp);
+ if ( resp.a0 == OPTEE_SMC_RETURN_OK )
Out of interest, when was this call added?
+ {
+ max_optee_threads = resp.a1;
+ printk(XENLOG_DEBUG "OP-TEE supports %u threads.\n",
max_optee_threads);
extra NIT: I would use XENLOG_INFO rather than XENLOG_DEBUG. This is a useful
information to have in bug report.
Regarding the message, what matters is the number of threads for guest. So I
would rework it to make it more meaning full for a user that does not know the
internal.
You might also want to move the message out of the if. So you have the message
even when OP-TEE does not support the SMC.
[...]
+static struct optee_std_call *get_std_call(struct optee_domain *ctx,
+ int thread_id)
+{
+ struct optee_std_call *call;
+
+ spin_lock(&ctx->lock);
+ list_for_each_entry( call, &ctx->call_list, list )
+ {
+ if ( call->optee_thread_id == thread_id )
+ {
+ if ( call->in_flight )
+ {
+ gdprintk(XENLOG_WARNING, "Guest tries to execute call which is
already in flight\n");
NIT: Missing full stop.
Also, the line is over 80 characters. While we don't want to split in the middle
of the message (so ack/grep can be used easily), you will want to split the line
after the comma.
+ goto out;
+ }
+ call->in_flight = true;
+ map_xen_arg(call);
NIT: Does this need to be done with the lock taken?
+ spin_unlock(&ctx->lock);
+
+ return call;
+ }
+ }
+
+out:
+ spin_unlock(&ctx->lock);
+
+ return NULL;
+}
+
+static void put_std_call(struct optee_domain *ctx, struct optee_std_call *call)
+{
+ spin_lock(&ctx->lock);
+ ASSERT(call->in_flight);
+ unmap_xen_arg(call);
Same question for the unmap.
+ call->in_flight = false;
+ spin_unlock(&ctx->lock);
+}
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
