On Tue, Nov 11, 2014 at 8:57 AM, Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> wrote: > On Tue, Nov 11, 2014 at 02:20:31PM +0000, Bjorn Helgaas wrote: >> On Tue, Nov 11, 2014 at 4:48 AM, Lorenzo Pieralisi >> <lorenzo.pieralisi@xxxxxxx> wrote: >> > On Mon, Nov 10, 2014 at 11:04:54PM +0000, Bjorn Helgaas wrote: >> >> ... >> >> Here's what I think I understand so far: >> >> >> >> Applications can mmap PCI memory space via either sysfs or procfs (the >> >> procfs method is deprecated but still supported): >> >> >> >> - In sysfs, there's a separate /sys/devices/pci*/.../resource* file >> >> for each device BAR, and the application opens the appropriate >> >> file and supplies the offset from the beginning of the BAR as the >> >> mmap(2) offset. >> >> >> >> - In procfs, the application opens the single /proc/bus/pci/... file >> >> for the device. On most platforms, it supplies the CPU physical >> >> address as the mmap(2) offset. On a few platforms, such as SPARC, >> >> it supplies the bus address, i.e., a BAR value, instead. >> >> >> >> But I'm not sure I have this right. If the procfs offset is either the >> >> CPU physical address or the BAR value, then pci_resource_to_user() >> >> should be (depending on the arch) either a no-op or use >> >> pci_resource_to_bus(). >> > >> > Exactly (pcibios_resource_to_bus() ?). >> > >> >> But that's not how it's implemented. Maybe it *could* be? If >> >> pci_resource_to_user() gives you something that's not a CPU physical >> >> address and not a bus address, what *does* it give you, and why would we >> >> need this third kind of thing? >> > >> > Well, you need a per arch function implementation where to define if >> > the conversion from CPU physical address to PCI bus should take place >> > or not right ? As you mentioned above, if that should be a per-arch >> > decision, there has to be a per-arch function to filter the resource >> > in question, I guess that's my understanding behind pci_resource_to_user(), >> > but I am not sure either, and understanding that was the primary reason >> > for this patchset so comments are welcome. >> >> I agree that we need pci_resource_to_user() because arches do >> different things, so we can't just remove pci_resource_to_user() and >> replace it with pci_resource_to_bus(). My point is that we have a >> generic pci_resource_to_user() implementation that does nothing, and >> if an arch *does* implement its own pci_resource_to_user(), it seems >> like it should simply call pci_resource_to_user(). > > to_bus() you mean. Yes, exactly, sorry for my typo. (And earlier, when I said pci_resource_to_bus() instead of pcibios_resource_to_bus(). pcibios_resource_to_bus() *used* to be an arch-specific function with many implementations, which is why it's named pcibios_*. Now that the PCI core supports address translation, it's no longer arch-specific. We haven't changed the name to reflect that, but I don't think of it along with the rest of the pcibios_*() functions anymore. Hmm, and now we have this pci_resource_to_user() thing, which *is* arch-specific, but doesn't have a pcibios_* name. No wonder this is confusing :)) > Well, I agree, but I am not sure it would work on all > arches that deviate from the generic implementation, I can't speak for other > architectures since I do not have an in-depth knowledge of their PCI > internal implementations, in particular in relation to CPU <-> PCI > address map conversions/mappings. > > I read your comment as an agreement on the approach I took in my patch, > except for the current pci_resource_to_user() implementation(s), which I did > not touch. Yes. I have two things I'd like to clear up: 1) Your patch changes behavior on platforms that implement their own pci_resource_to_user(). So I'd like to mention the details of that in the changelog, e.g., "procfs mmap on arches X, Y, Z has been broken since commit C, and this change fixes them." ARM doesn't implement pci_resource_to_user(), so I don't think ARM is one of those arches. But I'd really like to include specifics on what those arches are, and what we think is currently broken, so their maintainers at least get a heads-up and can look for that breakage. 2) I'd like to take this opportunity to analyze the current pci_resource_to_user() implementations and see if we can reduce them to calling pcibios_resource_to_bus(). This is obviously separable, and I'll apply these patches anyway, but this does seem like a perfect opportunity for someone (not necessarily you) to clean this stuff up, since it's fresh in our minds. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
