Linux RISC-V AIA Support
Subsystems (7)
v3 cover Linux RISC-V AIA Support
The RISC-V AIA specification is now frozen as-per the RISC-V international process. The latest frozen specifcation can be found at:
https://github.com/riscv/riscv-aia/releases/download/1.0-RC1/riscv-interrupts-1.0-RC1.pdf
At a high-level, the AIA specification adds three things:
1) AIA CSRs
- Improved local interrupt support
2) Incoming Message Signaled Interrupt Controller (IMSIC)
- Per-HART MSI controller
- Support MSI virtualization
- Support IPI along with virtualization
3) Advanced Platform-Level Interrupt Controller (APLIC)
- Wired interrupt controller
- In MSI-mode, converts wired interrupt into MSIs (i.e. MSI generator)
- In Direct-mode, injects external interrupts directly into HARTs
For an overview of the AIA specification, refer the recent AIA virtualization talk at KVM Forum 2022:
https://static.sched.com/hosted_files/kvmforum2022/a1/AIA_Virtualization_in_KVM_RISCV_final.pdf
https://www.youtube.com/watch?v=r071dL8Z0yo
The PATCH3 of this series conflicts with the "irqchip/riscv-intc: Add ACPI support" patch of the "Add basic ACPI support for RISC-V" series.
(Refer, https://lore.kernel.org/linux-riscv/[email protected]/)
To test this series, use QEMU v7.2 (or higher) and OpenSBI v1.2 (or higher).
These patches can also be found in the riscv_aia_v3 branch at:
https://github.com/avpatel/linux.git
Changes since v2:
- Rebased on Linux-6.4-rc1
- Addressed Rob's comments on DT bindings patches 4 and 8.
- Addessed Marc's comments on IMSIC driver PATCH5
- Replaced use of OF apis in APLIC and IMSIC drivers with FWNODE apis this makes both drivers easily portable for ACPI support. This also removes unnecessary indirection from the APLIC and IMSIC drivers.
- PATCH1 is a new patch for portability with ACPI support
- PATCH2 is a new patch to fix probing in APLIC drivers for APLIC-only systems.
- PATCH7 is a new patch which addresses the IOMMU DMA domain issues pointed out by SiFive
Changes since v1:
- Rebased on Linux-6.2-rc2
- Addressed comments on IMSIC DT bindings for PATCH4
- Use raw_spin_lock_irqsave() on ids_lock for PATCH5
- Improved MMIO alignment checks in PATCH5 to allow MMIO regions with holes.
- Addressed comments on APLIC DT bindings for PATCH6
- Fixed warning splat in aplic_msi_write_msg() caused by zeroed MSI message in PATCH7
- Dropped DT property riscv,slow-ipi instead will have module parameter in future.
Anup Patel (11):
RISC-V: Add riscv_fw_parent_hartid() function
of/irq: Set FWNODE_FLAG_BEST_EFFORT for the interrupt controller DT
nodes
irqchip/riscv-intc: Add support for RISC-V AIA
dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller
irqchip: Add RISC-V incoming MSI controller driver irqchip/riscv-imsic: Add support for PCI MSI irqdomain irqchip/riscv-imsic: Improve IOMMU DMA support
dt-bindings: interrupt-controller: Add RISC-V advanced PLIC
irqchip: Add RISC-V advanced PLIC driver
RISC-V: Select APLIC and IMSIC drivers
MAINTAINERS: Add entry for RISC-V AIA drivers
.../interrupt-controller/riscv,aplic.yaml | 162 +++ .../interrupt-controller/riscv,imsics.yaml | 172 +++ MAINTAINERS | 12 + arch/riscv/Kconfig | 2 + arch/riscv/include/asm/processor.h | 3 + arch/riscv/kernel/cpu.c | 12 + drivers/iommu/dma-iommu.c | 38 + drivers/irqchip/Kconfig | 20 +- drivers/irqchip/Makefile | 2 + drivers/irqchip/irq-riscv-aplic.c | 750 ++++++++++++ drivers/irqchip/irq-riscv-imsic.c | 1080 +++++++++++++++++ drivers/irqchip/irq-riscv-intc.c | 36 +- drivers/of/irq.c | 10 + include/linux/iommu.h | 6 + include/linux/irqchip/riscv-aplic.h | 119 ++ include/linux/irqchip/riscv-imsic.h | 86 ++ 16 files changed, 2503 insertions(+), 7 deletions(-) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml create mode 100644 drivers/irqchip/irq-riscv-aplic.c create mode 100644 drivers/irqchip/irq-riscv-imsic.c create mode 100644 include/linux/irqchip/riscv-aplic.h create mode 100644 include/linux/irqchip/riscv-imsic.h
Signature · 1 line
2.34.1
PATCH v3 1/11 RISC-V: Add riscv_fw_parent_hartid() function
We add common riscv_fw_parent_hartid() which help device drivers to get parent hartid of the INTC (i.e. local interrupt controller) fwnode. Currently, this new function only supports device tree but it can be extended to support ACPI as well.
--- arch/riscv/include/asm/processor.h | 3 +++ arch/riscv/kernel/cpu.c | 12 ++++++++++++ 2 files changed, 15 insertions(+)
arch/riscv/include/asm/processor.h +3 -0
@@ -77,6 +77,9 @@ struct device_node;int riscv_of_processor_hartid (struct device_node *node,unsigned long *hartid);int riscv_of_parent_hartid (struct device_node *node,unsigned long *hartid); +struct fwnode_handle ; +int riscv_fw_parent_hartid (struct fwnode_handle *node,unsigned long *hartid); +extern void riscv_fill_hwcap (void );extern int arch_dup_task_struct (struct task_struct *dst,struct task_struct *src);
arch/riscv/kernel/cpu.c +12 -0
@@ -73,6 +73,18 @@ int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)return -1 ; } +/* Find hart ID of the CPU fwnode under which given fwnode falls. */ +int riscv_fw_parent_hartid (struct fwnode_handle *node,unsigned long *hartid) +{ +/* +* Currently, this function only supports DT but it can be +* extended to support ACPI as well. +*/ +if (!is_of_node (node)) +return -EINVAL ; +return riscv_of_parent_hartid (to_of_node (node), hartid); +} + DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);unsigned long riscv_cached_mvendorid (unsigned int cpu_id)
Signature · 1 line
2.34.1
Re: [PATCH v3 01/11] RISC-V: Add riscv_fw_parent_hartid() function
Quoted 30 lines · Anup Patel
We add common
riscv_fw_parent_hartid()which help device drivers to get parent hartid of the INTC (i.e. local interrupt controller) fwnode. Currently, this new function only supports device tree but it can be extended to support ACPI as well.Signed-off-by Anup Patel <[email protected]>--- arch/riscv/include/asm/processor.h | 3 +++ arch/riscv/kernel/cpu.c | 12 ++++++++++++ 2 files changed, 15 insertions(+)arch/riscv/include/asm/processor.h +3
-0@@ -77,6 +77,9 @@ struct device_node;int riscv_of_processor_hartid (struct device_node *node,unsigned long *hartid);int riscv_of_parent_hartid (struct device_node *node,unsigned long *hartid); +struct fwnode_handle ; +int riscv_fw_parent_hartid (struct fwnode_handle *node,unsigned long *hartid); +extern void riscv_fill_hwcap (void );extern int arch_dup_task_struct (struct task_struct *dst,struct task_struct *src);arch/riscv/kernel/cpu.c +0
-0
arch/riscv/kernel/cpu.c int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid) · line 82
Quoted 10 lines
arch/riscv/kernel/cpu.c +7
-0@@ -73,6 +73,18 @@ int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)return -1 ; } +/* Find hart ID of the CPU fwnode under which given fwnode falls. */ +int riscv_fw_parent_hartid (struct fwnode_handle *node,unsigned long *hartid) +{ +/* +* Currently, this function only supports DT but it can be +* extended to support ACPI as well. +*/
Statement of the obvious here, no?
Although, it seems a little odd to read this comment & the corresponding statement in the commit message, when the series appears to have been based on the ACPI?
Perhaps by the time v4 comes around, ACPI support will have been merged & that'll be moot.
arch/riscv/kernel/cpu.c int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid) · line 85
Quoted 3 lines
arch/riscv/kernel/cpu.c +3
-0+if (!is_of_node (node)) +return -EINVAL ; +return riscv_of_parent_hartid (to_of_node (node), hartid);
nit: blank line before the return here please.
Thanks,
Conor.
Re: [PATCH v3 01/11] RISC-V: Add riscv_fw_parent_hartid() function
Quoted 48 lines · Conor Dooley
Quoted 30 lines · Anup Patel
We add common
riscv_fw_parent_hartid()which help device drivers to get parent hartid of the INTC (i.e. local interrupt controller) fwnode. Currently, this new function only supports device tree but it can be extended to support ACPI as well.Signed-off-by Anup Patel <[email protected]>--- arch/riscv/include/asm/processor.h | 3 +++ arch/riscv/kernel/cpu.c | 12 ++++++++++++ 2 files changed, 15 insertions(+)arch/riscv/include/asm/processor.h +3
-0@@ -77,6 +77,9 @@ struct device_node;int riscv_of_processor_hartid (struct device_node *node,unsigned long *hartid);int riscv_of_parent_hartid (struct device_node *node,unsigned long *hartid); +struct fwnode_handle ; +int riscv_fw_parent_hartid (struct fwnode_handle *node,unsigned long *hartid); +extern void riscv_fill_hwcap (void );extern int arch_dup_task_struct (struct task_struct *dst,struct task_struct *src);arch/riscv/kernel/cpu.c +0
-0arch/riscv/kernel/cpu.c int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid) · line 82
Quoted 10 lines
arch/riscv/kernel/cpu.c +7
-0@@ -73,6 +73,18 @@ int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)return -1 ; } +/* Find hart ID of the CPU fwnode under which given fwnode falls. */ +int riscv_fw_parent_hartid (struct fwnode_handle *node,unsigned long *hartid) +{ +/* +* Currently, this function only supports DT but it can be +* extended to support ACPI as well. +*/ Statement of the obvious here, no?
Although, it seems a little odd to read this comment & the corresponding statement in the commit message, when the series appears to have been based on the ACPI?
Quoted 2 lines
Perhaps by the time v4 comes around, ACPI support will have been merged & that'll be moot.
Yes, I was anyway going to update this in v4 to support both DT and ACPI.
Quoted 6 lines
arch/riscv/kernel/cpu.c int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid) · line 85
Quoted 4 lines
arch/riscv/kernel/cpu.c +3
-0+if (!is_of_node (node)) +return -EINVAL ; +return riscv_of_parent_hartid (to_of_node (node), hartid);nit: blank line before the return here please.
Okay, I will update.
Quoted 3 lines
Thanks,
Conor.
Regards,
Anup
PATCH v3 2/11 of/irq: Set FWNODE_FLAG_BEST_EFFORT for the interrupt controller DT nodes
In reply to Anup PatelThe RISC-V APLIC interrupt controller driver is a regular platform driver so we need to ensure that it is probed as soon as possible. To achieve this, we mark the interrupt controller device nodes with FWNODE_FLAG_BEST_EFFORT (just like console DT node).
--- drivers/of/irq.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
drivers/of/irq.c +10 -0
@@ -535,6 +535,16 @@ void __init of_irq_init(const struct of_device_id *matches)INIT_LIST_HEAD (&intc_desc_list);INIT_LIST_HEAD (&intc_parent_list); +/* +* We need interrupt controller platform drivers to work as soon +* as possible so mark the interrupt controller device nodes with +* FWNODE_FLAG_BEST_EFFORT so that fw_delink knows not to delay +* the probe of the interrupt controller device for suppliers +* without drivers. +*/ +for_each_node_with_property (np,"interrupt-controller" ) + np->fwnode.flags |=FWNODE_FLAG_BEST_EFFORT ; +for_each_matching_node_and_match (np, matches, &match) {if (!of_property_read_bool (np,"interrupt-controller" ) || !of_device_is_available (np))
Signature · 1 line
2.34.1
Rob Herring
maintainerRe: [PATCH v3 02/11] of/irq: Set FWNODE_FLAG_BEST_EFFORT for the interrupt controller DT nodes
+Saravana
Quoted 6 lines · Anup Patel
The RISC-V APLIC interrupt controller driver is a regular platform driver so we need to ensure that it is probed as soon as possible. To achieve this, we mark the interrupt controller device nodes with
FWNODE_FLAG_BEST_EFFORT(just like console DT node).Fixes 8f486cab263c ("driver core: fw_devlink: Allow firmware to mark devices as best effort")
It is good practice to Cc the commit author of what you are fixing.
Quoted 10 lines
Signed-off-by Anup Patel <[email protected]>--- drivers/of/irq.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/of/irq.c +0
-0
drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 539
Quoted 5 lines
drivers/of/irq.c +2
-0@@ -535,6 +535,16 @@ void __init of_irq_init(const struct of_device_id *matches)INIT_LIST_HEAD (&intc_desc_list);INIT_LIST_HEAD (&intc_parent_list); +/* +* We need interrupt controller platform drivers to work as soon
If it's platform drivers/devices you care about, then perhaps this should be done when platform devices are created.
drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 545
Quoted 6 lines
drivers/of/irq.c +6
-0+* as possible so mark the interrupt controller device nodes with +* FWNODE_FLAG_BEST_EFFORT so that fw_delink knows not to delay +* the probe of the interrupt controller device for suppliers +* without drivers. +*/ +for_each_node_with_property (np,"interrupt-controller" )
This function only operates on nodes matching 'matches', but this operates on everything.
It does make sense that if we init an interrupt controller here, then we will surely want to probe its driver later on. So maybe just move setting FWNODE_FLAG_BEST_EFFORT within for_each_matching_node_and_match() below.
Quoted 8 lines
drivers/of/irq.c +2
-0+ np->fwnode.flags |=FWNODE_FLAG_BEST_EFFORT ; +for_each_matching_node_and_match (np, matches, &match) {if (!of_property_read_bool (np,"interrupt-controller" ) || !of_device_is_available (np))Signature · 1 line
2.34.1
Re: [PATCH v3 02/11] of/irq: Set FWNODE_FLAG_BEST_EFFORT for the interrupt controller DT nodes
Quoted 39 lines · Rob Herring
+Saravana
Quoted 6 lines · Anup Patel
The RISC-V APLIC interrupt controller driver is a regular platform driver so we need to ensure that it is probed as soon as possible. To achieve this, we mark the interrupt controller device nodes with
FWNODE_FLAG_BEST_EFFORT(just like console DT node).Fixes 8f486cab263c ("driver core: fw_devlink: Allow firmware to mark devices as best effort")It is good practice to Cc the commit author of what you are fixing.
Quoted 10 lines
Signed-off-by Anup Patel <[email protected]>--- drivers/of/irq.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/of/irq.c +0
-0drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 539
Quoted 5 lines
drivers/of/irq.c +2
-0@@ -535,6 +535,16 @@ void __init of_irq_init(const struct of_device_id *matches)INIT_LIST_HEAD (&intc_desc_list);INIT_LIST_HEAD (&intc_parent_list); +/* +* We need interrupt controller platform drivers to work as soon If it's platform
drivers/devicesyou care about, then perhaps this should be done when platform devices are created.drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 545
Quoted 6 lines
drivers/of/irq.c +6
-0+* as possible so mark the interrupt controller device nodes with +* FWNODE_FLAG_BEST_EFFORT so that fw_delink knows not to delay +* the probe of the interrupt controller device for suppliers +* without drivers. +*/ +for_each_node_with_property (np,"interrupt-controller" )
Quoted 10 lines
This function only operates on nodes matching 'matches', but this operates on everything.
It does make sense that if we init an interrupt controller here, then we will surely want to probe its driver later on. So maybe just move setting
FWNODE_FLAG_BEST_EFFORTwithinfor_each_matching_node_and_match()below.Quoted 2 lines
drivers/of/irq.c +2
-0+ np->fwnode.flags |=FWNODE_FLAG_BEST_EFFORT ; +
Definite Nack. You are pretty much disabling fw_devlink for all interrupt controllers. There are plenty of examples of the IRQ drivers being needed very early on and still probing properly without the need for this flag. Please fix your driver/DT.
-Saravana
Quoted 6 lines
Quoted 6 lines
drivers/of/irq.c +0
-1for_each_matching_node_and_match (np, matches, &match) {if (!of_property_read_bool (np,"interrupt-controller" ) || !of_device_is_available (np)) --2.34.1
Re: [PATCH v3 02/11] of/irq: Set FWNODE_FLAG_BEST_EFFORT for the interrupt controller DT nodes
Quoted 49 lines · Saravana Kannan
Quoted 47 lines · Rob Herring
+Saravana
Quoted 6 lines · Anup Patel
The RISC-V APLIC interrupt controller driver is a regular platform driver so we need to ensure that it is probed as soon as possible. To achieve this, we mark the interrupt controller device nodes with
FWNODE_FLAG_BEST_EFFORT(just like console DT node).Fixes 8f486cab263c ("driver core: fw_devlink: Allow firmware to mark devices as best effort")It is good practice to Cc the commit author of what you are fixing.
Quoted 10 lines
Signed-off-by Anup Patel <[email protected]>--- drivers/of/irq.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/of/irq.c +0
-0drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 539
Quoted 5 lines
drivers/of/irq.c +2
-0@@ -535,6 +535,16 @@ void __init of_irq_init(const struct of_device_id *matches)INIT_LIST_HEAD (&intc_desc_list);INIT_LIST_HEAD (&intc_parent_list); +/* +* We need interrupt controller platform drivers to work as soon If it's platform
drivers/devicesyou care about, then perhaps this should be done when platform devices are created.drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 545
Quoted 6 lines
drivers/of/irq.c +6
-0+* as possible so mark the interrupt controller device nodes with +* FWNODE_FLAG_BEST_EFFORT so that fw_delink knows not to delay +* the probe of the interrupt controller device for suppliers +* without drivers. +*/ +for_each_node_with_property (np,"interrupt-controller" )This function only operates on nodes matching 'matches', but this operates on everything.
It does make sense that if we init an interrupt controller here, then we will surely want to probe its driver later on. So maybe just move setting
FWNODE_FLAG_BEST_EFFORTwithinfor_each_matching_node_and_match()below.
Quoted 7 lines
Quoted 2 lines
drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 547
Quoted 2 lines
drivers/of/irq.c +2
-0+ np->fwnode.flags |=FWNODE_FLAG_BEST_EFFORT ; +Definite Nack. You are pretty much disabling
fw_devlinkfor all interrupt controllers. There are plenty of examples of the IRQ drivers being needed very early on and still probing properly without the need for this flag. Please fix your driver/DT.
Okay, I will try to set FWNODE_FLAG_BEST_EFFORT only for APLIC device_node via IRQCHIP_DECLARE().
Regards,
Anup
Quoted 10 lines
-Saravana
Quoted 6 lines
Quoted 6 lines
drivers/of/irq.c +0
-1for_each_matching_node_and_match (np, matches, &match) {if (!of_property_read_bool (np,"interrupt-controller" ) || !of_device_is_available (np)) --2.34.1
Re: [PATCH v3 02/11] of/irq: Set FWNODE_FLAG_BEST_EFFORT for the interrupt controller DT nodes
Quoted 54 lines · Anup Patel
Quoted 52 lines · Saravana Kannan
Quoted 49 lines · Rob Herring
+Saravana
Quoted 6 lines · Anup Patel
The RISC-V APLIC interrupt controller driver is a regular platform driver so we need to ensure that it is probed as soon as possible. To achieve this, we mark the interrupt controller device nodes with
FWNODE_FLAG_BEST_EFFORT(just like console DT node).Fixes 8f486cab263c ("driver core: fw_devlink: Allow firmware to mark devices as best effort")It is good practice to Cc the commit author of what you are fixing.
Quoted 10 lines
Signed-off-by Anup Patel <[email protected]>--- drivers/of/irq.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/of/irq.c +0
-0drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 539
Quoted 5 lines
drivers/of/irq.c +2
-0@@ -535,6 +535,16 @@ void __init of_irq_init(const struct of_device_id *matches)INIT_LIST_HEAD (&intc_desc_list);INIT_LIST_HEAD (&intc_parent_list); +/* +* We need interrupt controller platform drivers to work as soon If it's platform
drivers/devicesyou care about, then perhaps this should be done when platform devices are created.drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 545
Quoted 6 lines
drivers/of/irq.c +6
-0+* as possible so mark the interrupt controller device nodes with +* FWNODE_FLAG_BEST_EFFORT so that fw_delink knows not to delay +* the probe of the interrupt controller device for suppliers +* without drivers. +*/ +for_each_node_with_property (np,"interrupt-controller" )This function only operates on nodes matching 'matches', but this operates on everything.
It does make sense that if we init an interrupt controller here, then we will surely want to probe its driver later on. So maybe just move setting
FWNODE_FLAG_BEST_EFFORTwithinfor_each_matching_node_and_match()below.Quoted 2 lines
drivers/of/irq.c +2
-0+ np->fwnode.flags |=FWNODE_FLAG_BEST_EFFORT ; +
Quoted 7 lines
Quoted 4 lines
Definite Nack. You are pretty much disabling
fw_devlinkfor all interrupt controllers. There are plenty of examples of the IRQ drivers being needed very early on and still probing properly without the need for this flag. Please fix your driver/DT.Okay, I will try to set
FWNODE_FLAG_BEST_EFFORTonly for APLICdevice_nodeviaIRQCHIP_DECLARE().
No, my point is that you very likely don't need to use that flag.
What exactly is the problem you are facing? Point to an example RISC-V dts (not dtsi) in upstream and give specifics please.
-Saravana
Anup Patel
maintainerRe: [PATCH v3 02/11] of/irq: Set FWNODE_FLAG_BEST_EFFORT for the interrupt controller DT nodes
Quoted 61 lines · Saravana Kannan
Quoted 59 lines · Anup Patel
Quoted 56 lines · Saravana Kannan
Quoted 39 lines · Rob Herring
+Saravana
Quoted 6 lines · Anup Patel
The RISC-V APLIC interrupt controller driver is a regular platform driver so we need to ensure that it is probed as soon as possible. To achieve this, we mark the interrupt controller device nodes with
FWNODE_FLAG_BEST_EFFORT(just like console DT node).Fixes 8f486cab263c ("driver core: fw_devlink: Allow firmware to mark devices as best effort")It is good practice to Cc the commit author of what you are fixing.
Quoted 10 lines
Signed-off-by Anup Patel <[email protected]>--- drivers/of/irq.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/of/irq.c +0
-0drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 539
Quoted 5 lines
drivers/of/irq.c +2
-0@@ -535,6 +535,16 @@ void __init of_irq_init(const struct of_device_id *matches)INIT_LIST_HEAD (&intc_desc_list);INIT_LIST_HEAD (&intc_parent_list); +/* +* We need interrupt controller platform drivers to work as soon If it's platform
drivers/devicesyou care about, then perhaps this should be done when platform devices are created.drivers/of/irq.c void __init of_irq_init(const struct of_device_id *matches) · line 545
Quoted 6 lines
drivers/of/irq.c +6
-0+* as possible so mark the interrupt controller device nodes with +* FWNODE_FLAG_BEST_EFFORT so that fw_delink knows not to delay +* the probe of the interrupt controller device for suppliers +* without drivers. +*/ +for_each_node_with_property (np,"interrupt-controller" )Quoted 10 lines
This function only operates on nodes matching 'matches', but this operates on everything.
It does make sense that if we init an interrupt controller here, then we will surely want to probe its driver later on. So maybe just move setting
FWNODE_FLAG_BEST_EFFORTwithinfor_each_matching_node_and_match()below.Quoted 2 lines
drivers/of/irq.c +2
-0+ np->fwnode.flags |=FWNODE_FLAG_BEST_EFFORT ; +Definite Nack. You are pretty much disabling
fw_devlinkfor all interrupt controllers. There are plenty of examples of the IRQ drivers being needed very early on and still probing properly without the need for this flag. Please fix your driver/DT.
Quoted 7 lines
Quoted 2 lines
Okay, I will try to set
FWNODE_FLAG_BEST_EFFORTonly for APLICdevice_nodeviaIRQCHIP_DECLARE().No, my point is that you very likely don't need to use that flag.
What exactly is the problem you are facing? Point to an example RISC-V dts (not dtsi) in upstream and give specifics please.
The APLIC irqchip driver added by this series is a platform driver. It used to work fine without setting FWNODE_FLAG_BEST_EFFORT flag in fwnode but after commit 8f486cab263c the APLIC irqchip driver did not probe hence this fix.
Regards,
Anup
PATCH v3 3/11 irqchip/riscv-intc: Add support for RISC-V AIA
In reply to Anup PatelThe RISC-V advanced interrupt architecture (AIA) extends the per-HART local interrupts in following ways:
1. Minimum 64 local interrupts for both RV32 and RV64
2. Ability to process multiple pending local interrupts in same interrupt handler
3. Priority configuration for each local interrupts
4. Special CSRs to configure/access the per-HART MSI controller
This patch adds support for RISC-V AIA in the RISC-V intc driver.
--- drivers/irqchip/irq-riscv-intc.c | 36 ++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-)
drivers/irqchip/irq-riscv-intc.c +30 -6
@@ -17,6 +17,7 @@#include <linux/module.h> #include <linux/of.h> #include <linux/smp.h> +#include <asm/hwcap.h> static struct irq_domain *intc_domain;
@@ -30,6 +31,15 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)generic_handle_domain_irq (intc_domain, cause); } +static asmlinkage void riscv_intc_aia_irq(struct pt_regs *regs) +{ +unsigned long topi; + +while ((topi =csr_read (CSR_TOPI ))) +generic_handle_domain_irq (intc_domain, + topi >>TOPI_IID_SHIFT ); +} +/* * On RISC-V systems local interrupts are masked or unmasked by writing * the SIE (Supervisor Interrupt Enable) CSR. As CSRs can only be written
@@ -39,12 +49,18 @@ static asmlinkage void riscv_intc_irq(struct pt_regs *regs)static void riscv_intc_irq_mask (struct irq_data *d) { -csr_clear (CSR_IE ,BIT (d->hwirq)); +if (d->hwirq <BITS_PER_LONG ) +csr_clear (CSR_IE ,BIT (d->hwirq)); +else +csr_clear (CSR_IEH ,BIT (d->hwirq -BITS_PER_LONG )); }static void riscv_intc_irq_unmask (struct irq_data *d) { -csr_set (CSR_IE ,BIT (d->hwirq)); +if (d->hwirq <BITS_PER_LONG ) +csr_set (CSR_IE ,BIT (d->hwirq)); +else +csr_set (CSR_IEH ,BIT (d->hwirq -BITS_PER_LONG )); }static void riscv_intc_irq_eoi (struct irq_data *d)
@@ -115,16 +131,22 @@ static struct fwnode_handle *riscv_intc_hwnode(void)static int __init riscv_intc_init_common (struct fwnode_handle *fn) { -int rc; +int rc, nr_irqs =BITS_PER_LONG ; + +if (riscv_isa_extension_available (NULL , SxAIA) &&BITS_PER_LONG ==32 ) + nr_irqs = nr_irqs *2 ; - intc_domain =irq_domain_create_linear (fn,BITS_PER_LONG , + intc_domain =irq_domain_create_linear (fn, nr_irqs, &riscv_intc_domain_ops,NULL );if (!intc_domain) {pr_err ("unable to add IRQ domain\n" );return -ENXIO ; } - rc =set_handle_irq (&riscv_intc_irq); +if (riscv_isa_extension_available (NULL , SxAIA)) + rc =set_handle_irq (&riscv_intc_aia_irq); +else + rc =set_handle_irq (&riscv_intc_irq);if (rc) {pr_err ("failed to set irq handler\n" );return rc;
@@ -132,7 +154,9 @@ static int __init riscv_intc_init_common(struct fwnode_handle *fn)riscv_set_intc_hwnode_fn (riscv_intc_hwnode); -pr_info ("%d local interrupts mapped\n" ,BITS_PER_LONG ); +pr_info ("%d local interrupts mapped%s\n" , + nr_irqs, (riscv_isa_extension_available (NULL , SxAIA)) ? +" using AIA" :"" );return 0 ; }
Signature · 1 line
2.34.1
PATCH v3 4/11 dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller
In reply to Anup PatelWe add DT bindings document for the RISC-V incoming MSI controller (IMSIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.
--- .../interrupt-controller/riscv,imsics.yaml | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml
Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +172 -0
@@ -0,0 +1,172 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/riscv,imsics.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: RISC-V Incoming MSI Controller (IMSIC) + +maintainers: + - Anup Patel <[email protected]> + +description: | + The RISC-V advanced interrupt architecture (AIA) defines a per-CPU incoming + MSI controller (IMSIC) for handling MSIs in a RISC-V platform. The RISC-V + AIA specification can be found at https://github.com/riscv/riscv-aia. + + The IMSIC is a per-CPU (or per-HART) device with separate interrupt file + for each privilege level (machine or supervisor). The configuration of + a IMSIC interrupt file is done using AIA CSRs and it also has a 4KB MMIO + space to receive MSIs from devices. Each IMSIC interrupt file supports a + fixed number of interrupt identities (to distinguish MSIs from devices) + which is same for given privilege level across CPUs (or HARTs). + + The device tree of a RISC-V platform will have one IMSIC device tree node + for each privilege level (machine or supervisor) which collectively describe + IMSIC interrupt files at that privilege level across CPUs (or HARTs). + + The arrangement of IMSIC interrupt files in MMIO space of a RISC-V platform + follows a particular scheme defined by the RISC-V AIA specification. A IMSIC + group is a set of IMSIC interrupt files co-located in MMIO space and we can + have multiple IMSIC groups (i.e. clusters, sockets, chiplets, etc) in a + RISC-V platform. The MSI target address of a IMSIC interrupt file at given + privilege level (machine or supervisor) encodes group index, HART index, + and guest index (shown below). + + XLEN-1 > (HART Index MSB) 12 0 + | | | | + ------------------------------------------------------------- + |xxxxxx|Group Index|xxxxxxxxxxx|HART Index|Guest Index| 0 | + ------------------------------------------------------------- + +allOf: + - $ref: /schemas/interrupt-controller.yaml# + - $ref: /schemas/interrupt-controller/msi-controller.yaml# + +properties: + compatible: + items: + - enum: + - qemu,imsics + - const: riscv,imsics + + reg: + minItems: 1 + maxItems: 16384 + description: + Base address of each IMSIC group. + + interrupt-controller: true + + "#interrupt-cells": + const: 0 + + msi-controller: true + + "#msi-cells": + const: 0 + + interrupts-extended: + minItems: 1 + maxItems: 16384 + description: + This property represents the set of CPUs (or HARTs) for which given + device tree node describes the IMSIC interrupt files. Each node pointed + to should be a riscv,cpu-intc node, which has a riscv node (i.e. RISC-V + HART) as parent. + + riscv,num-ids: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 63 + maximum: 2047 + description: + Number of interrupt identities supported by IMSIC interrupt file. + + riscv,num-guest-ids: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 63 + maximum: 2047 + description: + Number of interrupt identities are supported by IMSIC guest interrupt + file. When not specified it is assumed to be same as specified by the + riscv,num-ids property. + + riscv,guest-index-bits: + minimum: 0 + maximum: 7 + default: 0 + description: + Number of guest index bits in the MSI target address. + + riscv,hart-index-bits: + minimum: 0 + maximum: 15 + description: + Number of HART index bits in the MSI target address. When not + specified it is calculated based on the interrupts-extended property. + + riscv,group-index-bits: + minimum: 0 + maximum: 7 + default: 0 + description: + Number of group index bits in the MSI target address. + + riscv,group-index-shift: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 55 + default: 24 + description: + The least significant bit position of the group index bits in the + MSI target address. + +required: + - compatible + - reg + - interrupt-controller + - msi-controller + - "#msi-cells" + - interrupts-extended + - riscv,num-ids + +unevaluatedProperties: false + +examples: + - | + // Example 1 (Machine-level IMSIC files with just one group): + + interrupt-controller@24000000 { + compatible = "qemu,imsics", "riscv,imsics"; + interrupts-extended = <&cpu1_intc 11>, + <&cpu2_intc 11>, + <&cpu3_intc 11>, + <&cpu4_intc 11>; + reg = <0x28000000 0x4000>; + interrupt-controller; + #interrupt-cells = <0>; + msi-controller; + #msi-cells = <0>; + riscv,num-ids = <127>; + }; + + - | + // Example 2 (Supervisor-level IMSIC files with two groups): + + interrupt-controller@28000000 { + compatible = "qemu,imsics", "riscv,imsics"; + interrupts-extended = <&cpu1_intc 9>, + <&cpu2_intc 9>, + <&cpu3_intc 9>, + <&cpu4_intc 9>; + reg = <0x28000000 0x2000>, /* Group0 IMSICs */ + <0x29000000 0x2000>; /* Group1 IMSICs */ + interrupt-controller; + #interrupt-cells = <0>; + msi-controller; + #msi-cells = <0>; + riscv,num-ids = <127>; + riscv,group-index-bits = <1>; + riscv,group-index-shift = <24>; + }; +...
Signature · 1 line
2.34.1
Re: [PATCH v3 04/11] dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller
Hey Anup,
Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml line 76
Quoted 8 lines · Anup Patel
Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +8
-0+ interrupts-extended: + minItems: 1 + maxItems: 16384 + description: + This property represents the set of CPUs (or HARTs) for which given + device tree node describes the IMSIC interrupt files. Each node pointed + to should be a riscv,cpu-intc node, which has a riscv node (i.e. RISC-V + HART) as parent.
One minor nit here about wording - "riscv node" doesn't seem particularly clear to me, should it be s/riscv node/cpu node/?
My only thing last time around was my misunderstanding, and you also appear to have resolved Rob's complaints, so:
Just to note, it'd be great if you could CC me on series that I've already reviewed when you resubmit them?
Although in this case, if you ran get_maintainer.pl on v6.4-rc1 it'd have told you to CC me anyway ;)
Thanks,
Conor.
Re: [PATCH v3 04/11] dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller
Quoted 13 lines · Conor Dooley
Hey Anup,
Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml line 76
Quoted 8 lines · Anup Patel
Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml new file +8
-0+ interrupts-extended: + minItems: 1 + maxItems: 16384 + description: + This property represents the set of CPUs (or HARTs) for which given + device tree node describes the IMSIC interrupt files. Each node pointed + to should be a riscv,cpu-intc node, which has a riscv node (i.e. RISC-V + HART) as parent.
Quoted 2 lines
One minor nit here about wording - "riscv node" doesn't seem particularly clear to me, should it be s/riscv node/cpu node/?
Okay, I will update.
Quoted 5 lines
My only thing last time around was my misunderstanding, and you also appear to have resolved Rob's complaints, so:
Reviewed-by Conor Dooley <[email protected]>
Quoted 7 lines
Just to note, it'd be great if you could CC me on series that I've already reviewed when you resubmit them?
Although in this case, if you ran get_maintainer.pl on v6.4-rc1 it'd have told you to CC me anyway ;)Thanks,
Conor.
Regards,
Anup
Re: [PATCH v3 04/11] dt-bindings: interrupt-controller: Add RISC-V incoming MSI controller
In reply to Anup PatelQuoted 8 lines · Anup Patel
We add DT bindings document for the RISC-V incoming MSI controller (IMSIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.
Signed-off-by Anup Patel <[email protected]>--- .../interrupt-controller/riscv,imsics.yaml | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+)
Best regards,
Krzysztof
PATCH v3 5/11 irqchip: Add RISC-V incoming MSI controller driver
In reply to Anup PatelThe RISC-V advanced interrupt architecture (AIA) specification defines a new MSI controller for managing MSIs and IPIs on a RISC-V platform.
This new MSI controller is referred to as incoming message signalled interrupt controller (IMSIC) which manages MSI on per-HART (or per-CPU) basis. (For more details refer https://github.com/riscv/riscv-aia)
This patch adds an irqchip driver for RISC-V IMSIC which provides IPIs and platform MSIs to the Linux RISC-V kernel.
--- drivers/irqchip/Kconfig | 7 +- drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-riscv-imsic.c | 1028 +++++++++++++++++++++++++++ include/linux/irqchip/riscv-imsic.h | 86 +++ 4 files changed, 1121 insertions(+), 1 deletion(-) create mode 100644 drivers/irqchip/irq-riscv-imsic.c create mode 100644 include/linux/irqchip/riscv-imsic.h
drivers/irqchip/Kconfig +6 -1
@@ -30,7 +30,6 @@ config ARM_GIC_V2M config GIC_NON_BANKED bool - config ARM_GIC_V3 bool select IRQ_DOMAIN_HIERARCHY
@@ -545,6 +544,12 @@ config SIFIVE_PLIC select IRQ_DOMAIN_HIERARCHY select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP +config RISCV_IMSIC + bool + depends on RISCV + select IRQ_DOMAIN_HIERARCHY + select GENERIC_MSI_IRQ + config EXYNOS_IRQ_COMBINER bool "Samsung Exynos IRQ combiner support" if COMPILE_TEST depends on (ARCH_EXYNOS && ARM) || COMPILE_TEST
drivers/irqchip/Makefile +1 -0
@@ -95,6 +95,7 @@ obj-$(CONFIG_QCOM_MPM) += irq-qcom-mpm.o obj-$(CONFIG_CSKY_MPINTC) += irq-csky-mpintc.o obj-$(CONFIG_CSKY_APB_INTC) += irq-csky-apb-intc.o obj-$(CONFIG_RISCV_INTC) += irq-riscv-intc.o +obj-$(CONFIG_RISCV_IMSIC) += irq-riscv-imsic.o obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o obj-$(CONFIG_IMX_INTMUX) += irq-imx-intmux.o
drivers/irqchip/irq-riscv-imsic.c new file +1028 -0
1,028 lines not on this page · raw message
include/linux/irqchip/riscv-imsic.h new file +86 -0
86 lines not on this page · raw message
Signature · 1 line
2.34.1
PATCH v3 6/11 irqchip/riscv-imsic: Add support for PCI MSI irqdomain
In reply to Anup PatelThe Linux PCI framework requires it's own dedicated MSI irqdomain so let us create PCI MSI irqdomain as child of the IMSIC base irqdomain.
--- drivers/irqchip/Kconfig | 7 +++++ drivers/irqchip/irq-riscv-imsic.c | 49 +++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+)
drivers/irqchip/Kconfig +7 -0
@@ -550,6 +550,13 @@ config RISCV_IMSIC select IRQ_DOMAIN_HIERARCHY select GENERIC_MSI_IRQ +config RISCV_IMSIC_PCI + bool + depends on RISCV_IMSIC + depends on PCI + depends on PCI_MSI + default RISCV_IMSIC + config EXYNOS_IRQ_COMBINER bool "Samsung Exynos IRQ combiner support" if COMPILE_TEST depends on (ARCH_EXYNOS && ARM) || COMPILE_TEST
drivers/irqchip/irq-riscv-imsic.c +49 -0
79 lines not on this page · raw message
Signature · 1 line
2.34.1
PATCH v3 7/11 irqchip/riscv-imsic: Improve IOMMU DMA support
In reply to Anup PatelWe have a separate RISC-V IMSIC MSI address for each CPU so changing MSI (or IRQ) affinity results in re-programming of MSI address in the PCIe (or platform) device.
Currently, the iommu_dma_prepare_msi() is called only once at the time of IRQ allocation so IOMMU DMA domain will only have mapping for one MSI page. This means iommu_dma_compose_msi_msg() called by imsic_irq_compose_msi_msg() will always use the same MSI page irrespective to target CPU MSI address. In other words, changing MSI (or IRQ) affinity for device using IOMMU DMA domain will not work.
To address above issue, we do the following:
1) Map MSI pages for all CPUs in imsic_irq_domain_alloc() using iommu_dma_prepare_msi().
2) Add a new iommu_dma_select_msi() API to select a specific MSI page from a set of already mapped MSI pages.
3) Use iommu_dma_select_msi() to select a specific MSI page before calling iommu_dma_compose_msi_msg() in imsic_irq_compose_msi_msg().
--- drivers/iommu/dma-iommu.c | 38 +++++++++++++++++++++++++++++++ drivers/irqchip/irq-riscv-imsic.c | 27 ++++++++++++---------- include/linux/iommu.h | 6 +++++ 3 files changed, 59 insertions(+), 12 deletions(-)
drivers/iommu/dma-iommu.c +38 -0
@@ -1677,6 +1677,44 @@ int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr)return 0 ; } +/** +* iommu_dma_select_msi() - Select a MSI page from a set of +* already mapped MSI pages in the IOMMU domain. +* +* @desc: MSI descriptor prepared by iommu_dma_prepare_msi() +* @msi_addr: physical address of the MSI page to be selected +* +* Return: 0 on success or negative error code if the select failed. +*/ +int iommu_dma_select_msi (struct msi_desc *desc,phys_addr_t msi_addr) +{ +struct device *dev =msi_desc_to_dev (desc); +struct iommu_domain *domain =iommu_get_domain_for_dev (dev); +const struct iommu_dma_msi_page *msi_page; +struct iommu_dma_cookie *cookie; + +if (!domain || !domain->iova_cookie) { + desc->iommu_cookie =NULL ; +return 0 ; + } + + cookie = domain->iova_cookie; + msi_addr &= ~(phys_addr_t)(cookie_msi_granule (cookie) -1 ); + + msi_page =msi_desc_get_iommu_cookie (desc); +if (msi_page && msi_page->phys == msi_addr) +return 0 ; + +list_for_each_entry (msi_page, &cookie->msi_page_list, list) { +if (msi_page->phys == msi_addr) { +msi_desc_set_iommu_cookie (desc, msi_page); +return 0 ; + } + } + +return -ENOENT ; +} +/** * iommu_dma_compose_msi_msg() - Apply translation to an MSI message * @desc: MSI descriptor prepared by iommu_dma_prepare_msi()
drivers/irqchip/irq-riscv-imsic.c +15 -12
@@ -446,6 +446,10 @@ static void imsic_irq_compose_msi_msg(struct irq_data *d,if (WARN_ON (err))return ; + err =iommu_dma_select_msi (desc, msi_addr); +if (WARN_ON (err)) +return ; + msg->address_hi =upper_32_bits (msi_addr); msg->address_lo =lower_32_bits (msi_addr); msg->data = d->hwirq;
@@ -493,11 +497,18 @@ static int imsic_irq_domain_alloc(struct irq_domain *domain,int i, hwirq, err =0 ;unsigned int cpu; - err =imsic_get_cpu (&imsic->lmask, false, &cpu); -if (err) -return err; +/* Map MSI address of all CPUs */ +for_each_cpu (cpu, &imsic->lmask) { + err =imsic_cpu_page_phys (cpu,0 , &msi_addr); +if (err) +return err; - err =imsic_cpu_page_phys (cpu,0 , &msi_addr); + err =iommu_dma_prepare_msi (info->desc, msi_addr); +if (err) +return err; + } + + err =imsic_get_cpu (&imsic->lmask, false, &cpu);if (err)return err;
@@ -505,10 +516,6 @@ static int imsic_irq_domain_alloc(struct irq_domain *domain,if (hwirq <0 )return hwirq; - err =iommu_dma_prepare_msi (info->desc, msi_addr); -if (err) - goto fail; -for (i =0 ; i < nr_irqs; i++) {imsic_id_set_target (hwirq + i, cpu); irq_domain_set_info(domain, virq + i, hwirq + i,
@@ -528,10 +535,6 @@ static int imsic_irq_domain_alloc(struct irq_domain *domain, }return 0 ; - -fail: -imsic_ids_free (hwirq,get_count_order (nr_irqs)); -return err; } static void imsic_irq_domain_free(struct irq_domain *domain,
include/linux/iommu.h +6 -0
@@ -1117,6 +1117,7 @@ void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit);int iommu_get_msi_cookie (struct iommu_domain *domain,dma_addr_t base);int iommu_dma_prepare_msi (struct msi_desc *desc,phys_addr_t msi_addr); +int iommu_dma_select_msi (struct msi_desc *desc,phys_addr_t msi_addr);void iommu_dma_compose_msi_msg (struct msi_desc *desc,struct msi_msg *msg);#else /* CONFIG_IOMMU_DMA */
@@ -1138,6 +1139,11 @@ static inline int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_areturn 0 ; } +static inline int iommu_dma_select_msi (struct msi_desc *desc,phys_addr_t msi_addr) +{ +return 0 ; +} +static inline void iommu_dma_compose_msi_msg (struct msi_desc *desc,struct msi_msg *msg) { }
Signature · 1 line
2.34.1
Robin Murphy
maintainerRe: [PATCH v3 07/11] irqchip/riscv-imsic: Improve IOMMU DMA support
Quoted 12 lines · Anup Patel
We have a separate RISC-V IMSIC MSI address for each CPU so changing MSI (or IRQ) affinity results in re-programming of MSI address in the PCIe (or platform) device.
Currently, the
iommu_dma_prepare_msi()is called only once at the time of IRQ allocation so IOMMU DMA domain will only have mapping for one MSI page. This meansiommu_dma_compose_msi_msg()called byimsic_irq_compose_msi_msg()will always use the same MSI page irrespective to target CPU MSI address. In other words, changing MSI (or IRQ) affinity for device using IOMMU DMA domain will not work.
Quoted 8 lines
To address above issue, we do the following:
1) Map MSI pages for all CPUs inimsic_irq_domain_alloc()usingiommu_dma_prepare_msi().
2) Add a newiommu_dma_select_msi()API to select a specific MSI page from a set of already mapped MSI pages.
3) Useiommu_dma_select_msi()to select a specific MSI page before callingiommu_dma_compose_msi_msg()inimsic_irq_compose_msi_msg().
The high-level design is that prepare ensures any necessary page mappings exist, then compose retrieves the appropriate page for the given message. I think it generalises well enough without needing a new op, it just means that caching a single page in the msi_desc up-front no longer fits, so that wants tweaking to allow compose to do a more general lookup.
Thanks,
Robin.
Quoted 141 lines
Reported-by Vincent Chen <[email protected]>Signed-off-by Anup Patel <[email protected]>--- drivers/iommu/dma-iommu.c | 38 +++++++++++++++++++++++++++++++ drivers/irqchip/irq-riscv-imsic.c | 27 ++++++++++++---------- include/linux/iommu.h | 6 +++++ 3 files changed, 59 insertions(+), 12 deletions(-)drivers/iommu/dma-iommu.c +38
-0@@ -1677,6 +1677,44 @@ int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr)return 0 ; } +/** +* iommu_dma_select_msi() - Select a MSI page from a set of +* already mapped MSI pages in the IOMMU domain. +* +* @desc: MSI descriptor prepared by iommu_dma_prepare_msi() +* @msi_addr: physical address of the MSI page to be selected +* +* Return: 0 on success or negative error code if the select failed. +*/ +int iommu_dma_select_msi (struct msi_desc *desc,phys_addr_t msi_addr) +{ +struct device *dev =msi_desc_to_dev (desc); +struct iommu_domain *domain =iommu_get_domain_for_dev (dev); +const struct iommu_dma_msi_page *msi_page; +struct iommu_dma_cookie *cookie; + +if (!domain || !domain->iova_cookie) { + desc->iommu_cookie =NULL ; +return 0 ; + } + + cookie = domain->iova_cookie; + msi_addr &= ~(phys_addr_t)(cookie_msi_granule (cookie) -1 ); + + msi_page =msi_desc_get_iommu_cookie (desc); +if (msi_page && msi_page->phys == msi_addr) +return 0 ; + +list_for_each_entry (msi_page, &cookie->msi_page_list, list) { +if (msi_page->phys == msi_addr) { +msi_desc_set_iommu_cookie (desc, msi_page); +return 0 ; + } + } + +return -ENOENT ; +} +/** * iommu_dma_compose_msi_msg() - Apply translation to an MSI message * @desc: MSI descriptor prepared by iommu_dma_prepare_msi() drivers/irqchip/irq-riscv-imsic.c +15
-12@@ -446,6 +446,10 @@ static void imsic_irq_compose_msi_msg(struct irq_data *d,if (WARN_ON (err))return ; + err =iommu_dma_select_msi (desc, msi_addr); +if (WARN_ON (err)) +return ; + msg->address_hi =upper_32_bits (msi_addr); msg->address_lo =lower_32_bits (msi_addr); msg->data = d->hwirq;@@ -493,11 +497,18 @@ static int imsic_irq_domain_alloc(struct irq_domain *domain,int i, hwirq, err =0 ;unsigned int cpu; - err =imsic_get_cpu (&imsic->lmask, false, &cpu); -if (err) -return err; +/* Map MSI address of all CPUs */ +for_each_cpu (cpu, &imsic->lmask) { + err =imsic_cpu_page_phys (cpu,0 , &msi_addr); +if (err) +return err; - err =imsic_cpu_page_phys (cpu,0 , &msi_addr); + err =iommu_dma_prepare_msi (info->desc, msi_addr); +if (err) +return err; + } + + err =imsic_get_cpu (&imsic->lmask, false, &cpu);if (err)return err;@@ -505,10 +516,6 @@ static int imsic_irq_domain_alloc(struct irq_domain *domain,if (hwirq <0 )return hwirq; - err =iommu_dma_prepare_msi (info->desc, msi_addr); -if (err) - goto fail; -for (i =0 ; i < nr_irqs; i++) {imsic_id_set_target (hwirq + i, cpu); irq_domain_set_info(domain, virq + i, hwirq + i,@@ -528,10 +535,6 @@ static int imsic_irq_domain_alloc(struct irq_domain *domain, }return 0 ; - -fail: -imsic_ids_free (hwirq,get_count_order (nr_irqs)); -return err; } static void imsic_irq_domain_free(struct irq_domain *domain,include/linux/iommu.h +6
-0@@ -1117,6 +1117,7 @@ void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit);int iommu_get_msi_cookie (struct iommu_domain *domain,dma_addr_t base);int iommu_dma_prepare_msi (struct msi_desc *desc,phys_addr_t msi_addr); +int iommu_dma_select_msi (struct msi_desc *desc,phys_addr_t msi_addr);void iommu_dma_compose_msi_msg (struct msi_desc *desc,struct msi_msg *msg);#else /* CONFIG_IOMMU_DMA */ @@ -1138,6 +1139,11 @@ static inline int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_areturn 0 ; } +static inline int iommu_dma_select_msi (struct msi_desc *desc,phys_addr_t msi_addr) +{ +return 0 ; +} +static inline void iommu_dma_compose_msi_msg (struct msi_desc *desc,struct msi_msg *msg) { }
Re: [PATCH v3 07/11] irqchip/riscv-imsic: Improve IOMMU DMA support
Quoted 23 lines · Robin Murphy
Quoted 12 lines · Anup Patel
We have a separate RISC-V IMSIC MSI address for each CPU so changing MSI (or IRQ) affinity results in re-programming of MSI address in the PCIe (or platform) device.
Currently, the
iommu_dma_prepare_msi()is called only once at the time of IRQ allocation so IOMMU DMA domain will only have mapping for one MSI page. This meansiommu_dma_compose_msi_msg()called byimsic_irq_compose_msi_msg()will always use the same MSI page irrespective to target CPU MSI address. In other words, changing MSI (or IRQ) affinity for device using IOMMU DMA domain will not work.Quoted 8 lines
To address above issue, we do the following:
1) Map MSI pages for all CPUs inimsic_irq_domain_alloc()usingiommu_dma_prepare_msi().
2) Add a newiommu_dma_select_msi()API to select a specific MSI page from a set of already mapped MSI pages.
3) Useiommu_dma_select_msi()to select a specific MSI page before callingiommu_dma_compose_msi_msg()inimsic_irq_compose_msi_msg().
Quoted 6 lines
The high-level design is that prepare ensures any necessary page mappings exist, then compose retrieves the appropriate page for the given message. I think it generalises well enough without needing a new op, it just means that caching a single page in the
msi_descup-front no longer fits, so that wants tweaking to allow compose to do a more general lookup.
Yes, usage of msi_desc_get/set_iommu_cookie() is the only problem.
To have minimal impact on other irqchip drivers, I added a separate iommu_dma_select_msi() API but in the next patch revision I will tweak iommu_dma_compose_msi_msg() to lookup msi_page based on the existing MSI address in "struct msi_msg".
Regards,
Anup
Quoted 145 lines
Thanks,
Robin.Quoted 141 lines
Reported-by Vincent Chen <[email protected]>Signed-off-by Anup Patel <[email protected]>--- drivers/iommu/dma-iommu.c | 38 +++++++++++++++++++++++++++++++ drivers/irqchip/irq-riscv-imsic.c | 27 ++++++++++++---------- include/linux/iommu.h | 6 +++++ 3 files changed, 59 insertions(+), 12 deletions(-)drivers/iommu/dma-iommu.c +38
-0@@ -1677,6 +1677,44 @@ int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr)return 0 ; } +/** +* iommu_dma_select_msi() - Select a MSI page from a set of +* already mapped MSI pages in the IOMMU domain. +* +* @desc: MSI descriptor prepared by iommu_dma_prepare_msi() +* @msi_addr: physical address of the MSI page to be selected +* +* Return: 0 on success or negative error code if the select failed. +*/ +int iommu_dma_select_msi (struct msi_desc *desc,phys_addr_t msi_addr) +{ +struct device *dev =msi_desc_to_dev (desc); +struct iommu_domain *domain =iommu_get_domain_for_dev (dev); +const struct iommu_dma_msi_page *msi_page; +struct iommu_dma_cookie *cookie; + +if (!domain || !domain->iova_cookie) { + desc->iommu_cookie =NULL ; +return 0 ; + } + + cookie = domain->iova_cookie; + msi_addr &= ~(phys_addr_t)(cookie_msi_granule (cookie) -1 ); + + msi_page =msi_desc_get_iommu_cookie (desc); +if (msi_page && msi_page->phys == msi_addr) +return 0 ; + +list_for_each_entry (msi_page, &cookie->msi_page_list, list) { +if (msi_page->phys == msi_addr) { +msi_desc_set_iommu_cookie (desc, msi_page); +return 0 ; + } + } + +return -ENOENT ; +} +/** * iommu_dma_compose_msi_msg() - Apply translation to an MSI message * @desc: MSI descriptor prepared by iommu_dma_prepare_msi() drivers/irqchip/irq-riscv-imsic.c +15
-12@@ -446,6 +446,10 @@ static void imsic_irq_compose_msi_msg(struct irq_data *d,if (WARN_ON (err))return ; + err =iommu_dma_select_msi (desc, msi_addr); +if (WARN_ON (err)) +return ; + msg->address_hi =upper_32_bits (msi_addr); msg->address_lo =lower_32_bits (msi_addr); msg->data = d->hwirq;@@ -493,11 +497,18 @@ static int imsic_irq_domain_alloc(struct irq_domain *domain,int i, hwirq, err =0 ;unsigned int cpu; - err =imsic_get_cpu (&imsic->lmask, false, &cpu); -if (err) -return err; +/* Map MSI address of all CPUs */ +for_each_cpu (cpu, &imsic->lmask) { + err =imsic_cpu_page_phys (cpu,0 , &msi_addr); +if (err) +return err; - err =imsic_cpu_page_phys (cpu,0 , &msi_addr); + err =iommu_dma_prepare_msi (info->desc, msi_addr); +if (err) +return err; + } + + err =imsic_get_cpu (&imsic->lmask, false, &cpu);if (err)return err;@@ -505,10 +516,6 @@ static int imsic_irq_domain_alloc(struct irq_domain *domain,if (hwirq <0 )return hwirq; - err =iommu_dma_prepare_msi (info->desc, msi_addr); -if (err) - goto fail; -for (i =0 ; i < nr_irqs; i++) {imsic_id_set_target (hwirq + i, cpu); irq_domain_set_info(domain, virq + i, hwirq + i,@@ -528,10 +535,6 @@ static int imsic_irq_domain_alloc(struct irq_domain *domain, }return 0 ; - -fail: -imsic_ids_free (hwirq,get_count_order (nr_irqs)); -return err; } static void imsic_irq_domain_free(struct irq_domain *domain,include/linux/iommu.h +6
-0@@ -1117,6 +1117,7 @@ void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit);int iommu_get_msi_cookie (struct iommu_domain *domain,dma_addr_t base);int iommu_dma_prepare_msi (struct msi_desc *desc,phys_addr_t msi_addr); +int iommu_dma_select_msi (struct msi_desc *desc,phys_addr_t msi_addr);void iommu_dma_compose_msi_msg (struct msi_desc *desc,struct msi_msg *msg);#else /* CONFIG_IOMMU_DMA */ @@ -1138,6 +1139,11 @@ static inline int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_areturn 0 ; } +static inline int iommu_dma_select_msi (struct msi_desc *desc,phys_addr_t msi_addr) +{ +return 0 ; +} +static inline void iommu_dma_compose_msi_msg (struct msi_desc *desc,struct msi_msg *msg) { }
Re: [PATCH v3 07/11] irqchip/riscv-imsic: Improve IOMMU DMA support
In reply to Anup PatelQuoted 12 lines · Anup Patel
We have a separate RISC-V IMSIC MSI address for each CPU so changing MSI (or IRQ) affinity results in re-programming of MSI address in the PCIe (or platform) device.
Currently, the
iommu_dma_prepare_msi()is called only once at the time of IRQ allocation so IOMMU DMA domain will only have mapping for one MSI page. This meansiommu_dma_compose_msi_msg()called byimsic_irq_compose_msi_msg()will always use the same MSI page irrespective to target CPU MSI address. In other words, changing MSI (or IRQ) affinity for device using IOMMU DMA domain will not work.
Quoted 8 lines
To address above issue, we do the following:
1) Map MSI pages for all CPUs inimsic_irq_domain_alloc()usingiommu_dma_prepare_msi().
2) Add a newiommu_dma_select_msi()API to select a specific MSI page from a set of already mapped MSI pages.
3) Useiommu_dma_select_msi()to select a specific MSI page before callingiommu_dma_compose_msi_msg()inimsic_irq_compose_msi_msg().
Is there an iommu driver somewhere in all this? I don't obviously see one?
There should be no reason to use the dma-iommu.c stuff just to make interrupts work, that is only necessary if there is an iommu, and the platform architecture requires the iommu to have the MSI region programmed into IOPTEs.
And I'd be much happier if we could clean this design up before risc-v starts using it too :\
Jason
Re: [PATCH v3 07/11] irqchip/riscv-imsic: Improve IOMMU DMA support
Quoted 23 lines · Jason Gunthorpe
Quoted 12 lines · Anup Patel
We have a separate RISC-V IMSIC MSI address for each CPU so changing MSI (or IRQ) affinity results in re-programming of MSI address in the PCIe (or platform) device.
Currently, the
iommu_dma_prepare_msi()is called only once at the time of IRQ allocation so IOMMU DMA domain will only have mapping for one MSI page. This meansiommu_dma_compose_msi_msg()called byimsic_irq_compose_msi_msg()will always use the same MSI page irrespective to target CPU MSI address. In other words, changing MSI (or IRQ) affinity for device using IOMMU DMA domain will not work.Quoted 8 lines
To address above issue, we do the following:
1) Map MSI pages for all CPUs inimsic_irq_domain_alloc()usingiommu_dma_prepare_msi().
2) Add a newiommu_dma_select_msi()API to select a specific MSI page from a set of already mapped MSI pages.
3) Useiommu_dma_select_msi()to select a specific MSI page before callingiommu_dma_compose_msi_msg()inimsic_irq_compose_msi_msg().
Quoted 2 lines
Is there an iommu driver somewhere in all this? I don't obviously see one?
Sorry for the delayed response.
The RISC-V IOMMU specification is frozen and will be ratified/released anytime this month or next.
https://github.com/riscv-non-isa/riscv-iommu/releases/download/v1.0-rc6/riscv-iommu.pdf
The RISC-V IOMMU driver will be send-out on LKML pretty soon
https://github.com/tjeznach/linux/tree/tjeznach/riscv-iommu
which can be tested on QEMU
https://github.com/tjeznach/qemu/tree/tjeznach/riscv-iommu
Quoted 9 lines
There should be no reason to use the
dma-iommu.cstuff just to make interrupts work, that is only necessary if there is an iommu, and the platform architecture requires the iommu to have the MSI region programmed into IOPTEs.And I'd be much happier if we could clean this design up before risc-v starts using it too :\
Sure, I will send-out v4 in the next few days.
Regards,
Anup
PATCH v3 8/11 dt-bindings: interrupt-controller: Add RISC-V advanced PLIC
In reply to Anup PatelWe add DT bindings document for RISC-V advanced platform level interrupt controller (APLIC) defined by the RISC-V advanced interrupt architecture (AIA) specification.
--- .../interrupt-controller/riscv,aplic.yaml | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +162 -0
@@ -0,0 +1,162 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/riscv,aplic.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: RISC-V Advanced Platform Level Interrupt Controller (APLIC) + +maintainers: + - Anup Patel <[email protected]> + +description: + The RISC-V advanced interrupt architecture (AIA) defines an advanced + platform level interrupt controller (APLIC) for handling wired interrupts + in a RISC-V platform. The RISC-V AIA specification can be found at + https://github.com/riscv/riscv-aia. + + The RISC-V APLIC is implemented as hierarchical APLIC domains where all + interrupt sources connect to the root domain which can further delegate + interrupts to child domains. There is one device tree node for each APLIC + domain. + +allOf: + - $ref: /schemas/interrupt-controller.yaml# + +properties: + compatible: + items: + - enum: + - qemu,aplic + - const: riscv,aplic + + reg: + maxItems: 1 + + interrupt-controller: true + + "#interrupt-cells": + const: 2 + + interrupts-extended: + minItems: 1 + maxItems: 16384 + description: + Given APLIC domain directly injects external interrupts to a set of + RISC-V HARTS (or CPUs). Each node pointed to should be a riscv,cpu-intc + node, which has a riscv node (i.e. RISC-V HART) as parent. + + msi-parent: + description: + Given APLIC domain forwards wired interrupts as MSIs to a AIA incoming + message signaled interrupt controller (IMSIC). If both "msi-parent" and + "interrupts-extended" properties are present then it means the APLIC + domain supports both MSI mode and Direct mode in HW. In this case, the + APLIC driver has to choose between MSI mode or Direct mode. + + riscv,num-sources: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 1023 + description: + Specifies the number of wired interrupt sources supported by this + APLIC domain. + + riscv,children: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 1024 + items: + maxItems: 1 + description: + A list of child APLIC domains for the given APLIC domain. Each child + APLIC domain is assigned a child index in increasing order, with the + first child APLIC domain assigned child index 0. The APLIC domain child + index is used by firmware to delegate interrupts from the given APLIC + domain to a particular child APLIC domain. + + riscv,delegate: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 1024 + items: + items: + - description: child APLIC domain phandle + - description: first interrupt number of this APLIC domain (inclusive) + - description: last interrupt number of this APLIC domain (inclusive) + description: + A interrupt delegation list where each entry is a triple consisting of + child APLIC domain phandle, first interrupt number of this APLIC domain, + and last interrupt number of this APLIC domain. Firmware must configure + interrupt delegation registers based on interrupt delegation list. + +required: + - compatible + - reg + - interrupt-controller + - "#interrupt-cells" + - riscv,num-sources + +unevaluatedProperties: false + +examples: + - | + // Example 1 (APLIC domains directly injecting interrupt to HARTs): + + interrupt-controller@c000000 { + compatible = "qemu,aplic", "riscv,aplic"; + interrupts-extended = <&cpu1_intc 11>, + <&cpu2_intc 11>, + <&cpu3_intc 11>, + <&cpu4_intc 11>; + reg = <0xc000000 0x4080>; + interrupt-controller; + #interrupt-cells = <2>; + riscv,num-sources = <63>; + riscv,children = <&aplic1>, <&aplic2>; + riscv,delegate = <&aplic1 1 63>; + }; + + aplic1: interrupt-controller@d000000 { + compatible = "qemu,aplic", "riscv,aplic"; + interrupts-extended = <&cpu1_intc 9>, + <&cpu2_intc 9>; + reg = <0xd000000 0x4080>; + interrupt-controller; + #interrupt-cells = <2>; + riscv,num-sources = <63>; + }; + + aplic2: interrupt-controller@e000000 { + compatible = "qemu,aplic", "riscv,aplic"; + interrupts-extended = <&cpu3_intc 9>, + <&cpu4_intc 9>; + reg = <0xe000000 0x4080>; + interrupt-controller; + #interrupt-cells = <2>; + riscv,num-sources = <63>; + }; + + - | + // Example 2 (APLIC domains forwarding interrupts as MSIs): + + interrupt-controller@c000000 { + compatible = "qemu,aplic", "riscv,aplic"; + msi-parent = <&imsic_mlevel>; + reg = <0xc000000 0x4000>; + interrupt-controller; + #interrupt-cells = <2>; + riscv,num-sources = <63>; + riscv,children = <&aplic3>; + riscv,delegate = <&aplic3 1 63>; + }; + + aplic3: interrupt-controller@d000000 { + compatible = "qemu,aplic", "riscv,aplic"; + msi-parent = <&imsic_slevel>; + reg = <0xd000000 0x4000>; + interrupt-controller; + #interrupt-cells = <2>; + riscv,num-sources = <63>; + }; +...
Signature · 1 line
2.34.1
Re: [PATCH v3 08/11] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC
Hey Anup,
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 47
Quoted 7 lines · Anup Patel
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +7
-0+ interrupts-extended: + minItems: 1 + maxItems: 16384 + description: + Given APLIC domain directly injects external interrupts to a set of + RISC-V HARTS (or CPUs). Each node pointed to should be a riscv,cpu-intc + node, which has a riscv node (i.e. RISC-V HART) as parent.
Same nit here, s/riscv node/cpu node/?
Quoted 6 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +6
-0+ + msi-parent: + description: + Given APLIC domain forwards wired interrupts as MSIs to a AIA incoming + message signaled interrupt controller (IMSIC). If both "msi-parent" and + "interrupts-extended" properties are present then it means the APLIC
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 63
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +10
-0+ domain supports both MSI mode and Direct mode in HW. In this case, the + APLIC driver has to choose between MSI mode or Direct mode. + + riscv,num-sources: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 1023 + description: + Specifies the number of wired interrupt sources supported by this + APLIC domain.
Rob asked:
| We don't normally need to how many interrupts, why here?
But I do not see an answer to that on lore.
Quoted 18 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +18
-0+ + riscv,children: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 1024 + items: + maxItems: 1 + description: + A list of child APLIC domains for the given APLIC domain. Each child + APLIC domain is assigned a child index in increasing order, with the + first child APLIC domain assigned child index 0. The APLIC domain child + index is used by firmware to delegate interrupts from the given APLIC + domain to a particular child APLIC domain. + + riscv,delegate: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 1024
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 91
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +10
-0+ items: + items: + - description: child APLIC domain phandle + - description: first interrupt number of this APLIC domain (inclusive) + - description: last interrupt number of this APLIC domain (inclusive) + description: + A interrupt delegation list where each entry is a triple consisting of + child APLIC domain phandle, first interrupt number of this APLIC domain, + and last interrupt number of this APLIC domain. Firmware must configure + interrupt delegation registers based on interrupt delegation list.
I read back Rob's comments on this from last time. He said:
| The node's domain it delegating its interrupts to the child domain or | the other way around? The interrupt numbers here are this domain's or | the child's?
IMO it's ambiguous from the binding description whether the numbers refer to the "first interrupt in the parent domain that is delegated to the child" or to numbering of the interrupts within the child domain. "This" can be quite an ambiguous word, and it's not totally obvious whether the "this" refers to the "child domain" earlier in the sentence.
I also do not not think you have answered his question about the directionality of the delegation either (it should just be a copy-paste from riscv,children, no?).
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 98
Quoted 7 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +7
-0+ +required: + - compatible + - reg + - interrupt-controller + - "#interrupt-cells" + - riscv,num-sources
btw, do we need something like:
anyOf:
- required:
- interrupts-extended
- required:
- msi-parent
& hopefully I didn't previously ask this one:
dependencies:
riscv,delegate: [ riscv,children ]
As an aside, I still think "riscv,delegate" looks strange here alongside "riscv,children" since "delegate" is singular and "children" is plural. The plural would be "delegates", but "delegation" would also fit better than "delegate".
Cheers,
Conor.
Re: [PATCH v3 08/11] dt-bindings: interrupt-controller: Add RISC-V advanced PLIC
Quoted 12 lines · Conor Dooley
Hey Anup,
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 47
Quoted 7 lines · Anup Patel
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +7
-0+ interrupts-extended: + minItems: 1 + maxItems: 16384 + description: + Given APLIC domain directly injects external interrupts to a set of + RISC-V HARTS (or CPUs). Each node pointed to should be a riscv,cpu-intc + node, which has a riscv node (i.e. RISC-V HART) as parent.
Quoted 1 line
Same nit here, s/riscv node/cpu node/?
Okay, I will update.
Quoted 21 lines
Quoted 7 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +6
-0+ + msi-parent: + description: + Given APLIC domain forwards wired interrupts as MSIs to a AIA incoming + message signaled interrupt controller (IMSIC). If both "msi-parent" and + "interrupts-extended" properties are present then it means the APLICDocumentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 63
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +10
-0+ domain supports both MSI mode and Direct mode in HW. In this case, the + APLIC driver has to choose between MSI mode or Direct mode. + + riscv,num-sources: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 1023 + description: + Specifies the number of wired interrupt sources supported by this + APLIC domain.Rob asked:
| We don't normally need to how many interrupts, why here?
Quoted 1 line
But I do not see an answer to that on lore.
The APLIC spec defines maximum interrupt sources to be 1023.
Quoted 31 lines
Quoted 19 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +18
-0+ + riscv,children: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 1024 + items: + maxItems: 1 + description: + A list of child APLIC domains for the given APLIC domain. Each child + APLIC domain is assigned a child index in increasing order, with the + first child APLIC domain assigned child index 0. The APLIC domain child + index is used by firmware to delegate interrupts from the given APLIC + domain to a particular child APLIC domain. + + riscv,delegate: + $ref: /schemas/types.yaml#/definitions/phandle-array + minItems: 1 + maxItems: 1024Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 91
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +10
-0+ items: + items: + - description: child APLIC domain phandle + - description: first interrupt number of this APLIC domain (inclusive) + - description: last interrupt number of this APLIC domain (inclusive) + description: + A interrupt delegation list where each entry is a triple consisting of + child APLIC domain phandle, first interrupt number of this APLIC domain, + and last interrupt number of this APLIC domain. Firmware must configure + interrupt delegation registers based on interrupt delegation list.I read back Rob's comments on this from last time. He said:
Quoted 3 lines
| The node's domain it delegating its interrupts to the child domain or | the other way around? The interrupt numbers here are this domain's or | the child's?
The node's domain is delegating its interrupts to the child domain.
Quoted 10 lines
IMO it's ambiguous from the binding description whether the numbers refer to the "first interrupt in the parent domain that is delegated to the child" or to numbering of the interrupts within the child domain. "This" can be quite an ambiguous word, and it's not totally obvious whether the "this" refers to the "child domain" earlier in the sentence.
I also do not not think you have answered his question about the directionality of the delegation either (it should just be a copy-paste from riscv,children, no?).
For APLIC, the interrupt delegation is always from parent domain to the child domain.
I will add a statement about this in the description.
Quoted 11 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml line 98
Quoted 8 lines
Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml new file +7
-0+ +required: + - compatible + - reg + - interrupt-controller + - "#interrupt-cells" + - riscv,num-sourcesbtw, do we need something like:
Quoted 5 lines
anyOf:
- required:
- interrupts-extended
- required:
- msi-parent
Okay, I will update.
Quoted 9 lines
& hopefully I didn't previously ask this one:
dependencies:
riscv,delegate: [ riscv,children ]As an aside, I still think "riscv,delegate" looks strange here alongside "riscv,children" since "delegate" is singular and "children" is plural. The plural would be "delegates", but "delegation" would also fit better than "delegate".
Okay, I will rename it to "riscv,delegation".
Quoted 3 lines
Cheers,
Conor.
Regards,
Anup
PATCH v3 9/11 irqchip: Add RISC-V advanced PLIC driver
In reply to Anup PatelThe RISC-V advanced interrupt architecture (AIA) specification defines a new interrupt controller for managing wired interrupts on a RISC-V platform. This new interrupt controller is referred to as advanced platform-level interrupt controller (APLIC) which can forward wired interrupts to CPUs (or HARTs) as local interrupts OR as message signaled interrupts.
(For more details refer https://github.com/riscv/riscv-aia)
This patch adds an irqchip driver for RISC-V APLIC found on RISC-V platforms.
--- drivers/irqchip/Kconfig | 6 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-riscv-aplic.c | 750 ++++++++++++++++++++++++++++ include/linux/irqchip/riscv-aplic.h | 119 +++++ 4 files changed, 876 insertions(+) create mode 100644 drivers/irqchip/irq-riscv-aplic.c create mode 100644 include/linux/irqchip/riscv-aplic.h
drivers/irqchip/Kconfig +6 -0
@@ -544,6 +544,12 @@ config SIFIVE_PLIC select IRQ_DOMAIN_HIERARCHY select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP +config RISCV_APLIC + bool + depends on RISCV + select IRQ_DOMAIN_HIERARCHY + select GENERIC_MSI_IRQ + config RISCV_IMSIC bool depends on RISCV
drivers/irqchip/Makefile +1 -0
@@ -95,6 +95,7 @@ obj-$(CONFIG_QCOM_MPM) += irq-qcom-mpm.o obj-$(CONFIG_CSKY_MPINTC) += irq-csky-mpintc.o obj-$(CONFIG_CSKY_APB_INTC) += irq-csky-apb-intc.o obj-$(CONFIG_RISCV_INTC) += irq-riscv-intc.o +obj-$(CONFIG_RISCV_APLIC) += irq-riscv-aplic.o obj-$(CONFIG_RISCV_IMSIC) += irq-riscv-imsic.o obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o
drivers/irqchip/irq-riscv-aplic.c new file +750 -0
750 lines not on this page · raw message
include/linux/irqchip/riscv-aplic.h new file +119 -0
119 lines not on this page · raw message
Signature · 1 line
2.34.1
PATCH v3 10/11 RISC-V: Select APLIC and IMSIC drivers
In reply to Anup PatelThe QEMU virt machine supports AIA emulation and we also have quite a few RISC-V platforms with AIA support under development so let us select APLIC and IMSIC drivers for all RISC-V platforms.
--- arch/riscv/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
arch/riscv/Kconfig +2 -0
@@ -135,6 +135,8 @@ config RISCV select PCI_DOMAINS_GENERIC if PCI select PCI_MSI if PCI select RISCV_ALTERNATIVE if !XIP_KERNEL + select RISCV_APLIC + select RISCV_IMSIC select RISCV_INTC select RISCV_TIMER if RISCV_SBI select SIFIVE_PLIC
Signature · 1 line
2.34.1
Re: [PATCH v3 10/11] RISC-V: Select APLIC and IMSIC drivers
Thanks,
Conor.
PATCH v3 11/11 MAINTAINERS: Add entry for RISC-V AIA drivers
In reply to Anup PatelAdd myself as maintainer for RISC-V AIA drivers including the RISC-V INTC driver which supports both AIA and non-AIA platforms.
--- MAINTAINERS | 12 ++++++++++++ 1 file changed, 12 insertions(+)
MAINTAINERS +12 -0
@@ -18111,6 +18111,18 @@ F: drivers/perf/riscv_pmu.c F: drivers/perf/riscv_pmu_legacy.c F: drivers/perf/riscv_pmu_sbi.c +RISC-V AIA DRIVERS +M: Anup Patel <[email protected]> +L: [email protected] +S: Maintained +F: Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml +F: Documentation/devicetree/bindings/interrupt-controller/riscv,imsics.yaml +F: drivers/irqchip/irq-riscv-aplic.c +F: drivers/irqchip/irq-riscv-imsic.c +F: drivers/irqchip/irq-riscv-intc.c +F: include/linux/irqchip/riscv-aplic.h +F: include/linux/irqchip/riscv-imsic.h + RISC-V ARCHITECTURE M: Paul Walmsley <[email protected]> M: Palmer Dabbelt <[email protected]>
Signature · 1 line
2.34.1
Hey Anup,
Quoted 16 lines · Anup Patel
The RISC-V AIA specification is now frozen as-per the RISC-V international process. The latest frozen specifcation can be found at:
https://github.com/riscv/riscv-aia/releases/download/1.0-RC1/riscv-interrupts-1.0-RC1.pdfAt a high-level, the AIA specification adds three things:
1) AIA CSRs
- Improved local interrupt support
2) Incoming Message Signaled Interrupt Controller (IMSIC)
- Per-HART MSI controller
- Support MSI virtualization
- Support IPI along with virtualization
3) Advanced Platform-Level Interrupt Controller (APLIC)
- Wired interrupt controller
- In MSI-mode, converts wired interrupt into MSIs (i.e. MSI generator)
- In Direct-mode, injects external interrupts directly into HARTs
Quoted 8 lines
For an overview of the AIA specification, refer the recent AIA virtualization talk at KVM Forum 2022:
https://static.sched.com/hosted_files/kvmforum2022/a1/AIA_Virtualization_in_KVM_RISCV_final.pdf
https://www.youtube.com/watch?v=r071dL8Z0yoThe PATCH3 of this series conflicts with the "irqchip/riscv-intc: Add ACPI support" patch of the "Add basic ACPI support for RISC-V" series.
(Refer, https://lore.kernel.org/linux-riscv/[email protected]/)
This series does not apply for me (or the patchwork automation) on top of v6.4-rc1, which was released prior to you sending this & contains the AIA definitions this series relies on.
You note here that there is a *conflict* with Sunil's series, but what you actually did, as far as I can tell, is base your series on top of theirs?
If you're going to do that, at least point it out so that reviewers don't have to figure out what your intention was :( Better yet, for the sake of our automation in particular, don't do it at all.
Thanks,
Conor.