From: Jens Wiklander <jens.wiklander@linaro.org>
To: Bertrand Marquis <Bertrand.Marquis@arm.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
"patches@linaro.org" <patches@linaro.org>,
Stefano Stabellini <sstabellini@kernel.org>,
Julien Grall <julien@xen.org>,
Michal Orzel <michal.orzel@amd.com>,
Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [XEN PATCH v3 5/5] xen/arm: ffa: support notification
Date: Fri, 26 Apr 2024 14:32:02 +0200 [thread overview]
Message-ID: <CAHUa44EC_H7Q_A2M63dZnEGGNanyjGM6m2MuBg4_6BNEO-XDKA@mail.gmail.com> (raw)
In-Reply-To: <3A576E95-454B-4C66-A164-A2871DB4E2B2@arm.com>
Hi Bertrand,
On Fri, Apr 26, 2024 at 2:19 PM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
> Hi Jens,
>
> > On 26 Apr 2024, at 14:11, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >
> > Hi Bertrand,
> >
> > On Fri, Apr 26, 2024 at 11:20 AM Bertrand Marquis
> > <Bertrand.Marquis@arm.com> wrote:
> >>
> >> Hi Jens,
> >>
> >>> On 26 Apr 2024, at 10:47, Jens Wiklander <jens.wiklander@linaro.org> wrote:
> >>>
[...]
> >>> +struct notif_irq_info {
> >>> + unsigned int irq;
> >>> + int ret;
> >>> + struct irqaction *action;
> >>> +};
> >>> +
> >>> +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);
> >>> + 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)
> >>> + {
> >>> + irq_info.action = xmalloc(struct irqaction);
> >>
> >> You allocate one action per cpu but you have only one action pointer in your structure
> >> which means you will overload the previously allocated one and lose track.
> >>
> >> You should have a table of actions in your structure instead unless one action is
> >> enough and can be reused on all cpus and in this case you should move out of
> >> your loop the allocation part.
> >
> > That shouldn't be needed because this is done in sequence only one CPU
> > at a time.
>
> Sorry i do not understand here.
> You have a loop over each online cpu and on each loop you are assigning
> irq_info.action with a newly allocated struct irqaction so you are in practice
> overloading on cpu 2 the action that was allocated for cpu 1.
>
> What do you mean by sequence here ?
>
My understanding is that for_each_online_cpu(cpu) loops over each cpu,
one at a time. The call
on_selected_cpus(cpumask_of(cpu), notif_irq_enable, &irq_info, 1);
returns after notif_irq_enable() has returned on the CPU in question
thanks to the "1" (wait) parameter. So once it has returned &irq_info
isn't used by the other CPU any longer and we can assign a new value
to irq_info.action.
Thanks,
Jens
next prev parent reply other threads:[~2024-04-26 12:32 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-26 8:47 [XEN PATCH v3 0/5] FF-A notifications Jens Wiklander
2024-04-26 8:47 ` [XEN PATCH v3 1/5] xen/arm: ffa: refactor ffa_handle_call() Jens Wiklander
2024-04-26 8:47 ` [XEN PATCH v3 2/5] xen/arm: ffa: use ACCESS_ONCE() Jens Wiklander
2024-04-26 8:47 ` [XEN PATCH v3 3/5] xen/arm: ffa: simplify ffa_handle_mem_share() Jens Wiklander
2024-04-26 8:47 ` [XEN PATCH v3 4/5] xen/arm: allow dynamically assigned SGI handlers Jens Wiklander
2024-04-26 8:47 ` [XEN PATCH v3 5/5] xen/arm: ffa: support notification Jens Wiklander
2024-04-26 9:20 ` Bertrand Marquis
2024-04-26 12:11 ` Jens Wiklander
2024-04-26 12:19 ` Bertrand Marquis
2024-04-26 12:32 ` Jens Wiklander [this message]
2024-04-26 12:41 ` Bertrand Marquis
2024-04-26 13:02 ` Jens Wiklander
2024-04-26 15:12 ` Bertrand Marquis
2024-04-26 18:31 ` Julien Grall
2024-04-26 17:58 ` Julien Grall
2024-04-29 9:55 ` Jens Wiklander
2024-04-26 19:07 ` Julien Grall
2024-04-29 7:20 ` Bertrand Marquis
2024-04-29 9:49 ` Jens Wiklander
2024-04-29 20:55 ` Julien Grall
2024-04-29 8:43 ` Jens Wiklander
2024-04-26 9:23 ` [XEN PATCH v3 0/5] FF-A notifications Bertrand Marquis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHUa44EC_H7Q_A2M63dZnEGGNanyjGM6m2MuBg4_6BNEO-XDKA@mail.gmail.com \
--to=jens.wiklander@linaro.org \
--cc=Bertrand.Marquis@arm.com \
--cc=Volodymyr_Babchuk@epam.com \
--cc=julien@xen.org \
--cc=michal.orzel@amd.com \
--cc=patches@linaro.org \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.