| Message ID | 20210402090542.131194-1-marcan@marcan.st |
|---|---|
| Headers | show |
| Series | Apple M1 SoC platform bring-up | expand |
On Fri, 02 Apr 2021 18:05:29 +0900, Hector Martin wrote: > Not all platforms provide the same set of timers/interrupts, and Linux > only needs one (plus kvm/guest ones); some platforms are working around > this by using dummy fake interrupts. Implementing interrupt-names allows > the devicetree to specify an arbitrary set of available interrupts, so > the timer code can pick the right one. > > This also adds the hyp-virt timer/interrupt, which was previously not > expressed in the fixed 4-interrupt form. > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Acked-by: Marc Zyngier <maz@kernel.org> > Reviewed-by: Tony Lindgren <tony@atomide.com> > Signed-off-by: Hector Martin <marcan@marcan.st> > --- > .../bindings/timer/arm,arch_timer.yaml | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org>
On Fri, Apr 02, 2021 at 06:05:42PM +0900, Hector Martin wrote: > This currently supports: > > * SMP (via spin-tables) > * AIC IRQs > * Serial (with earlycon) > * Framebuffer > > A number of properties are dynamic, and based on system firmware > decisions that vary from version to version. These are expected > to be filled in by the loader. > > Signed-off-by: Hector Martin <marcan@marcan.st> > --- > MAINTAINERS | 1 + > arch/arm64/boot/dts/Makefile | 1 + > arch/arm64/boot/dts/apple/Makefile | 2 + > arch/arm64/boot/dts/apple/t8103-j274.dts | 45 ++++++++ > arch/arm64/boot/dts/apple/t8103.dtsi | 135 +++++++++++++++++++++++ > 5 files changed, 184 insertions(+) > create mode 100644 arch/arm64/boot/dts/apple/Makefile > create mode 100644 arch/arm64/boot/dts/apple/t8103-j274.dts > create mode 100644 arch/arm64/boot/dts/apple/t8103.dtsi Reviewed-by: Rob Herring <robh@kernel.org>
Hi Hector, On Fri, 02 Apr 2021 10:05:39 +0100, Hector Martin <marcan@marcan.st> 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 <marcan@marcan.st> > --- > MAINTAINERS | 2 + > drivers/irqchip/Kconfig | 8 + > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-apple-aic.c | 837 ++++++++++++++++++++++++++++++++ > include/linux/cpuhotplug.h | 1 + > 5 files changed, 849 insertions(+) > create mode 100644 drivers/irqchip/irq-apple-aic.c [...] > +static int aic_irq_domain_translate(struct irq_domain *id, > + struct irq_fwspec *fwspec, > + unsigned long *hwirq, > + unsigned int *type) > +{ > + struct aic_irq_chip *ic = id->host_data; > + > + if (fwspec->param_count != 3 || !is_of_node(fwspec->fwnode)) > + return -EINVAL; > + > + switch (fwspec->param[0]) { > + case AIC_IRQ: > + if (fwspec->param[1] >= ic->nr_hw) > + return -EINVAL; > + *hwirq = fwspec->param[1]; > + break; > + case AIC_FIQ: > + if (fwspec->param[1] >= AIC_NR_FIQ) > + return -EINVAL; > + *hwirq = ic->nr_hw + fwspec->param[1]; > + > + /* > + * In EL1 the non-redirected registers are the guest's, > + * not EL2's, so remap the hwirqs to match. > + */ > + if (!is_kernel_in_hyp_mode()) { > + switch (fwspec->param[1]) { > + case AIC_TMR_GUEST_PHYS: > + *hwirq = ic->nr_hw + AIC_TMR_HV_PHYS; > + break; > + case AIC_TMR_GUEST_VIRT: > + *hwirq = ic->nr_hw + AIC_TMR_HV_VIRT; > + break; > + case AIC_TMR_HV_PHYS: > + case AIC_TMR_HV_VIRT: > + return -ENOENT; > + default: > + break; > + } > + } Urgh, this is nasty. You are internally remapping the hwirq from one timer to another in order to avoid accessing the enable register which happens to be an EL2 only register? The way we normally deal with this kind of things is by providing a different set of irq_chip callbacks. In this particular case, this would leave mask/unmask as empty stubs. Or you could move the FIQ handling to use handle_simple_irq(), because there isn't any callback that is actually applicable. It isn't a big deal for now, but that's something we should consider addressing in the future. With that in mind: Reviewed-by: Marc Zyngier <maz@kernel.org> Thanks, M. -- Without deviation from the norm, progress is not possible.
On 07/04/2021 03.16, Marc Zyngier wrote: > Hi Hector, > > On Fri, 02 Apr 2021 10:05:39 +0100, > Hector Martin <marcan@marcan.st> wrote: >> + /* >> + * In EL1 the non-redirected registers are the guest's, >> + * not EL2's, so remap the hwirqs to match. >> + */ >> + if (!is_kernel_in_hyp_mode()) { >> + switch (fwspec->param[1]) { >> + case AIC_TMR_GUEST_PHYS: >> + *hwirq = ic->nr_hw + AIC_TMR_HV_PHYS; >> + break; >> + case AIC_TMR_GUEST_VIRT: >> + *hwirq = ic->nr_hw + AIC_TMR_HV_VIRT; >> + break; >> + case AIC_TMR_HV_PHYS: >> + case AIC_TMR_HV_VIRT: >> + return -ENOENT; >> + default: >> + break; >> + } >> + } > > Urgh, this is nasty. You are internally remapping the hwirq from one > timer to another in order to avoid accessing the enable register > which happens to be an EL2 only register? The remapping is to make the IRQs route properly at all. There are EL2 and EL0 timers, and on GIC each timer goes to its own IRQ. But here there are no real IRQs, everything's a FIQ. However, thanks to VHE, the EL2 timer shows up as the EL0 timer, and the EL0 timer is accessed via EL02 registers, when in EL2. So in EL2/VHE mode, "HV" means EL0 and "guest" means EL02, while in EL1, there is no HV and "guest" means EL0. And since we figure out which IRQ fired by reading timer registers, this is what matters. So I map the guest IRQs to the HV hwirqs in EL1 mode, which makes this all work out. Then the timer code goes and ends up undoing all this logic again, so we map to separate fake "IRQs" only to end up right back at using the same timer registers anuway :-) Really, the ugliness here is that the constant meaning is overloaded. In fwspec context they mean what they say on the tin, while in hwirq context "HV" means EL0 and "guest" means EL02 (other FIQs would be passed through unchanged). Perhaps some additional defines might help clarify this? Say, at the top of this file (not in the binding), /* * Pass-through mapping from real timers to the correct registers to * access them in EL2/VHE mode. When running in EL1, this gets * overridden to access the guest timer using EL0 registers. */ #define AIC_TMR_EL0_PHYS AIC_TMR_HV_PHYS #define AIC_TMR_EL0_VIRT AIC_TMR_HV_VIRT #define AIC_TMR_EL02_PHYS AIC_TMR_GUEST_PHYS #define AIC_TMR_EL02_VIRT AIC_TMR_GUEST_VIRT Then the irqchip/FIQ dispatch side can use the EL* constants, the default pass-through mapping is appropriate for VHE/EL2 mode, and translation can adjust it for EL1 mode. -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub
On Fri, Apr 2, 2021 at 12:07 PM Hector Martin <marcan@marcan.st> wrote: > > Now that we have ioremap_np(), we can make pci_remap_cfgspace() default > to it, falling back to ioremap() on platforms where it is not available. > > Remove the arm64 implementation, since that is now redundant. Future > cleanups should be able to do the same for other arches, and eventually > make the generic pci_remap_cfgspace() unconditional. ... > + void __iomem *ret = ioremap_np(offset, size); > + > + if (!ret) > + ret = ioremap(offset, size); > + > + return ret; Usually negative conditions are worse for cognitive functions of human beings. (On top of that some patterns are applied) I would rewrite above as void __iomem *ret; ret = ioremap_np(offset, size); if (ret) return ret; return ioremap(offset, size); -- With Best Regards, Andy Shevchenko
On Wed, Apr 07, 2021 at 04:27:42PM +0300, Andy Shevchenko wrote: > On Fri, Apr 2, 2021 at 12:07 PM Hector Martin <marcan@marcan.st> wrote: > > > > Now that we have ioremap_np(), we can make pci_remap_cfgspace() default > > to it, falling back to ioremap() on platforms where it is not available. > > > > Remove the arm64 implementation, since that is now redundant. Future > > cleanups should be able to do the same for other arches, and eventually > > make the generic pci_remap_cfgspace() unconditional. > > ... > > > + void __iomem *ret = ioremap_np(offset, size); > > + > > + if (!ret) > > + ret = ioremap(offset, size); > > + > > + return ret; > > Usually negative conditions are worse for cognitive functions of human beings. > (On top of that some patterns are applied) > > I would rewrite above as > > void __iomem *ret; > > ret = ioremap_np(offset, size); > if (ret) > return ret; > > return ioremap(offset, size); Looks like it might be one of those rare occasions where the GCC ternary if extension thingy comes in handy: return ioremap_np(offset, size) ?: ioremap(offset, size); but however it's done, the logic looks good to me and thanks Hector for updating this: Acked-by: Will Deacon <will@kernel.org> Will
On Fri, Apr 02, 2021 at 06:05:39PM +0900, 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 <marcan@marcan.st> > --- > MAINTAINERS | 2 + > drivers/irqchip/Kconfig | 8 + > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-apple-aic.c | 837 ++++++++++++++++++++++++++++++++ > include/linux/cpuhotplug.h | 1 + > 5 files changed, 849 insertions(+) > create mode 100644 drivers/irqchip/irq-apple-aic.c Couple of stale comment nits: > +static void aic_ipi_unmask(struct irq_data *d) > +{ > + struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d); > + u32 irq_bit = BIT(irqd_to_hwirq(d)); > + > + atomic_or(irq_bit, this_cpu_ptr(&aic_vipi_enable)); > + > + /* > + * The atomic_or() above must complete before the atomic_read_acquire() below to avoid > + * racing aic_ipi_send_mask(). > + */ (the atomic_read_acquire() is now an atomic_read()) > + smp_mb__after_atomic(); > + > + /* > + * If a pending vIPI was unmasked, raise a HW IPI to ourselves. > + * No barriers needed here since this is a self-IPI. > + */ > + if (atomic_read(this_cpu_ptr(&aic_vipi_flag)) & irq_bit) > + aic_ic_write(ic, AIC_IPI_SEND, AIC_IPI_SEND_CPU(smp_processor_id())); > +} > + > +static void aic_ipi_send_mask(struct irq_data *d, const struct cpumask *mask) > +{ > + struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d); > + u32 irq_bit = BIT(irqd_to_hwirq(d)); > + u32 send = 0; > + int cpu; > + unsigned long pending; > + > + for_each_cpu(cpu, mask) { > + /* > + * This sequence is the mirror of the one in aic_ipi_unmask(); > + * see the comment there. Additionally, release semantics > + * ensure that the vIPI flag set is ordered after any shared > + * memory accesses that precede it. This therefore also pairs > + * with the atomic_fetch_andnot in aic_handle_ipi(). > + */ > + pending = atomic_fetch_or_release(irq_bit, per_cpu_ptr(&aic_vipi_flag, cpu)); > + > + /* > + * The atomic_fetch_or_release() above must complete before the > + * atomic_read_acquire() below to avoid racing aic_ipi_unmask(). > + */ (same here) > + smp_mb__after_atomic(); > + > + if (!(pending & irq_bit) && > + (atomic_read(per_cpu_ptr(&aic_vipi_enable, cpu)) & irq_bit)) > + send |= AIC_IPI_SEND_CPU(cpu); > + } But with that: Acked-by: Will Deacon <will@kernel.org> Will
On 08/04/2021 06.03, Will Deacon wrote: >> I would rewrite above as >> >> void __iomem *ret; >> >> ret = ioremap_np(offset, size); >> if (ret) >> return ret; >> >> return ioremap(offset, size); > > Looks like it might be one of those rare occasions where the GCC ternary if > extension thingy comes in handy: > > return ioremap_np(offset, size) ?: ioremap(offset, size); Today I learned that this one is kosher in kernel code. Handy! Let's go with that. > Acked-by: Will Deacon <will@kernel.org> Thanks! -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub
On 08/04/2021 06.09, Will Deacon wrote: > Couple of stale comment nits: [...] > But with that: > > Acked-by: Will Deacon <will@kernel.org> Fixed those for the PR, thanks! -- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub
On Thu, Apr 8, 2021 at 2:01 PM Hector Martin <marcan@marcan.st> wrote: > On 08/04/2021 06.03, Will Deacon wrote: > >> I would rewrite above as > >> > >> void __iomem *ret; > >> > >> ret = ioremap_np(offset, size); > >> if (ret) > >> return ret; > >> > >> return ioremap(offset, size); > > > > Looks like it might be one of those rare occasions where the GCC ternary if > > extension thingy comes in handy: > > > > return ioremap_np(offset, size) ?: ioremap(offset, size); > > Today I learned that this one is kosher in kernel code. Handy! Let's go > with that. It depends on the maintainer. Greg, for example, doesn't like this. I have no strong opinion (I use both variants on case-by-case basis), though I think in headers better to spell out all conditionals clearly. -- With Best Regards, Andy Shevchenko
This series brings up initial support for the Apple M1 SoC, used in the 2020 Mac Mini, MacBook Pro, and MacBook Air models. The following features are supported in this initial port: - UART (samsung-style) with earlycon support - Interrupts, including affinity and IPIs (Apple Interrupt Controller) - SMP (through standard spin-table support) - simplefb-based framebuffer - Devicetree for the Mac Mini (should work for the others too at this stage) See below for an overview of changes since v3. == Merge notes == This patchset has several dependencies: * Build dep on the FIQ support series in [1]. * Runtime dep on the Samsung TTY changes in tty-next (modulo DT validation). * Runtime dep on the nVHE changes being reviewed in [2]. A tree containing this patchset on top of the required dependencies is available at [3][4], for those who want to test it. This series is expected to be merged by Arnd via the SoC tree. Maintainers, please ack if you are happy with the patches. We will coordinate with Arnd and Mark on the FIQ series to make sure all that goes smoothly. [1] https://lore.kernel.org/linux-arm-kernel/20210315115629.57191-1-mark.rutland@arm.com/T/ [2] https://lore.kernel.org/linux-arm-kernel/20210330173947.999859-1-maz@kernel.org/T/ [3] git://github.com/AsahiLinux/linux.git upstream-bringup-v4 [4] https://github.com/AsahiLinux/linux/tree/upstream-bringup-v4 == Testing notes == This has been tested on an Apple M1 Mac Mini booting to a framebuffer and serial console, with SMP and KASLR, with an arm64 defconfig (+ CONFIG_FB_SIMPLE for the fb). In addition, the AIC driver now supports running in EL1, tested in UP mode only. == Patch overview == - 01-02 Core platform DT bindings - 03-04 CPU DT bindings and MIDR defines - 05-06 Add interrupt-names support to the ARM timer driver This is used to cleanly express the lack of a secure timer; platforms in the past have used various hacks like dummy IRQs here. - 07-12 ioremap_np() (nGnRnE) support The fabric in these SoCs only supports nGnRnE accesses for standard MMIO, except for PCI ranges which use nGnRE. Linux currently defaults to the latter on ARM64, so this adds a new ioremap type and DT properties to automatically select it for drivers using OF and devm abstractions, under buses specified in DT. - 13-15 AIC (Apple Interrupt Controller) driver and support defines This also embeds FIQ handling for this platform. - 16 Introduce CONFIG_ARCH_APPLE & add it to defconfig - 17 simple-framebuffer bindings for Apple (trivial) - 18 Add the initial M1 Mac Mini (j274) devicetree == About the hardware == These machines officially support booting unsigned/user-provided XNU-like kernels, with a very different boot protocol and devicetree format. We are developing an initial bootloader, m1n1 [1], to take care of as many hardware peculiarities as possible and present a standard Linux arm64 boot protocol and device tree. In the future, I expect that production setups will add U-Boot and perhaps GRUB into the boot chain, to make the boot process similar to other ARM64 platforms. The machines expose their debug UART over USB Type C, triggered with vendor-specific USB-PD commands. Currently, the easiest way to get a serial console on these machines is to use a second M1 box and a simple USB C cable [2]. You can also build a DIY interface using an Arduino, a FUSB302 chip or board, and a 1.2V UART-TTL adapter [3]. In the coming weeks we will be designing an open hardware project to provide serial/debug connectivity to these machines (and, hopefully, also support other UART-over-Type C setups from other vendors). Please contact me privately if you are interested in getting an early prototype version of one of these devices. We also have WIP/not merged yet support for loading kernels and interacting via dwc3 usb-gadget, which works with a standard C-C or C-A cable and any Linux host. A quickstart guide to booting Linux kernels on these machines is available at [4], and we are documenting the hardware at [5]. [1] https://github.com/AsahiLinux/m1n1/ [2] https://github.com/AsahiLinux/macvdmtool/ [3] https://github.com/AsahiLinux/vdmtool/ [4] https://github.com/AsahiLinux/docs/wiki/Developer-Quickstart [5] https://github.com/AsahiLinux/docs/wiki == Project Blurb == Asahi Linux is an open community project dedicated to developing and maintaining mainline support for Apple Silicon on Linux. Feel free to drop by #asahi and #asahi-dev on freenode to chat with us, or check our website for more information on the project: https://asahilinux.org/ == Changes since v3 == * Serial patches have already been merged into tty-next and are no longer included * nVHE fixup patches are being reviewed separately and no longer part of this series * Updated arm,arch-timer bindings to be more restrictive, renamed phys-secure to sec-phys * Reordered ioremap() variants list in device-io.rst * Removed sysreg_apple.h (defines are in drivers now) * Many cleanups, bug fixes, and reworks to AIC, including some bug fixes from Marc's KVM series, kernel as EL1 support, and more. * Simplified non-posted MMIO DT handling to only apply to direct children of a bus, and only use "nonposted-mmio". Removed quirk optimization. * Implemented default pci_remap_cfgspace() in terms of ioremap_np() and removed arch-specific pci_remap_cfgspace override for arm64 (arm32 can come later) * Replaced license in AIC bindings header with GPL-2.0+ OR MIT * Other minor typo/style fixes Arnd Bergmann (1): docs: driver-api: device-io: Document I/O access functions Hector Martin (17): dt-bindings: vendor-prefixes: Add apple prefix dt-bindings: arm: apple: Add bindings for Apple ARM platforms dt-bindings: arm: cpus: Add apple,firestorm & icestorm compatibles arm64: cputype: Add CPU implementor & types for the Apple M1 cores dt-bindings: timer: arm,arch_timer: Add interrupt-names support arm64: arch_timer: Implement support for interrupt-names asm-generic/io.h: Add a non-posted variant of ioremap() docs: driver-api: device-io: Document ioremap() variants & access funcs arm64: Implement ioremap_np() to map MMIO as nGnRnE asm-generic/io.h: implement pci_remap_cfgspace using ioremap_np of/address: Add infrastructure to declare MMIO as non-posted arm64: Move ICH_ sysreg bits from arm-gic-v3.h to sysreg.h dt-bindings: interrupt-controller: Add DT bindings for apple-aic irqchip/apple-aic: Add support for the Apple Interrupt Controller arm64: Kconfig: Introduce CONFIG_ARCH_APPLE dt-bindings: display: Add apple,simple-framebuffer arm64: apple: Add initial Apple Mac mini (M1, 2020) devicetree .../devicetree/bindings/arm/apple.yaml | 64 ++ .../devicetree/bindings/arm/cpus.yaml | 2 + .../bindings/display/simple-framebuffer.yaml | 5 + .../interrupt-controller/apple,aic.yaml | 88 ++ .../bindings/timer/arm,arch_timer.yaml | 19 + .../devicetree/bindings/vendor-prefixes.yaml | 2 + Documentation/driver-api/device-io.rst | 356 ++++++++ .../driver-api/driver-model/devres.rst | 1 + MAINTAINERS | 14 + arch/arm64/Kconfig.platforms | 7 + arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/apple/Makefile | 2 + arch/arm64/boot/dts/apple/t8103-j274.dts | 45 + arch/arm64/boot/dts/apple/t8103.dtsi | 135 +++ arch/arm64/configs/defconfig | 1 + arch/arm64/include/asm/cputype.h | 6 + arch/arm64/include/asm/io.h | 11 +- arch/arm64/include/asm/sysreg.h | 60 ++ arch/sparc/include/asm/io_64.h | 4 + drivers/clocksource/arm_arch_timer.c | 24 +- drivers/irqchip/Kconfig | 8 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-apple-aic.c | 837 ++++++++++++++++++ drivers/of/address.c | 43 +- include/asm-generic/io.h | 22 +- include/asm-generic/iomap.h | 9 + include/clocksource/arm_arch_timer.h | 1 + .../interrupt-controller/apple-aic.h | 15 + include/linux/cpuhotplug.h | 1 + include/linux/io.h | 23 +- include/linux/ioport.h | 1 + include/linux/irqchip/arm-gic-v3.h | 56 -- include/linux/of_address.h | 1 + lib/devres.c | 22 + 34 files changed, 1807 insertions(+), 80 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/apple.yaml create mode 100644 Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml create mode 100644 arch/arm64/boot/dts/apple/Makefile create mode 100644 arch/arm64/boot/dts/apple/t8103-j274.dts create mode 100644 arch/arm64/boot/dts/apple/t8103.dtsi create mode 100644 drivers/irqchip/irq-apple-aic.c create mode 100644 include/dt-bindings/interrupt-controller/apple-aic.h -- 2.30.0