linux-kernel - Re: [PATCH v2] irqchip/riscv-aplic: add support for hart indexes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAhSdy0U3VXqKK-pReUSo5PYJhr0YGoO6pT194Me0Q18dHbE9g@mail.gmail.com>
Date: Mon, 6 Jan 2025 21:52:51 +0530
From: Anup Patel <anup@...infault.org>
To: Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] irqchip/riscv-aplic: add support for hart indexes

On Sun, Jan 5, 2025 at 2:09 PM Vladimir Kondratiev
<vladimir.kondratiev@...ileye.com> wrote:
>
> 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-index" that should be array of u32 elements, one per

I had commented on v1 that it's better to rename this property to
"riscv,hart-indexes" (plural).

> 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@...ileye.com>

Otherwise, this looks good to me.

Reviewed-by: Anup Patel <anup@...infault.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..c80a65c1732a 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-index";
> +       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
>

Regards,
Anup