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: <CAAhSdy2EERGy2siE4mCCGNN2ydfiKiMGwrsYts=bb52WAViYwg@mail.gmail.com>
Date: Tue, 6 Aug 2024 21:24:53 +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 16/17] irqchip/riscv-aplic: Add ACPI support

On Mon, Jul 29, 2024 at 7:54 PM Sunil V L <sunilvl@...tanamicro.com> wrote:
>
> Add ACPI support in APLIC drivers. Use the mapping created early during
> boot to get the details about the APLIC.
>
> Signed-off-by: Sunil V L <sunilvl@...tanamicro.com>
> ---
>  drivers/irqchip/irq-riscv-aplic-direct.c | 22 +++++---
>  drivers/irqchip/irq-riscv-aplic-main.c   | 69 ++++++++++++++++--------
>  drivers/irqchip/irq-riscv-aplic-main.h   |  1 +
>  drivers/irqchip/irq-riscv-aplic-msi.c    |  9 +++-
>  4 files changed, 69 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-aplic-direct.c b/drivers/irqchip/irq-riscv-aplic-direct.c
> index 4a3ffe856d6c..34540a0ca4da 100644
> --- a/drivers/irqchip/irq-riscv-aplic-direct.c
> +++ b/drivers/irqchip/irq-riscv-aplic-direct.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2022 Ventana Micro Systems Inc.
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/cpu.h>
> @@ -189,17 +190,22 @@ static int aplic_direct_starting_cpu(unsigned int cpu)
>  }
>
>  static int aplic_direct_parse_parent_hwirq(struct device *dev, u32 index,
> -                                          u32 *parent_hwirq, unsigned long *parent_hartid)
> +                                          u32 *parent_hwirq, unsigned long *parent_hartid,
> +                                          struct aplic_priv *priv)
>  {
>         struct of_phandle_args parent;
> +       unsigned long hartid;
>         int rc;
>
> -       /*
> -        * Currently, only OF fwnode is supported so extend this
> -        * function for ACPI support.
> -        */
> -       if (!is_of_node(dev->fwnode))
> -               return -EINVAL;
> +       if (!is_of_node(dev->fwnode)) {
> +               hartid = acpi_get_ext_intc_parent_hartid(priv->id, index);
> +               if (hartid == INVALID_HARTID)
> +                       return -ENODEV;
> +
> +               *parent_hartid = hartid;
> +               *parent_hwirq = RV_IRQ_EXT;
> +               return 0;
> +       }
>
>         rc = of_irq_parse_one(to_of_node(dev->fwnode), index, &parent);
>         if (rc)
> @@ -237,7 +243,7 @@ int aplic_direct_setup(struct device *dev, void __iomem *regs)
>         /* Setup per-CPU IDC and target CPU mask */
>         current_cpu = get_cpu();
>         for (i = 0; i < priv->nr_idcs; i++) {
> -               rc = aplic_direct_parse_parent_hwirq(dev, i, &hwirq, &hartid);
> +               rc = aplic_direct_parse_parent_hwirq(dev, i, &hwirq, &hartid, priv);
>                 if (rc) {
>                         dev_warn(dev, "parent irq for IDC%d not found\n", i);
>                         continue;
> diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
> index 28dd175b5764..8357c5f9921a 100644
> --- a/drivers/irqchip/irq-riscv-aplic-main.c
> +++ b/drivers/irqchip/irq-riscv-aplic-main.c
> @@ -4,8 +4,10 @@
>   * Copyright (C) 2022 Ventana Micro Systems Inc.
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/bitfield.h>
>  #include <linux/irqchip/riscv-aplic.h>
> +#include <linux/irqchip/riscv-imsic.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
> @@ -125,39 +127,50 @@ static void aplic_init_hw_irqs(struct aplic_priv *priv)
>         writel(0, priv->regs + APLIC_DOMAINCFG);
>  }
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id aplic_acpi_match[] = {
> +       { "RSCV0002", 0 },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(acpi, aplic_acpi_match);
> +
> +#endif
> +
>  int aplic_setup_priv(struct aplic_priv *priv, struct device *dev, void __iomem *regs)
>  {
>         struct device_node *np = to_of_node(dev->fwnode);
>         struct of_phandle_args parent;
>         int rc;
>
> -       /*
> -        * Currently, only OF fwnode is supported so extend this
> -        * function for ACPI support.
> -        */
> -       if (!np)
> -               return -EINVAL;
> -
>         /* Save device pointer and register base */
>         priv->dev = dev;
>         priv->regs = regs;
>
> -       /* Find out number of interrupt sources */
> -       rc = of_property_read_u32(np, "riscv,num-sources", &priv->nr_irqs);
> -       if (rc) {
> -               dev_err(dev, "failed to get number of interrupt sources\n");
> -               return rc;
> -       }
> +       if (np) {
> +               /* Find out number of interrupt sources */
> +               rc = of_property_read_u32(np, "riscv,num-sources", &priv->nr_irqs);
> +               if (rc) {
> +                       dev_err(dev, "failed to get number of interrupt sources\n");
> +                       return rc;
> +               }
>
> -       /*
> -        * Find out number of IDCs based on parent interrupts
> -        *
> -        * If "msi-parent" property is present then we ignore the
> -        * APLIC IDCs which forces the APLIC driver to use MSI mode.
> -        */
> -       if (!of_property_present(np, "msi-parent")) {
> -               while (!of_irq_parse_one(np, priv->nr_idcs, &parent))
> -                       priv->nr_idcs++;
> +               /*
> +                * Find out number of IDCs based on parent interrupts
> +                *
> +                * If "msi-parent" property is present then we ignore the
> +                * APLIC IDCs which forces the APLIC driver to use MSI mode.
> +                */
> +               if (!of_property_present(np, "msi-parent")) {
> +                       while (!of_irq_parse_one(np, priv->nr_idcs, &parent))
> +                               priv->nr_idcs++;
> +               }
> +       } else {
> +               rc = riscv_acpi_get_gsi_info(dev->fwnode, &priv->gsi_base, &priv->id,
> +                                            &priv->nr_irqs, &priv->nr_idcs);
> +               if (rc) {
> +                       dev_err(dev, "failed to find GSI mapping\n");
> +                       return rc;
> +               }
>         }
>
>         /* Setup initial state APLIC interrupts */
> @@ -184,7 +197,11 @@ static int aplic_probe(struct platform_device *pdev)
>          * If msi-parent property is present then setup APLIC MSI
>          * mode otherwise setup APLIC direct mode.
>          */
> -       msi_mode = of_property_present(to_of_node(dev->fwnode), "msi-parent");
> +       if (is_of_node(dev->fwnode))
> +               msi_mode = of_property_present(to_of_node(dev->fwnode), "msi-parent");
> +       else
> +               msi_mode = imsic_acpi_get_fwnode(NULL) ? 1 : 0;
> +
>         if (msi_mode)
>                 rc = aplic_msi_setup(dev, regs);
>         else
> @@ -192,6 +209,11 @@ static int aplic_probe(struct platform_device *pdev)
>         if (rc)
>                 dev_err(dev, "failed to setup APLIC in %s mode\n", msi_mode ? "MSI" : "direct");
>
> +#ifdef CONFIG_ACPI
> +       if (!acpi_disabled)
> +               acpi_dev_clear_dependencies(ACPI_COMPANION(dev));
> +#endif
> +
>         return rc;
>  }
>
> @@ -204,6 +226,7 @@ static struct platform_driver aplic_driver = {
>         .driver = {
>                 .name           = "riscv-aplic",
>                 .of_match_table = aplic_match,
> +               .acpi_match_table = ACPI_PTR(aplic_acpi_match),
>         },
>         .probe = aplic_probe,
>  };
> diff --git a/drivers/irqchip/irq-riscv-aplic-main.h b/drivers/irqchip/irq-riscv-aplic-main.h
> index 4393927d8c80..9fbf45c7b4f7 100644
> --- a/drivers/irqchip/irq-riscv-aplic-main.h
> +++ b/drivers/irqchip/irq-riscv-aplic-main.h
> @@ -28,6 +28,7 @@ struct aplic_priv {
>         u32                     gsi_base;
>         u32                     nr_irqs;
>         u32                     nr_idcs;
> +       u32                     id;

This "id" is only used for ACPI so better call it "acpi_id".

>         void __iomem            *regs;
>         struct aplic_msicfg     msicfg;
>  };
> diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
> index 028444af48bd..f5020241e0ed 100644
> --- a/drivers/irqchip/irq-riscv-aplic-msi.c
> +++ b/drivers/irqchip/irq-riscv-aplic-msi.c
> @@ -157,6 +157,7 @@ static const struct msi_domain_template aplic_msi_template = {
>  int aplic_msi_setup(struct device *dev, void __iomem *regs)
>  {
>         const struct imsic_global_config *imsic_global;
> +       struct irq_domain *msi_domain;
>         struct aplic_priv *priv;
>         struct aplic_msicfg *mc;
>         phys_addr_t pa;
> @@ -239,8 +240,14 @@ int aplic_msi_setup(struct device *dev, void __iomem *regs)
>                  * IMSIC and the IMSIC MSI domains are created later through
>                  * the platform driver probing so we set it explicitly here.
>                  */
> -               if (is_of_node(dev->fwnode))
> +               if (is_of_node(dev->fwnode)) {
>                         of_msi_configure(dev, to_of_node(dev->fwnode));
> +               } else {
> +                       msi_domain = irq_find_matching_fwnode(imsic_acpi_get_fwnode(dev),
> +                                                             DOMAIN_BUS_PLATFORM_MSI);
> +                       if (msi_domain)
> +                               dev_set_msi_domain(dev, msi_domain);
> +               }
>         }
>
>         if (!msi_create_device_irq_domain(dev, MSI_DEFAULT_DOMAIN, &aplic_msi_template,
> --
> 2.43.0
>

Otherwise, this looks good to me.

Reviewed-by: Anup Patel <anup@...infault.org>

Regards,
Anup

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ