Re: [PATCH 2/2] irqchip: Add Lightning Mountain irqchip support

[Florian Eckert]

Hello Thomas,

> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res)
> > +		return -EINVAL;
> > +
> > +	mdev->base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(mdev->base)) {
> > +		dev_err(&pdev->dev, "failed to ioremap %pR\n", res);
> > +		return PTR_ERR(mdev->base);
> > +	}
> > +
> > +	domain = irq_domain_create_hierarchy(x86_vector_domain, 0,
>
> So this is hardwired to the vector domain and does not allow the
> interrupts to be remapped? Those SoCs have VT-x which implies interrupt
> remapping support. But what do I know about the infinite wisdom of
> hardware designers.
>
> TBH, if they decided to hardwire it to the vector domain, then they are
> begging for a cluebat treatment.

Unfortunately, I don’t have a detailed hardware description for this
IP block. All I have from the Maxlinear is this driver [1] from
their SDK.

> Let me summarize what I can crystal-ball out of your comprehensive
> change log and the insane amount of comments in the code:
>
>     1) The IP block converts 'wired' interrupts to MSI messages
>
>     2) It needs to route four interrupts as NMI
>
> Right?

It has a total of 64 IRQs. Four of these are connected to the
individual CPU cores as NMIs.

> #1 The implementation gets the MSI interrupt domain concept completely
>    wrong
>
>    X86 uses the MSI parent domain concept.
>
>    [vector domain] -- [remap domain] -- [ device domain]
>
>    The remap domain is optional, but both the vector domain and the
>    remap domain act as MSI parent domains.
>
>    So what you want to create for that chip is a MSI device domain and
>    that domain needs to set the bus token to DOMAIN_BUS_WIRED_TO_MSI.
>
>    See drivers/irqchip/irq-mbigen.c mbigen_create_device_domain() and
>    related code as an example for a proper wired to MSI implementation.

Thanks for pointing that out. I’ll take a closer look at it this.

> #2 NMI routing
>
>    There has been attempts to implement that before in a clean way. The
>    patch set dried out, but the underlying changes for NMI support are
>    still valid and Ricardo (CC'ed) is working on them again, IIRC. See:
>
>
> https://lore.kernel.org/lkml/20230301234753.28582-1-ricardo.neri-calderon@xxxxxxxxxxxxxxx/

The v7 is already about two years old – let’s see if there’s anything
else coming.

All in all, thank you for taking the time to look at this. I’ll go back
to the drawing board and have another closer look at it.

Thanks *all* for your feedback.

Best regards

Florian

[1] 
https://github.com/maxlinear/linux/blob/updk_9.1.90/drivers/irqchip/irq-intel-soc-msi.c