| 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: <CAAhSdy0r++6enRx8m=XfKgacfnVau3_VWcArY93umYe5ET-gQw@mail.gmail.com>
Date: Tue, 6 Aug 2024 21:00:13 +0530
From: Anup Patel <anup@...infault.org>
To: Sunil V L <sunilvl@...tanamicro.com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-riscv@...ts.infradead.org, linux-pci@...r.kernel.org,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
"Rafael J . Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
Thomas Gleixner <tglx@...utronix.de>, Samuel Holland <samuel.holland@...ive.com>,
Robert Moore <robert.moore@...el.com>, Conor Dooley <conor.dooley@...rochip.com>,
Andrew Jones <ajones@...tanamicro.com>, Haibo Xu <haibo1.xu@...el.com>,
Atish Kumar Patra <atishp@...osinc.com>, Drew Fustini <dfustini@...storrent.com>
Subject: Re: [PATCH v7 15/17] irqchip/riscv-imsic: Add ACPI support
On Mon, Jul 29, 2024 at 7:54 PM Sunil V L <sunilvl@...tanamicro.com> wrote:
>
> RISC-V IMSIC interrupt controller provides IPI and MSI support.
> Currently, DT based drivers setup the IPI feature early during boot but
> defer setting up the MSI functionality. However, in ACPI systems, PCI
> subsystem is probed early and assume MSI controller is already setup.
> Hence, both IPI and MSI features need to be initialized early itself.
>
> Signed-off-by: Sunil V L <sunilvl@...tanamicro.com>
LGTM.
Reviewed-by: Anup Patel <anup@...infault.org>
Regards,
Anup
> ---
> drivers/irqchip/irq-riscv-imsic-early.c | 64 +++++++++++++++++++++-
> drivers/irqchip/irq-riscv-imsic-platform.c | 32 +++++++++--
> drivers/irqchip/irq-riscv-imsic-state.c | 57 +++++++++++--------
> drivers/irqchip/irq-riscv-imsic-state.h | 2 +-
> include/linux/irqchip/riscv-imsic.h | 9 +++
> 5 files changed, 134 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-imsic-early.c b/drivers/irqchip/irq-riscv-imsic-early.c
> index 4fbb37074d29..c5c2e6929a2f 100644
> --- a/drivers/irqchip/irq-riscv-imsic-early.c
> +++ b/drivers/irqchip/irq-riscv-imsic-early.c
> @@ -5,13 +5,16 @@
> */
>
> #define pr_fmt(fmt) "riscv-imsic: " fmt
> +#include <linux/acpi.h>
> #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/irqchip/riscv-imsic.h>
> #include <linux/module.h>
> +#include <linux/pci.h>
> #include <linux/spinlock.h>
> #include <linux/smp.h>
>
> @@ -182,7 +185,7 @@ static int __init imsic_early_dt_init(struct device_node *node, struct device_no
> int rc;
>
> /* Setup IMSIC state */
> - rc = imsic_setup_state(fwnode);
> + rc = imsic_setup_state(fwnode, NULL);
> if (rc) {
> pr_err("%pfwP: failed to setup state (error %d)\n", fwnode, rc);
> return rc;
> @@ -199,3 +202,62 @@ static int __init imsic_early_dt_init(struct device_node *node, struct device_no
> }
>
> IRQCHIP_DECLARE(riscv_imsic, "riscv,imsics", imsic_early_dt_init);
> +
> +#ifdef CONFIG_ACPI
> +
> +static struct fwnode_handle *imsic_acpi_fwnode;
> +
> +struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev)
> +{
> + return imsic_acpi_fwnode;
> +}
> +
> +static int __init imsic_early_acpi_init(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_imsic *imsic = (struct acpi_madt_imsic *)header;
> + int rc;
> +
> + imsic_acpi_fwnode = irq_domain_alloc_named_fwnode("imsic");
> + if (!imsic_acpi_fwnode) {
> + pr_err("unable to allocate IMSIC FW node\n");
> + return -ENOMEM;
> + }
> +
> + /* Setup IMSIC state */
> + rc = imsic_setup_state(imsic_acpi_fwnode, imsic);
> + if (rc) {
> + pr_err("%pfwP: failed to setup state (error %d)\n", imsic_acpi_fwnode, rc);
> + return rc;
> + }
> +
> + /* Do early setup of IMSIC state and IPIs */
> + rc = imsic_early_probe(imsic_acpi_fwnode);
> + if (rc) {
> + irq_domain_free_fwnode(imsic_acpi_fwnode);
> + imsic_acpi_fwnode = NULL;
> + return rc;
> + }
> +
> + rc = imsic_platform_acpi_probe(imsic_acpi_fwnode);
> +
> +#ifdef CONFIG_PCI
> + if (!rc)
> + pci_msi_register_fwnode_provider(&imsic_acpi_get_fwnode);
> +#endif
> +
> + if (rc)
> + pr_err("%pfwP: failed to register IMSIC for MSI functionality (error %d)\n",
> + imsic_acpi_fwnode, rc);
> +
> + /*
> + * Even if imsic_platform_acpi_probe() fails, the IPI part of IMSIC can
> + * continue to work. So, no need to return failure. This is similar to
> + * DT where IPI works but MSI probe fails for some reason.
> + */
> + return 0;
> +}
> +
> +IRQCHIP_ACPI_DECLARE(riscv_imsic, ACPI_MADT_TYPE_IMSIC, NULL,
> + 1, imsic_early_acpi_init);
> +#endif
> diff --git a/drivers/irqchip/irq-riscv-imsic-platform.c b/drivers/irqchip/irq-riscv-imsic-platform.c
> index 11723a763c10..64905e6f52d7 100644
> --- a/drivers/irqchip/irq-riscv-imsic-platform.c
> +++ b/drivers/irqchip/irq-riscv-imsic-platform.c
> @@ -5,6 +5,7 @@
> */
>
> #define pr_fmt(fmt) "riscv-imsic: " fmt
> +#include <linux/acpi.h>
> #include <linux/bitmap.h>
> #include <linux/cpu.h>
> #include <linux/interrupt.h>
> @@ -348,18 +349,37 @@ int imsic_irqdomain_init(void)
> return 0;
> }
>
> -static int imsic_platform_probe(struct platform_device *pdev)
> +static int imsic_platform_probe_common(struct fwnode_handle *fwnode)
> {
> - struct device *dev = &pdev->dev;
> -
> - if (imsic && imsic->fwnode != dev->fwnode) {
> - dev_err(dev, "fwnode mismatch\n");
> + if (imsic && imsic->fwnode != fwnode) {
> + pr_err("%pfwP: fwnode mismatch\n", fwnode);
> return -ENODEV;
> }
>
> return imsic_irqdomain_init();
> }
>
> +static int imsic_platform_dt_probe(struct platform_device *pdev)
> +{
> + return imsic_platform_probe_common(pdev->dev.fwnode);
> +}
> +
> +#ifdef CONFIG_ACPI
> +
> +/*
> + * On ACPI based systems, PCI enumeration happens early during boot in
> + * acpi_scan_init(). PCI enumeration expects MSI domain setup before
> + * it calls pci_set_msi_domain(). Hence, unlike in DT where
> + * imsic-platform drive probe happens late during boot, ACPI based
> + * systems need to setup the MSI domain early.
> + */
> +int imsic_platform_acpi_probe(struct fwnode_handle *fwnode)
> +{
> + return imsic_platform_probe_common(fwnode);
> +}
> +
> +#endif
> +
> static const struct of_device_id imsic_platform_match[] = {
> { .compatible = "riscv,imsics" },
> {}
> @@ -370,6 +390,6 @@ static struct platform_driver imsic_platform_driver = {
> .name = "riscv-imsic",
> .of_match_table = imsic_platform_match,
> },
> - .probe = imsic_platform_probe,
> + .probe = imsic_platform_dt_probe,
> };
> builtin_platform_driver(imsic_platform_driver);
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
> index f9e70832863a..73faa64bffda 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.c
> +++ b/drivers/irqchip/irq-riscv-imsic-state.c
> @@ -5,6 +5,7 @@
> */
>
> #define pr_fmt(fmt) "riscv-imsic: " fmt
> +#include <linux/acpi.h>
> #include <linux/cpu.h>
> #include <linux/bitmap.h>
> #include <linux/interrupt.h>
> @@ -564,18 +565,36 @@ static int __init imsic_populate_global_dt(struct fwnode_handle *fwnode,
> return 0;
> }
>
> +static int __init imsic_populate_global_acpi(struct fwnode_handle *fwnode,
> + struct imsic_global_config *global,
> + u32 *nr_parent_irqs, void *opaque)
> +{
> + struct acpi_madt_imsic *imsic = (struct acpi_madt_imsic *)opaque;
> +
> + global->guest_index_bits = imsic->guest_index_bits;
> + global->hart_index_bits = imsic->hart_index_bits;
> + global->group_index_bits = imsic->group_index_bits;
> + global->group_index_shift = imsic->group_index_shift;
> + global->nr_ids = imsic->num_ids;
> + global->nr_guest_ids = imsic->num_guest_ids;
> + return 0;
> +}
> +
> static int __init imsic_get_parent_hartid(struct fwnode_handle *fwnode,
> u32 index, unsigned long *hartid)
> {
> struct of_phandle_args parent;
> int rc;
>
> - /*
> - * Currently, only OF fwnode is supported so extend this
> - * function for ACPI support.
> - */
> - if (!is_of_node(fwnode))
> - return -EINVAL;
> + if (!is_of_node(fwnode)) {
> + if (hartid)
> + *hartid = acpi_get_intc_index_hartid(index);
> +
> + if (!hartid || (*hartid == INVALID_HARTID))
> + return -EINVAL;
> +
> + return 0;
> + }
>
> rc = of_irq_parse_one(to_of_node(fwnode), index, &parent);
> if (rc)
> @@ -594,12 +613,8 @@ static int __init imsic_get_parent_hartid(struct fwnode_handle *fwnode,
> static int __init imsic_get_mmio_resource(struct fwnode_handle *fwnode,
> u32 index, struct resource *res)
> {
> - /*
> - * Currently, only OF fwnode is supported so extend this
> - * function for ACPI support.
> - */
> if (!is_of_node(fwnode))
> - return -EINVAL;
> + return acpi_get_imsic_mmio_info(index, res);
>
> return of_address_to_resource(to_of_node(fwnode), index, res);
> }
> @@ -607,20 +622,14 @@ static int __init imsic_get_mmio_resource(struct fwnode_handle *fwnode,
> static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
> struct imsic_global_config *global,
> u32 *nr_parent_irqs,
> - u32 *nr_mmios)
> + u32 *nr_mmios,
> + void *opaque)
> {
> unsigned long hartid;
> struct resource res;
> int rc;
> u32 i;
>
> - /*
> - * Currently, only OF fwnode is supported so extend this
> - * function for ACPI support.
> - */
> - if (!is_of_node(fwnode))
> - return -EINVAL;
> -
> *nr_parent_irqs = 0;
> *nr_mmios = 0;
>
> @@ -632,7 +641,11 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
> return -EINVAL;
> }
>
> - rc = imsic_populate_global_dt(fwnode, global, nr_parent_irqs);
> + if (is_of_node(fwnode))
> + rc = imsic_populate_global_dt(fwnode, global, nr_parent_irqs);
> + else
> + rc = imsic_populate_global_acpi(fwnode, global, nr_parent_irqs, opaque);
> +
> if (rc)
> return rc;
>
> @@ -701,7 +714,7 @@ static int __init imsic_parse_fwnode(struct fwnode_handle *fwnode,
> return 0;
> }
>
> -int __init imsic_setup_state(struct fwnode_handle *fwnode)
> +int __init imsic_setup_state(struct fwnode_handle *fwnode, void *opaque)
> {
> u32 i, j, index, nr_parent_irqs, nr_mmios, nr_handlers = 0;
> struct imsic_global_config *global;
> @@ -742,7 +755,7 @@ int __init imsic_setup_state(struct fwnode_handle *fwnode)
> }
>
> /* Parse IMSIC fwnode */
> - rc = imsic_parse_fwnode(fwnode, global, &nr_parent_irqs, &nr_mmios);
> + rc = imsic_parse_fwnode(fwnode, global, &nr_parent_irqs, &nr_mmios, opaque);
> if (rc)
> goto out_free_local;
>
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.h b/drivers/irqchip/irq-riscv-imsic-state.h
> index 5ae2f69b035b..391e44280827 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.h
> +++ b/drivers/irqchip/irq-riscv-imsic-state.h
> @@ -102,7 +102,7 @@ void imsic_vector_debug_show_summary(struct seq_file *m, int ind);
>
> void imsic_state_online(void);
> void imsic_state_offline(void);
> -int imsic_setup_state(struct fwnode_handle *fwnode);
> +int imsic_setup_state(struct fwnode_handle *fwnode, void *opaque);
> int imsic_irqdomain_init(void);
>
> #endif
> diff --git a/include/linux/irqchip/riscv-imsic.h b/include/linux/irqchip/riscv-imsic.h
> index faf0b800b1b0..7494952c5518 100644
> --- a/include/linux/irqchip/riscv-imsic.h
> +++ b/include/linux/irqchip/riscv-imsic.h
> @@ -8,6 +8,8 @@
>
> #include <linux/types.h>
> #include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/fwnode.h>
> #include <asm/csr.h>
>
> #define IMSIC_MMIO_PAGE_SHIFT 12
> @@ -84,4 +86,11 @@ static inline const struct imsic_global_config *imsic_get_global_config(void)
>
> #endif
>
> +#ifdef CONFIG_ACPI
> +int imsic_platform_acpi_probe(struct fwnode_handle *fwnode);
> +struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev);
> +#else
> +static inline struct fwnode_handle *imsic_acpi_get_fwnode(struct device *dev) { return NULL; }
> +#endif
> +
> #endif
> --
> 2.43.0
>
Powered by blists - more mailing lists