bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties
PATCH 1/1 bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties
If the property was not specified then then the returned resource had a resource_size(..) == 1, rather than 0. The PCI-E driver checks for 0 so it blindly continues on with a corrupted resource.
--- drivers/bus/mvebu-mbus.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/bus/mvebu-mbus.c +2 -0
@@ -866,6 +866,8 @@ static void __init mvebu_mbus_get_pcie_resources(struct device_node *np,*/ memset (mem,0 ,sizeof (struct resource ));memset (io,0 ,sizeof (struct resource )); + mem->end--; + io->end--; ret =of_property_read_u32_array (np,"pcie-mem-aperture" , reg,ARRAY_SIZE (reg));if (!ret) {
Signature · 1 line
1.8.1.2
PATCH 1/1 bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties
Dear Jason Gunthorpe,
Quoted 14 lines · Jason Gunthorpe
If the property was not specified then then the returned resource had a
resource_size(..)== 1, rather than 0. The PCI-E driver checks for 0 so it blindly continues on with a corrupted resource.Signed-off-by Jason Gunthorpe <[email protected]>--- drivers/bus/mvebu-mbus.c | 2 ++ 1 file changed, 2 insertions(+)drivers/bus/mvebu-mbus.c +0
-0
drivers/bus/mvebu-mbus.c static void __init mvebu_mbus_get_pcie_resources(struct device_node *np, · line 873
Quoted 8 lines
drivers/bus/mvebu-mbus.c +2
-0@@ -866,6 +866,8 @@ static void __init mvebu_mbus_get_pcie_resources(struct device_node *np,*/ memset (mem,0 ,sizeof (struct resource ));memset (io,0 ,sizeof (struct resource )); + mem->end--; + io->end--; ret =of_property_read_u32_array (np,"pcie-mem-aperture" , reg,ARRAY_SIZE (reg));if (!ret) {
This seems a little bit nasty, isn't it? Can't we instead teach the PCIe driver to be a little bit smarter when testing if those resources are valid or not?
Thanks!
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/1 bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties
Quoted 21 lines · Thomas Petazzoni
Dear Jason Gunthorpe,
Quoted 18 lines · Jason Gunthorpe
If the property was not specified then then the returned resource had a
resource_size(..)== 1, rather than 0. The PCI-E driver checks for 0 so it blindly continues on with a corrupted resource.Signed-off-by Jason Gunthorpe <[email protected]>drivers/bus/mvebu-mbus.c | 2 ++ 1 file changed, 2 insertions(+)drivers/bus/mvebu-mbus.c +3
-0+++ b/drivers/bus/mvebu-mbus.c@@ -866,6 +866,8 @@ static void __init mvebu_mbus_get_pcie_resources(struct device_node *np,*/ memset (mem,0 ,sizeof (struct resource ));memset (io,0 ,sizeof (struct resource )); + mem->end--; + io->end--;
Quoted 4 lines
drivers/bus/mvebu-mbus.c static void __init mvebu_mbus_get_pcie_resources(struct device_node *np, · line 873
Quoted 2 lines
drivers/bus/mvebu-mbus.c +0
-0ret =of_property_read_u32_array (np,"pcie-mem-aperture" , reg,ARRAY_SIZE (reg));if (!ret) {This seems a little bit nasty, isn't it?
Well, I'm not 100% sure, but AFAICT that is how struct resources should be used. The start/end address are inclusive, so the resource (start=0,end=0) describes 1 byte at address 0.
The only way to get a 0 sized resource is (start=0,end=-1)
Before I did this I checked other user of resource_size and found tests to zero, so the above seems to be correct..
Eg a quick grep shows:
arch/powerpc/kernel/pci-common.c: pcibios_fixup_bridge
if (pci_has_flag(PCI_REASSIGN_ALL_RSRC)) {
res->flags |= IORESOURCE_UNSET;
res->start = 0;
res->end = -1;
continue;
}Do you see something different?
Jason
PATCH 1/1 bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties
Dear Jason Gunthorpe,
Quoted 5 lines · Jason Gunthorpe
Well, I'm not 100% sure, but AFAICT that is how struct resources should be used. The start/end address are inclusive, so the resource (start=0,end=0) describes 1 byte at address 0.
The only way to get a 0 sized resource is (start=0,end=-1)
So in the ideal world, we would have a resource_init(&res) that would set start=0 and end=-1 or something like that.
Quoted 6 lines
Before I did this I checked other user of
resource_sizeand found tests to zero, so the above seems to be correct..Eg a quick grep shows:
arch/powerpc/kernel/pci-common.c:pcibios_fixup_bridge
Quoted 8 lines
if (pci_has_flag(PCI_REASSIGN_ALL_RSRC)) { res->flags |= IORESOURCE_UNSET; res->start = 0; res->end = -1; continue; }Do you see something different?
Well, apparently what you did is the commonly adopted way. Maybe just add a comment in the code above those two lines to explain what may seem a little strange initially?
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com