[RFC,v2,06/21] RISC-V: Kconfig: Select deferred GSI probe for ACPI systems

  On RISC-V platforms, apart from root interrupt controllers (which
provide local interrupts and IPI), other interrupt controllers in the
hierarchy are probed late. Enable this select this CONFIG option for
RISC-V platforms so that device drivers which connect to deferred
interrupt controllers can take appropriate action.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 arch/riscv/Kconfig | 1 +
 1 file changed, 1 insertion(+)
  

On Thu, Oct 26, 2023 at 01:53:29AM +0530, Sunil V L wrote:
> On RISC-V platforms, apart from root interrupt controllers (which
> provide local interrupts and IPI), other interrupt controllers in the
> hierarchy are probed late. Enable this select this CONFIG option for
> RISC-V platforms so that device drivers which connect to deferred
> interrupt controllers can take appropriate action.

Quite a bit of this series seems related to the question of interrupt
controllers being probed "late".

I don't see anything specific about *how* late this might be, but from
the use of -EPROBE_DEFER in individual drivers (8250_pnp explicitly,
and acpi_register_gsi() and pnp_irq() and acpi_pci_irq_enable(), which
are called from driver .probe() paths) it seems like interrupt
controllers might be detected even after devices that use them.

That seems like a fairly invasive change to the driver probe flow.
If we really need to do that, I think it might merit a little more
background as justification since we haven't had to do it for any
other arch yet.

Bjorn

> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
>  arch/riscv/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 8c105a151e12..b62441aefa6a 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -12,6 +12,7 @@ config 32BIT
>  
>  config RISCV
>  	def_bool y
> +	select ARCH_ACPI_DEFERRED_GSI if ACPI
>  	select ACPI_GENERIC_GSI if ACPI
>  	select ACPI_MCFG if (ACPI && PCI)
>  	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
> -- 
> 2.39.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
  

On Thu, Oct 26, 2023 at 12:04:08PM -0500, Bjorn Helgaas wrote:
> On Thu, Oct 26, 2023 at 01:53:29AM +0530, Sunil V L wrote:
> > On RISC-V platforms, apart from root interrupt controllers (which
> > provide local interrupts and IPI), other interrupt controllers in the
> > hierarchy are probed late. Enable this select this CONFIG option for
> > RISC-V platforms so that device drivers which connect to deferred
> > interrupt controllers can take appropriate action.
> 
> Quite a bit of this series seems related to the question of interrupt
> controllers being probed "late".
> 
> I don't see anything specific about *how* late this might be, but from
> the use of -EPROBE_DEFER in individual drivers (8250_pnp explicitly,
> and acpi_register_gsi() and pnp_irq() and acpi_pci_irq_enable(), which
> are called from driver .probe() paths) it seems like interrupt
> controllers might be detected even after devices that use them.
> 
> That seems like a fairly invasive change to the driver probe flow.
> If we really need to do that, I think it might merit a little more
> background as justification since we haven't had to do it for any
> other arch yet.
> 

Hi Bjorn,

In RISC-V, the APLIC can be a converter from wired (GSI) to MSI interrupts.
Hence, especially in this mode, it has to be a platform device to use
device MSI domain. Also, according to Marc Zyngier there is no reason to
probe interrupt controllers early apart from root controller. So, the
device drivers which use wired interrupts need to be probed after APLIC.

The PNP devices and PCI INTx GSI links use either
acpi_dev_resource_interrupt() (PNP) or acpi_register_gsi() directly
(PCI). The approach taken here is to follow the example of
acpi_irq_get() which is enhanced to return EPROBE_DEFER and several
platform device drivers which use platform_get_irq() seem to be handling
this already.

Using ResourceSource dependency (mbigen uses) in the namespace as part of
Extended Interrupt Descriptor will not ensure the order since PNP/INTx
GSI devices don't work with that.

Is there any other better way to create dependency between IO devices
and the interrupt controllers when interrupt controller itself is a
platform device? While using core_initcall() for interrupt controllers
seem to work which forces the interrupt controller to be probed first,
Marc is not in favor of that approach since it is fragile.

Thanks a lot for your help with review and feedback!

Sunil
  

On Fri, Oct 27, 2023 at 06:25:03PM +0530, Sunil V L wrote:
> On Thu, Oct 26, 2023 at 12:04:08PM -0500, Bjorn Helgaas wrote:
> > On Thu, Oct 26, 2023 at 01:53:29AM +0530, Sunil V L wrote:
> > > On RISC-V platforms, apart from root interrupt controllers (which
> > > provide local interrupts and IPI), other interrupt controllers in the
> > > hierarchy are probed late. Enable this select this CONFIG option for
> > > RISC-V platforms so that device drivers which connect to deferred
> > > interrupt controllers can take appropriate action.
> > 
> > Quite a bit of this series seems related to the question of interrupt
> > controllers being probed "late".
> > 
> > I don't see anything specific about *how* late this might be, but from
> > the use of -EPROBE_DEFER in individual drivers (8250_pnp explicitly,
> > and acpi_register_gsi() and pnp_irq() and acpi_pci_irq_enable(), which
> > are called from driver .probe() paths) it seems like interrupt
> > controllers might be detected even after devices that use them.
> > 
> > That seems like a fairly invasive change to the driver probe flow.
> > If we really need to do that, I think it might merit a little more
> > background as justification since we haven't had to do it for any
> > other arch yet.
> 
> In RISC-V, the APLIC can be a converter from wired (GSI) to MSI interrupts.
> Hence, especially in this mode, it has to be a platform device to use
> device MSI domain. Also, according to Marc Zyngier there is no reason to
> probe interrupt controllers early apart from root controller. So, the
> device drivers which use wired interrupts need to be probed after APLIC.
> 
> The PNP devices and PCI INTx GSI links use either
> acpi_dev_resource_interrupt() (PNP) or acpi_register_gsi() directly
> (PCI). The approach taken here is to follow the example of
> acpi_irq_get() which is enhanced to return EPROBE_DEFER and several
> platform device drivers which use platform_get_irq() seem to be handling
> this already.

This series (patch 04/21 "ACPI: irq: Add support for deferred probe in
acpi_register_gsi()" [1]) makes acpi_register_gsi() return
-EPROBE_DEFER, which percolates up through pci_enable_device().

Maybe that's ok, but this affects *all* PCI drivers, and it's a new
case that did not occur before.  Many drivers emit warning or error
messages for any pci_enable_device() failure, which you probably don't
want in this case, since -EPROBE_DEFER is not really a "failure";
IIUC, it just means "probe again later."

> Using ResourceSource dependency (mbigen uses) in the namespace as part of
> Extended Interrupt Descriptor will not ensure the order since PNP/INTx
> GSI devices don't work with that.

Are these PNP/INTx GSI devices described in ACPI?  In the namespace?
Or in a static table?

> Is there any other better way to create dependency between IO devices
> and the interrupt controllers when interrupt controller itself is a
> platform device? While using core_initcall() for interrupt controllers
> seem to work which forces the interrupt controller to be probed first,
> Marc is not in favor of that approach since it is fragile.

I guess PCI interrupts from the PCI host bridges (PNP0A03 devices)
feed into the APLIC?  And APLIC is described via MADT?  Based on this
series, it looks like this:

    acpi_init
  +   acpi_riscv_init
  +     riscv_acpi_aplic_platform_init
  +       acpi_table_parse_madt(ACPI_MADT_TYPE_APLIC, aplic_parse_madt, 0)
      acpi_scan_init
        acpi_pci_root_init
        acpi_pci_link_init
	acpi_bus_scan             # add PCI host bridges, etc

If that's the sequence, it looks like aplic_parse_madt() should be
called before the PCI host bridges are added.

Or maybe this isn't how the APLICs are enumerated?

Bjorn

[1] https://lore.kernel.org/r/20231025202344.581132-5-sunilvl@ventanamicro.com
  

Hi Bjorn,

On Mon, Nov 06, 2023 at 04:16:06PM -0600, Bjorn Helgaas wrote:
> On Fri, Oct 27, 2023 at 06:25:03PM +0530, Sunil V L wrote:
> > On Thu, Oct 26, 2023 at 12:04:08PM -0500, Bjorn Helgaas wrote:
> > > On Thu, Oct 26, 2023 at 01:53:29AM +0530, Sunil V L wrote:
> > > > On RISC-V platforms, apart from root interrupt controllers (which
> > > > provide local interrupts and IPI), other interrupt controllers in the
> > > > hierarchy are probed late. Enable this select this CONFIG option for
> > > > RISC-V platforms so that device drivers which connect to deferred
> > > > interrupt controllers can take appropriate action.
> > > 
> > > Quite a bit of this series seems related to the question of interrupt
> > > controllers being probed "late".
> > > 
> > > I don't see anything specific about *how* late this might be, but from
> > > the use of -EPROBE_DEFER in individual drivers (8250_pnp explicitly,
> > > and acpi_register_gsi() and pnp_irq() and acpi_pci_irq_enable(), which
> > > are called from driver .probe() paths) it seems like interrupt
> > > controllers might be detected even after devices that use them.
> > > 
> > > That seems like a fairly invasive change to the driver probe flow.
> > > If we really need to do that, I think it might merit a little more
> > > background as justification since we haven't had to do it for any
> > > other arch yet.
> > 
> > In RISC-V, the APLIC can be a converter from wired (GSI) to MSI interrupts.
> > Hence, especially in this mode, it has to be a platform device to use
> > device MSI domain. Also, according to Marc Zyngier there is no reason to
> > probe interrupt controllers early apart from root controller. So, the
> > device drivers which use wired interrupts need to be probed after APLIC.
> > 
> > The PNP devices and PCI INTx GSI links use either
> > acpi_dev_resource_interrupt() (PNP) or acpi_register_gsi() directly
> > (PCI). The approach taken here is to follow the example of
> > acpi_irq_get() which is enhanced to return EPROBE_DEFER and several
> > platform device drivers which use platform_get_irq() seem to be handling
> > this already.
> 
> This series (patch 04/21 "ACPI: irq: Add support for deferred probe in
> acpi_register_gsi()" [1]) makes acpi_register_gsi() return
> -EPROBE_DEFER, which percolates up through pci_enable_device().
> 
> Maybe that's ok, but this affects *all* PCI drivers, and it's a new
> case that did not occur before.  Many drivers emit warning or error
> messages for any pci_enable_device() failure, which you probably don't
> want in this case, since -EPROBE_DEFER is not really a "failure";
> IIUC, it just means "probe again later."
>
Yeah, I think all the drivers which need to be supported on RISC-V
ACPI based systems will have to support deferred probe with this scheme.

> > Using ResourceSource dependency (mbigen uses) in the namespace as part of
> > Extended Interrupt Descriptor will not ensure the order since PNP/INTx
> > GSI devices don't work with that.
> 
> Are these PNP/INTx GSI devices described in ACPI?  In the namespace?
> Or in a static table?
> 
Yes, these are standard devices in the namespace. For ex: PNP0501(16550)
or PNP0C0F (PCI interrupt link devices) are in the namespace.

> > Is there any other better way to create dependency between IO devices
> > and the interrupt controllers when interrupt controller itself is a
> > platform device? While using core_initcall() for interrupt controllers
> > seem to work which forces the interrupt controller to be probed first,
> > Marc is not in favor of that approach since it is fragile.
> 
> I guess PCI interrupts from the PCI host bridges (PNP0A03 devices)
> feed into the APLIC?  And APLIC is described via MADT?  Based on this
> series, it looks like this:
> 
>     acpi_init
>   +   acpi_riscv_init
>   +     riscv_acpi_aplic_platform_init
>   +       acpi_table_parse_madt(ACPI_MADT_TYPE_APLIC, aplic_parse_madt, 0)
>       acpi_scan_init
>         acpi_pci_root_init
>         acpi_pci_link_init
> 	acpi_bus_scan             # add PCI host bridges, etc
> 
> If that's the sequence, it looks like aplic_parse_madt() should be
> called before the PCI host bridges are added.
> 
> Or maybe this isn't how the APLICs are enumerated?
> 
That's partly correct. APLIC platform devices are created prior to PCI
host bridges added. But the actual APLIC driver which creates the
irqdomain will be probed as a regular platform driver for the APLIC
device. The platform driver probe will happen using DD framework and
devices don't have any dependency on APLIC which can cause device probe
prior to APLIC driver probe.

DT supports fw_devlink framework which makes it easier for IRQ devices
to use regular platform drivers and produces-consumers are probed in the
order without requiring drivers to do deferred probe. But I don't see
that supported for ACPI framework.  Also, the way PNP devices get added
there is an assumption that interrupt controller is already setup fully.

With this new use case in RISC-V, here are the alternatives I am aware of.

1) Use core_initcall() in the APLIC drivers which makes APLIC driver to
be probed prior to PNP or PCI INTx devices. But this was ruled out in
the context of DT from Marc.

2) Like the approach tried in this series, add support for deferred
probe in drivers. This will be invasive change requiring many drivers to
change like you pointed.

I don't know which is less evil or if there is any other alternative
which I am not aware of.

Thomas/Marc, could you allow APLIC (and PLIC) irqchip drivers to use
core_initcall() for ACPI?

Thanks,
Sunil
  

On Wed, 08 Nov 2023 09:53:14 +0000,
Sunil V L <sunilvl@ventanamicro.com> wrote:
> 
> That's partly correct. APLIC platform devices are created prior to PCI
> host bridges added. But the actual APLIC driver which creates the
> irqdomain will be probed as a regular platform driver for the APLIC
> device. The platform driver probe will happen using DD framework and
> devices don't have any dependency on APLIC which can cause device probe
> prior to APLIC driver probe.
> 
> DT supports fw_devlink framework which makes it easier for IRQ devices
> to use regular platform drivers and produces-consumers are probed in the
> order without requiring drivers to do deferred probe. But I don't see
> that supported for ACPI framework.  Also, the way PNP devices get added
> there is an assumption that interrupt controller is already setup fully.
> 
> With this new use case in RISC-V, here are the alternatives I am aware of.
> 
> 1) Use core_initcall() in the APLIC drivers which makes APLIC driver to
> be probed prior to PNP or PCI INTx devices. But this was ruled out in
> the context of DT from Marc.
>
> 2) Like the approach tried in this series, add support for deferred
> probe in drivers. This will be invasive change requiring many drivers to
> change like you pointed.
> 
> I don't know which is less evil or if there is any other alternative
> which I am not aware of.
> 
> Thomas/Marc, could you allow APLIC (and PLIC) irqchip drivers to use
> core_initcall() for ACPI?

I don't have a say about this anymore, so this is only a passing
comment, which you are free to cast aside.

My personal view is that if you need to rely on core_initcall() for a
particular firmware interface, then your architecture will end-up
being an unmaintainable ball of hacks, with conflicting requirements
and increasingly diverging behaviours. Those who had the 'privilege'
to deal with the 32bit ARM transition to DT will understand what I
mean.

Having to rely on initcalls can only mean two things:

- you're missing crucial topology information that will eventually
  bite you where it hurts, and you're better off going back to the
  drawing board to fix it before any HW ships,

- you're not making use of the kernel's dependency management
  infrastructure, which is pretty sad. Yes, it is DT specific for now,
  but nothing prevents you from improving it to make it grok another
  firmware interface.

But as I said, I don't have much skin in that game anymore.

	M.
  

Hi Sunil!

I'm trying to decipher this thread, so apologies in advance for the
stupid questions! :-P

Sunil V L <sunilvl@ventanamicro.com> writes:

> Hi Bjorn,
>
> On Mon, Nov 06, 2023 at 04:16:06PM -0600, Bjorn Helgaas wrote:
>> On Fri, Oct 27, 2023 at 06:25:03PM +0530, Sunil V L wrote:
>> > On Thu, Oct 26, 2023 at 12:04:08PM -0500, Bjorn Helgaas wrote:
>> > > On Thu, Oct 26, 2023 at 01:53:29AM +0530, Sunil V L wrote:
>> > > > On RISC-V platforms, apart from root interrupt controllers (which
>> > > > provide local interrupts and IPI), other interrupt controllers in the
>> > > > hierarchy are probed late. Enable this select this CONFIG option for
>> > > > RISC-V platforms so that device drivers which connect to deferred
>> > > > interrupt controllers can take appropriate action.
>> > > 
>> > > Quite a bit of this series seems related to the question of interrupt
>> > > controllers being probed "late".
>> > > 
>> > > I don't see anything specific about *how* late this might be, but from
>> > > the use of -EPROBE_DEFER in individual drivers (8250_pnp explicitly,
>> > > and acpi_register_gsi() and pnp_irq() and acpi_pci_irq_enable(), which
>> > > are called from driver .probe() paths) it seems like interrupt
>> > > controllers might be detected even after devices that use them.
>> > > 
>> > > That seems like a fairly invasive change to the driver probe flow.
>> > > If we really need to do that, I think it might merit a little more
>> > > background as justification since we haven't had to do it for any
>> > > other arch yet.
>> > 
>> > In RISC-V, the APLIC can be a converter from wired (GSI) to MSI interrupts.
>> > Hence, especially in this mode, it has to be a platform device to use
>> > device MSI domain. Also, according to Marc Zyngier there is no reason to
>> > probe interrupt controllers early apart from root controller. So, the
>> > device drivers which use wired interrupts need to be probed after APLIC.
>> > 
>> > The PNP devices and PCI INTx GSI links use either
>> > acpi_dev_resource_interrupt() (PNP) or acpi_register_gsi() directly
>> > (PCI). The approach taken here is to follow the example of
>> > acpi_irq_get() which is enhanced to return EPROBE_DEFER and several
>> > platform device drivers which use platform_get_irq() seem to be handling
>> > this already.
>> 
>> This series (patch 04/21 "ACPI: irq: Add support for deferred probe in
>> acpi_register_gsi()" [1]) makes acpi_register_gsi() return
>> -EPROBE_DEFER, which percolates up through pci_enable_device().
>> 
>> Maybe that's ok, but this affects *all* PCI drivers, and it's a new
>> case that did not occur before.  Many drivers emit warning or error
>> messages for any pci_enable_device() failure, which you probably don't
>> want in this case, since -EPROBE_DEFER is not really a "failure";
>> IIUC, it just means "probe again later."
>>
> Yeah, I think all the drivers which need to be supported on RISC-V
> ACPI based systems will have to support deferred probe with this scheme.
>
>> > Using ResourceSource dependency (mbigen uses) in the namespace as part of
>> > Extended Interrupt Descriptor will not ensure the order since PNP/INTx
>> > GSI devices don't work with that.
>> 
>> Are these PNP/INTx GSI devices described in ACPI?  In the namespace?
>> Or in a static table?
>> 
> Yes, these are standard devices in the namespace. For ex: PNP0501(16550)
> or PNP0C0F (PCI interrupt link devices) are in the namespace.
>
>> > Is there any other better way to create dependency between IO devices
>> > and the interrupt controllers when interrupt controller itself is a
>> > platform device? While using core_initcall() for interrupt controllers
>> > seem to work which forces the interrupt controller to be probed first,
>> > Marc is not in favor of that approach since it is fragile.
>> 
>> I guess PCI interrupts from the PCI host bridges (PNP0A03 devices)
>> feed into the APLIC?  And APLIC is described via MADT?  Based on this
>> series, it looks like this:
>> 
>>     acpi_init
>>   +   acpi_riscv_init
>>   +     riscv_acpi_aplic_platform_init
>>   +       acpi_table_parse_madt(ACPI_MADT_TYPE_APLIC, aplic_parse_madt, 0)
>>       acpi_scan_init
>>         acpi_pci_root_init
>>         acpi_pci_link_init
>> 	acpi_bus_scan             # add PCI host bridges, etc
>> 
>> If that's the sequence, it looks like aplic_parse_madt() should be
>> called before the PCI host bridges are added.
>> 
>> Or maybe this isn't how the APLICs are enumerated?
>> 
> That's partly correct. APLIC platform devices are created prior to PCI
> host bridges added. But the actual APLIC driver which creates the
> irqdomain will be probed as a regular platform driver for the APLIC
> device. The platform driver probe will happen using DD framework and
> devices don't have any dependency on APLIC which can cause device probe
> prior to APLIC driver probe.
>
> DT supports fw_devlink framework which makes it easier for IRQ devices
> to use regular platform drivers and produces-consumers are probed in the
> order without requiring drivers to do deferred probe. But I don't see
> that supported for ACPI framework.  Also, the way PNP devices get added
> there is an assumption that interrupt controller is already setup fully.

AFAIU, the -EPROBE_DEFER changes are needed for GSIs (and the way the
IMSIC/APLIC irqchip series is structured), right?

There's a couple of separate pieces in play here:
1. IMSIC-IPI (MADT init)
2. IMSIC-MSI (MADT init, imsic_platform_acpi_probe() patch 14)
3. APLIC-wired (platform)
4. APLIC-MSI-bridge (platform)

APLIC-MSI-bridge is pretty much a RISC-V mbigen.

Some devices do not have ResourceSource parsing implemented yet. The PNP
devices that cannot use ResourceSource (you mention PNP0501 (16550) and
PNP0C0F (PCI interrupt link devices), do we really need to care about
them for the RISC-V platforms using ACPI? If that would change, the
kernel drivers can be adjusted (d44fa3d46079 ("ACPI: Add support for
ResourceSource/IRQ domain mapping"))?

I guess my question is we need to care about GSIs w/o explicit
ResourceSource, so that APLIC-MSI-bridge can be used.

GED works nicely with ResourceSource, and covers a lot of the GSI
use-cases, no?

And if we do care, then *both* 3 and 4 would need at MADT scan
point/init, and not be a platform device (late init).

From my, probably naive perspective, it's a bit weird *not* to create
the irq domains at MADT scan time.

> With this new use case in RISC-V, here are the alternatives I am aware of.
>
> 1) Use core_initcall() in the APLIC drivers which makes APLIC driver to
> be probed prior to PNP or PCI INTx devices. But this was ruled out in
> the context of DT from Marc.
>
> 2) Like the approach tried in this series, add support for deferred
> probe in drivers. This will be invasive change requiring many drivers to
> change like you pointed.

Again is this only for GSIs? Patch 14 moves the IMSIC-MSI init to MADT
for PCIe devices (which is different from DT), so it's not for PCIe
devices. I wonder if it's a lot of churn for something that will not be
used for RISC-V ACPI systems...

A quick look at what Arm's GICv3 does, all irq domains are created at
MADT init.


Björn (no, not PCI-Bjorn ;-))
  

Hi Björn!,

Apologies for the delay in response. Held up with something else.

On Wed, Nov 22, 2023 at 01:22:56PM +0100, Björn Töpel wrote:
> Hi Sunil!
> 
> I'm trying to decipher this thread, so apologies in advance for the
> stupid questions! :-P
>
Appreciate your help to review the patch and suggesting solutions.
Thank you very much!.

> Sunil V L <sunilvl@ventanamicro.com> writes:
> 
> > Hi Bjorn,
> >
> > On Mon, Nov 06, 2023 at 04:16:06PM -0600, Bjorn Helgaas wrote:
> >> On Fri, Oct 27, 2023 at 06:25:03PM +0530, Sunil V L wrote:
> >> > On Thu, Oct 26, 2023 at 12:04:08PM -0500, Bjorn Helgaas wrote:
> >> > > On Thu, Oct 26, 2023 at 01:53:29AM +0530, Sunil V L wrote:
> >> > > > On RISC-V platforms, apart from root interrupt controllers (which
> >> > > > provide local interrupts and IPI), other interrupt controllers in the
> >> > > > hierarchy are probed late. Enable this select this CONFIG option for
> >> > > > RISC-V platforms so that device drivers which connect to deferred
> >> > > > interrupt controllers can take appropriate action.
> >> > > 
> >> > > Quite a bit of this series seems related to the question of interrupt
> >> > > controllers being probed "late".
> >> > > 
> >> > > I don't see anything specific about *how* late this might be, but from
> >> > > the use of -EPROBE_DEFER in individual drivers (8250_pnp explicitly,
> >> > > and acpi_register_gsi() and pnp_irq() and acpi_pci_irq_enable(), which
> >> > > are called from driver .probe() paths) it seems like interrupt
> >> > > controllers might be detected even after devices that use them.
> >> > > 
> >> > > That seems like a fairly invasive change to the driver probe flow.
> >> > > If we really need to do that, I think it might merit a little more
> >> > > background as justification since we haven't had to do it for any
> >> > > other arch yet.
> >> > 
> >> > In RISC-V, the APLIC can be a converter from wired (GSI) to MSI interrupts.
> >> > Hence, especially in this mode, it has to be a platform device to use
> >> > device MSI domain. Also, according to Marc Zyngier there is no reason to
> >> > probe interrupt controllers early apart from root controller. So, the
> >> > device drivers which use wired interrupts need to be probed after APLIC.
> >> > 
> >> > The PNP devices and PCI INTx GSI links use either
> >> > acpi_dev_resource_interrupt() (PNP) or acpi_register_gsi() directly
> >> > (PCI). The approach taken here is to follow the example of
> >> > acpi_irq_get() which is enhanced to return EPROBE_DEFER and several
> >> > platform device drivers which use platform_get_irq() seem to be handling
> >> > this already.
> >> 
> >> This series (patch 04/21 "ACPI: irq: Add support for deferred probe in
> >> acpi_register_gsi()" [1]) makes acpi_register_gsi() return
> >> -EPROBE_DEFER, which percolates up through pci_enable_device().
> >> 
> >> Maybe that's ok, but this affects *all* PCI drivers, and it's a new
> >> case that did not occur before.  Many drivers emit warning or error
> >> messages for any pci_enable_device() failure, which you probably don't
> >> want in this case, since -EPROBE_DEFER is not really a "failure";
> >> IIUC, it just means "probe again later."
> >>
> > Yeah, I think all the drivers which need to be supported on RISC-V
> > ACPI based systems will have to support deferred probe with this scheme.
> >
> >> > Using ResourceSource dependency (mbigen uses) in the namespace as part of
> >> > Extended Interrupt Descriptor will not ensure the order since PNP/INTx
> >> > GSI devices don't work with that.
> >> 
> >> Are these PNP/INTx GSI devices described in ACPI?  In the namespace?
> >> Or in a static table?
> >> 
> > Yes, these are standard devices in the namespace. For ex: PNP0501(16550)
> > or PNP0C0F (PCI interrupt link devices) are in the namespace.
> >
> >> > Is there any other better way to create dependency between IO devices
> >> > and the interrupt controllers when interrupt controller itself is a
> >> > platform device? While using core_initcall() for interrupt controllers
> >> > seem to work which forces the interrupt controller to be probed first,
> >> > Marc is not in favor of that approach since it is fragile.
> >> 
> >> I guess PCI interrupts from the PCI host bridges (PNP0A03 devices)
> >> feed into the APLIC?  And APLIC is described via MADT?  Based on this
> >> series, it looks like this:
> >> 
> >>     acpi_init
> >>   +   acpi_riscv_init
> >>   +     riscv_acpi_aplic_platform_init
> >>   +       acpi_table_parse_madt(ACPI_MADT_TYPE_APLIC, aplic_parse_madt, 0)
> >>       acpi_scan_init
> >>         acpi_pci_root_init
> >>         acpi_pci_link_init
> >> 	acpi_bus_scan             # add PCI host bridges, etc
> >> 
> >> If that's the sequence, it looks like aplic_parse_madt() should be
> >> called before the PCI host bridges are added.
> >> 
> >> Or maybe this isn't how the APLICs are enumerated?
> >> 
> > That's partly correct. APLIC platform devices are created prior to PCI
> > host bridges added. But the actual APLIC driver which creates the
> > irqdomain will be probed as a regular platform driver for the APLIC
> > device. The platform driver probe will happen using DD framework and
> > devices don't have any dependency on APLIC which can cause device probe
> > prior to APLIC driver probe.
> >
> > DT supports fw_devlink framework which makes it easier for IRQ devices
> > to use regular platform drivers and produces-consumers are probed in the
> > order without requiring drivers to do deferred probe. But I don't see
> > that supported for ACPI framework.  Also, the way PNP devices get added
> > there is an assumption that interrupt controller is already setup fully.
> 
> AFAIU, the -EPROBE_DEFER changes are needed for GSIs (and the way the
> IMSIC/APLIC irqchip series is structured), right?
> 
Yes, It is only for GSI's.

> There's a couple of separate pieces in play here:
> 1. IMSIC-IPI (MADT init)
> 2. IMSIC-MSI (MADT init, imsic_platform_acpi_probe() patch 14)
> 3. APLIC-wired (platform)
> 4. APLIC-MSI-bridge (platform)
> 
> APLIC-MSI-bridge is pretty much a RISC-V mbigen.
> 
> Some devices do not have ResourceSource parsing implemented yet. The PNP
> devices that cannot use ResourceSource (you mention PNP0501 (16550) and
> PNP0C0F (PCI interrupt link devices), do we really need to care about
> them for the RISC-V platforms using ACPI? If that would change, the
> kernel drivers can be adjusted (d44fa3d46079 ("ACPI: Add support for
> ResourceSource/IRQ domain mapping"))?
> 
> I guess my question is we need to care about GSIs w/o explicit
> ResourceSource, so that APLIC-MSI-bridge can be used.
> 
> GED works nicely with ResourceSource, and covers a lot of the GSI
> use-cases, no?
> 
> And if we do care, then *both* 3 and 4 would need at MADT scan
> point/init, and not be a platform device (late init).
> 
I am not sure it is a good idea not to support PCI link devices. Not
allowing them removes the flexibility in _PRT. Also, is there a standard
16550 UART apart from PNP0501? ACPI platform devices already support
deferred probe as per the series you mentioned. IMO, PNP also should
support it. So, I am not sure it is a good idea to prohibit all PnP
devices on RISC-V platforms. Other OS's might be able to handle them.

> From my, probably naive perspective, it's a bit weird *not* to create
> the irq domains at MADT scan time.
> 
> > With this new use case in RISC-V, here are the alternatives I am aware of.
> >
> > 1) Use core_initcall() in the APLIC drivers which makes APLIC driver to
> > be probed prior to PNP or PCI INTx devices. But this was ruled out in
> > the context of DT from Marc.
> >
> > 2) Like the approach tried in this series, add support for deferred
> > probe in drivers. This will be invasive change requiring many drivers to
> > change like you pointed.
> 
> Again is this only for GSIs? Patch 14 moves the IMSIC-MSI init to MADT
> for PCIe devices (which is different from DT), so it's not for PCIe
> devices. I wonder if it's a lot of churn for something that will not be
> used for RISC-V ACPI systems...
> 
> A quick look at what Arm's GICv3 does, all irq domains are created at
> MADT init.
> 
The issue is primarily with APLIC-MSI. Since it needs MSI device domain,
it has to be a platform device.

I am investigating fw-devlink like Marc suggested atleast for IRQ
dependencies. If that works, it would be the best solution.

Thanks,
Sunil