* [PATCH v10 0/2] Loongarch-avec support
@ 2024-08-15 11:26 Tianyang Zhang
2024-08-15 11:26 ` [PATCH v10 1/2] irqchip/loongson-pch-msi: Switch to MSI parent domains Tianyang Zhang
2024-08-15 11:26 ` [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support Tianyang Zhang
0 siblings, 2 replies; 9+ messages in thread
From: Tianyang Zhang @ 2024-08-15 11:26 UTC (permalink / raw)
To: corbet, alexs, chenhuacai, kernel, tglx, jiaxun.yang, gaoliang,
wangliupu, lvjianmin, zhangtianyang, yijun, mhocko, akpm,
dianders, maobibo, xry111, zhaotianrui, nathan, yangtiezhu,
zhoubinbin
Cc: loongarch, linux-doc, linux-kernel
This series of patches introduces support for advanced extended
interrupt controllers (AVECINTC), and this hardware feature will
be supported on 3C6000 for the first time
Changes log:
V0->V1:
1.Modified some formats and declarations
2.Removed kmalloc/kfree when adding affinity related data to pending_list,
and used moving tag to replace the original behavior
3.Adjusted the process that enables AVEC interrupts, now it is at the end of all processes
4.Removed CPUHP related callbacks, now irq_matrix_online/irq_matrix_offline is completed in start_secondary/loongson_cpu_disable
5.Adjusted compatibility issues for CONFIG_ACPI
6.About question:
> irr = csr_read64(LOONGARCH_CSR_IRR0 + vector / 64);
> should be good enough, no?
csr_read64 was built-in as __csrrd_d, it doesn't seem to support variables as parameters
>>>>
drivers/irqchip/irq-loongarch-avec.c: In function ‘complete_irq_moving’:
./arch/loongarch/include/asm/loongarch.h:164:25: error: invalid argument to built-in function
164 | #define csr_read64(reg) __csrrd_d(reg)
| ^~~~~~~~~
drivers/irqchip/irq-loongarch-avec.c:170:23: note: in expansion of macro ‘csr_read64’
170 | irr = csr_read64(LOONGARCH_CSR_IRR_BASE + vector / VECTORS_PER_REG);
| ^~~~~~~~~~
>>>>
So we have temporarily retained the previous implementation.
V1->V2:
Fixed up coding style. Made on/offline functions void
Added compatibility when CONFIG_SMP is turned off
V2->V3:
Squash two patches into one
V3->V4:
Update NR_IRQS
Update Register's name
Fixed up coding style
V4->V5:
Retain feature CPUCFG1_MSGINT
Fixed up coding style
Delete the test code introduced by V4, and now msi msg address still uses the 32-bit address
V5->V6:
Fix definition of NR_IRQS
Define arch_probe_nr_irqs()
Handle all avecintc interrupts in one dispatch
Use cpuhotplug callbacks instead of direct call to avec_online_cpu()/avec_offline_cpu()
Rename {SMP,ACTION}_CLEAR_VECT to {SMP,ACTION}_CLEAR_VECTOR
Use avecintc_ prefix instead of loongarch_avec_ to keep consistancy
V6->V7:
Fixed compatibility issue with cpuhp_setup_state_nocalls when CONFIG_SMP is turned off
Rename avecintc_online/offline_cpu as avecintc_cpu_online/offline
Use pch_msi_handle[0] as default value of get_pch_msi_handle
Rework commit-message
V7->V8:
Fixed up coding style
Support per-device-MSI domain
Replaced spin_lock ops with guard/scope_guard
Always execute irq_metrix_offline while the CPU is offline
V8->V9:
Fixed up coding style and potential bugs
V9->V10:
Add a cover of series patch
Huacai Chen (1):
irqchip/loongson-pch-msi: Switch to MSI parent domains
Tianyang Zhang (1):
irqchip/loongarch-avec: Add AVEC irqchip support
.../arch/loongarch/irq-chip-model.rst | 32 ++
.../zh_CN/arch/loongarch/irq-chip-model.rst | 32 ++
arch/loongarch/Kconfig | 1 +
arch/loongarch/include/asm/cpu-features.h | 1 +
arch/loongarch/include/asm/cpu.h | 2 +
arch/loongarch/include/asm/hardirq.h | 3 +-
arch/loongarch/include/asm/hw_irq.h | 2 +
arch/loongarch/include/asm/irq.h | 25 +-
arch/loongarch/include/asm/loongarch.h | 18 +-
arch/loongarch/include/asm/smp.h | 2 +
arch/loongarch/kernel/cpu-probe.c | 3 +-
arch/loongarch/kernel/irq.c | 15 +-
arch/loongarch/kernel/paravirt.c | 5 +
arch/loongarch/kernel/smp.c | 6 +
drivers/irqchip/Kconfig | 1 +
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-loongarch-avec.c | 426 ++++++++++++++++++
drivers/irqchip/irq-loongarch-cpu.c | 5 +-
drivers/irqchip/irq-loongson-eiointc.c | 7 +-
drivers/irqchip/irq-loongson-pch-msi.c | 82 ++--
include/linux/cpuhotplug.h | 3 +-
21 files changed, 615 insertions(+), 58 deletions(-)
create mode 100644 drivers/irqchip/irq-loongarch-avec.c
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v10 1/2] irqchip/loongson-pch-msi: Switch to MSI parent domains
2024-08-15 11:26 [PATCH v10 0/2] Loongarch-avec support Tianyang Zhang
@ 2024-08-15 11:26 ` Tianyang Zhang
2024-08-15 11:26 ` [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support Tianyang Zhang
1 sibling, 0 replies; 9+ messages in thread
From: Tianyang Zhang @ 2024-08-15 11:26 UTC (permalink / raw)
To: corbet, alexs, chenhuacai, kernel, tglx, jiaxun.yang, gaoliang,
wangliupu, lvjianmin, zhangtianyang, yijun, mhocko, akpm,
dianders, maobibo, xry111, zhaotianrui, nathan, yangtiezhu,
zhoubinbin
Cc: loongarch, linux-doc, linux-kernel, Huacai Chen
From: Huacai Chen <chenhuacai@loongson.cn>
Now remove the global PCI/MSI irqdomain implementation and provide the
required MSI parent functionality by filling in msi_parent_ops, so the
PCI/MSI code can detect the new parent and setup per-device MSI domains.
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
---
drivers/irqchip/Kconfig | 1 +
drivers/irqchip/irq-loongson-pch-msi.c | 58 ++++++++++----------------
2 files changed, 24 insertions(+), 35 deletions(-)
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index d078bdc48c38..341cd9ca5a05 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -685,6 +685,7 @@ config LOONGSON_PCH_MSI
depends on PCI
default MACH_LOONGSON64
select IRQ_DOMAIN_HIERARCHY
+ select IRQ_MSI_LIB
select PCI_MSI
help
Support for the Loongson PCH MSI Controller.
diff --git a/drivers/irqchip/irq-loongson-pch-msi.c b/drivers/irqchip/irq-loongson-pch-msi.c
index dd4d699170f4..2242f63c66fc 100644
--- a/drivers/irqchip/irq-loongson-pch-msi.c
+++ b/drivers/irqchip/irq-loongson-pch-msi.c
@@ -15,6 +15,8 @@
#include <linux/pci.h>
#include <linux/slab.h>
+#include "irq-msi-lib.h"
+
static int nr_pics;
struct pch_msi_data {
@@ -27,26 +29,6 @@ struct pch_msi_data {
static struct fwnode_handle *pch_msi_handle[MAX_IO_PICS];
-static void pch_msi_mask_msi_irq(struct irq_data *d)
-{
- pci_msi_mask_irq(d);
- irq_chip_mask_parent(d);
-}
-
-static void pch_msi_unmask_msi_irq(struct irq_data *d)
-{
- irq_chip_unmask_parent(d);
- pci_msi_unmask_irq(d);
-}
-
-static struct irq_chip pch_msi_irq_chip = {
- .name = "PCH PCI MSI",
- .irq_mask = pch_msi_mask_msi_irq,
- .irq_unmask = pch_msi_unmask_msi_irq,
- .irq_ack = irq_chip_ack_parent,
- .irq_set_affinity = irq_chip_set_affinity_parent,
-};
-
static int pch_msi_allocate_hwirq(struct pch_msi_data *priv, int num_req)
{
int first;
@@ -85,12 +67,6 @@ static void pch_msi_compose_msi_msg(struct irq_data *data,
msg->data = data->hwirq;
}
-static struct msi_domain_info pch_msi_domain_info = {
- .flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
- MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX,
- .chip = &pch_msi_irq_chip,
-};
-
static struct irq_chip middle_irq_chip = {
.name = "PCH MSI",
.irq_mask = irq_chip_mask_parent,
@@ -155,13 +131,31 @@ static void pch_msi_middle_domain_free(struct irq_domain *domain,
static const struct irq_domain_ops pch_msi_middle_domain_ops = {
.alloc = pch_msi_middle_domain_alloc,
.free = pch_msi_middle_domain_free,
+ .select = msi_lib_irq_domain_select,
+};
+
+#define PCH_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
+ MSI_FLAG_USE_DEF_CHIP_OPS | \
+ MSI_FLAG_PCI_MSI_MASK_PARENT)
+
+#define PCH_MSI_FLAGS_SUPPORTED (MSI_GENERIC_FLAGS_MASK | \
+ MSI_FLAG_PCI_MSIX | \
+ MSI_FLAG_MULTI_PCI_MSI)
+
+static struct msi_parent_ops pch_msi_parent_ops = {
+ .required_flags = PCH_MSI_FLAGS_REQUIRED,
+ .supported_flags = PCH_MSI_FLAGS_SUPPORTED,
+ .bus_select_mask = MATCH_PCI_MSI,
+ .bus_select_token = DOMAIN_BUS_NEXUS,
+ .prefix = "PCH-",
+ .init_dev_msi_info = msi_lib_init_dev_msi_info,
};
static int pch_msi_init_domains(struct pch_msi_data *priv,
struct irq_domain *parent,
struct fwnode_handle *domain_handle)
{
- struct irq_domain *middle_domain, *msi_domain;
+ struct irq_domain *middle_domain;
middle_domain = irq_domain_create_hierarchy(parent, 0, priv->num_irqs,
domain_handle,
@@ -174,14 +168,8 @@ static int pch_msi_init_domains(struct pch_msi_data *priv,
irq_domain_update_bus_token(middle_domain, DOMAIN_BUS_NEXUS);
- msi_domain = pci_msi_create_irq_domain(domain_handle,
- &pch_msi_domain_info,
- middle_domain);
- if (!msi_domain) {
- pr_err("Failed to create PCI MSI domain\n");
- irq_domain_remove(middle_domain);
- return -ENOMEM;
- }
+ middle_domain->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
+ middle_domain->msi_parent_ops = &pch_msi_parent_ops;
return 0;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support
2024-08-15 11:26 [PATCH v10 0/2] Loongarch-avec support Tianyang Zhang
2024-08-15 11:26 ` [PATCH v10 1/2] irqchip/loongson-pch-msi: Switch to MSI parent domains Tianyang Zhang
@ 2024-08-15 11:26 ` Tianyang Zhang
2024-08-20 16:29 ` Thomas Gleixner
1 sibling, 1 reply; 9+ messages in thread
From: Tianyang Zhang @ 2024-08-15 11:26 UTC (permalink / raw)
To: corbet, alexs, chenhuacai, kernel, tglx, jiaxun.yang, gaoliang,
wangliupu, lvjianmin, zhangtianyang, yijun, mhocko, akpm,
dianders, maobibo, xry111, zhaotianrui, nathan, yangtiezhu,
zhoubinbin
Cc: loongarch, linux-doc, linux-kernel, Huacai Chen
Introduce the advanced extended interrupt controllers (AVECINTC). This
feature will allow each core to have 256 independent interrupt vectors
and MSI interrupts can be independently routed to any vector on any CPU.
The whole topology of irqchips in LoongArch machines looks like this if
AVECINTC is supported:
+-----+ +-----------------------+ +-------+
| IPI | --> | CPUINTC | <-- | Timer |
+-----+ +-----------------------+ +-------+
^ ^ ^
| | |
+---------+ +----------+ +---------+ +-------+
| EIOINTC | | AVECINTC | | LIOINTC | <-- | UARTs |
+---------+ +----------+ +---------+ +-------+
^ ^
| |
+---------+ +---------+
| PCH-PIC | | PCH-MSI |
+---------+ +---------+
^ ^ ^
| | |
+---------+ +---------+ +---------+
| Devices | | PCH-LPC | | Devices |
+---------+ +---------+ +---------+
^
|
+---------+
| Devices |
+---------+
Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
Co-developed-by: Liupu Wang <wangliupu@loongson.cn>
Signed-off-by: Liupu Wang <wangliupu@loongson.cn>
Co-developed-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
---
.../arch/loongarch/irq-chip-model.rst | 32 ++
.../zh_CN/arch/loongarch/irq-chip-model.rst | 32 ++
arch/loongarch/Kconfig | 1 +
arch/loongarch/include/asm/cpu-features.h | 1 +
arch/loongarch/include/asm/cpu.h | 2 +
arch/loongarch/include/asm/hardirq.h | 3 +-
arch/loongarch/include/asm/hw_irq.h | 2 +
arch/loongarch/include/asm/irq.h | 25 +-
arch/loongarch/include/asm/loongarch.h | 18 +-
arch/loongarch/include/asm/smp.h | 2 +
arch/loongarch/kernel/cpu-probe.c | 3 +-
arch/loongarch/kernel/irq.c | 15 +-
arch/loongarch/kernel/paravirt.c | 5 +
arch/loongarch/kernel/smp.c | 6 +
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-loongarch-avec.c | 426 ++++++++++++++++++
drivers/irqchip/irq-loongarch-cpu.c | 5 +-
drivers/irqchip/irq-loongson-eiointc.c | 7 +-
drivers/irqchip/irq-loongson-pch-msi.c | 24 +-
include/linux/cpuhotplug.h | 3 +-
20 files changed, 591 insertions(+), 23 deletions(-)
create mode 100644 drivers/irqchip/irq-loongarch-avec.c
diff --git a/Documentation/arch/loongarch/irq-chip-model.rst b/Documentation/arch/loongarch/irq-chip-model.rst
index 7988f4192363..6dd48256e39f 100644
--- a/Documentation/arch/loongarch/irq-chip-model.rst
+++ b/Documentation/arch/loongarch/irq-chip-model.rst
@@ -85,6 +85,38 @@ to CPUINTC directly::
| Devices |
+---------+
+Advanced Extended IRQ model
+===========================
+
+In this model, IPI (Inter-Processor Interrupt) and CPU Local Timer interrupt go
+to CPUINTC directly, CPU UARTS interrupts go to LIOINTC, PCH-MSI interrupts go
+to AVECINTC, and then go to CPUINTC directly, while all other devices interrupts
+go to PCH-PIC/PCH-LPC and gathered by EIOINTC, and then go to CPUINTC directly::
+
+ +-----+ +-----------------------+ +-------+
+ | IPI | --> | CPUINTC | <-- | Timer |
+ +-----+ +-----------------------+ +-------+
+ ^ ^ ^
+ | | |
+ +---------+ +----------+ +---------+ +-------+
+ | EIOINTC | | AVECINTC | | LIOINTC | <-- | UARTs |
+ +---------+ +----------+ +---------+ +-------+
+ ^ ^
+ | |
+ +---------+ +---------+
+ | PCH-PIC | | PCH-MSI |
+ +---------+ +---------+
+ ^ ^ ^
+ | | |
+ +---------+ +---------+ +---------+
+ | Devices | | PCH-LPC | | Devices |
+ +---------+ +---------+ +---------+
+ ^
+ |
+ +---------+
+ | Devices |
+ +---------+
+
ACPI-related definitions
========================
diff --git a/Documentation/translations/zh_CN/arch/loongarch/irq-chip-model.rst b/Documentation/translations/zh_CN/arch/loongarch/irq-chip-model.rst
index f1e9ab18206c..472761938682 100644
--- a/Documentation/translations/zh_CN/arch/loongarch/irq-chip-model.rst
+++ b/Documentation/translations/zh_CN/arch/loongarch/irq-chip-model.rst
@@ -87,6 +87,38 @@ PCH-LPC/PCH-MSI，然后被EIOINTC统一收集，再直接到达CPUINTC::
| Devices |
+---------+
+高级扩展IRQ模型
+===============
+
+在这种模型里面，IPI（Inter-Processor Interrupt）和CPU本地时钟中断直接发送到CPUINTC，
+CPU串口（UARTs）中断发送到LIOINTC，PCH-MSI中断发送到AVECINTC，而后通过AVECINTC直接
+送达CPUINTC，而其他所有设备的中断则分别发送到所连接的PCH-PIC/PCH-LPC，然后由EIOINTC
+统一收集，再直接到达CPUINTC::
+
+ +-----+ +-----------------------+ +-------+
+ | IPI | --> | CPUINTC | <-- | Timer |
+ +-----+ +-----------------------+ +-------+
+ ^ ^ ^
+ | | |
+ +---------+ +----------+ +---------+ +-------+
+ | EIOINTC | | AVECINTC | | LIOINTC | <-- | UARTs |
+ +---------+ +----------+ +---------+ +-------+
+ ^ ^
+ | |
+ +---------+ +---------+
+ | PCH-PIC | | PCH-MSI |
+ +---------+ +---------+
+ ^ ^ ^
+ | | |
+ +---------+ +---------+ +---------+
+ | Devices | | PCH-LPC | | Devices |
+ +---------+ +---------+ +---------+
+ ^
+ |
+ +---------+
+ | Devices |
+ +---------+
+
ACPI相关的定义
==============
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 70f169210b52..0e3abf7b0bd3 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -85,6 +85,7 @@ config LOONGARCH
select GENERIC_ENTRY
select GENERIC_GETTIMEOFDAY
select GENERIC_IOREMAP if !ARCH_IOREMAP
+ select GENERIC_IRQ_MATRIX_ALLOCATOR
select GENERIC_IRQ_MULTI_HANDLER
select GENERIC_IRQ_PROBE
select GENERIC_IRQ_SHOW
diff --git a/arch/loongarch/include/asm/cpu-features.h b/arch/loongarch/include/asm/cpu-features.h
index 2eafe6a6aca8..16a716f88a5c 100644
--- a/arch/loongarch/include/asm/cpu-features.h
+++ b/arch/loongarch/include/asm/cpu-features.h
@@ -65,5 +65,6 @@
#define cpu_has_guestid cpu_opt(LOONGARCH_CPU_GUESTID)
#define cpu_has_hypervisor cpu_opt(LOONGARCH_CPU_HYPERVISOR)
#define cpu_has_ptw cpu_opt(LOONGARCH_CPU_PTW)
+#define cpu_has_avecint cpu_opt(LOONGARCH_CPU_AVECINT)
#endif /* __ASM_CPU_FEATURES_H */
diff --git a/arch/loongarch/include/asm/cpu.h b/arch/loongarch/include/asm/cpu.h
index 48b9f7168bcc..843f9c4ec980 100644
--- a/arch/loongarch/include/asm/cpu.h
+++ b/arch/loongarch/include/asm/cpu.h
@@ -99,6 +99,7 @@ enum cpu_type_enum {
#define CPU_FEATURE_GUESTID 24 /* CPU has GuestID feature */
#define CPU_FEATURE_HYPERVISOR 25 /* CPU has hypervisor (running in VM) */
#define CPU_FEATURE_PTW 26 /* CPU has hardware page table walker */
+#define CPU_FEATURE_AVECINT 27 /* CPU has avec interrupt */
#define LOONGARCH_CPU_CPUCFG BIT_ULL(CPU_FEATURE_CPUCFG)
#define LOONGARCH_CPU_LAM BIT_ULL(CPU_FEATURE_LAM)
@@ -127,5 +128,6 @@ enum cpu_type_enum {
#define LOONGARCH_CPU_GUESTID BIT_ULL(CPU_FEATURE_GUESTID)
#define LOONGARCH_CPU_HYPERVISOR BIT_ULL(CPU_FEATURE_HYPERVISOR)
#define LOONGARCH_CPU_PTW BIT_ULL(CPU_FEATURE_PTW)
+#define LOONGARCH_CPU_AVECINT BIT_ULL(CPU_FEATURE_AVECINT)
#endif /* _ASM_CPU_H */
diff --git a/arch/loongarch/include/asm/hardirq.h b/arch/loongarch/include/asm/hardirq.h
index 1d7feb719515..10da8d6961cb 100644
--- a/arch/loongarch/include/asm/hardirq.h
+++ b/arch/loongarch/include/asm/hardirq.h
@@ -12,12 +12,13 @@
extern void ack_bad_irq(unsigned int irq);
#define ack_bad_irq ack_bad_irq
-#define NR_IPI 3
+#define NR_IPI 4
enum ipi_msg_type {
IPI_RESCHEDULE,
IPI_CALL_FUNCTION,
IPI_IRQ_WORK,
+ IPI_CLEAR_VECTOR,
};
typedef struct {
diff --git a/arch/loongarch/include/asm/hw_irq.h b/arch/loongarch/include/asm/hw_irq.h
index af4f4e8fbd85..8156ffb67415 100644
--- a/arch/loongarch/include/asm/hw_irq.h
+++ b/arch/loongarch/include/asm/hw_irq.h
@@ -9,6 +9,8 @@
extern atomic_t irq_err_count;
+#define ARCH_IRQ_INIT_FLAGS IRQ_NOPROBE
+
/*
* interrupt-retrigger: NOP for now. This may not be appropriate for all
* machines, we'll see ...
diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h
index 480418bc5071..1b255bf8168f 100644
--- a/arch/loongarch/include/asm/irq.h
+++ b/arch/loongarch/include/asm/irq.h
@@ -39,11 +39,22 @@ void spurious_interrupt(void);
#define NR_IRQS_LEGACY 16
+/*
+ * 256 Vectors Mapping for AVECINTC:
+ *
+ * 0 - 15: Mapping classic IPs, e.g. IP0-12.
+ * 16 - 255: Mapping vectors for external IRQ.
+ *
+ */
+#define NR_VECTORS 256
+#define NR_LEGACY_VECTORS 16
+#define IRQ_MATRIX_BITS NR_VECTORS
+
#define arch_trigger_cpumask_backtrace arch_trigger_cpumask_backtrace
void arch_trigger_cpumask_backtrace(const struct cpumask *mask, int exclude_cpu);
#define MAX_IO_PICS 2
-#define NR_IRQS (64 + (256 * MAX_IO_PICS))
+#define NR_IRQS (64 + NR_VECTORS * (NR_CPUS + MAX_IO_PICS))
struct acpi_vector_group {
int node;
@@ -65,7 +76,7 @@ extern struct acpi_vector_group msi_group[MAX_IO_PICS];
#define LOONGSON_LPC_LAST_IRQ (LOONGSON_LPC_IRQ_BASE + 15)
#define LOONGSON_CPU_IRQ_BASE 16
-#define LOONGSON_CPU_LAST_IRQ (LOONGSON_CPU_IRQ_BASE + 14)
+#define LOONGSON_CPU_LAST_IRQ (LOONGSON_CPU_IRQ_BASE + 15)
#define LOONGSON_PCH_IRQ_BASE 64
#define LOONGSON_PCH_ACPI_IRQ (LOONGSON_PCH_IRQ_BASE + 47)
@@ -92,15 +103,21 @@ int liointc_acpi_init(struct irq_domain *parent,
struct acpi_madt_lio_pic *acpi_liointc);
int eiointc_acpi_init(struct irq_domain *parent,
struct acpi_madt_eio_pic *acpi_eiointc);
+int avecintc_acpi_init(struct irq_domain *parent);
+
+void complete_irq_moving(void);
int htvec_acpi_init(struct irq_domain *parent,
struct acpi_madt_ht_pic *acpi_htvec);
int pch_lpc_acpi_init(struct irq_domain *parent,
struct acpi_madt_lpc_pic *acpi_pchlpc);
-int pch_msi_acpi_init(struct irq_domain *parent,
- struct acpi_madt_msi_pic *acpi_pchmsi);
int pch_pic_acpi_init(struct irq_domain *parent,
struct acpi_madt_bio_pic *acpi_pchpic);
+int pch_msi_acpi_init(struct irq_domain *parent,
+ struct acpi_madt_msi_pic *acpi_pchmsi);
+int pch_msi_acpi_init_v2(struct irq_domain *parent,
+ struct acpi_madt_msi_pic *acpi_pchmsi);
+
int find_pch_pic(u32 gsi);
struct fwnode_handle *get_pch_msi_handle(int pci_segment);
diff --git a/arch/loongarch/include/asm/loongarch.h b/arch/loongarch/include/asm/loongarch.h
index 04a78010fc72..70834a47257d 100644
--- a/arch/loongarch/include/asm/loongarch.h
+++ b/arch/loongarch/include/asm/loongarch.h
@@ -253,8 +253,8 @@
#define CSR_ESTAT_EXC_WIDTH 6
#define CSR_ESTAT_EXC (_ULCAST_(0x3f) << CSR_ESTAT_EXC_SHIFT)
#define CSR_ESTAT_IS_SHIFT 0
-#define CSR_ESTAT_IS_WIDTH 14
-#define CSR_ESTAT_IS (_ULCAST_(0x3fff) << CSR_ESTAT_IS_SHIFT)
+#define CSR_ESTAT_IS_WIDTH 15
+#define CSR_ESTAT_IS (_ULCAST_(0x7fff) << CSR_ESTAT_IS_SHIFT)
#define LOONGARCH_CSR_ERA 0x6 /* ERA */
@@ -649,6 +649,13 @@
#define LOONGARCH_CSR_CTAG 0x98 /* TagLo + TagHi */
+#define LOONGARCH_CSR_ISR0 0xa0
+#define LOONGARCH_CSR_ISR1 0xa1
+#define LOONGARCH_CSR_ISR2 0xa2
+#define LOONGARCH_CSR_ISR3 0xa3
+
+#define LOONGARCH_CSR_IRR 0xa4
+
#define LOONGARCH_CSR_PRID 0xc0
/* Shadow MCSR : 0xc0 ~ 0xff */
@@ -1011,7 +1018,7 @@
/*
* CSR_ECFG IM
*/
-#define ECFG0_IM 0x00001fff
+#define ECFG0_IM 0x00005fff
#define ECFGB_SIP0 0
#define ECFGF_SIP0 (_ULCAST_(1) << ECFGB_SIP0)
#define ECFGB_SIP1 1
@@ -1054,6 +1061,7 @@
#define IOCSRF_EIODECODE BIT_ULL(9)
#define IOCSRF_FLATMODE BIT_ULL(10)
#define IOCSRF_VM BIT_ULL(11)
+#define IOCSRF_AVEC BIT_ULL(15)
#define LOONGARCH_IOCSR_VENDOR 0x10
@@ -1065,6 +1073,7 @@
#define IOCSR_MISC_FUNC_SOFT_INT BIT_ULL(10)
#define IOCSR_MISC_FUNC_TIMER_RESET BIT_ULL(21)
#define IOCSR_MISC_FUNC_EXT_IOI_EN BIT_ULL(48)
+#define IOCSR_MISC_FUNC_AVEC_EN BIT_ULL(51)
#define LOONGARCH_IOCSR_CPUTEMP 0x428
@@ -1387,9 +1396,10 @@ __BUILD_CSR_OP(tlbidx)
#define INT_TI 11 /* Timer */
#define INT_IPI 12
#define INT_NMI 13
+#define INT_AVEC 14
/* ExcCodes corresponding to interrupts */
-#define EXCCODE_INT_NUM (INT_NMI + 1)
+#define EXCCODE_INT_NUM (INT_AVEC + 1)
#define EXCCODE_INT_START 64
#define EXCCODE_INT_END (EXCCODE_INT_START + EXCCODE_INT_NUM - 1)
diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
index 50db503f44e3..3383c9d24e94 100644
--- a/arch/loongarch/include/asm/smp.h
+++ b/arch/loongarch/include/asm/smp.h
@@ -70,10 +70,12 @@ extern int __cpu_logical_map[NR_CPUS];
#define ACTION_RESCHEDULE 1
#define ACTION_CALL_FUNCTION 2
#define ACTION_IRQ_WORK 3
+#define ACTION_CLEAR_VECTOR 4
#define SMP_BOOT_CPU BIT(ACTION_BOOT_CPU)
#define SMP_RESCHEDULE BIT(ACTION_RESCHEDULE)
#define SMP_CALL_FUNCTION BIT(ACTION_CALL_FUNCTION)
#define SMP_IRQ_WORK BIT(ACTION_IRQ_WORK)
+#define SMP_CLEAR_VECTOR BIT(ACTION_CLEAR_VECTOR)
struct secondary_data {
unsigned long stack;
diff --git a/arch/loongarch/kernel/cpu-probe.c b/arch/loongarch/kernel/cpu-probe.c
index 55320813ee08..14f0449f5452 100644
--- a/arch/loongarch/kernel/cpu-probe.c
+++ b/arch/loongarch/kernel/cpu-probe.c
@@ -106,7 +106,6 @@ static void cpu_probe_common(struct cpuinfo_loongarch *c)
elf_hwcap |= HWCAP_LOONGARCH_CRC32;
}
-
config = read_cpucfg(LOONGARCH_CPUCFG2);
if (config & CPUCFG2_LAM) {
c->options |= LOONGARCH_CPU_LAM;
@@ -174,6 +173,8 @@ static void cpu_probe_common(struct cpuinfo_loongarch *c)
c->options |= LOONGARCH_CPU_FLATMODE;
if (config & IOCSRF_EIODECODE)
c->options |= LOONGARCH_CPU_EIODECODE;
+ if (config & IOCSRF_AVEC)
+ c->options |= LOONGARCH_CPU_AVECINT;
if (config & IOCSRF_VM)
c->options |= LOONGARCH_CPU_HYPERVISOR;
diff --git a/arch/loongarch/kernel/irq.c b/arch/loongarch/kernel/irq.c
index f4991c03514f..d129039b368b 100644
--- a/arch/loongarch/kernel/irq.c
+++ b/arch/loongarch/kernel/irq.c
@@ -87,6 +87,18 @@ static void __init init_vec_parent_group(void)
acpi_table_parse(ACPI_SIG_MCFG, early_pci_mcfg_parse);
}
+int __init arch_probe_nr_irqs(void)
+{
+ int nr_io_pics = bitmap_weight(loongson_sysconf.cores_io_master, NR_CPUS);
+
+ if (!cpu_has_avecint)
+ nr_irqs = (64 + NR_VECTORS * nr_io_pics);
+ else
+ nr_irqs = (64 + NR_VECTORS * (nr_cpu_ids + nr_io_pics));
+
+ return NR_IRQS_LEGACY;
+}
+
void __init init_IRQ(void)
{
int i;
@@ -102,9 +114,6 @@ void __init init_IRQ(void)
mp_ops.init_ipi();
#endif
- for (i = 0; i < NR_IRQS; i++)
- irq_set_noprobe(i);
-
for_each_possible_cpu(i) {
page = alloc_pages_node(cpu_to_node(i), GFP_KERNEL, order);
diff --git a/arch/loongarch/kernel/paravirt.c b/arch/loongarch/kernel/paravirt.c
index 9c9b75b76f62..4d736a4e488d 100644
--- a/arch/loongarch/kernel/paravirt.c
+++ b/arch/loongarch/kernel/paravirt.c
@@ -134,6 +134,11 @@ static irqreturn_t pv_ipi_interrupt(int irq, void *dev)
info->ipi_irqs[IPI_IRQ_WORK]++;
}
+ if (action & SMP_CLEAR_VECTOR) {
+ complete_irq_moving();
+ info->ipi_irqs[IPI_CLEAR_VECTOR]++;
+ }
+
return IRQ_HANDLED;
}
diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
index ca405ab86aae..4adbbef3450a 100644
--- a/arch/loongarch/kernel/smp.c
+++ b/arch/loongarch/kernel/smp.c
@@ -72,6 +72,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
[IPI_RESCHEDULE] = "Rescheduling interrupts",
[IPI_CALL_FUNCTION] = "Function call interrupts",
[IPI_IRQ_WORK] = "IRQ work interrupts",
+ [IPI_CLEAR_VECTOR] = "Clear vector interrupts",
};
void show_ipi_list(struct seq_file *p, int prec)
@@ -248,6 +249,11 @@ static irqreturn_t loongson_ipi_interrupt(int irq, void *dev)
per_cpu(irq_stat, cpu).ipi_irqs[IPI_IRQ_WORK]++;
}
+ if (action & SMP_CLEAR_VECTOR) {
+ complete_irq_moving();
+ per_cpu(irq_stat, cpu).ipi_irqs[IPI_CLEAR_VECTOR]++;
+ }
+
return IRQ_HANDLED;
}
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 15635812b2d6..e3679ec2b9f7 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -110,7 +110,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
-obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
+obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o irq-loongarch-avec.o
obj-$(CONFIG_LOONGSON_LIOINTC) += irq-loongson-liointc.o
obj-$(CONFIG_LOONGSON_EIOINTC) += irq-loongson-eiointc.o
obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
diff --git a/drivers/irqchip/irq-loongarch-avec.c b/drivers/irqchip/irq-loongarch-avec.c
new file mode 100644
index 000000000000..d66fae41d95a
--- /dev/null
+++ b/drivers/irqchip/irq-loongarch-avec.c
@@ -0,0 +1,426 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020-2024 Loongson Technologies, Inc.
+ */
+
+#include <linux/cpuhotplug.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/msi.h>
+#include <linux/radix-tree.h>
+#include <linux/spinlock.h>
+
+#include <asm/loongarch.h>
+#include <asm/setup.h>
+
+#include "irq-msi-lib.h"
+
+#define VECTORS_PER_REG 64
+#define IRR_VECTOR_MASK 0xffUL
+#define IRR_INVALID_MASK 0x80000000UL
+#define AVEC_MSG_OFFSET 0x100000
+
+static phys_addr_t msi_base_addr;
+
+#ifdef CONFIG_SMP
+struct pending_list {
+ struct list_head head;
+};
+
+static struct cpumask intersect_mask;
+static DEFINE_PER_CPU(struct pending_list, pending_list);
+#endif
+
+static DEFINE_PER_CPU(struct irq_desc * [NR_VECTORS], irq_map);
+
+struct avecintc_chip {
+ struct fwnode_handle *fwnode;
+ struct irq_domain *domain;
+ struct irq_matrix *vector_matrix;
+ raw_spinlock_t lock;
+};
+
+static struct avecintc_chip loongarch_avec;
+
+struct avecintc_data {
+ struct list_head entry;
+ unsigned int cpu;
+ unsigned int vec;
+ unsigned int prev_cpu;
+ unsigned int prev_vec;
+ unsigned int moving : 1,
+ managed : 1;
+};
+
+static inline void avecintc_ack_irq(struct irq_data *d)
+{
+}
+
+static inline void avecintc_mask_irq(struct irq_data *d)
+{
+}
+
+static inline void avecintc_unmask_irq(struct irq_data *d)
+{
+}
+
+#ifdef CONFIG_SMP
+static inline void pending_list_init(int cpu)
+{
+ struct pending_list *plist = per_cpu_ptr(&pending_list, cpu);
+
+ INIT_LIST_HEAD(&plist->head);
+}
+
+static void avecintc_sync(struct avecintc_data *adata)
+{
+ struct pending_list *plist;
+
+ if (cpu_online(adata->prev_cpu)) {
+ plist = per_cpu_ptr(&pending_list, adata->prev_cpu);
+ list_add_tail(&adata->entry, &plist->head);
+ adata->moving = 1;
+ mp_ops.send_ipi_single(adata->prev_cpu, ACTION_CLEAR_VECTOR);
+ }
+}
+
+static int avecintc_set_affinity(struct irq_data *data, const struct cpumask *dest, bool force)
+{
+ unsigned int cpu, ret, vector;
+ struct avecintc_data *adata;
+
+ scoped_guard(raw_spinlock, &loongarch_avec.lock) {
+ adata = irq_data_get_irq_chip_data(data);
+
+ if (adata->moving)
+ return -EBUSY;
+
+ if (adata->vec == UINT_MAX)
+ return -EINVAL;
+
+ if (cpu_online(adata->cpu) && cpumask_test_cpu(adata->cpu, dest))
+ return 0;
+
+ cpumask_and(&intersect_mask, dest, cpu_online_mask);
+
+ ret = irq_matrix_alloc(loongarch_avec.vector_matrix, &intersect_mask, false, &cpu);
+ if (ret < 0)
+ return ret;
+
+ vector = ret;
+ adata->cpu = cpu;
+ adata->vec = vector;
+ per_cpu_ptr(irq_map, adata->cpu)[adata->vec] = irq_data_to_desc(data);
+ avecintc_sync(adata);
+ }
+
+ irq_data_update_effective_affinity(data, cpumask_of(cpu));
+
+ return IRQ_SET_MASK_OK;
+}
+
+static int avecintc_cpu_online(unsigned int cpu)
+{
+ if (!loongarch_avec.vector_matrix)
+ return 0;
+
+ guard(raw_spinlock)(&loongarch_avec.lock);
+
+ irq_matrix_online(loongarch_avec.vector_matrix);
+
+ pending_list_init(cpu);
+
+ return 0;
+}
+
+static int avecintc_cpu_offline(unsigned int cpu)
+{
+ struct pending_list *plist = per_cpu_ptr(&pending_list, cpu);
+
+ if (!loongarch_avec.vector_matrix)
+ return 0;
+
+ guard(raw_spinlock)(&loongarch_avec.lock);
+
+ if (!list_empty(&plist->head))
+ pr_warn("CPU#%d vector is busy\n", cpu);
+
+ irq_matrix_offline(loongarch_avec.vector_matrix);
+
+ return 0;
+}
+
+void complete_irq_moving(void)
+{
+ struct pending_list *plist = this_cpu_ptr(&pending_list);
+ struct avecintc_data *adata, *tdata;
+ int cpu, vector, bias;
+ uint64_t isr;
+
+ guard(raw_spinlock)(&loongarch_avec.lock);
+
+ list_for_each_entry_safe(adata, tdata, &plist->head, entry) {
+ cpu = adata->prev_cpu;
+ vector = adata->prev_vec;
+ bias = vector / VECTORS_PER_REG;
+ switch (bias) {
+ case 0:
+ isr = csr_read64(LOONGARCH_CSR_ISR0);
+ break;
+ case 1:
+ isr = csr_read64(LOONGARCH_CSR_ISR1);
+ break;
+ case 2:
+ isr = csr_read64(LOONGARCH_CSR_ISR2);
+ break;
+ case 3:
+ isr = csr_read64(LOONGARCH_CSR_ISR3);
+ break;
+ }
+
+ if (isr & (1UL << (vector % VECTORS_PER_REG))) {
+ mp_ops.send_ipi_single(cpu, ACTION_CLEAR_VECTOR);
+ continue;
+ }
+ list_del(&adata->entry);
+ irq_matrix_free(loongarch_avec.vector_matrix, cpu, vector, adata->managed);
+ this_cpu_write(irq_map[vector], NULL);
+ adata->moving = 0;
+ adata->prev_cpu = adata->cpu;
+ adata->prev_vec = adata->vec;
+ }
+}
+#endif
+
+static void avecintc_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
+{
+ struct avecintc_data *adata = irq_data_get_irq_chip_data(d);
+
+ msg->address_hi = 0x0;
+ msg->address_lo = (msi_base_addr | (adata->vec & 0xff) << 4) |
+ ((cpu_logical_map(adata->cpu & 0xffff)) << 12);
+ msg->data = 0x0;
+}
+
+static struct irq_chip avec_irq_controller = {
+ .name = "AVECINTC",
+ .irq_ack = avecintc_ack_irq,
+ .irq_mask = avecintc_mask_irq,
+ .irq_unmask = avecintc_unmask_irq,
+#ifdef CONFIG_SMP
+ .irq_set_affinity = avecintc_set_affinity,
+#endif
+ .irq_compose_msi_msg = avecintc_compose_msi_msg,
+};
+
+static void avecintc_irq_dispatch(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct irq_desc *d;
+
+ chained_irq_enter(chip, desc);
+
+ while (true) {
+ unsigned long vector = csr_read64(LOONGARCH_CSR_IRR);
+ if (vector & IRR_INVALID_MASK)
+ break;
+
+ vector &= IRR_VECTOR_MASK;
+
+ d = this_cpu_read(irq_map[vector]);
+ if (d) {
+ generic_handle_irq_desc(d);
+ } else {
+ spurious_interrupt();
+ pr_warn("Unexpected IRQ occurs on CPU#%d [vector %ld]\n", smp_processor_id(), vector);
+ }
+ }
+
+ chained_irq_exit(chip, desc);
+}
+
+static int avecintc_domain_alloc(struct irq_domain *domain,
+ unsigned int virq, unsigned int nr_irqs, void *arg)
+{
+ guard(raw_spinlock_irqsave)(&loongarch_avec.lock);
+
+ for (unsigned int i = 0; i < nr_irqs; i++) {
+ struct irq_data *irqd = irq_domain_get_irq_data(domain, virq + i);
+ struct avecintc_data *adata = kzalloc(sizeof(*adata), GFP_KERNEL);
+ unsigned int cpu, ret;
+
+ if (!adata)
+ return -ENOMEM;
+
+ ret = irq_matrix_alloc(loongarch_avec.vector_matrix, cpu_online_mask, false, &cpu);
+ if (ret < 0) {
+ kfree(adata);
+ return ret;
+ }
+
+ adata->moving = 0;
+ adata->prev_cpu = adata->cpu = cpu;
+ adata->prev_vec = adata->vec = ret;
+ adata->managed = irqd_affinity_is_managed(irqd);
+ irq_domain_set_info(domain, virq + i, virq + i, &avec_irq_controller,
+ adata, handle_edge_irq, NULL, NULL);
+ irqd_set_single_target(irqd);
+ irqd_set_affinity_on_activate(irqd);
+
+ per_cpu_ptr(irq_map, adata->cpu)[adata->vec] = irq_data_to_desc(irqd);
+ }
+
+ return 0;
+}
+
+static void clear_free_vector(struct irq_data *irqd)
+{
+ struct avecintc_data *adata = irq_data_get_irq_chip_data(irqd);
+ bool managed = irqd_affinity_is_managed(irqd);
+
+ per_cpu(irq_map, adata->cpu)[adata->vec] = NULL;
+ irq_matrix_free(loongarch_avec.vector_matrix, adata->cpu, adata->vec, managed);
+ adata->cpu = UINT_MAX;
+ adata->vec = UINT_MAX;
+
+#ifdef CONFIG_SMP
+ if (!adata->moving)
+ return;
+
+ per_cpu(irq_map, adata->prev_cpu)[adata->prev_vec] = NULL;
+ irq_matrix_free(loongarch_avec.vector_matrix,
+ adata->prev_cpu, adata->prev_vec, adata->managed);
+ adata->moving = 0;
+ adata->prev_vec = UINT_MAX;
+ adata->prev_cpu = UINT_MAX;
+ list_del_init(&adata->entry);
+#endif
+ kfree(adata);
+}
+
+static void avecintc_domain_free(struct irq_domain *domain,
+ unsigned int virq, unsigned int nr_irqs)
+{
+ guard(raw_spinlock_irqsave)(&loongarch_avec.lock);
+
+ for (unsigned int i = 0; i < nr_irqs; i++) {
+ struct irq_data *d = irq_domain_get_irq_data(domain, virq + i);
+ if (d) {
+ clear_free_vector(d);
+ irq_domain_reset_irq_data(d);
+ }
+ }
+}
+
+static const struct irq_domain_ops avecintc_domain_ops = {
+ .alloc = avecintc_domain_alloc,
+ .free = avecintc_domain_free,
+ .select = msi_lib_irq_domain_select,
+};
+
+static int __init irq_matrix_init(void)
+{
+ loongarch_avec.vector_matrix = irq_alloc_matrix(NR_VECTORS, 0, NR_VECTORS);
+ if (!loongarch_avec.vector_matrix)
+ return -ENOMEM;
+
+ for (int i = 0; i < NR_LEGACY_VECTORS; i++)
+ irq_matrix_assign_system(loongarch_avec.vector_matrix, i, false);
+
+ irq_matrix_online(loongarch_avec.vector_matrix);
+
+ return 0;
+}
+
+static int __init avecintc_init(struct irq_domain *parent)
+{
+ int ret, parent_irq;
+ unsigned long value;
+
+ raw_spin_lock_init(&loongarch_avec.lock);
+
+ loongarch_avec.fwnode = irq_domain_alloc_named_fwnode("AVECINTC");
+ if (!loongarch_avec.fwnode) {
+ pr_err("Unable to allocate domain handle\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ loongarch_avec.domain = irq_domain_create_tree(loongarch_avec.fwnode,
+ &avecintc_domain_ops, NULL);
+ if (!loongarch_avec.domain) {
+ pr_err("Unable to create IRQ domain\n");
+ ret = -ENOMEM;
+ goto out_free_handle;
+ }
+
+ parent_irq = irq_create_mapping(parent, INT_AVEC);
+ if (!parent_irq) {
+ pr_err("Failed to mapping hwirq\n");
+ ret = -EINVAL;
+ goto out_remove_domain;
+ }
+
+ ret = irq_matrix_init();
+ if (ret < 0) {
+ pr_err("Failed to init irq matrix\n");
+ goto out_remove_domain;
+ }
+ irq_set_chained_handler_and_data(parent_irq, avecintc_irq_dispatch, NULL);
+
+#ifdef CONFIG_SMP
+ pending_list_init(0);
+ cpuhp_setup_state_nocalls(CPUHP_AP_IRQ_AVECINTC_STARTING,
+ "irqchip/loongarch/avecintc:starting",
+ avecintc_cpu_online, avecintc_cpu_offline);
+#endif
+ value = iocsr_read64(LOONGARCH_IOCSR_MISC_FUNC);
+ value |= IOCSR_MISC_FUNC_AVEC_EN;
+ iocsr_write64(value, LOONGARCH_IOCSR_MISC_FUNC);
+
+ return ret;
+
+out_remove_domain:
+ irq_domain_remove(loongarch_avec.domain);
+out_free_handle:
+ irq_domain_free_fwnode(loongarch_avec.fwnode);
+out:
+ return ret;
+}
+
+static int __init pch_msi_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_msi_pic *pchmsi_entry = (struct acpi_madt_msi_pic *)header;
+
+ msi_base_addr = pchmsi_entry->msg_address - AVEC_MSG_OFFSET;
+
+ return pch_msi_acpi_init_v2(loongarch_avec.domain, pchmsi_entry);
+}
+
+static inline int __init acpi_cascade_irqdomain_init(void)
+{
+ return acpi_table_parse_madt(ACPI_MADT_TYPE_MSI_PIC, pch_msi_parse_madt, 1);
+}
+
+int __init avecintc_acpi_init(struct irq_domain *parent)
+{
+ int ret = avecintc_init(parent);
+ if (ret < 0) {
+ pr_err("Failed to init IRQ domain\n");
+ return ret;
+ }
+
+ ret = acpi_cascade_irqdomain_init();
+ if (ret < 0) {
+ pr_err("Failed to init cascade IRQ domain\n");
+ return ret;
+ }
+
+ return ret;
+}
diff --git a/drivers/irqchip/irq-loongarch-cpu.c b/drivers/irqchip/irq-loongarch-cpu.c
index 9d8f2c406043..4fdc490b94c3 100644
--- a/drivers/irqchip/irq-loongarch-cpu.c
+++ b/drivers/irqchip/irq-loongarch-cpu.c
@@ -138,7 +138,10 @@ static int __init acpi_cascade_irqdomain_init(void)
if (r < 0)
return r;
- return 0;
+ if (cpu_has_avecint)
+ r = avecintc_acpi_init(irq_domain);
+
+ return r;
}
static int __init cpuintc_acpi_init(union acpi_subtable_headers *header,
diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index b1f2080be2be..895d15b96669 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -360,6 +360,9 @@ static int __init acpi_cascade_irqdomain_init(void)
if (r < 0)
return r;
+ if (cpu_has_avecint)
+ return 0;
+
r = acpi_table_parse_madt(ACPI_MADT_TYPE_MSI_PIC, pch_msi_parse_madt, 1);
if (r < 0)
return r;
@@ -396,8 +399,8 @@ static int __init eiointc_init(struct eiointc_priv *priv, int parent_irq,
if (nr_pics == 1) {
register_syscore_ops(&eiointc_syscore_ops);
- cpuhp_setup_state_nocalls(CPUHP_AP_IRQ_LOONGARCH_STARTING,
- "irqchip/loongarch/intc:starting",
+ cpuhp_setup_state_nocalls(CPUHP_AP_IRQ_EIOINTC_STARTING,
+ "irqchip/loongarch/eiointc:starting",
eiointc_router_init, NULL);
}
diff --git a/drivers/irqchip/irq-loongson-pch-msi.c b/drivers/irqchip/irq-loongson-pch-msi.c
index 2242f63c66fc..a56127314ecc 100644
--- a/drivers/irqchip/irq-loongson-pch-msi.c
+++ b/drivers/irqchip/irq-loongson-pch-msi.c
@@ -254,17 +254,17 @@ IRQCHIP_DECLARE(pch_msi, "loongson,pch-msi-1.0", pch_msi_of_init);
#ifdef CONFIG_ACPI
struct fwnode_handle *get_pch_msi_handle(int pci_segment)
{
- int i;
+ if (cpu_has_avecint)
+ return pch_msi_handle[0];
- for (i = 0; i < MAX_IO_PICS; i++) {
+ for (int i = 0; i < MAX_IO_PICS; i++) {
if (msi_group[i].pci_segment == pci_segment)
return pch_msi_handle[i];
}
- return NULL;
+ return pch_msi_handle[0];
}
-int __init pch_msi_acpi_init(struct irq_domain *parent,
- struct acpi_madt_msi_pic *acpi_pchmsi)
+int __init pch_msi_acpi_init(struct irq_domain *parent, struct acpi_madt_msi_pic *acpi_pchmsi)
{
int ret;
struct fwnode_handle *domain_handle;
@@ -277,4 +277,18 @@ int __init pch_msi_acpi_init(struct irq_domain *parent,
return ret;
}
+
+int __init pch_msi_acpi_init_v2(struct irq_domain *parent, struct acpi_madt_msi_pic *acpi_pchmsi)
+{
+ if (pch_msi_handle[0])
+ return 0;
+
+ pch_msi_handle[0] = parent->fwnode;
+ irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
+
+ parent->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
+ parent->msi_parent_ops = &pch_msi_parent_ops;
+
+ return 0;
+}
#endif
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 51ba681b915a..55a726d317d4 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -145,7 +145,8 @@ enum cpuhp_state {
CPUHP_AP_IRQ_ARMADA_XP_STARTING,
CPUHP_AP_IRQ_BCM2836_STARTING,
CPUHP_AP_IRQ_MIPS_GIC_STARTING,
- CPUHP_AP_IRQ_LOONGARCH_STARTING,
+ CPUHP_AP_IRQ_EIOINTC_STARTING,
+ CPUHP_AP_IRQ_AVECINTC_STARTING,
CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
CPUHP_AP_IRQ_RISCV_IMSIC_STARTING,
CPUHP_AP_ARM_MVEBU_COHERENCY,
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support
2024-08-15 11:26 ` [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support Tianyang Zhang
@ 2024-08-20 16:29 ` Thomas Gleixner
2024-08-21 1:47 ` Tianyang Zhang
2024-08-21 13:14 ` Huacai Chen
0 siblings, 2 replies; 9+ messages in thread
From: Thomas Gleixner @ 2024-08-20 16:29 UTC (permalink / raw)
To: Tianyang Zhang, corbet, alexs, chenhuacai, kernel, jiaxun.yang,
gaoliang, wangliupu, lvjianmin, zhangtianyang, yijun, mhocko,
akpm, dianders, maobibo, xry111, zhaotianrui, nathan, yangtiezhu,
zhoubinbin
Cc: loongarch, linux-doc, linux-kernel, Huacai Chen
On Thu, Aug 15 2024 at 19:26, Tianyang Zhang wrote:
> .../arch/loongarch/irq-chip-model.rst | 32 ++
> .../zh_CN/arch/loongarch/irq-chip-model.rst | 32 ++
> arch/loongarch/Kconfig | 1 +
> arch/loongarch/include/asm/cpu-features.h | 1 +
> arch/loongarch/include/asm/cpu.h | 2 +
> arch/loongarch/include/asm/hardirq.h | 3 +-
> arch/loongarch/include/asm/hw_irq.h | 2 +
> arch/loongarch/include/asm/irq.h | 25 +-
> arch/loongarch/include/asm/loongarch.h | 18 +-
> arch/loongarch/include/asm/smp.h | 2 +
> arch/loongarch/kernel/cpu-probe.c | 3 +-
> arch/loongarch/kernel/irq.c | 15 +-
> arch/loongarch/kernel/paravirt.c | 5 +
> arch/loongarch/kernel/smp.c | 6 +
> drivers/irqchip/Makefile | 2 +-
> drivers/irqchip/irq-loongarch-avec.c | 426 ++++++++++++++++++
> drivers/irqchip/irq-loongarch-cpu.c | 5 +-
> drivers/irqchip/irq-loongson-eiointc.c | 7 +-
> drivers/irqchip/irq-loongson-pch-msi.c | 24 +-
> include/linux/cpuhotplug.h | 3 +-
This patch is doing too many things at once and is absolutely not
reviewable.
Please split it up into the obvious bits and pieces:
1) The IRQ_NOPROBE change
2) See below
3) Documentation
4) Add the arch/loongson parts, i.e. all the defines and
basic required function prototypes with a little twist.
Add a Kconfig symbol:
Kconfig IRQ_LOONGARCH_AVEC
bool
in drivers/irqchip/Kconfig. This allows you to add all
arch/loongarch/ changes with the simple tweak:
#ifdef CONFIG_IRQ_LOONGARCH_AVEC
# define cpu_has_avecint cpu_opt(LOONGARCH_CPU_AVECINT)
#else
# define cpu_has_avecint false
#endif
and
#ifdef CONFIG_IRQ_LOONGARCH_AVEC
# define SMP_CLEAR_VECTOR BIT(ACTION_CLEAR_VECTOR)
#else
# define SMP_CLEAR_VECTOR (0)
#endif
That way the compiler will optimize out stuff like the
SMP_CLEAR_VECTOR handling and you only need the prototype of
complete_irq_moving(), but no implementation.
5) Change the CPU hotplug callback for EOINTC and do
the acpi_cascade_irqdomain_init() change.
6) Prepare get_pch_msi_handle() in the pch MSI driver
7) Implement the driver and select IRQ_LOONGARCH_AVEC
from IRQ_LOONGARCH_CPU
8) Remove the IRQ_LOONGARCH_AVEC helpers
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index 70f169210b52..0e3abf7b0bd3 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -85,6 +85,7 @@ config LOONGARCH
> select GENERIC_ENTRY
> select GENERIC_GETTIMEOFDAY
> select GENERIC_IOREMAP if !ARCH_IOREMAP
> + select GENERIC_IRQ_MATRIX_ALLOCATOR
Please move this to IRQ_LOONGARCH_CPU in patch #7
> @@ -92,15 +103,21 @@ int liointc_acpi_init(struct irq_domain *parent,
> struct acpi_madt_lio_pic *acpi_liointc);
> int eiointc_acpi_init(struct irq_domain *parent,
> struct acpi_madt_eio_pic *acpi_eiointc);
> +int avecintc_acpi_init(struct irq_domain *parent);
> +
> +void complete_irq_moving(void);
>
> int htvec_acpi_init(struct irq_domain *parent,
> struct acpi_madt_ht_pic *acpi_htvec);
> int pch_lpc_acpi_init(struct irq_domain *parent,
> struct acpi_madt_lpc_pic *acpi_pchlpc);
> -int pch_msi_acpi_init(struct irq_domain *parent,
> - struct acpi_madt_msi_pic *acpi_pchmsi);
> int pch_pic_acpi_init(struct irq_domain *parent,
> struct acpi_madt_bio_pic *acpi_pchpic);
> +int pch_msi_acpi_init(struct irq_domain *parent,
> + struct acpi_madt_msi_pic *acpi_pchmsi);
> +int pch_msi_acpi_init_v2(struct irq_domain *parent,
> + struct acpi_madt_msi_pic *acpi_pchmsi);
This is really the wrong place for all these prototypes. They are only
used in drivers/irqchip/... except for complete_irq_moving().
So the proper place for them is drivers/irqchip/irq-loongarch.h
Move them there. This is patch #2 which I referred to above.
>
> +static phys_addr_t msi_base_addr;
>
So you have everything related to the avec chip in loongarch_avec, so
why don't you move that into that data structure?
> +struct avecintc_chip {
> + struct fwnode_handle *fwnode;
> + struct irq_domain *domain;
> + struct irq_matrix *vector_matrix;
> + raw_spinlock_t lock;
> +};
The lock should be the first member as spinlocks have alignment
requirements....
> +static int avecintc_domain_alloc(struct irq_domain *domain,
> + unsigned int virq, unsigned int nr_irqs, void *arg)
> +{
> + guard(raw_spinlock_irqsave)(&loongarch_avec.lock);
> +
> + for (unsigned int i = 0; i < nr_irqs; i++) {
> + struct irq_data *irqd = irq_domain_get_irq_data(domain, virq + i);
> + struct avecintc_data *adata = kzalloc(sizeof(*adata), GFP_KERNEL);
That was never tested with any debug. You _cannot_ do a GFP_KERNEL
allocation with the raw spinlock held. And no, don't use
GFP_ATOMIC. There is absolutely zero reason to hold the lock accross all
of that. As you got your ideas from x86_vector_alloc_irqs(), you could
have looked at how that's done correctly.
> + unsigned int cpu, ret;
> +
> + if (!adata)
> + return -ENOMEM;
> +
> + ret = irq_matrix_alloc(loongarch_avec.vector_matrix, cpu_online_mask, false, &cpu);
> + if (ret < 0) {
> + kfree(adata);
> + return ret;
> + }
> +
> + adata->moving = 0;
Redundant. The struct is allocated with kzalloc()...
> + adata->prev_cpu = adata->cpu = cpu;
> + adata->prev_vec = adata->vec = ret;
> + adata->managed = irqd_affinity_is_managed(irqd);
If you want to support managed interrupts, then you cannot allocate
from the CPU online mask. See x86...
> + irq_domain_set_info(domain, virq + i, virq + i, &avec_irq_controller,
> + adata, handle_edge_irq, NULL, NULL);
> + irqd_set_single_target(irqd);
> + irqd_set_affinity_on_activate(irqd);
> +
> + per_cpu_ptr(irq_map, adata->cpu)[adata->vec] = irq_data_to_desc(irqd);
static int avecintc_alloc_vector(struct avecintc_adata *adata)
{
int ret, cpu;
guard(raw_spinlock_irqsave)(&loongarch_avec.lock);
ret = irq_matrix_alloc(loongarch_avec.vector_matrix, cpu_online_mask, false, &cpu);
if (ret < 0)
return ret;
adata->prev_cpu = adata->cpu = cpu;
adata->prev_vec = adata->vec = ret;
per_cpu_ptr(irq_map, adata->cpu)[adata->vec] = irq_data_to_desc(irqd);
return 0;
}
static int avecintc_domain_alloc(struct irq_domain *domain, ...)
{
for (unsigned int i = 0; i < nr_irqs; i++) {
struct irq_data *irqd = irq_domain_get_irq_data(domain, virq + i);
struct avecintc_data *adata = kzalloc(sizeof(*adata), GFP_KERNEL);
int ret;
if (!adata)
return -ENOMEM;
irq_domain_set_info(domain, virq + i, virq + i, &avec_irq_controller,
adata, handle_edge_irq, NULL, NULL);
irqd_set_single_target(irqd);
irqd_set_affinity_on_activate(irqd);
ret = avecintc_alloc_vector(adata);
if (ret < 0) {
kfree(adata);
return ret;
}
}
No?
> +static void clear_free_vector(struct irq_data *irqd)
> +{
> + struct avecintc_data *adata = irq_data_get_irq_chip_data(irqd);
> + bool managed = irqd_affinity_is_managed(irqd);
Don't even try. Your managed support is broken at the allocation side
and at several other places.
> + per_cpu(irq_map, adata->cpu)[adata->vec] = NULL;
> + irq_matrix_free(loongarch_avec.vector_matrix, adata->cpu, adata->vec, managed);
> + adata->cpu = UINT_MAX;
> + adata->vec = UINT_MAX;
> +
> +#ifdef CONFIG_SMP
> + if (!adata->moving)
> + return;
> +
> + per_cpu(irq_map, adata->prev_cpu)[adata->prev_vec] = NULL;
> + irq_matrix_free(loongarch_avec.vector_matrix,
> + adata->prev_cpu, adata->prev_vec, adata->managed);
> + adata->moving = 0;
> + adata->prev_vec = UINT_MAX;
> + adata->prev_cpu = UINT_MAX;
What's all the clearing for when you kfree() it two lines further down?
> + list_del_init(&adata->entry);
> +#endif
> + kfree(adata);
And no, not under the lock .... Move the locking into this function and
kfree() at the call site. There is zero reason to hold the lock over the
full loop.
> +static int __init pch_msi_parse_madt(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_msi_pic *pchmsi_entry = (struct acpi_madt_msi_pic *)header;
> +
> + msi_base_addr = pchmsi_entry->msg_address - AVEC_MSG_OFFSET;
> +
> + return pch_msi_acpi_init_v2(loongarch_avec.domain, pchmsi_entry);
> +}
...
> +int __init pch_msi_acpi_init_v2(struct irq_domain *parent, struct acpi_madt_msi_pic *acpi_pchmsi)
The second argument is required because?
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support
2024-08-20 16:29 ` Thomas Gleixner
@ 2024-08-21 1:47 ` Tianyang Zhang
2024-08-21 13:14 ` Huacai Chen
1 sibling, 0 replies; 9+ messages in thread
From: Tianyang Zhang @ 2024-08-21 1:47 UTC (permalink / raw)
To: Thomas Gleixner, corbet, alexs, chenhuacai, kernel, jiaxun.yang,
gaoliang, wangliupu, lvjianmin, yijun, mhocko, akpm, dianders,
maobibo, xry111, zhaotianrui, nathan, yangtiezhu, zhoubinbin
Cc: loongarch, linux-doc, linux-kernel, Huacai Chen
Hi, Thomas
在 2024/8/21 上午12:29, Thomas Gleixner 写道:
> On Thu, Aug 15 2024 at 19:26, Tianyang Zhang wrote:
>> .../arch/loongarch/irq-chip-model.rst | 32 ++
>> .../zh_CN/arch/loongarch/irq-chip-model.rst | 32 ++
>> arch/loongarch/Kconfig | 1 +
>> arch/loongarch/include/asm/cpu-features.h | 1 +
>> arch/loongarch/include/asm/cpu.h | 2 +
>> arch/loongarch/include/asm/hardirq.h | 3 +-
>> arch/loongarch/include/asm/hw_irq.h | 2 +
>> arch/loongarch/include/asm/irq.h | 25 +-
>> arch/loongarch/include/asm/loongarch.h | 18 +-
>> arch/loongarch/include/asm/smp.h | 2 +
>> arch/loongarch/kernel/cpu-probe.c | 3 +-
>> arch/loongarch/kernel/irq.c | 15 +-
>> arch/loongarch/kernel/paravirt.c | 5 +
>> arch/loongarch/kernel/smp.c | 6 +
>> drivers/irqchip/Makefile | 2 +-
>> drivers/irqchip/irq-loongarch-avec.c | 426 ++++++++++++++++++
>> drivers/irqchip/irq-loongarch-cpu.c | 5 +-
>> drivers/irqchip/irq-loongson-eiointc.c | 7 +-
>> drivers/irqchip/irq-loongson-pch-msi.c | 24 +-
>> include/linux/cpuhotplug.h | 3 +-
> This patch is doing too many things at once and is absolutely not
> reviewable.
>
> Please split it up into the obvious bits and pieces:
>
> 1) The IRQ_NOPROBE change
>
> 2) See below
>
> 3) Documentation
>
> 4) Add the arch/loongson parts, i.e. all the defines and
> basic required function prototypes with a little twist.
> Add a Kconfig symbol:
>
> Kconfig IRQ_LOONGARCH_AVEC
> bool
>
> in drivers/irqchip/Kconfig. This allows you to add all
> arch/loongarch/ changes with the simple tweak:
>
> #ifdef CONFIG_IRQ_LOONGARCH_AVEC
> # define cpu_has_avecint cpu_opt(LOONGARCH_CPU_AVECINT)
> #else
> # define cpu_has_avecint false
> #endif
>
> and
>
> #ifdef CONFIG_IRQ_LOONGARCH_AVEC
> # define SMP_CLEAR_VECTOR BIT(ACTION_CLEAR_VECTOR)
> #else
> # define SMP_CLEAR_VECTOR (0)
> #endif
>
> That way the compiler will optimize out stuff like the
> SMP_CLEAR_VECTOR handling and you only need the prototype of
> complete_irq_moving(), but no implementation.
>
> 5) Change the CPU hotplug callback for EOINTC and do
> the acpi_cascade_irqdomain_init() change.
>
> 6) Prepare get_pch_msi_handle() in the pch MSI driver
>
> 7) Implement the driver and select IRQ_LOONGARCH_AVEC
> from IRQ_LOONGARCH_CPU
>
> 8) Remove the IRQ_LOONGARCH_AVEC helpers
Thanks for your guiding, we will complete the split as required
>
>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>> index 70f169210b52..0e3abf7b0bd3 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -85,6 +85,7 @@ config LOONGARCH
>> select GENERIC_ENTRY
>> select GENERIC_GETTIMEOFDAY
>> select GENERIC_IOREMAP if !ARCH_IOREMAP
>> + select GENERIC_IRQ_MATRIX_ALLOCATOR
> Please move this to IRQ_LOONGARCH_CPU in patch #7
OK, thanks
>
>> @@ -92,15 +103,21 @@ int liointc_acpi_init(struct irq_domain *parent,
>> struct acpi_madt_lio_pic *acpi_liointc);
>> int eiointc_acpi_init(struct irq_domain *parent,
>> struct acpi_madt_eio_pic *acpi_eiointc);
>> +int avecintc_acpi_init(struct irq_domain *parent);
>> +
>> +void complete_irq_moving(void);
>>
>> int htvec_acpi_init(struct irq_domain *parent,
>> struct acpi_madt_ht_pic *acpi_htvec);
>> int pch_lpc_acpi_init(struct irq_domain *parent,
>> struct acpi_madt_lpc_pic *acpi_pchlpc);
>> -int pch_msi_acpi_init(struct irq_domain *parent,
>> - struct acpi_madt_msi_pic *acpi_pchmsi);
>> int pch_pic_acpi_init(struct irq_domain *parent,
>> struct acpi_madt_bio_pic *acpi_pchpic);
>> +int pch_msi_acpi_init(struct irq_domain *parent,
>> + struct acpi_madt_msi_pic *acpi_pchmsi);
>> +int pch_msi_acpi_init_v2(struct irq_domain *parent,
>> + struct acpi_madt_msi_pic *acpi_pchmsi);
> This is really the wrong place for all these prototypes. They are only
> used in drivers/irqchip/... except for complete_irq_moving().
>
> So the proper place for them is drivers/irqchip/irq-loongarch.h
>
> Move them there. This is patch #2 which I referred to above.
Ok ,thanks
>
>> +static phys_addr_t msi_base_addr;
>>
> So you have everything related to the avec chip in loongarch_avec, so
> why don't you move that into that data structure?
Ok, thanks
>
>> It just looks more compatible with the previous one. It really doesn't make sense. We will delete it later+struct avecintc_chip {
>> + struct fwnode_handle *fwnode;
>> + struct irq_domain *domain;
>> + struct irq_matrix *vector_matrix;
>> + raw_spinlock_t lock;
>> +};
> The lock should be the first member as spinlocks have alignment
> requirements....
Ok ,thanks
> It just looks more compatible with the previous one. It really doesn't make sense. We will delete it later
>
>> +static int avecintc_domain_alloc(struct irq_domain *domain,
>> + unsigned int virq, unsigned int nr_irqs, void *arg)
>> +{
>> + guard(raw_spinlock_irqsave)(&loongarch_avec.lock);
>> +
>> + for (unsigned int i = 0; i < nr_irqs; i++) {
>> + struct irq_data *irqd = irq_domain_get_irq_data(domain, virq + i);
>> + struct avecintc_data *adata = kzalloc(sizeof(*adata), GFP_KERNEL);
> That was never tested with any debug. You _cannot_ do a GFP_KERNEL
> allocation with the raw spinlock held. And no, don't use
> GFP_ATOMIC. There is absolutely zero reason to hold the lock accross all
> of that. As you got your ideas from x86_vector_alloc_irqs(), you could
> have looked at how that's done correctly.
I got it , thanks
>
>> + unsigned int cpu, ret;
>> +
>> + if (!adata)
>> + return -ENOMEM;
>> +
>> + ret = irq_matrix_alloc(loongarch_avec.vector_matrix, cpu_online_mask, false, &cpu);
>> + if (ret < 0) {
>> + kfree(adata);
>> + return ret;
>> + }
>> +
>> + adata->moving = 0;
> Redundant. The struct is allocated with kzalloc()...
sorry, this is my stupid mistake ..... ,thanks
>
>> + adata->prev_cpu = adata->cpu = cpu;
>> + adata->prev_vec = adata->vec = ret;
>> + adata->managed = irqd_affinity_is_managed(irqd);
> If you want to support managed interrupts, then you cannot allocate
> from the CPU online mask. See x86...
Ok, we will reconsider these functions, thanks
>
>> + irq_domain_set_info(domain, virq + i, virq + i, &avec_irq_controller,
>> + adata, handle_edge_irq, NULL, NULL);
>> + irqd_set_single_target(irqd);
>> + irqd_set_affinity_on_activate(irqd);
>> +
>> + per_cpu_ptr(irq_map, adata->cpu)[adata->vec] = irq_data_to_desc(irqd);
> static int avecintc_alloc_vector(struct avecintc_adata *adata)
> {
> int ret, cpu;
>
> guard(raw_spinlock_irqsave)(&loongarch_avec.lock);
> ret = irq_matrix_alloc(loongarch_avec.vector_matrix, cpu_online_mask, false, &cpu);
> if (ret < 0)
> return ret;
>
> adata->prev_cpu = adata->cpu = cpu;
> adata->prev_vec = adata->vec = ret;
> per_cpu_ptr(irq_map, adata->cpu)[adata->vec] = irq_data_to_desc(irqd);
> return 0;
> }
>
> static int avecintc_domain_alloc(struct irq_domain *domain, ...)
> {
> for (unsigned int i = 0; i < nr_irqs; i++) {
> struct irq_data *irqd = irq_domain_get_irq_data(domain, virq + i);
> struct avecintc_data *adata = kzalloc(sizeof(*adata), GFP_KERNEL);
> int ret;
>
> if (!adata)
> return -ENOMEM;
>
> irq_domain_set_info(domain, virq + i, virq + i, &avec_irq_controller,
> adata, handle_edge_irq, NULL, NULL);
> irqd_set_single_target(irqd);
> irqd_set_affinity_on_activate(irqd);
>
> ret = avecintc_alloc_vector(adata);
> if (ret < 0) {
> kfree(adata);
> return ret;
> }
> }
> No?
OK , It really seems more appropriate, thanks
> It just looks more compatible with the previous one. It really doesn't make sense. We will delete it later
>
>> +static void clear_free_vector(struct irq_data *irqd)
>> +{
>> + struct avecintc_data *adata = irq_data_get_irq_chip_data(irqd);
>> + bool managed = irqd_affinity_is_managed(irqd);
> Don't even try. Your managed support is broken at the allocation side
> and at several other places.
OK, thanks
>> + per_cpu(irq_map, adata->cpu)[adata->vec] = NULL;
>> + irq_matrix_free(loongarch_avec.vector_matrix, adata->cpu, adata->vec, managed);
>> + adata->cpu = UINT_MAX;
>> + adata->vec = UINT_MAX;
>> +
>> +#ifdef CONFIG_SMP
>> + if (!adata->moving)
>> + return;
>> +
>> + per_cpu(irq_map, adata->prev_cpu)[adata->prev_vec] = NULL;
>> + irq_matrix_free(loongarch_avec.vector_matrix,
>> + adata->prev_cpu, adata->prev_vec, adata->managed);
>> + adata->moving = 0;
>> + adata->prev_vec = UINT_MAX;
>> + adata->prev_cpu = UINT_MAX;
> What's all the clearing for when you kfree() it two lines further down?
OK, we will reconsider there ,thanks
>
>> + list_del_init(&adata->entry);
>> +#endif
>> + kfree(adata);
> And no, not under the lock .... Move the locking into this function and
> kfree() at the call site. There is zero reason to hold the lock over the
> full loop.
Ok ,thanks
>> +static int __init pch_msi_parse_madt(union acpi_subtable_headers *header,
>> + const unsigned long end)
>> +{
>> + struct acpi_madt_msi_pic *pchmsi_entry = (struct acpi_madt_msi_pic *)header;
>> +
>> + msi_base_addr = pchmsi_entry->msg_address - AVEC_MSG_OFFSET;
>> +
>> + return pch_msi_acpi_init_v2(loongarch_avec.domain, pchmsi_entry);
>> +}
> ...
>
>> +int __init pch_msi_acpi_init_v2(struct irq_domain *parent, struct acpi_madt_msi_pic *acpi_pchmsi)
> The second argument is required because?
It just looks more compatible with the previous one. It really doesn't
make sense. We will delete it later, Thanks
>
> Thanks,
>
> tglx
Thanks again
Tianyang
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support
2024-08-20 16:29 ` Thomas Gleixner
2024-08-21 1:47 ` Tianyang Zhang
@ 2024-08-21 13:14 ` Huacai Chen
2024-08-21 14:31 ` Thomas Gleixner
1 sibling, 1 reply; 9+ messages in thread
From: Huacai Chen @ 2024-08-21 13:14 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Tianyang Zhang, corbet, alexs, kernel, jiaxun.yang, gaoliang,
wangliupu, lvjianmin, yijun, mhocko, akpm, dianders, maobibo,
xry111, zhaotianrui, nathan, yangtiezhu, zhoubinbin, loongarch,
linux-doc, linux-kernel, Huacai Chen
Hi, Thomas,
Just some complements for Tianyang.
On Wed, Aug 21, 2024 at 12:29 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Thu, Aug 15 2024 at 19:26, Tianyang Zhang wrote:
> > .../arch/loongarch/irq-chip-model.rst | 32 ++
> > .../zh_CN/arch/loongarch/irq-chip-model.rst | 32 ++
> > arch/loongarch/Kconfig | 1 +
> > arch/loongarch/include/asm/cpu-features.h | 1 +
> > arch/loongarch/include/asm/cpu.h | 2 +
> > arch/loongarch/include/asm/hardirq.h | 3 +-
> > arch/loongarch/include/asm/hw_irq.h | 2 +
> > arch/loongarch/include/asm/irq.h | 25 +-
> > arch/loongarch/include/asm/loongarch.h | 18 +-
> > arch/loongarch/include/asm/smp.h | 2 +
> > arch/loongarch/kernel/cpu-probe.c | 3 +-
> > arch/loongarch/kernel/irq.c | 15 +-
> > arch/loongarch/kernel/paravirt.c | 5 +
> > arch/loongarch/kernel/smp.c | 6 +
> > drivers/irqchip/Makefile | 2 +-
> > drivers/irqchip/irq-loongarch-avec.c | 426 ++++++++++++++++++
> > drivers/irqchip/irq-loongarch-cpu.c | 5 +-
> > drivers/irqchip/irq-loongson-eiointc.c | 7 +-
> > drivers/irqchip/irq-loongson-pch-msi.c | 24 +-
> > include/linux/cpuhotplug.h | 3 +-
>
> This patch is doing too many things at once and is absolutely not
> reviewable.
>
> Please split it up into the obvious bits and pieces:
Splitting may cause another problem: some patches will get upstream
via the arch tree and others via the irq tree. These dependencies may
cause build errors in a certain tree. But anyway, we will try our best
to do this.
>
> 1) The IRQ_NOPROBE change
>
> 2) See below
>
> 3) Documentation
>
> 4) Add the arch/loongson parts, i.e. all the defines and
> basic required function prototypes with a little twist.
> Add a Kconfig symbol:
>
> Kconfig IRQ_LOONGARCH_AVEC
> bool
>
> in drivers/irqchip/Kconfig. This allows you to add all
> arch/loongarch/ changes with the simple tweak:
>
> #ifdef CONFIG_IRQ_LOONGARCH_AVEC
> # define cpu_has_avecint cpu_opt(LOONGARCH_CPU_AVECINT)
> #else
> # define cpu_has_avecint false
> #endif
>
> and
>
> #ifdef CONFIG_IRQ_LOONGARCH_AVEC
> # define SMP_CLEAR_VECTOR BIT(ACTION_CLEAR_VECTOR)
> #else
> # define SMP_CLEAR_VECTOR (0)
> #endif
>
> That way the compiler will optimize out stuff like the
> SMP_CLEAR_VECTOR handling and you only need the prototype of
> complete_irq_moving(), but no implementation.
These macros are not in hot-path, and we have already tried our best
to avoid using #ifdefs for cpu_has_xxx, so I suggest not introduce a
new Kconfig option. Moreover, the new option should always be selected
due to the deep coupling among loongson's irqchips, which makes the
#ifdefs useless.
>
> 5) Change the CPU hotplug callback for EOINTC and do
> the acpi_cascade_irqdomain_init() change.
>
> 6) Prepare get_pch_msi_handle() in the pch MSI driver
>
> 7) Implement the driver and select IRQ_LOONGARCH_AVEC
> from IRQ_LOONGARCH_CPU
>
> 8) Remove the IRQ_LOONGARCH_AVEC helpers
>
> > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> > index 70f169210b52..0e3abf7b0bd3 100644
> > --- a/arch/loongarch/Kconfig
> > +++ b/arch/loongarch/Kconfig
> > @@ -85,6 +85,7 @@ config LOONGARCH
> > select GENERIC_ENTRY
> > select GENERIC_GETTIMEOFDAY
> > select GENERIC_IOREMAP if !ARCH_IOREMAP
> > + select GENERIC_IRQ_MATRIX_ALLOCATOR
>
> Please move this to IRQ_LOONGARCH_CPU in patch #7
>
> > @@ -92,15 +103,21 @@ int liointc_acpi_init(struct irq_domain *parent,
> > struct acpi_madt_lio_pic *acpi_liointc);
> > int eiointc_acpi_init(struct irq_domain *parent,
> > struct acpi_madt_eio_pic *acpi_eiointc);
> > +int avecintc_acpi_init(struct irq_domain *parent);
> > +
> > +void complete_irq_moving(void);
> >
> > int htvec_acpi_init(struct irq_domain *parent,
> > struct acpi_madt_ht_pic *acpi_htvec);
> > int pch_lpc_acpi_init(struct irq_domain *parent,
> > struct acpi_madt_lpc_pic *acpi_pchlpc);
> > -int pch_msi_acpi_init(struct irq_domain *parent,
> > - struct acpi_madt_msi_pic *acpi_pchmsi);
> > int pch_pic_acpi_init(struct irq_domain *parent,
> > struct acpi_madt_bio_pic *acpi_pchpic);
> > +int pch_msi_acpi_init(struct irq_domain *parent,
> > + struct acpi_madt_msi_pic *acpi_pchmsi);
> > +int pch_msi_acpi_init_v2(struct irq_domain *parent,
> > + struct acpi_madt_msi_pic *acpi_pchmsi);
>
> This is really the wrong place for all these prototypes. They are only
> used in drivers/irqchip/... except for complete_irq_moving().
>
> So the proper place for them is drivers/irqchip/irq-loongarch.h
irq-loongson.h is a little better because those drivers are shared by
MIPS and LoongArch.
>
> Move them there. This is patch #2 which I referred to above.
>
> >
> > +static phys_addr_t msi_base_addr;
> >
>
> So you have everything related to the avec chip in loongarch_avec, so
> why don't you move that into that data structure?
>
> > +struct avecintc_chip {
> > + struct fwnode_handle *fwnode;
> > + struct irq_domain *domain;
> > + struct irq_matrix *vector_matrix;
> > + raw_spinlock_t lock;
> > +};
>
> The lock should be the first member as spinlocks have alignment
> requirements....
>
> > +static int avecintc_domain_alloc(struct irq_domain *domain,
> > + unsigned int virq, unsigned int nr_irqs, void *arg)
> > +{
> > + guard(raw_spinlock_irqsave)(&loongarch_avec.lock);
> > +
> > + for (unsigned int i = 0; i < nr_irqs; i++) {
> > + struct irq_data *irqd = irq_domain_get_irq_data(domain, virq + i);
> > + struct avecintc_data *adata = kzalloc(sizeof(*adata), GFP_KERNEL);
>
> That was never tested with any debug. You _cannot_ do a GFP_KERNEL
> allocation with the raw spinlock held. And no, don't use
> GFP_ATOMIC. There is absolutely zero reason to hold the lock accross all
> of that. As you got your ideas from x86_vector_alloc_irqs(), you could
> have looked at how that's done correctly.
>
> > + unsigned int cpu, ret;
> > +
> > + if (!adata)
> > + return -ENOMEM;
> > +
> > + ret = irq_matrix_alloc(loongarch_avec.vector_matrix, cpu_online_mask, false, &cpu);
> > + if (ret < 0) {
> > + kfree(adata);
> > + return ret;
> > + }
> > +
> > + adata->moving = 0;
>
> Redundant. The struct is allocated with kzalloc()...
>
> > + adata->prev_cpu = adata->cpu = cpu;
> > + adata->prev_vec = adata->vec = ret;
> > + adata->managed = irqd_affinity_is_managed(irqd);
>
> If you want to support managed interrupts, then you cannot allocate
> from the CPU online mask. See x86...
>
> > + irq_domain_set_info(domain, virq + i, virq + i, &avec_irq_controller,
> > + adata, handle_edge_irq, NULL, NULL);
> > + irqd_set_single_target(irqd);
> > + irqd_set_affinity_on_activate(irqd);
> > +
> > + per_cpu_ptr(irq_map, adata->cpu)[adata->vec] = irq_data_to_desc(irqd);
>
> static int avecintc_alloc_vector(struct avecintc_adata *adata)
> {
> int ret, cpu;
>
> guard(raw_spinlock_irqsave)(&loongarch_avec.lock);
> ret = irq_matrix_alloc(loongarch_avec.vector_matrix, cpu_online_mask, false, &cpu);
> if (ret < 0)
> return ret;
>
> adata->prev_cpu = adata->cpu = cpu;
> adata->prev_vec = adata->vec = ret;
> per_cpu_ptr(irq_map, adata->cpu)[adata->vec] = irq_data_to_desc(irqd);
> return 0;
> }
>
> static int avecintc_domain_alloc(struct irq_domain *domain, ...)
> {
> for (unsigned int i = 0; i < nr_irqs; i++) {
> struct irq_data *irqd = irq_domain_get_irq_data(domain, virq + i);
> struct avecintc_data *adata = kzalloc(sizeof(*adata), GFP_KERNEL);
> int ret;
>
> if (!adata)
> return -ENOMEM;
>
> irq_domain_set_info(domain, virq + i, virq + i, &avec_irq_controller,
> adata, handle_edge_irq, NULL, NULL);
> irqd_set_single_target(irqd);
> irqd_set_affinity_on_activate(irqd);
>
> ret = avecintc_alloc_vector(adata);
> if (ret < 0) {
> kfree(adata);
> return ret;
> }
> }
> No?
>
> > +static void clear_free_vector(struct irq_data *irqd)
> > +{
> > + struct avecintc_data *adata = irq_data_get_irq_chip_data(irqd);
> > + bool managed = irqd_affinity_is_managed(irqd);
>
> Don't even try. Your managed support is broken at the allocation side
> and at several other places.
I'm a bit confused here, irq_create_affinity_masks() marks some
interrupts "managed". So if we completely ignore "managed" here, then
can irq_create_affinity_masks() still work? Or the two has nothing to
do with each other?
Huacai
>
> > + per_cpu(irq_map, adata->cpu)[adata->vec] = NULL;
> > + irq_matrix_free(loongarch_avec.vector_matrix, adata->cpu, adata->vec, managed);
> > + adata->cpu = UINT_MAX;
> > + adata->vec = UINT_MAX;
> > +
> > +#ifdef CONFIG_SMP
> > + if (!adata->moving)
> > + return;
> > +
> > + per_cpu(irq_map, adata->prev_cpu)[adata->prev_vec] = NULL;
> > + irq_matrix_free(loongarch_avec.vector_matrix,
> > + adata->prev_cpu, adata->prev_vec, adata->managed);
> > + adata->moving = 0;
> > + adata->prev_vec = UINT_MAX;
> > + adata->prev_cpu = UINT_MAX;
>
> What's all the clearing for when you kfree() it two lines further down?
>
> > + list_del_init(&adata->entry);
> > +#endif
> > + kfree(adata);
>
> And no, not under the lock .... Move the locking into this function and
> kfree() at the call site. There is zero reason to hold the lock over the
> full loop.
>
> > +static int __init pch_msi_parse_madt(union acpi_subtable_headers *header,
> > + const unsigned long end)
> > +{
> > + struct acpi_madt_msi_pic *pchmsi_entry = (struct acpi_madt_msi_pic *)header;
> > +
> > + msi_base_addr = pchmsi_entry->msg_address - AVEC_MSG_OFFSET;
> > +
> > + return pch_msi_acpi_init_v2(loongarch_avec.domain, pchmsi_entry);
> > +}
>
> ...
>
> > +int __init pch_msi_acpi_init_v2(struct irq_domain *parent, struct acpi_madt_msi_pic *acpi_pchmsi)
>
> The second argument is required because?
>
> Thanks,
>
> tglx
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support
2024-08-21 13:14 ` Huacai Chen
@ 2024-08-21 14:31 ` Thomas Gleixner
2024-08-22 11:53 ` Huacai Chen
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2024-08-21 14:31 UTC (permalink / raw)
To: Huacai Chen
Cc: Tianyang Zhang, corbet, alexs, kernel, jiaxun.yang, gaoliang,
wangliupu, lvjianmin, yijun, mhocko, akpm, dianders, maobibo,
xry111, zhaotianrui, nathan, yangtiezhu, zhoubinbin, loongarch,
linux-doc, linux-kernel, Huacai Chen
On Wed, Aug 21 2024 at 21:14, Huacai Chen wrote:
> On Wed, Aug 21, 2024 at 12:29 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>> This patch is doing too many things at once and is absolutely not
>> reviewable.
>>
>> Please split it up into the obvious bits and pieces:
> Splitting may cause another problem: some patches will get upstream
> via the arch tree and others via the irq tree. These dependencies may
> cause build errors in a certain tree. But anyway, we will try our best
> to do this.
That's not a problem at all. The trivial way to solve this is to apply
the architecture changes to the loongarch tree in a separate branch
which is based of some -rcX tag and only contains those dependencies.
That branch is then merged into the main loongarch branch and I can pull
it in to my tree for adding the irqchip changes. No conflicts, no merge
dependencies, nothing.
>> #ifdef CONFIG_IRQ_LOONGARCH_AVEC
>> # define SMP_CLEAR_VECTOR BIT(ACTION_CLEAR_VECTOR)
>> #else
>> # define SMP_CLEAR_VECTOR (0)
>> #endif
>>
>> That way the compiler will optimize out stuff like the
>> SMP_CLEAR_VECTOR handling and you only need the prototype of
>> complete_irq_moving(), but no implementation.
> These macros are not in hot-path, and we have already tried our best
> to avoid using #ifdefs for cpu_has_xxx, so I suggest not introduce a
> new Kconfig option. Moreover, the new option should always be selected
> due to the deep coupling among loongson's irqchips, which makes the
> #ifdefs useless.
They are removed in step 8 again. It's for having a sanely split up and
structured patch series instead of one big lump.
>> > +static void clear_free_vector(struct irq_data *irqd)
>> > +{
>> > + struct avecintc_data *adata = irq_data_get_irq_chip_data(irqd);
>> > + bool managed = irqd_affinity_is_managed(irqd);
>>
>> Don't even try. Your managed support is broken at the allocation side
>> and at several other places.
> I'm a bit confused here, irq_create_affinity_masks() marks some
> interrupts "managed". So if we completely ignore "managed" here, then
> can irq_create_affinity_masks() still work? Or the two has nothing to
> do with each other?
Managed interrupts have the property that the core and irqchip code
guarantees the interrupts to be affinable to the CPU masks which are
handed in to the allocator for them.
So the requirement for architectures which have a limited number of
vectors per CPU (x86, loongarch) is that you have to reserve the managed
vectors right at allocation time.
x86_vector_alloc_irqs()
assign_irq_vector_policy()
if (managed)
reserve_managed_vector()
irq_matrix_reserve_managed(mask)
irq_matrix_reserve_managed() then reserves a vector on all CPUs which
are in the affinity mask.
On activation:
x86_vector_activate()
if (managed)
activate_managed()
assign_managed_vector(mask)
irq_matrix_alloc_managed(mask)
irq_matrix_alloc_managed() then picks an unassigned vector out of the
managed vector space. Similar mechanism when the affinity is set.
Why is this important for x86 (and loongarch)?
Because both have a limited vector space of 256 vectors per CPU, where
some of the vectors might be reserved for exceptions and OS purposes or
the legacy space.
The managed mechanism guarantees at allocation time that the interrupt
will have a reserved vector on all CPUs in the mask. That ensures that
on CPU hotplug the interrupt can be migrated over to a still online CPU
in the mask. If the last CPU goes offline the interrupt is shut down.
You might not yet have run into the situation of vector exhaustion, but
once your number of CPUs gets big enough that is guaranteed to happen.
That's why x86 also uses the concept of reserved (not guaranteed)
regular vectors for non-managed interrupts. x86 uses the spurious vector
for that. That's important because there are enough device drivers out
there which allocate a gazillion of interrupts at probe time, but only
request a few of them.
If you allocate all vectors for them right upfront, then you exhaust
your vector space quickly for no reason. Only when the interrupt is
requested then a usable vector is allocated - or the allocation fails
and request_irq() fails. That's better than exhausting the vector space
for nothing.
The complexity of the x86 allocation/activate/set_affinity mechanisms
is there for a reason and not just because we did not have anything
better to do. :)
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support
2024-08-21 14:31 ` Thomas Gleixner
@ 2024-08-22 11:53 ` Huacai Chen
2024-08-22 20:52 ` Thomas Gleixner
0 siblings, 1 reply; 9+ messages in thread
From: Huacai Chen @ 2024-08-22 11:53 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Tianyang Zhang, corbet, alexs, kernel, jiaxun.yang, gaoliang,
wangliupu, lvjianmin, yijun, mhocko, akpm, dianders, maobibo,
xry111, zhaotianrui, nathan, yangtiezhu, zhoubinbin, loongarch,
linux-doc, linux-kernel, Huacai Chen
On Wed, Aug 21, 2024 at 10:31 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Wed, Aug 21 2024 at 21:14, Huacai Chen wrote:
> > On Wed, Aug 21, 2024 at 12:29 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> >> This patch is doing too many things at once and is absolutely not
> >> reviewable.
> >>
> >> Please split it up into the obvious bits and pieces:
> > Splitting may cause another problem: some patches will get upstream
> > via the arch tree and others via the irq tree. These dependencies may
> > cause build errors in a certain tree. But anyway, we will try our best
> > to do this.
>
> That's not a problem at all. The trivial way to solve this is to apply
> the architecture changes to the loongarch tree in a separate branch
> which is based of some -rcX tag and only contains those dependencies.
> That branch is then merged into the main loongarch branch and I can pull
> it in to my tree for adding the irqchip changes. No conflicts, no merge
> dependencies, nothing.
Emm, another way is apply all patches to the irq tree with my Acked-by.
>
> >> #ifdef CONFIG_IRQ_LOONGARCH_AVEC
> >> # define SMP_CLEAR_VECTOR BIT(ACTION_CLEAR_VECTOR)
> >> #else
> >> # define SMP_CLEAR_VECTOR (0)
> >> #endif
> >>
> >> That way the compiler will optimize out stuff like the
> >> SMP_CLEAR_VECTOR handling and you only need the prototype of
> >> complete_irq_moving(), but no implementation.
> > These macros are not in hot-path, and we have already tried our best
> > to avoid using #ifdefs for cpu_has_xxx, so I suggest not introduce a
> > new Kconfig option. Moreover, the new option should always be selected
> > due to the deep coupling among loongson's irqchips, which makes the
> > #ifdefs useless.
>
> They are removed in step 8 again. It's for having a sanely split up and
> structured patch series instead of one big lump.
I see, but I'm trying another splitting way to avoid
adding-and-then-removing, of course it should also make reviews easy.
>
> >> > +static void clear_free_vector(struct irq_data *irqd)
> >> > +{
> >> > + struct avecintc_data *adata = irq_data_get_irq_chip_data(irqd);
> >> > + bool managed = irqd_affinity_is_managed(irqd);
> >>
> >> Don't even try. Your managed support is broken at the allocation side
> >> and at several other places.
> > I'm a bit confused here, irq_create_affinity_masks() marks some
> > interrupts "managed". So if we completely ignore "managed" here, then
> > can irq_create_affinity_masks() still work? Or the two has nothing to
> > do with each other?
>
> Managed interrupts have the property that the core and irqchip code
> guarantees the interrupts to be affinable to the CPU masks which are
> handed in to the allocator for them.
>
> So the requirement for architectures which have a limited number of
> vectors per CPU (x86, loongarch) is that you have to reserve the managed
> vectors right at allocation time.
>
> x86_vector_alloc_irqs()
> assign_irq_vector_policy()
> if (managed)
> reserve_managed_vector()
> irq_matrix_reserve_managed(mask)
>
> irq_matrix_reserve_managed() then reserves a vector on all CPUs which
> are in the affinity mask.
>
> On activation:
>
> x86_vector_activate()
> if (managed)
> activate_managed()
> assign_managed_vector(mask)
> irq_matrix_alloc_managed(mask)
>
> irq_matrix_alloc_managed() then picks an unassigned vector out of the
> managed vector space. Similar mechanism when the affinity is set.
>
> Why is this important for x86 (and loongarch)?
>
> Because both have a limited vector space of 256 vectors per CPU, where
> some of the vectors might be reserved for exceptions and OS purposes or
> the legacy space.
>
> The managed mechanism guarantees at allocation time that the interrupt
> will have a reserved vector on all CPUs in the mask. That ensures that
> on CPU hotplug the interrupt can be migrated over to a still online CPU
> in the mask. If the last CPU goes offline the interrupt is shut down.
>
> You might not yet have run into the situation of vector exhaustion, but
> once your number of CPUs gets big enough that is guaranteed to happen.
>
> That's why x86 also uses the concept of reserved (not guaranteed)
> regular vectors for non-managed interrupts. x86 uses the spurious vector
> for that. That's important because there are enough device drivers out
> there which allocate a gazillion of interrupts at probe time, but only
> request a few of them.
>
> If you allocate all vectors for them right upfront, then you exhaust
> your vector space quickly for no reason. Only when the interrupt is
> requested then a usable vector is allocated - or the allocation fails
> and request_irq() fails. That's better than exhausting the vector space
> for nothing.
>
> The complexity of the x86 allocation/activate/set_affinity mechanisms
> is there for a reason and not just because we did not have anything
> better to do. :)
Frankly, I haven't absorbed everything here, but I think I can try to
answer my question "can irq_create_affinity_masks() still work".
irq_create_affinity_masks() can still mark interrupts "managed" if
avecintc driver doesn't support "managed", but it cannot guarantee
that set_affinity can always succeed. If the destination cpu has a
free vector, then set_affinity succeeds, otherwise it will fail. But
if avecintc driver supports "managed", set_affinity can always
succeed, because the destination cpu has already reserved a vector for
this. Am I right?
Huacai
>
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support
2024-08-22 11:53 ` Huacai Chen
@ 2024-08-22 20:52 ` Thomas Gleixner
0 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2024-08-22 20:52 UTC (permalink / raw)
To: Huacai Chen
Cc: Tianyang Zhang, corbet, alexs, kernel, jiaxun.yang, gaoliang,
wangliupu, lvjianmin, yijun, mhocko, akpm, dianders, maobibo,
xry111, zhaotianrui, nathan, yangtiezhu, zhoubinbin, loongarch,
linux-doc, linux-kernel, Huacai Chen
Huacai!
On Thu, Aug 22 2024 at 19:53, Huacai Chen wrote:
> On Wed, Aug 21, 2024 at 10:31 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> On Wed, Aug 21 2024 at 21:14, Huacai Chen wrote:
>> > On Wed, Aug 21, 2024 at 12:29 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>> >> This patch is doing too many things at once and is absolutely not
>> >> reviewable.
>> >>
>> >> Please split it up into the obvious bits and pieces:
>> > Splitting may cause another problem: some patches will get upstream
>> > via the arch tree and others via the irq tree. These dependencies may
>> > cause build errors in a certain tree. But anyway, we will try our best
>> > to do this.
>>
>> That's not a problem at all. The trivial way to solve this is to apply
>> the architecture changes to the loongarch tree in a separate branch
>> which is based of some -rcX tag and only contains those dependencies.
>> That branch is then merged into the main loongarch branch and I can pull
>> it in to my tree for adding the irqchip changes. No conflicts, no merge
>> dependencies, nothing.
> Emm, another way is apply all patches to the irq tree with my Acked-by.
Correct, but that has the potential of creating conflicts when the
loongarch tree grows changes in the same areas.
>> > These macros are not in hot-path, and we have already tried our best
>> > to avoid using #ifdefs for cpu_has_xxx, so I suggest not introduce a
>> > new Kconfig option. Moreover, the new option should always be selected
>> > due to the deep coupling among loongson's irqchips, which makes the
>> > #ifdefs useless.
>>
>> They are removed in step 8 again. It's for having a sanely split up and
>> structured patch series instead of one big lump.
> I see, but I'm trying another splitting way to avoid
> adding-and-then-removing, of course it should also make reviews easy.
That's the whole point of the exercise.
>> The complexity of the x86 allocation/activate/set_affinity mechanisms
>> is there for a reason and not just because we did not have anything
>> better to do. :)
>
> Frankly, I haven't absorbed everything here, but I think I can try to
> answer my question "can irq_create_affinity_masks() still work".
>
> irq_create_affinity_masks() can still mark interrupts "managed" if
> avecintc driver doesn't support "managed", but it cannot guarantee
> that set_affinity can always succeed. If the destination cpu has a
> free vector, then set_affinity succeeds, otherwise it will fail. But
> if avecintc driver supports "managed", set_affinity can always
> succeed, because the destination cpu has already reserved a vector for
> this. Am I right?
It can work by some definition of "works", but in case of vector
exhaustion it will fail which is contrary to the purpose of managed
interrupts.
The matrix allocator already provides all the infrastructure and the
x86 reference implementation does the right thing. So why do you want to
shortcut that and make loongarch a special snowflake?
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-08-22 20:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-15 11:26 [PATCH v10 0/2] Loongarch-avec support Tianyang Zhang
2024-08-15 11:26 ` [PATCH v10 1/2] irqchip/loongson-pch-msi: Switch to MSI parent domains Tianyang Zhang
2024-08-15 11:26 ` [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support Tianyang Zhang
2024-08-20 16:29 ` Thomas Gleixner
2024-08-21 1:47 ` Tianyang Zhang
2024-08-21 13:14 ` Huacai Chen
2024-08-21 14:31 ` Thomas Gleixner
2024-08-22 11:53 ` Huacai Chen
2024-08-22 20:52 ` Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).