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] [day] [month] [year] [list]
Message-ID: <CAAhSdy1372ZY_irkfZ5-81C3KL+2ap9HkfRKTjGZWVT-nrGoNQ@mail.gmail.com>
Date: Tue, 6 Aug 2024 21:29: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 17/17] irqchip/sifive-plic: Add ACPI support

On Mon, Jul 29, 2024 at 7:54 PM Sunil V L <sunilvl@...tanamicro.com> wrote:
>
> Add ACPI support in PLIC driver. Use the mapping created early during
> boot to get details about the PLIC.
>
> Signed-off-by: Sunil V L <sunilvl@...tanamicro.com>
> Co-developed-by: Haibo Xu <haibo1.xu@...el.com>
> Signed-off-by: Haibo Xu <haibo1.xu@...el.com>
> ---
>  drivers/irqchip/irq-sifive-plic.c | 94 ++++++++++++++++++++++++-------
>  1 file changed, 73 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index 9e22f7e378f5..12d60728329c 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -3,6 +3,7 @@
>   * Copyright (C) 2017 SiFive
>   * Copyright (C) 2018 Christoph Hellwig
>   */
> +#include <linux/acpi.h>
>  #include <linux/cpu.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -70,6 +71,8 @@ struct plic_priv {
>         unsigned long plic_quirks;
>         unsigned int nr_irqs;
>         unsigned long *prio_save;
> +       u32 gsi_base;
> +       int id;

Same as the other patch, better to call "id" as "acpi_id".

>  };
>
>  struct plic_handler {
> @@ -324,6 +327,10 @@ static int plic_irq_domain_translate(struct irq_domain *d,
>  {
>         struct plic_priv *priv = d->host_data;
>
> +       /* For DT, gsi_base is always zero. */
> +       if (fwspec->param[0] >= priv->gsi_base)
> +               fwspec->param[0] = fwspec->param[0] - priv->gsi_base;
> +
>         if (test_bit(PLIC_QUIRK_EDGE_INTERRUPT, &priv->plic_quirks))
>                 return irq_domain_translate_twocell(d, fwspec, hwirq, type);
>
> @@ -424,18 +431,37 @@ static const struct of_device_id plic_match[] = {
>         {}
>  };
>
> +#ifdef CONFIG_ACPI
> +
> +static const struct acpi_device_id plic_acpi_match[] = {
> +       { "RSCV0001", 0 },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(acpi, plic_acpi_match);
> +
> +#endif
>  static int plic_parse_nr_irqs_and_contexts(struct platform_device *pdev,
> -                                          u32 *nr_irqs, u32 *nr_contexts)
> +                                          u32 *nr_irqs, u32 *nr_contexts,
> +                                          u32 *gsi_base, u32 *id)
>  {
>         struct device *dev = &pdev->dev;
>         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)) {
> +               rc = riscv_acpi_get_gsi_info(dev->fwnode, gsi_base, id, nr_irqs, NULL);
> +               if (rc) {
> +                       dev_err(dev, "failed to find GSI mapping\n");
> +                       return rc;
> +               }
> +
> +               *nr_contexts = acpi_get_plic_nr_contexts(*id);
> +               if (WARN_ON(!*nr_contexts)) {
> +                       dev_err(dev, "no PLIC context available\n");
> +                       return -EINVAL;
> +               }
> +
> +               return 0;
> +       }
>
>         rc = of_property_read_u32(to_of_node(dev->fwnode), "riscv,ndev", nr_irqs);
>         if (rc) {
> @@ -449,23 +475,29 @@ static int plic_parse_nr_irqs_and_contexts(struct platform_device *pdev,
>                 return -EINVAL;
>         }
>
> +       *gsi_base = 0;
> +       *id = 0;
> +
>         return 0;
>  }
>
>  static int plic_parse_context_parent(struct platform_device *pdev, u32 context,
> -                                    u32 *parent_hwirq, int *parent_cpu)
> +                                    u32 *parent_hwirq, int *parent_cpu, u32 id)
>  {
>         struct device *dev = &pdev->dev;
>         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(id, context);
> +               if (hartid == INVALID_HARTID)
> +                       return -EINVAL;
> +
> +               *parent_cpu = riscv_hartid_to_cpuid(hartid);
> +               *parent_hwirq = RV_IRQ_EXT;
> +               return 0;
> +       }
>
>         rc = of_irq_parse_one(to_of_node(dev->fwnode), context, &parent);
>         if (rc)
> @@ -489,6 +521,8 @@ static int plic_probe(struct platform_device *pdev)
>         u32 nr_irqs, parent_hwirq;
>         struct plic_priv *priv;
>         irq_hw_number_t hwirq;
> +       int id, context_id;
> +       u32 gsi_base;
>
>         if (is_of_node(dev->fwnode)) {
>                 const struct of_device_id *id;
> @@ -498,7 +532,7 @@ static int plic_probe(struct platform_device *pdev)
>                         plic_quirks = (unsigned long)id->data;
>         }
>
> -       error = plic_parse_nr_irqs_and_contexts(pdev, &nr_irqs, &nr_contexts);
> +       error = plic_parse_nr_irqs_and_contexts(pdev, &nr_irqs, &nr_contexts, &gsi_base, &id);
>         if (error)
>                 return error;
>
> @@ -509,6 +543,8 @@ static int plic_probe(struct platform_device *pdev)
>         priv->dev = dev;
>         priv->plic_quirks = plic_quirks;
>         priv->nr_irqs = nr_irqs;
> +       priv->gsi_base = gsi_base;
> +       priv->id = id;
>
>         priv->regs = devm_platform_ioremap_resource(pdev, 0);
>         if (WARN_ON(!priv->regs))
> @@ -519,12 +555,22 @@ static int plic_probe(struct platform_device *pdev)
>                 return -ENOMEM;
>
>         for (i = 0; i < nr_contexts; i++) {
> -               error = plic_parse_context_parent(pdev, i, &parent_hwirq, &cpu);
> +               error = plic_parse_context_parent(pdev, i, &parent_hwirq, &cpu, priv->id);
>                 if (error) {
>                         dev_warn(dev, "hwirq for context%d not found\n", i);
>                         continue;
>                 }
>
> +               if (is_of_node(dev->fwnode)) {
> +                       context_id = i;
> +               } else {
> +                       context_id = acpi_get_plic_context(priv->id, i);
> +                       if (context_id == INVALID_CONTEXT) {
> +                               dev_warn(dev, "invalid context id for context%d\n", i);
> +                               continue;
> +                       }
> +               }
> +
>                 /*
>                  * Skip contexts other than external interrupts for our
>                  * privilege level.
> @@ -562,10 +608,10 @@ static int plic_probe(struct platform_device *pdev)
>                 cpumask_set_cpu(cpu, &priv->lmask);
>                 handler->present = true;
>                 handler->hart_base = priv->regs + CONTEXT_BASE +
> -                       i * CONTEXT_SIZE;
> +                       context_id * CONTEXT_SIZE;
>                 raw_spin_lock_init(&handler->enable_lock);
>                 handler->enable_base = priv->regs + CONTEXT_ENABLE_BASE +
> -                       i * CONTEXT_ENABLE_SIZE;
> +                       context_id * CONTEXT_ENABLE_SIZE;
>                 handler->priv = priv;
>
>                 handler->enable_save = devm_kcalloc(dev, DIV_ROUND_UP(nr_irqs, 32),
> @@ -581,8 +627,8 @@ static int plic_probe(struct platform_device *pdev)
>                 nr_handlers++;
>         }
>
> -       priv->irqdomain = irq_domain_add_linear(to_of_node(dev->fwnode), nr_irqs + 1,
> -                                               &plic_irqdomain_ops, priv);
> +       priv->irqdomain = irq_domain_create_linear(dev->fwnode, nr_irqs + 1,
> +                                                  &plic_irqdomain_ops, priv);
>         if (WARN_ON(!priv->irqdomain))
>                 goto fail_cleanup_contexts;
>
> @@ -619,13 +665,18 @@ static int plic_probe(struct platform_device *pdev)
>                 }
>         }
>
> +#ifdef CONFIG_ACPI
> +       if (!acpi_disabled)
> +               acpi_dev_clear_dependencies(ACPI_COMPANION(dev));
> +#endif
> +
>         dev_info(dev, "mapped %d interrupts with %d handlers for %d contexts.\n",
>                  nr_irqs, nr_handlers, nr_contexts);
>         return 0;
>
>  fail_cleanup_contexts:
>         for (i = 0; i < nr_contexts; i++) {
> -               if (plic_parse_context_parent(pdev, i, &parent_hwirq, &cpu))
> +               if (plic_parse_context_parent(pdev, i, &parent_hwirq, &cpu, priv->id))
>                         continue;
>                 if (parent_hwirq != RV_IRQ_EXT || cpu < 0)
>                         continue;
> @@ -644,6 +695,7 @@ static struct platform_driver plic_driver = {
>         .driver = {
>                 .name           = "riscv-plic",
>                 .of_match_table = plic_match,
> +               .acpi_match_table = ACPI_PTR(plic_acpi_match),
>         },
>         .probe = plic_probe,
>  };
> --
> 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