2019-03-29 15:11:49

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH v2] iommu/amd: Reserve exclusion range in iova-domain

From: Joerg Roedel <[email protected]>

If a device has an exclusion range specified in the IVRS
table, this region needs to be reserved in the iova-domain
of that device. This hasn't happened until now and can cause
data corruption on data transfered with these devices.

Treat exclusion ranges as reserved regions in the iommu-core
to fix the problem.

Fixes: be2a022c0dd0 ('x86, AMD IOMMU: add functions to parse IOMMU memory mapping requirements for devices')
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/amd_iommu.c | 9 ++++++---
drivers/iommu/amd_iommu_init.c | 7 ++++---
drivers/iommu/amd_iommu_types.h | 2 ++
3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 21cb088d6687..f7cdd2ab7f11 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3169,21 +3169,24 @@ static void amd_iommu_get_resv_regions(struct device *dev,
return;

list_for_each_entry(entry, &amd_iommu_unity_map, list) {
+ int type, prot = 0;
size_t length;
- int prot = 0;

if (devid < entry->devid_start || devid > entry->devid_end)
continue;

+ type = IOMMU_RESV_DIRECT;
length = entry->address_end - entry->address_start;
if (entry->prot & IOMMU_PROT_IR)
prot |= IOMMU_READ;
if (entry->prot & IOMMU_PROT_IW)
prot |= IOMMU_WRITE;
+ if (entry->prot & IOMMU_UNITY_MAP_FLAG_EXCL_RANGE)
+ /* Exclusion range */
+ type = IOMMU_RESV_RESERVED;

region = iommu_alloc_resv_region(entry->address_start,
- length, prot,
- IOMMU_RESV_DIRECT);
+ length, prot, type);
if (!region) {
dev_err(dev, "Out of memory allocating dm-regions\n");
return;
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index f773792d77fd..1b1378619fc9 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2013,6 +2013,9 @@ static int __init init_unity_map_range(struct ivmd_header *m)
if (e == NULL)
return -ENOMEM;

+ if (m->flags & IVMD_FLAG_EXCL_RANGE)
+ init_exclusion_range(m);
+
switch (m->type) {
default:
kfree(e);
@@ -2059,9 +2062,7 @@ static int __init init_memory_definitions(struct acpi_table_header *table)

while (p < end) {
m = (struct ivmd_header *)p;
- if (m->flags & IVMD_FLAG_EXCL_RANGE)
- init_exclusion_range(m);
- else if (m->flags & IVMD_FLAG_UNITY_MAP)
+ if (m->flags & (IVMD_FLAG_UNITY_MAP | IVMD_FLAG_EXCL_RANGE))
init_unity_map_range(m);

p += m->length;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index eae0741f72dc..87965e4d9647 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -374,6 +374,8 @@
#define IOMMU_PROT_IR 0x01
#define IOMMU_PROT_IW 0x02

+#define IOMMU_UNITY_MAP_FLAG_EXCL_RANGE (1 << 2)
+
/* IOMMU capabilities */
#define IOMMU_CAP_IOTLB 24
#define IOMMU_CAP_NPCACHE 26
--
2.16.4



2019-03-29 16:01:42

by Gary R Hook

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/amd: Reserve exclusion range in iova-domain

On 3/29/19 10:10 AM, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> If a device has an exclusion range specified in the IVRS
> table, this region needs to be reserved in the iova-domain
> of that device. This hasn't happened until now and can cause
> data corruption on data transfered with these devices.
>
> Treat exclusion ranges as reserved regions in the iommu-core
> to fix the problem.

Reviewed-by: Gary R Hook <[email protected]>

>
> Fixes: be2a022c0dd0 ('x86, AMD IOMMU: add functions to parse IOMMU memory mapping requirements for devices')
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/amd_iommu.c | 9 ++++++---
> drivers/iommu/amd_iommu_init.c | 7 ++++---
> drivers/iommu/amd_iommu_types.h | 2 ++
> 3 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 21cb088d6687..f7cdd2ab7f11 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -3169,21 +3169,24 @@ static void amd_iommu_get_resv_regions(struct device *dev,
> return;
>
> list_for_each_entry(entry, &amd_iommu_unity_map, list) {
> + int type, prot = 0;
> size_t length;
> - int prot = 0;
>
> if (devid < entry->devid_start || devid > entry->devid_end)
> continue;
>
> + type = IOMMU_RESV_DIRECT;
> length = entry->address_end - entry->address_start;
> if (entry->prot & IOMMU_PROT_IR)
> prot |= IOMMU_READ;
> if (entry->prot & IOMMU_PROT_IW)
> prot |= IOMMU_WRITE;
> + if (entry->prot & IOMMU_UNITY_MAP_FLAG_EXCL_RANGE)
> + /* Exclusion range */
> + type = IOMMU_RESV_RESERVED;
>
> region = iommu_alloc_resv_region(entry->address_start,
> - length, prot,
> - IOMMU_RESV_DIRECT);
> + length, prot, type);
> if (!region) {
> dev_err(dev, "Out of memory allocating dm-regions\n");
> return;
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index f773792d77fd..1b1378619fc9 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -2013,6 +2013,9 @@ static int __init init_unity_map_range(struct ivmd_header *m)
> if (e == NULL)
> return -ENOMEM;
>
> + if (m->flags & IVMD_FLAG_EXCL_RANGE)
> + init_exclusion_range(m);
> +
> switch (m->type) {
> default:
> kfree(e);
> @@ -2059,9 +2062,7 @@ static int __init init_memory_definitions(struct acpi_table_header *table)
>
> while (p < end) {
> m = (struct ivmd_header *)p;
> - if (m->flags & IVMD_FLAG_EXCL_RANGE)
> - init_exclusion_range(m);
> - else if (m->flags & IVMD_FLAG_UNITY_MAP)
> + if (m->flags & (IVMD_FLAG_UNITY_MAP | IVMD_FLAG_EXCL_RANGE))
> init_unity_map_range(m);
>
> p += m->length;
> diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
> index eae0741f72dc..87965e4d9647 100644
> --- a/drivers/iommu/amd_iommu_types.h
> +++ b/drivers/iommu/amd_iommu_types.h
> @@ -374,6 +374,8 @@
> #define IOMMU_PROT_IR 0x01
> #define IOMMU_PROT_IW 0x02
>
> +#define IOMMU_UNITY_MAP_FLAG_EXCL_RANGE (1 << 2)
> +
> /* IOMMU capabilities */
> #define IOMMU_CAP_IOTLB 24
> #define IOMMU_CAP_NPCACHE 26
>

2019-03-29 18:19:58

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/amd: Reserve exclusion range in iova-domain

On Fri Mar 29 19, Joerg Roedel wrote:
>From: Joerg Roedel <[email protected]>
>
>If a device has an exclusion range specified in the IVRS
>table, this region needs to be reserved in the iova-domain
>of that device. This hasn't happened until now and can cause
>data corruption on data transfered with these devices.
>
>Treat exclusion ranges as reserved regions in the iommu-core
>to fix the problem.
>
>Fixes: be2a022c0dd0 ('x86, AMD IOMMU: add functions to parse IOMMU memory mapping requirements for devices')
>Signed-off-by: Joerg Roedel <[email protected]>
>---
> drivers/iommu/amd_iommu.c | 9 ++++++---
> drivers/iommu/amd_iommu_init.c | 7 ++++---
> drivers/iommu/amd_iommu_types.h | 2 ++
> 3 files changed, 12 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
>index 21cb088d6687..f7cdd2ab7f11 100644
>--- a/drivers/iommu/amd_iommu.c
>+++ b/drivers/iommu/amd_iommu.c
>@@ -3169,21 +3169,24 @@ static void amd_iommu_get_resv_regions(struct device *dev,
> return;
>
> list_for_each_entry(entry, &amd_iommu_unity_map, list) {
>+ int type, prot = 0;
> size_t length;
>- int prot = 0;
>
> if (devid < entry->devid_start || devid > entry->devid_end)
> continue;
>
>+ type = IOMMU_RESV_DIRECT;
> length = entry->address_end - entry->address_start;
> if (entry->prot & IOMMU_PROT_IR)
> prot |= IOMMU_READ;
> if (entry->prot & IOMMU_PROT_IW)
> prot |= IOMMU_WRITE;
>+ if (entry->prot & IOMMU_UNITY_MAP_FLAG_EXCL_RANGE)
>+ /* Exclusion range */
>+ type = IOMMU_RESV_RESERVED;
>
> region = iommu_alloc_resv_region(entry->address_start,
>- length, prot,
>- IOMMU_RESV_DIRECT);
>+ length, prot, type);
> if (!region) {
> dev_err(dev, "Out of memory allocating dm-regions\n");
> return;
>diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
>index f773792d77fd..1b1378619fc9 100644
>--- a/drivers/iommu/amd_iommu_init.c
>+++ b/drivers/iommu/amd_iommu_init.c
>@@ -2013,6 +2013,9 @@ static int __init init_unity_map_range(struct ivmd_header *m)
> if (e == NULL)
> return -ENOMEM;
>
>+ if (m->flags & IVMD_FLAG_EXCL_RANGE)
>+ init_exclusion_range(m);
>+
> switch (m->type) {
> default:
> kfree(e);
>@@ -2059,9 +2062,7 @@ static int __init init_memory_definitions(struct acpi_table_header *table)
>
> while (p < end) {
> m = (struct ivmd_header *)p;
>- if (m->flags & IVMD_FLAG_EXCL_RANGE)
>- init_exclusion_range(m);
>- else if (m->flags & IVMD_FLAG_UNITY_MAP)
>+ if (m->flags & (IVMD_FLAG_UNITY_MAP | IVMD_FLAG_EXCL_RANGE))
> init_unity_map_range(m);
>
> p += m->length;
>diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
>index eae0741f72dc..87965e4d9647 100644
>--- a/drivers/iommu/amd_iommu_types.h
>+++ b/drivers/iommu/amd_iommu_types.h
>@@ -374,6 +374,8 @@
> #define IOMMU_PROT_IR 0x01
> #define IOMMU_PROT_IW 0x02
>
>+#define IOMMU_UNITY_MAP_FLAG_EXCL_RANGE (1 << 2)
>+
> /* IOMMU capabilities */
> #define IOMMU_CAP_IOTLB 24
> #define IOMMU_CAP_NPCACHE 26
>--
>2.16.4
>

Reviewed-by: Jerry Snitselaar <[email protected]>

2019-04-02 20:21:31

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH v2] iommu/amd: Reserve exclusion range in iova-domain

On Fri Mar 29 19, Joerg Roedel wrote:
>From: Joerg Roedel <[email protected]>
>
>If a device has an exclusion range specified in the IVRS
>table, this region needs to be reserved in the iova-domain
>of that device. This hasn't happened until now and can cause
>data corruption on data transfered with these devices.
>
>Treat exclusion ranges as reserved regions in the iommu-core
>to fix the problem.
>
>Fixes: be2a022c0dd0 ('x86, AMD IOMMU: add functions to parse IOMMU memory mapping requirements for devices')
>Signed-off-by: Joerg Roedel <[email protected]>

I have a version of this that applies to 4.4 and 4,9 using the older
dm_region code if that would be useful for stable.


--8<--

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 0ad8b7c78a43..f388458624cf 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3165,11 +3165,14 @@ static void amd_iommu_get_dm_regions(struct device *dev,
}

region->start = entry->address_start;
+ region->type = IOMMU_RESV_DIRECT;
region->length = entry->address_end - entry->address_start;
if (entry->prot & IOMMU_PROT_IR)
region->prot |= IOMMU_READ;
if (entry->prot & IOMMU_PROT_IW)
region->prot |= IOMMU_WRITE;
+ if (entry->prot & IOMMU_UNITY_MAP_FLAG_EXCL_RANGE)
+ region->type = IOMMU_RESV_RESERVED;

list_add_tail(&region->list, head);
}
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 94f1bf772ec9..d84041bc77ac 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1495,6 +1495,9 @@ static int __init init_unity_map_range(struct ivmd_header *m)
if (e == NULL)
return -ENOMEM;

+ if (m->flags & IVMD_FLAG_EXCL_RANGE)
+ init_exclusion_range(m);
+
switch (m->type) {
default:
kfree(e);
@@ -1541,9 +1544,7 @@ static int __init init_memory_definitions(struct acpi_table_header *table)

while (p < end) {
m = (struct ivmd_header *)p;
- if (m->flags & IVMD_FLAG_EXCL_RANGE)
- init_exclusion_range(m);
- else if (m->flags & IVMD_FLAG_UNITY_MAP)
+ if (m->flags & (IVMD_FLAG_UNITY_MAP | IVMD_FLAG_EXCL_RANGE))
init_unity_map_range(m);

p += m->length;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index b08cf57bf455..31d27eb70565 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -324,6 +324,8 @@
#define IOMMU_PROT_IR 0x01
#define IOMMU_PROT_IW 0x02

+#define IOMMU_UNITY_MAP_FLAG_EXCL_RANGE (1 << 2)
+
/* IOMMU capabilities */
#define IOMMU_CAP_IOTLB 24
#define IOMMU_CAP_NPCACHE 26
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a070fa39521a..ef4aa2879952 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -351,6 +351,9 @@ static int iommu_group_create_direct_mappings(struct iommu_group *group,
start = ALIGN(entry->start, pg_size);
end = ALIGN(entry->start + entry->length, pg_size);

+ if (entry->type != IOMMU_RESV_DIRECT)
+ continue;
+
for (addr = start; addr < end; addr += pg_size) {
phys_addr_t phys_addr;

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f28dff313b07..15b7378f67f3 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -115,18 +115,23 @@ enum iommu_attr {
DOMAIN_ATTR_MAX,
};

+#define IOMMU_RESV_DIRECT (1 << 0)
+#define IOMMU_RESV_RESERVED (1 << 1)
+
/**
* struct iommu_dm_region - descriptor for a direct mapped memory region
* @list: Linked list pointers
* @start: System physical start address of the region
* @length: Length of the region in bytes
* @prot: IOMMU Protection flags (READ/WRITE/...)
+ * @type: Type of region (DIRECT, RESERVED)
*/
struct iommu_dm_region {
struct list_head list;
phys_addr_t start;
size_t length;
int prot;
+ int type;
};

#ifdef CONFIG_IOMMU_API
--
2.21.0