[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH v2 5/5] xen/arm: ffa: support notification
Hi Julien, On Mon, Apr 22, 2024 at 1:40 PM Julien Grall <julien@xxxxxxx> 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? Thanks, Jens
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |