Re: [PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers
From: Qian Cai <hidden>
Date: 2020-12-08 16:09:46
Also in:
linux-arm-kernel, linux-next, linux-pci, linux-rockchip
On Sun, 2020-11-29 at 23:07 +0000, Krzysztof Wilczyński wrote:
Unify ECAM-related constants into a single set of standard constants
defining memory address shift values for the byte-level address that can
be used when accessing the PCI Express Configuration Space, and then
move native PCI Express controller drivers to use newly introduced
definitions retiring any driver-specific ones.
The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
PCI Express specification (see PCI Express Base Specification, Revision
5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
implement it the same way.
Most of the native PCI Express controller drivers define their ECAM-related
constants, many of these could be shared, or use open-coded values when
setting the ".bus_shift" field of the "struct pci_ecam_ops".
All of the newly added constants should remove ambiguity and reduce the
number of open-coded values, and also correlate more strongly with the
descriptions in the aforementioned specification (see Table 7-1
"Enhanced Configuration Address Mapping", p. 677).
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Krzysztof Wilczyński <redacted>Reverting this series on the top of today's linux-next fixed a boot crash on arm64 Thunder X2 server. .config: https://cailca.coding.net/public/linux/mm/git/files/master/arm64.config [ 186.285957][ T1] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-7f]) [ 186.293127][ T1] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig Segments MSI HPX-Type3] [ 186.317072][ T1] acpi PNP0A08:00: _OSC: not requesting OS control; OS requires [ExtendedConfig ASPM ClockPM MSI] [ 186.330336][ T1] acpi PNP0A08:00: ECAM area [mem 0x30000000-0x37ffffff] reserved by PNP0C02:00 [ 186.339538][ T1] acpi PNP0A08:00: ECAM at [mem 0x30000000-0x37ffffff] for [bus 00-7f] [ 186.353258][ T1] PCI host bridge to bus 0000:00 [ 186.358162][ T1] pci_bus 0000:00: root bus resource [mem 0x40000000-0x5fffffff window] [ 186.366509][ T1] pci_bus 0000:00: root bus resource [mem 0x10000000000-0x13fffffffff window] [ 186.375366][ T1] pci_bus 0000:00: root bus resource [bus 00-7f] [ 186.382652][ T1] pci 0000:00:00.0: [177d:af00] type 00 class 0x060000 [ 186.395174][ T1] pci 0000:00:01.0: [177d:af84] type 01 class 0x060400 [ 186.402433][ T1] pci 0000:00:01.0: PME# supported from D0 D3hot D3cold [ 186.415652][ T1] Unable to handle kernel paging request at virtual address ffff800029370000 [ 186.424398][ T1] Mem abort info: [ 186.427930][ T1] ESR = 0x96000007 [ 186.431725][ T1] EC = 0x25: DABT (current EL), IL = 32 bits [ 186.437805][ T1] SET = 0, FnV = 0 [ 186.441599][ T1] EA = 0, S1PTW = 0 [ 186.445485][ T1] Data abort info: [ 186.449104][ T1] ISV = 0, ISS = 0x00000007 [ 186.453687][ T1] CM = 0, WnR = 0 [ 186.457396][ T1] swapper pgtable: 64k pages, 48-bit VAs, pgdp=00000000da920000 [ 186.464979][ T1] [ffff800029370000] pgd=0000008ffcff0003, p4d=0000008ffcff0003, pud=0000008ffcff0003, pmd=00000000811d0003, pte=0000000000000000 [ 186.478424][ T1] Internal error: Oops: 96000007 [#1] SMP [ 186.484059][ T1] Modules linked in: [ 186.487854][ T1] CPU: 38 PID: 1 Comm: swapper/0 Tainted: G W L 5.10.0-rc7-next-20201208 #3 [ 186.497617][ T1] Hardware name: HPE Apollo 70 /C01_APACHE_MB , BIOS L50_5.13_1.16 07/29/2020 [ 186.508174][ T1] pstate: 20400089 (nzCv daIf +PAN -UAO -TCO BTYPE=--) [ 186.514954][ T1] pc : pci_generic_config_read+0x78/0x1d0 [ 186.520587][ T1] lr : pci_generic_config_read+0x64/0x1d0 pci_generic_config_read at drivers/pci/access.c:83 [ 186.526223][ T1] sp : ffff000005f0ef30 [ 186.530278][ T1] x29: ffff000005f0ef30 x28: 0000000000000010 [ 186.536359][ T1] x27: 0000000000000000 x26: 0000000000000087 [ 186.542441][ T1] x25: 0000000000000000 x24: ffff00084a3a5000 [ 186.548517][ T1] x23: ffff000005f0f150 x22: 0000000000000004 [ 186.554593][ T1] x21: ffff800011404588 x20: ffff000005f0eff0 [ 186.560669][ T1] x19: ffff00084a3a5000 x18: 1fffe001cf0d53ed [ 186.566750][ T1] x17: 0000000000000000 x16: 0000000000000003 [ 186.572831][ T1] x15: 0000000000000000 x14: 0000000000000003 [ 186.578908][ T1] x13: ffff600000be1ddf x12: 1fffe00000be1dde [ 186.584983][ T1] x11: 1fffe00000be1dde x10: ffff600000be1dde [ 186.591059][ T1] x9 : ffff800010c4f59c x8 : ffff000005f0eef3 [ 186.597139][ T1] x7 : 0000000000000001 x6 : 0000000000000001 [ 186.603222][ T1] x5 : 1fffe00109474a1c x4 : 1fffe010fd074cb2 [ 186.609298][ T1] x3 : 0000000000000000 x2 : 0000000000000000 [ 186.615374][ T1] x1 : 0000000000010000 x0 : ffff800029370000 [ 186.621451][ T1] Call trace: [ 186.624623][ T1] pci_generic_config_read+0x78/0x1d0 __raw_readl at arch/arm64/include/asm/io.h:75 (inlined by) pci_generic_config_read at drivers/pci/access.c:93 [ 186.629905][ T1] pci_bus_read_config_dword+0xfc/0x198 pci_bus_read_config_dword at drivers/pci/access.c:65 (discriminator 2) [ 186.635362][ T1] pci_bus_generic_read_dev_vendor_id+0x3c/0x310 pci_bus_generic_read_dev_vendor_id at drivers/pci/probe.c:2300 [ 186.641611][ T1] pci_bus_read_dev_vendor_id+0x7c/0xd0 pci_bus_read_dev_vendor_id at drivers/pci/probe.c:2329 [ 186.647067][ T1] pci_scan_single_device+0xe0/0x1f8 pci_scan_device at drivers/pci/probe.c:2342 (inlined by) pci_scan_single_device at drivers/pci/probe.c:2511 (inlined by) pci_scan_single_device at drivers/pci/probe.c:2501 [ 186.652261][ T1] pci_scan_slot+0x88/0x258 pci_scan_slot at drivers/pci/probe.c:2590 [ 186.656663][ T1] pci_scan_child_bus_extend+0x88/0x608 pci_scan_child_bus_extend at drivers/pci/probe.c:2807 [ 186.662120][ T1] pci_scan_child_bus+0x18/0x20 pci_scan_child_bus at drivers/pci/probe.c:2938 [ 186.666875][ T1] acpi_pci_root_create+0x518/0x7a8 acpi_pci_root_create at drivers/acpi/pci_root.c:938 [ 186.671982][ T1] pci_acpi_scan_root+0x2bc/0x4c0 pci_acpi_scan_root at arch/arm64/kernel/pci.c:189 [ 186.676911][ T1] acpi_pci_root_add+0x45c/0x920 acpi_pci_root_add at drivers/acpi/pci_root.c:609 [ 186.681754][ T1] acpi_bus_attach+0x270/0x6d8 acpi_scan_attach_handler at drivers/acpi/scan.c:1969 (inlined by) acpi_bus_attach at drivers/acpi/scan.c:2013 [ 186.686419][ T1] acpi_bus_attach+0x140/0x6d8 [ 186.691085][ T1] acpi_bus_attach+0x140/0x6d8 acpi_bus_attach at drivers/acpi/scan.c:2033 (discriminator 9) [ 186.695751][ T1] acpi_bus_scan+0x98/0xf8 acpi_bus_scan at drivers/acpi/scan.c:2087 [ 186.700067][ T1] acpi_scan_init+0x220/0x524 [ 186.704644][ T1] acpi_init+0x460/0x4e8 [ 186.708790][ T1] do_one_initcall+0x170/0xb70 [ 186.713461][ T1] kernel_init_freeable+0x6a8/0x734 [ 186.718574][ T1] kernel_init+0x18/0x12c [ 186.722806][ T1] ret_from_fork+0x10/0x1c [ 186.727123][ T1] Code: 710006df 540002e0 71000adf 540004e0 (b9400013) [ 186.733988][ T1] ---[ end trace bad65ebbc8c09fe0 ]--- [ 186.739359][ T1] Kernel panic - not syncing: Oops: Fatal exception [ 186.746115][ T1] SMP: stopping secondary CPUs [ 186.750968][ T1] ---[ end Kernel panic - not syncing: Oops: Fatal exception ]---
--- Changed in v6: Converted single patch into a series. Dropped changes related to PPC 4xx platform. Refactored pci_ecam_map_bus() so that bus, device function and offset are correctly masked, limiting offset to 4K as per the PCI Express Specification. After the refactor this function will now use sensible defaults allowing for removal of the ".bus_shit" initialiser from all the users of the "ecam_ops" structure who do not use a non-standard ECAM bus shit values. Changed in v5: Removed unused constant "PCIE_ECAM_DEV_SHIFT". Refactored ppc4xx_pciex_get_config_base() so that the "offset" parameter can be passed to so that the PCIE_ECAM_OFFSET() macro can be used. Used the ALIGN_DOWN() macro where 32 bit alignment is required instead using the 0xffc mask. Added CFG_ADDR_CFG_TYPE_1 macro to pci/controller/pcie-iproc.c to denote that this is a configuration type 1 address and access type. Refactored boundary check in pci/controller/vmd.c as used by the vmd_cfg_addr() function following addition of the PCIE_ECAM_OFFSET() macro. Changed the "bus->number" to simply pass the "bus" argument in the PCIE_ECAM_OFFSET() macro. Changed in v4: Removed constants related to "CAM". Added more platforms and devices that can use new ECAM macros and constants. Removed unused ".bus_shift" initialisers from pci-xgene.c as xgene_pcie_map_bus() did not use these. Changes in v3: Updated commit message wording. Updated regarding custom ECAM bus shift values and concerning PCI base configuration space access for Type 1 access. Refactored rockchip_pcie_rd_other_conf() and rockchip_pcie_wr_other_conf() and removed the "busdev" variable. Removed surplus "relbus" variable from nwl_pcie_map_bus() and xilinx_pcie_map_bus(). Renamed the PCIE_ECAM_ADDR() macro to PCIE_ECAM_OFFSET(). Changes in v2: Use PCIE_ECAM_ADDR macro when computing ECAM address offset, but drop PCI_SLOT and PCI_FUNC macros from the PCIE_ECAM_ADDR macro in favour of using a single value for the device/function. Krzysztof Wilczyński (5): PCI: Unify ECAM constants in native PCI Express drivers PCI: thunder-pem: Add constant for custom ".bus_shit" initialiser PCI: iproc: Convert to use the new ECAM constants PCI: vmd: Update type of the __iomem pointers PCI: xgene: Removed unused ".bus_shift" initialisers from pci-xgene.c drivers/pci/controller/dwc/pcie-al.c | 12 ++------ drivers/pci/controller/dwc/pcie-hisi.c | 2 -- drivers/pci/controller/pci-aardvark.c | 13 ++------- drivers/pci/controller/pci-host-generic.c | 1 - drivers/pci/controller/pci-thunder-ecam.c | 1 - drivers/pci/controller/pci-thunder-pem.c | 13 +++++++-- drivers/pci/controller/pci-xgene.c | 2 -- drivers/pci/controller/pcie-brcmstb.c | 16 ++--------- drivers/pci/controller/pcie-iproc.c | 31 ++++++--------------- drivers/pci/controller/pcie-rockchip-host.c | 27 +++++++++--------- drivers/pci/controller/pcie-rockchip.h | 8 +----- drivers/pci/controller/pcie-tango.c | 1 - drivers/pci/controller/pcie-xilinx-nwl.c | 9 ++---- drivers/pci/controller/pcie-xilinx.c | 11 ++------ drivers/pci/controller/vmd.c | 19 ++++++------- drivers/pci/ecam.c | 23 ++++++++++----- include/linux/pci-ecam.h | 27 ++++++++++++++++++ 17 files changed, 96 insertions(+), 120 deletions(-)