LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Lorenzo Pieralisi <firstname.lastname@example.org>
To: Bjorn Helgaas <email@example.com>
Cc: "firstname.lastname@example.org" <email@example.com>,
Arnd Bergmann <firstname.lastname@example.org>,
Jesse Barnes <email@example.com>,
Benjamin Herrenschmidt <firstname.lastname@example.org>,
Russell King <email@example.com>,
"David S. Miller" <firstname.lastname@example.org>,
Michal Simek <email@example.com>,
Martin Wilck <firstname.lastname@example.org>,
Linux PCI <email@example.com>
Subject: Re: [RFC PATCH v3 1/2] drivers: pci: fix pci_mmap_fits() implementation for procfs mmap
Date: Tue, 25 Nov 2014 10:03:26 +0000 [thread overview]
Message-ID: <20141125100326.GB1849@red-moon> (raw)
On Fri, Nov 21, 2014 at 05:51:14PM +0000, Bjorn Helgaas wrote:
> On Thu, Nov 13, 2014 at 11:19:15AM +0000, Lorenzo Pieralisi wrote:
> > The introduction of pci_mmap_fits() in commit:
> > b5ff7df3df9efab511244d5a299fce706c71af48
> > "Check mapped ranges on sysfs resource files"
> > allowed to check for valid range mappings of PCI resources to user space
> > when mapping PCI resources through the sysfs filesystem.
> > The mapping of resources through the sysfs expects the offset passed
> > by the user through the mmap syscall to be 0, and the pgoff is adjusted
> > by the kernel to memory map the resource to the CPU physical address
> > corresponding to the PCI resource in question.
> > The usage of procfs mapping of PCI resources (/proc/bus/pci files)
> > is more controversial in that userspace programs mapping PCI resources
> > are expected to pass in the mmap offset field either a CPU physical address
> > or a PCI bar value, depending on the architecture.
> > By the time pci_mmap_fits() was used to check PCI resource ranges for
> > procfs PCI resources mapping in commit:
> > 9eff02e2042f96fb2aedd02e032eca1c5333d7
> > "PCI: check mmap range of /proc/bus/pci files too"
> > the procfs interface for mmapping resources to user space broke, since
> > pci_mmap_fits() expected the offset passed from user space in the mmap
> > call to be 0, not the CPU physical address or PCI BAR value of the
> > resource in question.
> > Subsequent attempts at fixing the pci_mmap_fits() implementation failed
> > to fix the issue (or fixed the issue in some architectures but not for
> > all of them, ARM and SPARC procfs interface PCI resources mapping stayed
> > broken throughout) in particular commits:
> > 8c05cd08a7504b855c265263e84af61aabafa329
> > "PCI: fix offset check for sysfs mmapped files"
> > and
> > 3b519e4ea618b6943a82931630872907f9ac2c2b
> > "PCI: fix size checks for mmap() on /proc/bus/pci files"
> > fixed procfs PCI resources mapping checks in pci_mmap_fits for some
> > architectures, but not for architectures like SPARC that expects
> > the offset value passed from user space through the mmap syscall
> > (when mapping through procfs) to represent the PCI BAR value of the
> > resource to be mapped.
> > The reason behind the breakage is the following. The addresses stored
> > in PCI device resources for memory spaces correspond to CPU physical
> > addresses, which do not necessarily map 1:1 to PCI bus addresses as
> > programmed in PCI devices configuration spaces.
> > This implies that the sanity checks carried out in pci_mmap_fits() to
> > ensure that the user executes an mmap of a "real" pci resource are
> > erroneous when executed through procfs. Some platforms (ie SPARC)
> > expect the offset value to be passed in (procfs mapping) to be the
> > PCI BAR configuration value as read from the PCI device configuration
> > space, not the fixed-up CPU physical address that is present in PCI
> > device resources.
> > The required pgoff (offset in mmap syscall) value passed from userspace
> > is supposed to represent the resource value exported through
> > /proc/bus/pci/devices when the resource is mmapped though procfs (and 0
> > when the mapping is carried out through sysfs resource files), which
> > corresponds to the PCI resource filtered through the pci_resource_to_user()
> > API.
> > This patch converts the PCI resource to the expected "user visible"
> > value through pci_resource_to_user() before carrying out sanity checks
> > in pci_mmap_fits() so that the check is carried out on the resource
> > values as expected from the userspace mmap API.
> > Cc: Arnd Bergmann <firstname.lastname@example.org>
> > Cc: Jesse Barnes <email@example.com>
> > Cc: Bjorn Helgaas <firstname.lastname@example.org>
> > Cc: Benjamin Herrenschmidt <email@example.com>
> > Cc: Russell King <firstname.lastname@example.org>
> > Cc: David S. Miller <email@example.com>
> > Cc: Michal Simek <firstname.lastname@example.org>
> > Cc: Martin Wilck <email@example.com>
> > Signed-off-by: Lorenzo Pieralisi <firstname.lastname@example.org>
> Hi Lorenzo,
> I think this patch is the right thing to do. I'm going to try to write
> patches for microblaze, mips, powerpc, and sparc that implement their
> pci_resource_to_user() in terms of pcibios_resource_to_bus() (the patches
> are easy; it's the arguments for correctness that take time). Then I'll
> try to convince myself that those arches are currently broken and will be
> fixed by your patch below.
I think that in doing that you might end up breaking at least powerpc
and microblaze proc and sys interfaces, but I agree with the process.
> But I'll be on vacation all next week, so this will take me some time and
> it may not make the next merge window.
I understand that, it holds back patch 2, but it is fine since it can
get in as a fix even if it misses the merge window, as long as you
agree with the overall approach.
next prev parent reply other threads:[~2014-11-25 10:03 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-13 11:19 [RFC PATCH v3 0/2] Fix procfs PCI resources mmap Lorenzo Pieralisi
2014-11-13 11:19 ` [RFC PATCH v3 1/2] drivers: pci: fix pci_mmap_fits() implementation for procfs mmap Lorenzo Pieralisi
2014-11-21 17:51 ` Bjorn Helgaas
2014-11-25 10:03 ` Lorenzo Pieralisi [this message]
2014-11-25 23:10 ` Benjamin Herrenschmidt
2015-01-21 18:44 ` Lorenzo Pieralisi
2015-01-23 23:58 ` Bjorn Helgaas
2014-11-13 11:19 ` [RFC PATCH v3 2/2] arm: kernel: fix pci_mmap_page_range() offset calculation Lorenzo Pieralisi
2015-01-23 23:59 ` Bjorn Helgaas
2015-01-26 9:57 ` Lorenzo Pieralisi
2014-11-19 9:57 ` [RFC PATCH v3 0/2] Fix procfs PCI resources mmap Lorenzo Pieralisi
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--subject='Re: [RFC PATCH v3 1/2] drivers: pci: fix pci_mmap_fits() implementation for procfs mmap' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).