PCI: apple: Add depends on PAGE_SIZE_16KB

Commit Message

Alyssa Rosenzweig Feb. 11, 2025, 6:03 p.m. UTC

From: Janne Grunau <[e-mail]>

The iommu on Apple's M1 and M2 supports only a page size of 16kB and is
mandatory for PCIe devices. Mismatched page sizes will render devices
useless due to non-working DMA. While the iommu prints a warning in this
scenario, it seems a common and hard to debug problem, so prevent it at
build-time.

Signed-off-by: Janne Grunau <[e-mail]>
Signed-off-by: Alyssa Rosenzweig <[e-mail]>
---
 drivers/pci/controller/Kconfig | 1 +
 1 file changed, 1 insertion(+)


---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20250211-pci-16k-4c391a5dcd18

Best regards,

Comments

Bjorn Helgaas Feb. 11, 2025, 6:38 p.m. UTC | #1

On Tue, Feb 11, 2025 at 01:03:52PM -0500, Alyssa Rosenzweig wrote:
> From: Janne Grunau <[e-mail]>
> 
> The iommu on Apple's M1 and M2 supports only a page size of 16kB and is
> mandatory for PCIe devices. Mismatched page sizes will render devices
> useless due to non-working DMA. While the iommu prints a warning in this
> scenario, it seems a common and hard to debug problem, so prevent it at
> build-time.

Can we include a sample iommu warning here to help people debug this
problem?

> Signed-off-by: Janne Grunau <[e-mail]>
> Signed-off-by: Alyssa Rosenzweig <[e-mail]>
> ---
>  drivers/pci/controller/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 9800b768105402d6dd1ba4b134c2ec23da6e4201..507e6ac5d65257578e4eec74b459f6605c9c2907 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -39,6 +39,7 @@ config PCIE_APPLE
>  	depends on ARCH_APPLE || COMPILE_TEST
>  	depends on OF
>  	depends on PCI_MSI
> +	depends on PAGE_SIZE_16KB || COMPILE_TEST
>  	select PCI_HOST_COMMON
>  	help
>  	  Say Y here if you want to enable PCIe controller support on Apple
> 
> ---
> base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
> change-id: 20250211-pci-16k-4c391a5dcd18
> 
> Best regards,
> -- 
> Alyssa Rosenzweig <[e-mail]>
>

Janne Grunau Feb. 11, 2025, 7:56 p.m. UTC | #2

On Tue, Feb 11, 2025 at 12:38:59PM -0600, Bjorn Helgaas wrote:
> On Tue, Feb 11, 2025 at 01:03:52PM -0500, Alyssa Rosenzweig wrote:
> > From: Janne Grunau <[e-mail]>
> > 
> > The iommu on Apple's M1 and M2 supports only a page size of 16kB and is
> > mandatory for PCIe devices. Mismatched page sizes will render devices
> > useless due to non-working DMA. While the iommu prints a warning in this
> > scenario, it seems a common and hard to debug problem, so prevent it at
> > build-time.
> 
> Can we include a sample iommu warning here to help people debug this
> problem?

I don't remember and it might have changed in the meantime due to iommu
subsystem changes. What currently happens is that
apple_dart_attach_dev_identity() fails with -EINVAL. I can't say whether
that results in a failure to probe now. I'll test and report back.

Janne

Janne Grunau Feb. 11, 2025, 10 p.m. UTC | #3

On Tue, Feb 11, 2025 at 08:56:04PM +0100, Janne Grunau wrote:
> On Tue, Feb 11, 2025 at 12:38:59PM -0600, Bjorn Helgaas wrote:
> > On Tue, Feb 11, 2025 at 01:03:52PM -0500, Alyssa Rosenzweig wrote:
> > > From: Janne Grunau <[e-mail]>
> > > 
> > > The iommu on Apple's M1 and M2 supports only a page size of 16kB and is
> > > mandatory for PCIe devices. Mismatched page sizes will render devices
> > > useless due to non-working DMA. While the iommu prints a warning in this
> > > scenario, it seems a common and hard to debug problem, so prevent it at
> > > build-time.
> > 
> > Can we include a sample iommu warning here to help people debug this
> > problem?
> 
> I don't remember and it might have changed in the meantime due to iommu
> subsystem changes. What currently happens is that
> apple_dart_attach_dev_identity() fails with -EINVAL. I can't say whether
> that results in a failure to probe now. I'll test and report back.

Using a kernel with 4K page size It results now in following warning per
PCI device:

| ------------[ cut here ]------------
| WARNING: CPU: 7 PID: 260 at drivers/iommu/iommu.c:2979 iommu_setup_default_domain+0x348/0x590
| Modules linked in: sm4_ce_gcm(-) tg3(+) nvme_apple(+) apple_sart nvme_core snd_soc_apple_mca clk_apple_nco snd_pcm_dmaengine apple_admac i2c_pasemi_platform apple_dart apple_soc_cpufreq i2c_pasemi_core
| CPU: 7 UID: 0 PID: 260 Comm: systemd-udevd Not tainted 6.14.0-rc1+ #asahi-dev
| Hardware name: Apple Mac mini (M1, 2020) (DT)
| pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
| pc : iommu_setup_default_domain+0x348/0x590
| lr : iommu_setup_default_domain+0x340/0x590
| sp : ffffffc082c3b6b0
| x29: ffffffc082c3b6b0 x28: 0000000000000000 x27: ffffffc082c3bcc0
| x26: ffffffc082c3bbc0 x25: ffffffc0817d12f8 x24: ffffffc0816bf6c4
| x23: 0000000000000000 x22: ffffff80301d5400 x21: ffffff80301d5448
| x20: ffffffc079a332b8 x19: 00000000ffffffea x18: 0000000000000000
| x17: 00000000007bb020 x16: 0000000000000020 x15: 0000000000000004
| x14: 0000000000000002 x13: 0000000000000001 x12: 0000000000000000
| x11: 0000000000000000 x10: 0000000000082630 x9 : ffffffc0808e7ad4
| x8 : 0101010101010101 x7 : 0000000000000000 x6 : 0304000af2f9adf2
| x5 : ffffff803030a480 x4 : ffffff81dfb29d40 x3 : 0000000000000001
| x2 : ffffffc079a2d408 x1 : ffffff80266f60c8 x0 : 00000000ffffffea
| Call trace:
|  iommu_setup_default_domain+0x348/0x590 (P)
|  __iommu_probe_device+0x340/0x3f8
|  iommu_probe_device+0x3c/0x88
|  of_iommu_configure+0xa8/0x258
|  of_dma_configure_id+0x114/0x2a0
|  pci_dma_configure+0x5c/0xc0
|  really_probe+0x7c/0x3a0
|  __driver_probe_device+0x84/0x160
|  driver_probe_device+0x44/0x130
|  __driver_attach+0xcc/0x208
|  bus_for_each_dev+0x90/0x100
|  driver_attach+0x2c/0x40
|  bus_add_driver+0x118/0x250
|  driver_register+0x70/0x140
|  __pci_register_driver+0x4c/0x60
|  tg3_driver_init+0x30/0xff8 [tg3]
|  do_one_initcall+0x60/0x2a0
|  do_init_module+0x5c/0x228
|  load_module+0x1aec/0x20d8
|  init_module_from_file+0x90/0xe0
|  __arm64_sys_finit_module+0x274/0x380
|  invoke_syscall.constprop.0+0x58/0xf0
|  do_el0_svc+0x48/0xe8
|  el0_svc+0x34/0x148
|  el0t_64_sync_handler+0x10c/0x140
|  el0t_64_sync+0x198/0x1a0
| ---[ end trace 0000000000000000 ]---

In addition the ethernet device for the connected tg3 does not exists
and PCIe xhci_hcd spews following errors:

| xhci_hcd 0000:02:00.0: Abort failed to stop command ring: -110
| xhci_hcd 0000:02:00.0: xHCI host controller not responding, assume dead
| xhci_hcd 0000:02:00.0: HC died; cleaning up
| xhci_hcd 0000:02:00.0: Error while assigning device slot ID: Command Aborted
| xhci_hcd 0000:02:00.0: Max number of devices this xHCI host supports is 32.
| usb usb1-port1: couldn't allocate usb_device
| usb usb2-port1: couldn't allocate usb_device

This is much less sutle than I remember up to the point that I'm not
sure if this change is still needed.

Silently disabled CONFIG_* due to missing dependencies aren't nice either.

Janne

Manivannan Sadhasivam Feb. 14, 2025, 3:44 p.m. UTC | #4

On Tue, Feb 11, 2025 at 11:00:19PM +0100, Janne Grunau wrote:
> On Tue, Feb 11, 2025 at 08:56:04PM +0100, Janne Grunau wrote:
> > On Tue, Feb 11, 2025 at 12:38:59PM -0600, Bjorn Helgaas wrote:
> > > On Tue, Feb 11, 2025 at 01:03:52PM -0500, Alyssa Rosenzweig wrote:
> > > > From: Janne Grunau <[e-mail]>
> > > > 
> > > > The iommu on Apple's M1 and M2 supports only a page size of 16kB and is
> > > > mandatory for PCIe devices. Mismatched page sizes will render devices
> > > > useless due to non-working DMA. While the iommu prints a warning in this
> > > > scenario, it seems a common and hard to debug problem, so prevent it at
> > > > build-time.
> > > 
> > > Can we include a sample iommu warning here to help people debug this
> > > problem?
> > 
> > I don't remember and it might have changed in the meantime due to iommu
> > subsystem changes. What currently happens is that
> > apple_dart_attach_dev_identity() fails with -EINVAL. I can't say whether
> > that results in a failure to probe now. I'll test and report back.
> 
> Using a kernel with 4K page size It results now in following warning per
> PCI device:
> 
> | ------------[ cut here ]------------
> | WARNING: CPU: 7 PID: 260 at drivers/iommu/iommu.c:2979 iommu_setup_default_domain+0x348/0x590

This should be added to the patch description.

- Mani

Alyssa Rosenzweig Feb. 16, 2025, 7:51 p.m. UTC | #5

> This is much less sutle than I remember up to the point that I'm not
> sure if this change is still needed.
> 
> Silently disabled CONFIG_* due to missing dependencies aren't nice either.

Very true. Plus, it seems useful to be able to build-test the apple pcie
driver in a 4K situation, to catch build-breaks sooner for cross-driver
changes.

Let's drop this patch from our tree?

Alyssa

Patch

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 9800b768105402d6dd1ba4b134c2ec23da6e4201..507e6ac5d65257578e4eec74b459f6605c9c2907 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -39,6 +39,7 @@  config PCIE_APPLE
 	depends on ARCH_APPLE || COMPILE_TEST
 	depends on OF
 	depends on PCI_MSI
+	depends on PAGE_SIZE_16KB || COMPILE_TEST
 	select PCI_HOST_COMMON
 	help
 	  Say Y here if you want to enable PCIe controller support on Apple