[PATCH v4 0/2] riscv,aplic: support for hart indexes

[PATCH v4 0/2] riscv,aplic: support for hart indexes

[Vladimir Kondratiev]

Risc-v APLIC uses "hart index" to access data per destination hart.
Current implementation assumes hart indexes are consecutive integers
starting from 0, while Risc-V documentation says it may be
arbitrary numbers, with a clue that it may be related to the hart IDs.

In all boards I see in today's kernel, hart IDs are consecutive
integers, thus using dart IDs is the same as indexes.

However, for the MIPS P8700, hart IDs are different from indexes,
on this SoC they encode thread number, core and cluster in bits
[0..3], [4..15], [16..19] resulting Soc consisting of 3 clusters *
4 cores * 2 threads with hart IDs:
0x0, 0x1, 0x10, 0x11, 0x20, 0x21, 0x30, 0x31, 0x10000 etc.

Change default hart index to be hart ID related to the start of domain,
and add optional property to configure arbitrary indexes.

Use of "device_property" API allows to cover both ACPI and OF in single
code

1-st commit adds dt-bindings, 2-nd - code

Changed from v1:
1. use as fallback logical indexes instead of hart ids
2. refactor code to avoid unnecessary memory allocation

Changed from v2:
1. change property name to plural "riscv,hart-indexes"

Changed from v3:
1. added missing recepients as per "get_maintainer.pl"
   no other changes

Vladimir Kondratiev (2):
  dt-bindings: interrupt-controller: add risc-v,aplic hart indexes
  irqchip/riscv-aplic: add support for hart indexes

 .../interrupt-controller/riscv,aplic.yaml     |  8 ++++++
 drivers/irqchip/irq-riscv-aplic-direct.c      | 25 ++++++++++++++++---
 2 files changed, 30 insertions(+), 3 deletions(-)


base-commit: 9d89551994a430b50c4fffcb1e617a057fa76e20
-- 
2.43.0

[PATCH v4 1/2] dt-bindings: interrupt-controller: add risc-v,aplic hart indexes

[Vladimir Kondratiev]

Document optional property "riscv,hart-indexes"

Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 .../bindings/interrupt-controller/riscv,aplic.yaml        | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
index 190a6499c932..bef00521d5da 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
@@ -91,6 +91,14 @@ properties:
       Firmware must configure interrupt delegation registers based on
       interrupt delegation list.
 
+  riscv,hart-indexes:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 16384
+    description:
+      A list of hart indexes that APLIC should use to address each hart
+      that is mentioned in the "interrupts-extended"
+
 dependencies:
   riscv,delegation: [ "riscv,children" ]
 
-- 
2.43.0

[PATCH v4 2/2] irqchip/riscv-aplic: add support for hart indexes

[Vladimir Kondratiev]

Risc-V APLIC specification defines "hart index" in [1]:

Within a given interrupt domain, each of the domain’s harts has a
unique index number in the range 0 to 2^14 − 1 (= 16,383). The index
number a domain associates with a hart may or may not have any
relationship to the unique hart identifier (“hart ID”) that the
RISC-V Privileged Architecture assigns to the hart. Two different
interrupt domains may employ entirely different index numbers for
the same set of harts.

Further, this document says in "4.5 Memory-mapped control
region for an interrupt domain":

The array of IDC structures may include some for potential hart index
numbers that are not actual hart index numbers in the domain. For
example, the first IDC structure is always for hart index 0, but 0 is
not necessarily a valid index number for any hart in the domain.

Support arbitrary hart indexes specified in optional APLIC property
"riscv,hart-indexes" that should be array of u32 elements, one per
interrupt target. If this property not specified, fallback is to use
logical hart indexes within the domain.

[1]: https://github.com/riscv/riscv-aia

Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 drivers/irqchip/irq-riscv-aplic-direct.c | 25 +++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-riscv-aplic-direct.c b/drivers/irqchip/irq-riscv-aplic-direct.c
index 7cd6b646774b..ea61329decb2 100644
--- a/drivers/irqchip/irq-riscv-aplic-direct.c
+++ b/drivers/irqchip/irq-riscv-aplic-direct.c
@@ -31,7 +31,7 @@ struct aplic_direct {
 };
 
 struct aplic_idc {
-	unsigned int		hart_index;
+	u32			hart_index;
 	void __iomem		*regs;
 	struct aplic_direct	*direct;
 };
@@ -219,6 +219,21 @@ static int aplic_direct_parse_parent_hwirq(struct device *dev, u32 index,
 	return 0;
 }
 
+static int aplic_direct_get_hart_index(struct device *dev, u32 logical_index,
+				       u32 *hart_index)
+{
+	static const char *prop_hart_index = "riscv,hart-indexes";
+	struct device_node *np = to_of_node(dev->fwnode);
+
+	if (!np || !of_property_present(np, prop_hart_index)) {
+		*hart_index = logical_index;
+		return 0;
+	}
+
+	return of_property_read_u32_index(np, prop_hart_index,
+					  logical_index, hart_index);
+}
+
 int aplic_direct_setup(struct device *dev, void __iomem *regs)
 {
 	int i, j, rc, cpu, current_cpu, setup_count = 0;
@@ -265,8 +280,12 @@ int aplic_direct_setup(struct device *dev, void __iomem *regs)
 		cpumask_set_cpu(cpu, &direct->lmask);
 
 		idc = per_cpu_ptr(&aplic_idcs, cpu);
-		idc->hart_index = i;
-		idc->regs = priv->regs + APLIC_IDC_BASE + i * APLIC_IDC_SIZE;
+		rc = aplic_direct_get_hart_index(dev, i, &idc->hart_index);
+		if (rc) {
+			dev_warn(dev, "hart index not found for IDC%d\n", i);
+			continue;
+		}
+		idc->regs = priv->regs + APLIC_IDC_BASE + idc->hart_index * APLIC_IDC_SIZE;
 		idc->direct = direct;
 
 		aplic_idc_set_delivery(idc, true);
-- 
2.43.0

Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: add risc-v,aplic hart indexes

[Rob Herring]

On Thu, Jan 09, 2025 at 01:38:13PM +0200, Vladimir Kondratiev wrote:
> Document optional property "riscv,hart-indexes"
> 
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
>  .../bindings/interrupt-controller/riscv,aplic.yaml        | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> index 190a6499c932..bef00521d5da 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> @@ -91,6 +91,14 @@ properties:
>        Firmware must configure interrupt delegation registers based on
>        interrupt delegation list.
>  
> +  riscv,hart-indexes:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 16384
> +    description:
> +      A list of hart indexes that APLIC should use to address each hart
> +      that is mentioned in the "interrupts-extended"

Wouldn't using the 'cpus' property linking to each cpu/hart node work?

Rob

Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: add risc-v,aplic hart indexes

[Rob Herring]

On Thu, Jan 09, 2025 at 01:38:13PM +0200, Vladimir Kondratiev wrote:
> Document optional property "riscv,hart-indexes"

That is obvious reading the diff. Why do you need this?

Also, what happens when this property is not present?

> 
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
>  .../bindings/interrupt-controller/riscv,aplic.yaml        | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> index 190a6499c932..bef00521d5da 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
> @@ -91,6 +91,14 @@ properties:
>        Firmware must configure interrupt delegation registers based on
>        interrupt delegation list.
>  
> +  riscv,hart-indexes:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 16384
> +    description:
> +      A list of hart indexes that APLIC should use to address each hart
> +      that is mentioned in the "interrupts-extended"
> +
>  dependencies:
>    riscv,delegation: [ "riscv,children" ]
>  
> -- 
> 2.43.0
> 

Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: add risc-v,aplic hart indexes

[Vladimir Kondratiev]

>Wouldn't using the 'cpus' property linking to each cpu/hart node work?

>Rob

Hi,

unfortunately, per-CPU property would not work. "hart index" defined per
interrupt domain, and different controllers may define it differently.
This is from the "4.3 Hart index numbers" section of the interrupts spec found at
https://github.com/riscv/riscv-aia

Within a given interrupt domain, each of the domain’s harts has a unique index
number in the range 0 to 2^14 − 1 (= 16,383).

Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: add risc-v,aplic hart indexes

[Vladimir Kondratiev]

>> Document optional property "riscv,hart-indexes"

>That is obvious reading the diff. Why do you need this?

I say it briefly in the description for the property.
In more details this is described in the other patch comment
- for code that uses this property.
Is it better to repeat more detailed description in this patch
comment as well?

>Also, what happens when this property is not present?
Logical hart index get used, i.e. index in the "extended-interrupts"

Shall I add full explanation to this patch comment? This one, it is
a comment from the 2-nd patch in this set:

Risc-V APLIC specification defines "hart index" in [1]:

Within a given interrupt domain, each of the domain’s harts has a
unique index number in the range 0 to 2^14 − 1 (= 16,383). The index
number a domain associates with a hart may or may not have any
relationship to the unique hart identifier (“hart ID”) that the
RISC-V Privileged Architecture assigns to the hart. Two different
interrupt domains may employ entirely different index numbers for
the same set of harts.

Further, this document says in "4.5 Memory-mapped control
region for an interrupt domain":

The array of IDC structures may include some for potential hart index
numbers that are not actual hart index numbers in the domain. For
example, the first IDC structure is always for hart index 0, but 0 is
not necessarily a valid index number for any hart in the domain.

Support arbitrary hart indexes specified in optional APLIC property
"riscv,hart-indexes" that should be array of u32 elements, one per
interrupt target. If this property not specified, fallback is to use
logical hart indexes within the domain.

[1]: https://github.com/riscv/riscv-aia


Thanks, Vladimir

Re: [PATCH v4 0/2] riscv,aplic: support for hart indexes

[Vladimir Kondratiev]

Hi,
I want to complete with this patches, and trying to figure out where it stands.
I think I answered all questions asked about this patch set.
Any other questions/concerns?
Shall I do any changes?

Thanks, Vladimir

Re: [PATCH v4 1/2] dt-bindings: interrupt-controller: add risc-v,aplic hart indexes

[Thomas Gleixner]

On Sun, Jan 12 2025 at 08:38, Vladimir Kondratiev wrote:
>>> Document optional property "riscv,hart-indexes"
>
>>That is obvious reading the diff. Why do you need this?
>
> I say it briefly in the description for the property.
> In more details this is described in the other patch comment
> - for code that uses this property.
> Is it better to repeat more detailed description in this patch
> comment as well?

Obviously. Each patch has to be self contained and explain what it is
about.

Thanks,

        tglx

[PATCH v5 0/2] riscv,aplic: support for hart indexes

[Vladimir Kondratiev]

Risc-v APLIC uses "hart index" to access data per destination hart.
Current implementation assumes hart indexes are consecutive integers
starting from 0, while Risc-V documentation says it may be
arbitrary numbers, with a clue that it may be related to the hart IDs.

In all boards I see in today's kernel, hart IDs are consecutive
integers, thus using dart IDs is the same as indexes.

However, for the MIPS P8700, hart IDs are different from indexes,
on this SoC they encode thread number, core and cluster in bits
[0..3], [4..15], [16..19] resulting Soc consisting of 3 clusters *
4 cores * 2 threads with hart IDs:
0x0, 0x1, 0x10, 0x11, 0x20, 0x21, 0x30, 0x31, 0x10000 etc.

Change default hart index to be hart ID related to the start of domain,
and add optional property to configure arbitrary indexes.

Use of "device_property" API allows to cover both ACPI and OF in single
code

1-st commit adds dt-bindings, 2-nd - code

Changed from v1:
1. use as fallback logical indexes instead of hart ids
2. refactor code to avoid unnecessary memory allocation

Changed from v2:
1. change property name to plural "riscv,hart-indexes"

Changed from v3:
1. added missing recepients as per "get_maintainer.pl"
   no other changes

Changed from v4:
1. Verbose comment for the dt-bindings commit

Vladimir Kondratiev (2):
  dt-bindings: interrupt-controller: add risc-v,aplic hart indexes
  irqchip/riscv-aplic: add support for hart indexes

 .../interrupt-controller/riscv,aplic.yaml     |  8 ++++++
 drivers/irqchip/irq-riscv-aplic-direct.c      | 25 ++++++++++++++++---
 2 files changed, 30 insertions(+), 3 deletions(-)


base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
-- 
2.43.0

[PATCH v5 1/2] dt-bindings: interrupt-controller: add risc-v,aplic hart indexes

[Vladimir Kondratiev]

Document optional property "riscv,hart-indexes"

Risc-V APLIC specification defines "hart index" in [1]:

Within a given interrupt domain, each of the domain’s harts has a
unique index number in the range 0 to 2^14 − 1 (= 16,383). The index
number a domain associates with a hart may or may not have any
relationship to the unique hart identifier (“hart ID”) that the
RISC-V Privileged Architecture assigns to the hart. Two different
interrupt domains may employ entirely different index numbers for
the same set of harts.

Further, this document says in "4.5 Memory-mapped control
region for an interrupt domain":

The array of IDC structures may include some for potential hart index
numbers that are not actual hart index numbers in the domain. For
example, the first IDC structure is always for hart index 0, but 0 is
not necessarily a valid index number for any hart in the domain.

Support arbitrary hart indexes specified in optional APLIC property
"riscv,hart-indexes" that should be array of u32 elements, one per
interrupt target. If this property not specified, fallback is to use
logical hart indexes within the domain.

[1]: https://github.com/riscv/riscv-aia

Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 .../bindings/interrupt-controller/riscv,aplic.yaml        | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
index 190a6499c932..bef00521d5da 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/riscv,aplic.yaml
@@ -91,6 +91,14 @@ properties:
       Firmware must configure interrupt delegation registers based on
       interrupt delegation list.
 
+  riscv,hart-indexes:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 16384
+    description:
+      A list of hart indexes that APLIC should use to address each hart
+      that is mentioned in the "interrupts-extended"
+
 dependencies:
   riscv,delegation: [ "riscv,children" ]
 
-- 
2.43.0

[PATCH v5 2/2] irqchip/riscv-aplic: add support for hart indexes

[Vladimir Kondratiev]

Risc-V APLIC specification defines "hart index" in [1]:

Within a given interrupt domain, each of the domain’s harts has a
unique index number in the range 0 to 2^14 − 1 (= 16,383). The index
number a domain associates with a hart may or may not have any
relationship to the unique hart identifier (“hart ID”) that the
RISC-V Privileged Architecture assigns to the hart. Two different
interrupt domains may employ entirely different index numbers for
the same set of harts.

Further, this document says in "4.5 Memory-mapped control
region for an interrupt domain":

The array of IDC structures may include some for potential hart index
numbers that are not actual hart index numbers in the domain. For
example, the first IDC structure is always for hart index 0, but 0 is
not necessarily a valid index number for any hart in the domain.

Support arbitrary hart indexes specified in optional APLIC property
"riscv,hart-indexes" that should be array of u32 elements, one per
interrupt target. If this property not specified, fallback is to use
logical hart indexes within the domain.

[1]: https://github.com/riscv/riscv-aia

Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 drivers/irqchip/irq-riscv-aplic-direct.c | 25 +++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-riscv-aplic-direct.c b/drivers/irqchip/irq-riscv-aplic-direct.c
index 7cd6b646774b..ea61329decb2 100644
--- a/drivers/irqchip/irq-riscv-aplic-direct.c
+++ b/drivers/irqchip/irq-riscv-aplic-direct.c
@@ -31,7 +31,7 @@ struct aplic_direct {
 };
 
 struct aplic_idc {
-	unsigned int		hart_index;
+	u32			hart_index;
 	void __iomem		*regs;
 	struct aplic_direct	*direct;
 };
@@ -219,6 +219,21 @@ static int aplic_direct_parse_parent_hwirq(struct device *dev, u32 index,
 	return 0;
 }
 
+static int aplic_direct_get_hart_index(struct device *dev, u32 logical_index,
+				       u32 *hart_index)
+{
+	static const char *prop_hart_index = "riscv,hart-indexes";
+	struct device_node *np = to_of_node(dev->fwnode);
+
+	if (!np || !of_property_present(np, prop_hart_index)) {
+		*hart_index = logical_index;
+		return 0;
+	}
+
+	return of_property_read_u32_index(np, prop_hart_index,
+					  logical_index, hart_index);
+}
+
 int aplic_direct_setup(struct device *dev, void __iomem *regs)
 {
 	int i, j, rc, cpu, current_cpu, setup_count = 0;
@@ -265,8 +280,12 @@ int aplic_direct_setup(struct device *dev, void __iomem *regs)
 		cpumask_set_cpu(cpu, &direct->lmask);
 
 		idc = per_cpu_ptr(&aplic_idcs, cpu);
-		idc->hart_index = i;
-		idc->regs = priv->regs + APLIC_IDC_BASE + i * APLIC_IDC_SIZE;
+		rc = aplic_direct_get_hart_index(dev, i, &idc->hart_index);
+		if (rc) {
+			dev_warn(dev, "hart index not found for IDC%d\n", i);
+			continue;
+		}
+		idc->regs = priv->regs + APLIC_IDC_BASE + idc->hart_index * APLIC_IDC_SIZE;
 		idc->direct = direct;
 
 		aplic_idc_set_delivery(idc, true);
-- 
2.43.0

Re: [PATCH v5 1/2] dt-bindings: interrupt-controller: add risc-v,aplic hart indexes

[Rob Herring (Arm)]

On Wed, 29 Jan 2025 11:16:36 +0200, Vladimir Kondratiev wrote:
> Document optional property "riscv,hart-indexes"
> 
> Risc-V APLIC specification defines "hart index" in [1]:
> 
> Within a given interrupt domain, each of the domain’s harts has a
> unique index number in the range 0 to 2^14 − 1 (= 16,383). The index
> number a domain associates with a hart may or may not have any
> relationship to the unique hart identifier (“hart ID”) that the
> RISC-V Privileged Architecture assigns to the hart. Two different
> interrupt domains may employ entirely different index numbers for
> the same set of harts.
> 
> Further, this document says in "4.5 Memory-mapped control
> region for an interrupt domain":
> 
> The array of IDC structures may include some for potential hart index
> numbers that are not actual hart index numbers in the domain. For
> example, the first IDC structure is always for hart index 0, but 0 is
> not necessarily a valid index number for any hart in the domain.
> 
> Support arbitrary hart indexes specified in optional APLIC property
> "riscv,hart-indexes" that should be array of u32 elements, one per
> interrupt target. If this property not specified, fallback is to use
> logical hart indexes within the domain.
> 
> [1]: https://github.com/riscv/riscv-aia
> 
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
>  .../bindings/interrupt-controller/riscv,aplic.yaml        | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>