| 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
| ||
|
Message-ID: <tgkep7hurskxfgsfe7qogmxmym3im5isfkwhv6qvpa35uzzvkg@7gkplu67ohwf>
Date: Sun, 20 Oct 2024 12:01:43 +0800
From: Inochi Amaoto <inochiama@...il.com>
To: Samuel Holland <samuel.holland@...ive.com>,
Inochi Amaoto <inochiama@...il.com>
Cc: Yixun Lan <dlan@...too.org>, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org,
Thomas Gleixner <tglx@...utronix.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Peter Zijlstra <peterz@...radead.org>,
Chen Wang <unicorn_wang@...look.com>, Inochi Amaoto <inochiama@...look.com>,
Guo Ren <guoren@...nel.org>, Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Yangyu Chen <cyy@...self.name>,
Jinyu Tang <tangjinyu@...ylab.org>, Hal Feng <hal.feng@...rfivetech.com>,
Geert Uytterhoeven <geert+renesas@...der.be>
Subject: Re: [PATCH v2 2/3] irqchip: add T-HEAD C900 ACLINT SSWI driver
On Tue, Oct 15, 2024 at 05:43:16PM -0500, Samuel Holland wrote:
> Hi Inochi,
>
> On 2024-10-09 5:44 PM, Inochi Amaoto wrote:
> > Add a driver for the T-HEAD C900 ACLINT SSWI device, which is an
> > enhanced implementation of the RISC-V ACLINT SSWI specification.
> > This device allows the system to send ipi via fast device interface.
> >
> > Signed-off-by: Inochi Amaoto <inochiama@...il.com>
> > ---
> > drivers/irqchip/Kconfig | 10 ++
> > drivers/irqchip/Makefile | 1 +
> > drivers/irqchip/irq-thead-c900-aclint-sswi.c | 166 +++++++++++++++++++
> > include/linux/cpuhotplug.h | 1 +
> > 4 files changed, 178 insertions(+)
> > create mode 100644 drivers/irqchip/irq-thead-c900-aclint-sswi.c
> >
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 341cd9ca5a05..32671385cbb7 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -611,6 +611,16 @@ config STARFIVE_JH8100_INTC
> >
> > If you don't know what to do here, say Y.
> >
> > +config THEAD_C900_ACLINT_SSWI
> > + bool "THEAD C9XX ACLINT S-mode IPI Interrupt Controller"
> > + depends on RISCV
> > + select IRQ_DOMAIN_HIERARCHY
> > + help
> > + This enables support for T-HEAD specific ACLINT SSWI device
> > + support.
> > +
> > + If you don't know what to do here, say Y.
> > +
> > config EXYNOS_IRQ_COMBINER
> > bool "Samsung Exynos IRQ combiner support" if COMPILE_TEST
> > depends on (ARCH_EXYNOS && ARM) || COMPILE_TEST
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index e3679ec2b9f7..583418261253 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -101,6 +101,7 @@ obj-$(CONFIG_RISCV_APLIC_MSI) += irq-riscv-aplic-msi.o
> > obj-$(CONFIG_RISCV_IMSIC) += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
> > obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o
> > obj-$(CONFIG_STARFIVE_JH8100_INTC) += irq-starfive-jh8100-intc.o
> > +obj-$(CONFIG_THEAD_C900_ACLINT_SSWI) += irq-thead-c900-aclint-sswi.o
> > obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o
> > obj-$(CONFIG_IMX_INTMUX) += irq-imx-intmux.o
> > obj-$(CONFIG_IMX_MU_MSI) += irq-imx-mu-msi.o
> > diff --git a/drivers/irqchip/irq-thead-c900-aclint-sswi.c b/drivers/irqchip/irq-thead-c900-aclint-sswi.c
> > new file mode 100644
> > index 000000000000..b96d3b81dc14
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-thead-c900-aclint-sswi.c
> > @@ -0,0 +1,166 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2024 Inochi Amaoto <inochiama@...il.com>
> > + */
> > +
> > +#define pr_fmt(fmt) "thead-c900-aclint-sswi: " fmt
> > +#include <linux/cpu.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/irq.h>
> > +#include <linux/irqchip.h>
> > +#include <linux/irqchip/chained_irq.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/pci.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/smp.h>
> > +#include <linux/string_choices.h>
> > +
> > +#define ACLINT_xSWI_REGISTER_SIZE 4
> > +
> > +static int sswi_ipi_virq __ro_after_init;
> > +static DEFINE_PER_CPU(void __iomem *, sswi_cpu_regs);
> > +
> > +static void thead_aclint_sswi_ipi_send(unsigned int cpu)
> > +{
> > + writel_relaxed(0x1, per_cpu(sswi_cpu_regs, cpu));
> > +}
> > +
> > +static void thead_aclint_sswi_ipi_clear(void)
> > +{
> > + writel_relaxed(0x0, this_cpu_read(sswi_cpu_regs));
>
> This isn't quite compliant with the ACLINT spec[1], which states: "Writing 0 to
> the least significant bit of a SETSSIP register has no effect". In a RISC-V
> ACLINT, only the CSR write is required to clear the interrupt.
>
> This implementation does match the behavior of the T-HEAD CLINT extensions which
> are also present in C906/C910[2][3][4].
>
Yes, this is T-HEAD specific one, it does not match the spec exactly.
This is why I add "thead" prefix.
> This raises the question: in the older CPUs, using this functionality requires
> setting the mxstatus.CLINTEE bit from M-mode. Is this still the case for the
> C920 CPU in SG2044? If so, the driver should check sxstatus.CLINTEE when probing.
>
It seems that I forgot this. The SG2044 also needs to check the CLINTEE bit when
probing, it just set this bit in its zsbl. I will fix this in the next version.
Thanks for points it.
> It would also be ideal if we could support the other SoCs that use this same IP
> block, but with the other CLINT binding.
>
> Regards,
> Samuel
>
> [1]: https://github.com/riscv/riscv-aclint/blob/main/riscv-aclint.adoc
> [2]:
> https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource/XuanTie-OpenC906-UserManual.pdf
> [3]:
> https://github.com/XUANTIE-RV/openc906/blob/main/C906_RTL_FACTORY/gen_rtl/clint/rtl/clint_func.v#L285
> [4]:
> https://github.com/XUANTIE-RV/openc906/blob/main/C906_RTL_FACTORY/gen_rtl/cp0/rtl/aq_cp0_trap_csr.v#L1240
>
> > +}
> > +
> > +static void thead_aclint_sswi_ipi_handle(struct irq_desc *desc)
> > +{
> > + struct irq_chip *chip = irq_desc_get_chip(desc);
> > +
> > + chained_irq_enter(chip, desc);
> > +
> > + csr_clear(CSR_IP, IE_SIE);
> > + thead_aclint_sswi_ipi_clear();
> > +
> > + ipi_mux_process();
> > +
> > + chained_irq_exit(chip, desc);
> > +}
> > +
> > +static int thead_aclint_sswi_starting_cpu(unsigned int cpu)
> > +{
> > + enable_percpu_irq(sswi_ipi_virq, irq_get_trigger_type(sswi_ipi_virq));
> > +
> > + return 0;
> > +}
> > +
> > +static int thead_aclint_sswi_dying_cpu(unsigned int cpu)
> > +{
> > + thead_aclint_sswi_ipi_clear();
> > +
> > + disable_percpu_irq(sswi_ipi_virq);
> > +
> > + return 0;
> > +}
> > +
> > +static int __init aclint_sswi_parse_irq(struct fwnode_handle *fwnode,
> > + void __iomem *reg)
> > +{
> > + struct of_phandle_args parent;
> > + unsigned long hartid;
> > + u32 contexts, i;
> > + int rc, cpu;
> > +
> > + contexts = of_irq_count(to_of_node(fwnode));
> > + if (!(contexts)) {
> > + pr_err("%pfwP: no ACLINT SSWI context available\n", fwnode);
> > + return -EINVAL;
> > + }
> > +
> > + for (i = 0; i < contexts; i++) {
> > + rc = of_irq_parse_one(to_of_node(fwnode), i, &parent);
> > + if (rc)
> > + return rc;
> > +
> > + rc = riscv_of_parent_hartid(parent.np, &hartid);
> > + if (rc)
> > + return rc;
> > +
> > + if (parent.args[0] != RV_IRQ_SOFT)
> > + return -ENOTSUPP;
> > +
> > + cpu = riscv_hartid_to_cpuid(hartid);
> > +
> > + per_cpu(sswi_cpu_regs, cpu) = reg + i * ACLINT_xSWI_REGISTER_SIZE;
> > + }
> > +
> > + pr_info("%pfwP: register %u CPU%s\n", fwnode, contexts, str_plural(contexts));
> > +
> > + return 0;
> > +}
> > +
> > +static int __init aclint_sswi_probe(struct fwnode_handle *fwnode)
> > +{
> > + struct irq_domain *domain;
> > + void __iomem *reg;
> > + int virq, rc;
> > +
> > + if (!is_of_node(fwnode))
> > + return -EINVAL;
> > +
> > + reg = of_iomap(to_of_node(fwnode), 0);
> > + if (!reg)
> > + return -ENOMEM;
> > +
> > + /* Parse SSWI setting */
> > + rc = aclint_sswi_parse_irq(fwnode, reg);
> > + if (rc < 0)
> > + return rc;
> > +
> > + /* If mulitple SSWI devices are present, do not register irq again */
> > + if (sswi_ipi_virq)
> > + return 0;
> > +
> > + /* Find riscv intc domain and create IPI irq mapping */
> > + domain = irq_find_matching_fwnode(riscv_get_intc_hwnode(), DOMAIN_BUS_ANY);
> > + if (!domain) {
> > + pr_err("%pfwP: Failed to find INTC domain\n", fwnode);
> > + return -ENOENT;
> > + }
> > +
> > + sswi_ipi_virq = irq_create_mapping(domain, RV_IRQ_SOFT);
> > + if (!sswi_ipi_virq) {
> > + pr_err("unable to create ACLINT SSWI IRQ mapping\n");
> > + return -ENOMEM;
> > + }
> > +
> > + /* Register SSWI irq and handler */
> > + virq = ipi_mux_create(BITS_PER_BYTE, thead_aclint_sswi_ipi_send);
> > + if (virq <= 0) {
> > + pr_err("unable to create muxed IPIs\n");
> > + irq_dispose_mapping(sswi_ipi_virq);
> > + return virq < 0 ? virq : -ENOMEM;
> > + }
> > +
> > + irq_set_chained_handler(sswi_ipi_virq, thead_aclint_sswi_ipi_handle);
> > +
> > + cpuhp_setup_state(CPUHP_AP_IRQ_THEAD_ACLINT_SSWI_STARTING,
> > + "irqchip/thead-aclint-sswi:starting",
> > + thead_aclint_sswi_starting_cpu,
> > + thead_aclint_sswi_dying_cpu);
> > +
> > + riscv_ipi_set_virq_range(virq, BITS_PER_BYTE);
> > +
> > + /* Announce that SSWI is providing IPIs */
> > + pr_info("providing IPIs using THEAD ACLINT SSWI\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int __init aclint_sswi_early_probe(struct device_node *node,
> > + struct device_node *parent)
> > +{
> > + return aclint_sswi_probe(&node->fwnode);
> > +}
> > +IRQCHIP_DECLARE(thead_aclint_sswi, "thead,c900-aclint-sswi", aclint_sswi_early_probe);
> > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > index 2361ed4d2b15..799052249c7b 100644
> > --- a/include/linux/cpuhotplug.h
> > +++ b/include/linux/cpuhotplug.h
> > @@ -147,6 +147,7 @@ enum cpuhp_state {
> > CPUHP_AP_IRQ_EIOINTC_STARTING,
> > CPUHP_AP_IRQ_AVECINTC_STARTING,
> > CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
> > + CPUHP_AP_IRQ_THEAD_ACLINT_SSWI_STARTING,
> > CPUHP_AP_IRQ_RISCV_IMSIC_STARTING,
> > CPUHP_AP_IRQ_RISCV_SBI_IPI_STARTING,
> > CPUHP_AP_ARM_MVEBU_COHERENCY,
>
Powered by blists - more mailing lists