Re: [PATCH V11 00/10] irqchip: Add LoongArch-related irqchip drivers

From: Jianmin Lv
Date: Sat May 07 2022 - 03:06:28 EST

Next message: Hao Xu: "Re: [PATCH 3/5] io_uring: let fast poll support multishot"
Previous message: Greg KH: "Re: [PATCH v2] rfkill: uapi: fix RFKILL_IOCTL_MAX_SIZE ioctl request definition"
In reply to: Marc Zyngier: "Re: [PATCH V11 00/10] irqchip: Add LoongArch-related irqchip drivers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 2022/5/5 下午11:58, Marc Zyngier wrote:

On Sat, 30 Apr 2022 09:53:34 +0100,
Huacai Chen <chenhuacai@xxxxxxxxxxx> wrote:

LoongArch is a new RISC ISA, which is a bit like MIPS or RISC-V.
LoongArch includes a reduced 32-bit version (LA32R), a standard 32-bit
version (LA32S) and a 64-bit version (LA64). LoongArch use ACPI as its
boot protocol LoongArch-specific interrupt controllers (similar to APIC)
are already added in the next revision of ACPI Specification (current
revision is 6.4).

Currently, LoongArch based processors (e.g. Loongson-3A5000) can only
work together with LS7A chipsets. The irq chips in LoongArch computers
include CPUINTC (CPU Core Interrupt Controller), LIOINTC (Legacy I/O
Interrupt Controller), EIOINTC (Extended I/O Interrupt Controller),
HTVECINTC (Hyper-Transport Vector Interrupt Controller), PCH-PIC (Main
Interrupt Controller in LS7A chipset), PCH-LPC (LPC Interrupt Controller
in LS7A chipset) and PCH-MSI (MSI Interrupt Controller).

CPUINTC is a per-core controller (in CPU), LIOINTC/EIOINTC/HTVECINTC are
per-package controllers (in CPU), while PCH-PIC/PCH-LPC/PCH-MSI are all
controllers out of CPU (i.e., in chipsets). These controllers (in other
words, irqchips) are linked in a hierarchy, and there are two models of
hierarchy (legacy model and extended model).

Legacy IRQ model:

In this model, the IPI (Inter-Processor Interrupt) and CPU Local Timer
interrupt go to CPUINTC directly, CPU UARTS interrupts go to LIOINTC,
while all other devices interrupts go to PCH-PIC/PCH-LPC/PCH-MSI and
gathered by HTVECINTC, and then go to LIOINTC, and then CPUINTC.

+---------------------------------------------+
| |
| +-----+ +---------+ +-------+ |
| | IPI | --> | CPUINTC | <-- | Timer | |
| +-----+ +---------+ +-------+ |
| ^ |
| | |
| +---------+ +-------+ |
| | LIOINTC | <-- | UARTs | |
| +---------+ +-------+ |
| ^ |
| | |
| +-----------+ |
| | HTVECINTC | |
| +-----------+ |
| ^ ^ |
| | | |
| +---------+ +---------+ |
| | PCH-PIC | | PCH-MSI | |
| +---------+ +---------+ |
| ^ ^ ^ |
| | | | |
| +---------+ +---------+ +---------+ |
| | PCH-LPC | | Devices | | Devices | |
| +---------+ +---------+ +---------+ |
| ^ |
| | |
| +---------+ |
| | Devices | |
| +---------+ |
| |
| |
+---------------------------------------------+

Extended IRQ model:

In this model, the IPI (Inter-Processor Interrupt) and CPU Local Timer
interrupt go to CPUINTC directly, CPU UARTS interrupts go to LIOINTC,
while all other devices interrupts go to PCH-PIC/PCH-LPC/PCH-MSI and
gathered by EIOINTC, and then go to to CPUINTC directly.

+--------------------------------------------------------+
| |
| +-----+ +---------+ +-------+ |
| | IPI | --> | CPUINTC | <-- | Timer | |
| +-----+ +---------+ +-------+ |
| ^ ^ |
| | | |
| +---------+ +---------+ +-------+ |
| | EIOINTC | | LIOINTC | <-- | UARTs | |
| +---------+ +---------+ +-------+ |
| ^ ^ |
| | | |
| +---------+ +---------+ |
| | PCH-PIC | | PCH-MSI | |
| +---------+ +---------+ |
| ^ ^ ^ |
| | | | |
| +---------+ +---------+ +---------+ |
| | PCH-LPC | | Devices | | Devices | |
| +---------+ +---------+ +---------+ |
| ^ |
| | |
| +---------+ |
| | Devices | |
| +---------+ |
| |
| |
+--------------------------------------------------------+

This patchset adds some irqchip drivers for LoongArch, it is preparing
to add LoongArch support in mainline kernel, we can see a snapshot here:
https://github.com/loongson/linux/tree/loongarch-next

Cross-compile tool chain to build kernel:
https://github.com/loongson/build-tools/releases/latest/download/loongarch64-clfs-20211202-cross-tools.tar.xz

A CLFS-based Linux distro:
https://github.com/loongson/build-tools/releases/latest/download/loongarch64-clfs-system-2021-12-02.tar.bz2

Loongson and LoongArch documentations:
https://github.com/loongson/LoongArch-Documentation

LoongArch-specific interrupt controllers:
https://mantis.uefi.org/mantis/view.php?id=2203

Nothing to see here, unfortunately (you need a login to access this
information).

Sorry， the link of mantis needs login， LoongArch-specific interrupt controllers related content has been added into ACPI spec 6.5 draft, working group review of which has been done, and the board will start a 30-day legal review period, so spec 6.5 publication will be at some time in early Jun.

LoongArch use ACPI, but ACPI tables cannot describe the hierarchy of
irqchips, so we initilize the irqchip subsystem in this way (from arch
code):

cpu_domain = loongarch_cpu_irq_init();
liointc_domain = liointc_acpi_init(cpu_domain, acpi_liointc);
eiointc_domain = eiointc_acpi_init(cpu_domain, acpi_eiointc);
pch_pic_domain = pch_pic_acpi_init(eiointc_domain, acpi_pchpic);
pch_msi_domain = pch_msi_acpi_init(eiointc_domain, acpi_pchmsi);

I said no to this in the past, and I'm going to reiterate: this is
*not* acceptable. This obviously doesn't scale and isn't manageable in
the long run. Hardcoding the topology and the probing order in the
kernel code has repeatedly proved to be a disaster, and yet you refuse
to take any input from past experience. This is pretty worrying.

I completely agree with you about topology scaling, and we will change this according to LoongArch hardware feature as following:

- Delete fixed hierarchy code in arch file, and only call irqchip_init() to init irqchip controllers.
- Use IRQCHIP_ACPI_DECLARE to declare top-level core pic.
- For some controllers(lpc pic, lio pic, eio pic and ht pic) hardcodingly casecaded to parent one in LoongArch hardware, we'll initialize them (if they are found in MADT) in their parent directly.
- For some controllers(bio pic and msi pic) cascading to different parent for different CPU, we'll initialize them (if they are found in MADT) in their candidate parent in term of CPU hardware feature information(e.g. support eio pic )

By this way, the irqchip hierarchy topology will be scaled automatically.

Upstream irqchip init function return an irqdomain, and this domain
will be used by downstream irqchips as their parent domains. For more
infomation please refer:
https://lore.kernel.org/linux-arch/20210927064300.624279-11-chenhuacai@xxxxxxxxxxx/T/#u

Q: Why we don't declare irqchips in ACPI DSDT where hierarchy is possible?
A: This is answered in V8 of this series as below:

- There are several kinds of irq chips(e.g. pchpic、eiointc、cpuintc)
for LoongArch. SCI interrupt (Fixed hardware is implemented for
LoongArch in pch such as LS7A1000, and SCI interrupt is used for fixed
event handling.) is an irq input of pch irq chip which routes
interrupts to cpu as following irq chips path:

sci interrupt->|pchpic| ->|eiointc|->|cpuintc|

sci_interrupt will be transferred from gsi to irq through
acpi_gsi_to_irq in acpi_enable_subsystem called from acpi_bus_init
before acpi_scan_init where acpi device namespace is created, so we
should build pch irq domain and related upstream irq domains before
acpi_bus_init.

- PCI bus enumeration is executed from acpi_scan_init, and
pci_set_msi_domain will be called for setting msi_domain of enumerated
pci device. In pci_set_msi_domain, msi domain may be got through
pcibios_device_add, fdt, iort(used for arm64) or inheriting from host
bridge domain. And in each way, the msi domain needs to be found by
calling irq_find_matching_fwnode(fwnode, DOMAIN_BUS_PCI_MSI) to match
one from the registered msi domain before. So we build the msi domain
as x86 and arm64 before acpi_scan_init. The msi domain is hierarchic
as following:

msi interrupt->|msipic| ->|eiointc|->|cpuintc|

- Yes, a driver can be deferred probed when get -EPROBE_DEFER on
probing, but both sci interrupt transfer and pci bus enumeration are
common code (not private driver for LoongArch).

So, declaring pic devices in DSDT for seems not suitable, we can only
select the X86-like way which is a bit ugly.

ACPI *can* describe these topologies if you teach it. ARM managed to
do it with IORT, which is part of the ACPI specification. Given that
you are starting from scratch and that your bindings aren't even in
the official ACPI spec, there is zero reason not to do the right
thing.

Until then, I'm not interested in reviewing more of these patches.

Thanks,

M.

I understand your viewpoint, we'll change it and make it to be more reasonable, thanks very much for your suggestion again, and please checkout if what I said above is in line with your opinion.

Next message: Hao Xu: "Re: [PATCH 3/5] io_uring: let fast poll support multishot"
Previous message: Greg KH: "Re: [PATCH v2] rfkill: uapi: fix RFKILL_IOCTL_MAX_SIZE ioctl request definition"
In reply to: Marc Zyngier: "Re: [PATCH V11 00/10] irqchip: Add LoongArch-related irqchip drivers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]