PCI: mvebu: Take control of mbus windows setup by the firmware
Subsystems (2)
PATCH 1/1 PCI: mvebu: Take control of mbus windows setup by the firmware
The firmware may setup the mbus to access PCI-E and indicate this has happened with a ranges mapping for the PCI-E ID. If this happens then the mbus setup and the pci dynamic setup conflict, creating problems.
Have PCI-E assume control of the firmware specified default mapping by setting the value of the bridge window to match the firmware mapping.
--- drivers/bus/mvebu-mbus.c | 36 ++++++++++++++++++++++++++++++++++++ drivers/pci/host/pci-mvebu.c | 22 ++++++++++++++++++++++ include/linux/mbus.h | 3 +++ 3 files changed, 61 insertions(+)
In the DT this could look like:
mbus {
ranges = <MBUS_ID(0x04, 0xe8) 0 0xe0000000 0x8000000 /* PEX 0 MEM */
pex@e0000000 {
compatible = "marvell,kirkwood-pcie";
ranges = <0x82000000 0 0x40000 MBUS_ID(0xf0, 0x01) 0x40000 0 0x00002000 /* Controller regs */
0x82000000 1 0 MBUS_ID(0x04, 0xe8) 0 1 0 /* Port 0.0 MEM */
>;
pcie at 1,0 {
ranges = <0x82000000 0 0 0x82000000 0x1 0 1 0>;
reg = <0x0800 0 0 0 0>; // 0000:00:01.0
device at 0 {
reg = <0x0 0 0 0 0>; // 0000:01:00.0
ranges = <0x00000000 0x82000000 0x00000000 0x00000000 0x8000000>;Which is basically the OF way to describe PCI devices downstream of the interface and give information about what their BARs should be.
This is useful if there is even more DT stuff below the explicitly declared device as it allows standard DT address translation to work properly.
drivers/bus/mvebu-mbus.c +36 -0
@@ -922,6 +922,42 @@ int mvebu_mbus_add_window_by_id(unsigned int target, unsigned int attribute, size, MVEBU_MBUS_NO_REMAP); } +/** +* mvebu_mbus_get_single_window_by_id() - return the location of the window if +* the target/attribute has a single enabled mapping. +* +* RETURNS: +* -ENODEV if no mapping and -E2BIG if there is more than one mapping +*/ +int mvebu_mbus_get_single_window_by_id (unsigned int target, +unsigned int attribute, +phys_addr_t *base,phys_addr_t *size) +{ +unsigned int win; +unsigned int count =0 ; + +for (win =0 ; win < mbus_state.soc->num_wins; win++) { +u64 wbase; +u32 wsize; +u8 wtarget, wattr; +int enabled; + +mvebu_mbus_read_window (&mbus_state, win, &enabled, &wbase, + &wsize, &wtarget, &wattr,NULL ); +if (enabled && wtarget == target && wattr == attribute) { + *base = wbase; + *size = wsize; + count++; + } + } + +if (count ==1 ) +return 0 ; +if (count ==0 ) +return -ENODEV ; +return -E2BIG ; +} +int mvebu_mbus_del_window (phys_addr_t base,size_t size) {int win;
drivers/pci/host/pci-mvebu.c +22 -0
@@ -464,6 +464,8 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)static void mvebu_sw_pci_bridge_init (struct mvebu_pcie_port *port) {struct mvebu_sw_pci_bridge *bridge = &port->bridge; +int rc; +phys_addr_t base, size;memset (bridge,0 ,sizeof (struct mvebu_sw_pci_bridge ));
@@ -480,6 +482,26 @@ static void mvebu_sw_pci_bridge_init(struct mvebu_pcie_port *port)/* Add capabilities */ bridge->status =PCI_STATUS_CAP_LIST ; + +/* +* If the firmware has already setup a window for PCIe then assume +* control of it by defaulting the BAR to the window setting. +*/ + rc =mvebu_mbus_get_single_window_by_id (port->mem_target, + port->mem_attr, &base, &size); +if (rc == -E2BIG ) +pr_err (FW_BUG "%s: Too many pre-existing mbus mappings\n" , + port->dn->name); +if (!rc) { +if ((base &0xFFFFF ) !=0 || ((size + base) &0xFFFFF ) !=0 ) +pr_err (FW_BUG "%s: Invalid pre-existing mbus mapping\n" , + port->dn->name); + port->memwin_base = base; + port->memwin_size = size; + port->bridge.membase = (base >>16 ) &0xFFF0 ; + port->bridge.memlimit = ((size -1 + base) >>16 ) &0xFFF0 ; + port->bridge.command |=PCI_COMMAND_MEMORY ; + } }/*
include/linux/mbus.h +3 -0
@@ -83,5 +83,8 @@ int mvebu_mbus_init(const char *soc, phys_addr_t mbus_phys_base, size_t mbus_size, phys_addr_t sdram_phys_base, size_t sdram_size);int mvebu_mbus_dt_init (bool is_coherent); +int mvebu_mbus_get_single_window_by_id (unsigned int target, +unsigned int attribute, +phys_addr_t *base,phys_addr_t *size);#endif /* __LINUX_MBUS_H */
Signature · 1 line
2.1.4
Hey Thomas,
Could you take a look at this? Thanks
Jason
Quoted 135 lines · Jason Gunthorpe
The firmware may setup the mbus to access PCI-E and indicate this has happened with a ranges mapping for the PCI-E ID. If this happens then the mbus setup and the pci dynamic setup conflict, creating problems.
Have PCI-E assume control of the firmware specified default mapping by setting the value of the bridge window to match the firmware mapping.
Signed-off-by Jason Gunthorpe <[email protected]>drivers/bus/mvebu-mbus.c | 36 ++++++++++++++++++++++++++++++++++++ drivers/pci/host/pci-mvebu.c | 22 ++++++++++++++++++++++ include/linux/mbus.h | 3 +++ 3 files changed, 61 insertions(+)In the DT this could look like:
mbus { ranges = <MBUS_ID(0x04, 0xe8) 0 0xe0000000 0x8000000 /* PEX 0 MEM */ pex at e0000000 { compatible = "marvell,kirkwood-pcie"; ranges = <0x82000000 0 0x40000 MBUS_ID(0xf0, 0x01) 0x40000 0 0x00002000 /* Controller regs */ 0x82000000 1 0 MBUS_ID(0x04, 0xe8) 0 1 0 /* Port 0.0 MEM */ >; pcie at 1,0 { ranges = <0x82000000 0 0 0x82000000 0x1 0 1 0>; reg = <0x0800 0 0 0 0>; // 0000:00:01.0 device at 0 { reg = <0x0 0 0 0 0>; // 0000:01:00.0 ranges = <0x00000000 0x82000000 0x00000000 0x00000000 0x8000000>;Which is basically the OF way to describe PCI devices downstream of the interface and give information about what their BARs should be.
This is useful if there is even more DT stuff below the explicitly declared device as it allows standard DT address translation to work properly.
drivers/bus/mvebu-mbus.c +37
-0+++ b/drivers/bus/mvebu-mbus.c@@ -922,6 +922,42 @@ int mvebu_mbus_add_window_by_id(unsigned int target, unsigned int attribute, size, MVEBU_MBUS_NO_REMAP); } +/** +* mvebu_mbus_get_single_window_by_id() - return the location of the window if +* the target/attribute has a single enabled mapping. +* +* RETURNS: +* -ENODEV if no mapping and -E2BIG if there is more than one mapping +*/ +int mvebu_mbus_get_single_window_by_id (unsigned int target, +unsigned int attribute, +phys_addr_t *base,phys_addr_t *size) +{ +unsigned int win; +unsigned int count =0 ; + +for (win =0 ; win < mbus_state.soc->num_wins; win++) { +u64 wbase; +u32 wsize; +u8 wtarget, wattr; +int enabled; + +mvebu_mbus_read_window (&mbus_state, win, &enabled, &wbase, + &wsize, &wtarget, &wattr,NULL ); +if (enabled && wtarget == target && wattr == attribute) { + *base = wbase; + *size = wsize; + count++; + } + } + +if (count ==1 ) +return 0 ; +if (count ==0 ) +return -ENODEV ; +return -E2BIG ; +} +int mvebu_mbus_del_window (phys_addr_t base,size_t size) {int win;drivers/pci/host/pci-mvebu.c +23
-0+++ b/drivers/pci/host/pci-mvebu.c@@ -464,6 +464,8 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)static void mvebu_sw_pci_bridge_init (struct mvebu_pcie_port *port) {struct mvebu_sw_pci_bridge *bridge = &port->bridge; +int rc; +phys_addr_t base, size;memset (bridge,0 ,sizeof (struct mvebu_sw_pci_bridge ));@@ -480,6 +482,26 @@ static void mvebu_sw_pci_bridge_init(struct mvebu_pcie_port *port)/* Add capabilities */ bridge->status =PCI_STATUS_CAP_LIST ; + +/* +* If the firmware has already setup a window for PCIe then assume +* control of it by defaulting the BAR to the window setting. +*/ + rc =mvebu_mbus_get_single_window_by_id (port->mem_target, + port->mem_attr, &base, &size); +if (rc == -E2BIG ) +pr_err (FW_BUG "%s: Too many pre-existing mbus mappings\n" , + port->dn->name); +if (!rc) { +if ((base &0xFFFFF ) !=0 || ((size + base) &0xFFFFF ) !=0 ) +pr_err (FW_BUG "%s: Invalid pre-existing mbus mapping\n" , + port->dn->name); + port->memwin_base = base; + port->memwin_size = size; + port->bridge.membase = (base >>16 ) &0xFFF0 ; + port->bridge.memlimit = ((size -1 + base) >>16 ) &0xFFF0 ; + port->bridge.command |=PCI_COMMAND_MEMORY ; + } }/* include/linux/mbus.h +4
-0+++ b/include/linux/mbus.h@@ -83,5 +83,8 @@ int mvebu_mbus_init(const char *soc, phys_addr_t mbus_phys_base, size_t mbus_size, phys_addr_t sdram_phys_base, size_t sdram_size);int mvebu_mbus_dt_init (bool is_coherent); +int mvebu_mbus_get_single_window_by_id (unsigned int target, +unsigned int attribute, +phys_addr_t *base,phys_addr_t *size);#endif /* __LINUX_MBUS_H */
Hello,
Quoted 9 lines · Jason Gunthorpe
The firmware may setup the mbus to access PCI-E and indicate this has happened with a ranges mapping for the PCI-E ID. If this happens then the mbus setup and the pci dynamic setup conflict, creating problems.
Have PCI-E assume control of the firmware specified default mapping by setting the value of the bridge window to match the firmware mapping.
Signed-off-by Jason Gunthorpe <[email protected]>
Sorry for the late feedback. I am not sure to fully understand what you are trying to do here.
However, one thing that confuses me specifically is how can the kernel get any MBus mapping set up by the firmware? Indeed, when the mvebu-mbus driver initializes, it destroys all existing MBus windows that might have been left by the firmware/bootloader:
static int __init mvebu_mbus_common_init(struct mvebu_mbus_state *mbus,
phys_addr_t mbuswins_phys_base, size_t mbuswins_size, phys_addr_t sdramwins_phys_base, size_t sdramwins_size, phys_addr_t mbusbridge_phys_base, size_t mbusbridge_size, bool is_coherent)
[...]
for (win = 0; win < mbus->soc->num_wins; win++)
mvebu_mbus_disable_window(mbus, win);Why does Linux needs to rely on what the firmware has setup in terms of MBus windows? Why can't Linux just find out the right BAR base/size like it is doing for all other devices?
Thanks,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Quoted 13 lines · Thomas Petazzoni
Hello,
Quoted 9 lines · Jason Gunthorpe
The firmware may setup the mbus to access PCI-E and indicate this has happened with a ranges mapping for the PCI-E ID. If this happens then the mbus setup and the pci dynamic setup conflict, creating problems.
Have PCI-E assume control of the firmware specified default mapping by setting the value of the bridge window to match the firmware mapping.
Signed-off-by Jason Gunthorpe <[email protected]>
Quoted 7 lines
Sorry for the late feedback. I am not sure to fully understand what you are trying to do here.
However, one thing that confuses me specifically is how can the kernel get any MBus mapping set up by the firmware? Indeed, when the mvebu-mbus driver initializes, it destroys all existing MBus windows that might have been left by the
firmware/bootloader:
Sort of, yes, it wipes out the hardware, but then it parses the DT and puts back the ranges via mbus_dt_setup.
The issue is what happens if mbus_dt_setup adds a mapping for PCI aperature from the firmware's DT - in this case the PCI driver does not know that the mbus driver created the mapping and becomes unable to manipulate the mbus windows due to the conflict detection logic.
The solution is to have the PCI driver read the current state of the mbus window - out of the mbus registers that were programmed from the DT ranges by mbus_dt_setup. Then it knows to delete the DT described window before setting a new window and the conflicts are avoided.
Quoted 3 lines
Why does Linux needs to rely on what the firmware has setup in terms of MBus windows? Why can't Linux just find out the right BAR base/size like it is doing for all other devices?
Unfortunately that is sort of how DT is expected to work, in my case I have DT sub nodes off the PCI device in DT and I need DT address translation to work for those nodes.
In DT land the PCI stuff expects the firmware to provide a complete address map, and my firmware does, but in doing so I hit this bug :)
Jason