[PATCH] PCI/VMD: Fix config addressing with bus offsets

[PATCH] PCI/VMD: Fix config addressing with bus offsets

[Jon Derrick]

VMD config space addressing relies on mapping the BDF of the target into
the VMD config bar. When using bus number offsets to number the VMD
domain, the offset needs to be ignored in order to correctly map devices
to their config space.

Fixes: 2a5a9c9a20f9 ("PCI: vmd: Add offset to bus numbers if necessary")
Cc: <stable@vger.kernel.org> # v4.19
Cc: <stable@vger.kernel.org> # v4.18
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/controller/vmd.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index fd2dbd7..a59afec 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -94,6 +94,7 @@ struct vmd_dev {
 	struct resource		resources[3];
 	struct irq_domain	*irq_domain;
 	struct pci_bus		*bus;
+	u8			busn_start;
 
 #ifdef CONFIG_X86_DEV_DMA_OPS
 	struct dma_map_ops	dma_ops;
@@ -465,7 +466,8 @@ static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
 				  unsigned int devfn, int reg, int len)
 {
 	char __iomem *addr = vmd->cfgbar +
-			     (bus->number << 20) + (devfn << 12) + reg;
+			     ((bus->number - vmd->busn_start) << 20) +
+			     (devfn << 12) + reg;
 
 	if ((addr - vmd->cfgbar) + len >=
 	    resource_size(&vmd->dev->resource[VMD_CFGBAR]))
@@ -588,7 +590,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	unsigned long flags;
 	LIST_HEAD(resources);
 	resource_size_t offset[2] = {0};
-	resource_size_t membar2_offset = 0x2000, busn_start = 0;
+	resource_size_t membar2_offset = 0x2000;
 
 	/*
 	 * Shadow registers may exist in certain VMD device ids which allow
@@ -630,14 +632,14 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 		pci_read_config_dword(vmd->dev, PCI_REG_VMCONFIG, &vmconfig);
 		if (BUS_RESTRICT_CAP(vmcap) &&
 		    (BUS_RESTRICT_CFG(vmconfig) == 0x1))
-			busn_start = 128;
+			vmd->busn_start = 128;
 	}
 
 	res = &vmd->dev->resource[VMD_CFGBAR];
 	vmd->resources[0] = (struct resource) {
 		.name  = "VMD CFGBAR",
-		.start = busn_start,
-		.end   = busn_start + (resource_size(res) >> 20) - 1,
+		.start = vmd->busn_start,
+		.end   = vmd->busn_start + (resource_size(res) >> 20) - 1,
 		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
 	};
 
@@ -705,8 +707,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
 	pci_add_resource_offset(&resources, &vmd->resources[2], offset[1]);
 
-	vmd->bus = pci_create_root_bus(&vmd->dev->dev, busn_start, &vmd_ops,
-				       sd, &resources);
+	vmd->bus = pci_create_root_bus(&vmd->dev->dev, vmd->busn_start,
+				       &vmd_ops, sd, &resources);
 	if (!vmd->bus) {
 		pci_free_resource_list(&resources);
 		irq_domain_remove(vmd->irq_domain);
-- 
1.8.3.1

Re: [PATCH] PCI/VMD: Fix config addressing with bus offsets

[Lorenzo Pieralisi]

[dropped CC stable]

On Tue, Jun 11, 2019 at 03:15:38PM -0600, Jon Derrick wrote:
> VMD config space addressing relies on mapping the BDF of the target into
> the VMD config bar. When using bus number offsets to number the VMD
> domain, the offset needs to be ignored in order to correctly map devices
> to their config space.
> 
> Fixes: 2a5a9c9a20f9 ("PCI: vmd: Add offset to bus numbers if necessary")
> Cc: <stable@vger.kernel.org> # v4.19
> Cc: <stable@vger.kernel.org> # v4.18

Hi Jon,

that's not how stable should be handled. You should always start
by fixing mainline and if there are backports to be fixed too you
should add patch dependencies in the CC area, see:

Documentation/process/stable-kernel-rules.rst

Never add stable to the CC list in the email header, only in the
commit log.

When your patch hits mainline it will trickle back into stable,
if you specified dependencies as described above there is nothing
to do.

Thanks,
Lorenzo

> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/controller/vmd.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index fd2dbd7..a59afec 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -94,6 +94,7 @@ struct vmd_dev {
>  	struct resource		resources[3];
>  	struct irq_domain	*irq_domain;
>  	struct pci_bus		*bus;
> +	u8			busn_start;
>  
>  #ifdef CONFIG_X86_DEV_DMA_OPS
>  	struct dma_map_ops	dma_ops;
> @@ -465,7 +466,8 @@ static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
>  				  unsigned int devfn, int reg, int len)
>  {
>  	char __iomem *addr = vmd->cfgbar +
> -			     (bus->number << 20) + (devfn << 12) + reg;
> +			     ((bus->number - vmd->busn_start) << 20) +
> +			     (devfn << 12) + reg;
>  
>  	if ((addr - vmd->cfgbar) + len >=
>  	    resource_size(&vmd->dev->resource[VMD_CFGBAR]))
> @@ -588,7 +590,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  	unsigned long flags;
>  	LIST_HEAD(resources);
>  	resource_size_t offset[2] = {0};
> -	resource_size_t membar2_offset = 0x2000, busn_start = 0;
> +	resource_size_t membar2_offset = 0x2000;
>  
>  	/*
>  	 * Shadow registers may exist in certain VMD device ids which allow
> @@ -630,14 +632,14 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  		pci_read_config_dword(vmd->dev, PCI_REG_VMCONFIG, &vmconfig);
>  		if (BUS_RESTRICT_CAP(vmcap) &&
>  		    (BUS_RESTRICT_CFG(vmconfig) == 0x1))
> -			busn_start = 128;
> +			vmd->busn_start = 128;
>  	}
>  
>  	res = &vmd->dev->resource[VMD_CFGBAR];
>  	vmd->resources[0] = (struct resource) {
>  		.name  = "VMD CFGBAR",
> -		.start = busn_start,
> -		.end   = busn_start + (resource_size(res) >> 20) - 1,
> +		.start = vmd->busn_start,
> +		.end   = vmd->busn_start + (resource_size(res) >> 20) - 1,
>  		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
>  	};
>  
> @@ -705,8 +707,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
>  	pci_add_resource_offset(&resources, &vmd->resources[2], offset[1]);
>  
> -	vmd->bus = pci_create_root_bus(&vmd->dev->dev, busn_start, &vmd_ops,
> -				       sd, &resources);
> +	vmd->bus = pci_create_root_bus(&vmd->dev->dev, vmd->busn_start,
> +				       &vmd_ops, sd, &resources);
>  	if (!vmd->bus) {
>  		pci_free_resource_list(&resources);
>  		irq_domain_remove(vmd->irq_domain);
> -- 
> 1.8.3.1
> 

Re: [PATCH] PCI/VMD: Fix config addressing with bus offsets

[Derrick, Jonathan]

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

On Fri, 2019-06-21 at 15:28 +0100, Lorenzo Pieralisi wrote:
> [dropped CC stable]
> 
> On Tue, Jun 11, 2019 at 03:15:38PM -0600, Jon Derrick wrote:
> > VMD config space addressing relies on mapping the BDF of the target into
> > the VMD config bar. When using bus number offsets to number the VMD
> > domain, the offset needs to be ignored in order to correctly map devices
> > to their config space.
> > 
> > Fixes: 2a5a9c9a20f9 ("PCI: vmd: Add offset to bus numbers if necessary")
> > Cc: <stable@vger.kernel.org> # v4.19
> > Cc: <stable@vger.kernel.org> # v4.18
> 
> Hi Jon,
> 
> that's not how stable should be handled. You should always start
> by fixing mainline and if there are backports to be fixed too you
> should add patch dependencies in the CC area, see:
> 
> Documentation/process/stable-kernel-rules.rst
> 
> Never add stable to the CC list in the email header, only in the
> commit log.
> 
> When your patch hits mainline it will trickle back into stable,
> if you specified dependencies as described above there is nothing
> to do.
> 
> Thanks,
> Lorenzo
> 
Hi Lorenzo,

Thanks for correcting me here. It's not my normal flow so I apologize
for getting this so egregiously incorrect.


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]

Re: [PATCH] PCI/VMD: Fix config addressing with bus offsets

[Derrick, Jonathan]

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]

On Fri, 2019-06-21 at 15:28 +0100, Lorenzo Pieralisi wrote:
> [dropped CC stable]
> 
> On Tue, Jun 11, 2019 at 03:15:38PM -0600, Jon Derrick wrote:
> > VMD config space addressing relies on mapping the BDF of the target into
> > the VMD config bar. When using bus number offsets to number the VMD
> > domain, the offset needs to be ignored in order to correctly map devices
> > to their config space.
> > 
> > Fixes: 2a5a9c9a20f9 ("PCI: vmd: Add offset to bus numbers if necessary")
> > Cc: <stable@vger.kernel.org> # v4.19
> > Cc: <stable@vger.kernel.org> # v4.18
> 
> Hi Jon,
> 
> that's not how stable should be handled. You should always start
> by fixing mainline and if there are backports to be fixed too you
> should add patch dependencies in the CC area, see:
> 
> Documentation/process/stable-kernel-rules.rst
> 
> Never add stable to the CC list in the email header, only in the
> commit log.
> 
> When your patch hits mainline it will trickle back into stable,
> if you specified dependencies as described above there is nothing
> to do.
> 
> Thanks,
> Lorenzo
> 

Besides the stable issue, can we get this into 5.3?

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]

Re: [PATCH] PCI/VMD: Fix config addressing with bus offsets

[Lorenzo Pieralisi]

On Mon, Jul 22, 2019 at 04:02:18PM +0000, Derrick, Jonathan wrote:
> On Fri, 2019-06-21 at 15:28 +0100, Lorenzo Pieralisi wrote:
> > [dropped CC stable]
> > 
> > On Tue, Jun 11, 2019 at 03:15:38PM -0600, Jon Derrick wrote:
> > > VMD config space addressing relies on mapping the BDF of the target into
> > > the VMD config bar. When using bus number offsets to number the VMD
> > > domain, the offset needs to be ignored in order to correctly map devices
> > > to their config space.
> > > 
> > > Fixes: 2a5a9c9a20f9 ("PCI: vmd: Add offset to bus numbers if necessary")
> > > Cc: <stable@vger.kernel.org> # v4.19
> > > Cc: <stable@vger.kernel.org> # v4.18
> > 
> > Hi Jon,
> > 
> > that's not how stable should be handled. You should always start
> > by fixing mainline and if there are backports to be fixed too you
> > should add patch dependencies in the CC area, see:
> > 
> > Documentation/process/stable-kernel-rules.rst
> > 
> > Never add stable to the CC list in the email header, only in the
> > commit log.
> > 
> > When your patch hits mainline it will trickle back into stable,
> > if you specified dependencies as described above there is nothing
> > to do.
> > 
> > Thanks,
> > Lorenzo
> > 
> 
> Besides the stable issue, can we get this into 5.3?

Usually we send fixes at -rc for patches that were merged in the
previous merge window; this fix is not one of those so I think
we will send it for v5.4 unless it is very urgent.

We should still update stable info in the log appropriately
before queuing it.

Thanks,
Lorenzo

Re: [PATCH] PCI/VMD: Fix config addressing with bus offsets

[Derrick, Jonathan]

[-- Attachment #1: Type: text/plain, Size: 1879 bytes --]

On Tue, 2019-07-23 at 10:32 +0100, Lorenzo Pieralisi wrote:
> On Mon, Jul 22, 2019 at 04:02:18PM +0000, Derrick, Jonathan wrote:
> > On Fri, 2019-06-21 at 15:28 +0100, Lorenzo Pieralisi wrote:
> > > [dropped CC stable]
> > > 
> > > On Tue, Jun 11, 2019 at 03:15:38PM -0600, Jon Derrick wrote:
> > > > VMD config space addressing relies on mapping the BDF of the target into
> > > > the VMD config bar. When using bus number offsets to number the VMD
> > > > domain, the offset needs to be ignored in order to correctly map devices
> > > > to their config space.
> > > > 
> > > > Fixes: 2a5a9c9a20f9 ("PCI: vmd: Add offset to bus numbers if necessary")
> > > > Cc: <stable@vger.kernel.org> # v4.19
> > > > Cc: <stable@vger.kernel.org> # v4.18
> > > 
> > > Hi Jon,
> > > 
> > > that's not how stable should be handled. You should always start
> > > by fixing mainline and if there are backports to be fixed too you
> > > should add patch dependencies in the CC area, see:
> > > 
> > > Documentation/process/stable-kernel-rules.rst
> > > 
> > > Never add stable to the CC list in the email header, only in the
> > > commit log.
> > > 
> > > When your patch hits mainline it will trickle back into stable,
> > > if you specified dependencies as described above there is nothing
> > > to do.
> > > 
> > > Thanks,
> > > Lorenzo
> > > 
> > 
> > Besides the stable issue, can we get this into 5.3?
> 
> Usually we send fixes at -rc for patches that were merged in the
> previous merge window; this fix is not one of those so I think
> we will send it for v5.4 unless it is very urgent.
> 
> We should still update stable info in the log appropriately
> before queuing it.
> 
> Thanks,
> Lorenzo


Sure. Had assumed it would be queued for 5.3 as it was submitted in 5.2
I will resubmit during 5.4 merge window and deal with stables later.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]

Re: [PATCH] PCI/VMD: Fix config addressing with bus offsets

[Lorenzo Pieralisi]

On Tue, Jul 23, 2019 at 03:12:51PM +0000, Derrick, Jonathan wrote:

[...]

> > > Besides the stable issue, can we get this into 5.3?
> > 
> > Usually we send fixes at -rc for patches that were merged in the
> > previous merge window; this fix is not one of those so I think
> > we will send it for v5.4 unless it is very urgent.
> > 
> > We should still update stable info in the log appropriately
> > before queuing it.
> > 
> > Thanks,
> > Lorenzo
> 
> 
> Sure. Had assumed it would be queued for 5.3 as it was submitted in 5.2
> I will resubmit during 5.4 merge window and deal with stables later.

It should not be that complicated to add stable tags (possibly
with commit dependencies on stable kernels where the patch
does not apply and require additional patches to be pulled),
there is plenty of time to sort this out.

Apologies for not picking it up for v5.3 - I should have asked
you to just update the stable tag so that we could merge it.

Lorenzo

Re: [PATCH] PCI/VMD: Fix config addressing with bus offsets

[Derrick, Jonathan]

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

On Wed, 2019-07-24 at 12:11 +0100, Lorenzo Pieralisi wrote:
> On Tue, Jul 23, 2019 at 03:12:51PM +0000, Derrick, Jonathan wrote:
> 
> [...]
> 
> > > > Besides the stable issue, can we get this into 5.3?
> > > 
> > > Usually we send fixes at -rc for patches that were merged in the
> > > previous merge window; this fix is not one of those so I think
> > > we will send it for v5.4 unless it is very urgent.
> > > 
> > > We should still update stable info in the log appropriately
> > > before queuing it.
> > > 
> > > Thanks,
> > > Lorenzo
> > 
> > Sure. Had assumed it would be queued for 5.3 as it was submitted in 5.2
> > I will resubmit during 5.4 merge window and deal with stables later.
> 
> It should not be that complicated to add stable tags (possibly
> with commit dependencies on stable kernels where the patch
> does not apply and require additional patches to be pulled),
> there is plenty of time to sort this out.
> 
> Apologies for not picking it up for v5.3 - I should have asked
> you to just update the stable tag so that we could merge it.
> 
> Lorenzo

No problem. I have another fix I need to include so I will do that
during 5.4 merge window

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]

[PATCH] PCI/VMD: Fix config addressing with bus offsets

[Jon Derrick]

VMD config space addressing relies on mapping the BDF of the target into
the VMD config bar. When using bus number offsets to number the VMD
domain, the offset needs to be ignored in order to correctly map devices
to their config space.

Fixes: 2a5a9c9a20f9 ("PCI: vmd: Add offset to bus numbers if necessary")
Cc: <stable@vger.kernel.org> # v4.18+
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/controller/vmd.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 999a5509e57e..6c80b9098dd0 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -94,6 +94,7 @@ struct vmd_dev {
 	struct resource		resources[3];
 	struct irq_domain	*irq_domain;
 	struct pci_bus		*bus;
+	u8			busn_start;
 
 	struct dma_map_ops	dma_ops;
 	struct dma_domain	dma_domain;
@@ -440,7 +441,8 @@ static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
 				  unsigned int devfn, int reg, int len)
 {
 	char __iomem *addr = vmd->cfgbar +
-			     (bus->number << 20) + (devfn << 12) + reg;
+			     ((bus->number - vmd->busn_start) << 20) +
+			     (devfn << 12) + reg;
 
 	if ((addr - vmd->cfgbar) + len >=
 	    resource_size(&vmd->dev->resource[VMD_CFGBAR]))
@@ -563,7 +565,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	unsigned long flags;
 	LIST_HEAD(resources);
 	resource_size_t offset[2] = {0};
-	resource_size_t membar2_offset = 0x2000, busn_start = 0;
+	resource_size_t membar2_offset = 0x2000;
 	struct pci_bus *child;
 
 	/*
@@ -606,14 +608,14 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 		pci_read_config_dword(vmd->dev, PCI_REG_VMCONFIG, &vmconfig);
 		if (BUS_RESTRICT_CAP(vmcap) &&
 		    (BUS_RESTRICT_CFG(vmconfig) == 0x1))
-			busn_start = 128;
+			vmd->busn_start = 128;
 	}
 
 	res = &vmd->dev->resource[VMD_CFGBAR];
 	vmd->resources[0] = (struct resource) {
 		.name  = "VMD CFGBAR",
-		.start = busn_start,
-		.end   = busn_start + (resource_size(res) >> 20) - 1,
+		.start = vmd->busn_start,
+		.end   = vmd->busn_start + (resource_size(res) >> 20) - 1,
 		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
 	};
 
@@ -681,8 +683,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
 	pci_add_resource_offset(&resources, &vmd->resources[2], offset[1]);
 
-	vmd->bus = pci_create_root_bus(&vmd->dev->dev, busn_start, &vmd_ops,
-				       sd, &resources);
+	vmd->bus = pci_create_root_bus(&vmd->dev->dev, vmd->busn_start,
+				       &vmd_ops, sd, &resources);
 	if (!vmd->bus) {
 		pci_free_resource_list(&resources);
 		irq_domain_remove(vmd->irq_domain);
-- 
2.20.1

Re: [PATCH] PCI/VMD: Fix config addressing with bus offsets

[Derrick, Jonathan]

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

On Mon, 2019-06-10 at 15:18 +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: 2a5a9c9a20f9 PCI: vmd: Add offset to bus numbers if necessary.
> 
> The bot has tested the following trees: v5.1.7, v4.19.48.
> 
> v5.1.7: Build OK!
> v4.19.48: Failed to apply! Possible dependencies:
>     0294951030eb ("PCI/VMD: Configure MPS settings before adding devices")
> 
> 
> How should we proceed with this patch?
> 
> --
> Thanks,
> Sasha

Thanks Sasha-bot,

I'll resend for kernels without 0294951030eb

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]