linux-kernel - Re: [PATCH v17 04/10] PCI: Apply the new generic I/O management on PCI IO hosts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180403134531.GD27789@ulmo>
Date:   Tue, 3 Apr 2018 15:45:31 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     John Garry <john.garry@...wei.com>
Cc:     mika.westerberg@...ux.intel.com, rafael@...nel.org,
        lorenzo.pieralisi@....com, rjw@...ysocki.net,
        hanjun.guo@...aro.org, robh+dt@...nel.org, bhelgaas@...gle.com,
        arnd@...db.de, mark.rutland@....com, olof@...om.net,
        dann.frazier@...onical.com, andy.shevchenko@...il.com,
        robh@...nel.org, andriy.shevchenko@...ux.intel.com,
        joe@...ches.com, benh@...nel.crashing.org,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-acpi@...r.kernel.org, linuxarm@...wei.com, minyard@....org,
        devicetree@...r.kernel.org, linux-arch@...r.kernel.org,
        rdunlap@...radead.org, gregkh@...uxfoundation.org,
        akpm@...ux-foundation.org, frowand.list@...il.com, agraf@...e.de
Subject: Re: [PATCH v17 04/10] PCI: Apply the new generic I/O management on
 PCI IO hosts

On Thu, Mar 15, 2018 at 02:15:53AM +0800, John Garry wrote:
> From: Zhichang Yuan <yuanzhichang@...ilicon.com>
> 
> After introducing the new generic I/O space management(Logical PIO), the
> original PCI MMIO relevant helpers need to be updated based on the new
> interfaces defined in logical PIO.
> This patch adapts the corresponding code to match the changes introduced
> by logical PIO.
> 
> Signed-off-by: Zhichang Yuan <yuanzhichang@...ilicon.com>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@...wei.com>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>        #earlier draft
> Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
> Tested-by: dann frazier <dann.frazier@...onical.com>
> ---
>  drivers/pci/pci.c        | 92 +++++++++---------------------------------------
>  include/asm-generic/io.h |  2 +-
>  2 files changed, 18 insertions(+), 76 deletions(-)

Today's linux-next regresses on NFS boot for Jetson TK1. I was able to
bisect it to this commit. I'll comment below for where I think this is
buggy.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 3f30b7d..09c2490 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -22,6 +22,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/string.h>
>  #include <linux/log2.h>
> +#include <linux/logic_pio.h>
>  #include <linux/pci-aspm.h>
>  #include <linux/pm_wakeup.h>
>  #include <linux/interrupt.h>
> @@ -3436,17 +3437,6 @@ int pci_request_regions_exclusive(struct pci_dev *pdev, const char *res_name)
>  }
>  EXPORT_SYMBOL(pci_request_regions_exclusive);
>  
> -#ifdef PCI_IOBASE
> -struct io_range {
> -	struct list_head list;
> -	phys_addr_t start;
> -	resource_size_t size;
> -};
> -
> -static LIST_HEAD(io_range_list);
> -static DEFINE_SPINLOCK(io_range_lock);
> -#endif
> -
>  /*
>   * Record the PCI IO range (expressed as CPU physical address + size).
>   * Return a negative value if an error has occured, zero otherwise
> @@ -3454,51 +3444,28 @@ struct io_range {
>  int pci_register_io_range(struct fwnode_handle *fwnode, phys_addr_t addr,
>  			resource_size_t	size)
>  {
> -	int err = 0;
> -
> +	int ret = 0;
>  #ifdef PCI_IOBASE
> -	struct io_range *range;
> -	resource_size_t allocated_size = 0;
> -
> -	/* check if the range hasn't been previously recorded */
> -	spin_lock(&io_range_lock);
> -	list_for_each_entry(range, &io_range_list, list) {
> -		if (addr >= range->start && addr + size <= range->start + size) {
> -			/* range already registered, bail out */
> -			goto end_register;
> -		}
> -		allocated_size += range->size;
> -	}
> +	struct logic_pio_hwaddr *range;
>  
> -	/* range not registed yet, check for available space */
> -	if (allocated_size + size - 1 > IO_SPACE_LIMIT) {
> -		/* if it's too big check if 64K space can be reserved */
> -		if (allocated_size + SZ_64K - 1 > IO_SPACE_LIMIT) {
> -			err = -E2BIG;
> -			goto end_register;
> -		}
> -
> -		size = SZ_64K;
> -		pr_warn("Requested IO range too big, new size set to 64K\n");
> -	}
> +	if (!size || addr + size < addr)
> +		return -EINVAL;
>  
> -	/* add the range to the list */
>  	range = kzalloc(sizeof(*range), GFP_ATOMIC);
> -	if (!range) {
> -		err = -ENOMEM;
> -		goto end_register;
> -	}
> +	if (!range)
> +		return -ENOMEM;
>  
> -	range->start = addr;
> +	range->fwnode = fwnode;
>  	range->size = size;
> +	range->hw_start = addr;
> +	range->flags = LOGIC_PIO_CPU_MMIO;
>  
> -	list_add_tail(&range->list, &io_range_list);
> -
> -end_register:
> -	spin_unlock(&io_range_lock);
> +	ret = logic_pio_register_range(range);

This ends up returning -EFAULT at some point, causing the driver's
(pci-tegra.c) ->probe() to fail.

Let me comment on a prior patch to pinpoint what exactly goes wrong.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)