* Re: [PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers
[not found] <20201129230743.3006978-1-kw@linux.com>
@ 2020-12-08 16:07 ` Qian Cai
0 siblings, 0 replies; only message in thread
From: Qian Cai @ 2020-12-08 16:07 UTC (permalink / raw)
To: Krzysztof Wilczyński, Bjorn Helgaas
Cc: Heiko Stuebner, Benjamin Herrenschmidt, Shawn Lin, Paul Mackerras,
Thomas Petazzoni, Jonathan Chocron, Toan Le, Will Deacon,
Rob Herring, Lorenzo Pieralisi, Michael Ellerman, Michal Simek,
linux-rockchip, bcm-kernel-feedback-list, Jonathan Derrick,
linux-pci, Ray Jui, Florian Fainelli, linux-rpi-kernel,
Jonathan Cameron, linux-arm-kernel, Scott Branden, Zhou Wang,
Robert Richter, linuxppc-dev, Nicolas Saenz Julienne,
Stephen Rothwell, Linux Next Mailing List
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 <kw@linux.com>
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(-)
>
^ permalink raw reply [flat|nested] only message in thread