[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230912160634.lvaq5s5frx6ju2hs@bogus>
Date: Tue, 12 Sep 2023 17:06:34 +0100
From: Sudeep Holla <sudeep.holla@....com>
To: Jens Wiklander <jens.wiklander@...aro.org>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Lucian Paul-Trifu <lucian.paul-trifu@....com>,
Marc Bonnici <marc.bonnici@....com>,
Coboy Chen <coboy.chen@...iatek.com>
Subject: Re: [PATCH RFT 08/12] firmware: arm_ffa: Initial support for
scheduler receiver interrupt
On Tue, Sep 12, 2023 at 03:41:04PM +0200, Jens Wiklander wrote:
> Hi Sudeep,
>
> On Thu, Aug 03, 2023 at 08:02:12PM +0100, Sudeep Holla wrote:
> > The Framework uses the schedule receiver interrupt to inform the
> > receiver’s scheduler that the receiver must be run to handle a pending
> > notification. A receiver’s scheduler can obtain the description of the
> > schedule receiver interrupt by invoking the FFA_FEATURES interface.
> >
> > The delivery of the physical schedule receiver interrupt from the secure
> > state to the non-secure state depends upon the state of the interrupt
> > controller as configured by the hypervisor.
> >
> > The schedule seceiver interrupt is assumed to be a PPI. The Arm GIC
> > specification defines 16 SGIs. It recommends that they are equally
> > divided between the non-secure and secure states. OS like Linux kernel
> > in the non-secure state typically do not have SGIs to spare. The usage
> > of SGIs in the secure state is however limited. It is more likely that
> > software in the Secure world does not use all the SGIs allocated to it.
> >
> > It is recommended that the secure world software donates an unused SGI
> > to the normal world for use as the schedule receiver interrupt. This
> > implies that secure world software must configure the SGI in the GIC
> > as a non-secure interrupt before presenting it to the normal world.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@....com>
> > ---
> > drivers/firmware/arm_ffa/driver.c | 183 ++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 175 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> > index 77ca9753e3f0..84c934a0ec14 100644
> > --- a/drivers/firmware/arm_ffa/driver.c
> > +++ b/drivers/firmware/arm_ffa/driver.c
> > @@ -22,15 +22,20 @@
> > #define DRIVER_NAME "ARM FF-A"
> > #define pr_fmt(fmt) DRIVER_NAME ": " fmt
> >
> > +#include <linux/acpi.h>
> > #include <linux/arm_ffa.h>
> > #include <linux/bitfield.h>
> > +#include <linux/cpuhotplug.h>
> > #include <linux/device.h>
> > +#include <linux/interrupt.h>
> > #include <linux/io.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/mm.h>
> > +#include <linux/of_irq.h>
> > #include <linux/scatterlist.h>
> > #include <linux/slab.h>
> > +#include <linux/smp.h>
> > #include <linux/uuid.h>
> >
> > #include "common.h"
> > @@ -76,6 +81,10 @@ static inline int ffa_to_linux_errno(int errno)
> > return -EINVAL;
> > }
> >
> > +struct ffa_pcpu_irq {
> > + struct ffa_drv_info *info;
> > +};
> > +
> > struct ffa_drv_info {
> > u32 version;
> > u16 vm_id;
> > @@ -85,6 +94,12 @@ struct ffa_drv_info {
> > void *tx_buffer;
> > bool mem_ops_native;
> > bool bitmap_created;
> > + unsigned int sched_recv_irq;
> > + unsigned int cpuhp_state;
> > + struct ffa_pcpu_irq __percpu *irq_pcpu;
> > + struct workqueue_struct *notif_pcpu_wq;
> > + struct work_struct irq_work;
> > + bool info_get_64b;
> > };
> >
> > static struct ffa_drv_info *drv_info;
> > @@ -910,9 +925,147 @@ static void ffa_setup_partitions(void)
> > kfree(pbuf);
> > }
> >
> > +/* FFA FEATURE IDs */
> > +#define FFA_FEAT_NOTIFICATION_PENDING_INT (1)
> > +#define FFA_FEAT_SCHEDULE_RECEIVER_INT (2)
> > +#define FFA_FEAT_MANAGED_EXIT_INT (3)
> > +
> > +static irqreturn_t irq_handler(int irq, void *irq_data)
> > +{
> > + struct ffa_pcpu_irq *pcpu = irq_data;
> > + struct ffa_drv_info *info = pcpu->info;
> > +
> > + queue_work(info->notif_pcpu_wq, &info->irq_work);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void ffa_sched_recv_irq_work_fn(struct work_struct *work)
> > +{
> > + struct ffa_drv_info *info = container_of(work, struct ffa_drv_info,
> > + irq_work);
> > +
> > + ffa_notification_info_get(info->info_get_64b);
> > +}
> > +
> > +static int ffa_sched_recv_irq_map(void)
> > +{
> > + int ret, irq, sr_intid;
> > +
> > + ret = ffa_features(FFA_FEAT_SCHEDULE_RECEIVER_INT, 0, &sr_intid, NULL);
> > + if (ret < 0) {
> > + if (ret != -EOPNOTSUPP)
> > + pr_err("Failed to retrieve scheduler Rx interrupt\n");
> > + return ret;
> > + }
> > +
> > + if (acpi_disabled) {
> > + struct of_phandle_args oirq = {};
> > + struct device_node *gic;
> > +
> > + gic = of_find_compatible_node(NULL, NULL, "arm,gic-v3");
>
> "arm,gic-v3" might be good enough for now, but eventually we may need more scalable probing.
>
> > + if (!gic)
> > + return -ENXIO;
> > +
> > + oirq.np = gic;
> > + oirq.args_count = 1;
> > + oirq.args[0] = sr_intid;
> > + irq = irq_create_of_mapping(&oirq);
>
> You mention (or try to ;-)) in the commit message that SGIs only are
> assumed for now. A comment here stating that fact wouldn't hurt.
>
Fair enough, I will add.
> I'm trying to test this on QEMU, but I can't seem to recieve the SGI in
> the kernel. I've donated the previously secure SGI-8 at boot (simple
> patch in TF-A) before the kernel starts. I've verified with GDB that
> QEMU has accepted the write to ICC_ASGI1R_EL1 while in secure world, but
> it seems it never triggers when switching back to normal world. I'm
> starting to suspect that irq_create_of_mapping() might not update SGI-8
> properly. Perhaps something more is needed when donating SGI-8 on the
> kernel side.
>
> Did you have this working in your tests?
Nope, I haven't been able to get a working end-to-end stack and hence RFT.
However I had tried to test just the SGI part but did have some hacks which
I have lost. I wanted to check if those changes were just for my testing or
is indeed needed in the kernel. I will try to dig more details.
--
Regards,
Sudeep
Powered by blists - more mailing lists