All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] iommu/amd: enable ACPI hardware ID device support
@ 2016-01-05 10:07 ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu
  Cc: Suravee Suthikulpanit, Borislav Petkov, Ray Huang, ken.xue,
	linux-kernel, Wan Zongshun

From: Wan Zongshun <Vincent.Wan@amd.com>

This patch series enable ACPI hardware ID device support, 

There are some devices indentified using ACPI HID format in AMD chip.
This patch series enable iommu support for those ACPI HID device,
since the existing AMD iommu only supports PCI bus based device.

Suravee Suthikulpanit (3):
  iommu/amd: Modify ivhd_header structure to support type 11h and 40h
  iommu/amd: Use the most comprehensive IVHD type that the driver can
    support
  iommu/amd: Introduces ivrs_acpihid kernel parameter

Wan Zongshun (3):
  iommu/amd: Add new map for storing IVHD dev entry type HID
  iommu/amd: Add support for non-pci devices
  iommu/amd: Manage iommu_group for non-pci devices

 Documentation/kernel-parameters.txt |   7 +
 drivers/iommu/amd_iommu.c           | 158 +++++++++++++++---
 drivers/iommu/amd_iommu_init.c      | 309 +++++++++++++++++++++++++++++++-----
 drivers/iommu/amd_iommu_types.h     |  14 ++
 4 files changed, 433 insertions(+), 55 deletions(-)

-- 
1.9.1


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

* [PATCH 0/6] iommu/amd: enable ACPI hardware ID device support
@ 2016-01-05 10:07 ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wan Zongshun, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

From: Wan Zongshun <Vincent.Wan-5C7GfCeVMHo@public.gmane.org>

This patch series enable ACPI hardware ID device support, 

There are some devices indentified using ACPI HID format in AMD chip.
This patch series enable iommu support for those ACPI HID device,
since the existing AMD iommu only supports PCI bus based device.

Suravee Suthikulpanit (3):
  iommu/amd: Modify ivhd_header structure to support type 11h and 40h
  iommu/amd: Use the most comprehensive IVHD type that the driver can
    support
  iommu/amd: Introduces ivrs_acpihid kernel parameter

Wan Zongshun (3):
  iommu/amd: Add new map for storing IVHD dev entry type HID
  iommu/amd: Add support for non-pci devices
  iommu/amd: Manage iommu_group for non-pci devices

 Documentation/kernel-parameters.txt |   7 +
 drivers/iommu/amd_iommu.c           | 158 +++++++++++++++---
 drivers/iommu/amd_iommu_init.c      | 309 +++++++++++++++++++++++++++++++-----
 drivers/iommu/amd_iommu_types.h     |  14 ++
 4 files changed, 433 insertions(+), 55 deletions(-)

-- 
1.9.1

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

* [PATCH 1/6] iommu/amd: Modify ivhd_header structure to support type 11h and 40h
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu
  Cc: Suravee Suthikulpanit, Borislav Petkov, Ray Huang, ken.xue, linux-kernel

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

This patch modifies the existing struct ivhd_header, which currently
only support IVHD type 0x10, to add new fields from IVHD type 11h and 40h.
It also modifies the pointer calculation to allow support for IVHD type
11h and 40h

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu_init.c | 47 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 013bdff..2ff7000 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -88,7 +88,7 @@
 
 /*
  * structure describing one IOMMU in the ACPI table. Typically followed by one
- * or more ivhd_entrys.
+ * or more ivhd_entrys. This struct supports both IVHD type 10h, 11h and 40h.
  */
 struct ivhd_header {
 	u8 type;
@@ -99,7 +99,11 @@ struct ivhd_header {
 	u64 mmio_phys;
 	u16 pci_seg;
 	u16 info;
-	u32 efr;
+	u32 efr_attr;
+
+	/* Following only valid on IVHD type 11h and 40h */
+	u64 efr_reg_img; /* Exact copy of MMIO_EXT_FEATURES */
+	u64 res;
 } __attribute__((packed));
 
 /*
@@ -399,6 +403,22 @@ static void __init iommu_unmap_mmio_space(struct amd_iommu *iommu)
  *
  ****************************************************************************/
 
+static inline u32 get_ivhd_header_size(struct ivhd_header *h)
+{
+	u32 size = 0;
+
+	switch (h->type) {
+	case 0x10:
+		size = 24;
+		break;
+	case 0x11:
+	case 0x40:
+		size = 40;
+		break;
+	}
+	return size;
+}
+
 /*
  * This function calculates the length of a given IVHD entry
  */
@@ -415,8 +435,14 @@ static int __init find_last_devid_from_ivhd(struct ivhd_header *h)
 {
 	u8 *p = (void *)h, *end = (void *)h;
 	struct ivhd_entry *dev;
+	u32 ivhd_size = get_ivhd_header_size(h);
+
+	if (!ivhd_size) {
+		pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+		return -EINVAL;
+	}
 
-	p += sizeof(*h);
+	p += ivhd_size;
 	end += h->length;
 
 	while (p < end) {
@@ -781,6 +807,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 	u32 dev_i, ext_flags = 0;
 	bool alias = false;
 	struct ivhd_entry *e;
+	u32 ivhd_size;
 	int ret;
 
 
@@ -796,7 +823,13 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 	/*
 	 * Done. Now parse the device entries
 	 */
-	p += sizeof(struct ivhd_header);
+	ivhd_size = get_ivhd_header_size(h);
+	if (!ivhd_size) {
+		pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+		return -EINVAL;
+	}
+
+	p += ivhd_size;
 	end += h->length;
 
 
@@ -1047,9 +1080,9 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	iommu->mmio_phys = h->mmio_phys;
 
 	/* Check if IVHD EFR contains proper max banks/counters */
-	if ((h->efr != 0) &&
-	    ((h->efr & (0xF << 13)) != 0) &&
-	    ((h->efr & (0x3F << 17)) != 0)) {
+	if ((h->efr_attr != 0) &&
+	    ((h->efr_attr & (0xF << 13)) != 0) &&
+	    ((h->efr_attr & (0x3F << 17)) != 0)) {
 		iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
 	} else {
 		iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
-- 
1.9.1


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

* [PATCH 1/6] iommu/amd: Modify ivhd_header structure to support type 11h and 40h
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: Borislav Petkov, Ray Huang, ken.xue-5C7GfCeVMHo,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

From: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>

This patch modifies the existing struct ivhd_header, which currently
only support IVHD type 0x10, to add new fields from IVHD type 11h and 40h.
It also modifies the pointer calculation to allow support for IVHD type
11h and 40h

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_init.c | 47 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 013bdff..2ff7000 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -88,7 +88,7 @@
 
 /*
  * structure describing one IOMMU in the ACPI table. Typically followed by one
- * or more ivhd_entrys.
+ * or more ivhd_entrys. This struct supports both IVHD type 10h, 11h and 40h.
  */
 struct ivhd_header {
 	u8 type;
@@ -99,7 +99,11 @@ struct ivhd_header {
 	u64 mmio_phys;
 	u16 pci_seg;
 	u16 info;
-	u32 efr;
+	u32 efr_attr;
+
+	/* Following only valid on IVHD type 11h and 40h */
+	u64 efr_reg_img; /* Exact copy of MMIO_EXT_FEATURES */
+	u64 res;
 } __attribute__((packed));
 
 /*
@@ -399,6 +403,22 @@ static void __init iommu_unmap_mmio_space(struct amd_iommu *iommu)
  *
  ****************************************************************************/
 
+static inline u32 get_ivhd_header_size(struct ivhd_header *h)
+{
+	u32 size = 0;
+
+	switch (h->type) {
+	case 0x10:
+		size = 24;
+		break;
+	case 0x11:
+	case 0x40:
+		size = 40;
+		break;
+	}
+	return size;
+}
+
 /*
  * This function calculates the length of a given IVHD entry
  */
@@ -415,8 +435,14 @@ static int __init find_last_devid_from_ivhd(struct ivhd_header *h)
 {
 	u8 *p = (void *)h, *end = (void *)h;
 	struct ivhd_entry *dev;
+	u32 ivhd_size = get_ivhd_header_size(h);
+
+	if (!ivhd_size) {
+		pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+		return -EINVAL;
+	}
 
-	p += sizeof(*h);
+	p += ivhd_size;
 	end += h->length;
 
 	while (p < end) {
@@ -781,6 +807,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 	u32 dev_i, ext_flags = 0;
 	bool alias = false;
 	struct ivhd_entry *e;
+	u32 ivhd_size;
 	int ret;
 
 
@@ -796,7 +823,13 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 	/*
 	 * Done. Now parse the device entries
 	 */
-	p += sizeof(struct ivhd_header);
+	ivhd_size = get_ivhd_header_size(h);
+	if (!ivhd_size) {
+		pr_err("AMD-Vi: Unsupported IVHD type %#x\n", h->type);
+		return -EINVAL;
+	}
+
+	p += ivhd_size;
 	end += h->length;
 
 
@@ -1047,9 +1080,9 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	iommu->mmio_phys = h->mmio_phys;
 
 	/* Check if IVHD EFR contains proper max banks/counters */
-	if ((h->efr != 0) &&
-	    ((h->efr & (0xF << 13)) != 0) &&
-	    ((h->efr & (0x3F << 17)) != 0)) {
+	if ((h->efr_attr != 0) &&
+	    ((h->efr_attr & (0xF << 13)) != 0) &&
+	    ((h->efr_attr & (0x3F << 17)) != 0)) {
 		iommu->mmio_phys_end = MMIO_REG_END_OFFSET;
 	} else {
 		iommu->mmio_phys_end = MMIO_CNTR_CONF_OFFSET;
-- 
1.9.1

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

* [PATCH 2/6] iommu/amd: Use the most comprehensive IVHD type that the driver can support
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu
  Cc: Suravee Suthikulpanit, Borislav Petkov, Ray Huang, ken.xue,
	linux-kernel, Wan Zongshun

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

The IVRS in more recent AMD system usually contains multiple
IVHD block types (e.g. 0x10, 0x11, and 0x40) for each IOMMU.
The newer IVHD types provide more information (e.g. new features
specified in the IOMMU spec), while maintain compatibility with
the older IVHD type.

Having multiple IVHD type allows older IOMMU drivers to still function
(e.g. using the older IVHD type 0x10) while the newer IOMMU driver can use
the newer IVHD types (e.g. 0x11 and 0x40). Therefore, the IOMMU driver
should only make use of the newest IVHD type that it can support.

This patch adds new logic to determine the highest level of IVHD type
it can support, and use it throughout the to initialize the driver.
This requires adding another pass to the IVRS parsing to determine
appropriate IVHD type (see function get_highest_supported_ivhd_type())
before parsing the contents.

[Vincent: fix the build error of IVHD_DEV_ACPI_HID flag not found]

Signed-off-by: Wan Zongshun <vincent.wan@amd.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu_init.c | 107 ++++++++++++++++++++++++++++++-----------
 1 file changed, 78 insertions(+), 29 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 2ff7000..146f3ee 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -44,7 +44,7 @@
  */
 #define IVRS_HEADER_LENGTH 48
 
-#define ACPI_IVHD_TYPE                  0x10
+#define ACPI_IVHD_TYPE_MAX_SUPPORTED	0x40
 #define ACPI_IVMD_TYPE_ALL              0x20
 #define ACPI_IVMD_TYPE                  0x21
 #define ACPI_IVMD_TYPE_RANGE            0x22
@@ -58,6 +58,7 @@
 #define IVHD_DEV_EXT_SELECT             0x46
 #define IVHD_DEV_EXT_SELECT_RANGE       0x47
 #define IVHD_DEV_SPECIAL		0x48
+#define IVHD_DEV_ACPI_HID		0xf0
 
 #define IVHD_SPECIAL_IOAPIC		1
 #define IVHD_SPECIAL_HPET		2
@@ -137,6 +138,7 @@ bool amd_iommu_irq_remap __read_mostly;
 
 static bool amd_iommu_detected;
 static bool __initdata amd_iommu_disabled;
+static int amd_iommu_target_ivhd_type;
 
 u16 amd_iommu_last_bdf;			/* largest PCI device id we have
 					   to handle */
@@ -424,7 +426,15 @@ static inline u32 get_ivhd_header_size(struct ivhd_header *h)
  */
 static inline int ivhd_entry_length(u8 *ivhd)
 {
-	return 0x04 << (*ivhd >> 6);
+	u32 type = ((struct ivhd_entry *)ivhd)->type;
+
+	if (type < 0x80) {
+		return 0x04 << (*ivhd >> 6);
+	} else if (type == IVHD_DEV_ACPI_HID) {
+		/* For ACPI_HID, offset 21 is uid len */
+		return *((u8 *)ivhd + 21) + 22;
+	}
+	return 0;
 }
 
 /*
@@ -470,6 +480,22 @@ static int __init find_last_devid_from_ivhd(struct ivhd_header *h)
 	return 0;
 }
 
+static int __init check_ivrs_checksum(struct acpi_table_header *table)
+{
+	int i;
+	u8 checksum = 0, *p = (u8 *)table;
+
+	for (i = 0; i < table->length; ++i)
+		checksum += p[i];
+	if (checksum != 0) {
+		/* ACPI table corrupt */
+		pr_err(FW_BUG "AMD-Vi: IVRS invalid checksum\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 /*
  * Iterate over all IVHD entries in the ACPI table and find the highest device
  * id which we need to handle. This is the first of three functions which parse
@@ -477,31 +503,19 @@ static int __init find_last_devid_from_ivhd(struct ivhd_header *h)
  */
 static int __init find_last_devid_acpi(struct acpi_table_header *table)
 {
-	int i;
-	u8 checksum = 0, *p = (u8 *)table, *end = (u8 *)table;
+	u8 *p = (u8 *)table, *end = (u8 *)table;
 	struct ivhd_header *h;
 
-	/*
-	 * Validate checksum here so we don't need to do it when
-	 * we actually parse the table
-	 */
-	for (i = 0; i < table->length; ++i)
-		checksum += p[i];
-	if (checksum != 0)
-		/* ACPI table corrupt */
-		return -ENODEV;
-
 	p += IVRS_HEADER_LENGTH;
 
 	end += table->length;
 	while (p < end) {
 		h = (struct ivhd_header *)p;
-		switch (h->type) {
-		case ACPI_IVHD_TYPE:
-			find_last_devid_from_ivhd(h);
-			break;
-		default:
-			break;
+		if (h->type == amd_iommu_target_ivhd_type) {
+			int ret = find_last_devid_from_ivhd(h);
+
+			if (ret)
+				return ret;
 		}
 		p += h->length;
 	}
@@ -1118,6 +1132,32 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	return 0;
 }
 
+/**
+ * get_highest_supported_ivhd_type - Look up the appropriate IVHD type
+ * @ivrs          Pointer to the IVRS header
+ *
+ * This function search through all IVDB of the maximum supported IVHD
+ */
+static u8 get_highest_supported_ivhd_type(struct acpi_table_header *ivrs)
+{
+	u8 *base = (u8 *)ivrs;
+	struct ivhd_header *ivhd = (struct ivhd_header *)
+					(base + IVRS_HEADER_LENGTH);
+	u8 last_type = ivhd->type;
+	u16 devid = ivhd->devid;
+
+	while (((u8 *)ivhd - base < ivrs->length) &&
+	       (ivhd->type <= ACPI_IVHD_TYPE_MAX_SUPPORTED)) {
+		u8 *p = (u8 *) ivhd;
+
+		if (ivhd->devid == devid)
+			last_type = ivhd->type;
+		ivhd = (struct ivhd_header *)(p + ivhd->length);
+	}
+
+	return last_type;
+}
+
 /*
  * Iterates over all IOMMU entries in the ACPI table, allocates the
  * IOMMU structure and initializes it with init_iommu_one()
@@ -1134,8 +1174,7 @@ static int __init init_iommu_all(struct acpi_table_header *table)
 
 	while (p < end) {
 		h = (struct ivhd_header *)p;
-		switch (*p) {
-		case ACPI_IVHD_TYPE:
+		if (*p == amd_iommu_target_ivhd_type) {
 
 			DUMP_printk("device: %02x:%02x.%01x cap: %04x "
 				    "seg: %d flags: %01x info %04x\n",
@@ -1152,9 +1191,6 @@ static int __init init_iommu_all(struct acpi_table_header *table)
 			ret = init_iommu_one(iommu, h);
 			if (ret)
 				return ret;
-			break;
-		default:
-			break;
 		}
 		p += h->length;
 
@@ -1818,18 +1854,20 @@ static void __init free_dma_resources(void)
  * remapping setup code.
  *
  * This function basically parses the ACPI table for AMD IOMMU (IVRS)
- * three times:
+ * four times:
  *
- *	1 pass) Find the highest PCI device id the driver has to handle.
+ *	1 pass) Discover the most comprehensive IVHD type to use.
+ *
+ *	2 pass) Find the highest PCI device id the driver has to handle.
  *		Upon this information the size of the data structures is
  *		determined that needs to be allocated.
  *
- *	2 pass) Initialize the data structures just allocated with the
+ *	3 pass) Initialize the data structures just allocated with the
  *		information in the ACPI table about available AMD IOMMUs
  *		in the system. It also maps the PCI devices in the
  *		system to specific IOMMUs
  *
- *	3 pass) After the basic data structures are allocated and
+ *	4 pass) After the basic data structures are allocated and
  *		initialized we update them with information about memory
  *		remapping requirements parsed out of the ACPI table in
  *		this last pass.
@@ -1857,6 +1895,17 @@ static int __init early_amd_iommu_init(void)
 	}
 
 	/*
+	 * Validate checksum here so we don't need to do it when
+	 * we actually parse the table
+	 */
+	ret = check_ivrs_checksum(ivrs_base);
+	if (ret)
+		return ret;
+
+	amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base);
+	DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type);
+
+	/*
 	 * First parse ACPI tables to find the largest Bus/Dev/Func
 	 * we need to handle. Upon this information the shared data
 	 * structures for the IOMMUs in the system will be allocated
-- 
1.9.1


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

* [PATCH 2/6] iommu/amd: Use the most comprehensive IVHD type that the driver can support
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wan Zongshun, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

From: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>

The IVRS in more recent AMD system usually contains multiple
IVHD block types (e.g. 0x10, 0x11, and 0x40) for each IOMMU.
The newer IVHD types provide more information (e.g. new features
specified in the IOMMU spec), while maintain compatibility with
the older IVHD type.

Having multiple IVHD type allows older IOMMU drivers to still function
(e.g. using the older IVHD type 0x10) while the newer IOMMU driver can use
the newer IVHD types (e.g. 0x11 and 0x40). Therefore, the IOMMU driver
should only make use of the newest IVHD type that it can support.

This patch adds new logic to determine the highest level of IVHD type
it can support, and use it throughout the to initialize the driver.
This requires adding another pass to the IVRS parsing to determine
appropriate IVHD type (see function get_highest_supported_ivhd_type())
before parsing the contents.

[Vincent: fix the build error of IVHD_DEV_ACPI_HID flag not found]

Signed-off-by: Wan Zongshun <vincent.wan-5C7GfCeVMHo@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_init.c | 107 ++++++++++++++++++++++++++++++-----------
 1 file changed, 78 insertions(+), 29 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 2ff7000..146f3ee 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -44,7 +44,7 @@
  */
 #define IVRS_HEADER_LENGTH 48
 
-#define ACPI_IVHD_TYPE                  0x10
+#define ACPI_IVHD_TYPE_MAX_SUPPORTED	0x40
 #define ACPI_IVMD_TYPE_ALL              0x20
 #define ACPI_IVMD_TYPE                  0x21
 #define ACPI_IVMD_TYPE_RANGE            0x22
@@ -58,6 +58,7 @@
 #define IVHD_DEV_EXT_SELECT             0x46
 #define IVHD_DEV_EXT_SELECT_RANGE       0x47
 #define IVHD_DEV_SPECIAL		0x48
+#define IVHD_DEV_ACPI_HID		0xf0
 
 #define IVHD_SPECIAL_IOAPIC		1
 #define IVHD_SPECIAL_HPET		2
@@ -137,6 +138,7 @@ bool amd_iommu_irq_remap __read_mostly;
 
 static bool amd_iommu_detected;
 static bool __initdata amd_iommu_disabled;
+static int amd_iommu_target_ivhd_type;
 
 u16 amd_iommu_last_bdf;			/* largest PCI device id we have
 					   to handle */
@@ -424,7 +426,15 @@ static inline u32 get_ivhd_header_size(struct ivhd_header *h)
  */
 static inline int ivhd_entry_length(u8 *ivhd)
 {
-	return 0x04 << (*ivhd >> 6);
+	u32 type = ((struct ivhd_entry *)ivhd)->type;
+
+	if (type < 0x80) {
+		return 0x04 << (*ivhd >> 6);
+	} else if (type == IVHD_DEV_ACPI_HID) {
+		/* For ACPI_HID, offset 21 is uid len */
+		return *((u8 *)ivhd + 21) + 22;
+	}
+	return 0;
 }
 
 /*
@@ -470,6 +480,22 @@ static int __init find_last_devid_from_ivhd(struct ivhd_header *h)
 	return 0;
 }
 
+static int __init check_ivrs_checksum(struct acpi_table_header *table)
+{
+	int i;
+	u8 checksum = 0, *p = (u8 *)table;
+
+	for (i = 0; i < table->length; ++i)
+		checksum += p[i];
+	if (checksum != 0) {
+		/* ACPI table corrupt */
+		pr_err(FW_BUG "AMD-Vi: IVRS invalid checksum\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 /*
  * Iterate over all IVHD entries in the ACPI table and find the highest device
  * id which we need to handle. This is the first of three functions which parse
@@ -477,31 +503,19 @@ static int __init find_last_devid_from_ivhd(struct ivhd_header *h)
  */
 static int __init find_last_devid_acpi(struct acpi_table_header *table)
 {
-	int i;
-	u8 checksum = 0, *p = (u8 *)table, *end = (u8 *)table;
+	u8 *p = (u8 *)table, *end = (u8 *)table;
 	struct ivhd_header *h;
 
-	/*
-	 * Validate checksum here so we don't need to do it when
-	 * we actually parse the table
-	 */
-	for (i = 0; i < table->length; ++i)
-		checksum += p[i];
-	if (checksum != 0)
-		/* ACPI table corrupt */
-		return -ENODEV;
-
 	p += IVRS_HEADER_LENGTH;
 
 	end += table->length;
 	while (p < end) {
 		h = (struct ivhd_header *)p;
-		switch (h->type) {
-		case ACPI_IVHD_TYPE:
-			find_last_devid_from_ivhd(h);
-			break;
-		default:
-			break;
+		if (h->type == amd_iommu_target_ivhd_type) {
+			int ret = find_last_devid_from_ivhd(h);
+
+			if (ret)
+				return ret;
 		}
 		p += h->length;
 	}
@@ -1118,6 +1132,32 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	return 0;
 }
 
+/**
+ * get_highest_supported_ivhd_type - Look up the appropriate IVHD type
+ * @ivrs          Pointer to the IVRS header
+ *
+ * This function search through all IVDB of the maximum supported IVHD
+ */
+static u8 get_highest_supported_ivhd_type(struct acpi_table_header *ivrs)
+{
+	u8 *base = (u8 *)ivrs;
+	struct ivhd_header *ivhd = (struct ivhd_header *)
+					(base + IVRS_HEADER_LENGTH);
+	u8 last_type = ivhd->type;
+	u16 devid = ivhd->devid;
+
+	while (((u8 *)ivhd - base < ivrs->length) &&
+	       (ivhd->type <= ACPI_IVHD_TYPE_MAX_SUPPORTED)) {
+		u8 *p = (u8 *) ivhd;
+
+		if (ivhd->devid == devid)
+			last_type = ivhd->type;
+		ivhd = (struct ivhd_header *)(p + ivhd->length);
+	}
+
+	return last_type;
+}
+
 /*
  * Iterates over all IOMMU entries in the ACPI table, allocates the
  * IOMMU structure and initializes it with init_iommu_one()
@@ -1134,8 +1174,7 @@ static int __init init_iommu_all(struct acpi_table_header *table)
 
 	while (p < end) {
 		h = (struct ivhd_header *)p;
-		switch (*p) {
-		case ACPI_IVHD_TYPE:
+		if (*p == amd_iommu_target_ivhd_type) {
 
 			DUMP_printk("device: %02x:%02x.%01x cap: %04x "
 				    "seg: %d flags: %01x info %04x\n",
@@ -1152,9 +1191,6 @@ static int __init init_iommu_all(struct acpi_table_header *table)
 			ret = init_iommu_one(iommu, h);
 			if (ret)
 				return ret;
-			break;
-		default:
-			break;
 		}
 		p += h->length;
 
@@ -1818,18 +1854,20 @@ static void __init free_dma_resources(void)
  * remapping setup code.
  *
  * This function basically parses the ACPI table for AMD IOMMU (IVRS)
- * three times:
+ * four times:
  *
- *	1 pass) Find the highest PCI device id the driver has to handle.
+ *	1 pass) Discover the most comprehensive IVHD type to use.
+ *
+ *	2 pass) Find the highest PCI device id the driver has to handle.
  *		Upon this information the size of the data structures is
  *		determined that needs to be allocated.
  *
- *	2 pass) Initialize the data structures just allocated with the
+ *	3 pass) Initialize the data structures just allocated with the
  *		information in the ACPI table about available AMD IOMMUs
  *		in the system. It also maps the PCI devices in the
  *		system to specific IOMMUs
  *
- *	3 pass) After the basic data structures are allocated and
+ *	4 pass) After the basic data structures are allocated and
  *		initialized we update them with information about memory
  *		remapping requirements parsed out of the ACPI table in
  *		this last pass.
@@ -1857,6 +1895,17 @@ static int __init early_amd_iommu_init(void)
 	}
 
 	/*
+	 * Validate checksum here so we don't need to do it when
+	 * we actually parse the table
+	 */
+	ret = check_ivrs_checksum(ivrs_base);
+	if (ret)
+		return ret;
+
+	amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base);
+	DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type);
+
+	/*
 	 * First parse ACPI tables to find the largest Bus/Dev/Func
 	 * we need to handle. Upon this information the shared data
 	 * structures for the IOMMUs in the system will be allocated
-- 
1.9.1

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

* [PATCH 3/6] iommu/amd: Add new map for storing IVHD dev entry type HID
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu
  Cc: Suravee Suthikulpanit, Borislav Petkov, Ray Huang, ken.xue,
	linux-kernel, Wan Zongshun

From: Wan Zongshun <Vincent.Wan@amd.com>

This patch introduces acpihid_map, which is used to store
the new IVHD device entry extracted from BIOS IVRS table.

It also provides a utility function add_acpi_hid_device(),
to add this types of devices to the map.

Signed-off-by: Wan Zongshun <Vincent.Wan@amd.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c       |   1 +
 drivers/iommu/amd_iommu_init.c  | 122 ++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_types.h |  14 +++++
 3 files changed, 137 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 9097b11..98acb89 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -72,6 +72,7 @@ static DEFINE_SPINLOCK(dev_data_list_lock);
 
 LIST_HEAD(ioapic_map);
 LIST_HEAD(hpet_map);
+LIST_HEAD(acpihid_map);
 
 /*
  * Domain for untranslated devices - only allocated
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 146f3ee..0a8e033 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -60,6 +60,10 @@
 #define IVHD_DEV_SPECIAL		0x48
 #define IVHD_DEV_ACPI_HID		0xf0
 
+#define UID_NOT_PRESENT                 0
+#define UID_IS_INTEGER                  1
+#define UID_IS_CHARACTER                2
+
 #define IVHD_SPECIAL_IOAPIC		1
 #define IVHD_SPECIAL_HPET		2
 
@@ -116,6 +120,11 @@ struct ivhd_entry {
 	u16 devid;
 	u8 flags;
 	u32 ext;
+	u32 hidh;
+	u64 cid;
+	u8 uidf;
+	u8 uidl;
+	u8 uid;
 } __attribute__((packed));
 
 /*
@@ -224,8 +233,12 @@ enum iommu_init_state {
 #define EARLY_MAP_SIZE		4
 static struct devid_map __initdata early_ioapic_map[EARLY_MAP_SIZE];
 static struct devid_map __initdata early_hpet_map[EARLY_MAP_SIZE];
+static struct acpihid_map_entry __initdata early_acpihid_map[EARLY_MAP_SIZE];
+
 static int __initdata early_ioapic_map_size;
 static int __initdata early_hpet_map_size;
+static int __initdata early_acpihid_map_size;
+
 static bool __initdata cmdline_maps;
 
 static enum iommu_init_state init_state = IOMMU_START_STATE;
@@ -760,6 +773,42 @@ static int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line)
 	return 0;
 }
 
+static int __init add_acpi_hid_device(u8 *hid, u8 *uid, u16 *devid,
+				      bool cmd_line)
+{
+	struct acpihid_map_entry *entry;
+	struct list_head *list = &acpihid_map;
+
+	list_for_each_entry(entry, list, list) {
+		if (strcmp(entry->hid, hid) ||
+		    (*uid && *entry->uid && strcmp(entry->uid, uid)) ||
+		    !entry->cmd_line)
+			continue;
+
+		pr_info("AMD-Vi: Command-line override for hid:%s uid:%s\n",
+			hid, uid);
+		*devid = entry->devid;
+		return 0;
+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	memcpy(entry->uid, uid, strlen(uid));
+	memcpy(entry->hid, hid, strlen(hid));
+	entry->devid = *devid;
+	entry->cmd_line	= cmd_line;
+	entry->root_devid = (entry->devid & (~0x7));
+
+	pr_info("AMD-Vi:%s, add hid:%s, uid:%s, rdevid:%d\n",
+		entry->cmd_line ? "cmd" : "ivrs",
+		entry->hid, entry->uid, entry->root_devid);
+
+	list_add_tail(&entry->list, list);
+	return 0;
+}
+
 static int __init add_early_maps(void)
 {
 	int i, ret;
@@ -782,6 +831,15 @@ static int __init add_early_maps(void)
 			return ret;
 	}
 
+	for (i = 0; i < early_acpihid_map_size; ++i) {
+		ret = add_acpi_hid_device(early_acpihid_map[i].hid,
+					  early_acpihid_map[i].uid,
+					  &early_acpihid_map[i].devid,
+					  early_acpihid_map[i].cmd_line);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -1001,6 +1059,70 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 
 			break;
 		}
+		case IVHD_DEV_ACPI_HID: {
+			u16 devid;
+			u8 hid[ACPIHID_HID_LEN] = {0};
+			u8 uid[ACPIHID_UID_LEN] = {0};
+			int ret;
+
+			if (h->type != 0x40) {
+				pr_err(FW_BUG "Invalid IVHD device type %#x\n",
+				       e->type);
+				break;
+			}
+
+			memcpy(hid, (u8 *)(&e->ext), ACPIHID_HID_LEN - 1);
+			hid[ACPIHID_HID_LEN - 1] = '\0';
+
+			if (!(*hid)) {
+				pr_err(FW_BUG "Invalid HID.\n");
+				break;
+			}
+
+			switch (e->uidf) {
+			case UID_NOT_PRESENT:
+
+				if (e->uidl != 0)
+					pr_warn(FW_BUG "Invalid UID length.\n");
+
+				break;
+			case UID_IS_INTEGER:
+
+				sprintf(uid, "%d", e->uid);
+
+				break;
+			case UID_IS_CHARACTER:
+
+				memcpy(uid, (u8 *)(&e->uid), ACPIHID_UID_LEN - 1);
+				uid[ACPIHID_UID_LEN - 1] = '\0';
+
+				break;
+			default:
+				break;
+			}
+
+			DUMP_printk("  DEV_ACPI_HID(%s[%s])\t\tdevid: %02x:%02x.%x\n",
+				    hid, uid,
+				    PCI_BUS_NUM(devid),
+				    PCI_SLOT(devid),
+				    PCI_FUNC(devid));
+
+			devid  = e->devid;
+			flags = e->flags;
+
+			ret = add_acpi_hid_device(hid, uid, &devid, false);
+			if (ret)
+				return ret;
+
+			/*
+			 * add_special_device might update the devid in case a
+			 * command-line override is present. So call
+			 * set_dev_entry_from_acpi after add_special_device.
+			 */
+			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
+
+			break;
+		}
 		default:
 			break;
 		}
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index b08cf57..e7c58ec 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -567,6 +567,19 @@ struct amd_iommu {
 #endif
 };
 
+#define ACPIHID_UID_LEN 256
+#define ACPIHID_HID_LEN 9
+
+struct acpihid_map_entry {
+	struct list_head list;
+	u8 uid[ACPIHID_UID_LEN];
+	u8 hid[ACPIHID_HID_LEN];
+	u16 devid;
+	u16 root_devid;
+	bool cmd_line;
+	struct iommu_group *group;
+};
+
 struct devid_map {
 	struct list_head list;
 	u8 id;
@@ -577,6 +590,7 @@ struct devid_map {
 /* Map HPET and IOAPIC ids to the devid used by the IOMMU */
 extern struct list_head ioapic_map;
 extern struct list_head hpet_map;
+extern struct list_head acpihid_map;
 
 /*
  * List with all IOMMUs in the system. This list is not locked because it is
-- 
1.9.1


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

* [PATCH 3/6] iommu/amd: Add new map for storing IVHD dev entry type HID
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wan Zongshun, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

From: Wan Zongshun <Vincent.Wan-5C7GfCeVMHo@public.gmane.org>

This patch introduces acpihid_map, which is used to store
the new IVHD device entry extracted from BIOS IVRS table.

It also provides a utility function add_acpi_hid_device(),
to add this types of devices to the map.

Signed-off-by: Wan Zongshun <Vincent.Wan-5C7GfCeVMHo@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c       |   1 +
 drivers/iommu/amd_iommu_init.c  | 122 ++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_types.h |  14 +++++
 3 files changed, 137 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 9097b11..98acb89 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -72,6 +72,7 @@ static DEFINE_SPINLOCK(dev_data_list_lock);
 
 LIST_HEAD(ioapic_map);
 LIST_HEAD(hpet_map);
+LIST_HEAD(acpihid_map);
 
 /*
  * Domain for untranslated devices - only allocated
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 146f3ee..0a8e033 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -60,6 +60,10 @@
 #define IVHD_DEV_SPECIAL		0x48
 #define IVHD_DEV_ACPI_HID		0xf0
 
+#define UID_NOT_PRESENT                 0
+#define UID_IS_INTEGER                  1
+#define UID_IS_CHARACTER                2
+
 #define IVHD_SPECIAL_IOAPIC		1
 #define IVHD_SPECIAL_HPET		2
 
@@ -116,6 +120,11 @@ struct ivhd_entry {
 	u16 devid;
 	u8 flags;
 	u32 ext;
+	u32 hidh;
+	u64 cid;
+	u8 uidf;
+	u8 uidl;
+	u8 uid;
 } __attribute__((packed));
 
 /*
@@ -224,8 +233,12 @@ enum iommu_init_state {
 #define EARLY_MAP_SIZE		4
 static struct devid_map __initdata early_ioapic_map[EARLY_MAP_SIZE];
 static struct devid_map __initdata early_hpet_map[EARLY_MAP_SIZE];
+static struct acpihid_map_entry __initdata early_acpihid_map[EARLY_MAP_SIZE];
+
 static int __initdata early_ioapic_map_size;
 static int __initdata early_hpet_map_size;
+static int __initdata early_acpihid_map_size;
+
 static bool __initdata cmdline_maps;
 
 static enum iommu_init_state init_state = IOMMU_START_STATE;
@@ -760,6 +773,42 @@ static int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line)
 	return 0;
 }
 
+static int __init add_acpi_hid_device(u8 *hid, u8 *uid, u16 *devid,
+				      bool cmd_line)
+{
+	struct acpihid_map_entry *entry;
+	struct list_head *list = &acpihid_map;
+
+	list_for_each_entry(entry, list, list) {
+		if (strcmp(entry->hid, hid) ||
+		    (*uid && *entry->uid && strcmp(entry->uid, uid)) ||
+		    !entry->cmd_line)
+			continue;
+
+		pr_info("AMD-Vi: Command-line override for hid:%s uid:%s\n",
+			hid, uid);
+		*devid = entry->devid;
+		return 0;
+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	memcpy(entry->uid, uid, strlen(uid));
+	memcpy(entry->hid, hid, strlen(hid));
+	entry->devid = *devid;
+	entry->cmd_line	= cmd_line;
+	entry->root_devid = (entry->devid & (~0x7));
+
+	pr_info("AMD-Vi:%s, add hid:%s, uid:%s, rdevid:%d\n",
+		entry->cmd_line ? "cmd" : "ivrs",
+		entry->hid, entry->uid, entry->root_devid);
+
+	list_add_tail(&entry->list, list);
+	return 0;
+}
+
 static int __init add_early_maps(void)
 {
 	int i, ret;
@@ -782,6 +831,15 @@ static int __init add_early_maps(void)
 			return ret;
 	}
 
+	for (i = 0; i < early_acpihid_map_size; ++i) {
+		ret = add_acpi_hid_device(early_acpihid_map[i].hid,
+					  early_acpihid_map[i].uid,
+					  &early_acpihid_map[i].devid,
+					  early_acpihid_map[i].cmd_line);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -1001,6 +1059,70 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 
 			break;
 		}
+		case IVHD_DEV_ACPI_HID: {
+			u16 devid;
+			u8 hid[ACPIHID_HID_LEN] = {0};
+			u8 uid[ACPIHID_UID_LEN] = {0};
+			int ret;
+
+			if (h->type != 0x40) {
+				pr_err(FW_BUG "Invalid IVHD device type %#x\n",
+				       e->type);
+				break;
+			}
+
+			memcpy(hid, (u8 *)(&e->ext), ACPIHID_HID_LEN - 1);
+			hid[ACPIHID_HID_LEN - 1] = '\0';
+
+			if (!(*hid)) {
+				pr_err(FW_BUG "Invalid HID.\n");
+				break;
+			}
+
+			switch (e->uidf) {
+			case UID_NOT_PRESENT:
+
+				if (e->uidl != 0)
+					pr_warn(FW_BUG "Invalid UID length.\n");
+
+				break;
+			case UID_IS_INTEGER:
+
+				sprintf(uid, "%d", e->uid);
+
+				break;
+			case UID_IS_CHARACTER:
+
+				memcpy(uid, (u8 *)(&e->uid), ACPIHID_UID_LEN - 1);
+				uid[ACPIHID_UID_LEN - 1] = '\0';
+
+				break;
+			default:
+				break;
+			}
+
+			DUMP_printk("  DEV_ACPI_HID(%s[%s])\t\tdevid: %02x:%02x.%x\n",
+				    hid, uid,
+				    PCI_BUS_NUM(devid),
+				    PCI_SLOT(devid),
+				    PCI_FUNC(devid));
+
+			devid  = e->devid;
+			flags = e->flags;
+
+			ret = add_acpi_hid_device(hid, uid, &devid, false);
+			if (ret)
+				return ret;
+
+			/*
+			 * add_special_device might update the devid in case a
+			 * command-line override is present. So call
+			 * set_dev_entry_from_acpi after add_special_device.
+			 */
+			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
+
+			break;
+		}
 		default:
 			break;
 		}
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index b08cf57..e7c58ec 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -567,6 +567,19 @@ struct amd_iommu {
 #endif
 };
 
+#define ACPIHID_UID_LEN 256
+#define ACPIHID_HID_LEN 9
+
+struct acpihid_map_entry {
+	struct list_head list;
+	u8 uid[ACPIHID_UID_LEN];
+	u8 hid[ACPIHID_HID_LEN];
+	u16 devid;
+	u16 root_devid;
+	bool cmd_line;
+	struct iommu_group *group;
+};
+
 struct devid_map {
 	struct list_head list;
 	u8 id;
@@ -577,6 +590,7 @@ struct devid_map {
 /* Map HPET and IOAPIC ids to the devid used by the IOMMU */
 extern struct list_head ioapic_map;
 extern struct list_head hpet_map;
+extern struct list_head acpihid_map;
 
 /*
  * List with all IOMMUs in the system. This list is not locked because it is
-- 
1.9.1

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

* [PATCH 4/6] iommu/amd: Introduces ivrs_acpihid kernel parameter
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu
  Cc: Suravee Suthikulpanit, Borislav Petkov, Ray Huang, ken.xue,
	linux-kernel, Wan Zongshun

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

This patch introduces a new kernel parameter, ivrs_acpihid.
This is used to override existing ACPI-HID IVHD device entry,
or add an entry in case it is missing in the IVHD.

Signed-off-by: Wan Zongshun <Vincent.Wan@amd.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 Documentation/kernel-parameters.txt |  7 +++++++
 drivers/iommu/amd_iommu_init.c      | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 742f69d..5c364c6 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1680,6 +1680,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			PCI device 00:14.0 write the parameter as:
 				ivrs_hpet[0]=00:14.0
 
+	ivrs_acpihid	[HW,X86_64]
+			Provide an override to the ACPI-HID:UID<->DEVICE-ID
+			mapping provided in the IVRS ACPI table. For
+			example, to map UART-HID:UID AMD0020:0 to
+			PCI device 00:14.5 write the parameter as:
+				ivrs_acpihid[00:14.5]=AMD0020:0
+
 	js=		[HW,JOY] Analog joystick
 			See Documentation/input/joystick.txt.
 
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 0a8e033..ad05614 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2430,10 +2430,43 @@ static int __init parse_ivrs_hpet(char *str)
 	return 1;
 }
 
+static int __init parse_ivrs_acpihid(char *str)
+{
+	u32 bus, dev, fn;
+	char *hid, *uid, *p;
+	char acpiid[ACPIHID_UID_LEN + ACPIHID_HID_LEN] = {0};
+	int ret, i;
+
+	ret = sscanf(str, "[%x:%x.%x]=%s", &bus, &dev, &fn, acpiid);
+	if (ret != 4) {
+		pr_err("AMD-Vi: Invalid command line: ivrs_acpihid(%s)\n", str);
+		return 1;
+	}
+
+	p = acpiid;
+	hid = strsep(&p, ":");
+	uid = p;
+
+	if (!hid || !(*hid) || !uid) {
+		pr_err("AMD-Vi: Invalid command line: hid or uid\n");
+		return 1;
+	}
+
+	i = early_acpihid_map_size++;
+	memcpy(early_acpihid_map[i].hid, hid, strlen(hid));
+	memcpy(early_acpihid_map[i].uid, uid, strlen(uid));
+	early_acpihid_map[i].devid =
+		((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
+	early_acpihid_map[i].cmd_line	= true;
+
+	return 1;
+}
+
 __setup("amd_iommu_dump",	parse_amd_iommu_dump);
 __setup("amd_iommu=",		parse_amd_iommu_options);
 __setup("ivrs_ioapic",		parse_ivrs_ioapic);
 __setup("ivrs_hpet",		parse_ivrs_hpet);
+__setup("ivrs_acpihid",		parse_ivrs_acpihid);
 
 IOMMU_INIT_FINISH(amd_iommu_detect,
 		  gart_iommu_hole_init,
-- 
1.9.1


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

* [PATCH 4/6] iommu/amd: Introduces ivrs_acpihid kernel parameter
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wan Zongshun, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

From: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>

This patch introduces a new kernel parameter, ivrs_acpihid.
This is used to override existing ACPI-HID IVHD device entry,
or add an entry in case it is missing in the IVHD.

Signed-off-by: Wan Zongshun <Vincent.Wan-5C7GfCeVMHo@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 Documentation/kernel-parameters.txt |  7 +++++++
 drivers/iommu/amd_iommu_init.c      | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 742f69d..5c364c6 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1680,6 +1680,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			PCI device 00:14.0 write the parameter as:
 				ivrs_hpet[0]=00:14.0
 
+	ivrs_acpihid	[HW,X86_64]
+			Provide an override to the ACPI-HID:UID<->DEVICE-ID
+			mapping provided in the IVRS ACPI table. For
+			example, to map UART-HID:UID AMD0020:0 to
+			PCI device 00:14.5 write the parameter as:
+				ivrs_acpihid[00:14.5]=AMD0020:0
+
 	js=		[HW,JOY] Analog joystick
 			See Documentation/input/joystick.txt.
 
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 0a8e033..ad05614 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2430,10 +2430,43 @@ static int __init parse_ivrs_hpet(char *str)
 	return 1;
 }
 
+static int __init parse_ivrs_acpihid(char *str)
+{
+	u32 bus, dev, fn;
+	char *hid, *uid, *p;
+	char acpiid[ACPIHID_UID_LEN + ACPIHID_HID_LEN] = {0};
+	int ret, i;
+
+	ret = sscanf(str, "[%x:%x.%x]=%s", &bus, &dev, &fn, acpiid);
+	if (ret != 4) {
+		pr_err("AMD-Vi: Invalid command line: ivrs_acpihid(%s)\n", str);
+		return 1;
+	}
+
+	p = acpiid;
+	hid = strsep(&p, ":");
+	uid = p;
+
+	if (!hid || !(*hid) || !uid) {
+		pr_err("AMD-Vi: Invalid command line: hid or uid\n");
+		return 1;
+	}
+
+	i = early_acpihid_map_size++;
+	memcpy(early_acpihid_map[i].hid, hid, strlen(hid));
+	memcpy(early_acpihid_map[i].uid, uid, strlen(uid));
+	early_acpihid_map[i].devid =
+		((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
+	early_acpihid_map[i].cmd_line	= true;
+
+	return 1;
+}
+
 __setup("amd_iommu_dump",	parse_amd_iommu_dump);
 __setup("amd_iommu=",		parse_amd_iommu_options);
 __setup("ivrs_ioapic",		parse_ivrs_ioapic);
 __setup("ivrs_hpet",		parse_ivrs_hpet);
+__setup("ivrs_acpihid",		parse_ivrs_acpihid);
 
 IOMMU_INIT_FINISH(amd_iommu_detect,
 		  gart_iommu_hole_init,
-- 
1.9.1

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

* [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu
  Cc: Suravee Suthikulpanit, Borislav Petkov, Ray Huang, ken.xue,
	linux-kernel, Wan Zongshun

From: Wan Zongshun <Vincent.Wan@amd.com>

Current IOMMU driver make assumption that the downstream devices are PCI.
With the newly added ACPI-HID IVHD device entry support, this is no
longer true. This patch is to add dev type check and to distinguish the
pci and acpihid device code path.

Signed-off-by: Wan Zongshun <Vincent.Wan@amd.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c | 124 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 106 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 98acb89..1902d01 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -19,6 +19,7 @@
 
 #include <linux/ratelimit.h>
 #include <linux/pci.h>
+#include <linux/acpi.h>
 #include <linux/pci-ats.h>
 #include <linux/bitmap.h>
 #include <linux/slab.h>
@@ -176,13 +177,56 @@ static struct iommu_dev_data *find_dev_data(u16 devid)
 	return dev_data;
 }
 
-static inline u16 get_device_id(struct device *dev)
+static inline int match_hid_uid(struct device *dev,
+				struct acpihid_map_entry *entry)
+{
+	const char *hid, *uid;
+
+	hid = acpi_device_hid(ACPI_COMPANION(dev));
+	uid = acpi_device_uid(ACPI_COMPANION(dev));
+
+	if (!hid || !(*hid))
+		return -ENODEV;
+
+	if (!uid || !(*uid))
+		return strcmp(hid, entry->hid);
+
+	if (!(*entry->uid))
+		return strcmp(hid, entry->hid);
+
+	return -ENODEV;
+}
+
+static inline u16 get_pci_device_id(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
 	return PCI_DEVID(pdev->bus->number, pdev->devfn);
 }
 
+static inline int get_acpihid_device_id(struct device *dev,
+					struct acpihid_map_entry **entry)
+{
+	struct acpihid_map_entry *p;
+
+	list_for_each_entry(p, &acpihid_map, list) {
+		if (!match_hid_uid(dev, p)) {
+			if (entry)
+				*entry = p;
+			return p->devid;
+		}
+	}
+	return -EINVAL;
+}
+
+static inline u16 get_device_id(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		return get_pci_device_id(dev);
+	else
+		return get_acpihid_device_id(dev, NULL);
+}
+
 static struct iommu_dev_data *get_dev_data(struct device *dev)
 {
 	return dev->archdata.iommu;
@@ -262,7 +306,7 @@ static bool check_device(struct device *dev)
 		return false;
 
 	/* No PCI device */
-	if (!dev_is_pci(dev))
+	if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
 		return false;
 
 	devid = get_device_id(dev);
@@ -298,7 +342,7 @@ out:
 	iommu_group_put(group);
 }
 
-static int iommu_init_device(struct device *dev)
+static int iommu_init_pci_device(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct iommu_dev_data *dev_data;
@@ -325,6 +369,32 @@ static int iommu_init_device(struct device *dev)
 	return 0;
 }
 
+static int iommu_init_acpihid_device(struct device *dev)
+{
+	struct iommu_dev_data *dev_data;
+
+	if (dev->archdata.iommu)
+		return 0;
+
+	dev_data = find_dev_data(get_device_id(dev));
+	if (!dev_data)
+		return -ENOMEM;
+
+	dev->archdata.iommu = dev_data;
+
+	iommu_device_link(amd_iommu_rlookup_table[dev_data->devid]->iommu_dev,
+			  dev);
+
+	return 0;
+}
+
+static int iommu_init_device(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		return iommu_init_pci_device(dev);
+	return iommu_init_acpihid_device(dev);
+}
+
 static void iommu_ignore_device(struct device *dev)
 {
 	u16 devid, alias;
@@ -2066,19 +2136,16 @@ static bool pci_pri_tlp_required(struct pci_dev *pdev)
 	return (status & PCI_PRI_TLP_OFF) ? true : false;
 }
 
-/*
- * If a device is not yet associated with a domain, this function
- * assigns it visible for the hardware
- */
-static int attach_device(struct device *dev,
-			 struct protection_domain *domain)
+static int setup_ats_device(struct device *dev,
+			    struct protection_domain *domain)
 {
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct iommu_dev_data *dev_data;
-	unsigned long flags;
-	int ret;
+	struct pci_dev *pdev;
+	struct iommu_dev_data *dev_data = get_dev_data(dev);
 
-	dev_data = get_dev_data(dev);
+	if (!dev_is_pci(dev))
+		return 0;
+
+	pdev = to_pci_dev(dev);
 
 	if (domain->flags & PD_IOMMUV2_MASK) {
 		if (!dev_data->passthrough)
@@ -2098,6 +2165,25 @@ static int attach_device(struct device *dev,
 		dev_data->ats.qdep    = pci_ats_queue_depth(pdev);
 	}
 
+	return 0;
+}
+
+/*
+ * If a device is not yet associated with a domain, this function
+ * assigns it visible for the hardware
+ */
+static int attach_device(struct device *dev,
+			 struct protection_domain *domain)
+{
+	struct iommu_dev_data *dev_data;
+	unsigned long flags;
+	int ret;
+
+	dev_data = get_dev_data(dev);
+	ret = setup_ats_device(dev, domain);
+	if (ret)
+		return ret;
+
 	write_lock_irqsave(&amd_iommu_devtable_lock, flags);
 	ret = __attach_device(dev_data, domain);
 	write_unlock_irqrestore(&amd_iommu_devtable_lock, flags);
@@ -2154,10 +2240,12 @@ static void detach_device(struct device *dev)
 	__detach_device(dev_data);
 	write_unlock_irqrestore(&amd_iommu_devtable_lock, flags);
 
-	if (domain->flags & PD_IOMMUV2_MASK && dev_data->iommu_v2)
-		pdev_iommuv2_disable(to_pci_dev(dev));
-	else if (dev_data->ats.enabled)
-		pci_disable_ats(to_pci_dev(dev));
+	if (dev_is_pci(dev)) {
+		if (domain->flags & PD_IOMMUV2_MASK && dev_data->iommu_v2)
+			pdev_iommuv2_disable(to_pci_dev(dev));
+		else if (dev_data->ats.enabled)
+			pci_disable_ats(to_pci_dev(dev));
+	}
 
 	dev_data->ats.enabled = false;
 }
-- 
1.9.1


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

* [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wan Zongshun, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

From: Wan Zongshun <Vincent.Wan-5C7GfCeVMHo@public.gmane.org>

Current IOMMU driver make assumption that the downstream devices are PCI.
With the newly added ACPI-HID IVHD device entry support, this is no
longer true. This patch is to add dev type check and to distinguish the
pci and acpihid device code path.

Signed-off-by: Wan Zongshun <Vincent.Wan-5C7GfCeVMHo@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c | 124 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 106 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 98acb89..1902d01 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -19,6 +19,7 @@
 
 #include <linux/ratelimit.h>
 #include <linux/pci.h>
+#include <linux/acpi.h>
 #include <linux/pci-ats.h>
 #include <linux/bitmap.h>
 #include <linux/slab.h>
@@ -176,13 +177,56 @@ static struct iommu_dev_data *find_dev_data(u16 devid)
 	return dev_data;
 }
 
-static inline u16 get_device_id(struct device *dev)
+static inline int match_hid_uid(struct device *dev,
+				struct acpihid_map_entry *entry)
+{
+	const char *hid, *uid;
+
+	hid = acpi_device_hid(ACPI_COMPANION(dev));
+	uid = acpi_device_uid(ACPI_COMPANION(dev));
+
+	if (!hid || !(*hid))
+		return -ENODEV;
+
+	if (!uid || !(*uid))
+		return strcmp(hid, entry->hid);
+
+	if (!(*entry->uid))
+		return strcmp(hid, entry->hid);
+
+	return -ENODEV;
+}
+
+static inline u16 get_pci_device_id(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
 	return PCI_DEVID(pdev->bus->number, pdev->devfn);
 }
 
+static inline int get_acpihid_device_id(struct device *dev,
+					struct acpihid_map_entry **entry)
+{
+	struct acpihid_map_entry *p;
+
+	list_for_each_entry(p, &acpihid_map, list) {
+		if (!match_hid_uid(dev, p)) {
+			if (entry)
+				*entry = p;
+			return p->devid;
+		}
+	}
+	return -EINVAL;
+}
+
+static inline u16 get_device_id(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		return get_pci_device_id(dev);
+	else
+		return get_acpihid_device_id(dev, NULL);
+}
+
 static struct iommu_dev_data *get_dev_data(struct device *dev)
 {
 	return dev->archdata.iommu;
@@ -262,7 +306,7 @@ static bool check_device(struct device *dev)
 		return false;
 
 	/* No PCI device */
-	if (!dev_is_pci(dev))
+	if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
 		return false;
 
 	devid = get_device_id(dev);
@@ -298,7 +342,7 @@ out:
 	iommu_group_put(group);
 }
 
-static int iommu_init_device(struct device *dev)
+static int iommu_init_pci_device(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct iommu_dev_data *dev_data;
@@ -325,6 +369,32 @@ static int iommu_init_device(struct device *dev)
 	return 0;
 }
 
+static int iommu_init_acpihid_device(struct device *dev)
+{
+	struct iommu_dev_data *dev_data;
+
+	if (dev->archdata.iommu)
+		return 0;
+
+	dev_data = find_dev_data(get_device_id(dev));
+	if (!dev_data)
+		return -ENOMEM;
+
+	dev->archdata.iommu = dev_data;
+
+	iommu_device_link(amd_iommu_rlookup_table[dev_data->devid]->iommu_dev,
+			  dev);
+
+	return 0;
+}
+
+static int iommu_init_device(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		return iommu_init_pci_device(dev);
+	return iommu_init_acpihid_device(dev);
+}
+
 static void iommu_ignore_device(struct device *dev)
 {
 	u16 devid, alias;
@@ -2066,19 +2136,16 @@ static bool pci_pri_tlp_required(struct pci_dev *pdev)
 	return (status & PCI_PRI_TLP_OFF) ? true : false;
 }
 
-/*
- * If a device is not yet associated with a domain, this function
- * assigns it visible for the hardware
- */
-static int attach_device(struct device *dev,
-			 struct protection_domain *domain)
+static int setup_ats_device(struct device *dev,
+			    struct protection_domain *domain)
 {
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct iommu_dev_data *dev_data;
-	unsigned long flags;
-	int ret;
+	struct pci_dev *pdev;
+	struct iommu_dev_data *dev_data = get_dev_data(dev);
 
-	dev_data = get_dev_data(dev);
+	if (!dev_is_pci(dev))
+		return 0;
+
+	pdev = to_pci_dev(dev);
 
 	if (domain->flags & PD_IOMMUV2_MASK) {
 		if (!dev_data->passthrough)
@@ -2098,6 +2165,25 @@ static int attach_device(struct device *dev,
 		dev_data->ats.qdep    = pci_ats_queue_depth(pdev);
 	}
 
+	return 0;
+}
+
+/*
+ * If a device is not yet associated with a domain, this function
+ * assigns it visible for the hardware
+ */
+static int attach_device(struct device *dev,
+			 struct protection_domain *domain)
+{
+	struct iommu_dev_data *dev_data;
+	unsigned long flags;
+	int ret;
+
+	dev_data = get_dev_data(dev);
+	ret = setup_ats_device(dev, domain);
+	if (ret)
+		return ret;
+
 	write_lock_irqsave(&amd_iommu_devtable_lock, flags);
 	ret = __attach_device(dev_data, domain);
 	write_unlock_irqrestore(&amd_iommu_devtable_lock, flags);
@@ -2154,10 +2240,12 @@ static void detach_device(struct device *dev)
 	__detach_device(dev_data);
 	write_unlock_irqrestore(&amd_iommu_devtable_lock, flags);
 
-	if (domain->flags & PD_IOMMUV2_MASK && dev_data->iommu_v2)
-		pdev_iommuv2_disable(to_pci_dev(dev));
-	else if (dev_data->ats.enabled)
-		pci_disable_ats(to_pci_dev(dev));
+	if (dev_is_pci(dev)) {
+		if (domain->flags & PD_IOMMUV2_MASK && dev_data->iommu_v2)
+			pdev_iommuv2_disable(to_pci_dev(dev));
+		else if (dev_data->ats.enabled)
+			pci_disable_ats(to_pci_dev(dev));
+	}
 
 	dev_data->ats.enabled = false;
 }
-- 
1.9.1

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

* [PATCH 6/6] iommu/amd: Manage iommu_group for non-pci devices
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu
  Cc: Suravee Suthikulpanit, Borislav Petkov, Ray Huang, ken.xue,
	linux-kernel, Wan Zongshun

From: Wan Zongshun <Vincent.Wan@amd.com>

This patch creates a new function for finding or creating an IOMMU
group for acpihid(ACPI Hardware ID) device.

The acpihid devices with the same devid will be put into same group and
there will have the same domain id and share the same page table.

Signed-off-by: Wan Zongshun <Vincent.Wan@amd.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1902d01..e1083ce 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -232,6 +232,29 @@ static struct iommu_dev_data *get_dev_data(struct device *dev)
 	return dev->archdata.iommu;
 }
 
+/*
+* Find or create an IOMMU group for a acpihid device.
+*/
+static struct iommu_group *acpihid_device_group(struct device *dev)
+{
+	struct acpihid_map_entry *p, *entry = NULL;
+	u16 devid;
+
+	devid = get_acpihid_device_id(dev, &entry);
+	if (devid < 0)
+		return ERR_PTR(devid);
+
+	list_for_each_entry(p, &acpihid_map, list) {
+		if ((devid == p->devid) && p->group)
+			entry->group = p->group;
+	}
+
+	if (!entry->group)
+		entry->group = generic_device_group(dev);
+
+	return entry->group;
+}
+
 static bool pci_iommuv2_capable(struct pci_dev *pdev)
 {
 	static const int caps[] = {
@@ -2311,6 +2334,14 @@ static void amd_iommu_remove_device(struct device *dev)
 	iommu_completion_wait(iommu);
 }
 
+static struct iommu_group *amd_iommu_device_group(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		return pci_device_group(dev);
+
+	return acpihid_device_group(dev);
+}
+
 /*****************************************************************************
  *
  * The next functions belong to the dma_ops mapping/unmapping code.
@@ -3202,7 +3233,7 @@ static const struct iommu_ops amd_iommu_ops = {
 	.iova_to_phys = amd_iommu_iova_to_phys,
 	.add_device = amd_iommu_add_device,
 	.remove_device = amd_iommu_remove_device,
-	.device_group = pci_device_group,
+	.device_group = amd_iommu_device_group,
 	.get_dm_regions = amd_iommu_get_dm_regions,
 	.put_dm_regions = amd_iommu_put_dm_regions,
 	.pgsize_bitmap	= AMD_IOMMU_PGSIZES,
-- 
1.9.1


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

* [PATCH 6/6] iommu/amd: Manage iommu_group for non-pci devices
@ 2016-01-05 10:07   ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-05 10:07 UTC (permalink / raw)
  To: Joerg Roedel, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Wan Zongshun, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

From: Wan Zongshun <Vincent.Wan-5C7GfCeVMHo@public.gmane.org>

This patch creates a new function for finding or creating an IOMMU
group for acpihid(ACPI Hardware ID) device.

The acpihid devices with the same devid will be put into same group and
there will have the same domain id and share the same page table.

Signed-off-by: Wan Zongshun <Vincent.Wan-5C7GfCeVMHo@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1902d01..e1083ce 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -232,6 +232,29 @@ static struct iommu_dev_data *get_dev_data(struct device *dev)
 	return dev->archdata.iommu;
 }
 
+/*
+* Find or create an IOMMU group for a acpihid device.
+*/
+static struct iommu_group *acpihid_device_group(struct device *dev)
+{
+	struct acpihid_map_entry *p, *entry = NULL;
+	u16 devid;
+
+	devid = get_acpihid_device_id(dev, &entry);
+	if (devid < 0)
+		return ERR_PTR(devid);
+
+	list_for_each_entry(p, &acpihid_map, list) {
+		if ((devid == p->devid) && p->group)
+			entry->group = p->group;
+	}
+
+	if (!entry->group)
+		entry->group = generic_device_group(dev);
+
+	return entry->group;
+}
+
 static bool pci_iommuv2_capable(struct pci_dev *pdev)
 {
 	static const int caps[] = {
@@ -2311,6 +2334,14 @@ static void amd_iommu_remove_device(struct device *dev)
 	iommu_completion_wait(iommu);
 }
 
+static struct iommu_group *amd_iommu_device_group(struct device *dev)
+{
+	if (dev_is_pci(dev))
+		return pci_device_group(dev);
+
+	return acpihid_device_group(dev);
+}
+
 /*****************************************************************************
  *
  * The next functions belong to the dma_ops mapping/unmapping code.
@@ -3202,7 +3233,7 @@ static const struct iommu_ops amd_iommu_ops = {
 	.iova_to_phys = amd_iommu_iova_to_phys,
 	.add_device = amd_iommu_add_device,
 	.remove_device = amd_iommu_remove_device,
-	.device_group = pci_device_group,
+	.device_group = amd_iommu_device_group,
 	.get_dm_regions = amd_iommu_get_dm_regions,
 	.put_dm_regions = amd_iommu_put_dm_regions,
 	.pgsize_bitmap	= AMD_IOMMU_PGSIZES,
-- 
1.9.1

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

* Re: [PATCH 0/6] iommu/amd: enable ACPI hardware ID device support
@ 2016-01-05 15:40   ` Suravee Suthikulpanit
  0 siblings, 0 replies; 30+ messages in thread
From: Suravee Suthikulpanit @ 2016-01-05 15:40 UTC (permalink / raw)
  To: Wan Zongshun, Joerg Roedel, iommu
  Cc: Borislav Petkov, Ray Huang, ken.xue, linux-kernel

Hi All,

Also, the latest public version of AMD IOMMU specification that 
describes the support for IVHD type 40h and ACPI HID IVHD device type, 
implemented by this patch series, is available here

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

Thanks,
Suravee


On 01/05/2016 04:07 AM, Wan Zongshun wrote:
> From: Wan Zongshun <Vincent.Wan@amd.com>
>
> This patch series enable ACPI hardware ID device support,
>
> There are some devices indentified using ACPI HID format in AMD chip.
> This patch series enable iommu support for those ACPI HID device,
> since the existing AMD iommu only supports PCI bus based device.
>
> Suravee Suthikulpanit (3):
>    iommu/amd: Modify ivhd_header structure to support type 11h and 40h
>    iommu/amd: Use the most comprehensive IVHD type that the driver can
>      support
>    iommu/amd: Introduces ivrs_acpihid kernel parameter
>
> Wan Zongshun (3):
>    iommu/amd: Add new map for storing IVHD dev entry type HID
>    iommu/amd: Add support for non-pci devices
>    iommu/amd: Manage iommu_group for non-pci devices
>
>   Documentation/kernel-parameters.txt |   7 +
>   drivers/iommu/amd_iommu.c           | 158 +++++++++++++++---
>   drivers/iommu/amd_iommu_init.c      | 309 +++++++++++++++++++++++++++++++-----
>   drivers/iommu/amd_iommu_types.h     |  14 ++
>   4 files changed, 433 insertions(+), 55 deletions(-)
>

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

* Re: [PATCH 0/6] iommu/amd: enable ACPI hardware ID device support
@ 2016-01-05 15:40   ` Suravee Suthikulpanit
  0 siblings, 0 replies; 30+ messages in thread
From: Suravee Suthikulpanit @ 2016-01-05 15:40 UTC (permalink / raw)
  To: Wan Zongshun, Joerg Roedel,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: Borislav Petkov, Ray Huang, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	ken.xue-5C7GfCeVMHo

Hi All,

Also, the latest public version of AMD IOMMU specification that 
describes the support for IVHD type 40h and ACPI HID IVHD device type, 
implemented by this patch series, is available here

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

Thanks,
Suravee


On 01/05/2016 04:07 AM, Wan Zongshun wrote:
> From: Wan Zongshun <Vincent.Wan-5C7GfCeVMHo@public.gmane.org>
>
> This patch series enable ACPI hardware ID device support,
>
> There are some devices indentified using ACPI HID format in AMD chip.
> This patch series enable iommu support for those ACPI HID device,
> since the existing AMD iommu only supports PCI bus based device.
>
> Suravee Suthikulpanit (3):
>    iommu/amd: Modify ivhd_header structure to support type 11h and 40h
>    iommu/amd: Use the most comprehensive IVHD type that the driver can
>      support
>    iommu/amd: Introduces ivrs_acpihid kernel parameter
>
> Wan Zongshun (3):
>    iommu/amd: Add new map for storing IVHD dev entry type HID
>    iommu/amd: Add support for non-pci devices
>    iommu/amd: Manage iommu_group for non-pci devices
>
>   Documentation/kernel-parameters.txt |   7 +
>   drivers/iommu/amd_iommu.c           | 158 +++++++++++++++---
>   drivers/iommu/amd_iommu_init.c      | 309 +++++++++++++++++++++++++++++++-----
>   drivers/iommu/amd_iommu_types.h     |  14 ++
>   4 files changed, 433 insertions(+), 55 deletions(-)
>

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-07 12:04     ` Joerg Roedel
  0 siblings, 0 replies; 30+ messages in thread
From: Joerg Roedel @ 2016-01-07 12:04 UTC (permalink / raw)
  To: Wan Zongshun
  Cc: iommu, Suravee Suthikulpanit, Borislav Petkov, Ray Huang,
	ken.xue, linux-kernel

On Tue, Jan 05, 2016 at 05:07:23AM -0500, Wan Zongshun wrote:
> -static inline u16 get_device_id(struct device *dev)
> +static inline int match_hid_uid(struct device *dev,
> +				struct acpihid_map_entry *entry)
> +{
> +	const char *hid, *uid;
> +
> +	hid = acpi_device_hid(ACPI_COMPANION(dev));
> +	uid = acpi_device_uid(ACPI_COMPANION(dev));
> +
> +	if (!hid || !(*hid))
> +		return -ENODEV;
> +
> +	if (!uid || !(*uid))
> +		return strcmp(hid, entry->hid);
> +
> +	if (!(*entry->uid))
> +		return strcmp(hid, entry->hid);
> +
> +	return -ENODEV;
> +}
> +
> +static inline u16 get_pci_device_id(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  
>  	return PCI_DEVID(pdev->bus->number, pdev->devfn);
>  }
>  
> +static inline int get_acpihid_device_id(struct device *dev,
> +					struct acpihid_map_entry **entry)
> +{
> +	struct acpihid_map_entry *p;
> +
> +	list_for_each_entry(p, &acpihid_map, list) {
> +		if (!match_hid_uid(dev, p)) {
> +			if (entry)
> +				*entry = p;
> +			return p->devid;
> +		}
> +	}
> +	return -EINVAL;
> +}
> +
> +static inline u16 get_device_id(struct device *dev)
> +{
> +	if (dev_is_pci(dev))
> +		return get_pci_device_id(dev);
> +	else
> +		return get_acpihid_device_id(dev, NULL);
> +}

This is not robust, get_acpihid_device_id() returns int and can return a
negative value. This gets lost when converting it to u16 here. So either
you add error handling for get_acpihid_device_id() in get_device_id() or
you change get_device_id() to return int too and handle the error at the
callers of get_device_id().


	Joerg


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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-07 12:04     ` Joerg Roedel
  0 siblings, 0 replies; 30+ messages in thread
From: Joerg Roedel @ 2016-01-07 12:04 UTC (permalink / raw)
  To: Wan Zongshun
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

On Tue, Jan 05, 2016 at 05:07:23AM -0500, Wan Zongshun wrote:
> -static inline u16 get_device_id(struct device *dev)
> +static inline int match_hid_uid(struct device *dev,
> +				struct acpihid_map_entry *entry)
> +{
> +	const char *hid, *uid;
> +
> +	hid = acpi_device_hid(ACPI_COMPANION(dev));
> +	uid = acpi_device_uid(ACPI_COMPANION(dev));
> +
> +	if (!hid || !(*hid))
> +		return -ENODEV;
> +
> +	if (!uid || !(*uid))
> +		return strcmp(hid, entry->hid);
> +
> +	if (!(*entry->uid))
> +		return strcmp(hid, entry->hid);
> +
> +	return -ENODEV;
> +}
> +
> +static inline u16 get_pci_device_id(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  
>  	return PCI_DEVID(pdev->bus->number, pdev->devfn);
>  }
>  
> +static inline int get_acpihid_device_id(struct device *dev,
> +					struct acpihid_map_entry **entry)
> +{
> +	struct acpihid_map_entry *p;
> +
> +	list_for_each_entry(p, &acpihid_map, list) {
> +		if (!match_hid_uid(dev, p)) {
> +			if (entry)
> +				*entry = p;
> +			return p->devid;
> +		}
> +	}
> +	return -EINVAL;
> +}
> +
> +static inline u16 get_device_id(struct device *dev)
> +{
> +	if (dev_is_pci(dev))
> +		return get_pci_device_id(dev);
> +	else
> +		return get_acpihid_device_id(dev, NULL);
> +}

This is not robust, get_acpihid_device_id() returns int and can return a
negative value. This gets lost when converting it to u16 here. So either
you add error handling for get_acpihid_device_id() in get_device_id() or
you change get_device_id() to return int too and handle the error at the
callers of get_device_id().


	Joerg

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

* Re: [PATCH 6/6] iommu/amd: Manage iommu_group for non-pci devices
@ 2016-01-07 12:06     ` Joerg Roedel
  0 siblings, 0 replies; 30+ messages in thread
From: Joerg Roedel @ 2016-01-07 12:06 UTC (permalink / raw)
  To: Wan Zongshun
  Cc: iommu, Suravee Suthikulpanit, Borislav Petkov, Ray Huang,
	ken.xue, linux-kernel

On Tue, Jan 05, 2016 at 05:07:24AM -0500, Wan Zongshun wrote:
> +static struct iommu_group *amd_iommu_device_group(struct device *dev)
> +{
> +	if (dev_is_pci(dev))
> +		return pci_device_group(dev);
> +
> +	return acpihid_device_group(dev);
> +}
> +
>  /*****************************************************************************
>   *
>   * The next functions belong to the dma_ops mapping/unmapping code.
> @@ -3202,7 +3233,7 @@ static const struct iommu_ops amd_iommu_ops = {
>  	.iova_to_phys = amd_iommu_iova_to_phys,
>  	.add_device = amd_iommu_add_device,
>  	.remove_device = amd_iommu_remove_device,
> -	.device_group = pci_device_group,
> +	.device_group = amd_iommu_device_group,

Does this work? Which bus do the ACPIHID devices belong to (what does
dev->bus point to)? If it is not &pci_bus_type, then the iommu core code
will not create an iommu group for the devices.


	Joerg


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

* Re: [PATCH 6/6] iommu/amd: Manage iommu_group for non-pci devices
@ 2016-01-07 12:06     ` Joerg Roedel
  0 siblings, 0 replies; 30+ messages in thread
From: Joerg Roedel @ 2016-01-07 12:06 UTC (permalink / raw)
  To: Wan Zongshun
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

On Tue, Jan 05, 2016 at 05:07:24AM -0500, Wan Zongshun wrote:
> +static struct iommu_group *amd_iommu_device_group(struct device *dev)
> +{
> +	if (dev_is_pci(dev))
> +		return pci_device_group(dev);
> +
> +	return acpihid_device_group(dev);
> +}
> +
>  /*****************************************************************************
>   *
>   * The next functions belong to the dma_ops mapping/unmapping code.
> @@ -3202,7 +3233,7 @@ static const struct iommu_ops amd_iommu_ops = {
>  	.iova_to_phys = amd_iommu_iova_to_phys,
>  	.add_device = amd_iommu_add_device,
>  	.remove_device = amd_iommu_remove_device,
> -	.device_group = pci_device_group,
> +	.device_group = amd_iommu_device_group,

Does this work? Which bus do the ACPIHID devices belong to (what does
dev->bus point to)? If it is not &pci_bus_type, then the iommu core code
will not create an iommu group for the devices.


	Joerg

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

* Re: [PATCH 6/6] iommu/amd: Manage iommu_group for non-pci devices
  2016-01-07 12:06     ` Joerg Roedel
  (?)
@ 2016-01-08  1:44     ` Wan ZongShun
  -1 siblings, 0 replies; 30+ messages in thread
From: Wan ZongShun @ 2016-01-08  1:44 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Wan Zongshun, linux-kernel, iommu, Ray Huang, Borislav Petkov,
	ken.xue, vw

2016-01-07 20:06 GMT+08:00 Joerg Roedel <joro@8bytes.org>:
> On Tue, Jan 05, 2016 at 05:07:24AM -0500, Wan Zongshun wrote:
>> +static struct iommu_group *amd_iommu_device_group(struct device *dev)
>> +{
>> +     if (dev_is_pci(dev))
>> +             return pci_device_group(dev);
>> +
>> +     return acpihid_device_group(dev);
>> +}
>> +
>>  /*****************************************************************************
>>   *
>>   * The next functions belong to the dma_ops mapping/unmapping code.
>> @@ -3202,7 +3233,7 @@ static const struct iommu_ops amd_iommu_ops = {
>>       .iova_to_phys = amd_iommu_iova_to_phys,
>>       .add_device = amd_iommu_add_device,
>>       .remove_device = amd_iommu_remove_device,
>> -     .device_group = pci_device_group,
>> +     .device_group = amd_iommu_device_group,
>
> Does this work? Which bus do the ACPIHID devices belong to (what does
> dev->bus point to)? If it is not &pci_bus_type, then the iommu core code
> will not create an iommu group for the devices.

Yes, it works, we have already done validation on AMD platform.
Please refer to my previous patch :
[PATCH] iommu/amd: set AMD iommu-callbacks for the amba bus.

Currently, The UART DMA is the use case of ACPIHID device, the
dev->bus is amba_bus, so I add bus_set_iommu for this bus type.

We will do create iommu group in acpihid_device_group which will call
generic_device_group to alloc group.

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



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-08  3:15       ` Wan ZongShun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan ZongShun @ 2016-01-08  3:15 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Wan Zongshun, linux-kernel, iommu, Ray Huang, Borislav Petkov,
	ken.xue, vw

2016-01-07 20:04 GMT+08:00 Joerg Roedel <joro@8bytes.org>:
> On Tue, Jan 05, 2016 at 05:07:23AM -0500, Wan Zongshun wrote:
>> -static inline u16 get_device_id(struct device *dev)
>> +static inline int match_hid_uid(struct device *dev,
>> +                             struct acpihid_map_entry *entry)
>> +{
>> +     const char *hid, *uid;
>> +
>> +     hid = acpi_device_hid(ACPI_COMPANION(dev));
>> +     uid = acpi_device_uid(ACPI_COMPANION(dev));
>> +
>> +     if (!hid || !(*hid))
>> +             return -ENODEV;
>> +
>> +     if (!uid || !(*uid))
>> +             return strcmp(hid, entry->hid);
>> +
>> +     if (!(*entry->uid))
>> +             return strcmp(hid, entry->hid);
>> +
>> +     return -ENODEV;
>> +}
>> +
>> +static inline u16 get_pci_device_id(struct device *dev)
>>  {
>>       struct pci_dev *pdev = to_pci_dev(dev);
>>
>>       return PCI_DEVID(pdev->bus->number, pdev->devfn);
>>  }
>>
>> +static inline int get_acpihid_device_id(struct device *dev,
>> +                                     struct acpihid_map_entry **entry)
>> +{
>> +     struct acpihid_map_entry *p;
>> +
>> +     list_for_each_entry(p, &acpihid_map, list) {
>> +             if (!match_hid_uid(dev, p)) {
>> +                     if (entry)
>> +                             *entry = p;
>> +                     return p->devid;
>> +             }
>> +     }
>> +     return -EINVAL;
>> +}
>> +
>> +static inline u16 get_device_id(struct device *dev)
>> +{
>> +     if (dev_is_pci(dev))
>> +             return get_pci_device_id(dev);
>> +     else
>> +             return get_acpihid_device_id(dev, NULL);
>> +}
>
> This is not robust, get_acpihid_device_id() returns int and can return a
> negative value. This gets lost when converting it to u16 here. So either
> you add error handling for get_acpihid_device_id() in get_device_id() or
> you change get_device_id() to return int too and handle the error at the
> callers of get_device_id().

Joerg,

Please see the following function, since I judge this
'get_acpihid_device_id(dev, NULL) < 0'  in the front of
'get_device_id', so your concern should not exist. I have already
filtered the negative situation in check_device firstly, do you think
it is ok?


static bool check_device(struct device *dev)
{
        u16 devid;
......

        /* No PCI device */
        if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
                return false;

        devid = get_device_id(dev);

.....

        return true;
}


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



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-08  3:15       ` Wan ZongShun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan ZongShun @ 2016-01-08  3:15 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: vw-6ukY98dZOFrYtjvyW6yDsg, linux-kernel, Wan Zongshun,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

2016-01-07 20:04 GMT+08:00 Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>:
> On Tue, Jan 05, 2016 at 05:07:23AM -0500, Wan Zongshun wrote:
>> -static inline u16 get_device_id(struct device *dev)
>> +static inline int match_hid_uid(struct device *dev,
>> +                             struct acpihid_map_entry *entry)
>> +{
>> +     const char *hid, *uid;
>> +
>> +     hid = acpi_device_hid(ACPI_COMPANION(dev));
>> +     uid = acpi_device_uid(ACPI_COMPANION(dev));
>> +
>> +     if (!hid || !(*hid))
>> +             return -ENODEV;
>> +
>> +     if (!uid || !(*uid))
>> +             return strcmp(hid, entry->hid);
>> +
>> +     if (!(*entry->uid))
>> +             return strcmp(hid, entry->hid);
>> +
>> +     return -ENODEV;
>> +}
>> +
>> +static inline u16 get_pci_device_id(struct device *dev)
>>  {
>>       struct pci_dev *pdev = to_pci_dev(dev);
>>
>>       return PCI_DEVID(pdev->bus->number, pdev->devfn);
>>  }
>>
>> +static inline int get_acpihid_device_id(struct device *dev,
>> +                                     struct acpihid_map_entry **entry)
>> +{
>> +     struct acpihid_map_entry *p;
>> +
>> +     list_for_each_entry(p, &acpihid_map, list) {
>> +             if (!match_hid_uid(dev, p)) {
>> +                     if (entry)
>> +                             *entry = p;
>> +                     return p->devid;
>> +             }
>> +     }
>> +     return -EINVAL;
>> +}
>> +
>> +static inline u16 get_device_id(struct device *dev)
>> +{
>> +     if (dev_is_pci(dev))
>> +             return get_pci_device_id(dev);
>> +     else
>> +             return get_acpihid_device_id(dev, NULL);
>> +}
>
> This is not robust, get_acpihid_device_id() returns int and can return a
> negative value. This gets lost when converting it to u16 here. So either
> you add error handling for get_acpihid_device_id() in get_device_id() or
> you change get_device_id() to return int too and handle the error at the
> callers of get_device_id().

Joerg,

Please see the following function, since I judge this
'get_acpihid_device_id(dev, NULL) < 0'  in the front of
'get_device_id', so your concern should not exist. I have already
filtered the negative situation in check_device firstly, do you think
it is ok?


static bool check_device(struct device *dev)
{
        u16 devid;
......

        /* No PCI device */
        if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
                return false;

        devid = get_device_id(dev);

.....

        return true;
}


>
>
>         Joerg
>
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
  2016-01-08  3:15       ` Wan ZongShun
  (?)
@ 2016-01-08 12:18       ` Joerg Roedel
  2016-01-08 14:52           ` Wan Zongshun
  -1 siblings, 1 reply; 30+ messages in thread
From: Joerg Roedel @ 2016-01-08 12:18 UTC (permalink / raw)
  To: Wan ZongShun
  Cc: Wan Zongshun, linux-kernel, iommu, Ray Huang, Borislav Petkov,
	ken.xue, vw

On Fri, Jan 08, 2016 at 11:15:37AM +0800, Wan ZongShun wrote:
> Please see the following function, since I judge this
> 'get_acpihid_device_id(dev, NULL) < 0'  in the front of
> 'get_device_id', so your concern should not exist. I have already
> filtered the negative situation in check_device firstly, do you think
> it is ok?
> 
> 
> static bool check_device(struct device *dev)
> {
>         u16 devid;
> ......
> 
>         /* No PCI device */
>         if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
>                 return false;
> 
>         devid = get_device_id(dev);

That is true for this case, but the other call-sites of get_device_id()
still have to care about a potential negative return value, right?



	Joerg

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-08 14:52           ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-08 14:52 UTC (permalink / raw)
  To: Joerg Roedel, Wan ZongShun
  Cc: Wan Zongshun, linux-kernel, iommu, Ray Huang, Borislav Petkov, ken.xue


>>
>>
>> static bool check_device(struct device *dev)
>> {
>>          u16 devid;
>> ......
>>
>>          /* No PCI device */
>>          if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
>>                  return false;
>>
>>          devid = get_device_id(dev);
>
> That is true for this case, but the other call-sites of get_device_id()
> still have to care about a potential negative return value, right?
>

Actually I am supposing the '.add_device' will be the first called in 
iommu initializing stage, so I think as long as having no error of check 
device here, any call-sites of get_device_id() will be fine, because 
adding device successfully should be the pre-condition of any iommu 
function can be performed, please correct me.

static int amd_iommu_add_device(struct device *dev)
{
	struct iommu_dev_data *dev_data;
	struct iommu_domain *domain;
	struct amd_iommu *iommu;
	u16 devid;
	int ret;

	if (!check_device(dev) || get_dev_data(dev))
		return 0;

	devid = get_device_id(dev);
	iommu = amd_iommu_rlookup_table[devid];


Vincent.
>
> 	Joerg
>
>

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-08 14:52           ` Wan Zongshun
  0 siblings, 0 replies; 30+ messages in thread
From: Wan Zongshun @ 2016-01-08 14:52 UTC (permalink / raw)
  To: Joerg Roedel, Wan ZongShun
  Cc: linux-kernel, Wan Zongshun,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo


>>
>>
>> static bool check_device(struct device *dev)
>> {
>>          u16 devid;
>> ......
>>
>>          /* No PCI device */
>>          if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
>>                  return false;
>>
>>          devid = get_device_id(dev);
>
> That is true for this case, but the other call-sites of get_device_id()
> still have to care about a potential negative return value, right?
>

Actually I am supposing the '.add_device' will be the first called in 
iommu initializing stage, so I think as long as having no error of check 
device here, any call-sites of get_device_id() will be fine, because 
adding device successfully should be the pre-condition of any iommu 
function can be performed, please correct me.

static int amd_iommu_add_device(struct device *dev)
{
	struct iommu_dev_data *dev_data;
	struct iommu_domain *domain;
	struct amd_iommu *iommu;
	u16 devid;
	int ret;

	if (!check_device(dev) || get_dev_data(dev))
		return 0;

	devid = get_device_id(dev);
	iommu = amd_iommu_rlookup_table[devid];


Vincent.
>
> 	Joerg
>
>

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-08 17:01             ` Joerg Roedel
  0 siblings, 0 replies; 30+ messages in thread
From: Joerg Roedel @ 2016-01-08 17:01 UTC (permalink / raw)
  To: Wan Zongshun
  Cc: Wan ZongShun, Wan Zongshun, linux-kernel, iommu, Ray Huang,
	Borislav Petkov, ken.xue

On Fri, Jan 08, 2016 at 10:52:59PM +0800, Wan Zongshun wrote:
> Actually I am supposing the '.add_device' will be the first called
> in iommu initializing stage, so I think as long as having no error
> of check device here, any call-sites of get_device_id() will be
> fine, because adding device successfully should be the pre-condition
> of any iommu function can be performed, please correct me.
> 
> static int amd_iommu_add_device(struct device *dev)
> {
> 	struct iommu_dev_data *dev_data;
> 	struct iommu_domain *domain;
> 	struct amd_iommu *iommu;
> 	u16 devid;
> 	int ret;
> 
> 	if (!check_device(dev) || get_dev_data(dev))
> 		return 0;
> 
> 	devid = get_device_id(dev);
> 	iommu = amd_iommu_rlookup_table[devid];

There are places in the interrupt remapping code that call get_device_id
without calling check_device first. See get_irq_domain and get_devid.


	Joerg

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
@ 2016-01-08 17:01             ` Joerg Roedel
  0 siblings, 0 replies; 30+ messages in thread
From: Joerg Roedel @ 2016-01-08 17:01 UTC (permalink / raw)
  To: Wan Zongshun
  Cc: linux-kernel, Wan Zongshun,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Ray Huang,
	Borislav Petkov, ken.xue-5C7GfCeVMHo

On Fri, Jan 08, 2016 at 10:52:59PM +0800, Wan Zongshun wrote:
> Actually I am supposing the '.add_device' will be the first called
> in iommu initializing stage, so I think as long as having no error
> of check device here, any call-sites of get_device_id() will be
> fine, because adding device successfully should be the pre-condition
> of any iommu function can be performed, please correct me.
> 
> static int amd_iommu_add_device(struct device *dev)
> {
> 	struct iommu_dev_data *dev_data;
> 	struct iommu_domain *domain;
> 	struct amd_iommu *iommu;
> 	u16 devid;
> 	int ret;
> 
> 	if (!check_device(dev) || get_dev_data(dev))
> 		return 0;
> 
> 	devid = get_device_id(dev);
> 	iommu = amd_iommu_rlookup_table[devid];

There are places in the interrupt remapping code that call get_device_id
without calling check_device first. See get_irq_domain and get_devid.


	Joerg

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
  2016-01-08 17:01             ` Joerg Roedel
  (?)
@ 2016-01-09  9:47             ` Wan Zongshun
  2016-01-20 12:00               ` Joerg Roedel
  -1 siblings, 1 reply; 30+ messages in thread
From: Wan Zongshun @ 2016-01-09  9:47 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Wan Zongshun, linux-kernel, iommu, Ray Huang, Borislav Petkov, ken.xue



-------- Original Message --------
> On Fri, Jan 08, 2016 at 10:52:59PM +0800, Wan Zongshun wrote:
>> Actually I am supposing the '.add_device' will be the first called
>> in iommu initializing stage, so I think as long as having no error
>> of check device here, any call-sites of get_device_id() will be
>> fine, because adding device successfully should be the pre-condition
>> of any iommu function can be performed, please correct me.
>>
>> static int amd_iommu_add_device(struct device *dev)
>> {
>> 	struct iommu_dev_data *dev_data;
>> 	struct iommu_domain *domain;
>> 	struct amd_iommu *iommu;
>> 	u16 devid;
>> 	int ret;
>>
>> 	if (!check_device(dev) || get_dev_data(dev))
>> 		return 0;
>>
>> 	devid = get_device_id(dev);
>> 	iommu = amd_iommu_rlookup_table[devid];
>
> There are places in the interrupt remapping code that call get_device_id
> without calling check_device first. See get_irq_domain and get_devid.
>

Okay, I will change this get_device_id return to int, and judge this 
return value in caller of this function like get_devid style.

If so we will modify some existing amd iommu driver codes, and Can I 
merge those into this patch 5/6? or I will create another dedicated 
patch to take this action?

Vincent.

>
> 	Joerg
>
>

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

* Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices
  2016-01-09  9:47             ` Wan Zongshun
@ 2016-01-20 12:00               ` Joerg Roedel
  0 siblings, 0 replies; 30+ messages in thread
From: Joerg Roedel @ 2016-01-20 12:00 UTC (permalink / raw)
  To: Wan Zongshun
  Cc: Wan Zongshun, linux-kernel, iommu, Ray Huang, Borislav Petkov, ken.xue

Hi Vincent,

On Sat, Jan 09, 2016 at 05:47:15PM +0800, Wan Zongshun wrote:
> If so we will modify some existing amd iommu driver codes, and Can I
> merge those into this patch 5/6? or I will create another dedicated
> patch to take this action?

I think its best to make a seperate patch which makes the call-sites of
get_device_id() aware of negative return values. Please queue this patch
before the patch that actually converts get_device_id() to return
something negative.


	Joerg

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

end of thread, other threads:[~2016-01-20 12:00 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-05 10:07 [PATCH 0/6] iommu/amd: enable ACPI hardware ID device support Wan Zongshun
2016-01-05 10:07 ` Wan Zongshun
2016-01-05 10:07 ` [PATCH 1/6] iommu/amd: Modify ivhd_header structure to support type 11h and 40h Wan Zongshun
2016-01-05 10:07   ` Wan Zongshun
2016-01-05 10:07 ` [PATCH 2/6] iommu/amd: Use the most comprehensive IVHD type that the driver can support Wan Zongshun
2016-01-05 10:07   ` Wan Zongshun
2016-01-05 10:07 ` [PATCH 3/6] iommu/amd: Add new map for storing IVHD dev entry type HID Wan Zongshun
2016-01-05 10:07   ` Wan Zongshun
2016-01-05 10:07 ` [PATCH 4/6] iommu/amd: Introduces ivrs_acpihid kernel parameter Wan Zongshun
2016-01-05 10:07   ` Wan Zongshun
2016-01-05 10:07 ` [PATCH 5/6] iommu/amd: Add support for non-pci devices Wan Zongshun
2016-01-05 10:07   ` Wan Zongshun
2016-01-07 12:04   ` Joerg Roedel
2016-01-07 12:04     ` Joerg Roedel
2016-01-08  3:15     ` Wan ZongShun
2016-01-08  3:15       ` Wan ZongShun
2016-01-08 12:18       ` Joerg Roedel
2016-01-08 14:52         ` Wan Zongshun
2016-01-08 14:52           ` Wan Zongshun
2016-01-08 17:01           ` Joerg Roedel
2016-01-08 17:01             ` Joerg Roedel
2016-01-09  9:47             ` Wan Zongshun
2016-01-20 12:00               ` Joerg Roedel
2016-01-05 10:07 ` [PATCH 6/6] iommu/amd: Manage iommu_group " Wan Zongshun
2016-01-05 10:07   ` Wan Zongshun
2016-01-07 12:06   ` Joerg Roedel
2016-01-07 12:06     ` Joerg Roedel
2016-01-08  1:44     ` Wan ZongShun
2016-01-05 15:40 ` [PATCH 0/6] iommu/amd: enable ACPI hardware ID device support Suravee Suthikulpanit
2016-01-05 15:40   ` Suravee Suthikulpanit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.