Messages in this thread | | | Date | Fri, 12 Jun 2020 11:39:00 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH 06/11] irqchip/gic-v3: Configure SGIs as standard interrupts |
| |
Hi Valentin,
On Thu, 21 May 2020 15:04:54 +0100 Valentin Schneider <valentin.schneider@arm.com> wrote:
> On 19/05/20 17:17, Marc Zyngier wrote: > > Change the way we deal with GICv3 SGIs by turning them into proper > > IRQs, and calling into the arch code to register the interrupt range > > instead of a callback. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > drivers/irqchip/irq-gic-v3.c | 91 +++++++++++++++++++++--------------- > > 1 file changed, 53 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > > index 23d7c87da407..d57289057b75 100644 > > --- a/drivers/irqchip/irq-gic-v3.c > > +++ b/drivers/irqchip/irq-gic-v3.c > > @@ -1163,10 +1142,36 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) > > > > static void gic_smp_init(void) > > { > > - set_smp_cross_call(gic_raise_softirq); > > + struct irq_fwspec sgi_fwspec = { > > + .fwnode = gic_data.fwnode, > > + }; > > + int base_sgi; > > + > > cpuhp_setup_state_nocalls(CPUHP_AP_IRQ_GIC_STARTING, > > "irqchip/arm/gicv3:starting", > > gic_starting_cpu, NULL); > > + > > + if (is_of_node(gic_data.fwnode)) { > > + /* DT */ > > + sgi_fwspec.param_count = 3; > > + sgi_fwspec.param[0] = GIC_IRQ_TYPE_SGI; > > + sgi_fwspec.param[1] = 0; > > + sgi_fwspec.param[2] = IRQ_TYPE_EDGE_RISING; > > + } else { > > + /* ACPI */ > > + sgi_fwspec.param_count = 2; > > + sgi_fwspec.param[0] = 0; > > + sgi_fwspec.param[1] = IRQ_TYPE_EDGE_RISING; > > + } > > + > > + /* Register all 8 non-secure SGIs */ > > + base_sgi = __irq_domain_alloc_irqs(gic_data.domain, -1, 8, > > + NUMA_NO_NODE, &sgi_fwspec, > > + false, NULL); > > So IIUC using irq_reserve_ipi() would require us to have a separate IPI > domain, so instead here we can use a fwspec + the 'regular' GIC domain.
Indeed. Using an IPI domain wouldn't bring much. But the major point against the current state of the IPI domain is that it sucks a bit for our use case. We want interrupts to be contiguous in the Linux IRQ space, and the IPI allocator prevents this.
But maybe I should just bite the bullet and hack that as well.
> One thing I see is that by not going through irq_reserve_ipi(), we don't set > data->common->ipi_offset. I think this is all kzalloc'd, and we want an > offset of 0 so it all works out, but this feels somewhat fragile.
So far, nothing is using this field on the limited piece of code we use. But I agree, not the nicest behaviour.
> > + if (WARN_ON(base_sgi <= 0)) > > + return; > > + > > + set_smp_ipi_range(base_sgi, 8); > > } > > > > static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val, > > @@ -1289,6 +1296,13 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq, > > > > switch (__get_intid_range(hw)) { > > case SGI_RANGE: > > + irq_set_percpu_devid(irq); > > + irq_domain_set_info(d, irq, hw, chip, d->host_data, > > + handle_percpu_devid_fasteoi_ipi, > > + NULL, NULL); > > + irq_set_status_flags(irq, IRQ_NOAUTOEN); > > FWIW IRQ_NOAUTOEN is already set by irq_set_percpu_devid_flags(), so that's > not required. I know we do that for (E)PPIs, I think I already have a small > patch stashed somewhere regarding that.
Already merged! ;-)
Thanks,
M. -- Jazz is not dead. It just smells funny...
| |