linux-kernel - Re: [PATCH] irqchip/loongarch-avec:Add multi-nodes topology support

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: <CAAhV-H4VAyD+=Btt9S6HrN6-sVQJqe1_jeLPS13UR1kxXJ8S5w@mail.gmail.com>
Date: Sat, 7 Dec 2024 17:11:52 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Tianyang Zhang <zhangtianyang@...ngson.cn>
Cc: kernel@...0n.name, tglx@...utronix.de, loongarch@...ts.linux.dev, 
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] irqchip/loongarch-avec:Add multi-nodes topology support

Hi, Tianyang,

On Sat, Dec 7, 2024 at 11:31 AM Tianyang Zhang
<zhangtianyang@...ngson.cn> wrote:
>
> This patch enables the advanced interrupt controller function under
> multiple-node of 3C600. The topology of the advanced interrupt controller
> is consistent with NUMA node. We check the enable status of the node where
> each CPU is located once when it goes online, which may cause some
> additional operations, but it can ensure that the advanced interrupt
> controller can still be used in situations where some CPUs cannot start
The whole can be reworded:

Currently we only enable one chip's advanced interrupt controller
(AVEC) in hardware, so multi-chip machines with Loongson-3C6000 don't
work. This patch enables it for every chip (In theory every chip needs
to be enabled only once, but for simplicity we enable it on every core
in the CPU online hooks).

>
> Signed-off-by: Tianyang Zhang <zhangtianyang@...ngson.cn>
> ---
>  drivers/irqchip/irq-loongarch-avec.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/irq-loongarch-avec.c b/drivers/irqchip/irq-loongarch-avec.c
> index 0f6e465dd309..9e30198fa7e4 100644
> --- a/drivers/irqchip/irq-loongarch-avec.c
> +++ b/drivers/irqchip/irq-loongarch-avec.c
> @@ -56,6 +56,18 @@ struct avecintc_data {
>         unsigned int            moving;
>  };
>
> +static inline void avecintc_enable(void)
> +{
> +       u64 value;
> +
> +       if (!loongarch_avec.domain)
> +               return;
Is there any reason to check this? If domain is NULL, avecintc_init()
fails, there is no chance to execute this function.

Huacai

> +
> +       value = iocsr_read64(LOONGARCH_IOCSR_MISC_FUNC);
> +       value |= IOCSR_MISC_FUNC_AVEC_EN;
> +       iocsr_write64(value, LOONGARCH_IOCSR_MISC_FUNC);
> +}
> +
>  static inline void avecintc_ack_irq(struct irq_data *d)
>  {
>  }
> @@ -127,6 +139,8 @@ static int avecintc_cpu_online(unsigned int cpu)
>
>         guard(raw_spinlock)(&loongarch_avec.lock);
>
> +       avecintc_enable();
> +
>         irq_matrix_online(loongarch_avec.vector_matrix);
>
>         pending_list_init(cpu);
> @@ -339,7 +353,6 @@ static int __init irq_matrix_init(void)
>  static int __init avecintc_init(struct irq_domain *parent)
>  {
>         int ret, parent_irq;
> -       unsigned long value;
>
>         raw_spin_lock_init(&loongarch_avec.lock);
>
> @@ -378,14 +391,13 @@ static int __init avecintc_init(struct irq_domain *parent)
>                                   "irqchip/loongarch/avecintc:starting",
>                                   avecintc_cpu_online, avecintc_cpu_offline);
>  #endif
> -       value = iocsr_read64(LOONGARCH_IOCSR_MISC_FUNC);
> -       value |= IOCSR_MISC_FUNC_AVEC_EN;
> -       iocsr_write64(value, LOONGARCH_IOCSR_MISC_FUNC);
> +       avecintc_enable();
>
>         return ret;
>
>  out_remove_domain:
>         irq_domain_remove(loongarch_avec.domain);
> +       loongarch_avec.domain = NULL;
>  out_free_handle:
>         irq_domain_free_fwnode(loongarch_avec.fwnode);
>  out:
> --
> 2.20.1
>