On Thu, Jul 30, 2015 at 12:34 PM, Gabriele Paoloni <gabriele.paoloni@xxxxxxxxxx> wrote: >> -----Original Message----- >> From: Bjorn Helgaas [mailto:bhelgaas@xxxxxxxxxx] >> Sent: 30 July 2015 18:15 >> On Thu, Jul 30, 2015 at 04:50:55PM +0000, Gabriele Paoloni wrote: >> > > -----Original Message----- >> > > From: linux-pci-owner@xxxxxxxxxxxxxxx [mailto:linux-pci- >> > > owner@xxxxxxxxxxxxxxx] On Behalf Of Bjorn Helgaas >> > > Sent: Thursday, July 30, 2015 5:15 PM >> > > On Thu, Jul 30, 2015 at 01:52:13PM +0000, Gabriele Paoloni wrote: [...] >> > > > I don’t think we should rely on [CPU] addresses...what if the >> > > intermediate >> > > > translation layer changes the lower significant bits of the "bus >> > > address" >> > > > to translate into a cpu address? >> > > >> > > Is it really a possiblity that the lower bits could be changed? >> > >> > I've checked all the current deignware users DTs except "pci- >> layerscape" >> > that I could not find: >> > spear1310.dtsi >> > spear1340.dtsi >> > dra7.dtsi >> > imx6qdl.dtsi >> > imx6sx.dtsi >> > keystone.dtsi >> > exynos5440.dtsi >> > >> > None of them modifies the lower bits. To be more precise the only guy >> > that provides another translation layer is "dra7.dtsi": >> > axi0 >> > http://lxr.free-electrons.com/source/arch/arm/boot/dts/dra7.dtsi#L207 >> > >> > axi1 >> > http://lxr.free-electrons.com/source/arch/arm/boot/dts/dra7.dtsi#L241 >> > >> > For this case masking the top 4bits (bits28 to 31) should make the job. IMO, we should just fix this case. After further study, I don't think this is a DW issue, but rather an SOC integration issue. I believe you can just fixup the address in the pp->ops->host_init hook. In looking at this, I'm curious why only 2 ATU viewports are used by default as this causes switching on config accesses. Probably some configuration only has 2 viewports. This would not even work on SMP systems! Memory and i/o accesses do not have any lock. A config access on one core will temporarily disable the i/o or memory viewport which will cause an abort, dropped, or garbage data on an i/o or memory access from another core. You would have to be accessing cfg space on a bus other than the root bus, so maybe no one is doing that. >> > >> > Speaking in general terms so far I've always seen linear mappings >> > that differ by bitmask offset, however linear does not mean that you >> > cannot affect the lower bits: e.g. <0x0> to <0x0 + size> can map to >> > <0x0000cafe to 0x0000cafe + size>, but I guess that for HW design >> reasons >> > it is much easier to remap directly using a bitmask...but I was not >> sure >> > and I didn't think about the problems that can arise with ACPI. For higher speed buses, the h/w designs are not going to be doing complicated translations in order to meet timing requirements. At the top level only the highest order bits are going to be looked at. For downstream segments, the high order bits may get dropped to simplify routing. If an IP block has full address bits in this case, they could either be tied to 0 or tied off to the correct address. Either is reasonable, so I'm sure there is h/w doing both. That doesn't mean h/w designers haven't done something crazy, too. >> As I said, it wouldn't make sense for the bits that comprise the >> offset into the window to change, so the mapping only has to be linear >> inside the window. >> >> For the bits outside the window offset, I don't know enough to say >> whether masking is sufficient or safe. >> >> > If you think the bitmask is Ok then I can directly define it in >> > designware and we can drop this patch. >> >> It's OK by me, but I know nothing about the actual hardware involved. >> For the DesignWare question, I think you would just have to convince >> Jingoo and Pratyush (cc'd). > > Yes actually looking at > http://lxr.free-electrons.com/source/arch/arm/boot/dts/spear1310.dtsi#L99 > I can see that pci_addr=0 is mapped to bus_addr 0x80020000, so clearing > the top 4 bits would alter the behaviour of the current designware > SW framework...now I don't know if DW needs only the low 28b of that > value or not.... Given that most cases have same cpu and bus address, masking is not the right solution in general. There is also a bug here BTW. pcie0 and pcie2 have the same CPU address for the memory range. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
