PCI: pci-bridge-emul: Various fixes
Subsystems (4)
Pali Rohár
maintainercover PCI: pci-bridge-emul: Various fixes
This patch series contains various fixes for pci-bridge-emul code. This code is used only by pci-aardvark.c and pci-mvebu.c drivers.
Pali Rohár (6):
PCI: pci-bridge-emul: Make expansion ROM Base Address register
read-only
PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config
space
PCI: pci-bridge-emul: Add definitions for missing capabilities
registers
PCI: pci-bridge-emul: Fix definitions of reserved bits
PCI: pci-bridge-emul: Correctly set PCIe capabilities
PCI: pci-bridge-emul: Set PCI_STATUS_CAP_LIST for PCIe device
drivers/pci/controller/pci-aardvark.c | 4 +- drivers/pci/controller/pci-mvebu.c | 8 ++ drivers/pci/pci-bridge-emul.c | 113 ++++++++++++++++++++++---- 3 files changed, 108 insertions(+), 17 deletions(-)
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH 1/6 PCI: pci-bridge-emul: Make expansion ROM Base Address register read-only
If expansion ROM is unsupported (which is the case of pci-bridge-emul.c driver) then ROM Base Address register must be implemented as read-only register that return 0 when read, same as for unused Base Address registers.
--- drivers/pci/pci-bridge-emul.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/pci/pci-bridge-emul.c +6 -1
@@ -139,8 +139,13 @@ struct pci_bridge_reg_behavior pci_regs_behavior[PCI_STD_HEADER_SIZEOF / 4] = { .ro = GENMASK(7, 0), }, +/* +* If expansion ROM is unsupported then ROM Base Address register must +* be implemented as read-only register that return 0 when read, same +* as for unused Base Address registers. +*/ [PCI_ROM_ADDRESS1 / 4] = { - .rw = GENMASK(31, 11) | BIT(0), + .ro = ~0, },/*
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH 2/6 PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config space
In reply to Pali RohárSome bits in PCI config space are reserved when device is PCIe. Properly define behavior of PCI registers for PCIe emulated bridge and ensure that it would not be possible change these reserved bits.
--- drivers/pci/pci-bridge-emul.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
drivers/pci/pci-bridge-emul.c +21 -0
@@ -295,6 +295,27 @@ int pci_bridge_emul_init(struct pci_bridge_emul *bridge,kfree (bridge->pci_regs_behavior);return -ENOMEM ; } +/* These bits are applicable only for PCI and reserved on PCIe */ + bridge->pci_regs_behavior[PCI_CACHE_LINE_SIZE /4 ].ro &= + ~GENMASK (15 ,8 ); + bridge->pci_regs_behavior[PCI_COMMAND /4 ].ro &= + ~((PCI_COMMAND_SPECIAL |PCI_COMMAND_INVALIDATE | +PCI_COMMAND_VGA_PALETTE |PCI_COMMAND_WAIT | +PCI_COMMAND_FAST_BACK ) | + (PCI_STATUS_66MHZ |PCI_STATUS_FAST_BACK | +PCI_STATUS_DEVSEL_MASK ) <<16 ); + bridge->pci_regs_behavior[PCI_PRIMARY_BUS /4 ].ro &= + ~GENMASK (31 ,24 ); + bridge->pci_regs_behavior[PCI_IO_BASE /4 ].ro &= + ~((PCI_STATUS_66MHZ |PCI_STATUS_FAST_BACK | +PCI_STATUS_DEVSEL_MASK ) <<16 ); + bridge->pci_regs_behavior[PCI_INTERRUPT_LINE /4 ].rw &= + ~((PCI_BRIDGE_CTL_MASTER_ABORT | +BIT (8 ) |BIT (9 ) |BIT (11 )) <<16 ); + bridge->pci_regs_behavior[PCI_INTERRUPT_LINE /4 ].ro &= + ~((PCI_BRIDGE_CTL_FAST_BACK ) <<16 ); + bridge->pci_regs_behavior[PCI_INTERRUPT_LINE /4 ].w1c &= + ~(BIT (10 ) <<16 ); }if (flags &PCI_BRIDGE_EMUL_NO_PREFETCHABLE_BAR ) {
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH 3/6 PCI: pci-bridge-emul: Add definitions for missing capabilities registers
In reply to Pali Rohárpci-bridge-emul driver already allocates buffer for capabilities up to the PCI_EXP_SLTSTA2 register, but does not define bit access behavior for these registers. Fix it by adding missing definitions.
--- drivers/pci/pci-bridge-emul.c | 43 +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
drivers/pci/pci-bridge-emul.c +43 -0
@@ -256,6 +256,49 @@ struct pci_bridge_reg_behavior pcie_cap_regs_behavior[PCI_CAP_PCIE_SIZEOF / 4] = .ro = GENMASK(15, 0) | PCI_EXP_RTSTA_PENDING, .w1c = PCI_EXP_RTSTA_PME, }, + + [PCI_EXP_DEVCAP2 / 4] = { + /* + * Device capabilities 2 register has reserved bits [30:27]. + * Also bits [26:24] are reserved for non-upstream ports. + */ + .ro = BIT(31) | GENMASK(23, 0), + }, + + [PCI_EXP_DEVCTL2 / 4] = { + /* + * Device control 2 register is RW. Bit 11 is reserved for + * non-upstream ports. + * + * Device status 2 register is reserved. + */ + .rw = GENMASK(15, 12) | GENMASK(10, 0), + }, + + [PCI_EXP_LNKCAP2 / 4] = { + /* Link capabilities 2 register has reserved bits [30:25] and 0. */ + .ro = BIT(31) | GENMASK(24, 1), + }, + + [PCI_EXP_LNKCTL2 / 4] = { + /* + * Link control 2 register is RW. + * + * Link status 2 register has bits 5, 15 W1C; + * bits 10, 11 reserved and others are RO. + */ + .rw = GENMASK(15, 0), + .w1c = (BIT(15) | BIT(5)) << 16, + .ro = (GENMASK(14, 12) | GENMASK(9, 6) | GENMASK(4, 0)) << 16, + }, + + [PCI_EXP_SLTCAP2 / 4] = { + /* Slot capabilities 2 register is reserved. */ + }, + + [PCI_EXP_SLTCTL2 / 4] = { + /* Both Slot control 2 and Slot status 2 registers are reserved. */ + }, }; /*
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH 4/6 PCI: pci-bridge-emul: Fix definitions of reserved bits
In reply to Pali RohárSome bits in PCI_EXP registers are reserved for non-root ports. Driver pci-bridge-emul.c implements PCIe Root Port device therefore it should not allow setting reserved bits of registers.
Properly define non-reserved bits for all PCI_EXP registers.
--- drivers/pci/pci-bridge-emul.c | 36 ++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-)
drivers/pci/pci-bridge-emul.c +25 -11
@@ -176,41 +176,55 @@ struct pci_bridge_reg_behavior pcie_cap_regs_behavior[PCI_CAP_PCIE_SIZEOF / 4] = [PCI_CAP_LIST_ID / 4] = {/* * Capability ID, Next Capability Pointer and -* Capabilities register are all read-only. +* bits [14:0] of Capabilities register are all read-only. +* Bit 15 of Capabilities register is reserved. */ - .ro = ~0, + .ro = GENMASK(30, 0), }, [PCI_EXP_DEVCAP / 4] = { - .ro = ~0, +/* +* Bits [31:29] and [17:16] are reserved. +* Bits [27:18] are reserved for non-upstream ports. +* Bits 28 and [14:6] are reserved for non-endpoint devices. +* Other bits are read-only. +*/ + .ro = BIT(15) | GENMASK(5, 0), }, [PCI_EXP_DEVCTL / 4] = { -/* Device control register is RW */ - .rw = GENMASK(15, 0), +/* +* Device control register is RW, except bit 15 which is +* reserved for non-endpoints or non-PCIe-to-PCI/X bridges. +*/ + .rw = GENMASK(14, 0),/* * Device status register has bits 6 and [3:0] W1C, [5:4] RO, -* the rest is reserved +* the rest is reserved. Also bit 6 is reserved for non-upstream +* ports. */ - .w1c = (BIT(6) | GENMASK(3, 0)) << 16, + .w1c = GENMASK(3, 0) << 16, .ro = GENMASK(5, 4) << 16, }, [PCI_EXP_LNKCAP / 4] = { -/* All bits are RO, except bit 23 which is reserved */ - .ro = lower_32_bits(~BIT(23)), +/* +* All bits are RO, except bit 23 which is reserved and +* bit 18 which is reserved for non-upstream ports. +*/ + .ro = lower_32_bits(~(BIT(23) | PCI_EXP_LNKCAP_CLKPM)), }, [PCI_EXP_LNKCTL / 4] = {/* * Link control has bits [15:14], [11:3] and [1:0] RW, the -* rest is reserved. +* rest is reserved. Bit 8 is reserved for non-upstream ports. * * Link status has bits [13:0] RO, and bits [15:14] * W1C. */ - .rw = GENMASK(15, 14) | GENMASK(11, 3) | GENMASK(1, 0), + .rw = GENMASK(15, 14) | GENMASK(11, 9) | GENMASK(7, 3) | GENMASK(1, 0), .ro = GENMASK(13, 0) << 16, .w1c = GENMASK(15, 14) << 16, },
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH 5/6 PCI: pci-bridge-emul: Correctly set PCIe capabilities
In reply to Pali RohárOlder mvebu hardware provides PCIe Capability structure only in version 1. New mvebu and aardvark hardware provides it in version 2. So do not force version to 2 in pci_bridge_emul_init() and rather allow drivers to set correct version. Drivers need to set version in pcie_conf.cap field without overwriting PCI_CAP_LIST_ID register. Both drivers (mvebu and aardvark) do not provide slot support yet, so do not set PCI_EXP_FLAGS_SLOT flag.
--- drivers/pci/controller/pci-aardvark.c | 4 +++- drivers/pci/controller/pci-mvebu.c | 8 ++++++++ drivers/pci/pci-bridge-emul.c | 5 +---- 3 files changed, 12 insertions(+), 5 deletions(-)
drivers/pci/controller/pci-aardvark.c +3 -1
@@ -881,7 +881,6 @@ advk_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge,return PCI_BRIDGE_EMUL_HANDLED ; } -case PCI_CAP_LIST_ID :case PCI_EXP_DEVCAP :case PCI_EXP_DEVCTL : *value =advk_readl (pcie,PCIE_CORE_PCIEXP_CAP + reg);
@@ -962,6 +961,9 @@ static int advk_sw_pci_bridge_init(struct advk_pcie *pcie)/* Support interrupt A for MSI feature */ bridge->conf.intpin =PCIE_CORE_INT_A_ASSERT_ENABLE ; +/* Aardvark HW provides PCIe Capability structure in version 2 */ + bridge->pcie_conf.cap =cpu_to_le16 (2 ); +/* Indicates supports for Completion Retry Status */ bridge->pcie_conf.rootcap =cpu_to_le16 (PCI_EXP_RTCAP_CRSVIS );
drivers/pci/controller/pci-mvebu.c +8 -0
@@ -584,6 +584,8 @@ static struct pci_bridge_emul_ops mvebu_pci_bridge_emul_ops = {static void mvebu_pci_bridge_emul_init (struct mvebu_pcie_port *port) {struct pci_bridge_emul *bridge = &port->bridge; +u32 pcie_cap =mvebu_readl (port,PCIE_CAP_PCIEXP ); +u8 pcie_cap_ver = ((pcie_cap >>16 ) &PCI_EXP_FLAGS_VERS ); bridge->conf.vendor =PCI_VENDOR_ID_MARVELL ; bridge->conf.device =mvebu_readl (port,PCIE_DEV_ID_OFF ) >>16 ;
@@ -596,6 +598,12 @@ static void mvebu_pci_bridge_emul_init(struct mvebu_pcie_port *port) bridge->conf.iolimit =PCI_IO_RANGE_TYPE_32 ; } +/* +* Older mvebu hardware provides PCIe Capability structure only in +* version 1. New hardware provides it in version 2. +*/ + bridge->pcie_conf.cap =cpu_to_le16 (pcie_cap_ver); + bridge->has_pcie = true; bridge->data = port; bridge->ops = &mvebu_pci_bridge_emul_ops;
drivers/pci/pci-bridge-emul.c +1 -4
@@ -340,10 +340,7 @@ int pci_bridge_emul_init(struct pci_bridge_emul *bridge,if (bridge->has_pcie) { bridge->conf.capabilities_pointer =PCI_CAP_PCIE_START ; bridge->pcie_conf.cap_id =PCI_CAP_ID_EXP ; -/* Set PCIe v2, root port, slot support */ - bridge->pcie_conf.cap = -cpu_to_le16 (PCI_EXP_TYPE_ROOT_PORT <<4 |2 | -PCI_EXP_FLAGS_SLOT ); + bridge->pcie_conf.cap |=cpu_to_le16 (PCI_EXP_TYPE_ROOT_PORT <<4 ); bridge->pcie_cap_regs_behavior = kmemdup(pcie_cap_regs_behavior, sizeof(pcie_cap_regs_behavior),
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH 6/6 PCI: pci-bridge-emul: Set PCI_STATUS_CAP_LIST for PCIe device
In reply to Pali RohárSince all PCI Express device Functions are required to implement the PCI Express Capability structure, Capabilities List bit in PCI Status Register must be hardwired to 1b. Capabilities Pointer register (which is already set by pci-bride-emul.c driver) is valid only when Capabilities List is set to 1b.
--- drivers/pci/pci-bridge-emul.c | 1 + 1 file changed, 1 insertion(+)
drivers/pci/pci-bridge-emul.c +1 -0
@@ -339,6 +339,7 @@ int pci_bridge_emul_init(struct pci_bridge_emul *bridge,if (bridge->has_pcie) { bridge->conf.capabilities_pointer =PCI_CAP_PCIE_START ; + bridge->conf.status |=cpu_to_le16 (PCI_STATUS_CAP_LIST ); bridge->pcie_conf.cap_id =PCI_CAP_ID_EXP ; bridge->pcie_conf.cap |=cpu_to_le16 (PCI_EXP_TYPE_ROOT_PORT <<4 ); bridge->pcie_cap_regs_behavior =
Signature · 1 line
2.20.1
Quoted 14 lines · On Wed, 24 Nov 2021 16:59:38 +0100 Pali Rohár <[email protected]> wrote:
This patch series contains various fixes for pci-bridge-emul code. This code is used only by
pci-aardvark.candpci-mvebu.cdrivers.Pali Rohár (6):
PCI: pci-bridge-emul: Make expansion ROM Base Address register
read-only
PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config
space
PCI: pci-bridge-emul: Add definitions for missing capabilities
registers
PCI: pci-bridge-emul: Fix definitions of reserved bits
PCI: pci-bridge-emul: Correctly set PCIe capabilities
PCI: pci-bridge-emul: SetPCI_STATUS_CAP_LISTfor PCIe device
Quoted 5 lines
drivers/pci/controller/pci-aardvark.c | 4 +- drivers/pci/controller/pci-mvebu.c | 8 ++ drivers/pci/pci-bridge-emul.c | 113 ++++++++++++++++++++++---- 3 files changed, 108 insertions(+), 17 deletions(-)
I will send v2 of this series with the third patch dropped, since it will be applied via aardvark batch 3 series.
Marek
Pali Rohár
maintainerQuoted 23 lines · On Tuesday 30 November 2021 14:26:52 Marek Behún wrote:
Quoted 14 lines · On Wed, 24 Nov 2021 16:59:38 +0100 Pali Rohár <[email protected]> wrote:
This patch series contains various fixes for pci-bridge-emul code. This code is used only by
pci-aardvark.candpci-mvebu.cdrivers.Pali Rohár (6):
PCI: pci-bridge-emul: Make expansion ROM Base Address register
read-only
PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config
space
PCI: pci-bridge-emul: Add definitions for missing capabilities
registers
PCI: pci-bridge-emul: Fix definitions of reserved bits
PCI: pci-bridge-emul: Correctly set PCIe capabilities
PCI: pci-bridge-emul: SetPCI_STATUS_CAP_LISTfor PCIe deviceQuoted 5 lines
drivers/pci/controller/pci-aardvark.c | 4 +- drivers/pci/controller/pci-mvebu.c | 8 ++ drivers/pci/pci-bridge-emul.c | 113 ++++++++++++++++++++++---- 3 files changed, 108 insertions(+), 17 deletions(-)
Quoted 4 lines
I will send v2 of this series with the third patch dropped, since it will be applied via aardvark batch 3 series.
Marek
I marked third patch at patchwork as "Superseded" (as now it was applied into pci/aardvark branch) and so there is no need to resend same patches again.
Lorenzo Pieralisi
AppliedQuoted 14 lines · Pali Rohár
This patch series contains various fixes for pci-bridge-emul code. This code is used only by
pci-aardvark.candpci-mvebu.cdrivers.Pali Rohár (6):
PCI: pci-bridge-emul: Make expansion ROM Base Address register
read-only
PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config
space
PCI: pci-bridge-emul: Add definitions for missing capabilities
registers
PCI: pci-bridge-emul: Fix definitions of reserved bits
PCI: pci-bridge-emul: Correctly set PCIe capabilities
PCI: pci-bridge-emul: SetPCI_STATUS_CAP_LISTfor PCIe device
Quoted 1 line
[...]
Applied to pci/bridge-emul, thanks!
[1/6] PCI: pci-bridge-emul: Make expansion ROM Base Address register read-only
https://git.kernel.org/lpieralisi/pci/c/1c1a3b4d3e [2/6] PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config space https://git.kernel.org/lpieralisi/pci/c/7b067ac63a [3/6] PCI: pci-bridge-emul: Add definitions for missing capabilities registers https://git.kernel.org/lpieralisi/pci/c/faa3e547f4 [4/6] PCI: pci-bridge-emul: Fix definitions of reserved bits https://git.kernel.org/lpieralisi/pci/c/a3ab28a2e3 [5/6] PCI: pci-bridge-emul: Correctly set PCIe capabilities https://git.kernel.org/lpieralisi/pci/c/0f7ba81700 [6/6] PCI: pci-bridge-emul: Set PCI_STATUS_CAP_LIST for PCIe device https://git.kernel.org/lpieralisi/pci/c/32051099e8
Thanks,
Lorenzo
Pali Rohár
maintainerQuoted 17 lines · On Monday 06 December 2021 12:19:57 Lorenzo Pieralisi wrote:
Quoted 14 lines · Pali Rohár
This patch series contains various fixes for pci-bridge-emul code. This code is used only by
pci-aardvark.candpci-mvebu.cdrivers.Pali Rohár (6):
PCI: pci-bridge-emul: Make expansion ROM Base Address register
read-only
PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config
space
PCI: pci-bridge-emul: Add definitions for missing capabilities
registers
PCI: pci-bridge-emul: Fix definitions of reserved bits
PCI: pci-bridge-emul: Correctly set PCIe capabilities
PCI: pci-bridge-emul: SetPCI_STATUS_CAP_LISTfor PCIe deviceQuoted 1 line
[...]
Quoted 8 lines
Applied to pci/bridge-emul, thanks!
[1/6] PCI: pci-bridge-emul: Make expansion ROM Base Address register read-only
https://git.kernel.org/lpieralisi/pci/c/1c1a3b4d3e [2/6] PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config space https://git.kernel.org/lpieralisi/pci/c/7b067ac63a [3/6] PCI: pci-bridge-emul: Add definitions for missing capabilities registers https://git.kernel.org/lpieralisi/pci/c/faa3e547f4
Hello Lorenzo! This patch "PCI: pci-bridge-emul: Add definitions for missing capabilities registers" is now in your two different branches:
pci/bridge-emul and pci/aardvark. Not sure if you want to have this same patch on two places... So please check.
Quoted 9 lines
[4/6] PCI: pci-bridge-emul: Fix definitions of reserved bits
https://git.kernel.org/lpieralisi/pci/c/a3ab28a2e3 [5/6] PCI: pci-bridge-emul: Correctly set PCIe capabilities https://git.kernel.org/lpieralisi/pci/c/0f7ba81700 [6/6] PCI: pci-bridge-emul: Set PCI_STATUS_CAP_LIST for PCIe device https://git.kernel.org/lpieralisi/pci/c/32051099e8Thanks,
Lorenzo
Quoted 27 lines · Pali Rohár
Quoted 17 lines · On Monday 06 December 2021 12:19:57 Lorenzo Pieralisi wrote:
Quoted 14 lines · Pali Rohár
This patch series contains various fixes for pci-bridge-emul code. This code is used only by
pci-aardvark.candpci-mvebu.cdrivers.Pali Rohár (6):
PCI: pci-bridge-emul: Make expansion ROM Base Address register
read-only
PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config
space
PCI: pci-bridge-emul: Add definitions for missing capabilities
registers
PCI: pci-bridge-emul: Fix definitions of reserved bits
PCI: pci-bridge-emul: Correctly set PCIe capabilities
PCI: pci-bridge-emul: SetPCI_STATUS_CAP_LISTfor PCIe deviceQuoted 1 line
[...]
Quoted 8 lines
Applied to pci/bridge-emul, thanks!
[1/6] PCI: pci-bridge-emul: Make expansion ROM Base Address register read-only
https://git.kernel.org/lpieralisi/pci/c/1c1a3b4d3e [2/6] PCI: pci-bridge-emul: Properly mark reserved PCIe bits in PCI config space https://git.kernel.org/lpieralisi/pci/c/7b067ac63a [3/6] PCI: pci-bridge-emul: Add definitions for missing capabilities registers https://git.kernel.org/lpieralisi/pci/c/faa3e547f4
Quoted 4 lines
Hello Lorenzo! This patch "PCI: pci-bridge-emul: Add definitions for missing capabilities registers" is now in your two different branches:
pci/bridge-emul and pci/aardvark. Not sure if you want to have this same patch on two places... So please check.
Dropped, thanks.
Lorenzo
Quoted 9 lines
Quoted 9 lines
[4/6] PCI: pci-bridge-emul: Fix definitions of reserved bits
https://git.kernel.org/lpieralisi/pci/c/a3ab28a2e3 [5/6] PCI: pci-bridge-emul: Correctly set PCIe capabilities https://git.kernel.org/lpieralisi/pci/c/0f7ba81700 [6/6] PCI: pci-bridge-emul: Set PCI_STATUS_CAP_LIST for PCIe device https://git.kernel.org/lpieralisi/pci/c/32051099e8Thanks,
Lorenzo