Re: [PATCH v3 08/16] irqchip/gic: Configure SGIs as standard interrupts

Marc Zyngier
2020-09-15T08:14:20Z

Hi Marek,

On 2020-09-15 07:48, Marek Szyprowski wrote:
> Hi Marc,
>
>>> Both Exynos 4210 and 4412 use non-zero cpu-offset in GIC node in
>>> device-tree: arch/arm/boot/dts/exynos{4210,4412}.dtsi, so I assume
>>> that
>>> the GIC registers are not banked.
>>
>> Annoyingly, it seems to work correctly in QEMU:

[...]

>> Do you happen to know whether the QEMU emulation is trustworthy?
>
> I didn't play much with Exynos emulation on QEMU. All I know is that
> this patch simply doesn't work on the real hw.

I don't doubt it. The question was more whether we could trust QEMU
to be reliable, in which case the issue would be around a kernel
configuration problem. Could you stash your kernel config somewhere?

> If there is anything to check or test, let me know. I will try to help
> as much as possible.

It would be interesting to see whether the CPUs are getting any IPI.
Can you try the following patch, and send the results back?

Thanks,

M.

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 00327fa74b01..5b01d53de9af 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -420,7 +420,7 @@ asmlinkage void secondary_start_kernel(void)
#ifndef CONFIG_MMU
setup_vectors_base();
#endif
- pr_debug("CPU%u: Booted secondary processor\n", cpu);
+ pr_err("CPU%u: Booted secondary processor\n", cpu);

preempt_disable();
trace_hardirqs_off();
@@ -621,6 +621,8 @@ static void do_handle_IPI(int ipinr)
{
unsigned int cpu = smp_processor_id();

+ pr_info("CPU%d IPI%d received\n", cpu, ipinr);
+
if ((unsigned)ipinr < NR_IPI)
trace_ipi_entry_rcuidle(ipi_types[ipinr]);

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d7321ccf730f..7723cad6e406 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -190,6 +190,8 @@ static inline bool cascading_gic_irq(struct irq_data
*d)
static void gic_poke_irq(struct irq_data *d, u32 offset)
{
u32 mask = 1 << (gic_irq(d) % 32);
+ if (gic_irq(d) < 16)
+ pr_info("CPU%d IPI%lu base = %lx\n", smp_processor_id(), d->hwirq,
(unsigned long)gic_dist_base(d));
writel_relaxed(mask, gic_dist_base(d) + offset + (gic_irq(d) / 32) *
4);
}

@@ -814,6 +816,7 @@ static void gic_ipi_send_mask(struct irq_data *d,
const struct cpumask *mask)
*/
dmb(ishst);

+ pr_info("CPU%d send IPI%lu base = %lx\n", smp_processor_id(),
d->hwirq, (unsigned long)gic_data_dist_base(&gic_data[0]));
/* this always happens on GIC0 */
writel_relaxed(map << 16 | d->hwirq, gic_data_dist_base(&gic_data[0])
+ GIC_DIST_SOFTINT);

--
Jazz is not dead. It just smells funny...