On Mon, Jul 29, 2024 at 7:54 PM Sunil V L <sunilvl@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx> LGTM. Reviewed-by: Anup Patel <anup@xxxxxxxxxxxxxx> 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 >