[v2] PCI: don't UNSET valid resources when reassign fails

On Thu, Jun 19, 2014 at 07:58:16PM +0800, Guo Chao wrote:
> Commit bd064f0a sets IORESOURCE_UNSET flag if resources can not be
> assgined. Part of these changes breaks resource assignment.
> 
> If resource is too big, PCI core assgins basic part first and
> extends (reassigns) it to include optional part (notably SR-IOV resources).
> In this case, IORESOURCE_UNSET should not be set if resource is properly
> assigned before reassigning. Otherwise, these resources are never updated
> to hardware and we will see errors like this:
> 
> 	pci 0003:00:00.0: can't enable device: BAR 15 [mem size 0x0c000000 64bit pref] not assigned
> 	pci 0003:00:00.0: Error enabling bridge (-22), continuing
> 
> Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>

I propose the following simpler patch.  Can you verify that it works for
you?


commit 57ece376a3b9e013277032f473197052f1144142
Author: Guo Chao <yan@linux.vnet.ibm.com>
Date:   Thu Jul 3 18:30:29 2014 -0600

    PCI: Keep original resource if we fail to expand it
    
    If we have space assigned to a resource, we try to expand the resource
    (e.g., to accommodate SR-IOV resources), and the expansion attempt fails,
    we should keep the original assignment.
    
    After bd064f0a231a ("PCI: Mark resources as IORESOURCE_UNSET if we can't
    assign them"), we left the resource marked IORESOURCE_UNSET when the
    expansion failed, even if it had originally been set.  That caused errors
    like this:
    
      pci 0003:00:00.0: can't enable device: BAR 15 [mem size 0x0c000000 64bit pref] not assigned
      pci 0003:00:00.0: Error enabling bridge (-22), continuing
    
    Fix this by restoring the original flags when reassignment fails.
    
    [bhelgaas: reworked to simplify, changelog]
    Fixes: bd064f0a231a ("PCI: Mark resources as IORESOURCE_UNSET if we can't assign them")
    Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    CC: stable@vger.kernel.org	# v3.15+

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sat, Jul 05, 2014 at 10:27:08AM -0600, Bjorn Helgaas wrote:
> On Thu, Jun 19, 2014 at 07:58:16PM +0800, Guo Chao wrote:
> > Commit bd064f0a sets IORESOURCE_UNSET flag if resources can not be
> > assgined. Part of these changes breaks resource assignment.
> > 
> > If resource is too big, PCI core assgins basic part first and
> > extends (reassigns) it to include optional part (notably SR-IOV resources).
> > In this case, IORESOURCE_UNSET should not be set if resource is properly
> > assigned before reassigning. Otherwise, these resources are never updated
> > to hardware and we will see errors like this:
> > 
> > 	pci 0003:00:00.0: can't enable device: BAR 15 [mem size 0x0c000000 64bit pref] not assigned
> > 	pci 0003:00:00.0: Error enabling bridge (-22), continuing
> > 
> > Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
> 
> I propose the following simpler patch.  Can you verify that it works for
> you?

Hi Bjorn:

	Sure it works.

	(Sorry for this late response. The machine is out of reach for
	past few days.)

Thanks,
Guo Chao
> 
> 
> commit 57ece376a3b9e013277032f473197052f1144142
> Author: Guo Chao <yan@linux.vnet.ibm.com>
> Date:   Thu Jul 3 18:30:29 2014 -0600
> 
>     PCI: Keep original resource if we fail to expand it
>     
>     If we have space assigned to a resource, we try to expand the resource
>     (e.g., to accommodate SR-IOV resources), and the expansion attempt fails,
>     we should keep the original assignment.
>     
>     After bd064f0a231a ("PCI: Mark resources as IORESOURCE_UNSET if we can't
>     assign them"), we left the resource marked IORESOURCE_UNSET when the
>     expansion failed, even if it had originally been set.  That caused errors
>     like this:
>     
>       pci 0003:00:00.0: can't enable device: BAR 15 [mem size 0x0c000000 64bit pref] not assigned
>       pci 0003:00:00.0: Error enabling bridge (-22), continuing
>     
>     Fix this by restoring the original flags when reassignment fails.
>     
>     [bhelgaas: reworked to simplify, changelog]
>     Fixes: bd064f0a231a ("PCI: Mark resources as IORESOURCE_UNSET if we can't assign them")
>     Signed-off-by: Guo Chao <yan@linux.vnet.ibm.com>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>     CC: stable@vger.kernel.org	# v3.15+
> 
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index caed1ce6facd..9faebcfeec4d 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -320,9 +320,11 @@ int pci_reassign_resource(struct pci_dev *dev, int resno, resource_size_t addsiz
>  			resource_size_t min_align)
>  {
>  	struct resource *res = dev->resource + resno;
> +	unsigned long flags;
>  	resource_size_t new_size;
>  	int ret;
> 
> +	flags = res->flags;
>  	res->flags |= IORESOURCE_UNSET;
>  	if (!res->parent) {
>  		dev_info(&dev->dev, "BAR %d: can't reassign an unassigned resource %pR\n",
> @@ -339,7 +341,12 @@ int pci_reassign_resource(struct pci_dev *dev, int resno, resource_size_t addsiz
>  		dev_info(&dev->dev, "BAR %d: reassigned %pR\n", resno, res);
>  		if (resno < PCI_BRIDGE_RESOURCES)
>  			pci_update_resource(dev, resno);
> +	} else {
> +		res->flags = flags;
> +		dev_info(&dev->dev, "BAR %d: %pR (failed to expand by %#llx)\n",
> +			 resno, res, addsize);
>  	}
> +
>  	return ret;
>  }
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html