Message ID | 20200914190128.5114-1-jonathan.derrick@intel.com |
---|---|
State | New |
Headers | show |
Series | [v3] PCI: vmd: Offset Client VMD MSI-X vectors | expand |
On Mon, Sep 14, 2020 at 03:01:28PM -0400, Jon Derrick wrote: > Client VMD platforms have a software-triggered MSI-X vector 0 that will > not forward hardware-remapped MSI from the sub-device domain. This > causes an issue with VMD platforms that use AHCI behind VMD and have a > single MSI-X vector remapped to VMD vector 0. Add a VMD MSI-X vector > offset for these platforms. > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > --- > v3: Commit MSI-X cleanup > vmd_next_irq check fix per Keith > > drivers/pci/controller/vmd.c | 37 +++++++++++++++++++++++++----------- > 1 file changed, 26 insertions(+), 11 deletions(-) > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > index f69ef8c89f72..10c0d20190e0 100644 > --- a/drivers/pci/controller/vmd.c > +++ b/drivers/pci/controller/vmd.c > @@ -53,6 +53,12 @@ enum vmd_features { > * vendor-specific capability space > */ > VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP = (1 << 2), > + > + /* > + * Device may use MSI-X vector 0 for software triggering and will not > + * be used for MSI remapping > + */ > + VMD_FEAT_OFFSET_FIRST_VECTOR = (1 << 3), > }; > > /* > @@ -104,6 +110,7 @@ struct vmd_dev { > struct irq_domain *irq_domain; > struct pci_bus *bus; > u8 busn_start; > + u8 first_vec; > }; > > static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus) > @@ -199,25 +206,26 @@ static irq_hw_number_t vmd_get_hwirq(struct msi_domain_info *info, > */ > static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd, struct msi_desc *desc) > { > - int i, best = 1; > unsigned long flags; > + int i, best; > > - if (vmd->msix_count == 1) > - return &vmd->irqs[0]; > + if (vmd->msix_count == 1 + vmd->first_vec) > + return &vmd->irqs[vmd->first_vec]; > > /* > - * White list for fast-interrupt handlers. All others will share the > + * Allow list for fast-interrupt handlers. All others will share the Is this comment change related to this patch logical change ? Other than that ready to merge it, please let me know. Thanks, Lorenzo > * "slow" interrupt vector. > */ > switch (msi_desc_to_pci_dev(desc)->class) { > case PCI_CLASS_STORAGE_EXPRESS: > break; > default: > - return &vmd->irqs[0]; > + return &vmd->irqs[vmd->first_vec]; > } > > raw_spin_lock_irqsave(&list_lock, flags); > - for (i = 1; i < vmd->msix_count; i++) > + best = vmd->first_vec + 1; > + for (i = best; i < vmd->msix_count; i++) > if (vmd->irqs[i].count < vmd->irqs[best].count) > best = i; > vmd->irqs[best].count++; > @@ -629,6 +637,7 @@ static irqreturn_t vmd_irq(int irq, void *data) > > static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) > { > + unsigned long features = (unsigned long) id->driver_data; > struct vmd_dev *vmd; > int i, err; > > @@ -653,12 +662,15 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) > dma_set_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32))) > return -ENODEV; > > + if (features & VMD_FEAT_OFFSET_FIRST_VECTOR) > + vmd->first_vec = 1; > + > vmd->msix_count = pci_msix_vec_count(dev); > if (vmd->msix_count < 0) > return -ENODEV; > > - vmd->msix_count = pci_alloc_irq_vectors(dev, 1, vmd->msix_count, > - PCI_IRQ_MSIX); > + vmd->msix_count = pci_alloc_irq_vectors(dev, vmd->first_vec + 1, > + vmd->msix_count, PCI_IRQ_MSIX); > if (vmd->msix_count < 0) > return vmd->msix_count; > > @@ -755,13 +767,16 @@ static const struct pci_device_id vmd_ids[] = { > VMD_FEAT_HAS_BUS_RESTRICTIONS,}, > {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f), > .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | > - VMD_FEAT_HAS_BUS_RESTRICTIONS,}, > + VMD_FEAT_HAS_BUS_RESTRICTIONS | > + VMD_FEAT_OFFSET_FIRST_VECTOR,}, > {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4c3d), > .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | > - VMD_FEAT_HAS_BUS_RESTRICTIONS,}, > + VMD_FEAT_HAS_BUS_RESTRICTIONS | > + VMD_FEAT_OFFSET_FIRST_VECTOR,}, > {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B), > .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | > - VMD_FEAT_HAS_BUS_RESTRICTIONS,}, > + VMD_FEAT_HAS_BUS_RESTRICTIONS | > + VMD_FEAT_OFFSET_FIRST_VECTOR,}, > {0,} > }; > MODULE_DEVICE_TABLE(pci, vmd_ids); > -- > 2.18.1 >
On Mon, 2020-10-05 at 12:23 +0100, Lorenzo Pieralisi wrote: > On Mon, Sep 14, 2020 at 03:01:28PM -0400, Jon Derrick wrote: > > Client VMD platforms have a software-triggered MSI-X vector 0 that will > > not forward hardware-remapped MSI from the sub-device domain. This > > causes an issue with VMD platforms that use AHCI behind VMD and have a > > single MSI-X vector remapped to VMD vector 0. Add a VMD MSI-X vector > > offset for these platforms. > > > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > > --- > > v3: Commit MSI-X cleanup > > vmd_next_irq check fix per Keith > > > > drivers/pci/controller/vmd.c | 37 +++++++++++++++++++++++++----------- > > 1 file changed, 26 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > > index f69ef8c89f72..10c0d20190e0 100644 > > --- a/drivers/pci/controller/vmd.c > > +++ b/drivers/pci/controller/vmd.c > > @@ -53,6 +53,12 @@ enum vmd_features { > > * vendor-specific capability space > > */ > > VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP = (1 << 2), > > + > > + /* > > + * Device may use MSI-X vector 0 for software triggering and will not > > + * be used for MSI remapping > > + */ > > + VMD_FEAT_OFFSET_FIRST_VECTOR = (1 << 3), > > }; > > > > /* > > @@ -104,6 +110,7 @@ struct vmd_dev { > > struct irq_domain *irq_domain; > > struct pci_bus *bus; > > u8 busn_start; > > + u8 first_vec; > > }; > > > > static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus) > > @@ -199,25 +206,26 @@ static irq_hw_number_t vmd_get_hwirq(struct msi_domain_info *info, > > */ > > static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd, struct msi_desc *desc) > > { > > - int i, best = 1; > > unsigned long flags; > > + int i, best; > > > > - if (vmd->msix_count == 1) > > - return &vmd->irqs[0]; > > + if (vmd->msix_count == 1 + vmd->first_vec) > > + return &vmd->irqs[vmd->first_vec]; > > > > /* > > - * White list for fast-interrupt handlers. All others will share the > > + * Allow list for fast-interrupt handlers. All others will share the > > Is this comment change related to this patch logical change ? It's for following conduct standards but not related > > Other than that ready to merge it, please let me know. > This will be necessary as the 'remapping disable' set had conflicts with X86 MSI cleanup. It's ready to merge if the remapping set is dropped. > Thanks, > Lorenzo > > > * "slow" interrupt vector. > > */ > > switch (msi_desc_to_pci_dev(desc)->class) { > > case PCI_CLASS_STORAGE_EXPRESS: > > break; > > default: > > - return &vmd->irqs[0]; > > + return &vmd->irqs[vmd->first_vec]; > > } > > > > raw_spin_lock_irqsave(&list_lock, flags); > > - for (i = 1; i < vmd->msix_count; i++) > > + best = vmd->first_vec + 1; > > + for (i = best; i < vmd->msix_count; i++) > > if (vmd->irqs[i].count < vmd->irqs[best].count) > > best = i; > > vmd->irqs[best].count++; > > @@ -629,6 +637,7 @@ static irqreturn_t vmd_irq(int irq, void *data) > > > > static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) > > { > > + unsigned long features = (unsigned long) id->driver_data; > > struct vmd_dev *vmd; > > int i, err; > > > > @@ -653,12 +662,15 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) > > dma_set_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32))) > > return -ENODEV; > > > > + if (features & VMD_FEAT_OFFSET_FIRST_VECTOR) > > + vmd->first_vec = 1; > > + > > vmd->msix_count = pci_msix_vec_count(dev); > > if (vmd->msix_count < 0) > > return -ENODEV; > > > > - vmd->msix_count = pci_alloc_irq_vectors(dev, 1, vmd->msix_count, > > - PCI_IRQ_MSIX); > > + vmd->msix_count = pci_alloc_irq_vectors(dev, vmd->first_vec + 1, > > + vmd->msix_count, PCI_IRQ_MSIX); > > if (vmd->msix_count < 0) > > return vmd->msix_count; > > > > @@ -755,13 +767,16 @@ static const struct pci_device_id vmd_ids[] = { > > VMD_FEAT_HAS_BUS_RESTRICTIONS,}, > > {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f), > > .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | > > - VMD_FEAT_HAS_BUS_RESTRICTIONS,}, > > + VMD_FEAT_HAS_BUS_RESTRICTIONS | > > + VMD_FEAT_OFFSET_FIRST_VECTOR,}, > > {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4c3d), > > .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | > > - VMD_FEAT_HAS_BUS_RESTRICTIONS,}, > > + VMD_FEAT_HAS_BUS_RESTRICTIONS | > > + VMD_FEAT_OFFSET_FIRST_VECTOR,}, > > {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B), > > .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | > > - VMD_FEAT_HAS_BUS_RESTRICTIONS,}, > > + VMD_FEAT_HAS_BUS_RESTRICTIONS | > > + VMD_FEAT_OFFSET_FIRST_VECTOR,}, > > {0,} > > }; > > MODULE_DEVICE_TABLE(pci, vmd_ids); > > -- > > 2.18.1 > >
> Client VMD platforms have a software-triggered MSI-X vector 0 that will > not forward hardware-remapped MSI from the sub-device domain. This > causes an issue with VMD platforms that use AHCI behind VMD and have a > single MSI-X vector remapped to VMD vector 0. Add a VMD MSI-X vector > offset for these platforms. > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> Tested this patch based on latest mainline kernel at commit f804b3159482 ("Merge tag 'linux-watchdog-5.10-rc1' of git://www.linux-watchdog.org/linux-watchdog"). We have some laptops equipped with Tiger Lake chips and such configuration that hit the same issue. They all could be fixed by this patch. Thank you Tested-by: Jian-Hong Pan <jhp@endlessos.org>
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index f69ef8c89f72..10c0d20190e0 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -53,6 +53,12 @@ enum vmd_features { * vendor-specific capability space */ VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP = (1 << 2), + + /* + * Device may use MSI-X vector 0 for software triggering and will not + * be used for MSI remapping + */ + VMD_FEAT_OFFSET_FIRST_VECTOR = (1 << 3), }; /* @@ -104,6 +110,7 @@ struct vmd_dev { struct irq_domain *irq_domain; struct pci_bus *bus; u8 busn_start; + u8 first_vec; }; static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus) @@ -199,25 +206,26 @@ static irq_hw_number_t vmd_get_hwirq(struct msi_domain_info *info, */ static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd, struct msi_desc *desc) { - int i, best = 1; unsigned long flags; + int i, best; - if (vmd->msix_count == 1) - return &vmd->irqs[0]; + if (vmd->msix_count == 1 + vmd->first_vec) + return &vmd->irqs[vmd->first_vec]; /* - * 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: break; default: - return &vmd->irqs[0]; + return &vmd->irqs[vmd->first_vec]; } raw_spin_lock_irqsave(&list_lock, flags); - for (i = 1; i < vmd->msix_count; i++) + best = vmd->first_vec + 1; + for (i = best; i < vmd->msix_count; i++) if (vmd->irqs[i].count < vmd->irqs[best].count) best = i; vmd->irqs[best].count++; @@ -629,6 +637,7 @@ static irqreturn_t vmd_irq(int irq, void *data) static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) { + unsigned long features = (unsigned long) id->driver_data; struct vmd_dev *vmd; int i, err; @@ -653,12 +662,15 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id) dma_set_mask_and_coherent(&dev->dev, DMA_BIT_MASK(32))) return -ENODEV; + if (features & VMD_FEAT_OFFSET_FIRST_VECTOR) + vmd->first_vec = 1; + vmd->msix_count = pci_msix_vec_count(dev); if (vmd->msix_count < 0) return -ENODEV; - vmd->msix_count = pci_alloc_irq_vectors(dev, 1, vmd->msix_count, - PCI_IRQ_MSIX); + vmd->msix_count = pci_alloc_irq_vectors(dev, vmd->first_vec + 1, + vmd->msix_count, PCI_IRQ_MSIX); if (vmd->msix_count < 0) return vmd->msix_count; @@ -755,13 +767,16 @@ static const struct pci_device_id vmd_ids[] = { VMD_FEAT_HAS_BUS_RESTRICTIONS,}, {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f), .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | - VMD_FEAT_HAS_BUS_RESTRICTIONS,}, + VMD_FEAT_HAS_BUS_RESTRICTIONS | + VMD_FEAT_OFFSET_FIRST_VECTOR,}, {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4c3d), .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | - VMD_FEAT_HAS_BUS_RESTRICTIONS,}, + VMD_FEAT_HAS_BUS_RESTRICTIONS | + VMD_FEAT_OFFSET_FIRST_VECTOR,}, {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B), .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | - VMD_FEAT_HAS_BUS_RESTRICTIONS,}, + VMD_FEAT_HAS_BUS_RESTRICTIONS | + VMD_FEAT_OFFSET_FIRST_VECTOR,}, {0,} }; MODULE_DEVICE_TABLE(pci, vmd_ids);
Client VMD platforms have a software-triggered MSI-X vector 0 that will not forward hardware-remapped MSI from the sub-device domain. This causes an issue with VMD platforms that use AHCI behind VMD and have a single MSI-X vector remapped to VMD vector 0. Add a VMD MSI-X vector offset for these platforms. Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> --- v3: Commit MSI-X cleanup vmd_next_irq check fix per Keith drivers/pci/controller/vmd.c | 37 +++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-)