[PART2 PATCH v5 00/12] iommu/AMD: Introduce IOMMU AVIC support

[PART2 PATCH v5 00/12] iommu/AMD: Introduce IOMMU AVIC support

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

CHANGES FROM V4
===============
  * Remove the hash look up in the amd_iommu_update_ga() (see patch 7/12).
    Instead, use per-vcpu pi_list to keep track of posted interrupts so that
    SVM can directly update IOMMU interrupt remapping table entries directly
    when rescheduling VCPUs. (see patch 8/12 and 12/12)(per Radim's suggestion)

  * Re-implement AVIC VM-ID using a set of bit-mask to ensure no ID conflict
    between active VMs. (see patch 10/12)

  * Verify VM-ID of the hash entry in avic_ga_log_notifier() before referencing
    each per-VM data structure. (see patch 11/12) (per Radim's suggestion)

GITHUB
======
Latest git tree can be found at:
    http://github.com/ssuthiku/linux.git    avic_part2_v5

OVERVIEW
========
This patch set is the second part of the two-part patch series to introduce
the new AMD Advance Virtual Interrupt Controller (AVIC) support.

In addition to the SVM AVIC, AMD IOMMU also extends the AVIC capability
to allow I/O interrupts injection directly into the virtualized guest
local APIC without the need for hypervisor intervention.

This patch series introduces a new hardware interrupt remapping (IR) mode
in AMD IOMMU driver, the Guest Virtual APIC (GA) mode. This is in contrast
to the existing "legacy" mode. The IR mode can be specified with a new
kernel parameter:

    amd_iommu_guest_ir=[vapic (default) | legacy]

When enabling GA mode, the AMD IOMMU driver will configure device interrupt
remapping in GA mode when possible (i.e. SVM AVIC must be enabled, and if
the interrupt types are supported). Otherewise, the driver will fallback
to using the legacy IR mode.

This patch series also introduces new interfaces between SVM and IOMMU
to allow:
  * SVM driver to communicate to IOMMU with updated vcpu scheduling
    information.
  * IOMMU driver to notify SVM driver to schedule vcpu on to physical core
    handle IOMMU GALog entry.

DOCUMENTATIONS
==============
More information about SVM AVIC can be found in the
AMD64 Architecture Programmer’s Manual Volume 2 - System Programming.

    http://support.amd.com/TechDocs/24593.pdf

More information about IOMMU AVIC can be found int the
AMD I/O Virtualization Technology (IOMMU) Specification - Rev 2.62.

    http://support.amd.com/TechDocs/48882_IOMMU.pdf

Any feedback and comments are very much appreciated.

Thank you,
Suravee

Suravee Suthikulpanit (12):
  iommu/amd: Detect and enable guest vAPIC support
  iommu/amd: Move and introduce new IRTE-related unions and structures
  iommu/amd: Introduce interrupt remapping ops structure
  iommu/amd: Add support for multiple IRTE formats
  iommu/amd: Detect and initialize guest vAPIC log
  iommu/amd: Adding GALOG interrupt handler
  iommu/amd: Introduce amd_iommu_update_ga()
  iommu/amd: Implements irq_set_vcpu_affinity() hook to setup vapic mode
    for pass-through devices
  iommu/amd: Enable vAPIC interrupt remapping mode by default
  svm: Introduces AVIC per-VM ID
  svm: Introduce AMD IOMMU avic_ga_log_notifier
  svm: Implements update_pi_irte hook to setup posted interrupt

 arch/x86/include/asm/kvm_host.h |   2 +
 arch/x86/kvm/svm.c              | 381 +++++++++++++++++++++++++++++-
 drivers/iommu/amd_iommu.c       | 501 +++++++++++++++++++++++++++++++++++-----
 drivers/iommu/amd_iommu_init.c  | 183 ++++++++++++++-
 drivers/iommu/amd_iommu_proto.h |   1 +
 drivers/iommu/amd_iommu_types.h | 151 ++++++++++++
 include/linux/amd-iommu.h       |  42 +++-
 7 files changed, 1191 insertions(+), 70 deletions(-)

-- 
1.9.1

[PART2 PATCH v5 01/12] iommu/amd: Detect and enable guest vAPIC support

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch introduces a new IOMMU driver parameter, amd_iommu_guest_ir,
which can be used to specify different interrupt remapping mode for
passthrough devices to VM guest:
    * legacy: Legacy interrupt remapping (w/ 32-bit IRTE)
    * vapic : Guest vAPIC interrupt remapping (w/ GA mode 128-bit IRTE)

Note that in vapic mode, it can also supports legacy interrupt remapping
for non-passthrough devices with the 128-bit IRTE.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu_init.c  | 73 +++++++++++++++++++++++++++++++++++++----
 drivers/iommu/amd_iommu_proto.h |  1 +
 drivers/iommu/amd_iommu_types.h | 24 ++++++++++++++
 3 files changed, 92 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index d091def..d0930b1 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -145,6 +145,8 @@ struct ivmd_header {
 bool amd_iommu_dump;
 bool amd_iommu_irq_remap __read_mostly;
 
+int amd_iommu_guest_ir;
+
 static bool amd_iommu_detected;
 static bool __initdata amd_iommu_disabled;
 static int amd_iommu_target_ivhd_type;
@@ -1258,6 +1260,8 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 			iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
 		else
 			iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
+		if (((h->efr_attr & (0x1 << IOMMU_FEAT_GASUP_SHIFT)) == 0))
+			amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY;
 		break;
 	case 0x11:
 	case 0x40:
@@ -1265,6 +1269,8 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 			iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
 		else
 			iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
+		if (((h->efr_reg & (0x1 << IOMMU_EFR_GASUP_SHIFT)) == 0))
+			amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY;
 		break;
 	default:
 		return -EINVAL;
@@ -1488,6 +1494,14 @@ static int iommu_init_pci(struct amd_iommu *iommu)
 	if (iommu_feature(iommu, FEATURE_PPR) && alloc_ppr_log(iommu))
 		return -ENOMEM;
 
+	/* Note: We have already checked GASup from IVRS table.
+	 *       Now, we need to make sure that GAMSup is set.
+	 */
+	if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) &&
+	    !iommu_feature(iommu, FEATURE_GAM_VAPIC))
+		amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;
+
+
 	if (iommu->cap & (1UL << IOMMU_CAP_NPCACHE))
 		amd_iommu_np_cache = true;
 
@@ -1545,16 +1559,24 @@ static void print_iommu_info(void)
 			dev_name(&iommu->dev->dev), iommu->cap_ptr);
 
 		if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
-			pr_info("AMD-Vi:  Extended features: ");
+			pr_info("AMD-Vi: Extended features (%#llx):\n",
+				iommu->features);
 			for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
 				if (iommu_feature(iommu, (1ULL << i)))
 					pr_cont(" %s", feat_str[i]);
 			}
+
+			if (iommu->features & FEATURE_GAM_VAPIC)
+				pr_cont(" GA_vAPIC");
+
 			pr_cont("\n");
 		}
 	}
-	if (irq_remapping_enabled)
+	if (irq_remapping_enabled) {
 		pr_info("AMD-Vi: Interrupt remapping enabled\n");
+		if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
+			pr_info("AMD-Vi: virtual APIC enabled\n");
+	}
 }
 
 static int __init amd_iommu_init_pci(void)
@@ -1852,6 +1874,22 @@ static void iommu_apply_resume_quirks(struct amd_iommu *iommu)
 			       iommu->stored_addr_lo | 1);
 }
 
+static void iommu_enable_ga(struct amd_iommu *iommu)
+{
+#ifdef CONFIG_IRQ_REMAP
+	switch (amd_iommu_guest_ir) {
+	case AMD_IOMMU_GUEST_IR_VAPIC:
+		iommu_feature_enable(iommu, CONTROL_GAM_EN);
+		/* Fall through */
+	case AMD_IOMMU_GUEST_IR_LEGACY_GA:
+		iommu_feature_enable(iommu, CONTROL_GA_EN);
+		break;
+	default:
+		break;
+	}
+#endif
+}
+
 /*
  * This function finally enables all IOMMUs found in the system after
  * they have been initialized
@@ -1867,6 +1905,7 @@ static void early_enable_iommus(void)
 		iommu_enable_command_buffer(iommu);
 		iommu_enable_event_buffer(iommu);
 		iommu_set_exclusion_range(iommu);
+		iommu_enable_ga(iommu);
 		iommu_enable(iommu);
 		iommu_flush_all_caches(iommu);
 	}
@@ -2147,10 +2186,16 @@ static int __init early_amd_iommu_init(void)
 		 * remapping tables.
 		 */
 		ret = -ENOMEM;
-		amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
-				MAX_IRQS_PER_TABLE * sizeof(u32),
-				IRQ_TABLE_ALIGNMENT,
-				0, NULL);
+		if (!AMD_IOMMU_GUEST_IR_GA(amd_iommu_guest_ir))
+			amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
+					MAX_IRQS_PER_TABLE * sizeof(u32),
+					IRQ_TABLE_ALIGNMENT,
+					0, NULL);
+		else
+			amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
+					MAX_IRQS_PER_TABLE * (sizeof(u64) * 2),
+					IRQ_TABLE_ALIGNMENT,
+					0, NULL);
 		if (!amd_iommu_irq_cache)
 			goto out;
 
@@ -2403,6 +2448,21 @@ static int __init parse_amd_iommu_dump(char *str)
 	return 1;
 }
 
+static int __init parse_amd_iommu_intr(char *str)
+{
+	for (; *str; ++str) {
+		if (strncmp(str, "legacy", 6) == 0) {
+			amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY;
+			break;
+		}
+		if (strncmp(str, "vapic", 5) == 0) {
+			amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_VAPIC;
+			break;
+		}
+	}
+	return 1;
+}
+
 static int __init parse_amd_iommu_options(char *str)
 {
 	for (; *str; ++str) {
@@ -2511,6 +2571,7 @@ static int __init parse_ivrs_acpihid(char *str)
 
 __setup("amd_iommu_dump",	parse_amd_iommu_dump);
 __setup("amd_iommu=",		parse_amd_iommu_options);
+__setup("amd_iommu_intr=",	parse_amd_iommu_intr);
 __setup("ivrs_ioapic",		parse_ivrs_ioapic);
 __setup("ivrs_hpet",		parse_ivrs_hpet);
 __setup("ivrs_acpihid",		parse_ivrs_acpihid);
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 0bd9eb3..faa3b48 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -38,6 +38,7 @@ extern int amd_iommu_enable(void);
 extern void amd_iommu_disable(void);
 extern int amd_iommu_reenable(int);
 extern int amd_iommu_enable_faulting(void);
+extern int amd_iommu_guest_ir;
 
 /* IOMMUv2 specific functions */
 struct iommu_domain;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 590956a..25f939b 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -92,6 +92,7 @@
 #define FEATURE_GA		(1ULL<<7)
 #define FEATURE_HE		(1ULL<<8)
 #define FEATURE_PC		(1ULL<<9)
+#define FEATURE_GAM_VAPIC	(1ULL<<21)
 
 #define FEATURE_PASID_SHIFT	32
 #define FEATURE_PASID_MASK	(0x1fULL << FEATURE_PASID_SHIFT)
@@ -146,6 +147,8 @@
 #define CONTROL_PPFINT_EN       0x0eULL
 #define CONTROL_PPR_EN          0x0fULL
 #define CONTROL_GT_EN           0x10ULL
+#define CONTROL_GA_EN           0x11ULL
+#define CONTROL_GAM_EN          0x19ULL
 
 #define CTRL_INV_TO_MASK	(7 << CONTROL_INV_TIMEOUT)
 #define CTRL_INV_TO_NONE	0
@@ -329,6 +332,12 @@
 #define IOMMU_CAP_NPCACHE 26
 #define IOMMU_CAP_EFR     27
 
+/* IOMMU Feature Reporting Field (for IVHD type 10h */
+#define IOMMU_FEAT_GASUP_SHIFT	6
+
+/* IOMMU Extended Feature Register (EFR) */
+#define IOMMU_EFR_GASUP_SHIFT	7
+
 #define MAX_DOMAIN_ID 65536
 
 /* Protection domain flags */
@@ -682,4 +691,19 @@ static inline int get_hpet_devid(int id)
 	return -EINVAL;
 }
 
+enum amd_iommu_intr_mode_type {
+	AMD_IOMMU_GUEST_IR_LEGACY,
+
+	/* This mode is not visible to users. It is used when
+	 * we cannot fully enable vAPIC and fallback to only support
+	 * legacy interrupt remapping via 128-bit IRTE.
+	 */
+	AMD_IOMMU_GUEST_IR_LEGACY_GA,
+	AMD_IOMMU_GUEST_IR_VAPIC,
+};
+
+#define AMD_IOMMU_GUEST_IR_GA(x)	(x == AMD_IOMMU_GUEST_IR_VAPIC || \
+					 x == AMD_IOMMU_GUEST_IR_LEGACY_GA)
+
+#define AMD_IOMMU_GUEST_IR_VAPIC(x)	(x == AMD_IOMMU_GUEST_IR_VAPIC)
 #endif /* _ASM_X86_AMD_IOMMU_TYPES_H */
-- 
1.9.1

[PART2 PATCH v5 02/12] iommu/amd: Move and introduce new IRTE-related unions and structures

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Move existing unions and structs for accessing/managing IRTE to a proper
header file. This is mainly to simplify variable declarations in subsequent
patches.

Besides, this patch also introduces new struct irte_ga for the new
128-bit IRTE format.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c       | 28 ---------------
 drivers/iommu/amd_iommu_types.h | 78 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 28 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 634f636..ac2962f 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3693,34 +3693,6 @@ EXPORT_SYMBOL(amd_iommu_device_info);
  *
  *****************************************************************************/
 
-union irte {
-	u32 val;
-	struct {
-		u32 valid	: 1,
-		    no_fault	: 1,
-		    int_type	: 3,
-		    rq_eoi	: 1,
-		    dm		: 1,
-		    rsvd_1	: 1,
-		    destination	: 8,
-		    vector	: 8,
-		    rsvd_2	: 8;
-	} fields;
-};
-
-struct irq_2_irte {
-	u16 devid; /* Device ID for IRTE table */
-	u16 index; /* Index into IRTE table*/
-};
-
-struct amd_ir_data {
-	struct irq_2_irte			irq_2_irte;
-	union irte				irte_entry;
-	union {
-		struct msi_msg			msi_entry;
-	};
-};
-
 static struct irq_chip amd_ir_chip;
 
 #define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 25f939b..d9227a9 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -22,6 +22,7 @@
 
 #include <linux/types.h>
 #include <linux/mutex.h>
+#include <linux/msi.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
 #include <linux/pci.h>
@@ -706,4 +707,81 @@ enum amd_iommu_intr_mode_type {
 					 x == AMD_IOMMU_GUEST_IR_LEGACY_GA)
 
 #define AMD_IOMMU_GUEST_IR_VAPIC(x)	(x == AMD_IOMMU_GUEST_IR_VAPIC)
+
+union irte {
+	u32 val;
+	struct {
+		u32 valid	: 1,
+		    no_fault	: 1,
+		    int_type	: 3,
+		    rq_eoi	: 1,
+		    dm		: 1,
+		    rsvd_1	: 1,
+		    destination	: 8,
+		    vector	: 8,
+		    rsvd_2	: 8;
+	} fields;
+};
+
+union irte_ga_lo {
+	u64 val;
+
+	/* For int remapping */
+	struct {
+		u64 valid	: 1,
+		    no_fault	: 1,
+		    /* ------ */
+		    int_type	: 3,
+		    rq_eoi	: 1,
+		    dm		: 1,
+		    /* ------ */
+		    guest_mode	: 1,
+		    destination	: 8,
+		    rsvd	: 48;
+	} fields_remap;
+
+	/* For guest vAPIC */
+	struct {
+		u64 valid	: 1,
+		    no_fault	: 1,
+		    /* ------ */
+		    ga_log_intr	: 1,
+		    rsvd1	: 3,
+		    is_run	: 1,
+		    /* ------ */
+		    guest_mode	: 1,
+		    destination	: 8,
+		    rsvd2	: 16,
+		    ga_tag	: 32;
+	} fields_vapic;
+};
+
+union irte_ga_hi {
+	u64 val;
+	struct {
+		u64 vector	: 8,
+		    rsvd_1	: 4,
+		    ga_root_ptr	: 40,
+		    rsvd_2	: 12;
+	} fields;
+};
+
+struct irte_ga {
+	union irte_ga_lo lo;
+	union irte_ga_hi hi;
+};
+
+struct irq_2_irte {
+	u16 devid; /* Device ID for IRTE table */
+	u16 index; /* Index into IRTE table*/
+};
+
+struct amd_ir_data {
+	struct irq_2_irte irq_2_irte;
+	union irte irte_entry;
+	union {
+		struct msi_msg msi_entry;
+	};
+};
+
 #endif /* _ASM_X86_AMD_IOMMU_TYPES_H */
-- 
1.9.1

[PART2 PATCH v5 03/12] iommu/amd: Introduce interrupt remapping ops structure

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Currently, IOMMU support two interrupt remapping table entry formats,
32-bit (legacy) and 128-bit (GA). The spec also implies that it might
support additional modes/formats in the future.

So, this patch introduces the new struct amd_irte_ops, which allows
the same code to work with different irte formats by providing hooks
for various operations on an interrupt remapping table entry.

Suggested-by: Joerg Roedel <joro@8bytes.org>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c       | 193 ++++++++++++++++++++++++++++++++++++++--
 drivers/iommu/amd_iommu_types.h |  20 +++++
 2 files changed, 208 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index ac2962f..139ea8b 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3826,11 +3826,13 @@ out:
 	return index;
 }
 
-static int modify_irte(u16 devid, int index, union irte irte)
+static int modify_irte_ga(u16 devid, int index, struct irte_ga *irte)
 {
 	struct irq_remap_table *table;
 	struct amd_iommu *iommu;
 	unsigned long flags;
+	struct irte_ga *entry;
+	struct irte_ga tmp;
 
 	iommu = amd_iommu_rlookup_table[devid];
 	if (iommu == NULL)
@@ -3841,7 +3843,40 @@ static int modify_irte(u16 devid, int index, union irte irte)
 		return -ENOMEM;
 
 	spin_lock_irqsave(&table->lock, flags);
-	table->table[index] = irte.val;
+
+	entry = (struct irte_ga *)table->table;
+	entry = &entry[index];
+	memcpy(&tmp, entry, sizeof(struct irte_ga));
+	entry->lo.fields_remap.valid = 0;
+	entry->hi.val = irte->hi.val;
+	entry->hi.fields.ga_root_ptr = tmp.hi.fields.ga_root_ptr;
+	entry->lo.val = irte->lo.val;
+	entry->lo.fields_remap.valid = 1;
+
+	spin_unlock_irqrestore(&table->lock, flags);
+
+	iommu_flush_irt(iommu, devid);
+	iommu_completion_wait(iommu);
+
+	return 0;
+}
+
+static int modify_irte(u16 devid, int index, union irte *irte)
+{
+	struct irq_remap_table *table;
+	struct amd_iommu *iommu;
+	unsigned long flags;
+
+	iommu = amd_iommu_rlookup_table[devid];
+	if (iommu == NULL)
+		return -EINVAL;
+
+	table = get_irq_table(devid, false);
+	if (!table)
+		return -ENOMEM;
+
+	spin_lock_irqsave(&table->lock, flags);
+	table->table[index] = irte->val;
 	spin_unlock_irqrestore(&table->lock, flags);
 
 	iommu_flush_irt(iommu, devid);
@@ -3872,6 +3907,134 @@ static void free_irte(u16 devid, int index)
 	iommu_completion_wait(iommu);
 }
 
+static void irte_prepare(void *entry,
+			 u32 delivery_mode, u32 dest_mode,
+			 u8 vector, u32 dest_apicid)
+{
+	union irte *irte = (union irte *) entry;
+
+	irte->val                = 0;
+	irte->fields.vector      = vector;
+	irte->fields.int_type    = delivery_mode;
+	irte->fields.destination = dest_apicid;
+	irte->fields.dm          = dest_mode;
+	irte->fields.valid       = 1;
+}
+
+static void irte_ga_prepare(void *entry,
+			    u32 delivery_mode, u32 dest_mode,
+			    u8 vector, u32 dest_apicid)
+{
+	struct irte_ga *irte = (struct irte_ga *) entry;
+
+	irte->lo.val                      = 0;
+	irte->hi.val                      = 0;
+	irte->lo.fields_remap.guest_mode  = 0;
+	irte->lo.fields_remap.int_type    = delivery_mode;
+	irte->lo.fields_remap.dm          = dest_mode;
+	irte->hi.fields.vector            = vector;
+	irte->lo.fields_remap.destination = dest_apicid;
+	irte->lo.fields_remap.valid       = 1;
+}
+
+static void irte_activate(void *entry, u16 devid, u16 index)
+{
+	union irte *irte = (union irte *) entry;
+
+	irte->fields.valid = 1;
+	modify_irte(devid, index, irte);
+}
+
+static void irte_ga_activate(void *entry, u16 devid, u16 index)
+{
+	struct irte_ga *irte = (struct irte_ga *) entry;
+
+	irte->lo.fields_remap.valid = 1;
+	modify_irte_ga(devid, index, irte);
+}
+
+static void irte_deactivate(void *entry, u16 devid, u16 index)
+{
+	union irte *irte = (union irte *) entry;
+
+	irte->fields.valid = 0;
+	modify_irte(devid, index, irte);
+}
+
+static void irte_ga_deactivate(void *entry, u16 devid, u16 index)
+{
+	struct irte_ga *irte = (struct irte_ga *) entry;
+
+	irte->lo.fields_remap.valid = 0;
+	modify_irte_ga(devid, index, irte);
+}
+
+static void irte_set_affinity(void *entry, u16 devid, u16 index,
+			      u8 vector, u32 dest_apicid)
+{
+	union irte *irte = (union irte *) entry;
+
+	irte->fields.vector = vector;
+	irte->fields.destination = dest_apicid;
+	modify_irte(devid, index, irte);
+}
+
+static void irte_ga_set_affinity(void *entry, u16 devid, u16 index,
+				 u8 vector, u32 dest_apicid)
+{
+	struct irte_ga *irte = (struct irte_ga *) entry;
+
+	irte->hi.fields.vector = vector;
+	irte->lo.fields_remap.destination = dest_apicid;
+	irte->lo.fields_remap.guest_mode = 0;
+	modify_irte_ga(devid, index, irte);
+}
+
+static void irte_set_allocated(struct irq_remap_table *table, int index)
+{
+	table->table[index] = IRTE_ALLOCATED;
+}
+
+static void irte_ga_set_allocated(struct irq_remap_table *table, int index)
+{
+	struct irte_ga *ptr = (struct irte_ga *)table->table;
+	struct irte_ga *irte = &ptr[index];
+
+	memset(&irte->lo.val, 0, sizeof(u64));
+	memset(&irte->hi.val, 0, sizeof(u64));
+	irte->hi.fields.vector = 0xff;
+}
+
+static bool irte_is_allocated(struct irq_remap_table *table, int index)
+{
+	union irte *ptr = (union irte *)table->table;
+	union irte *irte = &ptr[index];
+
+	return irte->val != 0;
+}
+
+static bool irte_ga_is_allocated(struct irq_remap_table *table, int index)
+{
+	struct irte_ga *ptr = (struct irte_ga *)table->table;
+	struct irte_ga *irte = &ptr[index];
+
+	return irte->hi.fields.vector != 0;
+}
+
+static void irte_clear_allocated(struct irq_remap_table *table, int index)
+{
+	table->table[index] = 0;
+}
+
+static void irte_ga_clear_allocated(struct irq_remap_table *table, int index)
+{
+	struct irte_ga *ptr = (struct irte_ga *)table->table;
+	struct irte_ga *irte = &ptr[index];
+
+	memset(&irte->lo.val, 0, sizeof(u64));
+	memset(&irte->hi.val, 0, sizeof(u64));
+}
+
 static int get_devid(struct irq_alloc_info *info)
 {
 	int devid = -1;
@@ -3999,6 +4162,26 @@ static void irq_remapping_prepare_irte(struct amd_ir_data *data,
 	}
 }
 
+struct amd_irte_ops irte_32_ops = {
+	.prepare = irte_prepare,
+	.activate = irte_activate,
+	.deactivate = irte_deactivate,
+	.set_affinity = irte_set_affinity,
+	.set_allocated = irte_set_allocated,
+	.is_allocated = irte_is_allocated,
+	.clear_allocated = irte_clear_allocated,
+};
+
+struct amd_irte_ops irte_128_ops = {
+	.prepare = irte_ga_prepare,
+	.activate = irte_ga_activate,
+	.deactivate = irte_ga_deactivate,
+	.set_affinity = irte_ga_set_affinity,
+	.set_allocated = irte_ga_set_allocated,
+	.is_allocated = irte_ga_is_allocated,
+	.clear_allocated = irte_ga_clear_allocated,
+};
+
 static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq,
 			       unsigned int nr_irqs, void *arg)
 {
@@ -4104,7 +4287,7 @@ static void irq_remapping_activate(struct irq_domain *domain,
 	struct amd_ir_data *data = irq_data->chip_data;
 	struct irq_2_irte *irte_info = &data->irq_2_irte;
 
-	modify_irte(irte_info->devid, irte_info->index, data->irte_entry);
+	modify_irte(irte_info->devid, irte_info->index, &data->irte_entry);
 }
 
 static void irq_remapping_deactivate(struct irq_domain *domain,
@@ -4115,7 +4298,7 @@ static void irq_remapping_deactivate(struct irq_domain *domain,
 	union irte entry;
 
 	entry.val = 0;
-	modify_irte(irte_info->devid, irte_info->index, data->irte_entry);
+	modify_irte(irte_info->devid, irte_info->index, &data->irte_entry);
 }
 
 static struct irq_domain_ops amd_ir_domain_ops = {
@@ -4144,7 +4327,7 @@ static int amd_ir_set_affinity(struct irq_data *data,
 	 */
 	ir_data->irte_entry.fields.vector = cfg->vector;
 	ir_data->irte_entry.fields.destination = cfg->dest_apicid;
-	modify_irte(irte_info->devid, irte_info->index, ir_data->irte_entry);
+	modify_irte(irte_info->devid, irte_info->index, &ir_data->irte_entry);
 
 	/*
 	 * After this point, all the interrupts will start arriving
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index d9227a9..2a3819a 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -410,6 +410,7 @@ struct amd_iommu_fault {
 
 struct iommu_domain;
 struct irq_domain;
+struct amd_irte_ops;
 
 /*
  * This structure contains generic data for  IOMMU protection domains
@@ -534,6 +535,8 @@ struct amd_iommu {
 #ifdef CONFIG_IRQ_REMAP
 	struct irq_domain *ir_domain;
 	struct irq_domain *msi_domain;
+
+	struct amd_irte_ops *irte_ops;
 #endif
 };
 
@@ -776,12 +779,29 @@ struct irq_2_irte {
 	u16 index; /* Index into IRTE table*/
 };
 
+struct amd_irte_ops {
+	void (*prepare)(void *, u32, u32, u8, u32);
+	void (*activate)(void *, u16, u16);
+	void (*deactivate)(void *, u16, u16);
+	void (*set_affinity)(void *, u16, u16, u8, u32);
+	void *(*get)(struct irq_remap_table *, int);
+	void (*set_allocated)(struct irq_remap_table *, int);
+	bool (*is_allocated)(struct irq_remap_table *, int);
+	void (*clear_allocated)(struct irq_remap_table *, int);
+};
+
 struct amd_ir_data {
 	struct irq_2_irte irq_2_irte;
 	union irte irte_entry;
+	void *entry;	/* Pointer to union irte or struct irte_ga */
 	union {
 		struct msi_msg msi_entry;
 	};
 };
 
+#ifdef CONFIG_IRQ_REMAP
+extern struct amd_irte_ops irte_32_ops;
+extern struct amd_irte_ops irte_128_ops;
+#endif
+
 #endif /* _ASM_X86_AMD_IOMMU_TYPES_H */
-- 
1.9.1

[PART2 PATCH v5 04/12] iommu/amd: Add support for multiple IRTE formats

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch enables support for the new 128-bit IOMMU IRTE format,
which can be used for both legacy and vapic interrupt remapping modes.
It replaces the existing operations on IRTE, which can only support
the older 32-bit IRTE format, with calls to the new struct amd_irt_ops.

It also provides helper functions for setting up, accessing, and
updating interrupt remapping table entries in different mode.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c       | 72 +++++++++++++++++++++++++++--------------
 drivers/iommu/amd_iommu_init.c  |  2 ++
 drivers/iommu/amd_iommu_types.h |  1 -
 3 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 139ea8b..abfb2b7 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3714,8 +3714,6 @@ static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
 	amd_iommu_dev_table[devid].data[2] = dte;
 }
 
-#define IRTE_ALLOCATED (~1U)
-
 static struct irq_remap_table *get_irq_table(u16 devid, bool ioapic)
 {
 	struct irq_remap_table *table = NULL;
@@ -3761,13 +3759,18 @@ static struct irq_remap_table *get_irq_table(u16 devid, bool ioapic)
 		goto out;
 	}
 
-	memset(table->table, 0, MAX_IRQS_PER_TABLE * sizeof(u32));
+	if (!AMD_IOMMU_GUEST_IR_GA(amd_iommu_guest_ir))
+		memset(table->table, 0,
+		       MAX_IRQS_PER_TABLE * sizeof(u32));
+	else
+		memset(table->table, 0,
+		       (MAX_IRQS_PER_TABLE * (sizeof(u64) * 2)));
 
 	if (ioapic) {
 		int i;
 
 		for (i = 0; i < 32; ++i)
-			table->table[i] = IRTE_ALLOCATED;
+			iommu->irte_ops->set_allocated(table, i);
 	}
 
 	irq_lookup_table[devid] = table;
@@ -3793,6 +3796,10 @@ static int alloc_irq_index(u16 devid, int count)
 	struct irq_remap_table *table;
 	unsigned long flags;
 	int index, c;
+	struct amd_iommu *iommu = amd_iommu_rlookup_table[devid];
+
+	if (!iommu)
+		return -ENODEV;
 
 	table = get_irq_table(devid, false);
 	if (!table)
@@ -3804,14 +3811,14 @@ static int alloc_irq_index(u16 devid, int count)
 	for (c = 0, index = table->min_index;
 	     index < MAX_IRQS_PER_TABLE;
 	     ++index) {
-		if (table->table[index] == 0)
+		if (!iommu->irte_ops->is_allocated(table, index))
 			c += 1;
 		else
 			c = 0;
 
 		if (c == count)	{
 			for (; c != 0; --c)
-				table->table[index - c + 1] = IRTE_ALLOCATED;
+				iommu->irte_ops->set_allocated(table, index - c + 1);
 
 			index -= count - 1;
 			goto out;
@@ -3900,7 +3907,7 @@ static void free_irte(u16 devid, int index)
 		return;
 
 	spin_lock_irqsave(&table->lock, flags);
-	table->table[index] = 0;
+	iommu->irte_ops->clear_allocated(table, index);
 	spin_unlock_irqrestore(&table->lock, flags);
 
 	iommu_flush_irt(iommu, devid);
@@ -3990,6 +3997,7 @@ static void irte_ga_set_affinity(void *entry, u16 devid, u16 index,
 	modify_irte_ga(devid, index, irte);
 }
 
+#define IRTE_ALLOCATED (~1U)
 static void irte_set_allocated(struct irq_remap_table *table, int index)
 {
 	table->table[index] = IRTE_ALLOCATED;
@@ -4119,19 +4127,17 @@ static void irq_remapping_prepare_irte(struct amd_ir_data *data,
 {
 	struct irq_2_irte *irte_info = &data->irq_2_irte;
 	struct msi_msg *msg = &data->msi_entry;
-	union irte *irte = &data->irte_entry;
 	struct IO_APIC_route_entry *entry;
+	struct amd_iommu *iommu = amd_iommu_rlookup_table[devid];
+
+	if (!iommu)
+		return;
 
 	data->irq_2_irte.devid = devid;
 	data->irq_2_irte.index = index + sub_handle;
-
-	/* Setup IRTE for IOMMU */
-	irte->val = 0;
-	irte->fields.vector      = irq_cfg->vector;
-	irte->fields.int_type    = apic->irq_delivery_mode;
-	irte->fields.destination = irq_cfg->dest_apicid;
-	irte->fields.dm          = apic->irq_dest_mode;
-	irte->fields.valid       = 1;
+	iommu->irte_ops->prepare(data->entry, apic->irq_delivery_mode,
+				 apic->irq_dest_mode, irq_cfg->vector,
+				 irq_cfg->dest_apicid);
 
 	switch (info->type) {
 	case X86_IRQ_ALLOC_TYPE_IOAPIC:
@@ -4187,7 +4193,7 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq,
 {
 	struct irq_alloc_info *info = arg;
 	struct irq_data *irq_data;
-	struct amd_ir_data *data;
+	struct amd_ir_data *data = NULL;
 	struct irq_cfg *cfg;
 	int i, ret, devid;
 	int index = -1;
@@ -4239,6 +4245,16 @@ static int irq_remapping_alloc(struct irq_domain *domain, unsigned int virq,
 		if (!data)
 			goto out_free_data;
 
+		if (!AMD_IOMMU_GUEST_IR_GA(amd_iommu_guest_ir))
+			data->entry = kzalloc(sizeof(union irte), GFP_KERNEL);
+		else
+			data->entry = kzalloc(sizeof(struct irte_ga),
+						     GFP_KERNEL);
+		if (!data->entry) {
+			kfree(data);
+			goto out_free_data;
+		}
+
 		irq_data->hwirq = (devid << 16) + i;
 		irq_data->chip_data = data;
 		irq_data->chip = &amd_ir_chip;
@@ -4275,6 +4291,7 @@ static void irq_remapping_free(struct irq_domain *domain, unsigned int virq,
 			data = irq_data->chip_data;
 			irte_info = &data->irq_2_irte;
 			free_irte(irte_info->devid, irte_info->index);
+			kfree(data->entry);
 			kfree(data);
 		}
 	}
@@ -4286,8 +4303,11 @@ static void irq_remapping_activate(struct irq_domain *domain,
 {
 	struct amd_ir_data *data = irq_data->chip_data;
 	struct irq_2_irte *irte_info = &data->irq_2_irte;
+	struct amd_iommu *iommu = amd_iommu_rlookup_table[irte_info->devid];
 
-	modify_irte(irte_info->devid, irte_info->index, &data->irte_entry);
+	if (iommu)
+		iommu->irte_ops->activate(data->entry, irte_info->devid,
+					  irte_info->index);
 }
 
 static void irq_remapping_deactivate(struct irq_domain *domain,
@@ -4295,10 +4315,11 @@ static void irq_remapping_deactivate(struct irq_domain *domain,
 {
 	struct amd_ir_data *data = irq_data->chip_data;
 	struct irq_2_irte *irte_info = &data->irq_2_irte;
-	union irte entry;
+	struct amd_iommu *iommu = amd_iommu_rlookup_table[irte_info->devid];
 
-	entry.val = 0;
-	modify_irte(irte_info->devid, irte_info->index, &data->irte_entry);
+	if (iommu)
+		iommu->irte_ops->deactivate(data->entry, irte_info->devid,
+					    irte_info->index);
 }
 
 static struct irq_domain_ops amd_ir_domain_ops = {
@@ -4315,8 +4336,12 @@ static int amd_ir_set_affinity(struct irq_data *data,
 	struct irq_2_irte *irte_info = &ir_data->irq_2_irte;
 	struct irq_cfg *cfg = irqd_cfg(data);
 	struct irq_data *parent = data->parent_data;
+	struct amd_iommu *iommu = amd_iommu_rlookup_table[irte_info->devid];
 	int ret;
 
+	if (!iommu)
+		return -ENODEV;
+
 	ret = parent->chip->irq_set_affinity(parent, mask, force);
 	if (ret < 0 || ret == IRQ_SET_MASK_OK_DONE)
 		return ret;
@@ -4325,9 +4350,8 @@ static int amd_ir_set_affinity(struct irq_data *data,
 	 * Atomically updates the IRTE with the new destination, vector
 	 * and flushes the interrupt entry cache.
 	 */
-	ir_data->irte_entry.fields.vector = cfg->vector;
-	ir_data->irte_entry.fields.destination = cfg->dest_apicid;
-	modify_irte(irte_info->devid, irte_info->index, &ir_data->irte_entry);
+	iommu->irte_ops->set_affinity(ir_data->entry, irte_info->devid,
+			    irte_info->index, cfg->vector, cfg->dest_apicid);
 
 	/*
 	 * After this point, all the interrupts will start arriving
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index d0930b1..6635cdf 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1883,8 +1883,10 @@ static void iommu_enable_ga(struct amd_iommu *iommu)
 		/* Fall through */
 	case AMD_IOMMU_GUEST_IR_LEGACY_GA:
 		iommu_feature_enable(iommu, CONTROL_GA_EN);
+		iommu->irte_ops = &irte_128_ops;
 		break;
 	default:
+		iommu->irte_ops = &irte_32_ops;
 		break;
 	}
 #endif
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 2a3819a..4fad61a 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -792,7 +792,6 @@ struct amd_irte_ops {
 
 struct amd_ir_data {
 	struct irq_2_irte irq_2_irte;
-	union irte irte_entry;
 	void *entry;	/* Pointer to union irte or struct irte_ga */
 	union {
 		struct msi_msg msi_entry;
-- 
1.9.1

[PART2 PATCH v5 05/12] iommu/amd: Detect and initialize guest vAPIC log

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch adds support to detect and initialize IOMMU Guest vAPIC log
(GALOG). By default, it also enable GALog interrupt to notify IOMMU driver
when GA Log entry is created.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu_init.c  | 112 +++++++++++++++++++++++++++++++++++++---
 drivers/iommu/amd_iommu_types.h |  28 ++++++++++
 2 files changed, 133 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 6635cdf..c32d0b3 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -84,6 +84,7 @@
 #define ACPI_DEVFLAG_LINT1              0x80
 #define ACPI_DEVFLAG_ATSDIS             0x10000000
 
+#define LOOP_TIMEOUT	100000
 /*
  * ACPI table definitions
  *
@@ -388,6 +389,10 @@ static void iommu_disable(struct amd_iommu *iommu)
 	iommu_feature_disable(iommu, CONTROL_EVT_INT_EN);
 	iommu_feature_disable(iommu, CONTROL_EVT_LOG_EN);
 
+	/* Disable IOMMU GA_LOG */
+	iommu_feature_disable(iommu, CONTROL_GALOG_EN);
+	iommu_feature_disable(iommu, CONTROL_GAINT_EN);
+
 	/* Disable IOMMU hardware itself */
 	iommu_feature_disable(iommu, CONTROL_IOMMU_EN);
 }
@@ -673,6 +678,99 @@ static void __init free_ppr_log(struct amd_iommu *iommu)
 	free_pages((unsigned long)iommu->ppr_log, get_order(PPR_LOG_SIZE));
 }
 
+static void free_ga_log(struct amd_iommu *iommu)
+{
+#ifdef CONFIG_IRQ_REMAP
+	if (iommu->ga_log)
+		free_pages((unsigned long)iommu->ga_log,
+			    get_order(GA_LOG_SIZE));
+	if (iommu->ga_log_tail)
+		free_pages((unsigned long)iommu->ga_log_tail,
+			    get_order(8));
+#endif
+}
+
+static int iommu_ga_log_enable(struct amd_iommu *iommu)
+{
+#ifdef CONFIG_IRQ_REMAP
+	u32 status, i;
+
+	if (!iommu->ga_log)
+		return -EINVAL;
+
+	status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
+
+	/* Check if already running */
+	if (status & (MMIO_STATUS_GALOG_RUN_MASK))
+		return 0;
+
+	iommu_feature_enable(iommu, CONTROL_GAINT_EN);
+	iommu_feature_enable(iommu, CONTROL_GALOG_EN);
+
+	for (i = 0; i < LOOP_TIMEOUT; ++i) {
+		status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
+		if (status & (MMIO_STATUS_GALOG_RUN_MASK))
+			break;
+	}
+
+	if (i >= LOOP_TIMEOUT)
+		return -EINVAL;
+#endif /* CONFIG_IRQ_REMAP */
+	return 0;
+}
+
+#ifdef CONFIG_IRQ_REMAP
+static int iommu_init_ga_log(struct amd_iommu *iommu)
+{
+	u64 entry;
+
+	if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
+		return 0;
+
+	iommu->ga_log = (u8 *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+					get_order(GA_LOG_SIZE));
+	if (!iommu->ga_log)
+		goto err_out;
+
+	iommu->ga_log_tail = (u8 *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+					get_order(8));
+	if (!iommu->ga_log_tail)
+		goto err_out;
+
+	entry = (u64)virt_to_phys(iommu->ga_log) | GA_LOG_SIZE_512;
+	memcpy_toio(iommu->mmio_base + MMIO_GA_LOG_BASE_OFFSET,
+		    &entry, sizeof(entry));
+	entry = ((u64)virt_to_phys(iommu->ga_log) & 0xFFFFFFFFFFFFFULL) & ~7ULL;
+	memcpy_toio(iommu->mmio_base + MMIO_GA_LOG_TAIL_OFFSET,
+		    &entry, sizeof(entry));
+	writel(0x00, iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
+	writel(0x00, iommu->mmio_base + MMIO_GA_TAIL_OFFSET);
+
+	return 0;
+err_out:
+	free_ga_log(iommu);
+	return -EINVAL;
+}
+#endif /* CONFIG_IRQ_REMAP */
+
+static int iommu_init_ga(struct amd_iommu *iommu)
+{
+	int ret = 0;
+
+#ifdef CONFIG_IRQ_REMAP
+	/* Note: We have already checked GASup from IVRS table.
+	 *       Now, we need to make sure that GAMSup is set.
+	 */
+	if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) &&
+	    !iommu_feature(iommu, FEATURE_GAM_VAPIC))
+		amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;
+
+	ret = iommu_init_ga_log(iommu);
+#endif /* CONFIG_IRQ_REMAP */
+
+	return ret;
+}
+
 static void iommu_enable_gt(struct amd_iommu *iommu)
 {
 	if (!iommu_feature(iommu, FEATURE_GT))
@@ -1146,6 +1244,7 @@ static void __init free_iommu_one(struct amd_iommu *iommu)
 	free_command_buffer(iommu);
 	free_event_buffer(iommu);
 	free_ppr_log(iommu);
+	free_ga_log(iommu);
 	iommu_unmap_mmio_space(iommu);
 }
 
@@ -1438,6 +1537,7 @@ static int iommu_init_pci(struct amd_iommu *iommu)
 {
 	int cap_ptr = iommu->cap_ptr;
 	u32 range, misc, low, high;
+	int ret;
 
 	iommu->dev = pci_get_bus_and_slot(PCI_BUS_NUM(iommu->devid),
 					  iommu->devid & 0xff);
@@ -1494,13 +1594,9 @@ static int iommu_init_pci(struct amd_iommu *iommu)
 	if (iommu_feature(iommu, FEATURE_PPR) && alloc_ppr_log(iommu))
 		return -ENOMEM;
 
-	/* Note: We have already checked GASup from IVRS table.
-	 *       Now, we need to make sure that GAMSup is set.
-	 */
-	if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) &&
-	    !iommu_feature(iommu, FEATURE_GAM_VAPIC))
-		amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;
-
+	ret = iommu_init_ga(iommu);
+	if (ret)
+		return ret;
 
 	if (iommu->cap & (1UL << IOMMU_CAP_NPCACHE))
 		amd_iommu_np_cache = true;
@@ -1657,6 +1753,8 @@ enable_faults:
 	if (iommu->ppr_log != NULL)
 		iommu_feature_enable(iommu, CONTROL_PPFINT_EN);
 
+	iommu_ga_log_enable(iommu);
+
 	return 0;
 }
 
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 4fad61a..947c74a 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -70,6 +70,8 @@
 #define MMIO_EXCL_LIMIT_OFFSET  0x0028
 #define MMIO_EXT_FEATURES	0x0030
 #define MMIO_PPR_LOG_OFFSET	0x0038
+#define MMIO_GA_LOG_BASE_OFFSET	0x00e0
+#define MMIO_GA_LOG_TAIL_OFFSET	0x00e8
 #define MMIO_CMD_HEAD_OFFSET	0x2000
 #define MMIO_CMD_TAIL_OFFSET	0x2008
 #define MMIO_EVT_HEAD_OFFSET	0x2010
@@ -77,6 +79,8 @@
 #define MMIO_STATUS_OFFSET	0x2020
 #define MMIO_PPR_HEAD_OFFSET	0x2030
 #define MMIO_PPR_TAIL_OFFSET	0x2038
+#define MMIO_GA_HEAD_OFFSET	0x2040
+#define MMIO_GA_TAIL_OFFSET	0x2048
 #define MMIO_CNTR_CONF_OFFSET	0x4000
 #define MMIO_CNTR_REG_OFFSET	0x40000
 #define MMIO_REG_END_OFFSET	0x80000
@@ -112,6 +116,9 @@
 #define MMIO_STATUS_EVT_INT_MASK	(1 << 1)
 #define MMIO_STATUS_COM_WAIT_INT_MASK	(1 << 2)
 #define MMIO_STATUS_PPR_INT_MASK	(1 << 6)
+#define MMIO_STATUS_GALOG_RUN_MASK	(1 << 8)
+#define MMIO_STATUS_GALOG_OVERFLOW_MASK	(1 << 9)
+#define MMIO_STATUS_GALOG_INT_MASK	(1 << 10)
 
 /* event logging constants */
 #define EVENT_ENTRY_SIZE	0x10
@@ -150,6 +157,8 @@
 #define CONTROL_GT_EN           0x10ULL
 #define CONTROL_GA_EN           0x11ULL
 #define CONTROL_GAM_EN          0x19ULL
+#define CONTROL_GALOG_EN        0x1CULL
+#define CONTROL_GAINT_EN        0x1DULL
 
 #define CTRL_INV_TO_MASK	(7 << CONTROL_INV_TIMEOUT)
 #define CTRL_INV_TO_NONE	0
@@ -228,6 +237,19 @@
 
 #define PPR_REQ_FAULT		0x01
 
+/* Constants for GA Log handling */
+#define GA_LOG_ENTRIES		512
+#define GA_LOG_SIZE_SHIFT	56
+#define GA_LOG_SIZE_512		(0x8ULL << GA_LOG_SIZE_SHIFT)
+#define GA_ENTRY_SIZE		8
+#define GA_LOG_SIZE		(GA_ENTRY_SIZE * GA_LOG_ENTRIES)
+
+#define GA_TAG(x)		(u32)(x & 0xffffffffULL)
+#define GA_DEVID(x)		(u16)(((x) >> 32) & 0xffffULL)
+#define GA_REQ_TYPE(x)		(((x) >> 60) & 0xfULL)
+
+#define GA_GUEST_NR		0x1
+
 #define PAGE_MODE_NONE    0x00
 #define PAGE_MODE_1_LEVEL 0x01
 #define PAGE_MODE_2_LEVEL 0x02
@@ -502,6 +524,12 @@ struct amd_iommu {
 	/* Base of the PPR log, if present */
 	u8 *ppr_log;
 
+	/* Base of the GA log, if present */
+	u8 *ga_log;
+
+	/* Tail of the GA log, if present */
+	u8 *ga_log_tail;
+
 	/* true if interrupts for this IOMMU are already enabled */
 	bool int_enabled;
 
-- 
1.9.1

[PART2 PATCH v5 06/12] iommu/amd: Adding GALOG interrupt handler

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch adds AMD IOMMU guest virtual APIC log (GALOG) handler.
When IOMMU hardware receives an interrupt targeting a blocking vcpu,
it creates an entry in the GALOG, and generates an interrupt to notify
the AMD IOMMU driver.

At this point, the driver processes the log entry, and notify the SVM
driver via the registered iommu_ga_log_notifier function.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c | 77 +++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/amd-iommu.h | 20 ++++++++++--
 2 files changed, 91 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index abfb2b7..861d723 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -741,14 +741,78 @@ static void iommu_poll_ppr_log(struct amd_iommu *iommu)
 	}
 }
 
+#ifdef CONFIG_IRQ_REMAP
+static int (*iommu_ga_log_notifier)(u32);
+
+int amd_iommu_register_ga_log_notifier(int (*notifier)(u32))
+{
+	iommu_ga_log_notifier = notifier;
+
+	return 0;
+}
+EXPORT_SYMBOL(amd_iommu_register_ga_log_notifier);
+
+static void iommu_poll_ga_log(struct amd_iommu *iommu)
+{
+	u32 head, tail, cnt = 0;
+
+	if (iommu->ga_log == NULL)
+		return;
+
+	head = readl(iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
+	tail = readl(iommu->mmio_base + MMIO_GA_TAIL_OFFSET);
+
+	while (head != tail) {
+		volatile u64 *raw;
+		u64 log_entry;
+
+		raw = (u64 *)(iommu->ga_log + head);
+		cnt++;
+
+		/* Avoid memcpy function-call overhead */
+		log_entry = *raw;
+
+		/* Update head pointer of hardware ring-buffer */
+		head = (head + GA_ENTRY_SIZE) % GA_LOG_SIZE;
+		writel(head, iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
+
+		/* Handle GA entry */
+		switch (GA_REQ_TYPE(log_entry)) {
+		case GA_GUEST_NR:
+			if (!iommu_ga_log_notifier)
+				break;
+
+			pr_debug("AMD-Vi: %s: devid=%#x, ga_tag=%#x\n",
+				 __func__, GA_DEVID(log_entry),
+				 GA_TAG(log_entry));
+
+			if (iommu_ga_log_notifier(GA_TAG(log_entry)) != 0)
+				pr_err("AMD-Vi: GA log notifier failed.\n");
+			break;
+		default:
+			break;
+		}
+
+		/* Refresh ring-buffer information */
+		head = readl(iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
+		tail = readl(iommu->mmio_base + MMIO_GA_TAIL_OFFSET);
+	}
+}
+#endif /* CONFIG_IRQ_REMAP */
+
+#define AMD_IOMMU_INT_MASK	\
+	(MMIO_STATUS_EVT_INT_MASK | \
+	 MMIO_STATUS_PPR_INT_MASK | \
+	 MMIO_STATUS_GALOG_INT_MASK)
+
 irqreturn_t amd_iommu_int_thread(int irq, void *data)
 {
 	struct amd_iommu *iommu = (struct amd_iommu *) data;
 	u32 status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
 
-	while (status & (MMIO_STATUS_EVT_INT_MASK | MMIO_STATUS_PPR_INT_MASK)) {
-		/* Enable EVT and PPR interrupts again */
-		writel((MMIO_STATUS_EVT_INT_MASK | MMIO_STATUS_PPR_INT_MASK),
+	while (status & AMD_IOMMU_INT_MASK) {
+		/* Enable EVT and PPR and GA interrupts again */
+		writel(AMD_IOMMU_INT_MASK,
 			iommu->mmio_base + MMIO_STATUS_OFFSET);
 
 		if (status & MMIO_STATUS_EVT_INT_MASK) {
@@ -761,6 +825,13 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
 			iommu_poll_ppr_log(iommu);
 		}
 
+#ifdef CONFIG_IRQ_REMAP
+		if (status & MMIO_STATUS_GALOG_INT_MASK) {
+			pr_devel("AMD-Vi: Processing IOMMU GA Log\n");
+			iommu_poll_ga_log(iommu);
+		}
+#endif
+
 		/*
 		 * Hardware bug: ERBT1312
 		 * When re-enabling interrupt (by writing 1
diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h
index 2b08e79..465d096 100644
--- a/include/linux/amd-iommu.h
+++ b/include/linux/amd-iommu.h
@@ -168,11 +168,25 @@ typedef void (*amd_iommu_invalidate_ctx)(struct pci_dev *pdev, int pasid);
 
 extern int amd_iommu_set_invalidate_ctx_cb(struct pci_dev *pdev,
 					   amd_iommu_invalidate_ctx cb);
-
-#else
+#else /* CONFIG_AMD_IOMMU */
 
 static inline int amd_iommu_detect(void) { return -ENODEV; }
 
-#endif
+#endif /* CONFIG_AMD_IOMMU */
+
+#if defined(CONFIG_AMD_IOMMU) && defined(CONFIG_IRQ_REMAP)
+
+/* IOMMU AVIC Function */
+extern int amd_iommu_register_ga_log_notifier(int (*notifier)(u32));
+
+#else /* defined(CONFIG_AMD_IOMMU) && defined(CONFIG_IRQ_REMAP) */
+
+static inline int
+amd_iommu_register_ga_log_notifier(int (*notifier)(u32))
+{
+	return 0;
+}
+
+#endif /* defined(CONFIG_AMD_IOMMU) && defined(CONFIG_IRQ_REMAP) */
 
 #endif /* _ASM_X86_AMD_IOMMU_H */
-- 
1.9.1

[PART2 PATCH v5 07/12] iommu/amd: Introduce amd_iommu_update_ga()

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Introduces a new IOMMU API, amd_iommu_update_ga(), which allows
KVM (SVM) to update existing posted interrupt IOMMU IRTE when
load/unload vcpu.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c       | 50 +++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_types.h |  1 +
 include/linux/amd-iommu.h       | 22 ++++++++++++++++++
 3 files changed, 73 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 861d723..e6a271c 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -4458,4 +4458,54 @@ int amd_iommu_create_irq_domain(struct amd_iommu *iommu)
 
 	return 0;
 }
+
+static int
+update_irte_ga(struct irte_ga *irte, unsigned int devid,
+	       u64 base, int cpu, bool is_run)
+{
+	struct irq_remap_table *irt = get_irq_table(devid, false);
+	unsigned long flags;
+
+	if (!irt)
+		return -ENODEV;
+
+	spin_lock_irqsave(&irt->lock, flags);
+
+	if (irte->lo.fields_vapic.guest_mode) {
+		irte->hi.fields.ga_root_ptr = (base >> 12);
+		if (cpu >= 0)
+			irte->lo.fields_vapic.destination = cpu;
+		irte->lo.fields_vapic.is_run = is_run;
+		barrier();
+	}
+
+	spin_unlock_irqrestore(&irt->lock, flags);
+
+	return 0;
+}
+
+int amd_iommu_update_ga(u32 cpu, u64 base, bool is_run,
+			struct amd_iommu_pi_data *pi)
+{
+	struct amd_ir_data *ir_data = pi->ir_data;
+	int devid = ir_data->irq_2_irte.devid;
+	struct irte_ga *irte = (struct irte_ga *) ir_data->entry;
+	struct amd_iommu *iommu;
+
+	if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
+		return 0;
+
+	iommu = amd_iommu_rlookup_table[devid];
+	if (!iommu)
+		return -ENODEV;
+
+	if (!irte->lo.fields_vapic.guest_mode)
+		return 0;
+
+	update_irte_ga((struct irte_ga *)ir_data->ref, devid, base, cpu, is_run);
+	iommu_flush_irt(iommu, devid);
+	iommu_completion_wait(iommu);
+	return 0;
+}
+EXPORT_SYMBOL(amd_iommu_update_ga);
 #endif
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 947c74a..623ee9e 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -824,6 +824,7 @@ struct amd_ir_data {
 	union {
 		struct msi_msg msi_entry;
 	};
+	void *ref;	/* Pointer to the actual irte */
 };
 
 #ifdef CONFIG_IRQ_REMAP
diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h
index 465d096..2094d90 100644
--- a/include/linux/amd-iommu.h
+++ b/include/linux/amd-iommu.h
@@ -22,6 +22,19 @@
 
 #include <linux/types.h>
 
+/*
+ * This is mainly used to communicate information back-and-forth
+ * between SVM and IOMMU for setting up and tearing down posted
+ * interrupt
+ */
+struct amd_iommu_pi_data {
+	struct list_head node;		/* For per-vcpu pi_list */
+	u32 ga_tag;
+	bool is_guest_mode;
+	struct vcpu_data *vcpu_data;
+	struct amd_ir_data *ir_data;
+};
+
 #ifdef CONFIG_AMD_IOMMU
 
 struct task_struct;
@@ -179,6 +192,9 @@ static inline int amd_iommu_detect(void) { return -ENODEV; }
 /* IOMMU AVIC Function */
 extern int amd_iommu_register_ga_log_notifier(int (*notifier)(u32));
 
+extern int
+amd_iommu_update_ga(u32 cpu, u64 base, bool is_run, struct amd_iommu_pi_data *pi);
+
 #else /* defined(CONFIG_AMD_IOMMU) && defined(CONFIG_IRQ_REMAP) */
 
 static inline int
@@ -187,6 +203,12 @@ amd_iommu_register_ga_log_notifier(int (*notifier)(u32))
 	return 0;
 }
 
+static inline int
+amd_iommu_update_ga(u32 cpu, u64 base, bool is_run, struct amd_iommu_pi_data *pi)
+{
+	return 0;
+}
+
 #endif /* defined(CONFIG_AMD_IOMMU) && defined(CONFIG_IRQ_REMAP) */
 
 #endif /* _ASM_X86_AMD_IOMMU_H */
-- 
1.9.1

[PART2 PATCH v5 08/12] iommu/amd: Implements irq_set_vcpu_affinity() hook to setup vapic mode for pass-through devices

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch implements irq_set_vcpu_affinity() function to set up interrupt
remapping table entry with vapic mode for pass-through devices.

In case requirements for vapic mode are not met, it falls back to set up
the IRTE in legacy mode.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c       | 67 ++++++++++++++++++++++++++++++++++++++---
 drivers/iommu/amd_iommu_types.h |  1 +
 2 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index e6a271c..a044ce4 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3904,7 +3904,8 @@ out:
 	return index;
 }
 
-static int modify_irte_ga(u16 devid, int index, struct irte_ga *irte)
+static int modify_irte_ga(u16 devid, int index, struct irte_ga *irte,
+			  struct amd_ir_data *data)
 {
 	struct irq_remap_table *table;
 	struct amd_iommu *iommu;
@@ -3930,6 +3931,8 @@ static int modify_irte_ga(u16 devid, int index, struct irte_ga *irte)
 	entry->hi.fields.ga_root_ptr = tmp.hi.fields.ga_root_ptr;
 	entry->lo.val = irte->lo.val;
 	entry->lo.fields_remap.valid = 1;
+	if (data)
+		data->ref = entry;
 
 	spin_unlock_irqrestore(&table->lock, flags);
 
@@ -4028,7 +4031,7 @@ static void irte_ga_activate(void *entry, u16 devid, u16 index)
 	struct irte_ga *irte = (struct irte_ga *) entry;
 
 	irte->lo.fields_remap.valid = 1;
-	modify_irte_ga(devid, index, irte);
+	modify_irte_ga(devid, index, irte, NULL);
 }
 
 static void irte_deactivate(void *entry, u16 devid, u16 index)
@@ -4044,7 +4047,7 @@ static void irte_ga_deactivate(void *entry, u16 devid, u16 index)
 	struct irte_ga *irte = (struct irte_ga *) entry;
 
 	irte->lo.fields_remap.valid = 0;
-	modify_irte_ga(devid, index, irte);
+	modify_irte_ga(devid, index, irte, NULL);
 }
 
 static void irte_set_affinity(void *entry, u16 devid, u16 index,
@@ -4065,7 +4068,7 @@ static void irte_ga_set_affinity(void *entry, u16 devid, u16 index,
 	irte->hi.fields.vector = vector;
 	irte->lo.fields_remap.destination = dest_apicid;
 	irte->lo.fields_remap.guest_mode = 0;
-	modify_irte_ga(devid, index, irte);
+	modify_irte_ga(devid, index, irte, NULL);
 }
 
 #define IRTE_ALLOCATED (~1U)
@@ -4400,6 +4403,61 @@ static struct irq_domain_ops amd_ir_domain_ops = {
 	.deactivate = irq_remapping_deactivate,
 };
 
+static int amd_ir_set_vcpu_affinity(struct irq_data *data, void *vcpu_info)
+{
+	struct amd_iommu *iommu;
+	struct amd_iommu_pi_data *pi_data = vcpu_info;
+	struct vcpu_data *vcpu_pi_info = pi_data->vcpu_data;
+	struct amd_ir_data *ir_data = data->chip_data;
+	struct irte_ga *irte = (struct irte_ga *) ir_data->entry;
+	struct irq_2_irte *irte_info = &ir_data->irq_2_irte;
+
+	pi_data->ir_data = ir_data;
+
+	/* Note:
+	 * SVM tries to set up for VAPIC mode, but we are in
+	 * legacy mode. So, we force legacy mode instead.
+	 */
+	if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir)) {
+		pr_debug("AMD-Vi: %s: Fall back to using intr legacy remap\n",
+			 __func__);
+		pi_data->is_guest_mode = false;
+	}
+
+	iommu = amd_iommu_rlookup_table[irte_info->devid];
+	if (iommu == NULL)
+		return -EINVAL;
+
+	if (pi_data->is_guest_mode) {
+		/* Setting */
+		irte->hi.fields.vector = vcpu_pi_info->vector;
+		irte->lo.fields_vapic.guest_mode = 1;
+		irte->lo.fields_vapic.ga_tag = pi_data->ga_tag;
+
+		ir_data->cached_ga_tag = pi_data->ga_tag;
+	} else {
+		/* Un-Setting */
+		struct irq_cfg *cfg = irqd_cfg(data);
+
+		irte->hi.val = 0;
+		irte->lo.val = 0;
+		irte->hi.fields.vector = cfg->vector;
+		irte->lo.fields_remap.guest_mode = 0;
+		irte->lo.fields_remap.destination = cfg->dest_apicid;
+		irte->lo.fields_remap.int_type = apic->irq_delivery_mode;
+		irte->lo.fields_remap.dm = apic->irq_dest_mode;
+
+		/*
+		 * This communicates the ga_tag back to the caller
+		 * so that it can do all the necessary clean up.
+		 */
+		pi_data->ga_tag = ir_data->cached_ga_tag;
+		ir_data->cached_ga_tag = 0;
+	}
+
+	return modify_irte_ga(irte_info->devid, irte_info->index, irte, ir_data);
+}
+
 static int amd_ir_set_affinity(struct irq_data *data,
 			       const struct cpumask *mask, bool force)
 {
@@ -4444,6 +4502,7 @@ static void ir_compose_msi_msg(struct irq_data *irq_data, struct msi_msg *msg)
 static struct irq_chip amd_ir_chip = {
 	.irq_ack = ir_ack_apic_edge,
 	.irq_set_affinity = amd_ir_set_affinity,
+	.irq_set_vcpu_affinity = amd_ir_set_vcpu_affinity,
 	.irq_compose_msi_msg = ir_compose_msi_msg,
 };
 
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 623ee9e..35dfe02 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -819,6 +819,7 @@ struct amd_irte_ops {
 };
 
 struct amd_ir_data {
+	u32 cached_ga_tag;
 	struct irq_2_irte irq_2_irte;
 	void *entry;	/* Pointer to union irte or struct irte_ga */
 	union {
-- 
1.9.1

[PART2 PATCH v5 09/12] iommu/amd: Enable vAPIC interrupt remapping mode by default

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Introduce struct iommu_dev_data.use_vapic flag, which IOMMU driver
uses to determine if it should enable vAPIC support, by setting
the ga_mode bit in the device's interrupt remapping table entry.

Currently, it is enabled for all pass-through device if vAPIC mode
is enabled.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c       | 44 +++++++++++++++++++++++++++++++++--------
 drivers/iommu/amd_iommu_init.c  | 12 ++++++++++-
 drivers/iommu/amd_iommu_types.h |  2 +-
 3 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index a044ce4..155b4a6 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -105,6 +105,7 @@ struct iommu_dev_data {
 	bool pri_tlp;			  /* PASID TLB required for
 					     PPR completions */
 	u32 errata;			  /* Bitmap for errata to apply */
+	u32 use_vapic;			  /* Enable device to use vapic mode */
 };
 
 /*
@@ -3215,6 +3216,12 @@ static void amd_iommu_detach_device(struct iommu_domain *dom,
 	if (!iommu)
 		return;
 
+#ifdef CONFIG_IRQ_REMAP
+	if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) &&
+	    (dom->type == IOMMU_DOMAIN_UNMANAGED))
+		dev_data->use_vapic = 0;
+#endif
+
 	iommu_completion_wait(iommu);
 }
 
@@ -3240,6 +3247,15 @@ static int amd_iommu_attach_device(struct iommu_domain *dom,
 
 	ret = attach_device(dev, domain);
 
+#ifdef CONFIG_IRQ_REMAP
+	if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir)) {
+		if (dom->type == IOMMU_DOMAIN_UNMANAGED)
+			dev_data->use_vapic = 1;
+		else
+			dev_data->use_vapic = 0;
+	}
+#endif
+
 	iommu_completion_wait(iommu);
 
 	return ret;
@@ -3990,7 +4006,7 @@ static void free_irte(u16 devid, int index)
 
 static void irte_prepare(void *entry,
 			 u32 delivery_mode, u32 dest_mode,
-			 u8 vector, u32 dest_apicid)
+			 u8 vector, u32 dest_apicid, int devid)
 {
 	union irte *irte = (union irte *) entry;
 
@@ -4004,13 +4020,14 @@ static void irte_prepare(void *entry,
 
 static void irte_ga_prepare(void *entry,
 			    u32 delivery_mode, u32 dest_mode,
-			    u8 vector, u32 dest_apicid)
+			    u8 vector, u32 dest_apicid, int devid)
 {
 	struct irte_ga *irte = (struct irte_ga *) entry;
+	struct iommu_dev_data *dev_data = search_dev_data(devid);
 
 	irte->lo.val                      = 0;
 	irte->hi.val                      = 0;
-	irte->lo.fields_remap.guest_mode  = 0;
+	irte->lo.fields_remap.guest_mode  = dev_data ? dev_data->use_vapic : 0;
 	irte->lo.fields_remap.int_type    = delivery_mode;
 	irte->lo.fields_remap.dm          = dest_mode;
 	irte->hi.fields.vector            = vector;
@@ -4064,11 +4081,14 @@ static void irte_ga_set_affinity(void *entry, u16 devid, u16 index,
 				 u8 vector, u32 dest_apicid)
 {
 	struct irte_ga *irte = (struct irte_ga *) entry;
+	struct iommu_dev_data *dev_data = search_dev_data(devid);
 
-	irte->hi.fields.vector = vector;
-	irte->lo.fields_remap.destination = dest_apicid;
-	irte->lo.fields_remap.guest_mode = 0;
-	modify_irte_ga(devid, index, irte, NULL);
+	if (!dev_data || !dev_data->use_vapic) {
+		irte->hi.fields.vector = vector;
+		irte->lo.fields_remap.destination = dest_apicid;
+		irte->lo.fields_remap.guest_mode = 0;
+		modify_irte_ga(devid, index, irte, NULL);
+	}
 }
 
 #define IRTE_ALLOCATED (~1U)
@@ -4211,7 +4231,7 @@ static void irq_remapping_prepare_irte(struct amd_ir_data *data,
 	data->irq_2_irte.index = index + sub_handle;
 	iommu->irte_ops->prepare(data->entry, apic->irq_delivery_mode,
 				 apic->irq_dest_mode, irq_cfg->vector,
-				 irq_cfg->dest_apicid);
+				 irq_cfg->dest_apicid, devid);
 
 	switch (info->type) {
 	case X86_IRQ_ALLOC_TYPE_IOAPIC:
@@ -4411,6 +4431,14 @@ static int amd_ir_set_vcpu_affinity(struct irq_data *data, void *vcpu_info)
 	struct amd_ir_data *ir_data = data->chip_data;
 	struct irte_ga *irte = (struct irte_ga *) ir_data->entry;
 	struct irq_2_irte *irte_info = &ir_data->irq_2_irte;
+	struct iommu_dev_data *dev_data = search_dev_data(irte_info->devid);
+
+	/* Note:
+	 * This device has never been set up for guest mode.
+	 * we should not modify the IRTE
+	 */
+	if (!dev_data || !dev_data->use_vapic)
+		return 0;
 
 	pi_data->ir_data = ir_data;
 
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index c32d0b3..5eb1608 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -146,7 +146,7 @@ struct ivmd_header {
 bool amd_iommu_dump;
 bool amd_iommu_irq_remap __read_mostly;
 
-int amd_iommu_guest_ir;
+int amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_VAPIC;
 
 static bool amd_iommu_detected;
 static bool __initdata amd_iommu_disabled;
@@ -2009,6 +2009,11 @@ static void early_enable_iommus(void)
 		iommu_enable(iommu);
 		iommu_flush_all_caches(iommu);
 	}
+
+#ifdef CONFIG_IRQ_REMAP
+	if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
+		amd_iommu_irq_ops.capability |= (1 << IRQ_POSTING_CAP);
+#endif
 }
 
 static void enable_iommus_v2(void)
@@ -2034,6 +2039,11 @@ static void disable_iommus(void)
 
 	for_each_iommu(iommu)
 		iommu_disable(iommu);
+
+#ifdef CONFIG_IRQ_REMAP
+	if (AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
+		amd_iommu_irq_ops.capability &= ~(1 << IRQ_POSTING_CAP);
+#endif
 }
 
 /*
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 35dfe02..26f8347 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -808,7 +808,7 @@ struct irq_2_irte {
 };
 
 struct amd_irte_ops {
-	void (*prepare)(void *, u32, u32, u8, u32);
+	void (*prepare)(void *, u32, u32, u8, u32, int);
 	void (*activate)(void *, u16, u16);
 	void (*deactivate)(void *, u16, u16);
 	void (*set_affinity)(void *, u16, u16, u8, u32);
-- 
1.9.1

[PART2 PATCH v5 10/12] svm: Introduces AVIC per-VM ID

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Introduces per-VM AVIC ID and helper functions to manage the IDs.
Currently, the ID will be used to implement 32-bit AVIC IOMMU GA tag.

The ID is 24-bit one-based indexing value, and is managed via helper
functions to get the next ID, or to free an ID once a VM is destroyed.
There should be no ID conflict for any active VMs.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/svm.c              | 78 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 69e62862..16b4d1d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -776,6 +776,7 @@ struct kvm_arch {
 	bool disabled_lapic_found;
 
 	/* Struct members for AVIC */
+	u32 avic_vm_id;
 	u32 ldr_mode;
 	struct page *avic_logical_id_table_page;
 	struct page *avic_physical_id_table_page;
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 16ef31b..90dc0fb 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -96,6 +96,18 @@ MODULE_DEVICE_TABLE(x86cpu, svm_cpu_id);
 #define AVIC_UNACCEL_ACCESS_OFFSET_MASK		0xFF0
 #define AVIC_UNACCEL_ACCESS_VECTOR_MASK		0xFFFFFFFF
 
+/* AVIC GATAG is encoded using VM and VCPU IDs */
+#define AVIC_VCPU_ID_BITS		8
+#define AVIC_VCPU_ID_MASK		((1 << AVIC_VCPU_ID_BITS) - 1)
+
+#define AVIC_VM_ID_BITS			24
+#define AVIC_VM_ID_MASK			((1 << AVIC_VM_ID_BITS) - 1)
+
+#define AVIC_GATAG(x, y)		(((x & AVIC_VM_ID_MASK) << AVIC_VCPU_ID_BITS) | \
+						(y & AVIC_VCPU_ID_MASK))
+#define AVIC_GATAG_TO_VMID(x)		((x >> AVIC_VCPU_ID_BITS) & AVIC_VM_ID_MASK)
+#define AVIC_GATAG_TO_VCPUID(x)		(x & AVIC_VM_ID_BITS)
+
 static bool erratum_383_found __read_mostly;
 
 static const u32 host_save_user_msrs[] = {
@@ -242,6 +254,10 @@ static int avic;
 module_param(avic, int, S_IRUGO);
 #endif
 
+/* AVIC VM ID bit masks and lock */
+static unsigned long *avic_vm_id_bm;
+static DEFINE_SPINLOCK(avic_vm_id_lock);
+
 static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0);
 static void svm_flush_tlb(struct kvm_vcpu *vcpu);
 static void svm_complete_interrupts(struct vcpu_svm *svm);
@@ -1280,10 +1296,61 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
+static inline int avic_vm_id_init(void)
+{
+	if (avic_vm_id_bm)
+		return 0;
+
+	avic_vm_id_bm = kcalloc(BITS_TO_LONGS(AVIC_VM_ID_MASK),
+				sizeof(long), GFP_KERNEL);
+	if (!avic_vm_id_bm)
+		return -ENOMEM;
+	return 0;
+}
+
+static inline void avic_vm_id_deinit(void)
+{
+	if (!avic_vm_id_bm)
+		return;
+
+	kfree(avic_vm_id_bm);
+	avic_vm_id_bm = NULL;
+}
+
+static inline int avic_get_next_vm_id(void)
+{
+	int id;
+
+	spin_lock(&avic_vm_id_lock);
+
+	/* AVIC VM ID is one-based. */
+	id = find_next_zero_bit(avic_vm_id_bm, 1, 1);
+	if (id <= AVIC_VM_ID_MASK)
+		__set_bit(id, avic_vm_id_bm);
+	else
+		id = -EINVAL;
+
+	spin_unlock(&avic_vm_id_lock);
+	return id;
+}
+
+static inline int avic_free_vm_id(int id)
+{
+	if (id <= 0 || id > AVIC_VM_ID_MASK)
+		return -EINVAL;
+
+	spin_lock(&avic_vm_id_lock);
+	__clear_bit(id, avic_vm_id_bm);
+	spin_unlock(&avic_vm_id_lock);
+	return 0;
+}
+
 static void avic_vm_destroy(struct kvm *kvm)
 {
 	struct kvm_arch *vm_data = &kvm->arch;
 
+	avic_free_vm_id(vm_data->avic_vm_id);
+
 	if (vm_data->avic_logical_id_table_page)
 		__free_page(vm_data->avic_logical_id_table_page);
 	if (vm_data->avic_physical_id_table_page)
@@ -1300,6 +1367,10 @@ static int avic_vm_init(struct kvm *kvm)
 	if (!avic)
 		return 0;
 
+	vm_data->avic_vm_id = avic_get_next_vm_id();
+	if (vm_data->avic_vm_id < 0)
+		return vm_data->avic_vm_id;
+
 	/* Allocating physical APIC ID table (4KB) */
 	p_page = alloc_page(GFP_KERNEL);
 	if (!p_page)
@@ -5076,6 +5147,12 @@ static struct kvm_x86_ops svm_x86_ops = {
 
 static int __init svm_init(void)
 {
+	int ret;
+
+	ret = avic_vm_id_init();
+	if (ret)
+		return ret;
+
 	return kvm_init(&svm_x86_ops, sizeof(struct vcpu_svm),
 			__alignof__(struct vcpu_svm), THIS_MODULE);
 }
@@ -5083,6 +5160,7 @@ static int __init svm_init(void)
 static void __exit svm_exit(void)
 {
 	kvm_exit();
+	avic_vm_id_deinit();
 }
 
 module_init(svm_init)
-- 
1.9.1

[PART2 PATCH v5 11/12] svm: Introduce AMD IOMMU avic_ga_log_notifier

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch introduces avic_ga_log_notifier, which will be called
by IOMMU driver whenever it handles the Guest vAPIC (GA) log entry.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/svm.c              | 73 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 16b4d1d..a9466ad 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -780,6 +780,7 @@ struct kvm_arch {
 	u32 ldr_mode;
 	struct page *avic_logical_id_table_page;
 	struct page *avic_physical_id_table_page;
+	struct hlist_node hnode;
 };
 
 struct kvm_vm_stat {
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 90dc0fb..2be5ed7 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -34,6 +34,8 @@
 #include <linux/sched.h>
 #include <linux/trace_events.h>
 #include <linux/slab.h>
+#include <linux/amd-iommu.h>
+#include <linux/hashtable.h>
 
 #include <asm/apic.h>
 #include <asm/perf_event.h>
@@ -944,6 +946,58 @@ static void svm_disable_lbrv(struct vcpu_svm *svm)
 	set_msr_interception(msrpm, MSR_IA32_LASTINTTOIP, 0, 0);
 }
 
+/* Note:
+ * This hash table is used to map VM_ID to a struct kvm_arch,
+ * when handling AMD IOMMU GALOG notification to schedule in
+ * a particular vCPU.
+ */
+#define SVM_VM_DATA_HASH_BITS	8
+DECLARE_HASHTABLE(svm_vm_data_hash, SVM_VM_DATA_HASH_BITS);
+static spinlock_t svm_vm_data_hash_lock;
+
+/* Note:
+ * This function is called from IOMMU driver to notify
+ * SVM to schedule in a particular vCPU of a particular VM.
+ */
+static int avic_ga_log_notifier(u32 ga_tag)
+{
+	unsigned long flags;
+	struct kvm_arch *ka = NULL;
+	struct kvm_vcpu *vcpu = NULL;
+	struct vcpu_svm *svm = NULL;
+	u32 vm_id = AVIC_GATAG_TO_VMID(ga_tag);
+	u32 vcpu_id = AVIC_GATAG_TO_VCPUID(ga_tag);
+
+	pr_debug("SVM: %s: vm_id=%#x, vcpu_id=%#x\n", __func__, vm_id, vcpu_id);
+
+	spin_lock_irqsave(&svm_vm_data_hash_lock, flags);
+	hash_for_each_possible(svm_vm_data_hash, ka, hnode, vm_id) {
+		struct kvm *kvm = container_of(ka, struct kvm, arch);
+		struct kvm_arch *vm_data = &kvm->arch;
+
+		if (vm_data->avic_vm_id != vm_id)
+			continue;
+		vcpu = kvm_get_vcpu_by_id(kvm, vcpu_id);
+		break;
+	}
+	spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
+
+	if (!vcpu)
+		return 0;
+
+	svm = to_svm(vcpu);
+
+	/* Note:
+	 * At this point, the IOMMU should have already set the pending
+	 * bit in the vAPIC backing page. So, we just need to schedule
+	 * in the vcpu.
+	 */
+	if (vcpu->mode == OUTSIDE_GUEST_MODE)
+		kvm_vcpu_wake_up(vcpu);
+
+	return 0;
+}
+
 static __init int svm_hardware_setup(void)
 {
 	int cpu;
@@ -1002,10 +1056,15 @@ static __init int svm_hardware_setup(void)
 	if (avic) {
 		if (!npt_enabled ||
 		    !boot_cpu_has(X86_FEATURE_AVIC) ||
-		    !IS_ENABLED(CONFIG_X86_LOCAL_APIC))
+		    !IS_ENABLED(CONFIG_X86_LOCAL_APIC)) {
 			avic = false;
-		else
+		} else {
 			pr_info("AVIC enabled\n");
+
+			hash_init(svm_vm_data_hash);
+			spin_lock_init(&svm_vm_data_hash_lock);
+			amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier);
+		}
 	}
 
 	return 0;
@@ -1347,6 +1406,7 @@ static inline int avic_free_vm_id(int id)
 
 static void avic_vm_destroy(struct kvm *kvm)
 {
+	unsigned long flags;
 	struct kvm_arch *vm_data = &kvm->arch;
 
 	avic_free_vm_id(vm_data->avic_vm_id);
@@ -1355,10 +1415,15 @@ static void avic_vm_destroy(struct kvm *kvm)
 		__free_page(vm_data->avic_logical_id_table_page);
 	if (vm_data->avic_physical_id_table_page)
 		__free_page(vm_data->avic_physical_id_table_page);
+
+	spin_lock_irqsave(&svm_vm_data_hash_lock, flags);
+	hash_del(&vm_data->hnode);
+	spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
 }
 
 static int avic_vm_init(struct kvm *kvm)
 {
+	unsigned long flags;
 	int err = -ENOMEM;
 	struct kvm_arch *vm_data = &kvm->arch;
 	struct page *p_page;
@@ -1387,6 +1452,10 @@ static int avic_vm_init(struct kvm *kvm)
 	vm_data->avic_logical_id_table_page = l_page;
 	clear_page(page_address(l_page));
 
+	spin_lock_irqsave(&svm_vm_data_hash_lock, flags);
+	hash_add(svm_vm_data_hash, &vm_data->hnode, vm_data->avic_vm_id);
+	spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
+
 	return 0;
 
 free_avic:
-- 
1.9.1

[PART2 PATCH v5 12/12] svm: Implements update_pi_irte hook to setup posted interrupt

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch implements update_pi_irte function hook to allow SVM
communicate to IOMMU driver regarding how to set up IRTE for handling
posted interrupt.

In case AVIC is enabled, during vcpu_load/unload, SVM needs to update
IOMMU IRTE with appropriate host physical APIC ID. Also, when
vcpu_blocking/unblocking, SVM needs to update the is-running bit in
the IOMMU IRTE. Both are achieved via calling amd_iommu_update_ga().

However, if GA mode is not enabled for the pass-through device,
IOMMU driver will simply just return when calling amd_iommu_update_ga.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 arch/x86/kvm/svm.c | 230 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 228 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2be5ed7..39dc600 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -43,6 +43,7 @@
 #include <asm/desc.h>
 #include <asm/debugreg.h>
 #include <asm/kvm_para.h>
+#include <asm/irq_remapping.h>
 
 #include <asm/virtext.h>
 #include "trace.h"
@@ -199,6 +200,16 @@ struct vcpu_svm {
 	struct page *avic_backing_page;
 	u64 *avic_physical_id_cache;
 	bool avic_is_running;
+
+	/*
+	 * Per-vcpu list of struct amd_iommu_pi_data:
+	 * This is used mainly to track interrupt remapping table entry (IRTE)
+	 * to be updated when the vcpu affinity changes. This avoid the need
+	 * for scanning for IRTE and try to match ga_tag in the IOMMU driver
+	 * (or using hashtable).
+	 */
+	struct list_head pi_list;
+	spinlock_t pi_list_lock;
 };
 
 #define AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK	(0xFF)
@@ -1463,6 +1474,31 @@ free_avic:
 	return err;
 }
 
+static inline int
+avic_update_iommu(struct kvm_vcpu *vcpu, int cpu, phys_addr_t pa, bool r)
+{
+	int ret;
+	unsigned long flags;
+	struct vcpu_svm *svm = to_svm(vcpu);
+	struct amd_iommu_pi_data *pi;
+
+	if (!kvm_arch_has_assigned_device(vcpu->kvm))
+		return 0;
+
+	/*
+	 * Here, we go through the per-vcpu pi_list to update all existing
+	 * interrupt remapping table entry targeting this vcpu.
+	 */
+	spin_lock_irqsave(&svm->pi_list_lock, flags);
+	list_for_each_entry(pi, &svm->pi_list, node) {
+		ret = amd_iommu_update_ga(cpu, (pa & AVIC_HPA_MASK), r, pi);
+		if (ret)
+			break;
+	}
+	spin_unlock_irqrestore(&svm->pi_list_lock, flags);
+	return ret;
+}
+
 /**
  * This function is called during VCPU halt/unhalt.
  */
@@ -1485,9 +1521,16 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
 	WARN_ON(is_run == !!(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK));
 
 	entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
-	if (is_run)
+	if (is_run) {
 		entry |= AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
-	WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
+		WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
+		avic_update_iommu(vcpu, h_physical_id,
+				  page_to_phys(svm->avic_backing_page), 1);
+	} else {
+		avic_update_iommu(vcpu, h_physical_id,
+				  page_to_phys(svm->avic_backing_page), 0);
+		WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
+	}
 }
 
 static void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
@@ -1514,6 +1557,9 @@ static void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 		entry |= AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
 
 	WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
+	avic_update_iommu(vcpu, h_physical_id,
+			  page_to_phys(svm->avic_backing_page),
+			  svm->avic_is_running);
 }
 
 static void avic_vcpu_put(struct kvm_vcpu *vcpu)
@@ -1525,6 +1571,10 @@ static void avic_vcpu_put(struct kvm_vcpu *vcpu)
 		return;
 
 	entry = READ_ONCE(*(svm->avic_physical_id_cache));
+	if (entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK)
+		avic_update_iommu(vcpu, -1,
+				  page_to_phys(svm->avic_backing_page), 0);
+
 	entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
 	WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
 }
@@ -1590,6 +1640,9 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 		err = avic_init_backing_page(&svm->vcpu);
 		if (err)
 			goto free_page4;
+
+		INIT_LIST_HEAD(&svm->pi_list);
+		spin_lock_init(&svm->pi_list_lock);
 	}
 
 	/* We initialize this flag to true to make sure that the is_running
@@ -4386,6 +4439,178 @@ static void svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec)
 		kvm_vcpu_wake_up(vcpu);
 }
 
+static void svm_pi_list_add(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi)
+{
+	bool found = false;
+	unsigned long flags;
+	struct amd_iommu_pi_data *cur;
+
+	spin_lock_irqsave(&svm->pi_list_lock, flags);
+	list_for_each_entry(cur, &svm->pi_list, node) {
+		if (cur->ir_data != pi->ir_data)
+			continue;
+		found = true;
+		break;
+	}
+	if (!found)
+		list_add(&pi->node, &svm->pi_list);
+	spin_unlock_irqrestore(&svm->pi_list_lock, flags);
+}
+
+static void svm_pi_list_del(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi)
+{
+	unsigned long flags;
+	struct amd_iommu_pi_data *cur, *next;
+
+	spin_lock_irqsave(&svm->pi_list_lock, flags);
+	list_for_each_entry_safe(cur, next, &svm->pi_list, node) {
+		if (cur->ir_data != pi->ir_data)
+			continue;
+		list_del(&cur->node);
+		kfree(cur);
+		break;
+	}
+	spin_unlock_irqrestore(&svm->pi_list_lock, flags);
+}
+
+/*
+ * svm_update_pi_irte - set IRTE for Posted-Interrupts
+ *
+ * @kvm: kvm
+ * @host_irq: host irq of the interrupt
+ * @guest_irq: gsi of the interrupt
+ * @set: set or unset PI
+ * returns 0 on success, < 0 on failure
+ */
+static int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
+			      uint32_t guest_irq, bool set)
+{
+	struct kvm_kernel_irq_routing_entry *e;
+	struct kvm_irq_routing_table *irq_rt;
+	int idx, ret = -EINVAL;
+
+	if (!kvm_arch_has_assigned_device(kvm) ||
+	    !irq_remapping_cap(IRQ_POSTING_CAP))
+		return 0;
+
+	pr_debug("SVM: %s: host_irq=%#x, guest_irq=%#x, set=%#x\n",
+		 __func__, host_irq, guest_irq, set);
+
+	idx = srcu_read_lock(&kvm->irq_srcu);
+	irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
+	WARN_ON(guest_irq >= irq_rt->nr_rt_entries);
+
+	hlist_for_each_entry(e, &irq_rt->map[guest_irq], link) {
+		struct kvm_lapic_irq irq;
+		struct vcpu_data vcpu_info;
+		struct kvm_vcpu *vcpu = NULL;
+		struct vcpu_svm *svm = NULL;
+
+		if (e->type != KVM_IRQ_ROUTING_MSI)
+			continue;
+
+		/**
+		 * Note:
+		 * The HW cannot support posting multicast/broadcast
+		 * interrupts to a vCPU. So, we still use interrupt
+		 * remapping for these kind of interrupts.
+		 *
+		 * For lowest-priority interrupts, we only support
+		 * those with single CPU as the destination, e.g. user
+		 * configures the interrupts via /proc/irq or uses
+		 * irqbalance to make the interrupts single-CPU.
+		 */
+		kvm_set_msi_irq(e, &irq);
+		if (kvm_intr_is_single_vcpu(kvm, &irq, &vcpu)) {
+			svm = to_svm(vcpu);
+			vcpu_info.pi_desc_addr = page_to_phys(svm->avic_backing_page);
+			vcpu_info.vector = irq.vector;
+
+			trace_kvm_pi_irte_update(vcpu->vcpu_id, host_irq, e->gsi,
+						 vcpu_info.vector,
+						 vcpu_info.pi_desc_addr, set);
+
+			pr_debug("SVM: %s: use GA mode for irq %u\n", __func__,
+				 irq.vector);
+		} else {
+			set = false;
+
+			pr_debug("SVM: %s: use legacy intr remap mode for irq %u\n",
+				 __func__, irq.vector);
+		}
+
+		/**
+		 * When AVIC is disabled, we fall-back to setup
+		 * IRTE w/ legacy mode
+		 */
+		if (set && kvm_vcpu_apicv_active(&svm->vcpu)) {
+			struct amd_iommu_pi_data *pi_data;
+
+			/**
+			 * Allocating new amd_iommu_pi_data, which will get
+			 * add to the per-vcpu pi_list.
+			 */
+			pi_data = kzalloc(sizeof(struct amd_iommu_pi_data),
+					  GFP_KERNEL);
+			if (!pi_data) {
+				ret = -ENOMEM;
+				goto out;
+			}
+
+			/* Try to enable guest_mode in IRTE */
+			pi_data->ga_tag = AVIC_GATAG(kvm->arch.avic_vm_id,
+						     vcpu->vcpu_id);
+			pi_data->vcpu_data = &vcpu_info;
+			pi_data->is_guest_mode = true;
+			ret = irq_set_vcpu_affinity(host_irq, pi_data);
+
+			/**
+			 * We save the pointer to pi_data in the struct
+			 * vcpu_svm so that we can reference to them directly
+			 * when we update vcpu scheduling information in IOMMU
+			 * irte.
+			 */
+			if (!ret && pi_data->is_guest_mode)
+				svm_pi_list_add(svm, pi_data);
+		} else {
+			/* Use legacy mode in IRTE */
+			struct amd_iommu_pi_data pi;
+
+			/**
+			 * Here, pi is used to:
+			 * - Tell IOMMU to use legacy mode for this interrupt.
+			 * - Retrieve ga_tag of prior interrupt remapping data.
+			 */
+			pi.is_guest_mode = false;
+			ret = irq_set_vcpu_affinity(host_irq, &pi);
+
+			/**
+			 * We need to check if the interrupt was previously
+			 * setup with the guest_mode by checking if the ga_tag
+			 * was cached. If so, we need to clean up the per-vcpu
+			 * pi_list.
+			 */
+			if (!ret && pi.ga_tag) {
+				struct kvm_vcpu *vcpu = kvm_get_vcpu_by_id(kvm,
+						AVIC_GATAG_TO_VCPUID(pi.ga_tag));
+
+				if (vcpu)
+					svm_pi_list_del(to_svm(vcpu), &pi);
+			}
+		}
+
+		if (ret < 0) {
+			pr_err("%s: failed to update PI IRTE\n", __func__);
+			goto out;
+		}
+	}
+
+	ret = 0;
+out:
+	srcu_read_unlock(&kvm->irq_srcu, idx);
+	return ret;
+}
+
 static int svm_nmi_allowed(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
@@ -5212,6 +5437,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 
 	.pmu_ops = &amd_pmu_ops,
 	.deliver_posted_interrupt = svm_deliver_avic_intr,
+	.update_pi_irte = svm_update_pi_irte,
 };
 
 static int __init svm_init(void)
-- 
1.9.1

Re: [PART2 PATCH v5 00/12] iommu/AMD: Introduce IOMMU AVIC support

[Suravee Suthikulpanit]

Hi Joerg/Radim/Paolo,

Are there any other concerns about this series?

Thanks,
Suravee

On 7/25/16 16:31, Suravee Suthikulpanit wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>
> CHANGES FROM V4
> ===============
>   * Remove the hash look up in the amd_iommu_update_ga() (see patch 7/12).
>     Instead, use per-vcpu pi_list to keep track of posted interrupts so that
>     SVM can directly update IOMMU interrupt remapping table entries directly
>     when rescheduling VCPUs. (see patch 8/12 and 12/12)(per Radim's suggestion)
>
>   * Re-implement AVIC VM-ID using a set of bit-mask to ensure no ID conflict
>     between active VMs. (see patch 10/12)
>
>   * Verify VM-ID of the hash entry in avic_ga_log_notifier() before referencing
>     each per-VM data structure. (see patch 11/12) (per Radim's suggestion)
>
> GITHUB
> ======
> Latest git tree can be found at:
>     http://github.com/ssuthiku/linux.git    avic_part2_v5
>
> OVERVIEW
> ========
> This patch set is the second part of the two-part patch series to introduce
> the new AMD Advance Virtual Interrupt Controller (AVIC) support.
>
> In addition to the SVM AVIC, AMD IOMMU also extends the AVIC capability
> to allow I/O interrupts injection directly into the virtualized guest
> local APIC without the need for hypervisor intervention.
>
> This patch series introduces a new hardware interrupt remapping (IR) mode
> in AMD IOMMU driver, the Guest Virtual APIC (GA) mode. This is in contrast
> to the existing "legacy" mode. The IR mode can be specified with a new
> kernel parameter:
>
>     amd_iommu_guest_ir=[vapic (default) | legacy]
>
> When enabling GA mode, the AMD IOMMU driver will configure device interrupt
> remapping in GA mode when possible (i.e. SVM AVIC must be enabled, and if
> the interrupt types are supported). Otherewise, the driver will fallback
> to using the legacy IR mode.
>
> This patch series also introduces new interfaces between SVM and IOMMU
> to allow:
>   * SVM driver to communicate to IOMMU with updated vcpu scheduling
>     information.
>   * IOMMU driver to notify SVM driver to schedule vcpu on to physical core
>     handle IOMMU GALog entry.
>
> DOCUMENTATIONS
> ==============
> More information about SVM AVIC can be found in the
> AMD64 Architecture Programmer’s Manual Volume 2 - System Programming.
>
>     http://support.amd.com/TechDocs/24593.pdf
>
> More information about IOMMU AVIC can be found int the
> AMD I/O Virtualization Technology (IOMMU) Specification - Rev 2.62.
>
>     http://support.amd.com/TechDocs/48882_IOMMU.pdf
>
> Any feedback and comments are very much appreciated.
>
> Thank you,
> Suravee
>
> Suravee Suthikulpanit (12):
>   iommu/amd: Detect and enable guest vAPIC support
>   iommu/amd: Move and introduce new IRTE-related unions and structures
>   iommu/amd: Introduce interrupt remapping ops structure
>   iommu/amd: Add support for multiple IRTE formats
>   iommu/amd: Detect and initialize guest vAPIC log
>   iommu/amd: Adding GALOG interrupt handler
>   iommu/amd: Introduce amd_iommu_update_ga()
>   iommu/amd: Implements irq_set_vcpu_affinity() hook to setup vapic mode
>     for pass-through devices
>   iommu/amd: Enable vAPIC interrupt remapping mode by default
>   svm: Introduces AVIC per-VM ID
>   svm: Introduce AMD IOMMU avic_ga_log_notifier
>   svm: Implements update_pi_irte hook to setup posted interrupt
>
>  arch/x86/include/asm/kvm_host.h |   2 +
>  arch/x86/kvm/svm.c              | 381 +++++++++++++++++++++++++++++-
>  drivers/iommu/amd_iommu.c       | 501 +++++++++++++++++++++++++++++++++++-----
>  drivers/iommu/amd_iommu_init.c  | 183 ++++++++++++++-
>  drivers/iommu/amd_iommu_proto.h |   1 +
>  drivers/iommu/amd_iommu_types.h | 151 ++++++++++++
>  include/linux/amd-iommu.h       |  42 +++-
>  7 files changed, 1191 insertions(+), 70 deletions(-)
>

Re: [PART2 PATCH v5 01/12] iommu/amd: Detect and enable guest vAPIC support

[Joerg Roedel]

On Mon, Jul 25, 2016 at 04:32:00AM -0500, Suthikulpanit, Suravee wrote:
> @@ -2147,10 +2186,16 @@ static int __init early_amd_iommu_init(void)
>  		 * remapping tables.
>  		 */
>  		ret = -ENOMEM;
> -		amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
> -				MAX_IRQS_PER_TABLE * sizeof(u32),
> -				IRQ_TABLE_ALIGNMENT,
> -				0, NULL);
> +		if (!AMD_IOMMU_GUEST_IR_GA(amd_iommu_guest_ir))
> +			amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
> +					MAX_IRQS_PER_TABLE * sizeof(u32),
> +					IRQ_TABLE_ALIGNMENT,
> +					0, NULL);
> +		else
> +			amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
> +					MAX_IRQS_PER_TABLE * (sizeof(u64) * 2),
> +					IRQ_TABLE_ALIGNMENT,
> +					0, NULL);

Its better to calculate only the size-parameter in the if-statement and
make kmem_cache_create use that one.

>  		if (!amd_iommu_irq_cache)
>  			goto out;
>  
> @@ -2403,6 +2448,21 @@ static int __init parse_amd_iommu_dump(char *str)
>  	return 1;
>  }
>  
> +static int __init parse_amd_iommu_intr(char *str)
> +{
> +	for (; *str; ++str) {
> +		if (strncmp(str, "legacy", 6) == 0) {
> +			amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY;
> +			break;
> +		}
> +		if (strncmp(str, "vapic", 5) == 0) {
> +			amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_VAPIC;
> +			break;
> +		}
> +	}
> +	return 1;
> +}
> +
>  static int __init parse_amd_iommu_options(char *str)
>  {
>  	for (; *str; ++str) {
> @@ -2511,6 +2571,7 @@ static int __init parse_ivrs_acpihid(char *str)
>  
>  __setup("amd_iommu_dump",	parse_amd_iommu_dump);
>  __setup("amd_iommu=",		parse_amd_iommu_options);
> +__setup("amd_iommu_intr=",	parse_amd_iommu_intr);

This parameter needs to be documented in Documentation/kernel-parameters.txt

Re: [PART2 PATCH v5 06/12] iommu/amd: Adding GALOG interrupt handler

[Joerg Roedel]

On Mon, Jul 25, 2016 at 04:32:05AM -0500, Suthikulpanit, Suravee wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> This patch adds AMD IOMMU guest virtual APIC log (GALOG) handler.
> When IOMMU hardware receives an interrupt targeting a blocking vcpu,
> it creates an entry in the GALOG, and generates an interrupt to notify
> the AMD IOMMU driver.
> 
> At this point, the driver processes the log entry, and notify the SVM
> driver via the registered iommu_ga_log_notifier function.
> 
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
>  drivers/iommu/amd_iommu.c | 77 +++++++++++++++++++++++++++++++++++++++++++++--
>  include/linux/amd-iommu.h | 20 ++++++++++--
>  2 files changed, 91 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index abfb2b7..861d723 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -741,14 +741,78 @@ static void iommu_poll_ppr_log(struct amd_iommu *iommu)
>  	}
>  }
>  
> +#ifdef CONFIG_IRQ_REMAP
> +static int (*iommu_ga_log_notifier)(u32);
> +
> +int amd_iommu_register_ga_log_notifier(int (*notifier)(u32))
> +{
> +	iommu_ga_log_notifier = notifier;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(amd_iommu_register_ga_log_notifier);
> +
> +static void iommu_poll_ga_log(struct amd_iommu *iommu)
> +{
> +	u32 head, tail, cnt = 0;
> +
> +	if (iommu->ga_log == NULL)
> +		return;
> +
> +	head = readl(iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
> +	tail = readl(iommu->mmio_base + MMIO_GA_TAIL_OFFSET);
> +
> +	while (head != tail) {
> +		volatile u64 *raw;
> +		u64 log_entry;
> +
> +		raw = (u64 *)(iommu->ga_log + head);
> +		cnt++;
> +
> +		/* Avoid memcpy function-call overhead */
> +		log_entry = *raw;
> +
> +		/* Update head pointer of hardware ring-buffer */
> +		head = (head + GA_ENTRY_SIZE) % GA_LOG_SIZE;
> +		writel(head, iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
> +
> +		/* Handle GA entry */
> +		switch (GA_REQ_TYPE(log_entry)) {
> +		case GA_GUEST_NR:
> +			if (!iommu_ga_log_notifier)
> +				break;
> +
> +			pr_debug("AMD-Vi: %s: devid=%#x, ga_tag=%#x\n",
> +				 __func__, GA_DEVID(log_entry),
> +				 GA_TAG(log_entry));
> +
> +			if (iommu_ga_log_notifier(GA_TAG(log_entry)) != 0)
> +				pr_err("AMD-Vi: GA log notifier failed.\n");
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		/* Refresh ring-buffer information */
> +		head = readl(iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
> +		tail = readl(iommu->mmio_base + MMIO_GA_TAIL_OFFSET);

Couldn't that cause an endless-loop in case of an interrupt storm from a
device? I think it is better to just read head and tail once before the
loop and update head after we get out of the loop. Any new entries
could be handled by the next iommu interrupt. This avoids any
soft-lockups that might happen when the loop runs for too long.

Re: [PART2 PATCH v5 09/12] iommu/amd: Enable vAPIC interrupt remapping mode by default

[Joerg Roedel]

On Mon, Jul 25, 2016 at 04:32:08AM -0500, Suthikulpanit, Suravee wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> Introduce struct iommu_dev_data.use_vapic flag, which IOMMU driver
> uses to determine if it should enable vAPIC support, by setting
> the ga_mode bit in the device's interrupt remapping table entry.
> 
> Currently, it is enabled for all pass-through device if vAPIC mode
> is enabled.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  drivers/iommu/amd_iommu.c       | 44 +++++++++++++++++++++++++++++++++--------
>  drivers/iommu/amd_iommu_init.c  | 12 ++++++++++-
>  drivers/iommu/amd_iommu_types.h |  2 +-
>  3 files changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index a044ce4..155b4a6 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -105,6 +105,7 @@ struct iommu_dev_data {
>  	bool pri_tlp;			  /* PASID TLB required for
>  					     PPR completions */
>  	u32 errata;			  /* Bitmap for errata to apply */
> +	u32 use_vapic;			  /* Enable device to use vapic mode */

bool is a better choice here.

Re: [PART2 PATCH v5 00/12] iommu/AMD: Introduce IOMMU AVIC support

[Joerg Roedel]

On Mon, Aug 08, 2016 at 09:42:36PM +0700, Suthikulpanit, Suravee wrote:
> Hi Joerg/Radim/Paolo,
> 
> Are there any other concerns about this series?

Okay, looked again through the iommu-patches and sent you my feedback.
Overall I think we are on track to merge this for v4.9 when the
remaining small issues are addressed and Radim and Paolo give their
Acked-bys and/or Reviewed-bys too.



	Joerg

Re: [PART2 PATCH v5 00/12] iommu/AMD: Introduce IOMMU AVIC support

[Suravee Suthikulpanit]

On 08/09/2016 09:58 PM, Joerg Roedel wrote:
> On Mon, Aug 08, 2016 at 09:42:36PM +0700, Suthikulpanit, Suravee wrote:
>> Hi Joerg/Radim/Paolo,
>>
>> Are there any other concerns about this series?
>
> Okay, looked again through the iommu-patches and sent you my feedback.
> Overall I think we are on track to merge this for v4.9 when the
> remaining small issues are addressed and Radim and Paolo give their
> Acked-bys and/or Reviewed-bys too.
>
>
>
> 	Joerg
>


Thanks Joerg. I'll update the patch and send out V6.

Suravee

Re: [PART2 PATCH v5 10/12] svm: Introduces AVIC per-VM ID

[Radim Krčmář]

2016-07-25 04:32-0500, Suravee Suthikulpanit:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> Introduces per-VM AVIC ID and helper functions to manage the IDs.
> Currently, the ID will be used to implement 32-bit AVIC IOMMU GA tag.
> 
> The ID is 24-bit one-based indexing value, and is managed via helper
> functions to get the next ID, or to free an ID once a VM is destroyed.
> There should be no ID conflict for any active VMs.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> @@ -242,6 +254,10 @@ static int avic;
>  module_param(avic, int, S_IRUGO);
>  #endif
>  
> +/* AVIC VM ID bit masks and lock */
> +static unsigned long *avic_vm_id_bm;

Please expand "bm" to "bitmap" to match KVM conventions.

> +static DEFINE_SPINLOCK(avic_vm_id_lock);
> +
>  static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0);
>  static void svm_flush_tlb(struct kvm_vcpu *vcpu);
>  static void svm_complete_interrupts(struct vcpu_svm *svm);
> @@ -1280,10 +1296,61 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu)
>  	return 0;
>  }
>  
> +static inline int avic_vm_id_init(void)
> +{
> +	if (avic_vm_id_bm)
> +		return 0;
> +
> +	avic_vm_id_bm = kcalloc(BITS_TO_LONGS(AVIC_VM_ID_MASK),

Allocation is off by one.  avic_get_next_vm_id() uses
  if (id <= AVIC_VM_ID_MASK)
    __set_bit(id, avic_vm_id_bm);

and id=AVIC_VM_ID_MASK is stored in the AVIC_VM_ID_MASK+1 th bit.

> +				sizeof(long), GFP_KERNEL);
> +	if (!avic_vm_id_bm)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +static inline void avic_vm_id_deinit(void)
> +{
> +	if (!avic_vm_id_bm)
> +		return;
> +
> +	kfree(avic_vm_id_bm);
> +	avic_vm_id_bm = NULL;
> +}
> +
> +static inline int avic_get_next_vm_id(void)
> +{
> +	int id;
> +
> +	spin_lock(&avic_vm_id_lock);
> +
> +	/* AVIC VM ID is one-based. */

Why?

> +	id = find_next_zero_bit(avic_vm_id_bm, 1, 1);

The second argument is size, so this should always return 1. :)

> +	if (id <= AVIC_VM_ID_MASK)
> +		__set_bit(id, avic_vm_id_bm);
> +	else
> +		id = -EINVAL;

It is not really a problem that can be handled with changing the values,
so a temporary error would be nicer ... ENOMEM could be confusing and
EAGAIN lead to a loop, but I still like them better.

> +
> +	spin_unlock(&avic_vm_id_lock);
> +	return id;
> +}
> +
> +static inline int avic_free_vm_id(int id)
> +{
> +	if (id <= 0 || id > AVIC_VM_ID_MASK)
> +		return -EINVAL;
> +
> +	spin_lock(&avic_vm_id_lock);
> +	__clear_bit(id, avic_vm_id_bm);
> +	spin_unlock(&avic_vm_id_lock);
> +	return 0;
> +}
> +
>  static void avic_vm_destroy(struct kvm *kvm)
>  {
>  	struct kvm_arch *vm_data = &kvm->arch;
>  
> +	avic_free_vm_id(vm_data->avic_vm_id);
> +
>  	if (vm_data->avic_logical_id_table_page)
>  		__free_page(vm_data->avic_logical_id_table_page);
>  	if (vm_data->avic_physical_id_table_page)
> @@ -1300,6 +1367,10 @@ static int avic_vm_init(struct kvm *kvm)
>  	if (!avic)
>  		return 0;
>  
> +	vm_data->avic_vm_id = avic_get_next_vm_id();
> +	if (vm_data->avic_vm_id < 0)
> +		return vm_data->avic_vm_id;
> +
>  	/* Allocating physical APIC ID table (4KB) */
>  	p_page = alloc_page(GFP_KERNEL);
>  	if (!p_page)
> @@ -5076,6 +5147,12 @@ static struct kvm_x86_ops svm_x86_ops = {
>  
>  static int __init svm_init(void)
>  {
> +	int ret;
> +
> +	ret = avic_vm_id_init();

This is certainly useless when the CPU doesn't have AVIC, so we could
make it conditional.

I would prefer to make the bitmap allocated at module load, though:

  static DECLARE_BITMAP(avic_vm_id_bm, AVIC_VM_ID_MASK + 1);

The size is 2 KiB with 24 bit AVIC_VM_ID_MASK, which is IMO much better
than having extra lines of code dealing with allocation and failures.

> +	if (ret)
> +		return ret;
> +
>  	return kvm_init(&svm_x86_ops, sizeof(struct vcpu_svm),
>  			__alignof__(struct vcpu_svm), THIS_MODULE);
>  }

Re: [PART2 PATCH v5 11/12] svm: Introduce AMD IOMMU avic_ga_log_notifier

[Radim Krčmář]

2016-07-25 04:32-0500, Suravee Suthikulpanit:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> This patch introduces avic_ga_log_notifier, which will be called
> by IOMMU driver whenever it handles the Guest vAPIC (GA) log entry.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

Re: [PART2 PATCH v5 12/12] svm: Implements update_pi_irte hook to setup posted interrupt

[Radim Krčmář]

2016-07-25 04:32-0500, Suravee Suthikulpanit:
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> @@ -1485,9 +1521,16 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
>  	WARN_ON(is_run == !!(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK));
>  
>  	entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
> -	if (is_run)
> +	if (is_run) {
>  		entry |= AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
> -	WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
> +		WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
> +		avic_update_iommu(vcpu, h_physical_id,
> +				  page_to_phys(svm->avic_backing_page), 1);
> +	} else {
> +		avic_update_iommu(vcpu, h_physical_id,
> +				  page_to_phys(svm->avic_backing_page), 0);
> +		WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
> +	}

You need to do the same change twice ... I guess it is time to factor
the code. :)

Wouldn't the following be an improvement in the !is_run path too?

  static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
  {
  	svm->avic_is_running = is_run;
  
  	if (is_run)
  		avic_vcpu_load(vcpu, vcpu->cpu);
  	else
  		avic_vcpu_put(vcpu);
  }

> +static void svm_pi_list_add(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi)
> +{
> +	bool found = false;
> +	unsigned long flags;
> +	struct amd_iommu_pi_data *cur;
> +
> +	spin_lock_irqsave(&svm->pi_list_lock, flags);
> +	list_for_each_entry(cur, &svm->pi_list, node) {
> +		if (cur->ir_data != pi->ir_data)
> +			continue;
> +		found = true;

This optimization turned out to be ugly ... sorry.

Manipulation with pi_list is hard to understand, IMO, so a comment
explaining why we couldn't do that without traversing a list and
comparing pi->ir_data would be nice.

Maybe I was a bit confused by reusing amd_iommu_pi_data when all we care
about is a list of cur->ir_data -- can't we have a list of just ir_data?

Allocating the list node in svm_pi_list_add() would make it nicely
symmetrical with svm_pi_list_del() too.

> +		break;
> +	}
> +	if (!found)
> +		list_add(&pi->node, &svm->pi_list);
> +	spin_unlock_irqrestore(&svm->pi_list_lock, flags);
> +}
> +
> +static void svm_pi_list_del(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi)
> +{
> +	unsigned long flags;
> +	struct amd_iommu_pi_data *cur, *next;
> +
> +	spin_lock_irqsave(&svm->pi_list_lock, flags);
> +	list_for_each_entry_safe(cur, next, &svm->pi_list, node) {

No need to use _safe loop if you break on the first deletion.

> +		if (cur->ir_data != pi->ir_data)
> +			continue;
> +		list_del(&cur->node);
> +		kfree(cur);
> +		break;
> +	}
> +	spin_unlock_irqrestore(&svm->pi_list_lock, flags);
> +}
> +
> +/*
> + * svm_update_pi_irte - set IRTE for Posted-Interrupts
> + *
> + * @kvm: kvm
> + * @host_irq: host irq of the interrupt
> + * @guest_irq: gsi of the interrupt
> + * @set: set or unset PI
> + * returns 0 on success, < 0 on failure
> + */
> +static int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
> +			      uint32_t guest_irq, bool set)
> +{
> +	struct kvm_kernel_irq_routing_entry *e;
> +	struct kvm_irq_routing_table *irq_rt;
> +	int idx, ret = -EINVAL;
> +
> +	if (!kvm_arch_has_assigned_device(kvm) ||
> +	    !irq_remapping_cap(IRQ_POSTING_CAP))
> +		return 0;
> +
> +	pr_debug("SVM: %s: host_irq=%#x, guest_irq=%#x, set=%#x\n",
> +		 __func__, host_irq, guest_irq, set);
> +
> +	idx = srcu_read_lock(&kvm->irq_srcu);
> +	irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
> +	WARN_ON(guest_irq >= irq_rt->nr_rt_entries);
> +
> +	hlist_for_each_entry(e, &irq_rt->map[guest_irq], link) {
> +		struct kvm_lapic_irq irq;
> +		struct vcpu_data vcpu_info;
> +		struct kvm_vcpu *vcpu = NULL;
> +		struct vcpu_svm *svm = NULL;
> +
> +		if (e->type != KVM_IRQ_ROUTING_MSI)
> +			continue;
> +
> +		/**
> +		 * Note:
> +		 * The HW cannot support posting multicast/broadcast
> +		 * interrupts to a vCPU. So, we still use interrupt
> +		 * remapping for these kind of interrupts.
> +		 *
> +		 * For lowest-priority interrupts, we only support
> +		 * those with single CPU as the destination, e.g. user
> +		 * configures the interrupts via /proc/irq or uses
> +		 * irqbalance to make the interrupts single-CPU.
> +		 */
> +		kvm_set_msi_irq(e, &irq);
> +		if (kvm_intr_is_single_vcpu(kvm, &irq, &vcpu)) {
> +			svm = to_svm(vcpu);
> +			vcpu_info.pi_desc_addr = page_to_phys(svm->avic_backing_page);
> +			vcpu_info.vector = irq.vector;
> +
> +			trace_kvm_pi_irte_update(vcpu->vcpu_id, host_irq, e->gsi,
> +						 vcpu_info.vector,
> +						 vcpu_info.pi_desc_addr, set);

Trace below the if/else, we might update something in both of them.

> +
> +			pr_debug("SVM: %s: use GA mode for irq %u\n", __func__,
> +				 irq.vector);
> +		} else {
> +			set = false;
> +
> +			pr_debug("SVM: %s: use legacy intr remap mode for irq %u\n",
> +				 __func__, irq.vector);
> +		}
> +
> +		/**
> +		 * When AVIC is disabled, we fall-back to setup
> +		 * IRTE w/ legacy mode
> +		 */
> +		if (set && kvm_vcpu_apicv_active(&svm->vcpu)) {
> +			struct amd_iommu_pi_data *pi_data;
> +
> +			/**
> +			 * Allocating new amd_iommu_pi_data, which will get
> +			 * add to the per-vcpu pi_list.
> +			 */
> +			pi_data = kzalloc(sizeof(struct amd_iommu_pi_data),
> +					  GFP_KERNEL);
> +			if (!pi_data) {
> +				ret = -ENOMEM;
> +				goto out;
> +			}
> +
> +			/* Try to enable guest_mode in IRTE */
> +			pi_data->ga_tag = AVIC_GATAG(kvm->arch.avic_vm_id,
> +						     vcpu->vcpu_id);
> +			pi_data->vcpu_data = &vcpu_info;
> +			pi_data->is_guest_mode = true;
> +			ret = irq_set_vcpu_affinity(host_irq, pi_data);
> +
> +			/**
> +			 * We save the pointer to pi_data in the struct
> +			 * vcpu_svm so that we can reference to them directly
> +			 * when we update vcpu scheduling information in IOMMU
> +			 * irte.
> +			 */
> +			if (!ret && pi_data->is_guest_mode)
> +				svm_pi_list_add(svm, pi_data);

pi_data leaks in the else case.

(Allocating the element in svm_pi_list_add() would solve this.)

> +		} else {
> +			/* Use legacy mode in IRTE */
> +			struct amd_iommu_pi_data pi;
> +
> +			/**
> +			 * Here, pi is used to:
> +			 * - Tell IOMMU to use legacy mode for this interrupt.
> +			 * - Retrieve ga_tag of prior interrupt remapping data.
> +			 */
> +			pi.is_guest_mode = false;
> +			ret = irq_set_vcpu_affinity(host_irq, &pi);
> +
> +			/**
> +			 * We need to check if the interrupt was previously
> +			 * setup with the guest_mode by checking if the ga_tag
> +			 * was cached. If so, we need to clean up the per-vcpu
> +			 * pi_list.
> +			 */
> +			if (!ret && pi.ga_tag) {
> +				struct kvm_vcpu *vcpu = kvm_get_vcpu_by_id(kvm,
> +						AVIC_GATAG_TO_VCPUID(pi.ga_tag));
> +
> +				if (vcpu)
> +					svm_pi_list_del(to_svm(vcpu), &pi);
> +			}
> +		}
> +
> +		if (ret < 0) {
> +			pr_err("%s: failed to update PI IRTE\n", __func__);
> +			goto out;
> +		}
> +	}
> +
> +	ret = 0;
> +out:
> +	srcu_read_unlock(&kvm->irq_srcu, idx);
> +	return ret;
> +}
> +
>  static int svm_nmi_allowed(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);

Re: [PART2 PATCH v5 06/12] iommu/amd: Adding GALOG interrupt handler

[Suravee Suthikulpanit]

Hi Joerg,

On 8/9/16 21:43, Joerg Roedel wrote:
> On Mon, Jul 25, 2016 at 04:32:05AM -0500, Suthikulpanit, Suravee wrote:
>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>
>> This patch adds AMD IOMMU guest virtual APIC log (GALOG) handler.
>> When IOMMU hardware receives an interrupt targeting a blocking vcpu,
>> it creates an entry in the GALOG, and generates an interrupt to notify
>> the AMD IOMMU driver.
>>
>> At this point, the driver processes the log entry, and notify the SVM
>> driver via the registered iommu_ga_log_notifier function.
>>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>> ---
>>  drivers/iommu/amd_iommu.c | 77 +++++++++++++++++++++++++++++++++++++++++++++--
>>  include/linux/amd-iommu.h | 20 ++++++++++--
>>  2 files changed, 91 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
>> index abfb2b7..861d723 100644
>> --- a/drivers/iommu/amd_iommu.c
>> +++ b/drivers/iommu/amd_iommu.c
>> @@ -741,14 +741,78 @@ static void iommu_poll_ppr_log(struct amd_iommu *iommu)
>>  	}
>>  }
>>
>> +#ifdef CONFIG_IRQ_REMAP
>> +static int (*iommu_ga_log_notifier)(u32);
>> +
>> +int amd_iommu_register_ga_log_notifier(int (*notifier)(u32))
>> +{
>> +	iommu_ga_log_notifier = notifier;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(amd_iommu_register_ga_log_notifier);
>> +
>> +static void iommu_poll_ga_log(struct amd_iommu *iommu)
>> +{
>> +	u32 head, tail, cnt = 0;
>> +
>> +	if (iommu->ga_log == NULL)
>> +		return;
>> +
>> +	head = readl(iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
>> +	tail = readl(iommu->mmio_base + MMIO_GA_TAIL_OFFSET);
>> +
>> +	while (head != tail) {
>> +		volatile u64 *raw;
>> +		u64 log_entry;
>> +
>> +		raw = (u64 *)(iommu->ga_log + head);
>> +		cnt++;
>> +
>> +		/* Avoid memcpy function-call overhead */
>> +		log_entry = *raw;
>> +
>> +		/* Update head pointer of hardware ring-buffer */
>> +		head = (head + GA_ENTRY_SIZE) % GA_LOG_SIZE;
>> +		writel(head, iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
>> +
>> +		/* Handle GA entry */
>> +		switch (GA_REQ_TYPE(log_entry)) {
>> +		case GA_GUEST_NR:
>> +			if (!iommu_ga_log_notifier)
>> +				break;
>> +
>> +			pr_debug("AMD-Vi: %s: devid=%#x, ga_tag=%#x\n",
>> +				 __func__, GA_DEVID(log_entry),
>> +				 GA_TAG(log_entry));
>> +
>> +			if (iommu_ga_log_notifier(GA_TAG(log_entry)) != 0)
>> +				pr_err("AMD-Vi: GA log notifier failed.\n");
>> +			break;
>> +		default:
>> +			break;
>> +		}
>> +
>> +		/* Refresh ring-buffer information */
>> +		head = readl(iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
>> +		tail = readl(iommu->mmio_base + MMIO_GA_TAIL_OFFSET);
>
> Couldn't that cause an endless-loop in case of an interrupt storm from a
> device? I think it is better to just read head and tail once before the
> loop and update head after we get out of the loop. Any new entries
> could be handled by the next iommu interrupt. This avoids any
> soft-lockups that might happen when the loop runs for too long.
>

Sure. Also, we might need to start handling GALogOverflow as well. 
However, let's put that in a separate patch series. What do you think?

S

Re: [PART2 PATCH v5 12/12] svm: Implements update_pi_irte hook to setup posted interrupt

[Suravee Suthikulpanit]

Hi Radim,


On 8/13/16 19:03, Radim Krčmář wrote:
> 2016-07-25 04:32-0500, Suravee Suthikulpanit:
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> @@ -1485,9 +1521,16 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
>>  	WARN_ON(is_run == !!(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK));
>>
>>  	entry &= ~AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
>> -	if (is_run)
>> +	if (is_run) {
>>  		entry |= AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK;
>> -	WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
>> +		WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
>> +		avic_update_iommu(vcpu, h_physical_id,
>> +				  page_to_phys(svm->avic_backing_page), 1);
>> +	} else {
>> +		avic_update_iommu(vcpu, h_physical_id,
>> +				  page_to_phys(svm->avic_backing_page), 0);
>> +		WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
>> +	}
>
> You need to do the same change twice ... I guess it is time to factor
> the code. :)
>
> Wouldn't the following be an improvement in the !is_run path too?
>
>   static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
>   {
>   	svm->avic_is_running = is_run;
>
>   	if (is_run)
>   		avic_vcpu_load(vcpu, vcpu->cpu);
>   	else
>   		avic_vcpu_put(vcpu);
>   }
>

I like this change. Thanks.

>> +static void svm_pi_list_add(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi)
>> +{
>> +	bool found = false;
>> +	unsigned long flags;
>> +	struct amd_iommu_pi_data *cur;
>> +
>> +	spin_lock_irqsave(&svm->pi_list_lock, flags);
>> +	list_for_each_entry(cur, &svm->pi_list, node) {
>> +		if (cur->ir_data != pi->ir_data)
>> +			continue;
>> +		found = true;
>
> This optimization turned out to be ugly ... sorry.

That's okay. It makes sense to avoid using the hash table if we can.

> Manipulation with pi_list is hard to understand, IMO, so a comment
> explaining why we couldn't do that without traversing a list and
> comparing pi->ir_data would be nice.

I'll add more comment here.

> Maybe I was a bit confused by reusing amd_iommu_pi_data when all we care
> about is a list of cur->ir_data -- can't we have a list of just ir_data?

Actually, in SVM, we care about posted-interrupt information, which is 
generated from the SVM side, and stored in the amd_iommu_pi_data. This 
is also communicated to IOMMU via the irq_set_vcpu_affinity().

Here, I only use ir_data to differentiate amd_iommu_pi_data.

>> [....]
>> +
>> +			/* Try to enable guest_mode in IRTE */
>> +			pi_data->ga_tag = AVIC_GATAG(kvm->arch.avic_vm_id,
>> +						     vcpu->vcpu_id);
>> +			pi_data->vcpu_data = &vcpu_info;
>> +			pi_data->is_guest_mode = true;
>> +			ret = irq_set_vcpu_affinity(host_irq, pi_data);
>> +
>> +			/**
>> +			 * We save the pointer to pi_data in the struct
>> +			 * vcpu_svm so that we can reference to them directly
>> +			 * when we update vcpu scheduling information in IOMMU
>> +			 * irte.
>> +			 */
>> +			if (!ret && pi_data->is_guest_mode)
>> +				svm_pi_list_add(svm, pi_data);
>
> pi_data leaks in the else case.
>
> (Allocating the element in svm_pi_list_add() would solve this.)

Ahh .. good catch.

Thanks,
Suravee

Re: [PART2 PATCH v5 12/12] svm: Implements update_pi_irte hook to setup posted interrupt

[Radim Krčmář]

2016-08-16 22:19+0700, Suravee Suthikulpanit:
> On 8/13/16 19:03, Radim Krčmář wrote:
>> 2016-07-25 04:32-0500, Suravee Suthikulpanit:
>> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> > @@ -1485,9 +1521,16 @@ static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run)
>> > +static void svm_pi_list_add(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi)
>> > +{
>> > +	bool found = false;
>> > +	unsigned long flags;
>> > +	struct amd_iommu_pi_data *cur;
>> > +
>> > +	spin_lock_irqsave(&svm->pi_list_lock, flags);
>> > +	list_for_each_entry(cur, &svm->pi_list, node) {
>> > +		if (cur->ir_data != pi->ir_data)
>> > +			continue;
>> > +		found = true;
>> 
>> This optimization turned out to be ugly ... sorry.
> 
> That's okay. It makes sense to avoid using the hash table if we can.
> 
>> Manipulation with pi_list is hard to understand, IMO, so a comment
>> explaining why we couldn't do that without traversing a list and
>> comparing pi->ir_data would be nice.
> 
> I'll add more comment here.

Thanks.

>> Maybe I was a bit confused by reusing amd_iommu_pi_data when all we care
>> about is a list of cur->ir_data -- can't we have a list of just ir_data?
> 
> Actually, in SVM, we care about posted-interrupt information, which is
> generated from the SVM side, and stored in the amd_iommu_pi_data. This is
> also communicated to IOMMU via the irq_set_vcpu_affinity().
> 
> Here, I only use ir_data to differentiate amd_iommu_pi_data.

I'm still confused then. :)

struct amd_iommu_pi_data is a throwaway structure for I/O with
irq_set_vcpu_affinity(), but we keep it afterwards ...
is it more than a wrapper that allows us to put ir_data into a list,
because we don't want to add list_head directly to ir_data?

Re: [PART2 PATCH v5 10/12] svm: Introduces AVIC per-VM ID

[Suravee Suthikulpanit]

Hi Radim,

On 8/12/16 21:16, Radim Krčmář wrote:
>> +static DEFINE_SPINLOCK(avic_vm_id_lock);
>> > +
>> >  static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0);
>> >  static void svm_flush_tlb(struct kvm_vcpu *vcpu);
>> >  static void svm_complete_interrupts(struct vcpu_svm *svm);
>> > @@ -1280,10 +1296,61 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu)
>> >  	return 0;
>> >  }
>> >
>> > +static inline int avic_vm_id_init(void)
>> > +{
>> > +	if (avic_vm_id_bm)
>> > +		return 0;
>> > +
>> > +	avic_vm_id_bm = kcalloc(BITS_TO_LONGS(AVIC_VM_ID_MASK),
> Allocation is off by one.  avic_get_next_vm_id() uses
>   if (id <= AVIC_VM_ID_MASK)
>     __set_bit(id, avic_vm_id_bm);
>
> and id=AVIC_VM_ID_MASK is stored in the AVIC_VM_ID_MASK+1 th bit.

Ah... right. Sorry :(

>> > +static inline int avic_get_next_vm_id(void)
>> > +{
>> > +	int id;
>> > +
>> > +	spin_lock(&avic_vm_id_lock);
>> > +
>> > +	/* AVIC VM ID is one-based. */
> Why?

I use VM-ID 0 to represent unassigned ID since we use it to encode 
ga_tag, and ga_tag=0 out of reset by hardware.

>> > +	id = find_next_zero_bit(avic_vm_id_bm, 1, 1);
> The second argument is size, so this should always return 1. :)
>

My bad. I'll change to (AVIC_VM_ID_MASK + 1).

>> > +	if (id <= AVIC_VM_ID_MASK)
>> > +		__set_bit(id, avic_vm_id_bm);
>> > +	else
>> > +		id = -EINVAL;
> It is not really a problem that can be handled with changing the values,
> so a temporary error would be nicer ... ENOMEM could be confusing and
> EAGAIN lead to a loop, but I still like them better.
>

Ok. I think EAGAIN is better in this case.

>> >  static int __init svm_init(void)
>> >  {
>> > +	int ret;
>> > +
>> > +	ret = avic_vm_id_init();
> This is certainly useless when the CPU doesn't have AVIC, so we could
> make it conditional.
>
> I would prefer to make the bitmap allocated at module load, though:
>
>   static DECLARE_BITMAP(avic_vm_id_bm, AVIC_VM_ID_MASK + 1);
>
> The size is 2 KiB with 24 bit AVIC_VM_ID_MASK, which is IMO much better
> than having extra lines of code dealing with allocation and failures.
>

I also prefer this suggestion.

Thanks again,
Suravee

Re: [PART2 PATCH v5 12/12] svm: Implements update_pi_irte hook to setup posted interrupt

[Suravee Suthikulpanit]

Hi Radim,

On 8/16/16 23:33, Radim Krčmář wrote:
>>> Maybe I was a bit confused by reusing amd_iommu_pi_data when all we care
>>> >> about is a list of cur->ir_data -- can't we have a list of just ir_data?
>> >
>> > Actually, in SVM, we care about posted-interrupt information, which is
>> > generated from the SVM side, and stored in the amd_iommu_pi_data. This is
>> > also communicated to IOMMU via the irq_set_vcpu_affinity().
>> >
>> > Here, I only use ir_data to differentiate amd_iommu_pi_data.
> I'm still confused then. :)
>
> struct amd_iommu_pi_data is a throwaway structure for I/O with
> irq_set_vcpu_affinity(), but we keep it afterwards ...
> is it more than a wrapper that allows us to put ir_data into a list,
> because we don't want to add list_head directly to ir_data?

Hm... I see you point now. I'll change it to ir_data list.

Thanks,
Suraveee