Messages in this thread | | | Date | Tue, 12 Sep 2023 17:06:34 +0100 | From | Sudeep Holla <> | 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@arm.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
| |