2008-11-20 14:22:45

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH] intel-iommu: make init_dmars() static

init_dmars() is not used outside of drivers/pci/intel-iommu.c

Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 2 +-
include/linux/dma_remapping.h | 1 -
2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 271f869..5721196 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -1589,7 +1589,7 @@ static inline void iommu_prepare_isa(void)
}
#endif /* !CONFIG_DMAR_FLPY_WA */

-int __init init_dmars(void)
+static int __init init_dmars(void)
{
struct dmar_drhd_unit *drhd;
struct dmar_rmrr_unit *rmrr;
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 952df39..cf92c49 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -141,7 +141,6 @@ struct device_domain_info {
struct dmar_domain *domain; /* pointer to domain */
};

-extern int init_dmars(void);
extern void free_dmar_iommu(struct intel_iommu *iommu);

extern int dmar_disabled;
--
1.6.0.3


2008-11-20 14:26:34

by Mark McLoughlin

[permalink] [raw]
Subject: Re: [PATCH] intel-iommu: make init_dmars() static

On Thu, 2008-11-20 at 14:21 +0000, Mark McLoughlin wrote:
> init_dmars() is not used outside of drivers/pci/intel-iommu.c

Note, most of linux/dma_remapping.h is only needed by intel-iommu.c

Any objections into moving all of that into intel-iommu.c?

Cheers,
Mark.

2008-11-20 14:26:54

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] intel-iommu: make init_dmars() static

On Thu, 2008-11-20 at 14:21 +0000, Mark McLoughlin wrote:
> init_dmars() is not used outside of drivers/pci/intel-iommu.c
>
> Signed-off-by: Mark McLoughlin <[email protected]>

Applied; thank you.

--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation

2008-11-20 14:37:54

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] intel-iommu: make init_dmars() static

On Thu, 2008-11-20 at 14:24 +0000, Mark McLoughlin wrote:
> On Thu, 2008-11-20 at 14:21 +0000, Mark McLoughlin wrote:
> > init_dmars() is not used outside of drivers/pci/intel-iommu.c
>
> Note, most of linux/dma_remapping.h is only needed by intel-iommu.c

Most.

> Any objections into moving all of that into intel-iommu.c?

That would seem to make sense. Don't forget to check IA64 when declaring
stuff unused though :)

--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation

2008-11-20 15:51:18

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 2/8] intel-iommu: move root entry defs from dma_remapping.h

We keep the struct root_entry forward declaration for the
pointer in struct intel_iommu.

Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 33 +++++++++++++++++++++++++++++++++
include/linux/dma_remapping.h | 34 +---------------------------------
2 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index a9032e6..2e56e5a 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -57,6 +57,39 @@
#define DMA_32BIT_PFN IOVA_PFN(DMA_32BIT_MASK)
#define DMA_64BIT_PFN IOVA_PFN(DMA_64BIT_MASK)

+/*
+ * 0: Present
+ * 1-11: Reserved
+ * 12-63: Context Ptr (12 - (haw-1))
+ * 64-127: Reserved
+ */
+struct root_entry {
+ u64 val;
+ u64 rsvd1;
+};
+#define ROOT_ENTRY_NR (VTD_PAGE_SIZE/sizeof(struct root_entry))
+static inline bool root_present(struct root_entry *root)
+{
+ return (root->val & 1);
+}
+static inline void set_root_present(struct root_entry *root)
+{
+ root->val |= 1;
+}
+static inline void set_root_value(struct root_entry *root, unsigned long value)
+{
+ root->val |= value & VTD_PAGE_MASK;
+}
+
+static inline struct context_entry *
+get_context_addr_from_root(struct root_entry *root)
+{
+ return (struct context_entry *)
+ (root_present(root)?phys_to_virt(
+ root->val & VTD_PAGE_MASK) :
+ NULL);
+}
+
static void flush_unmaps_timeout(unsigned long data);

DEFINE_TIMER(unmap_timer, flush_unmaps_timeout, 0, 0);
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 2e5a5c0..d852166 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -9,39 +9,7 @@
#define VTD_PAGE_MASK (((u64)-1) << VTD_PAGE_SHIFT)
#define VTD_PAGE_ALIGN(addr) (((addr) + VTD_PAGE_SIZE - 1) & VTD_PAGE_MASK)

-/*
- * 0: Present
- * 1-11: Reserved
- * 12-63: Context Ptr (12 - (haw-1))
- * 64-127: Reserved
- */
-struct root_entry {
- u64 val;
- u64 rsvd1;
-};
-#define ROOT_ENTRY_NR (VTD_PAGE_SIZE/sizeof(struct root_entry))
-static inline bool root_present(struct root_entry *root)
-{
- return (root->val & 1);
-}
-static inline void set_root_present(struct root_entry *root)
-{
- root->val |= 1;
-}
-static inline void set_root_value(struct root_entry *root, unsigned long value)
-{
- root->val |= value & VTD_PAGE_MASK;
-}
-
-struct context_entry;
-static inline struct context_entry *
-get_context_addr_from_root(struct root_entry *root)
-{
- return (struct context_entry *)
- (root_present(root)?phys_to_virt(
- root->val & VTD_PAGE_MASK) :
- NULL);
-}
+struct root_entry;

/*
* low 64 bits:
--
1.5.4.3

2008-11-20 15:51:33

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 5/8] intel-iommu: move struct dmar_domain def out dma_remapping.h

Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 18 ++++++++++++++++++
include/linux/dma_remapping.h | 22 ++--------------------
2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 0f558c7..14066b1 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -150,6 +150,24 @@ struct dma_pte {
(p).val |= ((addr) & VTD_PAGE_MASK); } while (0)
#define dma_pte_present(p) (((p).val & 3) != 0)

+struct dmar_domain {
+ int id; /* domain id */
+ struct intel_iommu *iommu; /* back pointer to owning iommu */
+
+ struct list_head devices; /* all devices' list */
+ struct iova_domain iovad; /* iova's that belong to this domain */
+
+ struct dma_pte *pgd; /* virtual address */
+ spinlock_t mapping_lock; /* page table lock */
+ int gaw; /* max guest address width */
+
+ /* adjusted guest address width, 0 is level 2 30-bit */
+ int agaw;
+
+#define DOMAIN_FLAG_MULTIPLE_DEVICES 1
+ int flags;
+};
+
static void flush_unmaps_timeout(unsigned long data);

DEFINE_TIMER(unmap_timer, flush_unmaps_timeout, 0, 0);
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 9d5874e..3330144 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -9,30 +9,12 @@
#define VTD_PAGE_MASK (((u64)-1) << VTD_PAGE_SHIFT)
#define VTD_PAGE_ALIGN(addr) (((addr) + VTD_PAGE_SIZE - 1) & VTD_PAGE_MASK)

-struct root_entry;
-
#define DMA_PTE_READ (1)
#define DMA_PTE_WRITE (2)

struct intel_iommu;
-
-struct dmar_domain {
- int id; /* domain id */
- struct intel_iommu *iommu; /* back pointer to owning iommu */
-
- struct list_head devices; /* all devices' list */
- struct iova_domain iovad; /* iova's that belong to this domain */
-
- struct dma_pte *pgd; /* virtual address */
- spinlock_t mapping_lock; /* page table lock */
- int gaw; /* max guest address width */
-
- /* adjusted guest address width, 0 is level 2 30-bit */
- int agaw;
-
-#define DOMAIN_FLAG_MULTIPLE_DEVICES 1
- int flags;
-};
+struct dmar_domain;
+struct root_entry;

/* PCI domain-device relationship */
struct device_domain_info {
--
1.5.4.3

2008-11-20 15:51:50

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 4/8] intel-iommu: move DMA PTE defs out of dma_remapping.h

DMA_PTE_READ/WRITE are needed by kvm.

Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 22 ++++++++++++++++++++++
include/linux/dma_remapping.h | 22 ----------------------
2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 1c011bb..0f558c7 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -128,6 +128,28 @@ struct context_entry {
do {(c).hi |= ((val) & ((1 << 16) - 1)) << 8;} while (0)
#define context_clear_entry(c) do {(c).lo = 0; (c).hi = 0;} while (0)

+/*
+ * 0: readable
+ * 1: writable
+ * 2-6: reserved
+ * 7: super page
+ * 8-11: available
+ * 12-63: Host physcial address
+ */
+struct dma_pte {
+ u64 val;
+};
+#define dma_clear_pte(p) do {(p).val = 0;} while (0)
+
+#define dma_set_pte_readable(p) do {(p).val |= DMA_PTE_READ;} while (0)
+#define dma_set_pte_writable(p) do {(p).val |= DMA_PTE_WRITE;} while (0)
+#define dma_set_pte_prot(p, prot) \
+ do {(p).val = ((p).val & ~3) | ((prot) & 3); } while (0)
+#define dma_pte_addr(p) ((p).val & VTD_PAGE_MASK)
+#define dma_set_pte_addr(p, addr) do {\
+ (p).val |= ((addr) & VTD_PAGE_MASK); } while (0)
+#define dma_pte_present(p) (((p).val & 3) != 0)
+
static void flush_unmaps_timeout(unsigned long data);

DEFINE_TIMER(unmap_timer, flush_unmaps_timeout, 0, 0);
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 9a88f7d..9d5874e 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -11,31 +11,9 @@

struct root_entry;

-/*
- * 0: readable
- * 1: writable
- * 2-6: reserved
- * 7: super page
- * 8-11: available
- * 12-63: Host physcial address
- */
-struct dma_pte {
- u64 val;
-};
-#define dma_clear_pte(p) do {(p).val = 0;} while (0)
-
#define DMA_PTE_READ (1)
#define DMA_PTE_WRITE (2)

-#define dma_set_pte_readable(p) do {(p).val |= DMA_PTE_READ;} while (0)
-#define dma_set_pte_writable(p) do {(p).val |= DMA_PTE_WRITE;} while (0)
-#define dma_set_pte_prot(p, prot) \
- do {(p).val = ((p).val & ~3) | ((prot) & 3); } while (0)
-#define dma_pte_addr(p) ((p).val & VTD_PAGE_MASK)
-#define dma_set_pte_addr(p, addr) do {\
- (p).val |= ((addr) & VTD_PAGE_MASK); } while (0)
-#define dma_pte_present(p) (((p).val & 3) != 0)
-
struct intel_iommu;

struct dmar_domain {
--
1.5.4.3

2008-11-20 15:52:09

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 6/8] intel-iommu: move struct device_domain_info out of dma_remapping.h

Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 10 ++++++++++
include/linux/dma_remapping.h | 10 ----------
2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 14066b1..080a860 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -168,6 +168,16 @@ struct dmar_domain {
int flags;
};

+/* PCI domain-device relationship */
+struct device_domain_info {
+ struct list_head link; /* link to domain siblings */
+ struct list_head global; /* link to global list */
+ u8 bus; /* PCI bus numer */
+ u8 devfn; /* PCI devfn number */
+ struct pci_dev *dev; /* it's NULL for PCIE-to-PCI bridge */
+ struct dmar_domain *domain; /* pointer to domain */
+};
+
static void flush_unmaps_timeout(unsigned long data);

DEFINE_TIMER(unmap_timer, flush_unmaps_timeout, 0, 0);
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 3330144..4ef5f6b 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -16,16 +16,6 @@ struct intel_iommu;
struct dmar_domain;
struct root_entry;

-/* PCI domain-device relationship */
-struct device_domain_info {
- struct list_head link; /* link to domain siblings */
- struct list_head global; /* link to global list */
- u8 bus; /* PCI bus numer */
- u8 devfn; /* PCI devfn number */
- struct pci_dev *dev; /* it's NULL for PCIE-to-PCI bridge */
- struct dmar_domain *domain; /* pointer to domain */
-};
-
extern void free_dmar_iommu(struct intel_iommu *iommu);

extern int dmar_disabled;
--
1.5.4.3

2008-11-20 15:52:31

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 1/8] intel-iommu: move DMA_32/64BIT_PFN into intel-iommu.c

Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 3 +++
include/linux/dma_remapping.h | 5 -----
2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 5721196..a9032e6 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -53,6 +53,9 @@

#define DOMAIN_MAX_ADDR(gaw) ((((u64)1) << gaw) - 1)

+#define IOVA_PFN(addr) ((addr) >> PAGE_SHIFT)
+#define DMA_32BIT_PFN IOVA_PFN(DMA_32BIT_MASK)
+#define DMA_64BIT_PFN IOVA_PFN(DMA_64BIT_MASK)

static void flush_unmaps_timeout(unsigned long data);

diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index cf92c49..2e5a5c0 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -9,11 +9,6 @@
#define VTD_PAGE_MASK (((u64)-1) << VTD_PAGE_SHIFT)
#define VTD_PAGE_ALIGN(addr) (((addr) + VTD_PAGE_SIZE - 1) & VTD_PAGE_MASK)

-#define IOVA_PFN(addr) ((addr) >> PAGE_SHIFT)
-#define DMA_32BIT_PFN IOVA_PFN(DMA_32BIT_MASK)
-#define DMA_64BIT_PFN IOVA_PFN(DMA_64BIT_MASK)
-
-
/*
* 0: Present
* 1-11: Reserved
--
1.5.4.3

2008-11-20 15:52:46

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 8/8] intel-iommu: move iommu_prepare_gfx_mapping() out of dma_remapping.h

Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 5 +++++
include/linux/dma_remapping.h | 7 -------
2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 080a860..aec60ad 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -1686,6 +1686,11 @@ static void __init iommu_prepare_gfx_mapping(void)
printk(KERN_ERR "IOMMU: mapping reserved region failed\n");
}
}
+#else /* !CONFIG_DMAR_GFX_WA */
+static inline void iommu_prepare_gfx_mapping(void)
+{
+ return;
+}
#endif

#ifdef CONFIG_DMAR_FLOPPY_WA
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 4ef5f6b..7799a85 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -20,11 +20,4 @@ extern void free_dmar_iommu(struct intel_iommu *iommu);

extern int dmar_disabled;

-#ifndef CONFIG_DMAR_GFX_WA
-static inline void iommu_prepare_gfx_mapping(void)
-{
- return;
-}
-#endif /* !CONFIG_DMAR_GFX_WA */
-
#endif
--
1.5.4.3

2008-11-20 15:53:08

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 7/8] intel-iommu: kill off duplicate def of dmar_disabled

This is only used in dmar.c and intel-iommu.h, so dma_remapping.h
seems like the appropriate place for it.

Signed-off-by: Mark McLoughlin <[email protected]>
---
include/linux/dmar.h | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index f1984fc..f284407 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -144,7 +144,6 @@ struct dmar_rmrr_unit {
list_for_each_entry(rmrr, &dmar_rmrr_units, list)
/* Intel DMAR initialization functions */
extern int intel_iommu_init(void);
-extern int dmar_disabled;
#else
static inline int intel_iommu_init(void)
{
--
1.5.4.3

2008-11-20 15:53:31

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 3/8] intel-iommu: move context entry defs out from dma_remapping.h

Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 38 ++++++++++++++++++++++++++++++++++++++
include/linux/dma_remapping.h | 38 --------------------------------------
2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 2e56e5a..1c011bb 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -90,6 +90,44 @@ get_context_addr_from_root(struct root_entry *root)
NULL);
}

+/*
+ * low 64 bits:
+ * 0: present
+ * 1: fault processing disable
+ * 2-3: translation type
+ * 12-63: address space root
+ * high 64 bits:
+ * 0-2: address width
+ * 3-6: aval
+ * 8-23: domain id
+ */
+struct context_entry {
+ u64 lo;
+ u64 hi;
+};
+#define context_present(c) ((c).lo & 1)
+#define context_fault_disable(c) (((c).lo >> 1) & 1)
+#define context_translation_type(c) (((c).lo >> 2) & 3)
+#define context_address_root(c) ((c).lo & VTD_PAGE_MASK)
+#define context_address_width(c) ((c).hi & 7)
+#define context_domain_id(c) (((c).hi >> 8) & ((1 << 16) - 1))
+
+#define context_set_present(c) do {(c).lo |= 1;} while (0)
+#define context_set_fault_enable(c) \
+ do {(c).lo &= (((u64)-1) << 2) | 1;} while (0)
+#define context_set_translation_type(c, val) \
+ do { \
+ (c).lo &= (((u64)-1) << 4) | 3; \
+ (c).lo |= ((val) & 3) << 2; \
+ } while (0)
+#define CONTEXT_TT_MULTI_LEVEL 0
+#define context_set_address_root(c, val) \
+ do {(c).lo |= (val) & VTD_PAGE_MASK; } while (0)
+#define context_set_address_width(c, val) do {(c).hi |= (val) & 7;} while (0)
+#define context_set_domain_id(c, val) \
+ do {(c).hi |= ((val) & ((1 << 16) - 1)) << 8;} while (0)
+#define context_clear_entry(c) do {(c).lo = 0; (c).hi = 0;} while (0)
+
static void flush_unmaps_timeout(unsigned long data);

DEFINE_TIMER(unmap_timer, flush_unmaps_timeout, 0, 0);
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index d852166..9a88f7d 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -12,44 +12,6 @@
struct root_entry;

/*
- * low 64 bits:
- * 0: present
- * 1: fault processing disable
- * 2-3: translation type
- * 12-63: address space root
- * high 64 bits:
- * 0-2: address width
- * 3-6: aval
- * 8-23: domain id
- */
-struct context_entry {
- u64 lo;
- u64 hi;
-};
-#define context_present(c) ((c).lo & 1)
-#define context_fault_disable(c) (((c).lo >> 1) & 1)
-#define context_translation_type(c) (((c).lo >> 2) & 3)
-#define context_address_root(c) ((c).lo & VTD_PAGE_MASK)
-#define context_address_width(c) ((c).hi & 7)
-#define context_domain_id(c) (((c).hi >> 8) & ((1 << 16) - 1))
-
-#define context_set_present(c) do {(c).lo |= 1;} while (0)
-#define context_set_fault_enable(c) \
- do {(c).lo &= (((u64)-1) << 2) | 1;} while (0)
-#define context_set_translation_type(c, val) \
- do { \
- (c).lo &= (((u64)-1) << 4) | 3; \
- (c).lo |= ((val) & 3) << 2; \
- } while (0)
-#define CONTEXT_TT_MULTI_LEVEL 0
-#define context_set_address_root(c, val) \
- do {(c).lo |= (val) & VTD_PAGE_MASK; } while (0)
-#define context_set_address_width(c, val) do {(c).hi |= (val) & 7;} while (0)
-#define context_set_domain_id(c, val) \
- do {(c).hi |= ((val) & ((1 << 16) - 1)) << 8;} while (0)
-#define context_clear_entry(c) do {(c).lo = 0; (c).hi = 0;} while (0)
-
-/*
* 0: readable
* 1: writable
* 2-6: reserved
--
1.5.4.3

2008-11-20 16:06:09

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH 1/8] intel-iommu: move DMA_32/64BIT_PFN into intel-iommu.c

All 8 patches committed and pushed to the iommu-2.6.git tree; thanks.

--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation

2008-11-20 19:11:31

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 4/8] intel-iommu: move DMA PTE defs out of dma_remapping.h


* Mark McLoughlin <[email protected]> wrote:

> +struct dma_pte {
> + u64 val;
> +};
> +#define dma_clear_pte(p) do {(p).val = 0;} while (0)
> +
> +#define dma_set_pte_readable(p) do {(p).val |= DMA_PTE_READ;} while (0)
> +#define dma_set_pte_writable(p) do {(p).val |= DMA_PTE_WRITE;} while (0)
> +#define dma_set_pte_prot(p, prot) \
> + do {(p).val = ((p).val & ~3) | ((prot) & 3); } while (0)
> +#define dma_pte_addr(p) ((p).val & VTD_PAGE_MASK)
> +#define dma_set_pte_addr(p, addr) do {\
> + (p).val |= ((addr) & VTD_PAGE_MASK); } while (0)
> +#define dma_pte_present(p) (((p).val & 3) != 0)

If you touch this, please also clean this up to use proper inlines,
not CPP macros.

Thanks,

Ingo

2008-11-20 19:13:57

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 3/8] intel-iommu: move context entry defs out from dma_remapping.h


* Mark McLoughlin <[email protected]> wrote:

> Signed-off-by: Mark McLoughlin <[email protected]>
> ---
> drivers/pci/intel-iommu.c | 38 ++++++++++++++++++++++++++++++++++++++
> include/linux/dma_remapping.h | 38 --------------------------------------
> 2 files changed, 38 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index 2e56e5a..1c011bb 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -90,6 +90,44 @@ get_context_addr_from_root(struct root_entry *root)
> NULL);
> }
>
> +/*
> + * low 64 bits:
> + * 0: present
> + * 1: fault processing disable
> + * 2-3: translation type
> + * 12-63: address space root
> + * high 64 bits:
> + * 0-2: address width
> + * 3-6: aval
> + * 8-23: domain id
> + */
> +struct context_entry {
> + u64 lo;
> + u64 hi;
> +};
> +#define context_present(c) ((c).lo & 1)
> +#define context_fault_disable(c) (((c).lo >> 1) & 1)
> +#define context_translation_type(c) (((c).lo >> 2) & 3)
> +#define context_address_root(c) ((c).lo & VTD_PAGE_MASK)
> +#define context_address_width(c) ((c).hi & 7)
> +#define context_domain_id(c) (((c).hi >> 8) & ((1 << 16) - 1))
> +
> +#define context_set_present(c) do {(c).lo |= 1;} while (0)
> +#define context_set_fault_enable(c) \
> + do {(c).lo &= (((u64)-1) << 2) | 1;} while (0)
> +#define context_set_translation_type(c, val) \
> + do { \
> + (c).lo &= (((u64)-1) << 4) | 3; \
> + (c).lo |= ((val) & 3) << 2; \
> + } while (0)
> +#define CONTEXT_TT_MULTI_LEVEL 0
> +#define context_set_address_root(c, val) \
> + do {(c).lo |= (val) & VTD_PAGE_MASK; } while (0)
> +#define context_set_address_width(c, val) do {(c).hi |= (val) & 7;} while (0)
> +#define context_set_domain_id(c, val) \
> + do {(c).hi |= ((val) & ((1 << 16) - 1)) << 8;} while (0)
> +#define context_clear_entry(c) do {(c).lo = 0; (c).hi = 0;} while (0)

Since you move this code, please also do a patch to clean it up to use
inlines, not macros.

Thanks,

Ingo

2008-11-21 16:52:36

by Mark McLoughlin

[permalink] [raw]
Subject: Re: [PATCH 3/8] intel-iommu: move context entry defs out from dma_remapping.h

On Thu, 2008-11-20 at 20:11 +0100, Ingo Molnar wrote:

> Since you move this code, please also do a patch to clean it up to use
> inlines, not macros.

Okay, coming up.

Cheers,
Mark.

2008-11-21 16:56:19

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 1/2] intel-iommu: trivially inline context entry macros

Some macros were unused, so I just dropped them:

context_fault_disable
context_translation_type
context_address_root
context_address_width
context_domain_id

Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 85
+++++++++++++++++++++++++++++----------------
1 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index aec60ad..884291b 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -105,28 +105,53 @@ struct context_entry {
u64 lo;
u64 hi;
};
-#define context_present(c) ((c).lo & 1)
-#define context_fault_disable(c) (((c).lo >> 1) & 1)
-#define context_translation_type(c) (((c).lo >> 2) & 3)
-#define context_address_root(c) ((c).lo & VTD_PAGE_MASK)
-#define context_address_width(c) ((c).hi & 7)
-#define context_domain_id(c) (((c).hi >> 8) & ((1 << 16) - 1))
-
-#define context_set_present(c) do {(c).lo |= 1;} while (0)
-#define context_set_fault_enable(c) \
- do {(c).lo &= (((u64)-1) << 2) | 1;} while (0)
-#define context_set_translation_type(c, val) \
- do { \
- (c).lo &= (((u64)-1) << 4) | 3; \
- (c).lo |= ((val) & 3) << 2; \
- } while (0)
+
+static inline bool context_present(struct context_entry *context)
+{
+ return (context->lo & 1);
+}
+static inline void context_set_present(struct context_entry *context)
+{
+ context->lo |= 1;
+}
+
+static inline void context_set_fault_enable(struct context_entry
*context)
+{
+ context->lo &= (((u64)-1) << 2) | 1;
+}
+
#define CONTEXT_TT_MULTI_LEVEL 0
-#define context_set_address_root(c, val) \
- do {(c).lo |= (val) & VTD_PAGE_MASK; } while (0)
-#define context_set_address_width(c, val) do {(c).hi |= (val) & 7;}
while (0)
-#define context_set_domain_id(c, val) \
- do {(c).hi |= ((val) & ((1 << 16) - 1)) << 8;} while (0)
-#define context_clear_entry(c) do {(c).lo = 0; (c).hi = 0;} while (0)
+
+static inline void context_set_translation_type(struct context_entry
*context,
+ unsigned long value)
+{
+ context->lo &= (((u64)-1) << 4) | 3;
+ context->lo |= (value & 3) << 2;
+}
+
+static inline void context_set_address_root(struct context_entry
*context,
+ unsigned long value)
+{
+ context->lo |= value & VTD_PAGE_MASK;
+}
+
+static inline void context_set_address_width(struct context_entry
*context,
+ unsigned long value)
+{
+ context->hi |= value & 7;
+}
+
+static inline void context_set_domain_id(struct context_entry *context,
+ unsigned long value)
+{
+ context->hi |= (value & ((1 << 16) - 1)) << 8;
+}
+
+static inline void context_clear_entry(struct context_entry *context)
+{
+ context->lo = 0;
+ context->hi = 0;
+}

/*
* 0: readable
@@ -349,7 +374,7 @@ static int device_context_mapped(struct intel_iommu
*iommu, u8 bus, u8 devfn)
ret = 0;
goto out;
}
- ret = context_present(context[devfn]);
+ ret = context_present(&context[devfn]);
out:
spin_unlock_irqrestore(&iommu->lock, flags);
return ret;
@@ -365,7 +390,7 @@ static void clear_context_table(struct intel_iommu
*iommu, u8 bus, u8 devfn)
root = &iommu->root_entry[bus];
context = get_context_addr_from_root(root);
if (context) {
- context_clear_entry(context[devfn]);
+ context_clear_entry(&context[devfn]);
__iommu_flush_cache(iommu, &context[devfn], \
sizeof(*context));
}
@@ -1284,17 +1309,17 @@ static int domain_context_mapping_one(struct
dmar_domain *domain,
if (!context)
return -ENOMEM;
spin_lock_irqsave(&iommu->lock, flags);
- if (context_present(*context)) {
+ if (context_present(context)) {
spin_unlock_irqrestore(&iommu->lock, flags);
return 0;
}

- context_set_domain_id(*context, domain->id);
- context_set_address_width(*context, domain->agaw);
- context_set_address_root(*context, virt_to_phys(domain->pgd));
- context_set_translation_type(*context, CONTEXT_TT_MULTI_LEVEL);
- context_set_fault_enable(*context);
- context_set_present(*context);
+ context_set_domain_id(context, domain->id);
+ context_set_address_width(context, domain->agaw);
+ context_set_address_root(context, virt_to_phys(domain->pgd));
+ context_set_translation_type(context, CONTEXT_TT_MULTI_LEVEL);
+ context_set_fault_enable(context);
+ context_set_present(context);
__iommu_flush_cache(iommu, context, sizeof(*context));

/* it's a non-present to present mapping */
--
1.6.0.3

2008-11-21 16:58:33

by Mark McLoughlin

[permalink] [raw]
Subject: [PATCH 2/2] intel-iommu: trivially inline DMA PTE macros


Signed-off-by: Mark McLoughlin <[email protected]>
---
drivers/pci/intel-iommu.c | 71 ++++++++++++++++++++++++++++++--------------
1 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 884291b..b9cf713 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -164,16 +164,41 @@ static inline void context_clear_entry(struct context_entry *context)
struct dma_pte {
u64 val;
};
-#define dma_clear_pte(p) do {(p).val = 0;} while (0)

-#define dma_set_pte_readable(p) do {(p).val |= DMA_PTE_READ;} while (0)
-#define dma_set_pte_writable(p) do {(p).val |= DMA_PTE_WRITE;} while (0)
-#define dma_set_pte_prot(p, prot) \
- do {(p).val = ((p).val & ~3) | ((prot) & 3); } while (0)
-#define dma_pte_addr(p) ((p).val & VTD_PAGE_MASK)
-#define dma_set_pte_addr(p, addr) do {\
- (p).val |= ((addr) & VTD_PAGE_MASK); } while (0)
-#define dma_pte_present(p) (((p).val & 3) != 0)
+static inline void dma_clear_pte(struct dma_pte *pte)
+{
+ pte->val = 0;
+}
+
+static inline void dma_set_pte_readable(struct dma_pte *pte)
+{
+ pte->val |= DMA_PTE_READ;
+}
+
+static inline void dma_set_pte_writable(struct dma_pte *pte)
+{
+ pte->val |= DMA_PTE_WRITE;
+}
+
+static inline void dma_set_pte_prot(struct dma_pte *pte, unsigned long prot)
+{
+ pte->val = (pte->val & ~3) | (prot & 3);
+}
+
+static inline u64 dma_pte_addr(struct dma_pte *pte)
+{
+ return (pte->val & VTD_PAGE_MASK);
+}
+
+static inline void dma_set_pte_addr(struct dma_pte *pte, u64 addr)
+{
+ pte->val |= (addr & VTD_PAGE_MASK);
+}
+
+static inline bool dma_pte_present(struct dma_pte *pte)
+{
+ return (pte->val & 3) != 0;
+}

struct dmar_domain {
int id; /* domain id */
@@ -487,7 +512,7 @@ static struct dma_pte * addr_to_dma_pte(struct dmar_domain *domain, u64 addr)
if (level == 1)
break;

- if (!dma_pte_present(*pte)) {
+ if (!dma_pte_present(pte)) {
tmp_page = alloc_pgtable_page();

if (!tmp_page) {
@@ -497,16 +522,16 @@ static struct dma_pte * addr_to_dma_pte(struct dmar_domain *domain, u64 addr)
}
__iommu_flush_cache(domain->iommu, tmp_page,
PAGE_SIZE);
- dma_set_pte_addr(*pte, virt_to_phys(tmp_page));
+ dma_set_pte_addr(pte, virt_to_phys(tmp_page));
/*
* high level table always sets r/w, last level page
* table control read/write
*/
- dma_set_pte_readable(*pte);
- dma_set_pte_writable(*pte);
+ dma_set_pte_readable(pte);
+ dma_set_pte_writable(pte);
__iommu_flush_cache(domain->iommu, pte, sizeof(*pte));
}
- parent = phys_to_virt(dma_pte_addr(*pte));
+ parent = phys_to_virt(dma_pte_addr(pte));
level--;
}

@@ -529,9 +554,9 @@ static struct dma_pte *dma_addr_level_pte(struct dmar_domain *domain, u64 addr,
if (level == total)
return pte;

- if (!dma_pte_present(*pte))
+ if (!dma_pte_present(pte))
break;
- parent = phys_to_virt(dma_pte_addr(*pte));
+ parent = phys_to_virt(dma_pte_addr(pte));
total--;
}
return NULL;
@@ -546,7 +571,7 @@ static void dma_pte_clear_one(struct dmar_domain *domain, u64 addr)
pte = dma_addr_level_pte(domain, addr, 1);

if (pte) {
- dma_clear_pte(*pte);
+ dma_clear_pte(pte);
__iommu_flush_cache(domain->iommu, pte, sizeof(*pte));
}
}
@@ -593,8 +618,8 @@ static void dma_pte_free_pagetable(struct dmar_domain *domain,
pte = dma_addr_level_pte(domain, tmp, level);
if (pte) {
free_pgtable_page(
- phys_to_virt(dma_pte_addr(*pte)));
- dma_clear_pte(*pte);
+ phys_to_virt(dma_pte_addr(pte)));
+ dma_clear_pte(pte);
__iommu_flush_cache(domain->iommu,
pte, sizeof(*pte));
}
@@ -1421,9 +1446,9 @@ domain_page_mapping(struct dmar_domain *domain, dma_addr_t iova,
/* We don't need lock here, nobody else
* touches the iova range
*/
- BUG_ON(dma_pte_addr(*pte));
- dma_set_pte_addr(*pte, start_pfn << VTD_PAGE_SHIFT);
- dma_set_pte_prot(*pte, prot);
+ BUG_ON(dma_pte_addr(pte));
+ dma_set_pte_addr(pte, start_pfn << VTD_PAGE_SHIFT);
+ dma_set_pte_prot(pte, prot);
__iommu_flush_cache(domain->iommu, pte, sizeof(*pte));
start_pfn++;
index++;
@@ -2582,7 +2607,7 @@ u64 intel_iommu_iova_to_pfn(struct dmar_domain *domain, u64 iova)
pte = addr_to_dma_pte(domain, iova);

if (pte)
- pfn = dma_pte_addr(*pte);
+ pfn = dma_pte_addr(pte);

return pfn >> VTD_PAGE_SHIFT;
}
--
1.6.0.3