diff mbox series

[v2] riscv: Kconfig: do not select PCI_MSI if CONIFG_PCI is enabled

Message ID 1626838752-7013-1-git-send-email-vincent.chen@sifive.com (mailing list archive)
State New, archived
Headers show
Series [v2] riscv: Kconfig: do not select PCI_MSI if CONIFG_PCI is enabled | expand

Commit Message

Vincent Chen July 21, 2021, 3:39 a.m. UTC
The CONFIG_PCI_MSI is used to allow device drivers to enable MSI. The MSI
enables a device to generate an interrupt using an inbound Memory Write
on its PCI bus instead of asserting a device IRQ pin. The whole mechanism
needs support from the PCI controller or generic interrupt controller and
the corresponding software driver.

The RISC-V Kconfig file actively selects the PCI_MSI  if users enable
CONFIG_PCI. However, the RISC-V specification does not require every RISC-V
platform shall have MSI support. In other words, Kconfig enables CONFIG_PCI
to allow PCI devices to use MSI, but due to lack of MSI support, the kernel
may not have any function to deal with the MSI from PCI devices. When this
case happens, it leads to the following warning message displayed in
booting a v5.10~v5.12 kernel.

------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at include/linux/msi.h:219 __pci_enable_msix_range+0x4b6/0x50e
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.10.34 #76
epc: ffffffe0004fa1e0 ra : ffffffe0004fa02e sp : ffffffe07fea7920
 gp : ffffffe0010e25c0 tp : ffffffe07fea8000 t0 : ffffffe0808c3180
 t1 : ffffffd000239000 t2 : 0000000000000400 s0 : ffffffe07fea79e0
 s1 : ffffffe080803000 a0 : 0000000000000000 a1 : ffffffe080803210
 a2 : ffffffe0808c30c0 a3 : 0000000000000101 a4 : ffffffe0808c30c0
 a5 : 0000000000000010 a6 : ffffffe0004dcf98 a7 : ffffffe07fe0f010
 s2 : 0000000000000002 s3 : 0000000000000014 s4 : 0000000000000000
 s5 : ffffffe080803210 s6 : 0000000000000000 s7 : ffffffe0808c3120
 s8 : ffffffe0808c30c0 s9 : 0000000000000000 s10: 0000000000000002
 s11: ffffffe0808030b0 t3 : 0000000000001000 t4 : 0000000000001000
 t5 : ffffffd040239000 t6 : ffffffe07ff8b2c0
status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003

A simple reproduce way is to use defconfig to configure the 64-bit riscv
v5.10 ~ v5.12 kernel but disable CONFIG_PCIE_XILINX, and then booting this
kernel on the QEMU virt platform with virtio-mouse-pci.

ARM's implementation may be a good sample for this case. Its Kconfig file
does not select CONFIG_PCI_MSI. Instead, It makes the selection of
CONFIG_PCI_MSI depends on the capability of the interrupt controller. This
way seems to be more straightforward. Therefore, this patch follows the
same way to remove the dependency between CONFIG_PCI and CONFIG_PCI_MSI
from Kconfig, which allows users to enable or disable CONFIG_PCI_MSI
according to the capabilities of the platform.

Fixes: eb01d42a7778 ("PCI: consolidate PCI config entry in drivers/pci")
Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
Reviewed-by: Jisheng Zhang <jszhang@kernel.org>

---
 arch/riscv/Kconfig | 1 -
 1 file changed, 1 deletion(-)

Comments

Christoph Hellwig July 21, 2021, 5:57 a.m. UTC | #1
On Wed, Jul 21, 2021 at 11:39:12AM +0800, Vincent Chen wrote:
> The CONFIG_PCI_MSI is used to allow device drivers to enable MSI. The MSI
> enables a device to generate an interrupt using an inbound Memory Write
> on its PCI bus instead of asserting a device IRQ pin. The whole mechanism
> needs support from the PCI controller or generic interrupt controller and
> the corresponding software driver.
> 
> The RISC-V Kconfig file actively selects the PCI_MSI  if users enable
> CONFIG_PCI. However, the RISC-V specification does not require every RISC-V
> platform shall have MSI support. In other words, Kconfig enables CONFIG_PCI
> to allow PCI devices to use MSI, but due to lack of MSI support, the kernel
> may not have any function to deal with the MSI from PCI devices. When this
> case happens, it leads to the following warning message displayed in
> booting a v5.10~v5.12 kernel.

So make sure the runtime detection works fine.  An x86 kernel with
CONFIG_PCI_MSI also works on non-MSI systems.

NAK to this patch.
Vincent Chen July 22, 2021, 2:21 a.m. UTC | #2
On Wed, Jul 21, 2021 at 1:57 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Jul 21, 2021 at 11:39:12AM +0800, Vincent Chen wrote:
> > The CONFIG_PCI_MSI is used to allow device drivers to enable MSI. The MSI
> > enables a device to generate an interrupt using an inbound Memory Write
> > on its PCI bus instead of asserting a device IRQ pin. The whole mechanism
> > needs support from the PCI controller or generic interrupt controller and
> > the corresponding software driver.
> >
> > The RISC-V Kconfig file actively selects the PCI_MSI  if users enable
> > CONFIG_PCI. However, the RISC-V specification does not require every RISC-V
> > platform shall have MSI support. In other words, Kconfig enables CONFIG_PCI
> > to allow PCI devices to use MSI, but due to lack of MSI support, the kernel
> > may not have any function to deal with the MSI from PCI devices. When this
> > case happens, it leads to the following warning message displayed in
> > booting a v5.10~v5.12 kernel.
>
> So make sure the runtime detection works fine.  An x86 kernel with
> CONFIG_PCI_MSI also works on non-MSI systems.
>
> NAK to this patch.

Thanks for your information. I think this will be a good reference to
solve this warning message for the v5.10~v5.12 riscv kernel. However,
the current RISC-V does not specify that each platform must have a
controller to support PCI_MSI. Even there is no a ratified
specification to provide an interrupt controller to support MSI. In
this circumstance, removing the CONFIG_PCI_MSI selection from the
Kconfig file seems to more close to the RISC-V platform setting. I
guess that is why most architectures, such as ARM and x86, do not add
"select PCI_MSI if PCI" to their Kconfig file. Therefore, I prefer to
remove the "select PCI_MSI if PCI" from the Kconfig file. Please
correct me if I have misunderstands.

Thank you.
Vincent Chen Aug. 2, 2021, 5:23 a.m. UTC | #3
Just a gentle ping. If there are still problems with this patch,
please let me know. Thank you.



On Thu, Jul 22, 2021 at 10:21 AM Vincent Chen <vincent.chen@sifive.com> wrote:
>
> On Wed, Jul 21, 2021 at 1:57 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Wed, Jul 21, 2021 at 11:39:12AM +0800, Vincent Chen wrote:
> > > The CONFIG_PCI_MSI is used to allow device drivers to enable MSI. The MSI
> > > enables a device to generate an interrupt using an inbound Memory Write
> > > on its PCI bus instead of asserting a device IRQ pin. The whole mechanism
> > > needs support from the PCI controller or generic interrupt controller and
> > > the corresponding software driver.
> > >
> > > The RISC-V Kconfig file actively selects the PCI_MSI  if users enable
> > > CONFIG_PCI. However, the RISC-V specification does not require every RISC-V
> > > platform shall have MSI support. In other words, Kconfig enables CONFIG_PCI
> > > to allow PCI devices to use MSI, but due to lack of MSI support, the kernel
> > > may not have any function to deal with the MSI from PCI devices. When this
> > > case happens, it leads to the following warning message displayed in
> > > booting a v5.10~v5.12 kernel.
> >
> > So make sure the runtime detection works fine.  An x86 kernel with
> > CONFIG_PCI_MSI also works on non-MSI systems.
> >
> > NAK to this patch.
>
> Thanks for your information. I think this will be a good reference to
> solve this warning message for the v5.10~v5.12 riscv kernel. However,
> the current RISC-V does not specify that each platform must have a
> controller to support PCI_MSI. Even there is no a ratified
> specification to provide an interrupt controller to support MSI. In
> this circumstance, removing the CONFIG_PCI_MSI selection from the
> Kconfig file seems to more close to the RISC-V platform setting. I
> guess that is why most architectures, such as ARM and x86, do not add
> "select PCI_MSI if PCI" to their Kconfig file. Therefore, I prefer to
> remove the "select PCI_MSI if PCI" from the Kconfig file. Please
> correct me if I have misunderstands.
>
> Thank you.
Christoph Hellwig Aug. 3, 2021, 8:21 a.m. UTC | #4
Nothing changed at all from my previous reply.  This patch is wrong,
and you need to implement proper runtime detection.
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 469a70bd8da6..563d550cb682 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -97,7 +97,6 @@  config RISCV
 	select OF_EARLY_FLATTREE
 	select OF_IRQ
 	select PCI_DOMAINS_GENERIC if PCI
-	select PCI_MSI if PCI
 	select RISCV_INTC
 	select RISCV_TIMER if RISCV_SBI
 	select SPARSE_IRQ