Re: [RFC PATCH 01/15] irqchip/riscv-imsic: Use hierarchy to reach irq_set_affinity

Andrew Jones
2024-12-05T16:12:28Z

On Tue, Dec 03, 2024 at 05:50:13PM +0100, Thomas Gleixner wrote:
> On Tue, Dec 03 2024 at 17:27, Andrew Jones wrote:
> > On Tue, Dec 03, 2024 at 02:53:45PM +0100, Thomas Gleixner wrote:
> >> On Thu, Nov 14 2024 at 17:18, Andrew Jones wrote:
> >> The whole IMSIC MSI support can be moved over to MSI LIB which makes all
> >> of this indirection go away and your intermediate domain will just fit
> >> in.
> >>
> >> Uncompiled patch below. If that works, it needs to be split up properly.
> >
> > Thanks Thomas. I gave your patch below a go, but we now fail to have an
> > msi domain set up when probing devices which go through aplic_msi_setup(),
> > resulting in an immediate NULL deference in
> > msi_create_device_irq_domain(). I'll look closer tomorrow.
>
> Duh! I forgot to update the .select callback. I don't know how you fixed that
> compile fail up. Delta patch below.
>
> Thanks,
>
> tglx
> ---
> --- a/drivers/irqchip/irq-riscv-imsic-platform.c
> +++ b/drivers/irqchip/irq-riscv-imsic-platform.c
> @@ -180,7 +180,7 @@ static void imsic_irq_debug_show(struct
> static const struct irq_domain_ops imsic_base_domain_ops = {
> .alloc = imsic_irq_domain_alloc,
> .free = imsic_irq_domain_free,
> - .select = imsic_irq_domain_select,
> + .select = msi_lib_irq_domain_select,
> #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
> .debug_show = imsic_irq_debug_show,
> #endif

Hi Thomas,

With this select fix and replacing patch 03/15 of this series with the
following diff, this irqbypass PoC still works.

Based on what Anup said, I kept imsic_msi_update_msg(), which means I
kept this entire patch (01/15) as is. Anup is working on a series to fix
the non-atomic MSI message writes to the device and will likely pick this
patch up along with your changes to convert IMSIC to msi-lib.

I'd like to know your opinion on patch 02/15 of this series and the diff
below. afaict, x86 does something similar with the DOMAIN_BUS_DMAR and
DOMAIN_BUS_AMDVI tokens in x86_init_dev_msi_info().

Thanks,
drew

diff --git a/drivers/irqchip/irq-msi-lib.c b/drivers/irqchip/irq-msi-lib.c
index 51464c6257f3..cc18516a4e82 100644
--- a/drivers/irqchip/irq-msi-lib.c
+++ b/drivers/irqchip/irq-msi-lib.c
@@ -36,14 +36,14 @@ bool msi_lib_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
return false;

/*
- * MSI parent domain specific settings. For now there is only the
- * root parent domain, e.g. NEXUS, acting as a MSI parent, but it is
- * possible to stack MSI parents. See x86 vector -> irq remapping
+ * MSI parent domain specific settings. There may be only the root
+ * parent domain, e.g. NEXUS, acting as a MSI parent, or there may
+ * be stacked MSI parents, typically used for remapping.
*/
if (domain->bus_token == pops->bus_select_token) {
if (WARN_ON_ONCE(domain != real_parent))
return false;
- } else {
+ } else if (real_parent->bus_token != DOMAIN_BUS_MSI_REMAP) {
WARN_ON_ONCE(1);
return false;
}