mbox series

[RFC,v3,00/17] RISC-V: ACPI: Add external interrupt controller support

Message ID 20231219174526.2235150-1-sunilvl@ventanamicro.com
Headers show
Series RISC-V: ACPI: Add external interrupt controller support | expand

Message

Sunil V L Dec. 19, 2023, 5:45 p.m. UTC
This series adds support for the below ECR approved by ASWG.
1) MADT - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing

The series primarily enables irqchip drivers for RISC-V ACPI based
platforms.

The series can be broadly categorized like below. 

1) PCI ACPI related functions are migrated from arm64 to common file so
that we don't need to duplicate them for RISC-V.

2) Introduced support for fw_devlink for ACPI nodes for IRQ dependency.
This helps to support deferred probe of interrupt controller drivers.

3) Modified pnp_irq() to try registering the IRQ  again if it sees it in
disabled state. This solution is similar to how
platform_get_irq_optional() works for regular platform devices.

4) Added support for re-ordering the probe of interrupt controllers when
IRQCHIP_ACPI_DECLARE is used.

5) ACPI support added in RISC-V interrupt controller drivers.

This series is based on Anup's AIA v11 series. Since Anup's AIA v11 is
not merged yet and first time introducing fw_devlink, deferred probe and
reordering support for IRQCHIP probe, this series is still kept as RFC.
Looking forward for the feedback!

Changes since RFC v2:
	1) Introduced fw_devlink for ACPI nodes for IRQ dependency.
	2) Dropped patches in drivers which are not required due to
	   fw_devlink support.
	3) Dropped pci_set_msi() patch and added a patch in
	   pci_create_root_bus().
	4) Updated pnp_irq() patch so that none of the actual PNP
	   drivers need to change.

Changes since RFC v1:
	1) Abandoned swnode approach as per Marc's feedback.
	2) To cope up with AIA series changes which changed irqchip driver
	   probe from core_initcall() to platform_driver, added patches
	   to support deferred probing.
	3) Rebased on top of Anup's AIA v11 and added tags.

To test the series,

1) Qemu should be built using the riscv_acpi_b2_v8 branch at
https://github.com/vlsunil/qemu.git

2) EDK2 should be built using the instructions at:
https://github.com/tianocore/edk2/blob/master/OvmfPkg/RiscVVirt/README.md

3) Build Linux using this series on top of Anup's AIA v11 series.

Run Qemu:
qemu-system-riscv64 \
 -M virt,pflash0=pflash0,pflash1=pflash1,aia=aplic-imsic \
 -m 2G -smp 8 \
 -serial mon:stdio \
 -device virtio-gpu-pci -full-screen \
 -device qemu-xhci \
 -device usb-kbd \
 -blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
 -blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
 -netdev user,id=net0 -device virtio-net-pci,netdev=net0 \
 -kernel arch/riscv/boot/Image \
 -initrd rootfs.cpio \
 -append "root=/dev/ram ro console=ttyS0 rootwait earlycon=uart8250,mmio,0x10000000"

To boot with APLIC only, use aia=aplic.
To boot with PLIC, remove aia= option.

This series is also available in acpi_b2_v3_riscv_aia_v11 branch at
https://github.com/vlsunil/linux.git

Based-on: 20231023172800.315343-1-apatel@ventanamicro.com
(https://lore.kernel.org/lkml/20231023172800.315343-1-apatel@ventanamicro.com/)

Sunil V L (17):
  arm64: PCI: Migrate ACPI related functions to pci-acpi.c
  RISC-V: ACPI: Implement PCI related functionality
  PCI: Make pci_create_root_bus() declare its reliance on MSI domains
  ACPI: Add fw_devlink support for ACPI fwnode for IRQ dependency
  ACPI: irq: Add support for deferred probe in acpi_register_gsi()
  pnp.h: Reconfigure IRQ in pnp_irq() to support deferred probe
  ACPI: scan.c: Add weak arch specific function to reorder the IRQCHIP
    probe
  ACPI: RISC-V: Implement arch function to reorder irqchip probe entries
  irqchip: riscv-intc: Add ACPI support for AIA
  irqchip: riscv-imsic: Add ACPI support
  irqchip: riscv-aplic: Add ACPI support
  irqchip: irq-sifive-plic: Add ACPI support
  ACPI: bus: Add RINTC IRQ model for RISC-V
  ACPI: bus: Add acpi_riscv_init function
  ACPI: RISC-V: Create APLIC platform device
  ACPI: RISC-V: Create PLIC platform device
  irqchip: riscv-intc: Set ACPI irqmodel

 arch/arm64/kernel/pci.c                    | 191 ---------------------
 arch/riscv/Kconfig                         |   2 +
 arch/riscv/include/asm/irq.h               |  46 +++++
 arch/riscv/kernel/acpi.c                   |  31 ++--
 drivers/acpi/bus.c                         |   4 +
 drivers/acpi/irq.c                         |  22 +++
 drivers/acpi/property.c                    |  20 +++
 drivers/acpi/riscv/Makefile                |   2 +-
 drivers/acpi/riscv/init.c                  |  15 ++
 drivers/acpi/riscv/init.h                  |   6 +
 drivers/acpi/riscv/irq.c                   | 169 ++++++++++++++++++
 drivers/acpi/scan.c                        |   3 +
 drivers/irqchip/irq-riscv-aplic-direct.c   |  21 ++-
 drivers/irqchip/irq-riscv-aplic-main.c     |  64 ++++---
 drivers/irqchip/irq-riscv-aplic-main.h     |   1 +
 drivers/irqchip/irq-riscv-aplic-msi.c      |   9 +-
 drivers/irqchip/irq-riscv-imsic-early.c    |  52 +++++-
 drivers/irqchip/irq-riscv-imsic-platform.c |  51 ++++--
 drivers/irqchip/irq-riscv-imsic-state.c    | 128 +++++++-------
 drivers/irqchip/irq-riscv-imsic-state.h    |   2 +-
 drivers/irqchip/irq-riscv-intc.c           |  96 ++++++++++-
 drivers/irqchip/irq-sifive-plic.c          |  76 ++++++--
 drivers/pci/pci-acpi.c                     | 182 ++++++++++++++++++++
 drivers/pci/probe.c                        |   1 +
 drivers/pnp/pnpacpi/core.c                 |   7 +
 include/linux/acpi.h                       |  18 ++
 include/linux/irqchip/riscv-imsic.h        |  10 ++
 include/linux/pnp.h                        |  14 +-
 28 files changed, 895 insertions(+), 348 deletions(-)
 create mode 100644 drivers/acpi/riscv/init.c
 create mode 100644 drivers/acpi/riscv/init.h
 create mode 100644 drivers/acpi/riscv/irq.c

Comments

Sunil V L Jan. 30, 2024, 6:02 a.m. UTC | #1
On Tue, Dec 19, 2023 at 06:50:19PM +0100, Rafael J. Wysocki wrote:
> On Tue, Dec 19, 2023 at 6:45 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > This series adds support for the below ECR approved by ASWG.
> > 1) MADT - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
> >
> > The series primarily enables irqchip drivers for RISC-V ACPI based
> > platforms.
> >
> > The series can be broadly categorized like below.
> >
> > 1) PCI ACPI related functions are migrated from arm64 to common file so
> > that we don't need to duplicate them for RISC-V.
> >
> > 2) Introduced support for fw_devlink for ACPI nodes for IRQ dependency.
> > This helps to support deferred probe of interrupt controller drivers.
> >
> > 3) Modified pnp_irq() to try registering the IRQ  again if it sees it in
> > disabled state. This solution is similar to how
> > platform_get_irq_optional() works for regular platform devices.
> >
> > 4) Added support for re-ordering the probe of interrupt controllers when
> > IRQCHIP_ACPI_DECLARE is used.
> >
> > 5) ACPI support added in RISC-V interrupt controller drivers.
> >
> > This series is based on Anup's AIA v11 series. Since Anup's AIA v11 is
> > not merged yet and first time introducing fw_devlink, deferred probe and
> > reordering support for IRQCHIP probe, this series is still kept as RFC.
> > Looking forward for the feedback!
> >
> > Changes since RFC v2:
> >         1) Introduced fw_devlink for ACPI nodes for IRQ dependency.
> >         2) Dropped patches in drivers which are not required due to
> >            fw_devlink support.
> >         3) Dropped pci_set_msi() patch and added a patch in
> >            pci_create_root_bus().
> >         4) Updated pnp_irq() patch so that none of the actual PNP
> >            drivers need to change.
> >
> > Changes since RFC v1:
> >         1) Abandoned swnode approach as per Marc's feedback.
> >         2) To cope up with AIA series changes which changed irqchip driver
> >            probe from core_initcall() to platform_driver, added patches
> >            to support deferred probing.
> >         3) Rebased on top of Anup's AIA v11 and added tags.
> >
> > To test the series,
> >
> > 1) Qemu should be built using the riscv_acpi_b2_v8 branch at
> > https://github.com/vlsunil/qemu.git
> >
> > 2) EDK2 should be built using the instructions at:
> > https://github.com/tianocore/edk2/blob/master/OvmfPkg/RiscVVirt/README.md
> >
> > 3) Build Linux using this series on top of Anup's AIA v11 series.
> >
> > Run Qemu:
> > qemu-system-riscv64 \
> >  -M virt,pflash0=pflash0,pflash1=pflash1,aia=aplic-imsic \
> >  -m 2G -smp 8 \
> >  -serial mon:stdio \
> >  -device virtio-gpu-pci -full-screen \
> >  -device qemu-xhci \
> >  -device usb-kbd \
> >  -blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
> >  -blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
> >  -netdev user,id=net0 -device virtio-net-pci,netdev=net0 \
> >  -kernel arch/riscv/boot/Image \
> >  -initrd rootfs.cpio \
> >  -append "root=/dev/ram ro console=ttyS0 rootwait earlycon=uart8250,mmio,0x10000000"
> >
> > To boot with APLIC only, use aia=aplic.
> > To boot with PLIC, remove aia= option.
> >
> > This series is also available in acpi_b2_v3_riscv_aia_v11 branch at
> > https://github.com/vlsunil/linux.git
> >
> > Based-on: 20231023172800.315343-1-apatel@ventanamicro.com
> > (https://lore.kernel.org/lkml/20231023172800.315343-1-apatel@ventanamicro.com/)
> >
> > Sunil V L (17):
> >   arm64: PCI: Migrate ACPI related functions to pci-acpi.c
> >   RISC-V: ACPI: Implement PCI related functionality
> >   PCI: Make pci_create_root_bus() declare its reliance on MSI domains
> >   ACPI: Add fw_devlink support for ACPI fwnode for IRQ dependency
> >   ACPI: irq: Add support for deferred probe in acpi_register_gsi()
> >   pnp.h: Reconfigure IRQ in pnp_irq() to support deferred probe
> >   ACPI: scan.c: Add weak arch specific function to reorder the IRQCHIP
> >     probe
> >   ACPI: RISC-V: Implement arch function to reorder irqchip probe entries
> >   irqchip: riscv-intc: Add ACPI support for AIA
> >   irqchip: riscv-imsic: Add ACPI support
> >   irqchip: riscv-aplic: Add ACPI support
> >   irqchip: irq-sifive-plic: Add ACPI support
> >   ACPI: bus: Add RINTC IRQ model for RISC-V
> >   ACPI: bus: Add acpi_riscv_init function
> >   ACPI: RISC-V: Create APLIC platform device
> >   ACPI: RISC-V: Create PLIC platform device
> >   irqchip: riscv-intc: Set ACPI irqmodel
> 
> JFYI, I have no capacity to provide any feedback on this till 6.8-rc1 is out.
> 
Hi Rafael,

Gentle ping.

Could you please provide feedback on the series? Patches 4, 5, 6, 7 and
8 are bit critical IMO. So, I really look forward for your and other
ACPI experts!.

Thanks!
Sunil
Rafael J. Wysocki Feb. 1, 2024, 6:10 p.m. UTC | #2
On Tue, Jan 30, 2024 at 7:02 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> On Tue, Dec 19, 2023 at 06:50:19PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Dec 19, 2023 at 6:45 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > >
> > > This series adds support for the below ECR approved by ASWG.
> > > 1) MADT - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
> > >
> > > The series primarily enables irqchip drivers for RISC-V ACPI based
> > > platforms.
> > >
> > > The series can be broadly categorized like below.
> > >
> > > 1) PCI ACPI related functions are migrated from arm64 to common file so
> > > that we don't need to duplicate them for RISC-V.
> > >
> > > 2) Introduced support for fw_devlink for ACPI nodes for IRQ dependency.
> > > This helps to support deferred probe of interrupt controller drivers.
> > >
> > > 3) Modified pnp_irq() to try registering the IRQ  again if it sees it in
> > > disabled state. This solution is similar to how
> > > platform_get_irq_optional() works for regular platform devices.
> > >
> > > 4) Added support for re-ordering the probe of interrupt controllers when
> > > IRQCHIP_ACPI_DECLARE is used.
> > >
> > > 5) ACPI support added in RISC-V interrupt controller drivers.
> > >
> > > This series is based on Anup's AIA v11 series. Since Anup's AIA v11 is
> > > not merged yet and first time introducing fw_devlink, deferred probe and
> > > reordering support for IRQCHIP probe, this series is still kept as RFC.
> > > Looking forward for the feedback!
> > >
> > > Changes since RFC v2:
> > >         1) Introduced fw_devlink for ACPI nodes for IRQ dependency.
> > >         2) Dropped patches in drivers which are not required due to
> > >            fw_devlink support.
> > >         3) Dropped pci_set_msi() patch and added a patch in
> > >            pci_create_root_bus().
> > >         4) Updated pnp_irq() patch so that none of the actual PNP
> > >            drivers need to change.
> > >
> > > Changes since RFC v1:
> > >         1) Abandoned swnode approach as per Marc's feedback.
> > >         2) To cope up with AIA series changes which changed irqchip driver
> > >            probe from core_initcall() to platform_driver, added patches
> > >            to support deferred probing.
> > >         3) Rebased on top of Anup's AIA v11 and added tags.
> > >
> > > To test the series,
> > >
> > > 1) Qemu should be built using the riscv_acpi_b2_v8 branch at
> > > https://github.com/vlsunil/qemu.git
> > >
> > > 2) EDK2 should be built using the instructions at:
> > > https://github.com/tianocore/edk2/blob/master/OvmfPkg/RiscVVirt/README.md
> > >
> > > 3) Build Linux using this series on top of Anup's AIA v11 series.
> > >
> > > Run Qemu:
> > > qemu-system-riscv64 \
> > >  -M virt,pflash0=pflash0,pflash1=pflash1,aia=aplic-imsic \
> > >  -m 2G -smp 8 \
> > >  -serial mon:stdio \
> > >  -device virtio-gpu-pci -full-screen \
> > >  -device qemu-xhci \
> > >  -device usb-kbd \
> > >  -blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
> > >  -blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
> > >  -netdev user,id=net0 -device virtio-net-pci,netdev=net0 \
> > >  -kernel arch/riscv/boot/Image \
> > >  -initrd rootfs.cpio \
> > >  -append "root=/dev/ram ro console=ttyS0 rootwait earlycon=uart8250,mmio,0x10000000"
> > >
> > > To boot with APLIC only, use aia=aplic.
> > > To boot with PLIC, remove aia= option.
> > >
> > > This series is also available in acpi_b2_v3_riscv_aia_v11 branch at
> > > https://github.com/vlsunil/linux.git
> > >
> > > Based-on: 20231023172800.315343-1-apatel@ventanamicro.com
> > > (https://lore.kernel.org/lkml/20231023172800.315343-1-apatel@ventanamicro.com/)
> > >
> > > Sunil V L (17):
> > >   arm64: PCI: Migrate ACPI related functions to pci-acpi.c
> > >   RISC-V: ACPI: Implement PCI related functionality
> > >   PCI: Make pci_create_root_bus() declare its reliance on MSI domains
> > >   ACPI: Add fw_devlink support for ACPI fwnode for IRQ dependency
> > >   ACPI: irq: Add support for deferred probe in acpi_register_gsi()
> > >   pnp.h: Reconfigure IRQ in pnp_irq() to support deferred probe
> > >   ACPI: scan.c: Add weak arch specific function to reorder the IRQCHIP
> > >     probe
> > >   ACPI: RISC-V: Implement arch function to reorder irqchip probe entries
> > >   irqchip: riscv-intc: Add ACPI support for AIA
> > >   irqchip: riscv-imsic: Add ACPI support
> > >   irqchip: riscv-aplic: Add ACPI support
> > >   irqchip: irq-sifive-plic: Add ACPI support
> > >   ACPI: bus: Add RINTC IRQ model for RISC-V
> > >   ACPI: bus: Add acpi_riscv_init function
> > >   ACPI: RISC-V: Create APLIC platform device
> > >   ACPI: RISC-V: Create PLIC platform device
> > >   irqchip: riscv-intc: Set ACPI irqmodel
> >
> > JFYI, I have no capacity to provide any feedback on this till 6.8-rc1 is out.
> >
> Hi Rafael,
>
> Gentle ping.
>
> Could you please provide feedback on the series? Patches 4, 5, 6, 7 and
> 8 are bit critical IMO. So, I really look forward for your and other
> ACPI experts!.

There was quite a bit of discussion on patch [6/21] and it still seems
relevant to me.

ACPI actually has a way to at least indicate what the probe ordering
should be which is _DEP.

The current handling of _DEP in the kernel may not be covering this
particular use case, but I would rather extend it (if necessary)
instead of doing all of the -EPROBE_DEFER dance which seems fragile to
me.

Thanks!
Sunil V L Feb. 2, 2024, 12:17 p.m. UTC | #3
On Thu, Feb 01, 2024 at 07:10:28PM +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 30, 2024 at 7:02 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > On Tue, Dec 19, 2023 at 06:50:19PM +0100, Rafael J. Wysocki wrote:
> > > On Tue, Dec 19, 2023 at 6:45 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > >
> > > > This series adds support for the below ECR approved by ASWG.
> > > > 1) MADT - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
> > > >
> > > > The series primarily enables irqchip drivers for RISC-V ACPI based
> > > > platforms.
> > > >
> > > > The series can be broadly categorized like below.
> > > >
> > > > 1) PCI ACPI related functions are migrated from arm64 to common file so
> > > > that we don't need to duplicate them for RISC-V.
> > > >
> > > > 2) Introduced support for fw_devlink for ACPI nodes for IRQ dependency.
> > > > This helps to support deferred probe of interrupt controller drivers.
> > > >
> > > > 3) Modified pnp_irq() to try registering the IRQ  again if it sees it in
> > > > disabled state. This solution is similar to how
> > > > platform_get_irq_optional() works for regular platform devices.
> > > >
> > > > 4) Added support for re-ordering the probe of interrupt controllers when
> > > > IRQCHIP_ACPI_DECLARE is used.
> > > >
> > > > 5) ACPI support added in RISC-V interrupt controller drivers.
> > > >
> > > > This series is based on Anup's AIA v11 series. Since Anup's AIA v11 is
> > > > not merged yet and first time introducing fw_devlink, deferred probe and
> > > > reordering support for IRQCHIP probe, this series is still kept as RFC.
> > > > Looking forward for the feedback!
> > > >
> > > > Changes since RFC v2:
> > > >         1) Introduced fw_devlink for ACPI nodes for IRQ dependency.
> > > >         2) Dropped patches in drivers which are not required due to
> > > >            fw_devlink support.
> > > >         3) Dropped pci_set_msi() patch and added a patch in
> > > >            pci_create_root_bus().
> > > >         4) Updated pnp_irq() patch so that none of the actual PNP
> > > >            drivers need to change.
> > > >
> > > > Changes since RFC v1:
> > > >         1) Abandoned swnode approach as per Marc's feedback.
> > > >         2) To cope up with AIA series changes which changed irqchip driver
> > > >            probe from core_initcall() to platform_driver, added patches
> > > >            to support deferred probing.
> > > >         3) Rebased on top of Anup's AIA v11 and added tags.
> > > >
> > > > To test the series,
> > > >
> > > > 1) Qemu should be built using the riscv_acpi_b2_v8 branch at
> > > > https://github.com/vlsunil/qemu.git
> > > >
> > > > 2) EDK2 should be built using the instructions at:
> > > > https://github.com/tianocore/edk2/blob/master/OvmfPkg/RiscVVirt/README.md
> > > >
> > > > 3) Build Linux using this series on top of Anup's AIA v11 series.
> > > >
> > > > Run Qemu:
> > > > qemu-system-riscv64 \
> > > >  -M virt,pflash0=pflash0,pflash1=pflash1,aia=aplic-imsic \
> > > >  -m 2G -smp 8 \
> > > >  -serial mon:stdio \
> > > >  -device virtio-gpu-pci -full-screen \
> > > >  -device qemu-xhci \
> > > >  -device usb-kbd \
> > > >  -blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
> > > >  -blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
> > > >  -netdev user,id=net0 -device virtio-net-pci,netdev=net0 \
> > > >  -kernel arch/riscv/boot/Image \
> > > >  -initrd rootfs.cpio \
> > > >  -append "root=/dev/ram ro console=ttyS0 rootwait earlycon=uart8250,mmio,0x10000000"
> > > >
> > > > To boot with APLIC only, use aia=aplic.
> > > > To boot with PLIC, remove aia= option.
> > > >
> > > > This series is also available in acpi_b2_v3_riscv_aia_v11 branch at
> > > > https://github.com/vlsunil/linux.git
> > > >
> > > > Based-on: 20231023172800.315343-1-apatel@ventanamicro.com
> > > > (https://lore.kernel.org/lkml/20231023172800.315343-1-apatel@ventanamicro.com/)
> > > >
> > > > Sunil V L (17):
> > > >   arm64: PCI: Migrate ACPI related functions to pci-acpi.c
> > > >   RISC-V: ACPI: Implement PCI related functionality
> > > >   PCI: Make pci_create_root_bus() declare its reliance on MSI domains
> > > >   ACPI: Add fw_devlink support for ACPI fwnode for IRQ dependency
> > > >   ACPI: irq: Add support for deferred probe in acpi_register_gsi()
> > > >   pnp.h: Reconfigure IRQ in pnp_irq() to support deferred probe
> > > >   ACPI: scan.c: Add weak arch specific function to reorder the IRQCHIP
> > > >     probe
> > > >   ACPI: RISC-V: Implement arch function to reorder irqchip probe entries
> > > >   irqchip: riscv-intc: Add ACPI support for AIA
> > > >   irqchip: riscv-imsic: Add ACPI support
> > > >   irqchip: riscv-aplic: Add ACPI support
> > > >   irqchip: irq-sifive-plic: Add ACPI support
> > > >   ACPI: bus: Add RINTC IRQ model for RISC-V
> > > >   ACPI: bus: Add acpi_riscv_init function
> > > >   ACPI: RISC-V: Create APLIC platform device
> > > >   ACPI: RISC-V: Create PLIC platform device
> > > >   irqchip: riscv-intc: Set ACPI irqmodel
> > >
> > > JFYI, I have no capacity to provide any feedback on this till 6.8-rc1 is out.
> > >
> > Hi Rafael,
> >
> > Gentle ping.
> >
> > Could you please provide feedback on the series? Patches 4, 5, 6, 7 and
> > 8 are bit critical IMO. So, I really look forward for your and other
> > ACPI experts!.
> 
> There was quite a bit of discussion on patch [6/21] and it still seems
> relevant to me.
> 
> ACPI actually has a way to at least indicate what the probe ordering
> should be which is _DEP.
> 
> The current handling of _DEP in the kernel may not be covering this
> particular use case, but I would rather extend it (if necessary)
> instead of doing all of the -EPROBE_DEFER dance which seems fragile to
> me.
> 
Hi Rafael,

Appreciate your help to look at the patches. Thank you very much!.

I am not very sure whether you looked into patches in the v3 of the
series. Because, unlike in v2, v3 doesn't need changing all drivers to
handle EPROBE_DEFER. In v3, it creates fw_devlink for the dependency as
suggested by Marc. Please take a look at PATCH 4/17.

For the IRQ dependency, I think adding _DEP is not required. The
"Extended Interrupt Descriptor" supports ResourceSource to
indicate the dependency. Or GSI mapping can indicate the source. This is
already handled in acpi_irq_parse_one_cb(). PATCH 4 uses this
information to create links between producer and consumer so that DD
framework probes the driver in the required order.

As you know, PNP devices are enumerated in a different way. I don't know
why it was done like this. But pnpacpi_init() is called via
fs_initcall() and acpi_dev_resource_interrupt() called from
pnpacpi_allocated_resource() doesn't handle the ResourceSource
dependency. It caches the information in PNP data structure and expects
the IRQ mapping to be available. Even if we add support to
handle extended interrupt descriptor, it is not going to help. Hence, I
had to add PATCH 5/17 and PATCH 6/17. Again, the change is mainly in
pnp_irq() now and hence it doesn't need changing all drivers.

Thanks,
Sunil
Sunil V L March 4, 2024, 10:20 a.m. UTC | #4
On Fri, Feb 02, 2024 at 05:47:16PM +0530, Sunil V L wrote:
> On Thu, Feb 01, 2024 at 07:10:28PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 30, 2024 at 7:02 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > >
> > > On Tue, Dec 19, 2023 at 06:50:19PM +0100, Rafael J. Wysocki wrote:
> > > > On Tue, Dec 19, 2023 at 6:45 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > > >
> > > > > This series adds support for the below ECR approved by ASWG.
> > > > > 1) MADT - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
> > > > >
> > > > > The series primarily enables irqchip drivers for RISC-V ACPI based
> > > > > platforms.
> > > > >
> > > > > The series can be broadly categorized like below.
> > > > >
> > > > > 1) PCI ACPI related functions are migrated from arm64 to common file so
> > > > > that we don't need to duplicate them for RISC-V.
> > > > >
> > > > > 2) Introduced support for fw_devlink for ACPI nodes for IRQ dependency.
> > > > > This helps to support deferred probe of interrupt controller drivers.
> > > > >
> > > > > 3) Modified pnp_irq() to try registering the IRQ  again if it sees it in
> > > > > disabled state. This solution is similar to how
> > > > > platform_get_irq_optional() works for regular platform devices.
> > > > >
> > > > > 4) Added support for re-ordering the probe of interrupt controllers when
> > > > > IRQCHIP_ACPI_DECLARE is used.
> > > > >
> > > > > 5) ACPI support added in RISC-V interrupt controller drivers.
> > > > >
> > > > > This series is based on Anup's AIA v11 series. Since Anup's AIA v11 is
> > > > > not merged yet and first time introducing fw_devlink, deferred probe and
> > > > > reordering support for IRQCHIP probe, this series is still kept as RFC.
> > > > > Looking forward for the feedback!
> > > > >
> > > > > Changes since RFC v2:
> > > > >         1) Introduced fw_devlink for ACPI nodes for IRQ dependency.
> > > > >         2) Dropped patches in drivers which are not required due to
> > > > >            fw_devlink support.
> > > > >         3) Dropped pci_set_msi() patch and added a patch in
> > > > >            pci_create_root_bus().
> > > > >         4) Updated pnp_irq() patch so that none of the actual PNP
> > > > >            drivers need to change.
> > > > >
> > > > > Changes since RFC v1:
> > > > >         1) Abandoned swnode approach as per Marc's feedback.
> > > > >         2) To cope up with AIA series changes which changed irqchip driver
> > > > >            probe from core_initcall() to platform_driver, added patches
> > > > >            to support deferred probing.
> > > > >         3) Rebased on top of Anup's AIA v11 and added tags.
> > > > >
> > > > > To test the series,
> > > > >
> > > > > 1) Qemu should be built using the riscv_acpi_b2_v8 branch at
> > > > > https://github.com/vlsunil/qemu.git
> > > > >
> > > > > 2) EDK2 should be built using the instructions at:
> > > > > https://github.com/tianocore/edk2/blob/master/OvmfPkg/RiscVVirt/README.md
> > > > >
> > > > > 3) Build Linux using this series on top of Anup's AIA v11 series.
> > > > >
> > > > > Run Qemu:
> > > > > qemu-system-riscv64 \
> > > > >  -M virt,pflash0=pflash0,pflash1=pflash1,aia=aplic-imsic \
> > > > >  -m 2G -smp 8 \
> > > > >  -serial mon:stdio \
> > > > >  -device virtio-gpu-pci -full-screen \
> > > > >  -device qemu-xhci \
> > > > >  -device usb-kbd \
> > > > >  -blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
> > > > >  -blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
> > > > >  -netdev user,id=net0 -device virtio-net-pci,netdev=net0 \
> > > > >  -kernel arch/riscv/boot/Image \
> > > > >  -initrd rootfs.cpio \
> > > > >  -append "root=/dev/ram ro console=ttyS0 rootwait earlycon=uart8250,mmio,0x10000000"
> > > > >
> > > > > To boot with APLIC only, use aia=aplic.
> > > > > To boot with PLIC, remove aia= option.
> > > > >
> > > > > This series is also available in acpi_b2_v3_riscv_aia_v11 branch at
> > > > > https://github.com/vlsunil/linux.git
> > > > >
> > > > > Based-on: 20231023172800.315343-1-apatel@ventanamicro.com
> > > > > (https://lore.kernel.org/lkml/20231023172800.315343-1-apatel@ventanamicro.com/)
> > > > >
> > > > > Sunil V L (17):
> > > > >   arm64: PCI: Migrate ACPI related functions to pci-acpi.c
> > > > >   RISC-V: ACPI: Implement PCI related functionality
> > > > >   PCI: Make pci_create_root_bus() declare its reliance on MSI domains
> > > > >   ACPI: Add fw_devlink support for ACPI fwnode for IRQ dependency
> > > > >   ACPI: irq: Add support for deferred probe in acpi_register_gsi()
> > > > >   pnp.h: Reconfigure IRQ in pnp_irq() to support deferred probe
> > > > >   ACPI: scan.c: Add weak arch specific function to reorder the IRQCHIP
> > > > >     probe
> > > > >   ACPI: RISC-V: Implement arch function to reorder irqchip probe entries
> > > > >   irqchip: riscv-intc: Add ACPI support for AIA
> > > > >   irqchip: riscv-imsic: Add ACPI support
> > > > >   irqchip: riscv-aplic: Add ACPI support
> > > > >   irqchip: irq-sifive-plic: Add ACPI support
> > > > >   ACPI: bus: Add RINTC IRQ model for RISC-V
> > > > >   ACPI: bus: Add acpi_riscv_init function
> > > > >   ACPI: RISC-V: Create APLIC platform device
> > > > >   ACPI: RISC-V: Create PLIC platform device
> > > > >   irqchip: riscv-intc: Set ACPI irqmodel
> > > >
> > > > JFYI, I have no capacity to provide any feedback on this till 6.8-rc1 is out.
> > > >
> > > Hi Rafael,
> > >
> > > Gentle ping.
> > >
> > > Could you please provide feedback on the series? Patches 4, 5, 6, 7 and
> > > 8 are bit critical IMO. So, I really look forward for your and other
> > > ACPI experts!.
> > 
> > There was quite a bit of discussion on patch [6/21] and it still seems
> > relevant to me.
> > 
> > ACPI actually has a way to at least indicate what the probe ordering
> > should be which is _DEP.
> > 
> > The current handling of _DEP in the kernel may not be covering this
> > particular use case, but I would rather extend it (if necessary)
> > instead of doing all of the -EPROBE_DEFER dance which seems fragile to
> > me.
> > 
> Hi Rafael,
> 
> Appreciate your help to look at the patches. Thank you very much!.
> 
> I am not very sure whether you looked into patches in the v3 of the
> series. Because, unlike in v2, v3 doesn't need changing all drivers to
> handle EPROBE_DEFER. In v3, it creates fw_devlink for the dependency as
> suggested by Marc. Please take a look at PATCH 4/17.
> 
> For the IRQ dependency, I think adding _DEP is not required. The
> "Extended Interrupt Descriptor" supports ResourceSource to
> indicate the dependency. Or GSI mapping can indicate the source. This is
> already handled in acpi_irq_parse_one_cb(). PATCH 4 uses this
> information to create links between producer and consumer so that DD
> framework probes the driver in the required order.
> 
> As you know, PNP devices are enumerated in a different way. I don't know
> why it was done like this. But pnpacpi_init() is called via
> fs_initcall() and acpi_dev_resource_interrupt() called from
> pnpacpi_allocated_resource() doesn't handle the ResourceSource
> dependency. It caches the information in PNP data structure and expects
> the IRQ mapping to be available. Even if we add support to
> handle extended interrupt descriptor, it is not going to help. Hence, I
> had to add PATCH 5/17 and PATCH 6/17. Again, the change is mainly in
> pnp_irq() now and hence it doesn't need changing all drivers.
> 
Hi Rafael,

Any further feedback on this? Do you see any issues with fw_devlink and
pnp_irq() changes for the interrupt controller dependency?

BTW, I explored the _DEP option you mentioned. But I think the current
approach would probably be better with lesser impact than _DEP. Below are
my findings.

1) When PNP device like PNP0501 (16550A UART) has _DEP, it does not get
created in the first pass (as expected) but they won't get created even
after the supplier clears the dependency. This is because
acpi_pnp_attach() returns success without calling pnpacpi_add_device()
when acpi_scan_clear_dep_fn() calls acpi_bus_attach(). Either
acpi_pnp_attach() needs to be modified or pnpacpi_add_device()
should be called as part of clearing the dependency. I may be wrong but
I think modifying pnp_irq() would probably affect only RISC-V than any
of the approaches above.

2) _DEP doesn't support PCI devices. When we have PCI link devices
(PNP0C0F) in the _PRT, the PCI driver probe as part of PCI scan can happen
prior to link device probe since link device will have dependency on the
interrupt controller which may not be probed yet. This will cause issue
to the PCI device driver init because _PRT says there is link device for
legacy PCI interrupt routing but the link driver is not probed yet.

3) _DEP needs namespace interrupt controllers. We would like to avoid
adding namespace devices also along with MADT.

Please let me know your thoughts.

Thanks,
Sunil
Sunil V L April 15, 2024, 3:09 p.m. UTC | #5
On Thu, Feb 01, 2024 at 07:10:28PM +0100, Rafael J. Wysocki wrote:
> On Tue, Jan 30, 2024 at 7:02 AM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > On Tue, Dec 19, 2023 at 06:50:19PM +0100, Rafael J. Wysocki wrote:
> > > On Tue, Dec 19, 2023 at 6:45 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> > > >
> > > > This series adds support for the below ECR approved by ASWG.
> > > > 1) MADT - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing
> > > >
> > > > The series primarily enables irqchip drivers for RISC-V ACPI based
> > > > platforms.
> > > >
> > > > The series can be broadly categorized like below.
> > > >
> > > > 1) PCI ACPI related functions are migrated from arm64 to common file so
> > > > that we don't need to duplicate them for RISC-V.
> > > >
> > > > 2) Introduced support for fw_devlink for ACPI nodes for IRQ dependency.
> > > > This helps to support deferred probe of interrupt controller drivers.
> > > >
> > > > 3) Modified pnp_irq() to try registering the IRQ  again if it sees it in
> > > > disabled state. This solution is similar to how
> > > > platform_get_irq_optional() works for regular platform devices.
> > > >
> > > > 4) Added support for re-ordering the probe of interrupt controllers when
> > > > IRQCHIP_ACPI_DECLARE is used.
> > > >
> > > > 5) ACPI support added in RISC-V interrupt controller drivers.
> > > >
> > > > This series is based on Anup's AIA v11 series. Since Anup's AIA v11 is
> > > > not merged yet and first time introducing fw_devlink, deferred probe and
> > > > reordering support for IRQCHIP probe, this series is still kept as RFC.
> > > > Looking forward for the feedback!
> > > >
> > > > Changes since RFC v2:
> > > >         1) Introduced fw_devlink for ACPI nodes for IRQ dependency.
> > > >         2) Dropped patches in drivers which are not required due to
> > > >            fw_devlink support.
> > > >         3) Dropped pci_set_msi() patch and added a patch in
> > > >            pci_create_root_bus().
> > > >         4) Updated pnp_irq() patch so that none of the actual PNP
> > > >            drivers need to change.
> > > >
> > > > Changes since RFC v1:
> > > >         1) Abandoned swnode approach as per Marc's feedback.
> > > >         2) To cope up with AIA series changes which changed irqchip driver
> > > >            probe from core_initcall() to platform_driver, added patches
> > > >            to support deferred probing.
> > > >         3) Rebased on top of Anup's AIA v11 and added tags.
> > > >
> > > > To test the series,
> > > >
> > > > 1) Qemu should be built using the riscv_acpi_b2_v8 branch at
> > > > https://github.com/vlsunil/qemu.git
> > > >
> > > > 2) EDK2 should be built using the instructions at:
> > > > https://github.com/tianocore/edk2/blob/master/OvmfPkg/RiscVVirt/README.md
> > > >
> > > > 3) Build Linux using this series on top of Anup's AIA v11 series.
> > > >
> > > > Run Qemu:
> > > > qemu-system-riscv64 \
> > > >  -M virt,pflash0=pflash0,pflash1=pflash1,aia=aplic-imsic \
> > > >  -m 2G -smp 8 \
> > > >  -serial mon:stdio \
> > > >  -device virtio-gpu-pci -full-screen \
> > > >  -device qemu-xhci \
> > > >  -device usb-kbd \
> > > >  -blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
> > > >  -blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
> > > >  -netdev user,id=net0 -device virtio-net-pci,netdev=net0 \
> > > >  -kernel arch/riscv/boot/Image \
> > > >  -initrd rootfs.cpio \
> > > >  -append "root=/dev/ram ro console=ttyS0 rootwait earlycon=uart8250,mmio,0x10000000"
> > > >
> > > > To boot with APLIC only, use aia=aplic.
> > > > To boot with PLIC, remove aia= option.
> > > >
> > > > This series is also available in acpi_b2_v3_riscv_aia_v11 branch at
> > > > https://github.com/vlsunil/linux.git
> > > >
> > > > Based-on: 20231023172800.315343-1-apatel@ventanamicro.com
> > > > (https://lore.kernel.org/lkml/20231023172800.315343-1-apatel@ventanamicro.com/)
> > > >
> > > > Sunil V L (17):
> > > >   arm64: PCI: Migrate ACPI related functions to pci-acpi.c
> > > >   RISC-V: ACPI: Implement PCI related functionality
> > > >   PCI: Make pci_create_root_bus() declare its reliance on MSI domains
> > > >   ACPI: Add fw_devlink support for ACPI fwnode for IRQ dependency
> > > >   ACPI: irq: Add support for deferred probe in acpi_register_gsi()
> > > >   pnp.h: Reconfigure IRQ in pnp_irq() to support deferred probe
> > > >   ACPI: scan.c: Add weak arch specific function to reorder the IRQCHIP
> > > >     probe
> > > >   ACPI: RISC-V: Implement arch function to reorder irqchip probe entries
> > > >   irqchip: riscv-intc: Add ACPI support for AIA
> > > >   irqchip: riscv-imsic: Add ACPI support
> > > >   irqchip: riscv-aplic: Add ACPI support
> > > >   irqchip: irq-sifive-plic: Add ACPI support
> > > >   ACPI: bus: Add RINTC IRQ model for RISC-V
> > > >   ACPI: bus: Add acpi_riscv_init function
> > > >   ACPI: RISC-V: Create APLIC platform device
> > > >   ACPI: RISC-V: Create PLIC platform device
> > > >   irqchip: riscv-intc: Set ACPI irqmodel
> > >
> > > JFYI, I have no capacity to provide any feedback on this till 6.8-rc1 is out.
> > >
> > Hi Rafael,
> >
> > Gentle ping.
> >
> > Could you please provide feedback on the series? Patches 4, 5, 6, 7 and
> > 8 are bit critical IMO. So, I really look forward for your and other
> > ACPI experts!.
> 
> There was quite a bit of discussion on patch [6/21] and it still seems
> relevant to me.
> 
> ACPI actually has a way to at least indicate what the probe ordering
> should be which is _DEP.
> 
> The current handling of _DEP in the kernel may not be covering this
> particular use case, but I would rather extend it (if necessary)
> instead of doing all of the -EPROBE_DEFER dance which seems fragile to
> me.
> 
Hi Rafael,                                                                                                                                                                                                  
                                                                                                                                                                                                            
I found that _DEP is better than fw_devlink for ACPI since it works                                                                                                                                         
during scan itself. For ex: the way PCI host bridges are scanned in                                                                                                                                         
ACPI, PNP device handling. While some hacky solution could be done like                                                                                                                                     
I did in v3 (pnp_irq() changes), _DEP way seems much better.                                                                                                                                                
                                                                                                                                                                                                            
So, we decided to go with your suggested approach and defined namespace                                                                                                                                     
devices for PLIC and APLIC required to create dependency mechanism.                                                                                                                                         
However, there are concerns that every device will have to add _DEP now                                                                                                                                     
and whether it is intended for this use case. Also, actually the                                                                                                                                            
dependency is already available in the form of GSI number mapping.                                                                                                                                          
Hence, instead of explicit dependency, we would like to create implicit                                                                                                                                     
dependency. So, I will send RFC v4 series with those changes. Please                                                                                                                                        
help us with feedback. If you could provide quick high level go/no-go,                                                                                                                                      
that will help us a lot since we need to finalize the spec whether to                                                                                                                                       
mandate _DEP or not.                                                                                                                                                                                        
                                                                                                                                                                                                            
I request you to provide some high level feedback in the RFC v4 I am                                                                                                                                        
going to send.                                                                                                                                                                                              
                                                                                                                                                                                                            
1) Creation of implicit dependency.                                                                                                                                                                         
2) Handling PNP devices as part of clearing dependency                                                                                                                                                      
3) IRQCHIP probing reordering when there are multiple irqchips.                                                                                                                                             
                                                                                                                                                                                                            
Thanks,                                                                                                                                                                                                     
Sunil
Rafael J. Wysocki April 15, 2024, 3:18 p.m. UTC | #6
Hi,

[cut]

> >
> Hi Rafael,
>
> I found that _DEP is better than fw_devlink for ACPI since it works
> during scan itself. For ex: the way PCI host bridges are scanned in
> ACPI, PNP device handling. While some hacky solution could be done like
> I did in v3 (pnp_irq() changes), _DEP way seems much better.
>
> So, we decided to go with your suggested approach and defined namespace
> devices for PLIC and APLIC required to create dependency mechanism.
> However, there are concerns that every device will have to add _DEP now
> and whether it is intended for this use case. Also, actually the
> dependency is already available in the form of GSI number mapping.
> Hence, instead of explicit dependency, we would like to create implicit
> dependency. So, I will send RFC v4 series with those changes. Please
> help us with feedback. If you could provide quick high level go/no-go,
> that will help us a lot since we need to finalize the spec whether to
> mandate _DEP or not.
>
> I request you to provide some high level feedback in the RFC v4 I am
> going to send.

I will, thank you!