[v4 0/8] Add VT-d Posted-Interrupts support - IOMMU part

[v4 0/8] Add VT-d Posted-Interrupts support - IOMMU part

[Feng Wu]

VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
With VT-d Posted-Interrupts enabled, external interrupts from
direct-assigned devices can be delivered to guests without VMM
intervention when guest is running in non-root mode.

You can find the VT-d Posted-Interrtups Spec. in the following URL:
http://www.intel.com/content/www/us/en/intelligent-systems/intel-technology/vt-directed-io-spec.html

This series was part of http://thread.gmane.org/gmane.linux.kernel.iommu/7708. To make things clear, send out IOMMU part here.

v3->v4:
* Change capability to a int variant flags instead of a function call
* Add hotplug case for VT-d PI

Feng Wu (8):
  iommu: Add new member capability to struct irq_remap_ops
  iommu, x86: Define new irte structure for VT-d Posted-Interrupts
  iommu, x86: Implement irq_set_vcpu_affinity for intel_ir_chip
  iommu, x86: No need to migrating irq for VT-d Posted-Interrupts
  iommu, x86: Add cap_pi_support() to detect VT-d PI capability
  iommu, x86: Setup Posted-Interrupts capability for Intel iommu
  iommu, x86: define irq_remapping_cap()
  iommu, x86: Properly handler PI for IOMMU hotplug

 arch/x86/include/asm/irq_remapping.h |   11 +++++
 drivers/iommu/intel_irq_remapping.c  |   78 +++++++++++++++++++++++++++++++++-
 drivers/iommu/irq_remapping.c        |   11 +++++
 drivers/iommu/irq_remapping.h        |    6 +++
 include/linux/dmar.h                 |   32 ++++++++++++++
 include/linux/intel-iommu.h          |    1 +
 6 files changed, 138 insertions(+), 1 deletions(-)

[v4 1/8] iommu: Add new member capability to struct irq_remap_ops

[Feng Wu]

This patch adds a new member capability to struct irq_remap_ops,
this new function ops can be used to check whether some
features are supported, such as VT-d Posted-Interrupts.

Signed-off-by: Feng Wu <feng.wu@intel.com>
Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/include/asm/irq_remapping.h |    4 ++++
 drivers/iommu/irq_remapping.h        |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/irq_remapping.h b/arch/x86/include/asm/irq_remapping.h
index 6ba2431..f67ae08 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -31,6 +31,10 @@ struct irq_alloc_info;
 
 #ifdef CONFIG_IRQ_REMAP
 
+enum irq_remap_cap {
+	IRQ_POSTING_CAP = 0,
+};
+
 extern void setup_irq_remapping_ops(void);
 extern int irq_remapping_supported(void);
 extern void set_irq_remapping_broken(void);
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index 4bd791d..69b2a1c 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -36,6 +36,9 @@ extern int no_x2apic_optout;
 extern int irq_remapping_enabled;
 
 struct irq_remap_ops {
+	/* The supported capabilites */
+	int capability;
+
 	/* Check whether Interrupt Remapping is supported */
 	int (*supported)(void);
 
-- 
1.7.1

[v4 2/8] iommu, x86: Define new irte structure for VT-d Posted-Interrupts

[Feng Wu]

Add a new irte_pi structure for VT-d Posted-Interrupts.

Signed-off-by: Feng Wu <feng.wu@intel.com>
Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
Acked-by: David Woodhouse <David.Woodhouse@intel.com>
---
 include/linux/dmar.h |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 8473756..c7f9cda 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -212,6 +212,38 @@ struct irte {
 	};
 };
 
+struct irte_pi {
+	union {
+		struct {
+			__u64   present		: 1,
+				fpd		: 1,
+				__reserved_1	: 6,
+				avail		: 4,
+				__reserved_2	: 2,
+				urg		: 1,
+				pst		: 1,
+				vector		: 8,
+				__reserved_3	: 14,
+				pda_l		: 26;
+		};
+		__u64 low;
+	};
+
+	union {
+		struct {
+			__u64	sid		: 16,
+				sq		: 2,
+				svt		: 2,
+				__reserved_4	: 12,
+				pda_h		: 32;
+		};
+		__u64 high;
+	};
+};
+
+#define PDA_LOW_BIT    26
+#define PDA_HIGH_BIT   32
+
 enum {
 	IRQ_REMAP_XAPIC_MODE,
 	IRQ_REMAP_X2APIC_MODE,
-- 
1.7.1

[v4 3/8] iommu, x86: Implement irq_set_vcpu_affinity for intel_ir_chip

[Feng Wu]

Implement irq_set_vcpu_affinity for intel_ir_chip.

Signed-off-by: Feng Wu <feng.wu@intel.com>
Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
Acked-by: David Woodhouse <David.Woodhouse@intel.com>
---
 arch/x86/include/asm/irq_remapping.h |    5 ++++
 drivers/iommu/intel_irq_remapping.c  |   35 ++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/irq_remapping.h b/arch/x86/include/asm/irq_remapping.h
index f67ae08..f87ac70 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -60,6 +60,11 @@ static inline struct irq_domain *arch_get_ir_parent_domain(void)
 	return x86_vector_domain;
 }
 
+struct vcpu_data {
+	u64 pi_desc_addr;	/* Physical address of PI Descriptor */
+	u32 vector;		/* Guest vector of the interrupt */
+};
+
 #else  /* CONFIG_IRQ_REMAP */
 
 static inline void setup_irq_remapping_ops(void) { }
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index f6da3b2..48c2051 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -42,6 +42,7 @@ struct irq_2_iommu {
 struct intel_ir_data {
 	struct irq_2_iommu			irq_2_iommu;
 	struct irte				irte_entry;
+	struct irte_pi                          irte_pi_entry;
 	union {
 		struct msi_msg			msi_entry;
 	};
@@ -1010,10 +1011,44 @@ static void intel_ir_compose_msi_msg(struct irq_data *irq_data,
 	*msg = ir_data->msi_entry;
 }
 
+static int intel_ir_set_vcpu_affinity(struct irq_data *data, void *vcpu_info)
+{
+	struct intel_ir_data *ir_data = data->chip_data;
+	struct irte_pi *irte_pi = &ir_data->irte_pi_entry;
+	struct vcpu_data *vcpu_pi_info;
+
+	/* stop posting interrupts, back to remapping mode */
+	if (!vcpu_info)
+		modify_irte(&ir_data->irq_2_iommu, &ir_data->irte_entry);
+	else {
+		vcpu_pi_info = (struct vcpu_data *)vcpu_info;
+		memcpy(irte_pi, &ir_data->irte_entry, sizeof(struct irte));
+
+		irte_pi->urg = 0;
+		irte_pi->vector = vcpu_pi_info->vector;
+		irte_pi->pda_l = (vcpu_pi_info->pi_desc_addr >>
+				 (32 - PDA_LOW_BIT)) & ~(-1UL << PDA_LOW_BIT);
+		irte_pi->pda_h = (vcpu_pi_info->pi_desc_addr >> 32) &
+				 ~(-1UL << PDA_HIGH_BIT);
+
+		irte_pi->__reserved_1 = 0;
+		irte_pi->__reserved_2 = 0;
+		irte_pi->__reserved_3 = 0;
+		irte_pi->__reserved_4 = 0;
+
+		irte_pi->pst = 1;
+
+		modify_irte(&ir_data->irq_2_iommu, (struct irte *)irte_pi);
+	}
+
+	return 0;
+}
+
 static struct irq_chip intel_ir_chip = {
 	.irq_ack = ir_ack_apic_edge,
 	.irq_set_affinity = intel_ir_set_affinity,
 	.irq_compose_msi_msg = intel_ir_compose_msi_msg,
+	.irq_set_vcpu_affinity = intel_ir_set_vcpu_affinity,
 };
 
 static void intel_irq_remapping_prepare_irte(struct intel_ir_data *data,
-- 
1.7.1

[v4 4/8] iommu, x86: No need to migrating irq for VT-d Posted-Interrupts

[Feng Wu]

We don't need to migrate the irqs for VT-d Posted-Interrupts here.
When 'pst' is set in IRTE, the associated irq will be posted to
guests instead of interrupt remapping. The destination of the
interrupt is set in Posted-Interrupts Descriptor, and the migration
happens during vCPU scheduling.

However, we still update the cached irte here, which can be used
when changing back to remapping mode.

Signed-off-by: Feng Wu <feng.wu@intel.com>
Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
Acked-by: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/iommu/intel_irq_remapping.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 48c2051..ab9057a 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -977,6 +977,7 @@ intel_ir_set_affinity(struct irq_data *data, const struct cpumask *mask,
 {
 	struct intel_ir_data *ir_data = data->chip_data;
 	struct irte *irte = &ir_data->irte_entry;
+	struct irte_pi *irte_pi = (struct irte_pi *)irte;
 	struct irq_cfg *cfg = irqd_cfg(data);
 	struct irq_data *parent = data->parent_data;
 	int ret;
@@ -991,7 +992,10 @@ intel_ir_set_affinity(struct irq_data *data, const struct cpumask *mask,
 	 */
 	irte->vector = cfg->vector;
 	irte->dest_id = IRTE_DEST(cfg->dest_apicid);
-	modify_irte(&ir_data->irq_2_iommu, irte);
+
+	/* We don't need to modify irte if the interrupt is for posting. */
+	if (irte_pi->pst != 1)
+		modify_irte(&ir_data->irq_2_iommu, irte);
 
 	/*
 	 * After this point, all the interrupts will start arriving
-- 
1.7.1

[v4 5/8] iommu, x86: Add cap_pi_support() to detect VT-d PI capability

[Feng Wu]

Add helper function to detect VT-d Posted-Interrupts capability.

Signed-off-by: Feng Wu <feng.wu@intel.com>
Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
Acked-by: David Woodhouse <David.Woodhouse@intel.com>
---
 include/linux/intel-iommu.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index ecaf3a9..8174ae8 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -87,6 +87,7 @@ static inline void dmar_writeq(void __iomem *addr, u64 val)
 /*
  * Decoding Capability Register
  */
+#define cap_pi_support(c)	(((c) >> 59) & 1)
 #define cap_read_drain(c)	(((c) >> 55) & 1)
 #define cap_write_drain(c)	(((c) >> 54) & 1)
 #define cap_max_amask_val(c)	(((c) >> 48) & 0x3f)
-- 
1.7.1

[v4 6/8] iommu, x86: Setup Posted-Interrupts capability for Intel iommu

[Feng Wu]

Set Posted-Interrupts capability for Intel iommu when IR is enabled,
clear it when IR is disabled.

Signed-off-by: Feng Wu <feng.wu@intel.com>
---
 drivers/iommu/intel_irq_remapping.c |   34 ++++++++++++++++++++++++++++++++++
 drivers/iommu/irq_remapping.c       |    2 ++
 drivers/iommu/irq_remapping.h       |    3 +++
 3 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index ab9057a..130a92b 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -635,6 +635,20 @@ static int __init intel_enable_irq_remapping(void)
 
 	irq_remapping_enabled = 1;
 
+	/*
+	 * Set Posted-Interrupts capability.
+	 */
+	if (!disable_irq_post) {
+		intel_irq_remap_ops.capability |= 1 << IRQ_POSTING_CAP;
+
+		for_each_iommu(iommu, drhd)
+			if (!cap_pi_support(iommu->cap)) {
+				intel_irq_remap_ops.capability &=
+						~(1 << IRQ_POSTING_CAP);
+				break;
+			}
+	}
+
 	pr_info("Enabled IRQ remapping in %s mode\n", eim ? "x2apic" : "xapic");
 
 	return eim ? IRQ_REMAP_X2APIC_MODE : IRQ_REMAP_XAPIC_MODE;
@@ -843,6 +857,12 @@ static void disable_irq_remapping(void)
 
 		iommu_disable_irq_remapping(iommu);
 	}
+
+	/*
+	 * Clear Posted-Interrupts capability.
+	 */
+	if (!disable_irq_post)
+		intel_irq_remap_ops.capability &= ~(1 << IRQ_POSTING_CAP);
 }
 
 static int reenable_irq_remapping(int eim)
@@ -870,6 +890,20 @@ static int reenable_irq_remapping(int eim)
 	if (!setup)
 		goto error;
 
+	/*
+	 * Set Posted-Interrupts capability.
+	 */
+	if (!disable_irq_post) {
+		intel_irq_remap_ops.capability |= 1 << IRQ_POSTING_CAP;
+
+		for_each_iommu(iommu, drhd)
+			if (!cap_pi_support(iommu->cap)) {
+				intel_irq_remap_ops.capability &=
+						~(1 << IRQ_POSTING_CAP);
+				break;
+			}
+	}
+
 	return 0;
 
 error:
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 3c3da04..e63e969 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -24,6 +24,8 @@ int irq_remap_broken;
 int disable_sourceid_checking;
 int no_x2apic_optout;
 
+int disable_irq_post = 1;
+
 static struct irq_remap_ops *remap_ops;
 
 static void irq_remapping_disable_io_apic(void)
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index 69b2a1c..fe6ef36 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -35,6 +35,8 @@ extern int disable_sourceid_checking;
 extern int no_x2apic_optout;
 extern int irq_remapping_enabled;
 
+extern int disable_irq_post;
+
 struct irq_remap_ops {
 	/* The supported capabilites */
 	int capability;
@@ -74,6 +76,7 @@ extern void ir_ack_apic_edge(struct irq_data *data);
 #define irq_remapping_enabled 0
 #define disable_irq_remap     1
 #define irq_remap_broken      0
+#define disable_irq_post      1
 
 #endif /* CONFIG_IRQ_REMAP */
 
-- 
1.7.1

[v4 7/8] iommu, x86: define irq_remapping_cap()

[Feng Wu]

This patch adds a new interface irq_remapping_cap() to detect
whether irq remapping supports new features, such as VT-d
Posted-Interrupts. We export this function out, so that KVM
code can check this and use this mechanism properly.

Signed-off-by: Feng Wu <feng.wu@intel.com>
Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/include/asm/irq_remapping.h |    2 ++
 drivers/iommu/irq_remapping.c        |    9 +++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/irq_remapping.h b/arch/x86/include/asm/irq_remapping.h
index f87ac70..b3ad067 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -37,6 +37,7 @@ enum irq_remap_cap {
 
 extern void setup_irq_remapping_ops(void);
 extern int irq_remapping_supported(void);
+extern bool irq_remapping_cap(enum irq_remap_cap cap);
 extern void set_irq_remapping_broken(void);
 extern int irq_remapping_prepare(void);
 extern int irq_remapping_enable(void);
@@ -69,6 +70,7 @@ struct vcpu_data {
 
 static inline void setup_irq_remapping_ops(void) { }
 static inline int irq_remapping_supported(void) { return 0; }
+static bool irq_remapping_cap(enum irq_remap_cap cap) { return 0; }
 static inline void set_irq_remapping_broken(void) { }
 static inline int irq_remapping_prepare(void) { return -ENODEV; }
 static inline int irq_remapping_enable(void) { return -ENODEV; }
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index e63e969..51c3b48 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -103,6 +103,15 @@ int irq_remapping_supported(void)
 	return remap_ops->supported();
 }
 
+bool irq_remapping_cap(enum irq_remap_cap cap)
+{
+	if (!remap_ops || disable_irq_post)
+		return 0;
+
+	return (remap_ops->capability & (1 << cap));
+}
+EXPORT_SYMBOL_GPL(irq_remapping_cap);
+
 int __init irq_remapping_prepare(void)
 {
 	if (!remap_ops || !remap_ops->prepare)
-- 
1.7.1

[v4 8/8] iommu, x86: Properly handler PI for IOMMU hotplug

[Feng Wu]

Return error when inserting a new IOMMU which doesn't support PI
if PI is currently in use.

Signed-off-by: Feng Wu <feng.wu@intel.com>
---
 drivers/iommu/intel_irq_remapping.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 130a92b..0c2c317 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -1339,6 +1339,9 @@ int dmar_ir_hotplug(struct dmar_drhd_unit *dmaru, bool insert)
 		return -EINVAL;
 	if (!ecap_ir_support(iommu->ecap))
 		return 0;
+	if (irq_remapping_cap(IRQ_POSTING_CAP) &&
+	    !cap_pi_support(iommu->cap))
+		return -EBUSY;
 
 	if (insert) {
 		if (!iommu->ir_table)
-- 
1.7.1

Re: [v4 2/8] iommu, x86: Define new irte structure for VT-d Posted-Interrupts

[Joerg Roedel]

Hi Feng,

On Mon, Feb 02, 2015 at 04:06:58PM +0800, Feng Wu wrote:
> Add a new irte_pi structure for VT-d Posted-Interrupts.
> 
> Signed-off-by: Feng Wu <feng.wu@intel.com>
> Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
> Acked-by: David Woodhouse <David.Woodhouse@intel.com>
> ---
>  include/linux/dmar.h |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 8473756..c7f9cda 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -212,6 +212,38 @@ struct irte {
>  	};
>  };
>  
> +struct irte_pi {

I think it is better to put this as a union into struct irte. It saves
memory and unnecessary casting in later patches.


	Joerg

RE: [v4 2/8] iommu, x86: Define new irte structure for VT-d Posted-Interrupts

[Wu, Feng]

> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Monday, March 23, 2015 7:58 PM
> To: Wu, Feng
> Cc: dwmw2@infradead.org; jiang.liu@linux.intel.com;
> iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org
> Subject: Re: [v4 2/8] iommu, x86: Define new irte structure for VT-d
> Posted-Interrupts
> 
> Hi Feng,
> 
> On Mon, Feb 02, 2015 at 04:06:58PM +0800, Feng Wu wrote:
> > Add a new irte_pi structure for VT-d Posted-Interrupts.
> >
> > Signed-off-by: Feng Wu <feng.wu@intel.com>
> > Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
> > Acked-by: David Woodhouse <David.Woodhouse@intel.com>
> > ---
> >  include/linux/dmar.h |   32 ++++++++++++++++++++++++++++++++
> >  1 files changed, 32 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> > index 8473756..c7f9cda 100644
> > --- a/include/linux/dmar.h
> > +++ b/include/linux/dmar.h
> > @@ -212,6 +212,38 @@ struct irte {
> >  	};
> >  };
> >
> > +struct irte_pi {
> 
> I think it is better to put this as a union into struct irte. It saves
> memory and unnecessary casting in later patches.

Thanks for the comments!

Do you mean doing it like the following?

struct irte {
        union {
                struct {
                        __u64   present         : 1,
                                fpd             : 1,
                                dst_mode        : 1,
                                redir_hint      : 1,
                                trigger_mode    : 1,
                                dlvry_mode      : 3,
                                avail           : 4,
                                __reserved_1    : 4,
                                vector          : 8,
                                __reserved_2    : 8,
                                dest_id         : 32;
                };
                struct {
                        __u64   present         : 1,
                                fpd             : 1,
                                __reserved_1    : 6,
                                avail           : 4,
                                __reserved_2    : 2,
                                urg             : 1,
                                pst             : 1,
                                vector          : 8,
                                __reserved_3    : 14,
                                pda_l           : 26;
                };
                __u64 low;
        };

        union {
                struct {
                        __u64   sid             : 16,
                                sq              : 2,
                                svt             : 2,
                                __reserved_3    : 44;
                };
                struct {
                        __u64   sid             : 16,
                                sq              : 2,
                                svt             : 2,
                                __reserved_4    : 12,
                                pda_h           : 32;
                };
                __u64 high;
        };
};

In fact, I also intended to make these two defines as one, however, this code will get build
error ("duplicated member") with new version of GCC, such as, gcc 4.9.1. I cannot find a
good way to handle this gracefully, since I don't want to impact the existing usage of this
structure . Do you have any ideas about this? Thanks a lot!

Thanks,
Feng

> 
> 
> 	Joerg

Re: [v4 2/8] iommu, x86: Define new irte structure for VT-d Posted-Interrupts

[Joerg Roedel]

On Tue, Mar 24, 2015 at 02:32:01AM +0000, Wu, Feng wrote:
> > I think it is better to put this as a union into struct irte. It saves
> > memory and unnecessary casting in later patches.
> 
> Thanks for the comments!

Thinking more about this, I think its probably fine to keep the two
versions of the irte seperate like in this patch-set. It allows to
update the non-posted irte when the posted irte is active at the moment
and makes the transition between both irte variants easier.

But what I still don't like is the type casting necessary when calling
modify_irte(). Can you abstract this and put the decission whether irte
or irte_pi is set active into modify_irte? It required to change the
interface of modify_irte, but that should be easy.


	Joerg

RE: [v4 2/8] iommu, x86: Define new irte structure for VT-d Posted-Interrupts

[Wu, Feng]

> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Tuesday, March 31, 2015 11:17 PM
> To: Wu, Feng
> Cc: dwmw2@infradead.org; jiang.liu@linux.intel.com;
> iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org
> Subject: Re: [v4 2/8] iommu, x86: Define new irte structure for VT-d
> Posted-Interrupts
> 
> On Tue, Mar 24, 2015 at 02:32:01AM +0000, Wu, Feng wrote:
> > > I think it is better to put this as a union into struct irte. It saves
> > > memory and unnecessary casting in later patches.
> >
> > Thanks for the comments!
> 
> Thinking more about this, I think its probably fine to keep the two
> versions of the irte seperate like in this patch-set. It allows to
> update the non-posted irte when the posted irte is active at the moment
> and makes the transition between both irte variants easier.
> 
> But what I still don't like is the type casting necessary when calling
> modify_irte(). Can you abstract this and put the decission whether irte
> or irte_pi is set active into modify_irte? It required to change the
> interface of modify_irte, but that should be easy.
> 

Sound good! Then we can keep the difference inside modify_irte().

BTW, could you please have a look at other patches in this series?

Thanks,
Feng

> 
> 	Joerg

Re: [v4 0/8] Add VT-d Posted-Interrupts support - IOMMU part

[Joerg Roedel]

On Mon, Feb 02, 2015 at 04:06:56PM +0800, Feng Wu wrote:
> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
> With VT-d Posted-Interrupts enabled, external interrupts from
> direct-assigned devices can be delivered to guests without VMM
> intervention when guest is running in non-root mode.
> 
> You can find the VT-d Posted-Interrtups Spec. in the following URL:
> http://www.intel.com/content/www/us/en/intelligent-systems/intel-technology/vt-directed-io-spec.html
> 
> This series was part of http://thread.gmane.org/gmane.linux.kernel.iommu/7708. To make things clear, send out IOMMU part here.

Besides the modify_irte() changes I asked for the patch-set looks good.
I just have some concerns what these changes mean for the VT-d kdump
improvements Zhen-Hua Li is working on. Can you please discuss the
implications of having both patch-sets applied with him and make sure
they work together? I think in its current form your patch-set breaks
the kdump support patches. I added Zhen-Hua to Cc.

Thanks,

	Joerg

Re: [v4 0/8] Add VT-d Posted-Interrupts support - IOMMU part

[Li, ZhenHua]

Hi Feng Wu,
In my patchset, I created a new member ir_table->base_old_phys; In the 
normal kernel, everything is the same. In kdump kernel, ir_table->base 
is used for a buffer, and  ir_table->base_old_phys is the physical 
address of the tables used by the old kernel, also being used by the 
current kernel.

I did this in modify_irte():

         set_64bit(&irte->high, irte_modified->high);
+
+#ifdef CONFIG_CRASH_DUMP
+       if (is_kdump_kernel())
+               __iommu_update_old_irte(iommu, index);
+#endif
         __iommu_flush_cache(iommu, irte, sizeof(*irte));


Here the irte tables are stored in two places:
iommu->ir_table->base : It is a buffer in kdump kernel, which is the 
running kernel;
iommu->ir_table->base_old_phys : It is the irte used by the old kernel;

And function __iommu_update_old_irte is used to save the content of 
iommu->ir_table->base  to iommu->ir_table->base_old_phys. Because in 
kdump kernel, the vt-d is using ir_table->base_old_phys, not 
ir_table->base, so we need to copy the updated ir_table->base to 
ir_table->base_old_phys .


Thanks
Zhenhua

On 04/02/2015 07:28 PM, Joerg Roedel wrote:
> On Mon, Feb 02, 2015 at 04:06:56PM +0800, Feng Wu wrote:
>> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
>> With VT-d Posted-Interrupts enabled, external interrupts from
>> direct-assigned devices can be delivered to guests without VMM
>> intervention when guest is running in non-root mode.
>>
>> You can find the VT-d Posted-Interrtups Spec. in the following URL:
>> http://www.intel.com/content/www/us/en/intelligent-systems/intel-technology/vt-directed-io-spec.html
>>
>> This series was part of http://thread.gmane.org/gmane.linux.kernel.iommu/7708. To make things clear, send out IOMMU part here.
>
> Besides the modify_irte() changes I asked for the patch-set looks good.
> I just have some concerns what these changes mean for the VT-d kdump
> improvements Zhen-Hua Li is working on. Can you please discuss the
> implications of having both patch-sets applied with him and make sure
> they work together? I think in its current form your patch-set breaks
> the kdump support patches. I added Zhen-Hua to Cc.
>
> Thanks,
>
> 	Joerg
>

RE: [v4 0/8] Add VT-d Posted-Interrupts support - IOMMU part

[Wu, Feng]

> -----Original Message-----
> From: Li, ZhenHua [mailto:zhen-hual@hp.com]
> Sent: Friday, April 03, 2015 4:13 PM
> To: Wu, Feng
> Cc: Joerg Roedel; dwmw2@infradead.org; jiang.liu@linux.intel.com;
> iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org; Li, ZhenHua
> Subject: Re: [v4 0/8] Add VT-d Posted-Interrupts support - IOMMU part
> 
> Hi Feng Wu,
> In my patchset, I created a new member ir_table->base_old_phys; In the
> normal kernel, everything is the same. In kdump kernel, ir_table->base
> is used for a buffer, and  ir_table->base_old_phys is the physical
> address of the tables used by the old kernel, also being used by the
> current kernel.
> 
> I did this in modify_irte():
> 
>          set_64bit(&irte->high, irte_modified->high);
> +
> +#ifdef CONFIG_CRASH_DUMP
> +       if (is_kdump_kernel())
> +               __iommu_update_old_irte(iommu, index);
> +#endif
>          __iommu_flush_cache(iommu, irte, sizeof(*irte));
> 
> 
> Here the irte tables are stored in two places:
> iommu->ir_table->base : It is a buffer in kdump kernel, which is the
> running kernel;
> iommu->ir_table->base_old_phys : It is the irte used by the old kernel;
> 
> And function __iommu_update_old_irte is used to save the content of
> iommu->ir_table->base  to iommu->ir_table->base_old_phys. Because in
> kdump kernel, the vt-d is using ir_table->base_old_phys, not
> ir_table->base, so we need to copy the updated ir_table->base to
> ir_table->base_old_phys .
> 

Hi Zhenhua,

Thanks very much for your clarification! Basically, the main purpose of my
Patch-set is to provide an interface to KVM, so that KVM can update irte
for posted-interrupts. In this interface, it calls modify_irte(). I also go
through your patch set with the associated part, seems I cannot find
any conflicts with your patches. What is your idea about this? Thanks a lot!

Thanks,
Feng

-


> 
> Thanks
> Zhenhua
> 
> On 04/02/2015 07:28 PM, Joerg Roedel wrote:
> > On Mon, Feb 02, 2015 at 04:06:56PM +0800, Feng Wu wrote:
> >> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
> >> With VT-d Posted-Interrupts enabled, external interrupts from
> >> direct-assigned devices can be delivered to guests without VMM
> >> intervention when guest is running in non-root mode.
> >>
> >> You can find the VT-d Posted-Interrtups Spec. in the following URL:
> >>
> http://www.intel.com/content/www/us/en/intelligent-systems/intel-technolog
> y/vt-directed-io-spec.html
> >>
> >> This series was part of
> http://thread.gmane.org/gmane.linux.kernel.iommu/7708. To make things
> clear, send out IOMMU part here.
> >
> > Besides the modify_irte() changes I asked for the patch-set looks good.
> > I just have some concerns what these changes mean for the VT-d kdump
> > improvements Zhen-Hua Li is working on. Can you please discuss the
> > implications of having both patch-sets applied with him and make sure
> > they work together? I think in its current form your patch-set breaks
> > the kdump support patches. I added Zhen-Hua to Cc.
> >
> > Thanks,
> >
> > 	Joerg
> >

[v4 4/8] iommu, x86: No need to migrating irq for VT-d Posted-Interrupts

[Feng Wu]

We don't need to migrate the irqs for VT-d Posted-Interrupts here.
When 'pst' is set in IRTE, the associated irq will be posted to
guests instead of interrupt remapping. The destination of the
interrupt is set in Posted-Interrupts Descriptor, and the migration
happens during vCPU scheduling.

However, we still update the cached irte here, which can be used
when changing back to remapping mode.

Signed-off-by: Feng Wu <feng.wu@intel.com>
Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
Acked-by: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/iommu/intel_irq_remapping.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 48c2051..ab9057a 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -977,6 +977,7 @@ intel_ir_set_affinity(struct irq_data *data, const struct cpumask *mask,
 {
 	struct intel_ir_data *ir_data = data->chip_data;
 	struct irte *irte = &ir_data->irte_entry;
+	struct irte_pi *irte_pi = (struct irte_pi *)irte;
 	struct irq_cfg *cfg = irqd_cfg(data);
 	struct irq_data *parent = data->parent_data;
 	int ret;
@@ -991,7 +992,10 @@ intel_ir_set_affinity(struct irq_data *data, const struct cpumask *mask,
 	 */
 	irte->vector = cfg->vector;
 	irte->dest_id = IRTE_DEST(cfg->dest_apicid);
-	modify_irte(&ir_data->irq_2_iommu, irte);
+
+	/* We don't need to modify irte if the interrupt is for posting. */
+	if (irte_pi->pst != 1)
+		modify_irte(&ir_data->irq_2_iommu, irte);
 
 	/*
 	 * After this point, all the interrupts will start arriving
-- 
1.7.1