Hi Jens, > On 23 Apr 2024, at 17:26, Jens Wiklander <[email protected]> wrote: > > Hi Julien, > > On Mon, Apr 22, 2024 at 1:40 PM 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? > > Good catch, no this wasn't what I expected. I'll need to change this. > >> >> [...] >> >>> 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? > > Yes, I'll move it. > >> >>> + * >>> + * 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? > > There's one at the top of arch/arm/tee/ffa.c, > https://developer.arm.com/documentation/den0077/e > Do you want the link close to the defines when I've moved them to > public/arch-arm.h? > Or is it perhaps better to give a link to "Arm Base System > Architecture v1.0C", https://developer.arm.com/documentation/den0094/ > instead?
I would say we need the link to Arm Base System Architecture in arch-arm. Regards Bertrand > > Thanks, > Jens
