Re: [XEN PATCH v3 5/5] xen/arm: ffa: support notification

[Julien Grall <julien@xxxxxxx>]

Hi Jens,

On 26/04/2024 09:47, Jens Wiklander wrote:
> +static void notif_irq_enable(void *info)
> +{
> +    struct notif_irq_info *irq_info = info;
> +
> +    irq_info->ret = setup_irq(irq_info->irq, 0, irq_info->action);
In v2, you were using request_irq(). But now you seem to be open-coding 
it. Can you explain why?
> +    if ( irq_info->ret )
> +        printk(XENLOG_ERR "ffa: request_irq irq %u failed: error %d\n",
> +               irq_info->irq, irq_info->ret);
> +}
> +
> +void ffa_notif_init(void)
> +{
> +    const struct arm_smccc_1_2_regs arg = {
> +        .a0 = FFA_FEATURES,
> +        .a1 = FFA_FEATURE_SCHEDULE_RECV_INTR,
> +    };
> +    struct notif_irq_info irq_info = { };
> +    struct arm_smccc_1_2_regs resp;
> +    unsigned int cpu;
> +
> +    arm_smccc_1_2_smc(&arg, &resp);
> +    if ( resp.a0 != FFA_SUCCESS_32 )
> +        return;
> +
> +    irq_info.irq = resp.a2;
> +    if ( irq_info.irq < GIC_SGI_STATIC_MAX || irq_info.irq >= NR_GIC_SGI )
> +    {
> +        printk(XENLOG_ERR "ffa: notification initialization failed: conflicting SGI 
> %u\n",
> +               irq_info.irq);
> +        return;
> +    }
> +
> +    /*
> +     * SGIs are per-CPU so we must enable the IRQ on each CPU. We use an
> +     * IPI to call notif_irq_enable() on each CPU including the current
> +     * CPU. The struct irqaction is preallocated since we can't allocate
> +     * memory while in interrupt context.
> +     */
> +    for_each_online_cpu(cpu)
Even though we currently don't support CPU hotplug, you want to add a 
CPU Notifier to also register the IRQ when a CPU is onlined 
ffa_notif_init().
For an example, see time.c. We may also want to consider to enable TEE 
in presmp_initcalls() so we don't need to have a for_each_online_cpu().
Cheers,

--
Julien Grall