Hi Julien, > On 22 Apr 2024, at 13:40, Julien Grall <[email protected]> wrote: > > Hi Jens, > > This is not a full review of the code. I will let Bertrand doing it. > > On 22/04/2024 08:37, Jens Wiklander wrote: >> +void ffa_notif_init(void) >> +{ >> + const struct arm_smccc_1_2_regs arg = { >> + .a0 = FFA_FEATURES, >> + .a1 = FFA_FEATURE_SCHEDULE_RECV_INTR, >> + }; >> + struct arm_smccc_1_2_regs resp; >> + unsigned int irq; >> + int ret; >> + >> + arm_smccc_1_2_smc(&arg, &resp); >> + if ( resp.a0 != FFA_SUCCESS_32 ) >> + return; >> + >> + irq = resp.a2; >> + if ( irq >= NR_GIC_SGI ) >> + irq_set_type(irq, IRQ_TYPE_EDGE_RISING); >> + ret = request_irq(irq, 0, notif_irq_handler, "FF-A notif", NULL); > > If I am not mistaken, ffa_notif_init() is only called once on the boot CPU. > However, request_irq() needs to be called on every CPU so the callback is > registered every where and the interrupt enabled. > > I know the name of the function is rather confusing. So can you confirm this > is what you expected? > > [...] > >> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h >> index 98236cbf14a3..ef8ffd4526bd 100644 >> --- a/xen/arch/arm/tee/ffa_private.h >> +++ b/xen/arch/arm/tee/ffa_private.h >> @@ -25,6 +25,7 @@ >> #define FFA_RET_DENIED -6 >> #define FFA_RET_RETRY -7 >> #define FFA_RET_ABORTED -8 >> +#define FFA_RET_NO_DATA -9 >> /* FFA_VERSION helpers */ >> #define FFA_VERSION_MAJOR_SHIFT 16U >> @@ -97,6 +98,18 @@ >> */ >> #define FFA_MAX_SHM_COUNT 32 >> +/* >> + * TODO How to manage the available SGIs? SGI 8-15 seem to be entirely >> + * unused, but that may change. > > Are the value below intended for the guests? If so, can they be moved in > public/arch-arm.h along with the others guest interrupts?
The values are to be used by the guest but they will discover them through the FFA_FEATURES ABI so I do not think those should belong the public headers. Cheers Bertrand > >> + * >> + * SGI is the preferred delivery mechanism. SGIs 8-15 are normally not used >> + * by a guest as they in a non-virtualized system typically are assigned to >> + * the secure world. Here we're free to use SGI 8-15 since they are virtual >> + * and have nothing to do with the secure world. > > Do you have a pointer to the specification? > > [...] > > Cheers, > > -- > Julien Grall
