[PATCH v2] PCI: vmd: Offset Client VMD MSI/X vectors

[PATCH v2] PCI: vmd: Offset Client VMD MSI/X vectors

[Jon Derrick]

Client VMD platforms have a software-triggered MSI/X vector 0 that will
not forward hardware-remapped MSI/X vectors from the sub-device domain.
This causes an issue with VMD platforms that use AHCI behind VMD and
have a single MSI/X vector mapping to vector 0. Add an MSI/X vector
offset for these platforms.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/controller/vmd.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index f69ef8c89f72..f8195bad79d1 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/X 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];
+		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

Re: [PATCH v2] PCI: vmd: Offset Client VMD MSI/X vectors

[Keith Busch]

On Mon, Sep 14, 2020 at 01:32:55PM -0400, Jon Derrick wrote:
> Client VMD platforms have a software-triggered MSI/X vector 0 that will
> not forward hardware-remapped MSI/X vectors from the sub-device domain.
> This causes an issue with VMD platforms that use AHCI behind VMD and
> have a single MSI/X vector mapping to vector 0. Add an MSI/X vector
> offset for these platforms.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/controller/vmd.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index f69ef8c89f72..f8195bad79d1 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/X 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)

This condition needs account for the vector offset, right?

  	if (vmd->msix_count == 1 + vmd->first_vec))

> -		return &vmd->irqs[0];
> +		return &vmd->irqs[vmd->first_vec];

Re: [PATCH v2] PCI: vmd: Offset Client VMD MSI/X vectors

[Bjorn Helgaas]

Hi Jon,

This refers to "MSI/X" (subject, commit log of this patch, as well as
other recent patches).  I don't know if that means what the spec calls
"MSI-X", or if you mean something like "MSI or MSI-X" (there is one
reference in vmd.c to "MSI/MSI-X").

I'd sort of rather avoid adding "MSI/X" to the PCI lexicon because of
this ambiguity.

If it's not important to the meaning or if it's clear from context
that "MSI" is a generic term for "either MSI or MSI-X", I'm ok with
using that.

Bjorn

On Mon, Sep 14, 2020 at 01:32:55PM -0400, Jon Derrick wrote:
> Client VMD platforms have a software-triggered MSI/X vector 0 that will
> not forward hardware-remapped MSI/X vectors from the sub-device domain.
> This causes an issue with VMD platforms that use AHCI behind VMD and
> have a single MSI/X vector mapping to vector 0. Add an MSI/X vector
> offset for these platforms.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/controller/vmd.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index f69ef8c89f72..f8195bad79d1 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/X remapping
> +	 */
> +	VMD_FEAT_OFFSET_FIRST_VECTOR		= (1 << 3),
...

Re: [PATCH v2] PCI: vmd: Offset Client VMD MSI/X vectors

[Derrick, Jonathan]

On Tue, 2020-09-15 at 03:42 +0900, Keith Busch wrote:
> On Mon, Sep 14, 2020 at 01:32:55PM -0400, Jon Derrick wrote:
> > Client VMD platforms have a software-triggered MSI/X vector 0 that will
> > not forward hardware-remapped MSI/X vectors from the sub-device domain.
> > This causes an issue with VMD platforms that use AHCI behind VMD and
> > have a single MSI/X vector mapping to vector 0. Add an MSI/X vector
> > offset for these platforms.
> > 
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > ---
> >  drivers/pci/controller/vmd.c | 35 +++++++++++++++++++++++++----------
> >  1 file changed, 25 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> > index f69ef8c89f72..f8195bad79d1 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/X 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)
> 
> This condition needs account for the vector offset, right?
> 
>   	if (vmd->msix_count == 1 + vmd->first_vec))

Looks like it, yes

Thank you

> 
> > -		return &vmd->irqs[0];
> > +		return &vmd->irqs[vmd->first_vec];