| Message ID | 20200904171325.64959-1-jonathan.derrick@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: vmd: Add AHCI to fast interrupt list | expand |
On 2020-09-05 01:13, Jon Derrick wrote: > Some platforms have an AHCI controller behind VMD. These platforms are > working correctly except for a case when the AHCI MSI is programmed with > VMD IRQ vector 0 (0xfee00000). When programmed with any other interrupt > (0xfeeNN000), the MSI is routed correctly and is handled by VMD. Placing > the AHCI MSI(s) in the fast-interrupt allow list solves the issue. > > This also requires that VMD allocate more than one MSI/X vector and > changes the minimum MSI/X vectors allocated to two. > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> Verified two platforms with such configuration. Thank you. You-Sheng Yang > --- > drivers/pci/controller/vmd.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > index f69ef8c89f72..d9c72613082a 100644 > --- a/drivers/pci/controller/vmd.c > +++ b/drivers/pci/controller/vmd.c > @@ -202,15 +202,13 @@ static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd, struct msi_desc *d > int i, best = 1; > unsigned long flags; > > - if (vmd->msix_count == 1) > - return &vmd->irqs[0]; > - > /* > - * White list for fast-interrupt handlers. All others will share the > + * Allow list for fast-interrupt handlers. All others will share the > * "slow" interrupt vector. > */ > switch (msi_desc_to_pci_dev(desc)->class) { > case PCI_CLASS_STORAGE_EXPRESS: > + case PCI_CLASS_STORAGE_SATA_AHCI: > break; > default: > return &vmd->irqs[0]; > @@ -657,7 +655,7 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) > if (vmd->msix_count < 0) > return -ENODEV; > > - vmd->msix_count = pci_alloc_irq_vectors(dev, 1, vmd->msix_count, > + vmd->msix_count = pci_alloc_irq_vectors(dev, 2, vmd->msix_count, > PCI_IRQ_MSIX); > if (vmd->msix_count < 0) > return vmd->msix_count; >
On Fri, Sep 04, 2020 at 11:13:25AM -0600, Jon Derrick wrote: > Some platforms have an AHCI controller behind VMD. These platforms are > working correctly except for a case when the AHCI MSI is programmed with > VMD IRQ vector 0 (0xfee00000). When programmed with any other interrupt > (0xfeeNN000), the MSI is routed correctly and is handled by VMD. Placing > the AHCI MSI(s) in the fast-interrupt allow list solves the issue. The only reason we have the fast vs. slow is because of the non-posted transactions from slow driver's interrupt handlers tanking performance of the nvme devices sharing the same vector. The microsemi switchtec was one of the first such devices identified that led to the current split in the VMD domain. AHCI's driver also has non-posted transactions in their interrupt handling, so you probably don't want those devices sharing vectors with your fast nvme devices.
On Fri, 2020-09-11 at 19:26 -0700, Keith Busch wrote: > On Fri, Sep 04, 2020 at 11:13:25AM -0600, Jon Derrick wrote: > > Some platforms have an AHCI controller behind VMD. These platforms are > > working correctly except for a case when the AHCI MSI is programmed with > > VMD IRQ vector 0 (0xfee00000). When programmed with any other interrupt > > (0xfeeNN000), the MSI is routed correctly and is handled by VMD. Placing > > the AHCI MSI(s) in the fast-interrupt allow list solves the issue. > > The only reason we have the fast vs. slow is because of the non-posted > transactions from slow driver's interrupt handlers tanking performance > of the nvme devices sharing the same vector. The microsemi switchtec was > one of the first such devices identified that led to the current > split in the VMD domain. AHCI's driver also has non-posted transactions > in their interrupt handling, so you probably don't want those devices > sharing vectors with your fast nvme devices. Good points and I think this would be simply resolved by offsetting the starting vector and avoiding vector 0 for all vectors. Thanks
>On 2020-09-05 01:13, Jon Derrick wrote: >> Some platforms have an AHCI controller behind VMD. These platforms are >> working correctly except for a case when the AHCI MSI is programmed with >> VMD IRQ vector 0 (0xfee00000). When programmed with any other interrupt >> (0xfeeNN000), the MSI is routed correctly and is handled by VMD. Placing >> the AHCI MSI(s) in the fast-interrupt allow list solves the issue. >> >> This also requires that VMD allocate more than one MSI/X vector and >> changes the minimum MSI/X vectors allocated to two. >> >> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > > Verified two platforms with such configuration. Thank you. > > You-Sheng Yang We have some laptops equipped with Tiger Lake chips and such configuration that hit the same issue, too. They all could be fixed by this patch. Thank you Tested-by: Jian-Hong Pan <jhp@endlessos.org>
On Mon, Oct 19, 2020 at 05:37:35PM +0800, Jian-Hong Pan wrote: > >On 2020-09-05 01:13, Jon Derrick wrote: > >> Some platforms have an AHCI controller behind VMD. These platforms are > >> working correctly except for a case when the AHCI MSI is programmed with > >> VMD IRQ vector 0 (0xfee00000). When programmed with any other interrupt > >> (0xfeeNN000), the MSI is routed correctly and is handled by VMD. Placing > >> the AHCI MSI(s) in the fast-interrupt allow list solves the issue. > >> > >> This also requires that VMD allocate more than one MSI/X vector and > >> changes the minimum MSI/X vectors allocated to two. > >> > >> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > > > > Verified two platforms with such configuration. Thank you. > > > > You-Sheng Yang > > We have some laptops equipped with Tiger Lake chips and such configuration that > hit the same issue, too. They all could be fixed by this patch. Thank you > > Tested-by: Jian-Hong Pan <jhp@endlessos.org> Please use the v3 instead: https://patchwork.kernel.org/project/linux-pci/patch/20200914190128.5114-1-jonathan.derrick@intel.com/ I'm not sure if there's anything remaining that's gating inclusion, though.
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index f69ef8c89f72..d9c72613082a 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -202,15 +202,13 @@ static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd, struct msi_desc *d int i, best = 1; unsigned long flags; - if (vmd->msix_count == 1) - return &vmd->irqs[0]; - /* - * White list for fast-interrupt handlers. All others will share the + * Allow list for fast-interrupt handlers. All others will share the * "slow" interrupt vector. */ switch (msi_desc_to_pci_dev(desc)->class) { case PCI_CLASS_STORAGE_EXPRESS: + case PCI_CLASS_STORAGE_SATA_AHCI: break; default: return &vmd->irqs[0]; @@ -657,7 +655,7 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) if (vmd->msix_count < 0) return -ENODEV; - vmd->msix_count = pci_alloc_irq_vectors(dev, 1, vmd->msix_count, + vmd->msix_count = pci_alloc_irq_vectors(dev, 2, vmd->msix_count, PCI_IRQ_MSIX); if (vmd->msix_count < 0) return vmd->msix_count;
Some platforms have an AHCI controller behind VMD. These platforms are working correctly except for a case when the AHCI MSI is programmed with VMD IRQ vector 0 (0xfee00000). When programmed with any other interrupt (0xfeeNN000), the MSI is routed correctly and is handled by VMD. Placing the AHCI MSI(s) in the fast-interrupt allow list solves the issue. This also requires that VMD allocate more than one MSI/X vector and changes the minimum MSI/X vectors allocated to two. Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> --- drivers/pci/controller/vmd.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)