The FF-A specification defines framework messages sent as direct
requests when certain events occurs. For instance when a VM (guest) is
created or destroyed. Only SPs which have subscribed to these events
will receive them. An SP can subscribe to these messages in its
partition properties.
Adds a check that the SP supports the needed FF-A features
FFA_PARTITION_INFO_GET and FFA_RX_RELEASE.
The partition properties of each SP is retrieved with
FFA_PARTITION_INFO_GET which returns the information in our RX buffer.
Using FFA_PARTITION_INFO_GET changes the owner of the RX buffer to the
caller (us), so once we're done with the buffer it must be released
using FFA_RX_RELEASE before another call can be made.
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
xen/arch/arm/tee/ffa.c | 200 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 199 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
index 054121fe4321..b4fea65ce31d 100644
--- a/xen/arch/arm/tee/ffa.c
+++ b/xen/arch/arm/tee/ffa.c
@@ -160,6 +160,14 @@
#define FFA_MSG_SEND 0x8400006EU
#define FFA_MSG_POLL 0x8400006AU
+/* Partition information descriptor */
+struct ffa_partition_info_1_1 {
+ uint16_t id;
+ uint16_t execution_context;
+ uint32_t partition_properties;
+ uint8_t uuid[16];
+};
+
struct ffa_ctx {
/* FF-A version used by the guest */
uint32_t guest_vers;
@@ -168,6 +176,12 @@ struct ffa_ctx {
/* Negotiated FF-A version to use with the SPMC */
static uint32_t ffa_version __ro_after_init;
+/* SPs subscribing to VM_CREATE and VM_DESTROYED events */
+static uint16_t *subscr_vm_created __read_mostly;
+static uint16_t subscr_vm_created_count __read_mostly;
+static uint16_t *subscr_vm_destroyed __read_mostly;
+static uint16_t subscr_vm_destroyed_count __read_mostly;
+
/*
* Our rx/tx buffers shared with the SPMC. FFA_RXTX_PAGE_COUNT is the
* number of pages used in each of these buffers.
@@ -251,6 +265,72 @@ static int32_t ffa_rxtx_map(paddr_t tx_addr, paddr_t rx_addr,
return ffa_simple_call(FFA_RXTX_MAP_64, tx_addr, rx_addr, page_count, 0);
}
+static int32_t ffa_partition_info_get(uint32_t w1, uint32_t w2, uint32_t w3,
+ uint32_t w4, uint32_t w5,
+ uint32_t *count)
+{
+ const struct arm_smccc_1_2_regs arg = {
+ .a0 = FFA_PARTITION_INFO_GET,
+ .a1 = w1,
+ .a2 = w2,
+ .a3 = w3,
+ .a4 = w4,
+ .a5 = w5,
+ };
+ struct arm_smccc_1_2_regs resp;
+ uint32_t ret;
+
+ arm_smccc_1_2_smc(&arg, &resp);
+
+ ret = get_ffa_ret_code(&resp);
+ if ( !ret )
+ *count = resp.a2;
+
+ return ret;
+}
+
+static int32_t ffa_rx_release(void)
+{
+ return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
+}
+
+static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
+ uint8_t msg)
+{
+ uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
+ int32_t res;
+
+ if ( msg == FFA_MSG_SEND_VM_CREATED )
+ exp_resp |= FFA_MSG_RESP_VM_CREATED;
+ else if ( msg == FFA_MSG_SEND_VM_DESTROYED )
+ exp_resp |= FFA_MSG_RESP_VM_DESTROYED;
+ else
+ return FFA_RET_INVALID_PARAMETERS;
+
+ do {
+ const struct arm_smccc_1_2_regs arg = {
+ .a0 = FFA_MSG_SEND_DIRECT_REQ_32,
+ .a1 = sp_id,
+ .a2 = FFA_MSG_FLAG_FRAMEWORK | msg,
+ .a5 = vm_id,
+ };
+ struct arm_smccc_1_2_regs resp;
+
+ arm_smccc_1_2_smc(&arg, &resp);
+ if ( resp.a0 != FFA_MSG_SEND_DIRECT_RESP_32 || resp.a2 != exp_resp )
+ {
+ /*
+ * This is an invalid response, likely due to some error in the
+ * implementation of the ABI.
+ */
+ return FFA_RET_INVALID_PARAMETERS;
+ }
+ res = resp.a3;
+ } while ( res == FFA_RET_INTERRUPTED || res == FFA_RET_RETRY );
+
+ return res;
+}
+
static uint16_t get_vm_id(const struct domain *d)
{
/* +1 since 0 is reserved for the hypervisor in FF-A */
@@ -374,6 +454,10 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
static int ffa_domain_init(struct domain *d)
{
struct ffa_ctx *ctx;
+ unsigned int n;
+ unsigned int m;
+ unsigned int c_pos;
+ int32_t res;
if ( !ffa_version )
return -ENODEV;
@@ -388,24 +472,134 @@ static int ffa_domain_init(struct domain *d)
if ( !ctx )
return -ENOMEM;
+ for ( n = 0; n < subscr_vm_created_count; n++ )
+ {
+ res = ffa_direct_req_send_vm(subscr_vm_created[n], get_vm_id(d),
+ FFA_MSG_SEND_VM_CREATED);
+ if ( res )
+ {
+ printk(XENLOG_ERR "ffa: Failed to report creation of vm_id %u to %u: res %d\n",
+ get_vm_id(d), subscr_vm_created[n], res);
+ c_pos = n;
+ goto err;
+ }
+ }
+
d->arch.tee = ctx;
return 0;
+
+err:
+ /* Undo any already sent vm created messaged */
+ for ( n = 0; n < c_pos; n++ )
+ for ( m = 0; m < subscr_vm_destroyed_count; m++ )
+ if ( subscr_vm_destroyed[m] == subscr_vm_created[n] )
+ ffa_direct_req_send_vm(subscr_vm_destroyed[n], get_vm_id(d),
+ FFA_MSG_SEND_VM_DESTROYED);
+
+ return -EIO;
}
/* This function is supposed to undo what ffa_domain_init() has done */
static int ffa_relinquish_resources(struct domain *d)
{
struct ffa_ctx *ctx = d->arch.tee;
+ unsigned int n;
+ int32_t res;
if ( !ctx )
return 0;
+ for ( n = 0; n < subscr_vm_destroyed_count; n++ )
+ {
+ res = ffa_direct_req_send_vm(subscr_vm_destroyed[n], get_vm_id(d),
+ FFA_MSG_SEND_VM_DESTROYED);
+
+ if ( res )
+ printk(XENLOG_ERR "ffa: Failed to report destruction of vm_id %u to %u: res %d\n",
+ get_vm_id(d), subscr_vm_destroyed[n], res);
+ }
+
XFREE(d->arch.tee);
return 0;
}
+static void uninit_subscribers(void)
+{
+ subscr_vm_created_count = 0;
+ subscr_vm_destroyed_count = 0;
+ XFREE(subscr_vm_created);
+ XFREE(subscr_vm_destroyed);
+}
+
+static bool init_subscribers(struct ffa_partition_info_1_1 *fpi, uint16_t count)
+{
+ uint16_t n;
+ uint16_t c_pos;
+ uint16_t d_pos;
+
+ subscr_vm_created_count = 0;
+ subscr_vm_destroyed_count = 0;
+ for ( n = 0; n < count; n++ )
+ {
+ if (fpi[n].partition_properties & FFA_PART_PROP_NOTIF_CREATED)
+ subscr_vm_created_count++;
+ if (fpi[n].partition_properties & FFA_PART_PROP_NOTIF_DESTROYED)
+ subscr_vm_destroyed_count++;
+ }
+
+ if ( subscr_vm_created_count )
+ subscr_vm_created = xzalloc_array(uint16_t, subscr_vm_created_count);
+ if ( subscr_vm_destroyed_count )
+ subscr_vm_destroyed = xzalloc_array(uint16_t,
+ subscr_vm_destroyed_count);
+ if ( (subscr_vm_created_count && !subscr_vm_created) ||
+ (subscr_vm_destroyed_count && !subscr_vm_destroyed) )
+ {
+ printk(XENLOG_ERR "ffa: Failed to allocate subscription lists\n");
+ uninit_subscribers();
+ return false;
+ }
+
+ for ( c_pos = 0, d_pos = 0, n = 0; n < count; n++ )
+ {
+ if ( fpi[n].partition_properties & FFA_PART_PROP_NOTIF_CREATED )
+ subscr_vm_created[c_pos++] = fpi[n].id;
+ if ( fpi[n].partition_properties & FFA_PART_PROP_NOTIF_DESTROYED )
+ subscr_vm_destroyed[d_pos++] = fpi[n].id;
+ }
+
+ return true;
+}
+
+static bool init_sps(void)
+{
+ bool ret = false;
+ uint32_t count;
+ int e;
+
+ e = ffa_partition_info_get(0, 0, 0, 0, 0, &count);
+ if ( e )
+ {
+ printk(XENLOG_ERR "ffa: Failed to get list of SPs: %d\n", e);
+ goto out;
+ }
+
+ if ( count >= UINT16_MAX )
+ {
+ printk(XENLOG_ERR "ffa: Impossible number of SPs: %u\n", count);
+ goto out;
+ }
+
+ ret = init_subscribers(ffa_rx, count);
+
+out:
+ ffa_rx_release();
+
+ return ret;
+}
+
static bool ffa_probe(void)
{
uint32_t vers;
@@ -456,7 +650,8 @@ static bool ffa_probe(void)
* TODO save result of checked features and use that information to
* accept or reject requests from guests.
*/
- if (
+ if ( !check_mandatory_feature(FFA_PARTITION_INFO_GET) ||
+ !check_mandatory_feature(FFA_RX_RELEASE) ||
!check_mandatory_feature(FFA_RXTX_MAP_64) ||
!check_mandatory_feature(FFA_RXTX_UNMAP) ||
!check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
@@ -478,6 +673,9 @@ static bool ffa_probe(void)
}
ffa_version = vers;
+ if ( !init_sps() )
+ goto err_free_ffa_tx;
+
return true;
err_free_ffa_tx:
--
2.34.1Hi,
On 13/04/2023 08:14, Jens Wiklander wrote:
> +static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
> + uint8_t msg)
> +{
> + uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
> + int32_t res;
> +
> + if ( msg == FFA_MSG_SEND_VM_CREATED )
> + exp_resp |= FFA_MSG_RESP_VM_CREATED;
> + else if ( msg == FFA_MSG_SEND_VM_DESTROYED )
> + exp_resp |= FFA_MSG_RESP_VM_DESTROYED;
> + else
> + return FFA_RET_INVALID_PARAMETERS;
> +
> + do {
> + const struct arm_smccc_1_2_regs arg = {
> + .a0 = FFA_MSG_SEND_DIRECT_REQ_32,
> + .a1 = sp_id,
> + .a2 = FFA_MSG_FLAG_FRAMEWORK | msg,
> + .a5 = vm_id,
> + };
> + struct arm_smccc_1_2_regs resp;
> +
> + arm_smccc_1_2_smc(&arg, &resp);
> + if ( resp.a0 != FFA_MSG_SEND_DIRECT_RESP_32 || resp.a2 != exp_resp )
> + {
> + /*
> + * This is an invalid response, likely due to some error in the
> + * implementation of the ABI.
> + */
> + return FFA_RET_INVALID_PARAMETERS;
> + }
> + res = resp.a3;
> + } while ( res == FFA_RET_INTERRUPTED || res == FFA_RET_RETRY );
This loop seems potentially unbounded to me. Can you add a comment
explaining why this is fine?
> +
> + return res;
> +}
> +
Cheers,
--
Julien Grall
Hi,
On Thu, Apr 13, 2023 at 3:24 PM Julien Grall <julien@xen.org> wrote:
>
> Hi,
>
> On 13/04/2023 08:14, Jens Wiklander wrote:
> > +static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
> > + uint8_t msg)
> > +{
> > + uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
> > + int32_t res;
> > +
> > + if ( msg == FFA_MSG_SEND_VM_CREATED )
> > + exp_resp |= FFA_MSG_RESP_VM_CREATED;
> > + else if ( msg == FFA_MSG_SEND_VM_DESTROYED )
> > + exp_resp |= FFA_MSG_RESP_VM_DESTROYED;
> > + else
> > + return FFA_RET_INVALID_PARAMETERS;
> > +
> > + do {
> > + const struct arm_smccc_1_2_regs arg = {
> > + .a0 = FFA_MSG_SEND_DIRECT_REQ_32,
> > + .a1 = sp_id,
> > + .a2 = FFA_MSG_FLAG_FRAMEWORK | msg,
> > + .a5 = vm_id,
> > + };
> > + struct arm_smccc_1_2_regs resp;
> > +
> > + arm_smccc_1_2_smc(&arg, &resp);
> > + if ( resp.a0 != FFA_MSG_SEND_DIRECT_RESP_32 || resp.a2 != exp_resp )
> > + {
> > + /*
> > + * This is an invalid response, likely due to some error in the
> > + * implementation of the ABI.
> > + */
> > + return FFA_RET_INVALID_PARAMETERS;
> > + }
> > + res = resp.a3;
> > + } while ( res == FFA_RET_INTERRUPTED || res == FFA_RET_RETRY );
>
> This loop seems potentially unbounded to me. Can you add a comment
> explaining why this is fine?
In the FF-A 1.1 specification
(https://developer.arm.com/documentation/den0077/e/?lang=en) Table
18.26 at page 330 it says that FFA_RET_INTERRUPTED and FFA_RET_RETRY
should be handled in this way. When looking at this from the
hypervisor's point of view it is troublesome since there isn't any
guarantee that we're progressing.
We should be able to rule out FFA_RET_INTERRUPTED since non-secure
interrupts should be masked at this point. I'm not sure if
FFA_RET_RETRY can be avoided entirely, but we should be able to put a
limit on how many times we're prepared to retry.
How about setting a limit of max 10 retries and treating
FFA_RET_INTERRUPTED as an error? Or is it better to not loop at all
and treat all but FFA_RET_OK as errors? What do others think?
Thanks,
Jens
>
> > +
> > + return res;
> > +}
> > +
>
> Cheers,
>
> --
> Julien Grall
Hi Jens,
> On 14 Apr 2023, at 10:19, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> Hi,
>
> On Thu, Apr 13, 2023 at 3:24 PM Julien Grall <julien@xen.org> wrote:
>>
>> Hi,
>>
>> On 13/04/2023 08:14, Jens Wiklander wrote:
>>> +static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
>>> + uint8_t msg)
>>> +{
>>> + uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
>>> + int32_t res;
>>> +
>>> + if ( msg == FFA_MSG_SEND_VM_CREATED )
>>> + exp_resp |= FFA_MSG_RESP_VM_CREATED;
>>> + else if ( msg == FFA_MSG_SEND_VM_DESTROYED )
>>> + exp_resp |= FFA_MSG_RESP_VM_DESTROYED;
>>> + else
>>> + return FFA_RET_INVALID_PARAMETERS;
>>> +
>>> + do {
>>> + const struct arm_smccc_1_2_regs arg = {
>>> + .a0 = FFA_MSG_SEND_DIRECT_REQ_32,
>>> + .a1 = sp_id,
>>> + .a2 = FFA_MSG_FLAG_FRAMEWORK | msg,
>>> + .a5 = vm_id,
>>> + };
>>> + struct arm_smccc_1_2_regs resp;
>>> +
>>> + arm_smccc_1_2_smc(&arg, &resp);
>>> + if ( resp.a0 != FFA_MSG_SEND_DIRECT_RESP_32 || resp.a2 != exp_resp )
>>> + {
>>> + /*
>>> + * This is an invalid response, likely due to some error in the
>>> + * implementation of the ABI.
>>> + */
>>> + return FFA_RET_INVALID_PARAMETERS;
>>> + }
>>> + res = resp.a3;
>>> + } while ( res == FFA_RET_INTERRUPTED || res == FFA_RET_RETRY );
>>
>> This loop seems potentially unbounded to me. Can you add a comment
>> explaining why this is fine?
>
> In the FF-A 1.1 specification
> (https://developer.arm.com/documentation/den0077/e/?lang=en) Table
> 18.26 at page 330 it says that FFA_RET_INTERRUPTED and FFA_RET_RETRY
> should be handled in this way. When looking at this from the
> hypervisor's point of view it is troublesome since there isn't any
> guarantee that we're progressing.
>
> We should be able to rule out FFA_RET_INTERRUPTED since non-secure
> interrupts should be masked at this point. I'm not sure if
> FFA_RET_RETRY can be avoided entirely, but we should be able to put a
> limit on how many times we're prepared to retry.
The fact that interrupts are masked in Xen does not mean they will be in secure.
In fact what we should do when INTERRUPTED is received is something we have
to clear up but we should unmask interrupt to process them in Xen before retrying I think.
>
> How about setting a limit of max 10 retries and treating
> FFA_RET_INTERRUPTED as an error? Or is it better to not loop at all
> and treat all but FFA_RET_OK as errors? What do others think?
I would suggest to do a max retry for both cases and add a TODO in the code.
We will need to define a generic way to handle those cases but at this stage
INTERRUPTED should be considered TODO.
RETRY will probably stay with a limit here, in the case of a guest message both
of those possibilities could just be returned to the guest.
Do you agree ?
Cheers
Bertrand
>
> Thanks,
> Jens
>
>>
>>> +
>>> + return res;
>>> +}
>>> +
>>
>> Cheers,
>>
>> --
>> Julien Grall
Hi Bertrand,
On Fri, Apr 14, 2023 at 10:29 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
> Hi Jens,
>
> > On 14 Apr 2023, at 10:19, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > Hi,
> >
> > On Thu, Apr 13, 2023 at 3:24 PM Julien Grall <julien@xen.org> wrote:
> >>
> >> Hi,
> >>
> >> On 13/04/2023 08:14, Jens Wiklander wrote:
> >>> +static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
> >>> + uint8_t msg)
> >>> +{
> >>> + uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
> >>> + int32_t res;
> >>> +
> >>> + if ( msg == FFA_MSG_SEND_VM_CREATED )
> >>> + exp_resp |= FFA_MSG_RESP_VM_CREATED;
> >>> + else if ( msg == FFA_MSG_SEND_VM_DESTROYED )
> >>> + exp_resp |= FFA_MSG_RESP_VM_DESTROYED;
> >>> + else
> >>> + return FFA_RET_INVALID_PARAMETERS;
> >>> +
> >>> + do {
> >>> + const struct arm_smccc_1_2_regs arg = {
> >>> + .a0 = FFA_MSG_SEND_DIRECT_REQ_32,
> >>> + .a1 = sp_id,
> >>> + .a2 = FFA_MSG_FLAG_FRAMEWORK | msg,
> >>> + .a5 = vm_id,
> >>> + };
> >>> + struct arm_smccc_1_2_regs resp;
> >>> +
> >>> + arm_smccc_1_2_smc(&arg, &resp);
> >>> + if ( resp.a0 != FFA_MSG_SEND_DIRECT_RESP_32 || resp.a2 != exp_resp )
> >>> + {
> >>> + /*
> >>> + * This is an invalid response, likely due to some error in the
> >>> + * implementation of the ABI.
> >>> + */
> >>> + return FFA_RET_INVALID_PARAMETERS;
> >>> + }
> >>> + res = resp.a3;
> >>> + } while ( res == FFA_RET_INTERRUPTED || res == FFA_RET_RETRY );
> >>
> >> This loop seems potentially unbounded to me. Can you add a comment
> >> explaining why this is fine?
> >
> > In the FF-A 1.1 specification
> > (https://developer.arm.com/documentation/den0077/e/?lang=en) Table
> > 18.26 at page 330 it says that FFA_RET_INTERRUPTED and FFA_RET_RETRY
> > should be handled in this way. When looking at this from the
> > hypervisor's point of view it is troublesome since there isn't any
> > guarantee that we're progressing.
> >
> > We should be able to rule out FFA_RET_INTERRUPTED since non-secure
> > interrupts should be masked at this point. I'm not sure if
> > FFA_RET_RETRY can be avoided entirely, but we should be able to put a
> > limit on how many times we're prepared to retry.
>
> The fact that interrupts are masked in Xen does not mean they will be in secure.
> In fact what we should do when INTERRUPTED is received is something we have
> to clear up but we should unmask interrupt to process them in Xen before retrying I think.
>
> >
> > How about setting a limit of max 10 retries and treating
> > FFA_RET_INTERRUPTED as an error? Or is it better to not loop at all
> > and treat all but FFA_RET_OK as errors? What do others think?
>
> I would suggest to do a max retry for both cases and add a TODO in the code.
> We will need to define a generic way to handle those cases but at this stage
> INTERRUPTED should be considered TODO.
> RETRY will probably stay with a limit here, in the case of a guest message both
> of those possibilities could just be returned to the guest.
>
>
> Do you agree ?
Sounds good to me, I'll do that.
Thanks,
Jens
>
> Cheers
> Bertrand
>
> >
> > Thanks,
> > Jens
> >
> >>
> >>> +
> >>> + return res;
> >>> +}
> >>> +
> >>
> >> Cheers,
> >>
> >> --
> >> Julien Grall
>
>
© 2016 - 2026 Red Hat, Inc.