linux-kernel - Re: [PATCH V3 20/22] LoongArch: Add multi-processor (SMP) 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]

Date:   Fri, 17 Sep 2021 11:57:04 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Huacai Chen <chenhuacai@...ngson.cn>
Cc:     Arnd Bergmann <arnd@...db.de>, Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Airlie <airlied@...ux.ie>,
        Jonathan Corbet <corbet@....net>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Xuefeng Li <lixuefeng@...ngson.cn>,
        Yanteng Si <siyanteng@...ngson.cn>,
        Huacai Chen <chenhuacai@...il.com>,
        Jiaxun Yang <jiaxun.yang@...goat.com>
Subject: Re: [PATCH V3 20/22] LoongArch: Add multi-processor (SMP) support

On Fri, Sep 17, 2021 at 5:57 AM Huacai Chen <chenhuacai@...ngson.cn> wrote:

> +
> +struct task_struct;
> +
> +struct plat_smp_ops {
> +       void (*send_ipi_single)(int cpu, unsigned int action);
> +       void (*send_ipi_mask)(const struct cpumask *mask, unsigned int action);
> +       void (*smp_setup)(void);
> +       void (*prepare_cpus)(unsigned int max_cpus);
> +       int (*boot_secondary)(int cpu, struct task_struct *idle);
> +       void (*init_secondary)(void);
> +       void (*smp_finish)(void);
> +#ifdef CONFIG_HOTPLUG_CPU
> +       int (*cpu_disable)(void);
> +       void (*cpu_die)(unsigned int cpu);
> +#endif
> +};


Do you foresee having more than one implementation of these in the
near future? If not, I would suggest leaving out the extra indirection
and just using direct function calls.

> +#ifdef CONFIG_SMP
> +
> +static inline void plat_smp_setup(void)
> +{
> +       extern const struct plat_smp_ops *mp_ops;       /* private */
> +
> +       mp_ops->smp_setup();
> +}
> +
> +#else /* !CONFIG_SMP */
> +
> +static inline void plat_smp_setup(void) { }
> +
> +#endif /* !CONFIG_SMP */

You could even go further and do what arch/arm64 has, making
SMP support unconditional. This obviously depends on hardware
roadmaps, but if all harfdware you support has multiple cores,
then non-SMP mode just adds complexity.

> +
> +#define MAX_CPUS 64

You CONFIG_NR_CPUS allows up to 256. I think you need to
adjust one of the numbers to match the other, or remove this
definition and just use CONFIG_NR_CPUS directly.

> +
> +static volatile void *ipi_set_regs[MAX_CPUS];
> +static volatile void *ipi_clear_regs[MAX_CPUS];
> +static volatile void *ipi_status_regs[MAX_CPUS];
> +static volatile void *ipi_en_regs[MAX_CPUS];
> +static volatile void *ipi_mailbox_buf[MAX_CPUS];

Why are these 'volatile'? If they are MMIO registers, they
should be __iomem, and accessed using readl()/writel()
 etc

       Arnd