| Message ID | 20190611211538.29151-1-jonathan.derrick@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/VMD: Fix config addressing with bus offsets | expand |
[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 >
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.
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?
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
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.
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
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
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);
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(-)