[PATCH 1/2] PCI: fix restoring resized BAR state on resume

[PATCH 1/2] PCI: fix restoring resized BAR state on resume

[Christian König]

Resize BARs after resume to the expected size again.

Signed-off-by: Christian König <christian.koenig@amd.com>
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959
CC: stable@vger.kernel.org      # v4.15+
---
 drivers/pci/pci.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bd6f156dc3cf..d4685090378b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1159,6 +1159,33 @@ static void pci_restore_config_space(struct pci_dev *pdev)
 	}
 }
 
+static void pci_restore_rebar_state(struct pci_dev *pdev)
+{
+	unsigned int pos, nbars, i;
+	u32 ctrl;
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+	if (!pos)
+		return;
+
+	pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+	nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
+		    PCI_REBAR_CTRL_NBAR_SHIFT;
+
+	for (i = 0; i < nbars; i++, pos += 8) {
+		struct resource *res;
+		int bar_idx, size;
+
+		pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
+		bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
+		res = pdev->resource + bar_idx;
+		size = order_base_2((resource_size(res) >> 20) | 1) - 1;
+		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
+		ctrl |= size << 8;
+		pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
+	}
+}
+
 /**
  * pci_restore_state - Restore the saved state of a PCI device
  * @dev: - PCI device that we're dealing with
@@ -1174,6 +1201,7 @@ void pci_restore_state(struct pci_dev *dev)
 	pci_restore_pri_state(dev);
 	pci_restore_ats_state(dev);
 	pci_restore_vc_state(dev);
+	pci_restore_rebar_state(dev);
 
 	pci_cleanup_aer_error_status_regs(dev);
 
-- 
2.14.1

[PATCH 2/2] PCI: cleanup PCI_REBAR_CTRL_BAR_SHIFT handling

[Christian König]

That was hard coded instead of properly defined in the header for some
reason.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/pci/pci.c             | 6 +++---
 include/uapi/linux/pci_regs.h | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d4685090378b..a905e35d9360 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1181,7 +1181,7 @@ static void pci_restore_rebar_state(struct pci_dev *pdev)
 		res = pdev->resource + bar_idx;
 		size = order_base_2((resource_size(res) >> 20) | 1) - 1;
 		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
-		ctrl |= size << 8;
+		ctrl |= size << PCI_REBAR_CTRL_BAR_SHIFT;
 		pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
 	}
 }
@@ -3066,7 +3066,7 @@ int pci_rebar_get_current_size(struct pci_dev *pdev, int bar)
 		return pos;
 
 	pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
-	return (ctrl & PCI_REBAR_CTRL_BAR_SIZE) >> 8;
+	return (ctrl & PCI_REBAR_CTRL_BAR_SIZE) >> PCI_REBAR_CTRL_BAR_SHIFT;
 }
 
 /**
@@ -3089,7 +3089,7 @@ int pci_rebar_set_size(struct pci_dev *pdev, int bar, int size)
 
 	pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
 	ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
-	ctrl |= size << 8;
+	ctrl |= size << PCI_REBAR_CTRL_BAR_SHIFT;
 	pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
 	return 0;
 }
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 0c79eac5e9b8..7358ad00f5dd 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -950,8 +950,9 @@
 #define PCI_REBAR_CTRL		8	/* control register */
 #define  PCI_REBAR_CTRL_BAR_IDX		0x00000007  /* BAR index */
 #define  PCI_REBAR_CTRL_NBAR_MASK	0x000000E0  /* # of resizable BARs */
-#define  PCI_REBAR_CTRL_NBAR_SHIFT	5  	    /* shift for # of BARs */
+#define  PCI_REBAR_CTRL_NBAR_SHIFT	5	    /* shift for # of BARs */
 #define  PCI_REBAR_CTRL_BAR_SIZE	0x00001F00  /* BAR size */
+#define  PCI_REBAR_CTRL_BAR_SHIFT	8	    /* shift for BAR size */
 
 /* Dynamic Power Allocation */
 #define PCI_DPA_CAP		4	/* capability register */
-- 
2.14.1

Re: [PATCH 1/2] PCI: fix restoring resized BAR state on resume

[Bjorn Helgaas]

On Thu, Jun 14, 2018 at 02:21:45PM +0200, Christian König wrote:
> Resize BARs after resume to the expected size again.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959
> CC: stable@vger.kernel.org      # v4.15+

I added 

  Fixes: d6895ad39f3b ("drm/amdgpu: resize VRAM BAR for CPU access v6")
  Fixes: 276b738deb5b ("PCI: Add resizable BAR infrastructure")

Per the bugzilla, the bug was bisected to d6895ad39f3b, which appeared
in v4.16.  But 276b738deb5b appeared in v4.15, so I'm OK with the
v4.15+ stable tag.

> ---
>  drivers/pci/pci.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index bd6f156dc3cf..d4685090378b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1159,6 +1159,33 @@ static void pci_restore_config_space(struct pci_dev *pdev)
>  	}
>  }
>  
> +static void pci_restore_rebar_state(struct pci_dev *pdev)
> +{
> +	unsigned int pos, nbars, i;
> +	u32 ctrl;
> +
> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> +	if (!pos)
> +		return;
> +
> +	pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> +	nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
> +		    PCI_REBAR_CTRL_NBAR_SHIFT;
> +
> +	for (i = 0; i < nbars; i++, pos += 8) {
> +		struct resource *res;
> +		int bar_idx, size;
> +
> +		pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> +		bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
> +		res = pdev->resource + bar_idx;
> +		size = order_base_2((resource_size(res) >> 20) | 1) - 1;
> +		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
> +		ctrl |= size << 8;
> +		pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
> +	}
> +}
> +
>  /**
>   * pci_restore_state - Restore the saved state of a PCI device
>   * @dev: - PCI device that we're dealing with
> @@ -1174,6 +1201,7 @@ void pci_restore_state(struct pci_dev *dev)
>  	pci_restore_pri_state(dev);
>  	pci_restore_ats_state(dev);
>  	pci_restore_vc_state(dev);
> +	pci_restore_rebar_state(dev);
>  
>  	pci_cleanup_aer_error_status_regs(dev);
>  
> -- 
> 2.14.1
> 

Re: [PATCH 1/2] PCI: fix restoring resized BAR state on resume

[Bjorn Helgaas]

On Thu, Jun 14, 2018 at 02:21:45PM +0200, Christian König wrote:
> Resize BARs after resume to the expected size again.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959
> CC: stable@vger.kernel.org      # v4.15+

I applied both of these to pci/resource for v4.19, thanks!  It'd be nice to
have them in v4.18, but since it's not a regression and the problem has
been there since v4.15, I'm not sure I could justify it.

> ---
>  drivers/pci/pci.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index bd6f156dc3cf..d4685090378b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1159,6 +1159,33 @@ static void pci_restore_config_space(struct pci_dev *pdev)
>  	}
>  }
>  
> +static void pci_restore_rebar_state(struct pci_dev *pdev)
> +{
> +	unsigned int pos, nbars, i;
> +	u32 ctrl;
> +
> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> +	if (!pos)
> +		return;
> +
> +	pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> +	nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
> +		    PCI_REBAR_CTRL_NBAR_SHIFT;
> +
> +	for (i = 0; i < nbars; i++, pos += 8) {
> +		struct resource *res;
> +		int bar_idx, size;
> +
> +		pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> +		bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
> +		res = pdev->resource + bar_idx;
> +		size = order_base_2((resource_size(res) >> 20) | 1) - 1;
> +		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
> +		ctrl |= size << 8;
> +		pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
> +	}
> +}
> +
>  /**
>   * pci_restore_state - Restore the saved state of a PCI device
>   * @dev: - PCI device that we're dealing with
> @@ -1174,6 +1201,7 @@ void pci_restore_state(struct pci_dev *dev)
>  	pci_restore_pri_state(dev);
>  	pci_restore_ats_state(dev);
>  	pci_restore_vc_state(dev);
> +	pci_restore_rebar_state(dev);
>  
>  	pci_cleanup_aer_error_status_regs(dev);
>  
> -- 
> 2.14.1
> 

Re: [PATCH 1/2] PCI: fix restoring resized BAR state on resume

[Christian König]

Am 30.06.2018 um 02:57 schrieb Bjorn Helgaas:
> On Thu, Jun 14, 2018 at 02:21:45PM +0200, Christian König wrote:
>> Resize BARs after resume to the expected size again.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959
>> CC: stable@vger.kernel.org      # v4.15+
> I applied both of these to pci/resource for v4.19, thanks!  It'd be nice to
> have them in v4.18, but since it's not a regression and the problem has
> been there since v4.15, I'm not sure I could justify it.

Well it is a regression. The change to enable this in the upstream 
driver was added in 4.16, but a good bunch of people also compile our 
backported DKMS module.

And now those people are complaining that the driver stopped working 
after resume in 4.15 with the DKMS module and with 4.16 with the 
upstream module.

Alternatively I can disable resizing from the driver from kernels older 
than v4.19.

Christian.

>
>> ---
>>   drivers/pci/pci.c | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index bd6f156dc3cf..d4685090378b 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -1159,6 +1159,33 @@ static void pci_restore_config_space(struct pci_dev *pdev)
>>   	}
>>   }
>>   
>> +static void pci_restore_rebar_state(struct pci_dev *pdev)
>> +{
>> +	unsigned int pos, nbars, i;
>> +	u32 ctrl;
>> +
>> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
>> +	if (!pos)
>> +		return;
>> +
>> +	pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
>> +	nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
>> +		    PCI_REBAR_CTRL_NBAR_SHIFT;
>> +
>> +	for (i = 0; i < nbars; i++, pos += 8) {
>> +		struct resource *res;
>> +		int bar_idx, size;
>> +
>> +		pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
>> +		bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
>> +		res = pdev->resource + bar_idx;
>> +		size = order_base_2((resource_size(res) >> 20) | 1) - 1;
>> +		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
>> +		ctrl |= size << 8;
>> +		pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
>> +	}
>> +}
>> +
>>   /**
>>    * pci_restore_state - Restore the saved state of a PCI device
>>    * @dev: - PCI device that we're dealing with
>> @@ -1174,6 +1201,7 @@ void pci_restore_state(struct pci_dev *dev)
>>   	pci_restore_pri_state(dev);
>>   	pci_restore_ats_state(dev);
>>   	pci_restore_vc_state(dev);
>> +	pci_restore_rebar_state(dev);
>>   
>>   	pci_cleanup_aer_error_status_regs(dev);
>>   
>> -- 
>> 2.14.1
>>

Re: [PATCH 1/2] PCI: fix restoring resized BAR state on resume

[Bjorn Helgaas]

On Mon, Jul 02, 2018 at 03:20:59PM +0200, Christian König wrote:
> Am 30.06.2018 um 02:57 schrieb Bjorn Helgaas:
> > On Thu, Jun 14, 2018 at 02:21:45PM +0200, Christian König wrote:
> > > Resize BARs after resume to the expected size again.
> > > 
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959
> > > CC: stable@vger.kernel.org      # v4.15+
> > I applied both of these to pci/resource for v4.19, thanks!  It'd be nice to
> > have them in v4.18, but since it's not a regression and the problem has
> > been there since v4.15, I'm not sure I could justify it.
> 
> Well it is a regression. The change to enable this in the upstream driver
> was added in 4.16, but a good bunch of people also compile our backported
> DKMS module.
> 
> And now those people are complaining that the driver stopped working after
> resume in 4.15 with the DKMS module and with 4.16 with the upstream module.
> 
> Alternatively I can disable resizing from the driver from kernels older than
> v4.19.

Feel free to help justify including this in v4.18 by providing the
details of the regression and the relevant commits.  If this is
something that worked in v4.17, and some commit we merged for
v4.18-rc1 broke it, it would be a slam dunk to include the fix for
v4.18.  I took a quick look through the commits we added in v4.18-rc1
but didn't see anything obvious.

Obviously, putting a fix in v4.18 doesn't help the people running
v4.15 or v4.16.  The stable backports *will* help with that, and we
already have the tag for that.

The current trajectory we're on is that these are headed for
v4.19-rc1, and soon after they appear there, they will be backported
to the v4.15-v4.18 stable kernels.

Bjorn

> > > ---
> > >   drivers/pci/pci.c | 28 ++++++++++++++++++++++++++++
> > >   1 file changed, 28 insertions(+)
> > > 
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index bd6f156dc3cf..d4685090378b 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -1159,6 +1159,33 @@ static void pci_restore_config_space(struct pci_dev *pdev)
> > >   	}
> > >   }
> > > +static void pci_restore_rebar_state(struct pci_dev *pdev)
> > > +{
> > > +	unsigned int pos, nbars, i;
> > > +	u32 ctrl;
> > > +
> > > +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> > > +	if (!pos)
> > > +		return;
> > > +
> > > +	pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> > > +	nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
> > > +		    PCI_REBAR_CTRL_NBAR_SHIFT;
> > > +
> > > +	for (i = 0; i < nbars; i++, pos += 8) {
> > > +		struct resource *res;
> > > +		int bar_idx, size;
> > > +
> > > +		pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> > > +		bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
> > > +		res = pdev->resource + bar_idx;
> > > +		size = order_base_2((resource_size(res) >> 20) | 1) - 1;
> > > +		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
> > > +		ctrl |= size << 8;
> > > +		pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
> > > +	}
> > > +}
> > > +
> > >   /**
> > >    * pci_restore_state - Restore the saved state of a PCI device
> > >    * @dev: - PCI device that we're dealing with
> > > @@ -1174,6 +1201,7 @@ void pci_restore_state(struct pci_dev *dev)
> > >   	pci_restore_pri_state(dev);
> > >   	pci_restore_ats_state(dev);
> > >   	pci_restore_vc_state(dev);
> > > +	pci_restore_rebar_state(dev);
> > >   	pci_cleanup_aer_error_status_regs(dev);
> > > -- 
> > > 2.14.1
> > > 
> 

Re: [PATCH 1/2] PCI: fix restoring resized BAR state on resume

[Christian König]

Am 02.07.2018 um 15:49 schrieb Bjorn Helgaas:
> On Mon, Jul 02, 2018 at 03:20:59PM +0200, Christian König wrote:
>> Am 30.06.2018 um 02:57 schrieb Bjorn Helgaas:
>>> On Thu, Jun 14, 2018 at 02:21:45PM +0200, Christian König wrote:
>>>> Resize BARs after resume to the expected size again.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959
>>>> CC: stable@vger.kernel.org      # v4.15+
>>> I applied both of these to pci/resource for v4.19, thanks!  It'd be nice to
>>> have them in v4.18, but since it's not a regression and the problem has
>>> been there since v4.15, I'm not sure I could justify it.
>> Well it is a regression. The change to enable this in the upstream driver
>> was added in 4.16, but a good bunch of people also compile our backported
>> DKMS module.
>>
>> And now those people are complaining that the driver stopped working after
>> resume in 4.15 with the DKMS module and with 4.16 with the upstream module.
>>
>> Alternatively I can disable resizing from the driver from kernels older than
>> v4.19.
> Feel free to help justify including this in v4.18 by providing the
> details of the regression and the relevant commits.  If this is
> something that worked in v4.17, and some commit we merged for
> v4.18-rc1 broke it, it would be a slam dunk to include the fix for
> v4.18.  I took a quick look through the commits we added in v4.18-rc1
> but didn't see anything obvious.
>
> Obviously, putting a fix in v4.18 doesn't help the people running
> v4.15 or v4.16.  The stable backports *will* help with that, and we
> already have the tag for that.
>
> The current trajectory we're on is that these are headed for
> v4.19-rc1, and soon after they appear there, they will be backported
> to the v4.15-v4.18 stable kernels.

Ah! Thanks for the explanation on how fixes flow into stable kernels and 
you are right it's indeed not a regression in 4.18.

In this case everything is fine and the backports to stable kernels 
should handle that.

Thanks,
Christian.

>
> Bjorn
>
>>>> ---
>>>>    drivers/pci/pci.c | 28 ++++++++++++++++++++++++++++
>>>>    1 file changed, 28 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>>> index bd6f156dc3cf..d4685090378b 100644
>>>> --- a/drivers/pci/pci.c
>>>> +++ b/drivers/pci/pci.c
>>>> @@ -1159,6 +1159,33 @@ static void pci_restore_config_space(struct pci_dev *pdev)
>>>>    	}
>>>>    }
>>>> +static void pci_restore_rebar_state(struct pci_dev *pdev)
>>>> +{
>>>> +	unsigned int pos, nbars, i;
>>>> +	u32 ctrl;
>>>> +
>>>> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
>>>> +	if (!pos)
>>>> +		return;
>>>> +
>>>> +	pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
>>>> +	nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
>>>> +		    PCI_REBAR_CTRL_NBAR_SHIFT;
>>>> +
>>>> +	for (i = 0; i < nbars; i++, pos += 8) {
>>>> +		struct resource *res;
>>>> +		int bar_idx, size;
>>>> +
>>>> +		pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
>>>> +		bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
>>>> +		res = pdev->resource + bar_idx;
>>>> +		size = order_base_2((resource_size(res) >> 20) | 1) - 1;
>>>> +		ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
>>>> +		ctrl |= size << 8;
>>>> +		pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
>>>> +	}
>>>> +}
>>>> +
>>>>    /**
>>>>     * pci_restore_state - Restore the saved state of a PCI device
>>>>     * @dev: - PCI device that we're dealing with
>>>> @@ -1174,6 +1201,7 @@ void pci_restore_state(struct pci_dev *dev)
>>>>    	pci_restore_pri_state(dev);
>>>>    	pci_restore_ats_state(dev);
>>>>    	pci_restore_vc_state(dev);
>>>> +	pci_restore_rebar_state(dev);
>>>>    	pci_cleanup_aer_error_status_regs(dev);
>>>> -- 
>>>> 2.14.1
>>>>