2011-04-08 08:18:36

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 0/5] ATS support for AMD IOMMU driver

Hi,

this patch-set implements support for ATS devices in the AMD IOMMU
driver. For that it uses the ATS capability implementation already
present in the core PCI code.

Jesse, patch 1/5 touches generic PCI code, can you please have a look at
it an ACK it if you think it is ok? The change basically move out the
declarations relevant for ATS into a header file under include/linux.
This header file will also be used later to put declarations for PRI
support there (which is also part of the ATS spec).

Any feedback appreciated.

Regards,

Joerg

Diffstat:

arch/x86/Kconfig | 1 +
arch/x86/include/asm/amd_iommu_types.h | 7 ++-
arch/x86/kernel/amd_iommu.c | 103 +++++++++++++++++++++++++++++--
arch/x86/kernel/amd_iommu_init.c | 4 +
drivers/pci/intel-iommu.c | 1 +
drivers/pci/iov.c | 1 +
drivers/pci/pci.h | 37 -----------
include/linux/pci-ats.h | 52 ++++++++++++++++
8 files changed, 161 insertions(+), 45 deletions(-)

Shortlog:

Joerg Roedel (5):
PCI: Move ATS declarations in seperate header file
x86/amd-iommu: Select PCI_IOV with AMD IOMMU driver
x86/amd-iommu: Flush device IOTLB if ATS is enabled
x86/amd-iommu: Add flag to indicate IOTLB support
x86/amd-iommu: Add ATS enable/disable code


2011-04-08 08:17:24

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 4/5] x86/amd-iommu: Add flag to indicate IOTLB support

This patch adds a flag to the AMD IOMMU driver to indicate
that all IOMMUs present in the system support device IOTLBs.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/amd_iommu_types.h | 2 ++
arch/x86/kernel/amd_iommu_init.c | 4 ++++
2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/amd_iommu_types.h b/arch/x86/include/asm/amd_iommu_types.h
index f5d184e..cb811c9 100644
--- a/arch/x86/include/asm/amd_iommu_types.h
+++ b/arch/x86/include/asm/amd_iommu_types.h
@@ -250,6 +250,8 @@ extern bool amd_iommu_dump;

/* global flag if IOMMUs cache non-present entries */
extern bool amd_iommu_np_cache;
+/* Only true if all IOMMUs support device IOTLBs */
+extern bool amd_iommu_iotlb_sup;

/*
* Make iterating over all IOMMUs easier
diff --git a/arch/x86/kernel/amd_iommu_init.c b/arch/x86/kernel/amd_iommu_init.c
index 8848dda..b6c634f 100644
--- a/arch/x86/kernel/amd_iommu_init.c
+++ b/arch/x86/kernel/amd_iommu_init.c
@@ -137,6 +137,7 @@ int amd_iommus_present;

/* IOMMUs have a non-present cache? */
bool amd_iommu_np_cache __read_mostly;
+bool amd_iommu_iotlb_sup __read_mostly = true;

/*
* The ACPI table parsing functions set this variable on an error
@@ -673,6 +674,9 @@ static void __init init_iommu_from_pci(struct amd_iommu *iommu)
MMIO_GET_LD(range));
iommu->evt_msi_num = MMIO_MSI_NUM(misc);

+ if (!(iommu->cap & (1 << IOMMU_CAP_IOTLB)))
+ amd_iommu_iotlb_sup = false;
+
if (!is_rd890_iommu(iommu->dev))
return;

--
1.7.1

2011-04-08 08:17:42

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 5/5] x86/amd-iommu: Add ATS enable/disable code

This patch adds the necessary code to the AMD IOMMU driver
for enabling and disabling the ATS capability on a device
and to setup the IOMMU data structures correctly.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/amd_iommu_types.h | 2 ++
arch/x86/kernel/amd_iommu.c | 29 +++++++++++++++++++++++------
2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/amd_iommu_types.h b/arch/x86/include/asm/amd_iommu_types.h
index cb811c9..7434377 100644
--- a/arch/x86/include/asm/amd_iommu_types.h
+++ b/arch/x86/include/asm/amd_iommu_types.h
@@ -216,6 +216,8 @@
#define IOMMU_PTE_IR (1ULL << 61)
#define IOMMU_PTE_IW (1ULL << 62)

+#define DTE_FLAG_IOTLB 0x01
+
#define IOMMU_PAGE_MASK (((1ULL << 52) - 1) & ~0xfffULL)
#define IOMMU_PTE_PRESENT(pte) ((pte) & IOMMU_PTE_P)
#define IOMMU_PTE_PAGE(pte) (phys_to_virt((pte) & IOMMU_PAGE_MASK))
diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index f3ce433..ad97fd6 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -1452,17 +1452,22 @@ static bool dma_ops_domain(struct protection_domain *domain)
return domain->flags & PD_DMA_OPS_MASK;
}

-static void set_dte_entry(u16 devid, struct protection_domain *domain)
+static void set_dte_entry(u16 devid, struct protection_domain *domain, bool ats)
{
u64 pte_root = virt_to_phys(domain->pt_root);
+ u32 flags = 0;

pte_root |= (domain->mode & DEV_ENTRY_MODE_MASK)
<< DEV_ENTRY_MODE_SHIFT;
pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_P | IOMMU_PTE_TV;

- amd_iommu_dev_table[devid].data[2] = domain->id;
- amd_iommu_dev_table[devid].data[1] = upper_32_bits(pte_root);
- amd_iommu_dev_table[devid].data[0] = lower_32_bits(pte_root);
+ if (ats)
+ flags |= DTE_FLAG_IOTLB;
+
+ amd_iommu_dev_table[devid].data[3] |= flags;
+ amd_iommu_dev_table[devid].data[2] = domain->id;
+ amd_iommu_dev_table[devid].data[1] = upper_32_bits(pte_root);
+ amd_iommu_dev_table[devid].data[0] = lower_32_bits(pte_root);
}

static void clear_dte_entry(u16 devid)
@@ -1479,16 +1484,22 @@ static void do_attach(struct device *dev, struct protection_domain *domain)
{
struct iommu_dev_data *dev_data;
struct amd_iommu *iommu;
+ struct pci_dev *pdev;
+ bool ats = false;
u16 devid;

devid = get_device_id(dev);
iommu = amd_iommu_rlookup_table[devid];
dev_data = get_dev_data(dev);
+ pdev = to_pci_dev(dev);
+
+ if (amd_iommu_iotlb_sup)
+ ats = pci_enable_ats(pdev, PAGE_SHIFT) == 0;

/* Update data structures */
dev_data->domain = domain;
list_add(&dev_data->list, &domain->dev_list);
- set_dte_entry(devid, domain);
+ set_dte_entry(devid, domain, ats);

/* Do reference counting */
domain->dev_iommu[iommu->index] += 1;
@@ -1502,11 +1513,13 @@ static void do_detach(struct device *dev)
{
struct iommu_dev_data *dev_data;
struct amd_iommu *iommu;
+ struct pci_dev *pdev;
u16 devid;

devid = get_device_id(dev);
iommu = amd_iommu_rlookup_table[devid];
dev_data = get_dev_data(dev);
+ pdev = to_pci_dev(dev);

/* decrease reference counters */
dev_data->domain->dev_iommu[iommu->index] -= 1;
@@ -1517,6 +1530,9 @@ static void do_detach(struct device *dev)
list_del(&dev_data->list);
clear_dte_entry(devid);

+ if (amd_iommu_iotlb_sup && pci_ats_enabled(pdev))
+ pci_disable_ats(pdev);
+
/* Flush the DTE entry */
device_flush_dte(dev);
}
@@ -1795,8 +1811,9 @@ static void update_device_table(struct protection_domain *domain)
struct iommu_dev_data *dev_data;

list_for_each_entry(dev_data, &domain->dev_list, list) {
+ struct pci_dev *pdev = to_pci_dev(dev_data->dev);
u16 devid = get_device_id(dev_data->dev);
- set_dte_entry(devid, domain);
+ set_dte_entry(devid, domain, pci_ats_enabled(pdev));
}
}

--
1.7.1

2011-04-08 08:17:23

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 2/5] x86/amd-iommu: Select PCI_IOV with AMD IOMMU driver

In order to support ATS in the AMD IOMMU driver this patch
makes sure that the generic support for ATS is compiled in.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/Kconfig | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc6c53a..8cc29da 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -690,6 +690,7 @@ config AMD_IOMMU
bool "AMD IOMMU support"
select SWIOTLB
select PCI_MSI
+ select PCI_IOV
depends on X86_64 && PCI && ACPI
---help---
With this option you can enable support for AMD IOMMU hardware in
--
1.7.1

2011-04-08 08:18:07

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 3/5] x86/amd-iommu: Flush device IOTLB if ATS is enabled

This patch implements a function to flush the IOTLB on
devices supporting ATS and makes sure that this TLB is also
flushed if necessary.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/amd_iommu_types.h | 3 +-
arch/x86/kernel/amd_iommu.c | 74 +++++++++++++++++++++++++++++++-
2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/amd_iommu_types.h b/arch/x86/include/asm/amd_iommu_types.h
index 878ae00..f5d184e 100644
--- a/arch/x86/include/asm/amd_iommu_types.h
+++ b/arch/x86/include/asm/amd_iommu_types.h
@@ -113,7 +113,8 @@
/* command specific defines */
#define CMD_COMPL_WAIT 0x01
#define CMD_INV_DEV_ENTRY 0x02
-#define CMD_INV_IOMMU_PAGES 0x03
+#define CMD_INV_IOMMU_PAGES 0x03
+#define CMD_INV_IOTLB_PAGES 0x04

#define CMD_COMPL_WAIT_STORE_MASK 0x01
#define CMD_COMPL_WAIT_INT_MASK 0x02
diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index bcf58ea..f3ce433 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -18,6 +18,7 @@
*/

#include <linux/pci.h>
+#include <linux/pci-ats.h>
#include <linux/bitmap.h>
#include <linux/slab.h>
#include <linux/debugfs.h>
@@ -463,6 +464,37 @@ static void build_inv_iommu_pages(struct iommu_cmd *cmd, u64 address,
cmd->data[2] |= CMD_INV_IOMMU_PAGES_PDE_MASK;
}

+static void build_inv_iotlb_pages(struct iommu_cmd *cmd, u16 devid, int qdep,
+ u64 address, size_t size)
+{
+ u64 pages;
+ int s;
+
+ pages = iommu_num_pages(address, size, PAGE_SIZE);
+ s = 0;
+
+ if (pages > 1) {
+ /*
+ * If we have to flush more than one page, flush all
+ * TLB entries for this domain
+ */
+ address = CMD_INV_IOMMU_ALL_PAGES_ADDRESS;
+ s = 1;
+ }
+
+ address &= PAGE_MASK;
+
+ memset(cmd, 0, sizeof(*cmd));
+ cmd->data[0] = devid;
+ cmd->data[0] |= (qdep & 0xff) << 24;
+ cmd->data[1] = devid;
+ cmd->data[2] = lower_32_bits(address);
+ cmd->data[3] = upper_32_bits(address);
+ CMD_SET_TYPE(cmd, CMD_INV_IOTLB_PAGES);
+ if (s)
+ cmd->data[2] |= CMD_INV_IOMMU_PAGES_SIZE_MASK;
+}
+
/*
* Writes the command to the IOMMUs command buffer and informs the
* hardware about the new command.
@@ -574,17 +606,47 @@ void iommu_flush_all_caches(struct amd_iommu *iommu)
}

/*
+ * Command send function for flushing on-device TLB
+ */
+static int device_flush_iotlb(struct device *dev, u64 address, size_t size)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct amd_iommu *iommu;
+ struct iommu_cmd cmd;
+ u16 devid;
+ int qdep;
+
+ qdep = pci_ats_queue_depth(pdev);
+ devid = get_device_id(dev);
+ iommu = amd_iommu_rlookup_table[devid];
+
+ build_inv_iotlb_pages(&cmd, devid, qdep, address, size);
+
+ return iommu_queue_command(iommu, &cmd);
+}
+
+/*
* Command send function for invalidating a device table entry
*/
static int device_flush_dte(struct device *dev)
{
struct amd_iommu *iommu;
+ struct pci_dev *pdev;
u16 devid;
+ int ret;

+ pdev = to_pci_dev(dev);
devid = get_device_id(dev);
iommu = amd_iommu_rlookup_table[devid];

- return iommu_flush_dte(iommu, devid);
+ ret = iommu_flush_dte(iommu, devid);
+ if (ret)
+ return ret;
+
+ if (pci_ats_enabled(pdev))
+ ret = device_flush_iotlb(dev, 0, ~0UL);
+
+ return ret;
}

/*
@@ -595,6 +657,7 @@ static int device_flush_dte(struct device *dev)
static void __domain_flush_pages(struct protection_domain *domain,
u64 address, size_t size, int pde)
{
+ struct iommu_dev_data *dev_data;
struct iommu_cmd cmd;
int ret = 0, i;

@@ -611,6 +674,15 @@ static void __domain_flush_pages(struct protection_domain *domain,
ret |= iommu_queue_command(amd_iommus[i], &cmd);
}

+ list_for_each_entry(dev_data, &domain->dev_list, list) {
+ struct pci_dev *pdev = to_pci_dev(dev_data->dev);
+
+ if (!pci_ats_enabled(pdev))
+ continue;
+
+ ret |= device_flush_iotlb(dev_data->dev, address, size);
+ }
+
WARN_ON(ret);
}

--
1.7.1

2011-04-08 08:18:35

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 1/5] PCI: Move ATS declarations in seperate header file

This patch moves the relevant declarations from the local
header file in drivers/pci to a more accessible locations so
that it can be used by the AMD IOMMU driver too.
The file is named pci-ats-pri.h because support for the PCI
PRI capability will also be added there in a later
patch-set.

Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/pci/intel-iommu.c | 1 +
drivers/pci/iov.c | 1 +
drivers/pci/pci.h | 37 --------------------------------
include/linux/pci-ats.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 54 insertions(+), 37 deletions(-)
create mode 100644 include/linux/pci-ats.h

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 7da3bef..fdb2cef 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -39,6 +39,7 @@
#include <linux/syscore_ops.h>
#include <linux/tboot.h>
#include <linux/dmi.h>
+#include <linux/pci-ats.h>
#include <asm/cacheflush.h>
#include <asm/iommu.h>
#include "pci.h"
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 553d8ee..42fae47 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -13,6 +13,7 @@
#include <linux/mutex.h>
#include <linux/string.h>
#include <linux/delay.h>
+#include <linux/pci-ats.h>
#include "pci.h"

#define VIRTFN_ID_LEN 16
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a6ec200..4020025 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -250,15 +250,6 @@ struct pci_sriov {
u8 __iomem *mstate; /* VF Migration State Array */
};

-/* Address Translation Service */
-struct pci_ats {
- int pos; /* capability position */
- int stu; /* Smallest Translation Unit */
- int qdep; /* Invalidate Queue Depth */
- int ref_cnt; /* Physical Function reference count */
- unsigned int is_enabled:1; /* Enable bit is set */
-};
-
#ifdef CONFIG_PCI_IOV
extern int pci_iov_init(struct pci_dev *dev);
extern void pci_iov_release(struct pci_dev *dev);
@@ -269,19 +260,6 @@ extern resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev,
extern void pci_restore_iov_state(struct pci_dev *dev);
extern int pci_iov_bus_range(struct pci_bus *bus);

-extern int pci_enable_ats(struct pci_dev *dev, int ps);
-extern void pci_disable_ats(struct pci_dev *dev);
-extern int pci_ats_queue_depth(struct pci_dev *dev);
-/**
- * pci_ats_enabled - query the ATS status
- * @dev: the PCI device
- *
- * Returns 1 if ATS capability is enabled, or 0 if not.
- */
-static inline int pci_ats_enabled(struct pci_dev *dev)
-{
- return dev->ats && dev->ats->is_enabled;
-}
#else
static inline int pci_iov_init(struct pci_dev *dev)
{
@@ -304,21 +282,6 @@ static inline int pci_iov_bus_range(struct pci_bus *bus)
return 0;
}

-static inline int pci_enable_ats(struct pci_dev *dev, int ps)
-{
- return -ENODEV;
-}
-static inline void pci_disable_ats(struct pci_dev *dev)
-{
-}
-static inline int pci_ats_queue_depth(struct pci_dev *dev)
-{
- return -ENODEV;
-}
-static inline int pci_ats_enabled(struct pci_dev *dev)
-{
- return 0;
-}
#endif /* CONFIG_PCI_IOV */

static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
new file mode 100644
index 0000000..655824f
--- /dev/null
+++ b/include/linux/pci-ats.h
@@ -0,0 +1,52 @@
+#ifndef LINUX_PCI_ATS_H
+#define LINUX_PCI_ATS_H
+
+/* Address Translation Service */
+struct pci_ats {
+ int pos; /* capability position */
+ int stu; /* Smallest Translation Unit */
+ int qdep; /* Invalidate Queue Depth */
+ int ref_cnt; /* Physical Function reference count */
+ unsigned int is_enabled:1; /* Enable bit is set */
+};
+
+#ifdef CONFIG_PCI_IOV
+
+extern int pci_enable_ats(struct pci_dev *dev, int ps);
+extern void pci_disable_ats(struct pci_dev *dev);
+extern int pci_ats_queue_depth(struct pci_dev *dev);
+/**
+ * pci_ats_enabled - query the ATS status
+ * @dev: the PCI device
+ *
+ * Returns 1 if ATS capability is enabled, or 0 if not.
+ */
+static inline int pci_ats_enabled(struct pci_dev *dev)
+{
+ return dev->ats && dev->ats->is_enabled;
+}
+
+#else /* CONFIG_PCI_IOV */
+
+static inline int pci_enable_ats(struct pci_dev *dev, int ps)
+{
+ return -ENODEV;
+}
+
+static inline void pci_disable_ats(struct pci_dev *dev)
+{
+}
+
+static inline int pci_ats_queue_depth(struct pci_dev *dev)
+{
+ return -ENODEV;
+}
+
+static inline int pci_ats_enabled(struct pci_dev *dev)
+{
+ return 0;
+}
+
+#endif /* CONFIG_PCI_IOV */
+
+#endif /* LINUX_PCI_ATS_H*/
--
1.7.1

2011-04-08 15:26:12

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH 1/5] PCI: Move ATS declarations in seperate header file

On Fri, 8 Apr 2011 10:17:06 +0200
Joerg Roedel <[email protected]> wrote:

> This patch moves the relevant declarations from the local
> header file in drivers/pci to a more accessible locations so
> that it can be used by the AMD IOMMU driver too.
> The file is named pci-ats-pri.h because support for the PCI
> PRI capability will also be added there in a later
> patch-set.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/pci/intel-iommu.c | 1 +
> drivers/pci/iov.c | 1 +
> drivers/pci/pci.h | 37 --------------------------------
> include/linux/pci-ats.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 54 insertions(+), 37 deletions(-)
> create mode 100644 include/linux/pci-ats.h

Looks fine.

Acked-by: Jesse Barnes <[email protected]>

--
Jesse Barnes, Intel Open Source Technology Center

2011-04-08 18:33:09

by Rolf Eike Beer

[permalink] [raw]
Subject: Re: [PATCH 1/5] PCI: Move ATS declarations in seperate header file

Am Freitag, 8. April 2011, 10:17:06 schrieb Joerg Roedel:
> This patch moves the relevant declarations from the local
> header file in drivers/pci to a more accessible locations so
> that it can be used by the AMD IOMMU driver too.
> The file is named pci-ats-pri.h because support for the PCI
^^^^^^^^^^^^^

> include/linux/pci-ats.h | 52
^^^^^^^^^

Ehm, no, it is not ;)

Eike


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2011-04-11 06:20:51

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 1/5] PCI: Move ATS declarations in seperate header file

On Fri, Apr 08, 2011 at 02:33:04PM -0400, Rolf Eike Beer wrote:
> Am Freitag, 8. April 2011, 10:17:06 schrieb Joerg Roedel:
> > This patch moves the relevant declarations from the local
> > header file in drivers/pci to a more accessible locations so
> > that it can be used by the AMD IOMMU driver too.
> > The file is named pci-ats-pri.h because support for the PCI
> ^^^^^^^^^^^^^
>
> > include/linux/pci-ats.h | 52
> ^^^^^^^^^
>
> Ehm, no, it is not ;)

Right, I renamed the file at some point and forgot to change it in the
commit-msg too. I'll fix that on the next rebase :)

Joerg

--
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632