Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 24 Feb 2022 18:26:41 +0000
From:   Mark Rutland <mark.rutland@arm.com>
To:     Hector Martin <marcan@marcan.st>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Marc Zyngier <maz@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Sven Peter <sven@svenpeter.dev>,
        Alyssa Rosenzweig <alyssa@rosenzweig.io>,
        Mark Kettenis <mark.kettenis@xs4all.nl>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org
Subject: Re: [PATCH v2 0/7] irqchip/apple-aic: Add support for AICv2
Message-ID: <YhfN4YtR+Nq5vmVr@lakrids>
References: <20220224130741.63924-1-marcan@marcan.st>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220224130741.63924-1-marcan@marcan.st>
Precedence: bulk

On Thu, Feb 24, 2022 at 10:07:34PM +0900, Hector Martin wrote:
> Hi folks,

Hi Hector,

> In the t6000/t6001 (M1 Pro / Max) SoCs, Apple introduced a new version
> of their interrupt controller. This is a significant departure from
> AICv1 and seems designed to better scale to larger chips. This series
> adds support for it to the existing AIC driver.
> 
> Gone are CPU affinities; instead there seems to be some kind of
> "automagic" dispatch to willing CPU cores, and cores can also opt-out
> via an IMP-DEF sysreg (!). Right now the bootloader just sets up all
> cores to accept IRQs, and we ignore all this and let the magic
> algorithm pick a CPU to accept the IRQ.

Maybe that's ok for the set of peripherals attached, but in general that
violates existing expectations regarding affinity, and I fear there'll
be some subtle brokenness resulting from this automatic target
selection.

For example, in the perf events subsystem there are PMU drivers (even
those for "uncore" or "system" devices which are shared by many/all
CPUs) which rely on a combination of interrupt affinity and local IRQ
masking (without any other locking) to provide exclusion between a PMU's
IRQ handler and any other management operations for that PMU (which are
all handled from the same CPU).

> In the future, we might start making use of these finer-grained
> capabilities for e.g. better real-time guarantees (CPUs running RT
> threads might opt out of IRQs).

What mechanism does the HW have for affinity selection? The wording
above makes it sound like each CPU has to opt-out rather than having a
central affinity selection. Is there a mechanism to select a single
target?

Thanks,
Mark.

> Legacy IPI support is also gone, so this implements Fast IPI support.
> Fast IPIs are implemented entirely in the CPU core complexes, using
> FIQs and IMP-DEF sysregs. This is also supported on t8103/M1, so we
> enable it there too, but we keep the legacy AIC IPI codepath in case
> it is useful for backporting to older chips.
> 
> This also adds support for multi-die AIC2 controllers. While no
> multi-die products exist yet, the AIC2 in t600x is built to support
> up to 2 dies, and it's pretty clear how it works, so let's implement
> it. If we're lucky, when multi-die products roll around, this will
> let us support them with only DT changes. In order to support the
> extra die dimension, this introduces a 4-argument IRQ phandle form
> (3-argument is always supported and just implies die 0).
> 
> All register offsets are computed based on capability register values,
> which should allow forward-compatibility with future AIC2 variants...
> except for one. For some inexplicable reason, the number of actually
> implemented die register sets is nowhere to be found (t600x has 2,
> but claims 1 die in use and 8 dies max, neither of which is what we
> need), and this is necessary to compute the event register offset,
> which is page-aligned after the die register sets. We have no choice
> but to stick this offset in the device tree... which is the same thing
> Apple do in their ADT.
> 
> Changes since v1:
> - Split off the DT binding
> - Changed fast-ipi codepath selection to use a static key for performance
> - Added fix for PCI driver to support the new 4-cell IRQ form
> - Minor style / review feedback fixes
> 
> Hector Martin (7):
>   PCI: apple: Change MSI handling to handle 4-cell AIC fwspec form
>   dt-bindings: interrupt-controller: apple,aic2: New binding for AICv2
>   irqchip/apple-aic: Add Fast IPI support
>   irqchip/apple-aic: Switch to irq_domain_create_tree and sparse hwirqs
>   irqchip/apple-aic: Dynamically compute register offsets
>   irqchip/apple-aic: Support multiple dies
>   irqchip/apple-aic: Add support for AICv2
> 
>  .../interrupt-controller/apple,aic2.yaml      |  99 ++++
>  MAINTAINERS                                   |   2 +-
>  drivers/irqchip/irq-apple-aic.c               | 432 +++++++++++++++---
>  drivers/pci/controller/pcie-apple.c           |   2 +-
>  4 files changed, 458 insertions(+), 77 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml
> 
> -- 
> 2.33.0
>