Hi Jon,
+Linus, who is facing a similar issue.
On 2020-09-16 15:16, Jon Hunter wrote:
Hi Marc,
On 14/09/2020 14:06, Marek Szyprowski wrote:
Hi Marc,
On 01.09.2020 16:43, Marc Zyngier wrote:
Change the way we deal with GIC SGIs by turning them into proper
IRQs, and calling into the arch code to register the interrupt range
instead of a callback.
Reviewed-by: Valentin Schneider <valentin.schneider@xxxxxxx>
Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
This patch landed in linux next-20200914 as commit ac063232d4b0
("irqchip/gic: Configure SGIs as standard interrupts"). Sadly it
breaks
booting of all Samsung Exynos 4210/4412 based boards (dual/quad ARM
Cortex A9 based). Here are the last lines from the bootlog:
I am observing the same thing on several Tegra boards (both arm and
arm64). Bisect is pointing to this commit. Reverting this alone does
not
appear to be enough to fix the issue.
Right, I am just massively by the GICv3 spec, and failed to remember
that ye olde GIC exposes the source CPU in AIR *and* wants it back,
while
newer GICs deal with that transparently.
Can you try the patch below and let me know?
M.
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 98743afdaea6..56492bf8b6f9 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -121,9 +121,10 @@ static struct gic_chip_data
gic_data[CONFIG_ARM_GIC_MAX_NR] __read_mostly;
static struct gic_kvm_info gic_v2_kvm_info;
+static DEFINE_PER_CPU(u32, sgi_intid);
+
#ifdef CONFIG_GIC_NON_BANKED
static DEFINE_STATIC_KEY_FALSE(frankengic_key);
-static DEFINE_PER_CPU(u32, sgi_intid);
static void enable_frankengic(void)
{
@@ -135,16 +136,6 @@ static inline bool is_frankengic(void)
return static_branch_unlikely(&frankengic_key);
}
-static inline void set_sgi_intid(u32 intid)
-{
- this_cpu_write(sgi_intid, intid);
-}
-
-static inline u32 get_sgi_intid(void)
-{
- return this_cpu_read(sgi_intid);
-}
-
static inline void __iomem *__get_base(union gic_base *base)
{
if (is_frankengic())
@@ -160,8 +151,6 @@ static inline void __iomem *__get_base(union
gic_base *base)
#define gic_data_cpu_base(d) ((d)->cpu_base.common_base)
#define enable_frankengic() do { } while(0)
#define is_frankengic() false
-#define set_sgi_intid(i) do { } while(0)
-#define get_sgi_intid() 0
#endif
static inline void __iomem *gic_dist_base(struct irq_data *d)
@@ -236,8 +225,8 @@ static void gic_eoi_irq(struct irq_data *d)
{
u32 hwirq = gic_irq(d);
- if (is_frankengic() && hwirq < 16)
- hwirq = get_sgi_intid();
+ if (hwirq < 16)
+ hwirq = this_cpu_read(sgi_intid);
writel_relaxed(hwirq, gic_cpu_base(d) + GIC_CPU_EOI);
}
@@ -365,14 +354,13 @@ static void __exception_irq_entry
gic_handle_irq(struct pt_regs *regs)
smp_rmb();
/*
- * Samsung's funky GIC encodes the source CPU in
- * GICC_IAR, leading to the deactivation to fail if
- * not written back as is to GICC_EOI. Stash the
- * INTID away for gic_eoi_irq() to write back.
- * This only works because we don't nest SGIs...
+ * The GIC encodes the source CPU in GICC_IAR,
+ * leading to the deactivation to fail if not
+ * written back as is to GICC_EOI. Stash the INTID
+ * away for gic_eoi_irq() to write back. This only
+ * works because we don't nest SGIs...
*/
- if (is_frankengic())
- set_sgi_intid(irqstat);
+ this_cpu_write(sgi_intid, intid);
}
handle_domain_irq(gic->domain, irqnr, regs);
--
Jazz is not dead. It just smells funny...
