Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp1746782pxb; Mon, 8 Mar 2021 05:32:42 -0800 (PST) X-Google-Smtp-Source: ABdhPJxIigU76lVI4D5cOXPqCg0CUYau7uZiRdb3WRGE0vT2vuvK3b07dXugFBtnNr2ta4Aka1Mm X-Received: by 2002:aa7:c907:: with SMTP id b7mr22380739edt.37.1615210362409; Mon, 08 Mar 2021 05:32:42 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1615210362; cv=none; d=google.com; s=arc-20160816; b=BMflH7gHKWHw8wZWtcegr4u5iZPN5p7DG/eDr9K7np9mVJwWGhu2n872dSlp5O5FN1 Rw8miuGqZ2gIzTgXn5dc1xFxCwbI1dYHitz1YvMup938dKBMx7BS4J3seYjX/FwtbUnv HxVcBwN5dmsYSoTZ+tb9PVGQ33V7zWnp2tfpCnl+1DXKEhfgnj5TlJWd2WZYhPOaxBQs 0caJpYXWXoFjkgBPW3ndqPS13Np3Zrnwd20K+tsYnJvHbu0gyqV3jbQT+KV/uhAxFcOE WmwtXqQw9sZTISH3CAjBYTOwbWJBJqoKQJDD1sVw03ejTDsKbbbVvJpPFXfRY6Yv9YF5 X25w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:user-agent:references:in-reply-to :subject:cc:to:from:message-id:date; bh=lm/bSiDNFrAqIeD3Qm6imjmR2mtiWRxcptxIOluMs8g=; b=N6joLplnN82e5tAKbJ95kbxTnNSJ/s4lGxWRIgB53pVhdux5wrHPA9+1CfyStuH7Fd 32EmR+akbP68noIwkibg0foCrPey8uv+fAVNxtbkntb5hx0rGcNRW4t87rsl6/arM402 LlRS01yLfG1SXZJlNMcCzRQOkyjFuB80SqR42TroIM840bH/T9TOgipu6jJJmrB27CP7 gmDesHNdP5LMxUUEWg9gzGVbwWoqwI+6DpO+Z95QqHybBrE8pHJQhrGandxdQmIa+kMD yh2SjDpZiNqPmZm8HBhaojIrY6nlEXFkYm/E3hUwybgpJoWmTdnwH2XSRnFg+mAemm+U 2N+A== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id h18si7095774ejd.89.2021.03.08.05.32.18; Mon, 08 Mar 2021 05:32:42 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230075AbhCHNbZ (ORCPT + 99 others); Mon, 8 Mar 2021 08:31:25 -0500 Received: from mail.kernel.org ([198.145.29.99]:57666 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229591AbhCHNbE (ORCPT ); Mon, 8 Mar 2021 08:31:04 -0500 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B7206651C2; Mon, 8 Mar 2021 13:31:03 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lJFyP-000L8E-P9; Mon, 08 Mar 2021 13:31:01 +0000 Date: Mon, 08 Mar 2021 13:31:00 +0000 Message-ID: <874khlzsa3.wl-maz@kernel.org> From: Marc Zyngier To: Hector Martin Cc: linux-arm-kernel@lists.infradead.org, Rob Herring , Arnd Bergmann , Olof Johansson , Krzysztof Kozlowski , Mark Kettenis , Tony Lindgren , Mohamed Mediouni , Stan Skowronek , Alexander Graf , Will Deacon , Linus Walleij , Mark Rutland , Andy Shevchenko , Greg Kroah-Hartman , Jonathan Corbet , Catalin Marinas , Christoph Hellwig , "David S. Miller" , devicetree@vger.kernel.org, linux-serial@vger.kernel.org, linux-doc@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFT PATCH v3 16/27] irqchip/apple-aic: Add support for the Apple Interrupt Controller In-Reply-To: <20210304213902.83903-17-marcan@marcan.st> References: <20210304213902.83903-1-marcan@marcan.st> <20210304213902.83903-17-marcan@marcan.st> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: marcan@marcan.st, linux-arm-kernel@lists.infradead.org, robh@kernel.org, arnd@kernel.org, olof@lixom.net, krzk@kernel.org, mark.kettenis@xs4all.nl, tony@atomide.com, mohamed.mediouni@caramail.com, stan@corellium.com, graf@amazon.com, will@kernel.org, linus.walleij@linaro.org, mark.rutland@arm.com, andy.shevchenko@gmail.com, gregkh@linuxfoundation.org, corbet@lwn.net, catalin.marinas@arm.com, hch@infradead.org, davem@davemloft.net, devicetree@vger.kernel.org, linux-serial@vger.kernel.org, linux-doc@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 04 Mar 2021 21:38:51 +0000, Hector Martin wrote: > > This is the root interrupt controller used on Apple ARM SoCs such as the > M1. This irqchip driver performs multiple functions: > > * Handles both IRQs and FIQs > > * Drives the AIC peripheral itself (which handles IRQs) > > * Dispatches FIQs to downstream hard-wired clients (currently the ARM > timer). > > * Implements a virtual IPI multiplexer to funnel multiple Linux IPIs > into a single hardware IPI > [...] > Signed-off-by: Hector Martin > +static void __exception_irq_entry aic_handle_irq(struct pt_regs *regs) > +{ > + struct aic_irq_chip *ic = aic_irqc; > + u32 event, type, irq; > + > + do { > + /* > + * We cannot use a relaxed read here, as DMA needs to be > + * ordered with respect to the IRQ firing. > + */ > + event = readl(ic->base + AIC_EVENT); > + type = FIELD_GET(AIC_EVENT_TYPE, event); > + irq = FIELD_GET(AIC_EVENT_NUM, event); > + > + if (type == AIC_EVENT_TYPE_HW) > + handle_domain_irq(aic_irqc->hw_domain, irq, regs); > + else if (type == AIC_EVENT_TYPE_IPI && irq == 1) > + aic_handle_ipi(regs); > + else if (event != 0) > + pr_err("Unknown IRQ event %d, %d\n", type, irq); > + } while (event); > + > + /* > + * vGIC maintenance interrupts end up here too, so we need to check > + * for them separately. Just report and disable vGIC for now, until > + * we implement this properly. > + */ > + if ((read_sysreg_s(SYS_ICH_HCR_EL2) & ICH_HCR_EN) && > + read_sysreg_s(SYS_ICH_MISR_EL2) != 0) { > + pr_err("vGIC IRQ fired, disabling.\n"); Please add a _ratelimited here. Whilst debugging KVM on this machine, I ended up with this firing at such a rate that it was impossible to do anything useful. Ratelimiting it allowed me to pinpoint the problem. [...] > +/* > + * FIQ irqchip > + */ > + > +static void aic_fiq_mask(struct irq_data *d) > +{ > + /* Only the guest timers have real mask bits, unfortunately. */ > + switch (d->hwirq) { > + case AIC_TMR_GUEST_PHYS: > + sysreg_clear_set_s(SYS_APL_VM_TMR_FIQ_ENA_EL1, VM_TMR_FIQ_ENABLE_P, 0); > + break; > + case AIC_TMR_GUEST_VIRT: > + sysreg_clear_set_s(SYS_APL_VM_TMR_FIQ_ENA_EL1, VM_TMR_FIQ_ENABLE_V, 0); > + break; > + } > +} > + > +static void aic_fiq_unmask(struct irq_data *d) > +{ > + switch (d->hwirq) { > + case AIC_TMR_GUEST_PHYS: > + sysreg_clear_set_s(SYS_APL_VM_TMR_FIQ_ENA_EL1, 0, VM_TMR_FIQ_ENABLE_P); > + break; > + case AIC_TMR_GUEST_VIRT: > + sysreg_clear_set_s(SYS_APL_VM_TMR_FIQ_ENA_EL1, 0, VM_TMR_FIQ_ENABLE_V); > + break; > + } > +} > + > +static void aic_fiq_eoi(struct irq_data *d) > +{ > + /* We mask to ack (where we can), so we need to unmask at EOI. */ > + if (!irqd_irq_disabled(d) && !irqd_irq_masked(d)) Ah, be careful here: irqd_irq_masked() doesn't do what you think it does for per-CPU interrupts. It's been on my list to fix for the rVIC implementation, but I never got around to doing it, and all decent ICs hide this from SW by having a HW-managed mask, similar to what is on the IRQ side. I can see two possibilities: - you can track the masked state directly and use that instead of these predicates - you can just drop the masking altogether as this is only useful to a hosted hypervisor (KVM), which will have to do its own masking behind the scenes anyway > + aic_fiq_unmask(d); > +} > + The rest looks good to me. Thanks, M. -- Without deviation from the norm, progress is not possible.