linux-kernel - Re: [PATCH V2] 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-H56tOfi1oqnwh1O94Z3sAS7sAbeKQ_U_O6AD98YVq6F=w@mail.gmail.com>
Date: Tue, 31 Dec 2024 15:56:21 +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 V2] irqchip/loongarch-avec:Add multi-nodes topology support

Hi, Tianyang,

On Mon, Dec 30, 2024 at 10:15 AM Tianyang Zhang
<zhangtianyang@...ngson.cn> wrote:
>
> avecintc_init() enables the Advanced Interrupt Controller (AVEC) of
> the boot CPU node, but nothing enables the AVEC on secondary nodes.
>
> Move the enablement to the CPU hotplug callback so that secondary
> nodes get the AVEC enabled too. In theory enabling it once per node
> would be sufficient, but redundant enabling does no hard, so keep the
> code simple and do it unconditionally.
>
> Signed-off-by: Tianyang Zhang <zhangtianyang@...ngson.cn>
> ---
>  drivers/irqchip/irq-loongarch-avec.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/irq-loongarch-avec.c b/drivers/irqchip/irq-loongarch-avec.c
> index 0f6e465dd309..988d3e02ac54 100644
> --- a/drivers/irqchip/irq-loongarch-avec.c
> +++ b/drivers/irqchip/irq-loongarch-avec.c
> @@ -56,6 +56,15 @@ struct avecintc_data {
>         unsigned int            moving;
>  };
>
> +static inline void avecintc_enable(void)
> +{
> +       u64 value;
> +
> +       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 +136,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 +350,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 +388,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;
This is unnecessary, since no one checks it, and there is no memory
leak even if without it.

Huacai

>  out_free_handle:
>         irq_domain_free_fwnode(loongarch_avec.fwnode);
>  out:
> --
> 2.20.1
>