* [PATCH v3] PCI: vmd: Offset Client VMD MSI-X vectors
@ 2020-09-14 19:01 Jon Derrick
2020-10-05 11:23 ` Lorenzo Pieralisi
2020-10-22 6:24 ` Jian-Hong Pan
0 siblings, 2 replies; 4+ messages in thread
From: Jon Derrick @ 2020-09-14 19:01 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-pci, Keith Busch, Bjorn Helgaas, You-Sheng Yang, Jon Derrick
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
* "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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v3] PCI: vmd: Offset Client VMD MSI-X vectors
2020-09-14 19:01 [PATCH v3] PCI: vmd: Offset Client VMD MSI-X vectors Jon Derrick
@ 2020-10-05 11:23 ` Lorenzo Pieralisi
2020-10-05 14:39 ` Derrick, Jonathan
2020-10-22 6:24 ` Jian-Hong Pan
1 sibling, 1 reply; 4+ messages in thread
From: Lorenzo Pieralisi @ 2020-10-05 11:23 UTC (permalink / raw)
To: Jon Derrick; +Cc: linux-pci, Keith Busch, Bjorn Helgaas, You-Sheng Yang
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3] PCI: vmd: Offset Client VMD MSI-X vectors
2020-10-05 11:23 ` Lorenzo Pieralisi
@ 2020-10-05 14:39 ` Derrick, Jonathan
0 siblings, 0 replies; 4+ messages in thread
From: Derrick, Jonathan @ 2020-10-05 14:39 UTC (permalink / raw)
To: lorenzo.pieralisi; +Cc: kbusch, linux-pci, helgaas, vicamo.yang
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
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] PCI: vmd: Offset Client VMD MSI-X vectors
2020-09-14 19:01 [PATCH v3] PCI: vmd: Offset Client VMD MSI-X vectors Jon Derrick
2020-10-05 11:23 ` Lorenzo Pieralisi
@ 2020-10-22 6:24 ` Jian-Hong Pan
1 sibling, 0 replies; 4+ messages in thread
From: Jian-Hong Pan @ 2020-10-22 6:24 UTC (permalink / raw)
To: Lorenzo Pieralisi, Jon Derrick
Cc: Jian-Hong Pan, linux-pci, Keith Busch, Bjorn Helgaas,
You-Sheng Yang, linux
> 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>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-22 6:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 19:01 [PATCH v3] PCI: vmd: Offset Client VMD MSI-X vectors Jon Derrick
2020-10-05 11:23 ` Lorenzo Pieralisi
2020-10-05 14:39 ` Derrick, Jonathan
2020-10-22 6:24 ` Jian-Hong Pan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).