* [PATCH 0/2] VMD fixes for v5.4
@ 2019-09-16 13:54 Jon Derrick
2019-09-16 13:54 ` [PATCH 1/2] PCI: vmd: Fix config addressing when using bus offsets Jon Derrick
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Jon Derrick @ 2019-09-16 13:54 UTC (permalink / raw)
To: Lorenzo Pieralisi; +Cc: Bjorn Helgaas, Keith Busch, linux-pci, linux-kernel
Hi Lorenzo, Bjorn, Keith,
Please consider the following patches for 5.4 inclusion.
These will apply to 5.2 stable. 4.19 has a few feature deps so I will instead
follow-up with a backport.
Jon Derrick (2):
PCI: vmd: Fix config addressing when using bus offsets
PCI: vmd: Fix shadow offsets to reflect spec changes
drivers/pci/controller/vmd.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] PCI: vmd: Fix config addressing when using bus offsets 2019-09-16 13:54 [PATCH 0/2] VMD fixes for v5.4 Jon Derrick @ 2019-09-16 13:54 ` Jon Derrick [not found] ` <20190918110800.18D9021920@mail.kernel.org> 2019-09-16 13:54 ` [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes Jon Derrick 2019-09-18 9:15 ` [PATCH 0/2] VMD fixes for v5.4 Lorenzo Pieralisi 2 siblings, 1 reply; 13+ messages in thread From: Jon Derrick @ 2019-09-16 13:54 UTC (permalink / raw) To: Lorenzo Pieralisi; +Cc: Bjorn Helgaas, Keith Busch, linux-pci, linux-kernel VMD maps child device config spaces to the VMD Config BAR linearly regardless of the starting bus offset. Because of this, the config address decode must ignore starting bus offsets when mapping the BDF to the config space address. Cc: stable@vger.kernel.org # v5.2+ Fixes: 2a5a9c9a ("PCI: vmd: Add offset to bus numbers if necessary") 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 4575e0c6dc4b..2e4da3f51d6b 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
[parent not found: <20190918110800.18D9021920@mail.kernel.org>]
* Re: [PATCH 1/2] PCI: vmd: Fix config addressing when using bus offsets [not found] ` <20190918110800.18D9021920@mail.kernel.org> @ 2019-09-18 14:06 ` Lorenzo Pieralisi 0 siblings, 0 replies; 13+ messages in thread From: Lorenzo Pieralisi @ 2019-09-18 14:06 UTC (permalink / raw) To: Sasha Levin; +Cc: Jon Derrick, Bjorn Helgaas, stable On Wed, Sep 18, 2019 at 11:07:59AM +0000, Sasha Levin wrote: > Hi, > > [This is an automated email] > > This commit has been processed because it contains a "Fixes:" tag, It also contains a Cc: stable tag :) > fixing commit: 2a5a9c9a20f9 PCI: vmd: Add offset to bus numbers if necessary. > > The bot has tested the following trees: v5.2.15, v4.19.73. > > v5.2.15: Build OK! > v4.19.73: Failed to apply! Possible dependencies: > 0294951030eb ("PCI/VMD: Configure MPS settings before adding devices") > > > NOTE: The patch will not be queued to stable trees until it is upstream. > > How should we proceed with this patch? You should take into account the Cc: stable tag requests, namely v5.2+. Thanks, Lorenzo ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes 2019-09-16 13:54 [PATCH 0/2] VMD fixes for v5.4 Jon Derrick 2019-09-16 13:54 ` [PATCH 1/2] PCI: vmd: Fix config addressing when using bus offsets Jon Derrick @ 2019-09-16 13:54 ` Jon Derrick 2019-09-17 10:41 ` Lorenzo Pieralisi 2019-09-18 9:15 ` [PATCH 0/2] VMD fixes for v5.4 Lorenzo Pieralisi 2 siblings, 1 reply; 13+ messages in thread From: Jon Derrick @ 2019-09-16 13:54 UTC (permalink / raw) To: Lorenzo Pieralisi; +Cc: Bjorn Helgaas, Keith Busch, linux-pci, linux-kernel The shadow offset scratchpad was moved to 0x2000-0x2010. Update the location to get the correct shadow offset. Cc: stable@vger.kernel.org # v5.2+ Fixes: 6788958e ("PCI: vmd: Assign membar addresses from shadow registers") Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> --- drivers/pci/controller/vmd.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index 2e4da3f51d6b..a35d3f3996d7 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -31,6 +31,9 @@ #define PCI_REG_VMLOCK 0x70 #define MB2_SHADOW_EN(vmlock) (vmlock & 0x2) +#define MB2_SHADOW_OFFSET 0x2000 +#define MB2_SHADOW_SIZE 16 + enum vmd_features { /* * Device may contain registers which hint the physical location of the @@ -578,7 +581,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) u32 vmlock; int ret; - membar2_offset = 0x2018; + membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE; ret = pci_read_config_dword(vmd->dev, PCI_REG_VMLOCK, &vmlock); if (ret || vmlock == ~0) return -ENODEV; @@ -590,9 +593,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) if (!membar2) return -ENOMEM; offset[0] = vmd->dev->resource[VMD_MEMBAR1].start - - readq(membar2 + 0x2008); + readq(membar2 + MB2_SHADOW_OFFSET); offset[1] = vmd->dev->resource[VMD_MEMBAR2].start - - readq(membar2 + 0x2010); + readq(membar2 + MB2_SHADOW_OFFSET + 8); pci_iounmap(vmd->dev, membar2); } } -- 2.20.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes 2019-09-16 13:54 ` [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes Jon Derrick @ 2019-09-17 10:41 ` Lorenzo Pieralisi 2019-09-17 13:55 ` Derrick, Jonathan 0 siblings, 1 reply; 13+ messages in thread From: Lorenzo Pieralisi @ 2019-09-17 10:41 UTC (permalink / raw) To: Jon Derrick; +Cc: Bjorn Helgaas, Keith Busch, linux-pci, linux-kernel On Mon, Sep 16, 2019 at 07:54:35AM -0600, Jon Derrick wrote: > The shadow offset scratchpad was moved to 0x2000-0x2010. Update the > location to get the correct shadow offset. Hi Jon, what does "was moved" mean ? Would this code still work on previous HW ? We must make sure that the address move is managed seamlessly by the kernel. For the time being I have to drop this fix and it is extremely tight to get it in v5.4, I can't send stable changes that we may have to revert. Thanks, Lorenzo > Cc: stable@vger.kernel.org # v5.2+ > Fixes: 6788958e ("PCI: vmd: Assign membar addresses from shadow registers") > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > --- > drivers/pci/controller/vmd.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > index 2e4da3f51d6b..a35d3f3996d7 100644 > --- a/drivers/pci/controller/vmd.c > +++ b/drivers/pci/controller/vmd.c > @@ -31,6 +31,9 @@ > #define PCI_REG_VMLOCK 0x70 > #define MB2_SHADOW_EN(vmlock) (vmlock & 0x2) > > +#define MB2_SHADOW_OFFSET 0x2000 > +#define MB2_SHADOW_SIZE 16 > + > enum vmd_features { > /* > * Device may contain registers which hint the physical location of the > @@ -578,7 +581,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > u32 vmlock; > int ret; > > - membar2_offset = 0x2018; > + membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE; > ret = pci_read_config_dword(vmd->dev, PCI_REG_VMLOCK, &vmlock); > if (ret || vmlock == ~0) > return -ENODEV; > @@ -590,9 +593,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > if (!membar2) > return -ENOMEM; > offset[0] = vmd->dev->resource[VMD_MEMBAR1].start - > - readq(membar2 + 0x2008); > + readq(membar2 + MB2_SHADOW_OFFSET); > offset[1] = vmd->dev->resource[VMD_MEMBAR2].start - > - readq(membar2 + 0x2010); > + readq(membar2 + MB2_SHADOW_OFFSET + 8); > pci_iounmap(vmd->dev, membar2); > } > } > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes 2019-09-17 10:41 ` Lorenzo Pieralisi @ 2019-09-17 13:55 ` Derrick, Jonathan 2019-09-17 14:05 ` Lorenzo Pieralisi 0 siblings, 1 reply; 13+ messages in thread From: Derrick, Jonathan @ 2019-09-17 13:55 UTC (permalink / raw) To: lorenzo.pieralisi; +Cc: linux-kernel, Busch, Keith, linux-pci, helgaas On Tue, 2019-09-17 at 11:41 +0100, Lorenzo Pieralisi wrote: > On Mon, Sep 16, 2019 at 07:54:35AM -0600, Jon Derrick wrote: > > The shadow offset scratchpad was moved to 0x2000-0x2010. Update the > > location to get the correct shadow offset. > > Hi Jon, > > what does "was moved" mean ? Would this code still work on previous HW ? > The previous code won't work on (not yet released) hw. Guests using the domain will see the wrong offset and enumerate the domain incorrectly. > We must make sure that the address move is managed seamlessly by the > kernel. If we need to avoid changing addressing within stable, then that's fine. But otherwise is there an issue merging with 5.4? > > For the time being I have to drop this fix and it is extremely > tight to get it in v5.4, I can't send stable changes that we may > have to revert. Aren't we in the beginning of the merge window? > > Thanks, > Lorenzo > > > Cc: stable@vger.kernel.org # v5.2+ > > Fixes: 6788958e ("PCI: vmd: Assign membar addresses from shadow registers") > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > > --- > > drivers/pci/controller/vmd.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > > index 2e4da3f51d6b..a35d3f3996d7 100644 > > --- a/drivers/pci/controller/vmd.c > > +++ b/drivers/pci/controller/vmd.c > > @@ -31,6 +31,9 @@ > > #define PCI_REG_VMLOCK 0x70 > > #define MB2_SHADOW_EN(vmlock) (vmlock & 0x2) > > > > +#define MB2_SHADOW_OFFSET 0x2000 > > +#define MB2_SHADOW_SIZE 16 > > + > > enum vmd_features { > > /* > > * Device may contain registers which hint the physical location of the > > @@ -578,7 +581,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > > u32 vmlock; > > int ret; > > > > - membar2_offset = 0x2018; > > + membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE; > > ret = pci_read_config_dword(vmd->dev, PCI_REG_VMLOCK, &vmlock); > > if (ret || vmlock == ~0) > > return -ENODEV; > > @@ -590,9 +593,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > > if (!membar2) > > return -ENOMEM; > > offset[0] = vmd->dev->resource[VMD_MEMBAR1].start - > > - readq(membar2 + 0x2008); > > + readq(membar2 + MB2_SHADOW_OFFSET); > > offset[1] = vmd->dev->resource[VMD_MEMBAR2].start - > > - readq(membar2 + 0x2010); > > + readq(membar2 + MB2_SHADOW_OFFSET + 8); > > pci_iounmap(vmd->dev, membar2); > > } > > } > > -- > > 2.20.1 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes 2019-09-17 13:55 ` Derrick, Jonathan @ 2019-09-17 14:05 ` Lorenzo Pieralisi 2019-09-17 14:45 ` Derrick, Jonathan 0 siblings, 1 reply; 13+ messages in thread From: Lorenzo Pieralisi @ 2019-09-17 14:05 UTC (permalink / raw) To: Derrick, Jonathan; +Cc: linux-kernel, Busch, Keith, linux-pci, helgaas On Tue, Sep 17, 2019 at 01:55:59PM +0000, Derrick, Jonathan wrote: > On Tue, 2019-09-17 at 11:41 +0100, Lorenzo Pieralisi wrote: > > On Mon, Sep 16, 2019 at 07:54:35AM -0600, Jon Derrick wrote: > > > The shadow offset scratchpad was moved to 0x2000-0x2010. Update the > > > location to get the correct shadow offset. > > > > Hi Jon, > > > > what does "was moved" mean ? Would this code still work on previous HW ? > > > The previous code won't work on (not yet released) hw. Guests using the > domain will see the wrong offset and enumerate the domain incorrectly. That's true also for new kernels on _current_ hardware, isn't it ? What I am saying is that there must be a way to detect the right offset from HW probing or firmware otherwise things will break one way of another. > > We must make sure that the address move is managed seamlessly by the > > kernel. > If we need to avoid changing addressing within stable, then that's > fine. But otherwise is there an issue merging with 5.4? See above. Would 5.4 with this patch applied work on systems where the offset is the same as the _current_ one without this patch applied ? > > For the time being I have to drop this fix and it is extremely > > tight to get it in v5.4, I can't send stable changes that we may > > have to revert. > Aren't we in the beginning of the merge window? Yes and that's the problem, patches for v5.4 should have already being queued a while ago, I do not know when Bjorn will send the PR for v5.4 but that's going to happen shortly, I am making an exception to squeeze these patches in since it is vmd only code but still it is very very tight. Thanks, Lorenzo > > Thanks, > > Lorenzo > > > > > Cc: stable@vger.kernel.org # v5.2+ > > > Fixes: 6788958e ("PCI: vmd: Assign membar addresses from shadow registers") > > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > > > --- > > > drivers/pci/controller/vmd.c | 9 ++++++--- > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > > > index 2e4da3f51d6b..a35d3f3996d7 100644 > > > --- a/drivers/pci/controller/vmd.c > > > +++ b/drivers/pci/controller/vmd.c > > > @@ -31,6 +31,9 @@ > > > #define PCI_REG_VMLOCK 0x70 > > > #define MB2_SHADOW_EN(vmlock) (vmlock & 0x2) > > > > > > +#define MB2_SHADOW_OFFSET 0x2000 > > > +#define MB2_SHADOW_SIZE 16 > > > + > > > enum vmd_features { > > > /* > > > * Device may contain registers which hint the physical location of the > > > @@ -578,7 +581,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > > > u32 vmlock; > > > int ret; > > > > > > - membar2_offset = 0x2018; > > > + membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE; > > > ret = pci_read_config_dword(vmd->dev, PCI_REG_VMLOCK, &vmlock); > > > if (ret || vmlock == ~0) > > > return -ENODEV; > > > @@ -590,9 +593,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > > > if (!membar2) > > > return -ENOMEM; > > > offset[0] = vmd->dev->resource[VMD_MEMBAR1].start - > > > - readq(membar2 + 0x2008); > > > + readq(membar2 + MB2_SHADOW_OFFSET); > > > offset[1] = vmd->dev->resource[VMD_MEMBAR2].start - > > > - readq(membar2 + 0x2010); > > > + readq(membar2 + MB2_SHADOW_OFFSET + 8); > > > pci_iounmap(vmd->dev, membar2); > > > } > > > } > > > -- > > > 2.20.1 > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes 2019-09-17 14:05 ` Lorenzo Pieralisi @ 2019-09-17 14:45 ` Derrick, Jonathan 2019-09-17 15:15 ` Lorenzo Pieralisi 0 siblings, 1 reply; 13+ messages in thread From: Derrick, Jonathan @ 2019-09-17 14:45 UTC (permalink / raw) To: lorenzo.pieralisi; +Cc: linux-kernel, Busch, Keith, linux-pci, helgaas On Tue, 2019-09-17 at 15:05 +0100, Lorenzo Pieralisi wrote: > On Tue, Sep 17, 2019 at 01:55:59PM +0000, Derrick, Jonathan wrote: > > On Tue, 2019-09-17 at 11:41 +0100, Lorenzo Pieralisi wrote: > > > On Mon, Sep 16, 2019 at 07:54:35AM -0600, Jon Derrick wrote: > > > > The shadow offset scratchpad was moved to 0x2000-0x2010. Update the > > > > location to get the correct shadow offset. > > > > > > Hi Jon, > > > > > > what does "was moved" mean ? Would this code still work on previous HW ? > > > > > The previous code won't work on (not yet released) hw. Guests using the > > domain will see the wrong offset and enumerate the domain incorrectly. > > That's true also for new kernels on _current_ hardware, isn't it ? > > What I am saying is that there must be a way to detect the right > offset from HW probing or firmware otherwise things will break > one way of another. > I think this is basically that, but the spec changed which register addresses contained the offset. The offset was still discoverable either way, but is now within 0x2000-0x2010, with 0x2010-0x2090 as oob interface. > > > We must make sure that the address move is managed seamlessly by the > > > kernel. > > If we need to avoid changing addressing within stable, then that's > > fine. But otherwise is there an issue merging with 5.4? > > See above. Would 5.4 with this patch applied work on systems where > the offset is the same as the _current_ one without this patch > applied ? I understand your concern, but these systems with wrong addressing won't exist because the hardware isn't released yet. In the future, the hardware will be released and users will inevitably load some unfixed kernel, and we would like to point to stable for the fix. > > > > For the time being I have to drop this fix and it is extremely > > > tight to get it in v5.4, I can't send stable changes that we may > > > have to revert. > > Aren't we in the beginning of the merge window? > > Yes and that's the problem, patches for v5.4 should have already > being queued a while ago, I do not know when Bjorn will send the > PR for v5.4 but that's going to happen shortly, I am making an > exception to squeeze these patches in since it is vmd only code > but still it is very very tight. > If you feel there's a risk, then I think it can be staged for v5.5. Hardware will not be available for some time. Best regards, Jon > Thanks, > Lorenzo > > > > Thanks, > > > Lorenzo > > > > > > > Cc: stable@vger.kernel.org # v5.2+ > > > > Fixes: 6788958e ("PCI: vmd: Assign membar addresses from shadow registers") > > > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > > > > --- > > > > drivers/pci/controller/vmd.c | 9 ++++++--- > > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > > > > index 2e4da3f51d6b..a35d3f3996d7 100644 > > > > --- a/drivers/pci/controller/vmd.c > > > > +++ b/drivers/pci/controller/vmd.c > > > > @@ -31,6 +31,9 @@ > > > > #define PCI_REG_VMLOCK 0x70 > > > > #define MB2_SHADOW_EN(vmlock) (vmlock & 0x2) > > > > > > > > +#define MB2_SHADOW_OFFSET 0x2000 > > > > +#define MB2_SHADOW_SIZE 16 > > > > + > > > > enum vmd_features { > > > > /* > > > > * Device may contain registers which hint the physical location of the > > > > @@ -578,7 +581,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > > > > u32 vmlock; > > > > int ret; > > > > > > > > - membar2_offset = 0x2018; > > > > + membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE; > > > > ret = pci_read_config_dword(vmd->dev, PCI_REG_VMLOCK, &vmlock); > > > > if (ret || vmlock == ~0) > > > > return -ENODEV; > > > > @@ -590,9 +593,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) > > > > if (!membar2) > > > > return -ENOMEM; > > > > offset[0] = vmd->dev->resource[VMD_MEMBAR1].start - > > > > - readq(membar2 + 0x2008); > > > > + readq(membar2 + MB2_SHADOW_OFFSET); > > > > offset[1] = vmd->dev->resource[VMD_MEMBAR2].start - > > > > - readq(membar2 + 0x2010); > > > > + readq(membar2 + MB2_SHADOW_OFFSET + 8); > > > > pci_iounmap(vmd->dev, membar2); > > > > } > > > > } > > > > -- > > > > 2.20.1 > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes 2019-09-17 14:45 ` Derrick, Jonathan @ 2019-09-17 15:15 ` Lorenzo Pieralisi 2019-09-17 15:51 ` Derrick, Jonathan 0 siblings, 1 reply; 13+ messages in thread From: Lorenzo Pieralisi @ 2019-09-17 15:15 UTC (permalink / raw) To: Derrick, Jonathan; +Cc: linux-kernel, Busch, Keith, linux-pci, helgaas On Tue, Sep 17, 2019 at 02:45:03PM +0000, Derrick, Jonathan wrote: > On Tue, 2019-09-17 at 15:05 +0100, Lorenzo Pieralisi wrote: > > On Tue, Sep 17, 2019 at 01:55:59PM +0000, Derrick, Jonathan wrote: > > > On Tue, 2019-09-17 at 11:41 +0100, Lorenzo Pieralisi wrote: > > > > On Mon, Sep 16, 2019 at 07:54:35AM -0600, Jon Derrick wrote: > > > > > The shadow offset scratchpad was moved to 0x2000-0x2010. Update the > > > > > location to get the correct shadow offset. > > > > > > > > Hi Jon, > > > > > > > > what does "was moved" mean ? Would this code still work on previous HW ? > > > > > > > The previous code won't work on (not yet released) hw. Guests using the > > > domain will see the wrong offset and enumerate the domain incorrectly. > > > > That's true also for new kernels on _current_ hardware, isn't it ? > > > > What I am saying is that there must be a way to detect the right > > offset from HW probing or firmware otherwise things will break > > one way of another. > > > I think this is basically that, but the spec changed which register > addresses contained the offset. The offset was still discoverable > either way, but is now within 0x2000-0x2010, with 0x2010-0x2090 as oob > interface. > > > > > > > We must make sure that the address move is managed seamlessly by the > > > > kernel. > > > If we need to avoid changing addressing within stable, then that's > > > fine. But otherwise is there an issue merging with 5.4? > > > > See above. Would 5.4 with this patch applied work on systems where > > the offset is the same as the _current_ one without this patch > > applied ? > I understand your concern, but these systems with wrong addressing > won't exist because the hardware isn't released yet. > > In the future, the hardware will be released and users will inevitably > load some unfixed kernel, and we would like to point to stable for the > fix. I am sorry for being blunt but I need to understand. If we apply this patch, are you telling me that the _current_ HW would fail ? I assume the current HW+kernel set-up is working, maybe that's what I got wrong. Reworded: on existing HW, is this patch fixing anything ? This patch when it hits the mainline will trickle into stable kernel unchanged. > > > > For the time being I have to drop this fix and it is extremely > > > > tight to get it in v5.4, I can't send stable changes that we may > > > > have to revert. > > > Aren't we in the beginning of the merge window? > > > > Yes and that's the problem, patches for v5.4 should have already > > being queued a while ago, I do not know when Bjorn will send the > > PR for v5.4 but that's going to happen shortly, I am making an > > exception to squeeze these patches in since it is vmd only code > > but still it is very very tight. > > > If you feel there's a risk, then I think it can be staged for v5.5. > Hardware will not be available for some time. I do not feel it is risky, I feel it would be much better if the scratchpad address could be detected at runtime through versioning of sorts either HW or firmware based. If we can't probe it inevitably we will have systems where kernels would break and that's something we should avoid. Lorenzo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes 2019-09-17 15:15 ` Lorenzo Pieralisi @ 2019-09-17 15:51 ` Derrick, Jonathan 2019-09-17 16:37 ` Lorenzo Pieralisi 0 siblings, 1 reply; 13+ messages in thread From: Derrick, Jonathan @ 2019-09-17 15:51 UTC (permalink / raw) To: lorenzo.pieralisi; +Cc: linux-kernel, Busch, Keith, linux-pci, helgaas On Tue, 2019-09-17 at 16:15 +0100, Lorenzo Pieralisi wrote: > On Tue, Sep 17, 2019 at 02:45:03PM +0000, Derrick, Jonathan wrote: > > On Tue, 2019-09-17 at 15:05 +0100, Lorenzo Pieralisi wrote: > > > On Tue, Sep 17, 2019 at 01:55:59PM +0000, Derrick, Jonathan wrote: > > > > On Tue, 2019-09-17 at 11:41 +0100, Lorenzo Pieralisi wrote: > > > > > On Mon, Sep 16, 2019 at 07:54:35AM -0600, Jon Derrick wrote: > > > > > > The shadow offset scratchpad was moved to 0x2000-0x2010. Update the > > > > > > location to get the correct shadow offset. > > > > > > > > > > Hi Jon, > > > > > > > > > > what does "was moved" mean ? Would this code still work on previous HW ? > > > > > > > > > The previous code won't work on (not yet released) hw. Guests using the > > > > domain will see the wrong offset and enumerate the domain incorrectly. > > > > > > That's true also for new kernels on _current_ hardware, isn't it ? > > > > > > What I am saying is that there must be a way to detect the right > > > offset from HW probing or firmware otherwise things will break > > > one way of another. > > > > > I think this is basically that, but the spec changed which register > > addresses contained the offset. The offset was still discoverable > > either way, but is now within 0x2000-0x2010, with 0x2010-0x2090 as oob > > interface. > > > > > > > > > > > We must make sure that the address move is managed seamlessly by the > > > > > kernel. > > > > If we need to avoid changing addressing within stable, then that's > > > > fine. But otherwise is there an issue merging with 5.4? > > > > > > See above. Would 5.4 with this patch applied work on systems where > > > the offset is the same as the _current_ one without this patch > > > applied ? > > I understand your concern, but these systems with wrong addressing > > won't exist because the hardware isn't released yet. > > > > In the future, the hardware will be released and users will inevitably > > load some unfixed kernel, and we would like to point to stable for the > > fix. > > I am sorry for being blunt but I need to understand. If we apply > this patch, are you telling me that the _current_ HW would fail ? > > I assume the current HW+kernel set-up is working, maybe that's > what I got wrong. > > Reworded: on existing HW, is this patch fixing anything ? > > This patch when it hits the mainline will trickle into stable > kernel unchanged. Sorry for the confusion. These changes only affect systems with VMD devices with 8086:28C0 device IDs, but these won't be production hardware for some time. Systems with VMD devices exist in the wild with 8086:201D device IDs. These don't support the guest passthrough mode and this code won't break anything with them. Additionally, patch 1/2 (bus numbering) only affects 8086:28C0. So on existing HW, these patches won't affect anything > > > > > > For the time being I have to drop this fix and it is extremely > > > > > tight to get it in v5.4, I can't send stable changes that we may > > > > > have to revert. > > > > Aren't we in the beginning of the merge window? > > > > > > Yes and that's the problem, patches for v5.4 should have already > > > being queued a while ago, I do not know when Bjorn will send the > > > PR for v5.4 but that's going to happen shortly, I am making an > > > exception to squeeze these patches in since it is vmd only code > > > but still it is very very tight. > > > > > If you feel there's a risk, then I think it can be staged for v5.5. > > Hardware will not be available for some time. > > I do not feel it is risky, I feel it would be much better if the > scratchpad address could be detected at runtime through versioning > of sorts either HW or firmware based. > > If we can't probe it inevitably we will have systems where kernels > would break and that's something we should avoid. > I agree that it might have been nicer if it were an ACPI/EFI var, but I think there were some complexities with teaching hypervisors to expose it to the guests for use when enumerating the domain from the passed- through endpoint. The method that exists in 8086:28C0 hardware divorces the firmware descriptors from the device so that the guest driver only needs to read the host-to-guest physical offset from the device itself. Best regards, Jon > Lorenzo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes 2019-09-17 15:51 ` Derrick, Jonathan @ 2019-09-17 16:37 ` Lorenzo Pieralisi 2019-09-17 18:00 ` Derrick, Jonathan 0 siblings, 1 reply; 13+ messages in thread From: Lorenzo Pieralisi @ 2019-09-17 16:37 UTC (permalink / raw) To: Derrick, Jonathan; +Cc: linux-kernel, Busch, Keith, linux-pci, helgaas On Tue, Sep 17, 2019 at 03:51:39PM +0000, Derrick, Jonathan wrote: [...] > Sorry for the confusion. > > These changes only affect systems with VMD devices with 8086:28C0 > device IDs, but these won't be production hardware for some time. > > Systems with VMD devices exist in the wild with 8086:201D device IDs. > These don't support the guest passthrough mode and this code won't > break anything with them. Additionally, patch 1/2 (bus numbering) only > affects 8086:28C0. > > So on existing HW, these patches won't affect anything It is me who created confusion, apologies. I read the code properly and I understand that both patches are fixes for HW that is still not available (and they can't create an issue with current kernel because HAS_MEMBAR_SHADOW and HAS_BUS_RESTRICTIONS features are not implemented on 8086:201D), we should take these patches and trickle them to stable kernels as soon as possible so that when HW _is_ available mainline and stable kernels are fixed. Correct ? Lorenzo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes 2019-09-17 16:37 ` Lorenzo Pieralisi @ 2019-09-17 18:00 ` Derrick, Jonathan 0 siblings, 0 replies; 13+ messages in thread From: Derrick, Jonathan @ 2019-09-17 18:00 UTC (permalink / raw) To: lorenzo.pieralisi; +Cc: linux-kernel, Busch, Keith, linux-pci, helgaas On Tue, 2019-09-17 at 17:37 +0100, Lorenzo Pieralisi wrote: > On Tue, Sep 17, 2019 at 03:51:39PM +0000, Derrick, Jonathan wrote: > > [...] > > > Sorry for the confusion. > > > > These changes only affect systems with VMD devices with 8086:28C0 > > device IDs, but these won't be production hardware for some time. > > > > Systems with VMD devices exist in the wild with 8086:201D device IDs. > > These don't support the guest passthrough mode and this code won't > > break anything with them. Additionally, patch 1/2 (bus numbering) only > > affects 8086:28C0. > > > > So on existing HW, these patches won't affect anything > > It is me who created confusion, apologies. I read the code properly and > I understand that both patches are fixes for HW that is still not > available (and they can't create an issue with current kernel because > HAS_MEMBAR_SHADOW and HAS_BUS_RESTRICTIONS features are not implemented > on 8086:201D), we should take these patches and trickle them to stable > kernels as soon as possible so that when HW _is_ available mainline and > stable kernels are fixed. > > Correct ? > > Lorenzo That's correct. It will apply to 5.2 stable but is missing a few deps for 4.19 that I wouldn't consider as qualifying as stable. I can backport to 4.19 fairly easily. Thanks, Jon ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] VMD fixes for v5.4 2019-09-16 13:54 [PATCH 0/2] VMD fixes for v5.4 Jon Derrick 2019-09-16 13:54 ` [PATCH 1/2] PCI: vmd: Fix config addressing when using bus offsets Jon Derrick 2019-09-16 13:54 ` [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes Jon Derrick @ 2019-09-18 9:15 ` Lorenzo Pieralisi 2 siblings, 0 replies; 13+ messages in thread From: Lorenzo Pieralisi @ 2019-09-18 9:15 UTC (permalink / raw) To: Jon Derrick; +Cc: Bjorn Helgaas, Keith Busch, linux-pci, linux-kernel On Mon, Sep 16, 2019 at 07:54:33AM -0600, Jon Derrick wrote: > Hi Lorenzo, Bjorn, Keith, > > Please consider the following patches for 5.4 inclusion. > > These will apply to 5.2 stable. 4.19 has a few feature deps so I will instead > follow-up with a backport. > > Jon Derrick (2): > PCI: vmd: Fix config addressing when using bus offsets > PCI: vmd: Fix shadow offsets to reflect spec changes > > drivers/pci/controller/vmd.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) I have pulled them into pci/vmd hopefully for v5.4. Thanks, Lorenzo ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-09-18 14:07 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-16 13:54 [PATCH 0/2] VMD fixes for v5.4 Jon Derrick
2019-09-16 13:54 ` [PATCH 1/2] PCI: vmd: Fix config addressing when using bus offsets Jon Derrick
[not found] ` <20190918110800.18D9021920@mail.kernel.org>
2019-09-18 14:06 ` Lorenzo Pieralisi
2019-09-16 13:54 ` [PATCH 2/2] PCI: vmd: Fix shadow offsets to reflect spec changes Jon Derrick
2019-09-17 10:41 ` Lorenzo Pieralisi
2019-09-17 13:55 ` Derrick, Jonathan
2019-09-17 14:05 ` Lorenzo Pieralisi
2019-09-17 14:45 ` Derrick, Jonathan
2019-09-17 15:15 ` Lorenzo Pieralisi
2019-09-17 15:51 ` Derrick, Jonathan
2019-09-17 16:37 ` Lorenzo Pieralisi
2019-09-17 18:00 ` Derrick, Jonathan
2019-09-18 9:15 ` [PATCH 0/2] VMD fixes for v5.4 Lorenzo Pieralisi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.