[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 02/10] xen/arm: ffa: Rework feature discovery
Hi Jens, > On 22 Oct 2024, at 08:30, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote: > > Hi Bertrand, > > On Wed, Oct 16, 2024 at 10:32 AM Bertrand Marquis > <bertrand.marquis@xxxxxxx> wrote: >> >> Store the list of ABI we need in a list and go through the list instead >> of having a list of conditions inside the code. >> >> No functional change. >> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx> >> --- >> Changes in v2: >> - Store a string version of ABI needed from firmware and print the name >> of the ABI not supported instead of the id >> - Restore comment with TODO which should not have been removed at this >> stage >> - fix to unsigned int the index in the loop on the array >> - use abi instead of feature in the names of the functions and variables >> as we are not checking features but abis >> --- >> xen/arch/arm/tee/ffa.c | 57 +++++++++++++++++++++++++----------------- >> 1 file changed, 34 insertions(+), 23 deletions(-) >> >> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c >> index 1cc4023135d5..dde932422ecf 100644 >> --- a/xen/arch/arm/tee/ffa.c >> +++ b/xen/arch/arm/tee/ffa.c >> @@ -74,6 +74,31 @@ >> /* Negotiated FF-A version to use with the SPMC, 0 if not there or supported >> */ >> static uint32_t __ro_after_init ffa_fw_version; >> >> +struct ffa_fw_abi { >> + const uint32_t id; > > I prefer removing the const attribute for this struct member since it > doesn't add anything when the struct itself is const. Works for me. Will fix in v3. Cheers Bertrand > > Cheers, > Jens > >> + const char *name; >> +}; >> + >> +#define FW_ABI(abi) {abi,#abi} >> + >> +/* List of ABI we use from the firmware */ >> +static const struct ffa_fw_abi ffa_fw_abi_needed[] = { >> + FW_ABI(FFA_VERSION), >> + FW_ABI(FFA_FEATURES), >> + FW_ABI(FFA_NOTIFICATION_BITMAP_CREATE), >> + FW_ABI(FFA_NOTIFICATION_BITMAP_DESTROY), >> + FW_ABI(FFA_PARTITION_INFO_GET), >> + FW_ABI(FFA_NOTIFICATION_INFO_GET_64), >> + FW_ABI(FFA_NOTIFICATION_GET), >> + FW_ABI(FFA_RX_RELEASE), >> + FW_ABI(FFA_RXTX_MAP_64), >> + FW_ABI(FFA_RXTX_UNMAP), >> + FW_ABI(FFA_MEM_SHARE_32), >> + FW_ABI(FFA_MEM_SHARE_64), >> + FW_ABI(FFA_MEM_RECLAIM), >> + FW_ABI(FFA_MSG_SEND_DIRECT_REQ_32), >> + FW_ABI(FFA_MSG_SEND_DIRECT_REQ_64), >> +}; >> >> /* >> * Our rx/tx buffers shared with the SPMC. FFA_RXTX_PAGE_COUNT is the >> @@ -112,20 +137,9 @@ static bool ffa_get_version(uint32_t *vers) >> return true; >> } >> >> -static int32_t ffa_features(uint32_t id) >> -{ >> - return ffa_simple_call(FFA_FEATURES, id, 0, 0, 0); >> -} >> - >> -static bool check_mandatory_feature(uint32_t id) >> +static bool ffa_abi_supported(uint32_t id) >> { >> - int32_t ret = ffa_features(id); >> - >> - if ( ret ) >> - printk(XENLOG_ERR "ffa: mandatory feature id %#x missing: error >> %d\n", >> - id, ret); >> - >> - return !ret; >> + return !ffa_simple_call(FFA_FEATURES, id, 0, 0, 0); >> } >> >> static void handle_version(struct cpu_user_regs *regs) >> @@ -540,17 +554,14 @@ static bool ffa_probe(void) >> * TODO: Rework the code to allow domain to use a subset of the >> * features supported. >> */ >> - 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_MEM_SHARE_64) || >> - !check_mandatory_feature(FFA_RXTX_UNMAP) || >> - !check_mandatory_feature(FFA_MEM_SHARE_32) || >> - !check_mandatory_feature(FFA_MEM_RECLAIM) || >> - !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) ) >> + for ( unsigned int i = 0; i < ARRAY_SIZE(ffa_fw_abi_needed); i++ ) >> { >> - printk(XENLOG_ERR "ffa: Mandatory feature not supported by fw\n"); >> - goto err_no_fw; >> + if ( !ffa_abi_supported(ffa_fw_abi_needed[i].id) ) >> + { >> + printk(XENLOG_INFO "ARM FF-A Firmware does not support %s\n", >> + ffa_fw_abi_needed[i].name); >> + goto err_no_fw; >> + } >> } >> >> ffa_fw_version = vers; >> -- >> 2.47.0
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |