From patchwork Wed Jun 22 19:22:56 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yinghai Lu X-Patchwork-Id: 639376 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3rZZL63Mb3z9t0N for ; Thu, 23 Jun 2016 05:25:02 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=qtl/xqaT; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752894AbcFVTY5 (ORCPT ); Wed, 22 Jun 2016 15:24:57 -0400 Received: from mail-vk0-f65.google.com ([209.85.213.65]:34025 "EHLO mail-vk0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751854AbcFVTW6 (ORCPT ); Wed, 22 Jun 2016 15:22:58 -0400 Received: by mail-vk0-f65.google.com with SMTP id w1so9626117vka.1; Wed, 22 Jun 2016 12:22:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=oJF3XlCzgVE2GqLIZQ5wN6v4MsFV0Rg8NZySaZGZwao=; b=qtl/xqaTvHowlmWukbitZUh77ZYj9vPh+pKDqa2HCUiEWWOzpHM+wmzZcCnjD/x0jQ WcUvNsFSpYeD4xZ1CoQZ05hxYn0HqaDmzuRhKJX6MfhE10WxTtgUq4+biyL2rapJGnXP 5sjAJWum5A4Rxz2J0hVxG7Y5u/dWHjwY1b7SaETtoBqUqdSVdF+biWI/PYzOcsGHn1mf nbHlA9tnTz3vA3KcMgW/Q7wu6KgxRLqh8Yszlcd+/0lQ/vIVPagRBcyJBtAiH2Fmeqyh kUI9ygTlzAG+m2PGMbirt+UGKbFW7C05dFyiB4KDgLWQs6XlusUxSSlMt+3r/qDquTX2 JKlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=oJF3XlCzgVE2GqLIZQ5wN6v4MsFV0Rg8NZySaZGZwao=; b=ROQJ9f/wVEwfj8sZhc43tMD51Fu3kehGA1EypCvgCsgR2HrNVj6WVRl7/RrLae4vvV /h/BMxL9cKdZsS+IoopzdGekfn7iAtVaQRuOs9XlDqpVXb2k/CK7osXGl0ADPfvAEukc vdDpmn0YgLfngn2lQHwkT+XQ2wMPayHbTNPpeBs5VCc61Ix7Pt7cYdjxPh/Uk/pI7zET vRNVfnkGILfGC2Sqt4+hzTiHGj7EaB7B28zzWsvaANT/zPhI0pr/0NKCRLnVAspDIGcz iTSNByZoRf3YKB8gCpdsqBJvGKjPWlMn/+I7pyiuU2AHHRgEWenhOclBpTm/999NHNas mTdQ== X-Gm-Message-State: ALyK8tJTGcaFAGZKVMyjm+G3jNY1afmwdqfPuxABne343O/mDWR20q5y0zHwOoyhC8e5Yaj48Q9cqxgo7hn/Ug== X-Received: by 10.31.65.10 with SMTP id o10mr13006786vka.40.1466623377574; Wed, 22 Jun 2016 12:22:57 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.81.11 with HTTP; Wed, 22 Jun 2016 12:22:56 -0700 (PDT) In-Reply-To: <20160622152243.GC25485@localhost> References: <20160618022501.15648-1-yinghai@kernel.org> <20160618022501.15648-2-yinghai@kernel.org> <20160618121752.GA22721@localhost> <20160622152243.GC25485@localhost> From: Yinghai Lu Date: Wed, 22 Jun 2016 12:22:56 -0700 X-Google-Sender-Auth: TwFrVCAlwbtql2AeXkXzL8NSiuU Message-ID: Subject: Re: [PATCH v13 01/16] PCI: Let pci_mmap_page_range() take resource address To: Bjorn Helgaas Cc: Bjorn Helgaas , David Miller , Benjamin Herrenschmidt , Linus Torvalds , Wei Yang , Khalid Aziz , "linux-pci@vger.kernel.org" , Linux Kernel Mailing List , linuxppc-dev , "sparclinux@vger.kernel.org" , linux-xtensa@linux-xtensa.org Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Wed, Jun 22, 2016 at 8:22 AM, Bjorn Helgaas wrote: > On Tue, Jun 21, 2016 at 09:32:49PM -0700, Yinghai Lu wrote: > > If sparc is broken, let's make this a tiny sparc-only patch that fixes > only the breakage -- no cleanup or restructuring. Then we can do the > more extensive work in a separate patch. ok. please check attached two patches that take position for [PATCH v13 01/16] > > The example mmap() I keep asking for would be very helpful to me in > understanding the problem. It would probably also help folks who > maintain user programs that use mmap. They need to figure out whether > they have code that worked most places but has always been broken on > sparc, or code that depended on the previous sparc behavior and will > be broken by this change, or what. ok. calling : ./test_mmap_proc /proc/bus/pci/0000:00/04.0 0x2000000 code : test_mmap_proc.c #include #include #include #include #include #define PCIIOC_BASE ('P' << 24 | 'C' << 16 | 'I' << 8) #define PCIIOC_CONTROLLER (PCIIOC_BASE | 0x00) /* Get controller for PCI device. */ #define PCIIOC_MMAP_IS_IO (PCIIOC_BASE | 0x01) /* Set mmap state to I/O space. */ #define PCIIOC_MMAP_IS_MEM (PCIIOC_BASE | 0x02) /* Set mmap state to MEM space. */ #define PCIIOC_WRITE_COMBINE (PCIIOC_BASE | 0x03) /* Enable/disable write-combining. */ /* sparc64 */ #define PAGE_SHIFT 13 #define PAGE_SIZE (1UL << PAGE_SHIFT) #define PAGE_MASK (~(PAGE_SIZE-1)) int main(int argc, char *argv[]) { int i; int fd; char *addr; unsigned long offset = 0; unsigned long left = 0; fd = open(argv[1], O_RDONLY); if (fd < 0) { perror("open"); return 1; } if (argc > 2) sscanf(argv[2], "0x%lx", &offset); left = offset & (PAGE_SIZE - 1); offset &= PAGE_MASK; ioctl(fd, PCIIOC_MMAP_IS_MEM); addr = mmap(NULL, PAGE_SIZE, PROT_READ, MAP_SHARED, fd, offset); if (addr == MAP_FAILED) { perror("mmap"); return 1; } printf("map finished\n"); for (i = 0; i < 8; i++) printf("%x ", addr[i + left]); printf("\n"); munmap(addr, PAGE_SIZE); close(fd); return 0; } From: Yinghai Lu Subject: [PATCH v13.update.part2 01/16] PCI: Let pci_mmap_page_range() take resource address Original pci_mmap_page_range() is taking PCI BAR value aka usr_address. Bjorn found out that it would be much simple to pass resource address directly and avoid extra those __pci_mmap_make_offset. In this patch: 1. in proc path: proc_bus_pci_mmap, try convert back to resource before calling pci_mmap_page_range 2. in sysfs path: pci_mmap_resource will just offset with resource start. 3. all pci_mmap_page_range will have vma->vm_pgoff with in resource range instead of BAR value. 4. skip calling __pci_mmap_make_offset, as the checking is done in pci_mmap_fits(). -v2: add pci_user_to_resource and remove __pci_mmap_make_offset -v3: pass resource pointer with pci_mmap_page_range() -v4: put __pci_mmap_make_offset() removing to following patch seperate /sys io access alignment checking to another patch updated after Bjorn's pci_resource_to_user() changes. -v5: update after fix for pci_mmap with proc path accoring to Bjorn. Signed-off-by: Yinghai Lu Cc: linuxppc-dev@lists.ozlabs.org Cc: sparclinux@vger.kernel.org Cc: linux-xtensa@linux-xtensa.org --- arch/microblaze/pci/pci-common.c | 11 ++++--- arch/powerpc/kernel/pci-common.c | 11 ++++--- arch/sparc/kernel/pci.c | 4 -- arch/xtensa/kernel/pci.c | 13 ++++++-- drivers/pci/pci-sysfs.c | 32 ++++++++++---------- drivers/pci/pci.h | 2 - drivers/pci/proc.c | 60 +++++++++++++++++++++++++++++++++------ 7 files changed, 93 insertions(+), 40 deletions(-) Index: linux-2.6/arch/microblaze/pci/pci-common.c =================================================================== --- linux-2.6.orig/arch/microblaze/pci/pci-common.c +++ linux-2.6/arch/microblaze/pci/pci-common.c @@ -282,12 +282,15 @@ int pci_mmap_page_range(struct pci_dev * { resource_size_t offset = ((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT; - struct resource *rp; int ret; - rp = __pci_mmap_make_offset(dev, &offset, mmap_state); - if (rp == NULL) - return -EINVAL; + if (mmap_state == pci_mmap_io) { + struct pci_controller *hose = pci_bus_to_host(dev->bus); + + /* hose should never be NULL */ + offset += hose->io_base_phys - + ((unsigned long)hose->io_base_virt - _IO_BASE); + } vma->vm_pgoff = offset >> PAGE_SHIFT; vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); Index: linux-2.6/arch/powerpc/kernel/pci-common.c =================================================================== --- linux-2.6.orig/arch/powerpc/kernel/pci-common.c +++ linux-2.6/arch/powerpc/kernel/pci-common.c @@ -420,12 +420,15 @@ int pci_mmap_page_range(struct pci_dev * { resource_size_t offset = ((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT; - struct resource *rp; int ret; - rp = __pci_mmap_make_offset(dev, &offset, mmap_state); - if (rp == NULL) - return -EINVAL; + if (mmap_state == pci_mmap_io) { + struct pci_controller *hose = pci_bus_to_host(dev->bus); + + /* hose should never be NULL */ + offset += hose->io_base_phys - + ((unsigned long)hose->io_base_virt - _IO_BASE); + } vma->vm_pgoff = offset >> PAGE_SHIFT; if (write_combine) Index: linux-2.6/arch/sparc/kernel/pci.c =================================================================== --- linux-2.6.orig/arch/sparc/kernel/pci.c +++ linux-2.6/arch/sparc/kernel/pci.c @@ -868,10 +868,6 @@ int pci_mmap_page_range(struct pci_dev * { int ret; - ret = __pci_mmap_make_offset(dev, vma, mmap_state); - if (ret < 0) - return ret; - __pci_mmap_set_pgprot(dev, vma, mmap_state); vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); Index: linux-2.6/arch/xtensa/kernel/pci.c =================================================================== --- linux-2.6.orig/arch/xtensa/kernel/pci.c +++ linux-2.6/arch/xtensa/kernel/pci.c @@ -366,11 +366,18 @@ int pci_mmap_page_range(struct pci_dev * enum pci_mmap_state mmap_state, int write_combine) { + unsigned long offset = vma->vm_pgoff << PAGE_SHIFT; int ret; - ret = __pci_mmap_make_offset(dev, vma, mmap_state); - if (ret < 0) - return ret; + if (mmap_state == pci_mmap_io) { + struct pci_controller *pci_ctrl = + (struct pci_controller *)dev->sysdata; + + /* pci_ctrl should never be NULL */ + offset += pci_ctrl->io_space.start - pci_ctrl->io_space.base; + } + + vma->vm_pgoff = offset >> PAGE_SHIFT; __pci_mmap_set_pgprot(dev, vma, mmap_state, write_combine); Index: linux-2.6/drivers/pci/pci-sysfs.c =================================================================== --- linux-2.6.orig/drivers/pci/pci-sysfs.c +++ linux-2.6/drivers/pci/pci-sysfs.c @@ -972,22 +972,28 @@ void pci_remove_legacy_files(struct pci_ #ifdef HAVE_PCI_MMAP int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vma, + enum pci_mmap_state mmap_type, enum pci_mmap_api mmap_api) { unsigned long nr, start, size, pci_start = 0; + int flags; if (pci_resource_len(pdev, resno) == 0) return 0; + + if (mmap_type == pci_mmap_mem) + flags = IORESOURCE_MEM; + else + flags = IORESOURCE_IO; + + if (!(pci_resource_flags(pdev, resno) & flags)) + return 0; + nr = vma_pages(vma); start = vma->vm_pgoff; size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1; - if (mmap_api == PCI_MMAP_PROCFS) { - resource_size_t user_start, user_end; - - pci_resource_to_user(pdev, resno, &pdev->resource[resno], - &user_start, &user_end); - pci_start = user_start >> PAGE_SHIFT; - } + pci_start = (mmap_api == PCI_MMAP_PROCFS) ? + pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0; if (start >= pci_start && start < pci_start + size && start + nr <= pci_start + size) return 1; @@ -1009,7 +1015,6 @@ static int pci_mmap_resource(struct kobj struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj)); struct resource *res = attr->private; enum pci_mmap_state mmap_type; - resource_size_t start, end; int i; for (i = 0; i < PCI_ROM_RESOURCE; i++) @@ -1021,7 +1026,8 @@ static int pci_mmap_resource(struct kobj if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start)) return -EINVAL; - if (!pci_mmap_fits(pdev, i, vma, PCI_MMAP_SYSFS)) { + mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io; + if (!pci_mmap_fits(pdev, i, vma, mmap_type, PCI_MMAP_SYSFS)) { WARN(1, "process \"%s\" tried to map 0x%08lx bytes at page 0x%08lx on %s BAR %d (start 0x%16Lx, size 0x%16Lx)\n", current->comm, vma->vm_end-vma->vm_start, vma->vm_pgoff, pci_name(pdev), i, @@ -1030,13 +1036,7 @@ static int pci_mmap_resource(struct kobj return -EINVAL; } - /* pci_mmap_page_range() expects the same kind of entry as coming - * from /proc/bus/pci/ which is a "user visible" value. If this is - * different from the resource itself, arch will do necessary fixup. - */ - pci_resource_to_user(pdev, i, res, &start, &end); - vma->vm_pgoff += start >> PAGE_SHIFT; - mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io; + vma->vm_pgoff += res->start >> PAGE_SHIFT; return pci_mmap_page_range(pdev, vma, mmap_type, write_combine); } Index: linux-2.6/drivers/pci/proc.c =================================================================== --- linux-2.6.orig/drivers/pci/proc.c +++ linux-2.6/drivers/pci/proc.c @@ -227,24 +227,68 @@ static long proc_bus_pci_ioctl(struct fi } #ifdef HAVE_PCI_MMAP + +static int pci_user_to_resource(struct pci_dev *dev, resource_size_t *offset, + int flags) +{ + int i; + + for (i = 0; i < PCI_ROM_RESOURCE; i++) { + resource_size_t start, end; + struct resource *res = &dev->resource[i]; + + if (!(res->flags & flags)) + continue; + + if (pci_resource_len(dev, i) == 0) + continue; + + /* + * here *offset is PAGE_SIZE aligned from caller. + * need align start/end for io port resource that is + * usually not PAGE_SIZE aligned. + * that means we let it go if they falls in same page. + */ + pci_resource_to_user(dev, i, res, &start, &end); + if ((start & PAGE_MASK) <= *offset && + *offset <= (end & PAGE_MASK)) { + *offset = res->start + (*offset - start); + return i; + } + } + + return -ENODEV; +} + static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma) { struct pci_dev *dev = PDE_DATA(file_inode(file)); struct pci_filp_private *fpriv = file->private_data; - int i, ret, write_combine; + resource_size_t offset; + int i, ret, flags, write_combine; if (!capable(CAP_SYS_RAWIO)) return -EPERM; - /* Make sure the caller is mapping a real resource for this device */ - for (i = 0; i < PCI_ROM_RESOURCE; i++) { - if (pci_mmap_fits(dev, i, vma, PCI_MMAP_PROCFS)) - break; - } - - if (i >= PCI_ROM_RESOURCE) + offset = vma->vm_pgoff << PAGE_SHIFT; + if (fpriv->mmap_state == pci_mmap_mem) + flags = IORESOURCE_MEM; + else + flags = IORESOURCE_IO; + i = pci_user_to_resource(dev, &offset, flags); + if (i < 0) return -ENODEV; + vma->vm_pgoff = offset >> PAGE_SHIFT; + if (!pci_mmap_fits(dev, i, vma, fpriv->mmap_state, PCI_MMAP_PROCFS)) { + WARN(1, "process \"%s\" tried to map 0x%08lx bytes at page 0x%08lx on %s BAR %d (start 0x%16Lx, size 0x%16Lx)\n", + current->comm, vma->vm_end-vma->vm_start, vma->vm_pgoff, + pci_name(dev), i, + (u64)pci_resource_start(dev, i), + (u64)pci_resource_len(dev, i)); + return -EINVAL; + } + if (fpriv->mmap_state == pci_mmap_mem) write_combine = fpriv->write_combine; else Index: linux-2.6/drivers/pci/pci.h =================================================================== --- linux-2.6.orig/drivers/pci/pci.h +++ linux-2.6/drivers/pci/pci.h @@ -30,7 +30,7 @@ enum pci_mmap_api { PCI_MMAP_PROCFS /* mmap on /proc/bus/pci/ */ }; int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vmai, - enum pci_mmap_api mmap_api); + enum pci_mmap_state mmap_type, enum pci_mmap_api mmap_api); #endif int pci_probe_reset_function(struct pci_dev *dev);