All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] VMD MSI Remapping Bypass
@ 2021-02-04 19:09 ` Jon Derrick
  0 siblings, 0 replies; 14+ messages in thread
From: Jon Derrick @ 2021-02-04 19:09 UTC (permalink / raw)
  To: linux-pci, iommu
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, David Woodhouse, Lu Baolu,
	Joerg Roedel, Nirmal Patel, Kapil Karkra, Will Deacon,
	Jon Derrick

The Intel Volume Management Device acts similar to a PCI-to-PCI bridge in that
it changes downstream devices' requester-ids to its own. As VMD supports PCIe
devices, it has its own MSI/X table and transmits child device MSI/X by
remapping child device MSI/X and handling like a demultiplexer.

Some newer VMD devices (Icelake Server) have an option to bypass the VMD MSI/X
remapping table. This allows for better performance scaling as the child device
MSI/X won't be limited by VMD's MSI/X count and IRQ handler.

V1->V2:
Updated for 5.12-next
Moved IRQ allocation and remapping enable/disable to a more logical location

V1 patches 1-4 were already merged
V1, 5/6: https://patchwork.kernel.org/project/linux-pci/patch/20200728194945.14126-6-jonathan.derrick@intel.com/
V1, 6/6: https://patchwork.kernel.org/project/linux-pci/patch/20200728194945.14126-7-jonathan.derrick@intel.com/

Jon Derrick (2):
  iommu/vt-d: Use Real PCI DMA device for IRTE
  PCI: vmd: Disable MSI/X remapping when possible

 drivers/iommu/intel/irq_remapping.c |  3 +-
 drivers/pci/controller/vmd.c        | 60 +++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 13 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 0/2] VMD MSI Remapping Bypass
@ 2021-02-04 19:09 ` Jon Derrick
  0 siblings, 0 replies; 14+ messages in thread
From: Jon Derrick @ 2021-02-04 19:09 UTC (permalink / raw)
  To: linux-pci, iommu
  Cc: Nirmal Patel, Will Deacon, Kapil Karkra, Bjorn Helgaas,
	David Woodhouse, Jon Derrick

The Intel Volume Management Device acts similar to a PCI-to-PCI bridge in that
it changes downstream devices' requester-ids to its own. As VMD supports PCIe
devices, it has its own MSI/X table and transmits child device MSI/X by
remapping child device MSI/X and handling like a demultiplexer.

Some newer VMD devices (Icelake Server) have an option to bypass the VMD MSI/X
remapping table. This allows for better performance scaling as the child device
MSI/X won't be limited by VMD's MSI/X count and IRQ handler.

V1->V2:
Updated for 5.12-next
Moved IRQ allocation and remapping enable/disable to a more logical location

V1 patches 1-4 were already merged
V1, 5/6: https://patchwork.kernel.org/project/linux-pci/patch/20200728194945.14126-6-jonathan.derrick@intel.com/
V1, 6/6: https://patchwork.kernel.org/project/linux-pci/patch/20200728194945.14126-7-jonathan.derrick@intel.com/

Jon Derrick (2):
  iommu/vt-d: Use Real PCI DMA device for IRTE
  PCI: vmd: Disable MSI/X remapping when possible

 drivers/iommu/intel/irq_remapping.c |  3 +-
 drivers/pci/controller/vmd.c        | 60 +++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 13 deletions(-)

-- 
2.27.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/2] iommu/vt-d: Use Real PCI DMA device for IRTE
  2021-02-04 19:09 ` Jon Derrick
@ 2021-02-04 19:09   ` Jon Derrick
  -1 siblings, 0 replies; 14+ messages in thread
From: Jon Derrick @ 2021-02-04 19:09 UTC (permalink / raw)
  To: linux-pci, iommu
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, David Woodhouse, Lu Baolu,
	Joerg Roedel, Nirmal Patel, Kapil Karkra, Will Deacon,
	Jon Derrick

VMD retransmits child device MSI/X with the VMD endpoint's requester-id.
In order to support direct interrupt remapping of VMD child devices,
ensure that the IRTE is programmed with the VMD endpoint's requester-id
using pci_real_dma_dev().

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/iommu/intel/irq_remapping.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/irq_remapping.c b/drivers/iommu/intel/irq_remapping.c
index 685200a5cff0..1939e070eec8 100644
--- a/drivers/iommu/intel/irq_remapping.c
+++ b/drivers/iommu/intel/irq_remapping.c
@@ -1276,7 +1276,8 @@ static void intel_irq_remapping_prepare_irte(struct intel_ir_data *data,
 		break;
 	case X86_IRQ_ALLOC_TYPE_PCI_MSI:
 	case X86_IRQ_ALLOC_TYPE_PCI_MSIX:
-		set_msi_sid(irte, msi_desc_to_pci_dev(info->desc));
+		set_msi_sid(irte,
+			    pci_real_dma_dev(msi_desc_to_pci_dev(info->desc)));
 		break;
 	default:
 		BUG_ON(1);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 1/2] iommu/vt-d: Use Real PCI DMA device for IRTE
@ 2021-02-04 19:09   ` Jon Derrick
  0 siblings, 0 replies; 14+ messages in thread
From: Jon Derrick @ 2021-02-04 19:09 UTC (permalink / raw)
  To: linux-pci, iommu
  Cc: Nirmal Patel, Will Deacon, Kapil Karkra, Bjorn Helgaas,
	David Woodhouse, Jon Derrick

VMD retransmits child device MSI/X with the VMD endpoint's requester-id.
In order to support direct interrupt remapping of VMD child devices,
ensure that the IRTE is programmed with the VMD endpoint's requester-id
using pci_real_dma_dev().

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/iommu/intel/irq_remapping.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/irq_remapping.c b/drivers/iommu/intel/irq_remapping.c
index 685200a5cff0..1939e070eec8 100644
--- a/drivers/iommu/intel/irq_remapping.c
+++ b/drivers/iommu/intel/irq_remapping.c
@@ -1276,7 +1276,8 @@ static void intel_irq_remapping_prepare_irte(struct intel_ir_data *data,
 		break;
 	case X86_IRQ_ALLOC_TYPE_PCI_MSI:
 	case X86_IRQ_ALLOC_TYPE_PCI_MSIX:
-		set_msi_sid(irte, msi_desc_to_pci_dev(info->desc));
+		set_msi_sid(irte,
+			    pci_real_dma_dev(msi_desc_to_pci_dev(info->desc)));
 		break;
 	default:
 		BUG_ON(1);
-- 
2.27.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/2] PCI: vmd: Disable MSI/X remapping when possible
  2021-02-04 19:09 ` Jon Derrick
@ 2021-02-04 19:09   ` Jon Derrick
  -1 siblings, 0 replies; 14+ messages in thread
From: Jon Derrick @ 2021-02-04 19:09 UTC (permalink / raw)
  To: linux-pci, iommu
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, David Woodhouse, Lu Baolu,
	Joerg Roedel, Nirmal Patel, Kapil Karkra, Will Deacon,
	Jon Derrick

VMD will retransmit child device MSI/X using its own MSI/X table and
requester-id. This limits the number of MSI/X available to the whole
child device domain to the number of VMD MSI/X interrupts.

Some VMD devices have a mode where this remapping can be disabled,
allowing child device interrupts to bypass processing with the VMD MSI/X
domain interrupt handler and going straight the child device interrupt
handler, allowing for better performance and scaling. The requester-id
still gets changed to the VMD endpoint's requester-id, and the interrupt
remapping handlers have been updated to properly set IRTE for child
device interrupts to the VMD endpoint's context.

Some VMD platforms have existing production BIOS which rely on MSI/X
remapping and won't explicitly program the MSI/X remapping bit. This
re-enables MSI/X remapping on unload.

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

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 5e80f28f0119..a319ce49645b 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -59,6 +59,13 @@ enum vmd_features {
 	 * be used for MSI remapping
 	 */
 	VMD_FEAT_OFFSET_FIRST_VECTOR		= (1 << 3),
+
+	/*
+	 * Device can bypass remapping MSI/X transactions into its MSI/X table,
+	 * avoding the requirement of a VMD MSI domain for child device
+	 * interrupt handling
+	 */
+	VMD_FEAT_BYPASS_MSI_REMAP		= (1 << 4),
 };
 
 /*
@@ -306,6 +313,15 @@ static struct msi_domain_info vmd_msi_domain_info = {
 	.chip		= &vmd_msi_controller,
 };
 
+static void vmd_enable_msi_remapping(struct vmd_dev *vmd, bool enable)
+{
+	u16 reg;
+
+	pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, &reg);
+	reg = enable ? (reg & ~0x2) : (reg | 0x2);
+	pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
+}
+
 static int vmd_create_irq_domain(struct vmd_dev *vmd)
 {
 	struct fwnode_handle *fn;
@@ -325,6 +341,13 @@ static int vmd_create_irq_domain(struct vmd_dev *vmd)
 
 static void vmd_remove_irq_domain(struct vmd_dev *vmd)
 {
+	/*
+	 * Some production BIOS won't enable remapping between soft reboots.
+	 * Ensure remapping is restored before unloading the driver.
+	 */
+	if (!vmd->msix_count)
+		vmd_enable_msi_remapping(vmd, true);
+
 	if (vmd->irq_domain) {
 		struct fwnode_handle *fn = vmd->irq_domain->fwnode;
 
@@ -679,15 +702,31 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 
 	sd->node = pcibus_to_node(vmd->dev->bus);
 
-	ret = vmd_create_irq_domain(vmd);
-	if (ret)
-		return ret;
-
 	/*
-	 * Override the irq domain bus token so the domain can be distinguished
-	 * from a regular PCI/MSI domain.
+	 * Currently MSI remapping must be enabled in guest passthrough mode
+	 * due to some missing interrupt remapping plumbing. This is probably
+	 * acceptable because the guest is usually CPU-limited and MSI
+	 * remapping doesn't become a performance bottleneck.
 	 */
-	irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
+	if (!(features & VMD_FEAT_BYPASS_MSI_REMAP) || offset[0] || offset[1]) {
+		ret = vmd_alloc_irqs(vmd);
+		if (ret)
+			return ret;
+
+		vmd_enable_msi_remapping(vmd, true);
+
+		ret = vmd_create_irq_domain(vmd);
+		if (ret)
+			return ret;
+
+		/*
+		 * Override the irq domain bus token so the domain can be
+		 * distinguished from a regular PCI/MSI domain.
+		 */
+		irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
+	} else {
+		vmd_enable_msi_remapping(vmd, false);
+	}
 
 	pci_add_resource(&resources, &vmd->resources[0]);
 	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
@@ -753,10 +792,6 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (features & VMD_FEAT_OFFSET_FIRST_VECTOR)
 		vmd->first_vec = 1;
 
-	err = vmd_alloc_irqs(vmd);
-	if (err)
-		return err;
-
 	spin_lock_init(&vmd->cfg_lock);
 	pci_set_drvdata(dev, vmd);
 	err = vmd_enable_domain(vmd, features);
@@ -825,7 +860,8 @@ static const struct pci_device_id vmd_ids[] = {
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
-				VMD_FEAT_HAS_BUS_RESTRICTIONS,},
+				VMD_FEAT_HAS_BUS_RESTRICTIONS |
+				VMD_FEAT_BYPASS_MSI_REMAP,},
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
 				VMD_FEAT_HAS_BUS_RESTRICTIONS |
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/2] PCI: vmd: Disable MSI/X remapping when possible
@ 2021-02-04 19:09   ` Jon Derrick
  0 siblings, 0 replies; 14+ messages in thread
From: Jon Derrick @ 2021-02-04 19:09 UTC (permalink / raw)
  To: linux-pci, iommu
  Cc: Nirmal Patel, Will Deacon, Kapil Karkra, Bjorn Helgaas,
	David Woodhouse, Jon Derrick

VMD will retransmit child device MSI/X using its own MSI/X table and
requester-id. This limits the number of MSI/X available to the whole
child device domain to the number of VMD MSI/X interrupts.

Some VMD devices have a mode where this remapping can be disabled,
allowing child device interrupts to bypass processing with the VMD MSI/X
domain interrupt handler and going straight the child device interrupt
handler, allowing for better performance and scaling. The requester-id
still gets changed to the VMD endpoint's requester-id, and the interrupt
remapping handlers have been updated to properly set IRTE for child
device interrupts to the VMD endpoint's context.

Some VMD platforms have existing production BIOS which rely on MSI/X
remapping and won't explicitly program the MSI/X remapping bit. This
re-enables MSI/X remapping on unload.

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

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 5e80f28f0119..a319ce49645b 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -59,6 +59,13 @@ enum vmd_features {
 	 * be used for MSI remapping
 	 */
 	VMD_FEAT_OFFSET_FIRST_VECTOR		= (1 << 3),
+
+	/*
+	 * Device can bypass remapping MSI/X transactions into its MSI/X table,
+	 * avoding the requirement of a VMD MSI domain for child device
+	 * interrupt handling
+	 */
+	VMD_FEAT_BYPASS_MSI_REMAP		= (1 << 4),
 };
 
 /*
@@ -306,6 +313,15 @@ static struct msi_domain_info vmd_msi_domain_info = {
 	.chip		= &vmd_msi_controller,
 };
 
+static void vmd_enable_msi_remapping(struct vmd_dev *vmd, bool enable)
+{
+	u16 reg;
+
+	pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, &reg);
+	reg = enable ? (reg & ~0x2) : (reg | 0x2);
+	pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
+}
+
 static int vmd_create_irq_domain(struct vmd_dev *vmd)
 {
 	struct fwnode_handle *fn;
@@ -325,6 +341,13 @@ static int vmd_create_irq_domain(struct vmd_dev *vmd)
 
 static void vmd_remove_irq_domain(struct vmd_dev *vmd)
 {
+	/*
+	 * Some production BIOS won't enable remapping between soft reboots.
+	 * Ensure remapping is restored before unloading the driver.
+	 */
+	if (!vmd->msix_count)
+		vmd_enable_msi_remapping(vmd, true);
+
 	if (vmd->irq_domain) {
 		struct fwnode_handle *fn = vmd->irq_domain->fwnode;
 
@@ -679,15 +702,31 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 
 	sd->node = pcibus_to_node(vmd->dev->bus);
 
-	ret = vmd_create_irq_domain(vmd);
-	if (ret)
-		return ret;
-
 	/*
-	 * Override the irq domain bus token so the domain can be distinguished
-	 * from a regular PCI/MSI domain.
+	 * Currently MSI remapping must be enabled in guest passthrough mode
+	 * due to some missing interrupt remapping plumbing. This is probably
+	 * acceptable because the guest is usually CPU-limited and MSI
+	 * remapping doesn't become a performance bottleneck.
 	 */
-	irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
+	if (!(features & VMD_FEAT_BYPASS_MSI_REMAP) || offset[0] || offset[1]) {
+		ret = vmd_alloc_irqs(vmd);
+		if (ret)
+			return ret;
+
+		vmd_enable_msi_remapping(vmd, true);
+
+		ret = vmd_create_irq_domain(vmd);
+		if (ret)
+			return ret;
+
+		/*
+		 * Override the irq domain bus token so the domain can be
+		 * distinguished from a regular PCI/MSI domain.
+		 */
+		irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
+	} else {
+		vmd_enable_msi_remapping(vmd, false);
+	}
 
 	pci_add_resource(&resources, &vmd->resources[0]);
 	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
@@ -753,10 +792,6 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (features & VMD_FEAT_OFFSET_FIRST_VECTOR)
 		vmd->first_vec = 1;
 
-	err = vmd_alloc_irqs(vmd);
-	if (err)
-		return err;
-
 	spin_lock_init(&vmd->cfg_lock);
 	pci_set_drvdata(dev, vmd);
 	err = vmd_enable_domain(vmd, features);
@@ -825,7 +860,8 @@ static const struct pci_device_id vmd_ids[] = {
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
-				VMD_FEAT_HAS_BUS_RESTRICTIONS,},
+				VMD_FEAT_HAS_BUS_RESTRICTIONS |
+				VMD_FEAT_BYPASS_MSI_REMAP,},
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
 				VMD_FEAT_HAS_BUS_RESTRICTIONS |
-- 
2.27.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] iommu/vt-d: Use Real PCI DMA device for IRTE
  2021-02-04 19:09   ` Jon Derrick
@ 2021-02-05  1:39     ` Lu Baolu
  -1 siblings, 0 replies; 14+ messages in thread
From: Lu Baolu @ 2021-02-05  1:39 UTC (permalink / raw)
  To: Jon Derrick, linux-pci, iommu
  Cc: baolu.lu, Bjorn Helgaas, Lorenzo Pieralisi, David Woodhouse,
	Joerg Roedel, Nirmal Patel, Kapil Karkra, Will Deacon

On 2/5/21 3:09 AM, Jon Derrick wrote:
> VMD retransmits child device MSI/X with the VMD endpoint's requester-id.
> In order to support direct interrupt remapping of VMD child devices,
> ensure that the IRTE is programmed with the VMD endpoint's requester-id
> using pci_real_dma_dev().
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>   drivers/iommu/intel/irq_remapping.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel/irq_remapping.c b/drivers/iommu/intel/irq_remapping.c
> index 685200a5cff0..1939e070eec8 100644
> --- a/drivers/iommu/intel/irq_remapping.c
> +++ b/drivers/iommu/intel/irq_remapping.c
> @@ -1276,7 +1276,8 @@ static void intel_irq_remapping_prepare_irte(struct intel_ir_data *data,
>   		break;
>   	case X86_IRQ_ALLOC_TYPE_PCI_MSI:
>   	case X86_IRQ_ALLOC_TYPE_PCI_MSIX:
> -		set_msi_sid(irte, msi_desc_to_pci_dev(info->desc));
> +		set_msi_sid(irte,
> +			    pci_real_dma_dev(msi_desc_to_pci_dev(info->desc)));
>   		break;
>   	default:
>   		BUG_ON(1);
> 

Acked-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] iommu/vt-d: Use Real PCI DMA device for IRTE
@ 2021-02-05  1:39     ` Lu Baolu
  0 siblings, 0 replies; 14+ messages in thread
From: Lu Baolu @ 2021-02-05  1:39 UTC (permalink / raw)
  To: Jon Derrick, linux-pci, iommu
  Cc: Nirmal Patel, Will Deacon, Kapil Karkra, Bjorn Helgaas, David Woodhouse

On 2/5/21 3:09 AM, Jon Derrick wrote:
> VMD retransmits child device MSI/X with the VMD endpoint's requester-id.
> In order to support direct interrupt remapping of VMD child devices,
> ensure that the IRTE is programmed with the VMD endpoint's requester-id
> using pci_real_dma_dev().
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>   drivers/iommu/intel/irq_remapping.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel/irq_remapping.c b/drivers/iommu/intel/irq_remapping.c
> index 685200a5cff0..1939e070eec8 100644
> --- a/drivers/iommu/intel/irq_remapping.c
> +++ b/drivers/iommu/intel/irq_remapping.c
> @@ -1276,7 +1276,8 @@ static void intel_irq_remapping_prepare_irte(struct intel_ir_data *data,
>   		break;
>   	case X86_IRQ_ALLOC_TYPE_PCI_MSI:
>   	case X86_IRQ_ALLOC_TYPE_PCI_MSIX:
> -		set_msi_sid(irte, msi_desc_to_pci_dev(info->desc));
> +		set_msi_sid(irte,
> +			    pci_real_dma_dev(msi_desc_to_pci_dev(info->desc)));
>   		break;
>   	default:
>   		BUG_ON(1);
> 

Acked-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/2] VMD MSI Remapping Bypass
  2021-02-04 19:09 ` Jon Derrick
@ 2021-02-05  8:45   ` Joerg Roedel
  -1 siblings, 0 replies; 14+ messages in thread
From: Joerg Roedel @ 2021-02-05  8:45 UTC (permalink / raw)
  To: Jon Derrick
  Cc: linux-pci, iommu, Bjorn Helgaas, Lorenzo Pieralisi,
	David Woodhouse, Lu Baolu, Nirmal Patel, Kapil Karkra,
	Will Deacon

On Thu, Feb 04, 2021 at 12:09:04PM -0700, Jon Derrick wrote:
> Jon Derrick (2):
>   iommu/vt-d: Use Real PCI DMA device for IRTE
>   PCI: vmd: Disable MSI/X remapping when possible
> 
>  drivers/iommu/intel/irq_remapping.c |  3 +-
>  drivers/pci/controller/vmd.c        | 60 +++++++++++++++++++++++------
>  2 files changed, 50 insertions(+), 13 deletions(-)

This probably goes via Bjorn's tree, so

	Acked-by: Joerg Roedel <jroedel@suse.de>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/2] VMD MSI Remapping Bypass
@ 2021-02-05  8:45   ` Joerg Roedel
  0 siblings, 0 replies; 14+ messages in thread
From: Joerg Roedel @ 2021-02-05  8:45 UTC (permalink / raw)
  To: Jon Derrick
  Cc: Nirmal Patel, Will Deacon, linux-pci, Kapil Karkra, iommu,
	Bjorn Helgaas, David Woodhouse

On Thu, Feb 04, 2021 at 12:09:04PM -0700, Jon Derrick wrote:
> Jon Derrick (2):
>   iommu/vt-d: Use Real PCI DMA device for IRTE
>   PCI: vmd: Disable MSI/X remapping when possible
> 
>  drivers/iommu/intel/irq_remapping.c |  3 +-
>  drivers/pci/controller/vmd.c        | 60 +++++++++++++++++++++++------
>  2 files changed, 50 insertions(+), 13 deletions(-)

This probably goes via Bjorn's tree, so

	Acked-by: Joerg Roedel <jroedel@suse.de>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] PCI: vmd: Disable MSI/X remapping when possible
  2021-02-04 19:09   ` Jon Derrick
@ 2021-02-05 21:57     ` Bjorn Helgaas
  -1 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2021-02-05 21:57 UTC (permalink / raw)
  To: Jon Derrick
  Cc: linux-pci, iommu, Lorenzo Pieralisi, David Woodhouse, Lu Baolu,
	Joerg Roedel, Nirmal Patel, Kapil Karkra, Will Deacon

On Thu, Feb 04, 2021 at 12:09:06PM -0700, Jon Derrick wrote:
> VMD will retransmit child device MSI/X using its own MSI/X table and
> requester-id. This limits the number of MSI/X available to the whole
> child device domain to the number of VMD MSI/X interrupts.
> 
> Some VMD devices have a mode where this remapping can be disabled,
> allowing child device interrupts to bypass processing with the VMD MSI/X
> domain interrupt handler and going straight the child device interrupt
> handler, allowing for better performance and scaling. The requester-id
> still gets changed to the VMD endpoint's requester-id, and the interrupt
> remapping handlers have been updated to properly set IRTE for child
> device interrupts to the VMD endpoint's context.
> 
> Some VMD platforms have existing production BIOS which rely on MSI/X
> remapping and won't explicitly program the MSI/X remapping bit. This
> re-enables MSI/X remapping on unload.

Trivial comments below.  Would you mind using "MSI-X" instead of
"MSI/X" so it matches the usage in the PCIe specs?  Several mentions
above (including subject) and below.

> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/controller/vmd.c | 60 ++++++++++++++++++++++++++++--------
>  1 file changed, 48 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 5e80f28f0119..a319ce49645b 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -59,6 +59,13 @@ enum vmd_features {
>  	 * be used for MSI remapping
>  	 */
>  	VMD_FEAT_OFFSET_FIRST_VECTOR		= (1 << 3),
> +
> +	/*
> +	 * Device can bypass remapping MSI/X transactions into its MSI/X table,
> +	 * avoding the requirement of a VMD MSI domain for child device

s/avoding/avoiding/

> +	 * interrupt handling

Maybe a period at the end of the sentence.

> +	 */
> +	VMD_FEAT_BYPASS_MSI_REMAP		= (1 << 4),
>  };
>  
>  /*
> @@ -306,6 +313,15 @@ static struct msi_domain_info vmd_msi_domain_info = {
>  	.chip		= &vmd_msi_controller,
>  };
>  
> +static void vmd_enable_msi_remapping(struct vmd_dev *vmd, bool enable)
> +{
> +	u16 reg;
> +
> +	pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, &reg);
> +	reg = enable ? (reg & ~0x2) : (reg | 0x2);

Would be nice to have a #define for 0x2.

> +	pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
> +}
> +
>  static int vmd_create_irq_domain(struct vmd_dev *vmd)
>  {
>  	struct fwnode_handle *fn;
> @@ -325,6 +341,13 @@ static int vmd_create_irq_domain(struct vmd_dev *vmd)
>  
>  static void vmd_remove_irq_domain(struct vmd_dev *vmd)
>  {
> +	/*
> +	 * Some production BIOS won't enable remapping between soft reboots.
> +	 * Ensure remapping is restored before unloading the driver.
> +	 */
> +	if (!vmd->msix_count)
> +		vmd_enable_msi_remapping(vmd, true);
> +
>  	if (vmd->irq_domain) {
>  		struct fwnode_handle *fn = vmd->irq_domain->fwnode;
>  
> @@ -679,15 +702,31 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  
>  	sd->node = pcibus_to_node(vmd->dev->bus);
>  
> -	ret = vmd_create_irq_domain(vmd);
> -	if (ret)
> -		return ret;
> -
>  	/*
> -	 * Override the irq domain bus token so the domain can be distinguished
> -	 * from a regular PCI/MSI domain.
> +	 * Currently MSI remapping must be enabled in guest passthrough mode
> +	 * due to some missing interrupt remapping plumbing. This is probably
> +	 * acceptable because the guest is usually CPU-limited and MSI
> +	 * remapping doesn't become a performance bottleneck.
>  	 */
> -	irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
> +	if (!(features & VMD_FEAT_BYPASS_MSI_REMAP) || offset[0] || offset[1]) {
> +		ret = vmd_alloc_irqs(vmd);
> +		if (ret)
> +			return ret;
> +
> +		vmd_enable_msi_remapping(vmd, true);
> +
> +		ret = vmd_create_irq_domain(vmd);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Override the irq domain bus token so the domain can be
> +		 * distinguished from a regular PCI/MSI domain.
> +		 */
> +		irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
> +	} else {
> +		vmd_enable_msi_remapping(vmd, false);
> +	}
>  
>  	pci_add_resource(&resources, &vmd->resources[0]);
>  	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
> @@ -753,10 +792,6 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	if (features & VMD_FEAT_OFFSET_FIRST_VECTOR)
>  		vmd->first_vec = 1;
>  
> -	err = vmd_alloc_irqs(vmd);
> -	if (err)
> -		return err;
> -
>  	spin_lock_init(&vmd->cfg_lock);
>  	pci_set_drvdata(dev, vmd);
>  	err = vmd_enable_domain(vmd, features);
> @@ -825,7 +860,8 @@ static const struct pci_device_id vmd_ids[] = {
>  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
>  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
>  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
> -				VMD_FEAT_HAS_BUS_RESTRICTIONS,},
> +				VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +				VMD_FEAT_BYPASS_MSI_REMAP,},
>  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
>  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
>  				VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] PCI: vmd: Disable MSI/X remapping when possible
@ 2021-02-05 21:57     ` Bjorn Helgaas
  0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2021-02-05 21:57 UTC (permalink / raw)
  To: Jon Derrick
  Cc: Nirmal Patel, Will Deacon, linux-pci, Kapil Karkra, iommu,
	David Woodhouse

On Thu, Feb 04, 2021 at 12:09:06PM -0700, Jon Derrick wrote:
> VMD will retransmit child device MSI/X using its own MSI/X table and
> requester-id. This limits the number of MSI/X available to the whole
> child device domain to the number of VMD MSI/X interrupts.
> 
> Some VMD devices have a mode where this remapping can be disabled,
> allowing child device interrupts to bypass processing with the VMD MSI/X
> domain interrupt handler and going straight the child device interrupt
> handler, allowing for better performance and scaling. The requester-id
> still gets changed to the VMD endpoint's requester-id, and the interrupt
> remapping handlers have been updated to properly set IRTE for child
> device interrupts to the VMD endpoint's context.
> 
> Some VMD platforms have existing production BIOS which rely on MSI/X
> remapping and won't explicitly program the MSI/X remapping bit. This
> re-enables MSI/X remapping on unload.

Trivial comments below.  Would you mind using "MSI-X" instead of
"MSI/X" so it matches the usage in the PCIe specs?  Several mentions
above (including subject) and below.

> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/pci/controller/vmd.c | 60 ++++++++++++++++++++++++++++--------
>  1 file changed, 48 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 5e80f28f0119..a319ce49645b 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -59,6 +59,13 @@ enum vmd_features {
>  	 * be used for MSI remapping
>  	 */
>  	VMD_FEAT_OFFSET_FIRST_VECTOR		= (1 << 3),
> +
> +	/*
> +	 * Device can bypass remapping MSI/X transactions into its MSI/X table,
> +	 * avoding the requirement of a VMD MSI domain for child device

s/avoding/avoiding/

> +	 * interrupt handling

Maybe a period at the end of the sentence.

> +	 */
> +	VMD_FEAT_BYPASS_MSI_REMAP		= (1 << 4),
>  };
>  
>  /*
> @@ -306,6 +313,15 @@ static struct msi_domain_info vmd_msi_domain_info = {
>  	.chip		= &vmd_msi_controller,
>  };
>  
> +static void vmd_enable_msi_remapping(struct vmd_dev *vmd, bool enable)
> +{
> +	u16 reg;
> +
> +	pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, &reg);
> +	reg = enable ? (reg & ~0x2) : (reg | 0x2);

Would be nice to have a #define for 0x2.

> +	pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
> +}
> +
>  static int vmd_create_irq_domain(struct vmd_dev *vmd)
>  {
>  	struct fwnode_handle *fn;
> @@ -325,6 +341,13 @@ static int vmd_create_irq_domain(struct vmd_dev *vmd)
>  
>  static void vmd_remove_irq_domain(struct vmd_dev *vmd)
>  {
> +	/*
> +	 * Some production BIOS won't enable remapping between soft reboots.
> +	 * Ensure remapping is restored before unloading the driver.
> +	 */
> +	if (!vmd->msix_count)
> +		vmd_enable_msi_remapping(vmd, true);
> +
>  	if (vmd->irq_domain) {
>  		struct fwnode_handle *fn = vmd->irq_domain->fwnode;
>  
> @@ -679,15 +702,31 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  
>  	sd->node = pcibus_to_node(vmd->dev->bus);
>  
> -	ret = vmd_create_irq_domain(vmd);
> -	if (ret)
> -		return ret;
> -
>  	/*
> -	 * Override the irq domain bus token so the domain can be distinguished
> -	 * from a regular PCI/MSI domain.
> +	 * Currently MSI remapping must be enabled in guest passthrough mode
> +	 * due to some missing interrupt remapping plumbing. This is probably
> +	 * acceptable because the guest is usually CPU-limited and MSI
> +	 * remapping doesn't become a performance bottleneck.
>  	 */
> -	irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
> +	if (!(features & VMD_FEAT_BYPASS_MSI_REMAP) || offset[0] || offset[1]) {
> +		ret = vmd_alloc_irqs(vmd);
> +		if (ret)
> +			return ret;
> +
> +		vmd_enable_msi_remapping(vmd, true);
> +
> +		ret = vmd_create_irq_domain(vmd);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Override the irq domain bus token so the domain can be
> +		 * distinguished from a regular PCI/MSI domain.
> +		 */
> +		irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
> +	} else {
> +		vmd_enable_msi_remapping(vmd, false);
> +	}
>  
>  	pci_add_resource(&resources, &vmd->resources[0]);
>  	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
> @@ -753,10 +792,6 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	if (features & VMD_FEAT_OFFSET_FIRST_VECTOR)
>  		vmd->first_vec = 1;
>  
> -	err = vmd_alloc_irqs(vmd);
> -	if (err)
> -		return err;
> -
>  	spin_lock_init(&vmd->cfg_lock);
>  	pci_set_drvdata(dev, vmd);
>  	err = vmd_enable_domain(vmd, features);
> @@ -825,7 +860,8 @@ static const struct pci_device_id vmd_ids[] = {
>  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
>  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
>  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
> -				VMD_FEAT_HAS_BUS_RESTRICTIONS,},
> +				VMD_FEAT_HAS_BUS_RESTRICTIONS |
> +				VMD_FEAT_BYPASS_MSI_REMAP,},
>  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
>  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
>  				VMD_FEAT_HAS_BUS_RESTRICTIONS |
> -- 
> 2.27.0
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] PCI: vmd: Disable MSI/X remapping when possible
  2021-02-05 21:57     ` Bjorn Helgaas
@ 2021-02-06  2:50       ` Derrick, Jonathan
  -1 siblings, 0 replies; 14+ messages in thread
From: Derrick, Jonathan @ 2021-02-06  2:50 UTC (permalink / raw)
  To: helgaas
  Cc: dwmw2, joro, Patel, Nirmal, baolu.lu, iommu, lorenzo.pieralisi,
	Karkra, Kapil, will, linux-pci

On Fri, 2021-02-05 at 15:57 -0600, Bjorn Helgaas wrote:
> On Thu, Feb 04, 2021 at 12:09:06PM -0700, Jon Derrick wrote:
> > VMD will retransmit child device MSI/X using its own MSI/X table and
> > requester-id. This limits the number of MSI/X available to the whole
> > child device domain to the number of VMD MSI/X interrupts.
> > 
> > Some VMD devices have a mode where this remapping can be disabled,
> > allowing child device interrupts to bypass processing with the VMD MSI/X
> > domain interrupt handler and going straight the child device interrupt
> > handler, allowing for better performance and scaling. The requester-id
> > still gets changed to the VMD endpoint's requester-id, and the interrupt
> > remapping handlers have been updated to properly set IRTE for child
> > device interrupts to the VMD endpoint's context.
> > 
> > Some VMD platforms have existing production BIOS which rely on MSI/X
> > remapping and won't explicitly program the MSI/X remapping bit. This
> > re-enables MSI/X remapping on unload.
> 
> Trivial comments below.  Would you mind using "MSI-X" instead of
> "MSI/X" so it matches the usage in the PCIe specs?  Several mentions
> above (including subject) and below.
Thanks Bjorn, will do

> 
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > ---
> >  drivers/pci/controller/vmd.c | 60 ++++++++++++++++++++++++++++--------
> >  1 file changed, 48 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> > index 5e80f28f0119..a319ce49645b 100644
> > --- a/drivers/pci/controller/vmd.c
> > +++ b/drivers/pci/controller/vmd.c
> > @@ -59,6 +59,13 @@ enum vmd_features {
> >  	 * be used for MSI remapping
> >  	 */
> >  	VMD_FEAT_OFFSET_FIRST_VECTOR		= (1 << 3),
> > +
> > +	/*
> > +	 * Device can bypass remapping MSI/X transactions into its MSI/X table,
> > +	 * avoding the requirement of a VMD MSI domain for child device
> 
> s/avoding/avoiding/
> 
> > +	 * interrupt handling
> 
> Maybe a period at the end of the sentence.
> 
> > +	 */
> > +	VMD_FEAT_BYPASS_MSI_REMAP		= (1 << 4),
> >  };
> >  
> >  /*
> > @@ -306,6 +313,15 @@ static struct msi_domain_info vmd_msi_domain_info = {
> >  	.chip		= &vmd_msi_controller,
> >  };
> >  
> > +static void vmd_enable_msi_remapping(struct vmd_dev *vmd, bool enable)
> > +{
> > +	u16 reg;
> > +
> > +	pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, &reg);
> > +	reg = enable ? (reg & ~0x2) : (reg | 0x2);
> 
> Would be nice to have a #define for 0x2.
> 
> > +	pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
> > +}
> > +
> >  static int vmd_create_irq_domain(struct vmd_dev *vmd)
> >  {
> >  	struct fwnode_handle *fn;
> > @@ -325,6 +341,13 @@ static int vmd_create_irq_domain(struct vmd_dev *vmd)
> >  
> >  static void vmd_remove_irq_domain(struct vmd_dev *vmd)
> >  {
> > +	/*
> > +	 * Some production BIOS won't enable remapping between soft reboots.
> > +	 * Ensure remapping is restored before unloading the driver.
> > +	 */
> > +	if (!vmd->msix_count)
> > +		vmd_enable_msi_remapping(vmd, true);
> > +
> >  	if (vmd->irq_domain) {
> >  		struct fwnode_handle *fn = vmd->irq_domain->fwnode;
> >  
> > @@ -679,15 +702,31 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
> >  
> >  	sd->node = pcibus_to_node(vmd->dev->bus);
> >  
> > -	ret = vmd_create_irq_domain(vmd);
> > -	if (ret)
> > -		return ret;
> > -
> >  	/*
> > -	 * Override the irq domain bus token so the domain can be distinguished
> > -	 * from a regular PCI/MSI domain.
> > +	 * Currently MSI remapping must be enabled in guest passthrough mode
> > +	 * due to some missing interrupt remapping plumbing. This is probably
> > +	 * acceptable because the guest is usually CPU-limited and MSI
> > +	 * remapping doesn't become a performance bottleneck.
> >  	 */
> > -	irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
> > +	if (!(features & VMD_FEAT_BYPASS_MSI_REMAP) || offset[0] || offset[1]) {
> > +		ret = vmd_alloc_irqs(vmd);
> > +		if (ret)
> > +			return ret;
> > +
> > +		vmd_enable_msi_remapping(vmd, true);
> > +
> > +		ret = vmd_create_irq_domain(vmd);
> > +		if (ret)
> > +			return ret;
> > +
> > +		/*
> > +		 * Override the irq domain bus token so the domain can be
> > +		 * distinguished from a regular PCI/MSI domain.
> > +		 */
> > +		irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
> > +	} else {
> > +		vmd_enable_msi_remapping(vmd, false);
> > +	}
> >  
> >  	pci_add_resource(&resources, &vmd->resources[0]);
> >  	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
> > @@ -753,10 +792,6 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >  	if (features & VMD_FEAT_OFFSET_FIRST_VECTOR)
> >  		vmd->first_vec = 1;
> >  
> > -	err = vmd_alloc_irqs(vmd);
> > -	if (err)
> > -		return err;
> > -
> >  	spin_lock_init(&vmd->cfg_lock);
> >  	pci_set_drvdata(dev, vmd);
> >  	err = vmd_enable_domain(vmd, features);
> > @@ -825,7 +860,8 @@ static const struct pci_device_id vmd_ids[] = {
> >  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
> >  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
> >  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
> > -				VMD_FEAT_HAS_BUS_RESTRICTIONS,},
> > +				VMD_FEAT_HAS_BUS_RESTRICTIONS |
> > +				VMD_FEAT_BYPASS_MSI_REMAP,},
> >  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
> >  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> >  				VMD_FEAT_HAS_BUS_RESTRICTIONS |
> > -- 
> > 2.27.0
> > 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] PCI: vmd: Disable MSI/X remapping when possible
@ 2021-02-06  2:50       ` Derrick, Jonathan
  0 siblings, 0 replies; 14+ messages in thread
From: Derrick, Jonathan @ 2021-02-06  2:50 UTC (permalink / raw)
  To: helgaas; +Cc: Patel, Nirmal, will, Karkra, Kapil, iommu, linux-pci, dwmw2

On Fri, 2021-02-05 at 15:57 -0600, Bjorn Helgaas wrote:
> On Thu, Feb 04, 2021 at 12:09:06PM -0700, Jon Derrick wrote:
> > VMD will retransmit child device MSI/X using its own MSI/X table and
> > requester-id. This limits the number of MSI/X available to the whole
> > child device domain to the number of VMD MSI/X interrupts.
> > 
> > Some VMD devices have a mode where this remapping can be disabled,
> > allowing child device interrupts to bypass processing with the VMD MSI/X
> > domain interrupt handler and going straight the child device interrupt
> > handler, allowing for better performance and scaling. The requester-id
> > still gets changed to the VMD endpoint's requester-id, and the interrupt
> > remapping handlers have been updated to properly set IRTE for child
> > device interrupts to the VMD endpoint's context.
> > 
> > Some VMD platforms have existing production BIOS which rely on MSI/X
> > remapping and won't explicitly program the MSI/X remapping bit. This
> > re-enables MSI/X remapping on unload.
> 
> Trivial comments below.  Would you mind using "MSI-X" instead of
> "MSI/X" so it matches the usage in the PCIe specs?  Several mentions
> above (including subject) and below.
Thanks Bjorn, will do

> 
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > ---
> >  drivers/pci/controller/vmd.c | 60 ++++++++++++++++++++++++++++--------
> >  1 file changed, 48 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> > index 5e80f28f0119..a319ce49645b 100644
> > --- a/drivers/pci/controller/vmd.c
> > +++ b/drivers/pci/controller/vmd.c
> > @@ -59,6 +59,13 @@ enum vmd_features {
> >  	 * be used for MSI remapping
> >  	 */
> >  	VMD_FEAT_OFFSET_FIRST_VECTOR		= (1 << 3),
> > +
> > +	/*
> > +	 * Device can bypass remapping MSI/X transactions into its MSI/X table,
> > +	 * avoding the requirement of a VMD MSI domain for child device
> 
> s/avoding/avoiding/
> 
> > +	 * interrupt handling
> 
> Maybe a period at the end of the sentence.
> 
> > +	 */
> > +	VMD_FEAT_BYPASS_MSI_REMAP		= (1 << 4),
> >  };
> >  
> >  /*
> > @@ -306,6 +313,15 @@ static struct msi_domain_info vmd_msi_domain_info = {
> >  	.chip		= &vmd_msi_controller,
> >  };
> >  
> > +static void vmd_enable_msi_remapping(struct vmd_dev *vmd, bool enable)
> > +{
> > +	u16 reg;
> > +
> > +	pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, &reg);
> > +	reg = enable ? (reg & ~0x2) : (reg | 0x2);
> 
> Would be nice to have a #define for 0x2.
> 
> > +	pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
> > +}
> > +
> >  static int vmd_create_irq_domain(struct vmd_dev *vmd)
> >  {
> >  	struct fwnode_handle *fn;
> > @@ -325,6 +341,13 @@ static int vmd_create_irq_domain(struct vmd_dev *vmd)
> >  
> >  static void vmd_remove_irq_domain(struct vmd_dev *vmd)
> >  {
> > +	/*
> > +	 * Some production BIOS won't enable remapping between soft reboots.
> > +	 * Ensure remapping is restored before unloading the driver.
> > +	 */
> > +	if (!vmd->msix_count)
> > +		vmd_enable_msi_remapping(vmd, true);
> > +
> >  	if (vmd->irq_domain) {
> >  		struct fwnode_handle *fn = vmd->irq_domain->fwnode;
> >  
> > @@ -679,15 +702,31 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
> >  
> >  	sd->node = pcibus_to_node(vmd->dev->bus);
> >  
> > -	ret = vmd_create_irq_domain(vmd);
> > -	if (ret)
> > -		return ret;
> > -
> >  	/*
> > -	 * Override the irq domain bus token so the domain can be distinguished
> > -	 * from a regular PCI/MSI domain.
> > +	 * Currently MSI remapping must be enabled in guest passthrough mode
> > +	 * due to some missing interrupt remapping plumbing. This is probably
> > +	 * acceptable because the guest is usually CPU-limited and MSI
> > +	 * remapping doesn't become a performance bottleneck.
> >  	 */
> > -	irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
> > +	if (!(features & VMD_FEAT_BYPASS_MSI_REMAP) || offset[0] || offset[1]) {
> > +		ret = vmd_alloc_irqs(vmd);
> > +		if (ret)
> > +			return ret;
> > +
> > +		vmd_enable_msi_remapping(vmd, true);
> > +
> > +		ret = vmd_create_irq_domain(vmd);
> > +		if (ret)
> > +			return ret;
> > +
> > +		/*
> > +		 * Override the irq domain bus token so the domain can be
> > +		 * distinguished from a regular PCI/MSI domain.
> > +		 */
> > +		irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
> > +	} else {
> > +		vmd_enable_msi_remapping(vmd, false);
> > +	}
> >  
> >  	pci_add_resource(&resources, &vmd->resources[0]);
> >  	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
> > @@ -753,10 +792,6 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
> >  	if (features & VMD_FEAT_OFFSET_FIRST_VECTOR)
> >  		vmd->first_vec = 1;
> >  
> > -	err = vmd_alloc_irqs(vmd);
> > -	if (err)
> > -		return err;
> > -
> >  	spin_lock_init(&vmd->cfg_lock);
> >  	pci_set_drvdata(dev, vmd);
> >  	err = vmd_enable_domain(vmd, features);
> > @@ -825,7 +860,8 @@ static const struct pci_device_id vmd_ids[] = {
> >  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
> >  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
> >  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
> > -				VMD_FEAT_HAS_BUS_RESTRICTIONS,},
> > +				VMD_FEAT_HAS_BUS_RESTRICTIONS |
> > +				VMD_FEAT_BYPASS_MSI_REMAP,},
> >  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
> >  		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
> >  				VMD_FEAT_HAS_BUS_RESTRICTIONS |
> > -- 
> > 2.27.0
> > 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-02-06  3:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-04 19:09 [PATCH v2 0/2] VMD MSI Remapping Bypass Jon Derrick
2021-02-04 19:09 ` Jon Derrick
2021-02-04 19:09 ` [PATCH v2 1/2] iommu/vt-d: Use Real PCI DMA device for IRTE Jon Derrick
2021-02-04 19:09   ` Jon Derrick
2021-02-05  1:39   ` Lu Baolu
2021-02-05  1:39     ` Lu Baolu
2021-02-04 19:09 ` [PATCH v2 2/2] PCI: vmd: Disable MSI/X remapping when possible Jon Derrick
2021-02-04 19:09   ` Jon Derrick
2021-02-05 21:57   ` Bjorn Helgaas
2021-02-05 21:57     ` Bjorn Helgaas
2021-02-06  2:50     ` Derrick, Jonathan
2021-02-06  2:50       ` Derrick, Jonathan
2021-02-05  8:45 ` [PATCH v2 0/2] VMD MSI Remapping Bypass Joerg Roedel
2021-02-05  8:45   ` Joerg Roedel

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.