* [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up
@ 2018-10-04 10:45 Paul Durrant
2018-10-04 10:45 ` [PATCH v14 1/9] iommu: introduce the concept of DFN Paul Durrant
` (8 more replies)
0 siblings, 9 replies; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
Konrad Rzeszutek Wilk, Andrew Cooper, Ian Jackson, George Dunlap,
Tim Deegan, Julien Grall, Paul Durrant, Tamas K Lengyel,
Jan Beulich, Brian Woods, Suravee Suthikulpanit
This series contains pre-requisites and clean-up needed for paravirtual
IOMMU support.
I have separated these patches to avoid further delaying their application
whilst I re-work the implementation of paravirtual IOMMU after review of
v6 of the series. Several of them already have all necessary acks.
v11:
- Pull in two more patches from v6.
Paul Durrant (9):
iommu: introduce the concept of DFN...
iommu: make use of type-safe DFN and MFN in exported functions
iommu: push use of type-safe DFN and MFN into iommu_ops
iommu: don't domain_crash() inside iommu_map/unmap_page()
memory: add check_get_page_from_gfn() as a wrapper...
vtd: add missing check for shared EPT...
vtd: add lookup_page method to iommu_ops
mm / iommu: include need_iommu() test in iommu_use_hap_pt()
mm / iommu: split need_iommu() into has_iommu_pt() and
need_iommu_pt_sync()
xen/arch/arm/p2m.c | 9 ++-
xen/arch/x86/hvm/emulate.c | 25 ++++----
xen/arch/x86/hvm/hvm.c | 14 +---
xen/arch/x86/hvm/mtrr.c | 4 +-
xen/arch/x86/mm.c | 15 +++--
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 19 ++++--
xen/arch/x86/mm/p2m-pt.c | 52 +++++++++------
xen/arch/x86/mm/p2m.c | 42 ++++++++----
xen/arch/x86/mm/paging.c | 2 +-
xen/arch/x86/x86_64/mm.c | 15 +++--
xen/common/grant_table.c | 48 +++++++-------
xen/common/memory.c | 66 +++++++++++++------
xen/common/vm_event.c | 2 +-
xen/drivers/passthrough/amd/iommu_cmd.c | 18 +++---
xen/drivers/passthrough/amd/iommu_map.c | 88 +++++++++++++------------
xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +-
xen/drivers/passthrough/arm/smmu.c | 20 +++---
xen/drivers/passthrough/device_tree.c | 21 +++---
xen/drivers/passthrough/iommu.c | 92 ++++++++++++++++-----------
xen/drivers/passthrough/pci.c | 11 ++--
xen/drivers/passthrough/vtd/iommu.c | 88 +++++++++++++++++++------
xen/drivers/passthrough/vtd/iommu.h | 3 +
xen/drivers/passthrough/vtd/x86/vtd.c | 1 -
xen/drivers/passthrough/x86/iommu.c | 11 ++--
xen/include/asm-arm/grant_table.h | 2 +-
xen/include/asm-arm/iommu.h | 2 +-
xen/include/asm-arm/p2m.h | 4 +-
xen/include/asm-x86/grant_table.h | 2 +-
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 8 +--
xen/include/asm-x86/iommu.h | 17 ++++-
xen/include/asm-x86/p2m.h | 5 +-
xen/include/xen/iommu.h | 68 +++++++++++++++++---
xen/include/xen/mm.h | 5 ++
xen/include/xen/p2m-common.h | 6 ++
xen/include/xen/sched.h | 14 ++--
36 files changed, 512 insertions(+), 291 deletions(-)
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Brian Woods <brian.woods@amd.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v14 1/9] iommu: introduce the concept of DFN...
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
@ 2018-10-04 10:45 ` Paul Durrant
2018-10-04 10:45 ` [PATCH v14 2/9] iommu: make use of type-safe DFN and MFN in exported functions Paul Durrant
` (7 subsequent siblings)
8 siblings, 0 replies; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Wei Liu, Stefano Stabellini
...meaning 'device DMA frame number' i.e. a frame number mapped in the IOMMU
(rather than the MMU) and hence used for DMA address translation.
This patch is a largely cosmetic change that substitutes the terms 'gfn'
and 'gaddr' for 'dfn' and 'daddr' in all the places where the frame number
or address relate to a device rather than the CPU.
The parts that are not purely cosmetic are:
- the introduction of a type-safe declaration of dfn_t and definition of
INVALID_DFN to make the substitution of gfn_x(INVALID_GFN) mechanical.
- the introduction of __dfn_to_daddr and __daddr_to_dfn (and type-safe
variants without the leading __) with some use of the former.
Subsequent patches will convert code to make use of type-safe DFNs.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
v9:
- Re-word comment in mm.h.
- Move definitions relating to daddr into asm-x86/iommu.h since these are
not used by any ARM IOMMU implementation.
- Fix __daddr_to_dfn() to properly parenthesize and remove cast.
v8:
- Correct definition of INVALID_DFN.
- Don't use _AC in definition of IOMMU_PAGE_SIZE.
v7:
- Re-name BFN -> DFN as requested by Jan.
- Dropped Wei's R-b because of name change.
v6:
- Dropped changes to 'mfn' section in xen/mm.h as suggested by Kevin.
v3:
- Get rid of intermediate 'frame' variables again.
v2:
- Addressed comments from Jan.
---
xen/drivers/passthrough/amd/iommu_cmd.c | 18 +++----
xen/drivers/passthrough/amd/iommu_map.c | 78 ++++++++++++++---------------
xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +-
xen/drivers/passthrough/arm/smmu.c | 16 +++---
xen/drivers/passthrough/iommu.c | 28 +++++------
xen/drivers/passthrough/vtd/iommu.c | 30 +++++------
xen/include/asm-x86/iommu.h | 12 +++++
xen/include/xen/iommu.h | 26 +++++++---
xen/include/xen/mm.h | 5 ++
9 files changed, 123 insertions(+), 92 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c b/xen/drivers/passthrough/amd/iommu_cmd.c
index 08247fa354..d4d071e53e 100644
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -284,7 +284,7 @@ void invalidate_iommu_all(struct amd_iommu *iommu)
}
void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
- uint64_t gaddr, unsigned int order)
+ daddr_t daddr, unsigned int order)
{
unsigned long flags;
struct amd_iommu *iommu;
@@ -315,12 +315,12 @@ void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
/* send INVALIDATE_IOTLB_PAGES command */
spin_lock_irqsave(&iommu->lock, flags);
- invalidate_iotlb_pages(iommu, maxpend, 0, queueid, gaddr, req_id, order);
+ invalidate_iotlb_pages(iommu, maxpend, 0, queueid, daddr, req_id, order);
flush_command_buffer(iommu);
spin_unlock_irqrestore(&iommu->lock, flags);
}
-static void amd_iommu_flush_all_iotlbs(struct domain *d, uint64_t gaddr,
+static void amd_iommu_flush_all_iotlbs(struct domain *d, daddr_t daddr,
unsigned int order)
{
struct pci_dev *pdev;
@@ -333,7 +333,7 @@ static void amd_iommu_flush_all_iotlbs(struct domain *d, uint64_t gaddr,
u8 devfn = pdev->devfn;
do {
- amd_iommu_flush_iotlb(devfn, pdev, gaddr, order);
+ amd_iommu_flush_iotlb(devfn, pdev, daddr, order);
devfn += pdev->phantom_stride;
} while ( devfn != pdev->devfn &&
PCI_SLOT(devfn) == PCI_SLOT(pdev->devfn) );
@@ -342,7 +342,7 @@ static void amd_iommu_flush_all_iotlbs(struct domain *d, uint64_t gaddr,
/* Flush iommu cache after p2m changes. */
static void _amd_iommu_flush_pages(struct domain *d,
- uint64_t gaddr, unsigned int order)
+ daddr_t daddr, unsigned int order)
{
unsigned long flags;
struct amd_iommu *iommu;
@@ -352,13 +352,13 @@ static void _amd_iommu_flush_pages(struct domain *d,
for_each_amd_iommu ( iommu )
{
spin_lock_irqsave(&iommu->lock, flags);
- invalidate_iommu_pages(iommu, gaddr, dom_id, order);
+ invalidate_iommu_pages(iommu, daddr, dom_id, order);
flush_command_buffer(iommu);
spin_unlock_irqrestore(&iommu->lock, flags);
}
if ( ats_enabled )
- amd_iommu_flush_all_iotlbs(d, gaddr, order);
+ amd_iommu_flush_all_iotlbs(d, daddr, order);
}
void amd_iommu_flush_all_pages(struct domain *d)
@@ -367,9 +367,9 @@ void amd_iommu_flush_all_pages(struct domain *d)
}
void amd_iommu_flush_pages(struct domain *d,
- unsigned long gfn, unsigned int order)
+ unsigned long dfn, unsigned int order)
{
- _amd_iommu_flush_pages(d, (uint64_t) gfn << PAGE_SHIFT, order);
+ _amd_iommu_flush_pages(d, __dfn_to_daddr(dfn), order);
}
void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c
index 70b4345b37..61ade71850 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -35,12 +35,12 @@ static unsigned int pfn_to_pde_idx(unsigned long pfn, unsigned int level)
return idx;
}
-void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long gfn)
+void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long dfn)
{
u64 *table, *pte;
table = map_domain_page(_mfn(l1_mfn));
- pte = table + pfn_to_pde_idx(gfn, IOMMU_PAGING_MODE_LEVEL_1);
+ pte = table + pfn_to_pde_idx(dfn, IOMMU_PAGING_MODE_LEVEL_1);
*pte = 0;
unmap_domain_page(table);
}
@@ -104,7 +104,7 @@ static bool_t set_iommu_pde_present(u32 *pde, unsigned long next_mfn,
return need_flush;
}
-static bool_t set_iommu_pte_present(unsigned long pt_mfn, unsigned long gfn,
+static bool_t set_iommu_pte_present(unsigned long pt_mfn, unsigned long dfn,
unsigned long next_mfn, int pde_level,
bool_t iw, bool_t ir)
{
@@ -114,7 +114,7 @@ static bool_t set_iommu_pte_present(unsigned long pt_mfn, unsigned long gfn,
table = map_domain_page(_mfn(pt_mfn));
- pde = (u32*)(table + pfn_to_pde_idx(gfn, pde_level));
+ pde = (u32*)(table + pfn_to_pde_idx(dfn, pde_level));
need_flush = set_iommu_pde_present(pde, next_mfn,
IOMMU_PAGING_MODE_LEVEL_0, iw, ir);
@@ -331,7 +331,7 @@ static void set_pde_count(u64 *pde, unsigned int count)
* otherwise increase pde count if mfn is contigous with mfn - 1
*/
static int iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
- unsigned long gfn, unsigned long mfn,
+ unsigned long dfn, unsigned long mfn,
unsigned int merge_level)
{
unsigned int pde_count, next_level;
@@ -347,7 +347,7 @@ static int iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
/* get pde at merge level */
table = map_domain_page(_mfn(pt_mfn));
- pde = table + pfn_to_pde_idx(gfn, merge_level);
+ pde = table + pfn_to_pde_idx(dfn, merge_level);
/* get page table of next level */
ntable_maddr = amd_iommu_get_next_table_from_pte((u32*)pde);
@@ -362,7 +362,7 @@ static int iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
mask = (1ULL<< (PTE_PER_TABLE_SHIFT * next_level)) - 1;
if ( ((first_mfn & mask) == 0) &&
- (((gfn & mask) | first_mfn) == mfn) )
+ (((dfn & mask) | first_mfn) == mfn) )
{
pde_count = get_pde_count(*pde);
@@ -387,7 +387,7 @@ out:
}
static int iommu_merge_pages(struct domain *d, unsigned long pt_mfn,
- unsigned long gfn, unsigned int flags,
+ unsigned long dfn, unsigned int flags,
unsigned int merge_level)
{
u64 *table, *pde, *ntable;
@@ -398,7 +398,7 @@ static int iommu_merge_pages(struct domain *d, unsigned long pt_mfn,
ASSERT( spin_is_locked(&hd->arch.mapping_lock) && pt_mfn );
table = map_domain_page(_mfn(pt_mfn));
- pde = table + pfn_to_pde_idx(gfn, merge_level);
+ pde = table + pfn_to_pde_idx(dfn, merge_level);
/* get first mfn */
ntable_mfn = amd_iommu_get_next_table_from_pte((u32*)pde) >> PAGE_SHIFT;
@@ -436,7 +436,7 @@ static int iommu_merge_pages(struct domain *d, unsigned long pt_mfn,
* {Re, un}mapping super page frames causes re-allocation of io
* page tables.
*/
-static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
+static int iommu_pde_from_dfn(struct domain *d, unsigned long dfn,
unsigned long pt_mfn[])
{
u64 *pde, *next_table_vaddr;
@@ -465,7 +465,7 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
pt_mfn[level] = next_table_mfn;
next_table_vaddr = map_domain_page(_mfn(next_table_mfn));
- pde = next_table_vaddr + pfn_to_pde_idx(pfn, level);
+ pde = next_table_vaddr + pfn_to_pde_idx(dfn, level);
/* Here might be a super page frame */
next_table_mfn = amd_iommu_get_next_table_from_pte((uint32_t*)pde)
@@ -477,11 +477,11 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
next_table_mfn != 0 )
{
int i;
- unsigned long mfn, gfn;
+ unsigned long mfn, pfn;
unsigned int page_sz;
page_sz = 1 << (PTE_PER_TABLE_SHIFT * (next_level - 1));
- gfn = pfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
+ pfn = dfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
mfn = next_table_mfn;
/* allocate lower level page table */
@@ -499,10 +499,10 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
for ( i = 0; i < PTE_PER_TABLE_SIZE; i++ )
{
- set_iommu_pte_present(next_table_mfn, gfn, mfn, next_level,
+ set_iommu_pte_present(next_table_mfn, pfn, mfn, next_level,
!!IOMMUF_writable, !!IOMMUF_readable);
mfn += page_sz;
- gfn += page_sz;
+ pfn += page_sz;
}
amd_iommu_flush_all_pages(d);
@@ -540,7 +540,7 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
return 0;
}
-static int update_paging_mode(struct domain *d, unsigned long gfn)
+static int update_paging_mode(struct domain *d, unsigned long dfn)
{
u16 bdf;
void *device_entry;
@@ -554,13 +554,13 @@ static int update_paging_mode(struct domain *d, unsigned long gfn)
unsigned long old_root_mfn;
struct domain_iommu *hd = dom_iommu(d);
- if ( gfn == gfn_x(INVALID_GFN) )
+ if ( dfn == dfn_x(INVALID_DFN) )
return -EADDRNOTAVAIL;
- ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
+ ASSERT(!(dfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
level = hd->arch.paging_mode;
old_root = hd->arch.root_table;
- offset = gfn >> (PTE_PER_TABLE_SHIFT * (level - 1));
+ offset = dfn >> (PTE_PER_TABLE_SHIFT * (level - 1));
ASSERT(spin_is_locked(&hd->arch.mapping_lock) && is_hvm_domain(d));
@@ -631,7 +631,7 @@ static int update_paging_mode(struct domain *d, unsigned long gfn)
return 0;
}
-int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
+int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
unsigned int flags)
{
bool_t need_flush = 0;
@@ -651,34 +651,34 @@ int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
if ( rc )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Root table alloc failed, gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %lx\n", dfn);
domain_crash(d);
return rc;
}
/* Since HVM domain is initialized with 2 level IO page table,
- * we might need a deeper page table for lager gfn now */
+ * we might need a deeper page table for wider dfn now */
if ( is_hvm_domain(d) )
{
- if ( update_paging_mode(d, gfn) )
+ if ( update_paging_mode(d, dfn) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Update page mode failed gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn);
domain_crash(d);
return -EFAULT;
}
}
- if ( iommu_pde_from_gfn(d, gfn, pt_mfn) || (pt_mfn[1] == 0) )
+ if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Invalid IO pagetable entry gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn);
domain_crash(d);
return -EFAULT;
}
/* Install 4k mapping first */
- need_flush = set_iommu_pte_present(pt_mfn[1], gfn, mfn,
+ need_flush = set_iommu_pte_present(pt_mfn[1], dfn, mfn,
IOMMU_PAGING_MODE_LEVEL_1,
!!(flags & IOMMUF_writable),
!!(flags & IOMMUF_readable));
@@ -690,7 +690,7 @@ int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
/* 4K mapping for PV guests never changes,
* no need to flush if we trust non-present bits */
if ( is_hvm_domain(d) )
- amd_iommu_flush_pages(d, gfn, 0);
+ amd_iommu_flush_pages(d, dfn, 0);
for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2;
merge_level <= hd->arch.paging_mode; merge_level++ )
@@ -698,15 +698,15 @@ int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
if ( pt_mfn[merge_level] == 0 )
break;
if ( !iommu_update_pde_count(d, pt_mfn[merge_level],
- gfn, mfn, merge_level) )
+ dfn, mfn, merge_level) )
break;
- if ( iommu_merge_pages(d, pt_mfn[merge_level], gfn,
+ if ( iommu_merge_pages(d, pt_mfn[merge_level], dfn,
flags, merge_level) )
{
spin_unlock(&hd->arch.mapping_lock);
AMD_IOMMU_DEBUG("Merge iommu page failed at level %d, "
- "gfn = %lx mfn = %lx\n", merge_level, gfn, mfn);
+ "dfn = %lx mfn = %lx\n", merge_level, dfn, mfn);
domain_crash(d);
return -EFAULT;
}
@@ -720,7 +720,7 @@ out:
return 0;
}
-int amd_iommu_unmap_page(struct domain *d, unsigned long gfn)
+int amd_iommu_unmap_page(struct domain *d, unsigned long dfn)
{
unsigned long pt_mfn[7];
struct domain_iommu *hd = dom_iommu(d);
@@ -739,34 +739,34 @@ int amd_iommu_unmap_page(struct domain *d, unsigned long gfn)
}
/* Since HVM domain is initialized with 2 level IO page table,
- * we might need a deeper page table for lager gfn now */
+ * we might need a deeper page table for lager dfn now */
if ( is_hvm_domain(d) )
{
- int rc = update_paging_mode(d, gfn);
+ int rc = update_paging_mode(d, dfn);
if ( rc )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Update page mode failed gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn);
if ( rc != -EADDRNOTAVAIL )
domain_crash(d);
return rc;
}
}
- if ( iommu_pde_from_gfn(d, gfn, pt_mfn) || (pt_mfn[1] == 0) )
+ if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Invalid IO pagetable entry gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn);
domain_crash(d);
return -EFAULT;
}
/* mark PTE as 'page not present' */
- clear_iommu_pte_present(pt_mfn[1], gfn);
+ clear_iommu_pte_present(pt_mfn[1], dfn);
spin_unlock(&hd->arch.mapping_lock);
- amd_iommu_flush_pages(d, gfn, 0);
+ amd_iommu_flush_pages(d, dfn, 0);
return 0;
}
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 821fe03df5..77323395f2 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -548,7 +548,7 @@ static void amd_dump_p2m_table_level(struct page_info* pg, int level,
maddr_to_page(next_table_maddr), next_level,
address, indent + 1);
else
- printk("%*sgfn: %08lx mfn: %08lx\n",
+ printk("%*sdfn: %08lx mfn: %08lx\n",
indent, "",
(unsigned long)PFN_DOWN(address),
(unsigned long)PFN_DOWN(next_table_maddr));
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 8f91807b1b..1eda96a72a 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2551,7 +2551,7 @@ static int __must_check arm_smmu_iotlb_flush_all(struct domain *d)
}
static int __must_check arm_smmu_iotlb_flush(struct domain *d,
- unsigned long gfn,
+ unsigned long dfn,
unsigned int page_count)
{
/* ARM SMMU v1 doesn't have flush by VMA and VMID */
@@ -2748,7 +2748,7 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
xfree(xen_domain);
}
-static int __must_check arm_smmu_map_page(struct domain *d, unsigned long gfn,
+static int __must_check arm_smmu_map_page(struct domain *d, unsigned long dfn,
unsigned long mfn, unsigned int flags)
{
p2m_type_t t;
@@ -2759,10 +2759,10 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long gfn,
* protected by an IOMMU, Xen needs to add a 1:1 mapping in the domain
* p2m to allow DMA request to work.
* This is only valid when the domain is directed mapped. Hence this
- * function should only be used by gnttab code with gfn == mfn.
+ * function should only be used by gnttab code with gfn == mfn == dfn.
*/
BUG_ON(!is_domain_direct_mapped(d));
- BUG_ON(mfn != gfn);
+ BUG_ON(mfn != dfn);
/* We only support readable and writable flags */
if (!(flags & (IOMMUF_readable | IOMMUF_writable)))
@@ -2774,19 +2774,19 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long gfn,
* The function guest_physmap_add_entry replaces the current mapping
* if there is already one...
*/
- return guest_physmap_add_entry(d, _gfn(gfn), _mfn(mfn), 0, t);
+ return guest_physmap_add_entry(d, _gfn(dfn), _mfn(dfn), 0, t);
}
-static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long gfn)
+static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long dfn)
{
/*
* This function should only be used by gnttab code when the domain
- * is direct mapped
+ * is direct mapped (i.e. gfn == mfn == dfn).
*/
if ( !is_domain_direct_mapped(d) )
return -EINVAL;
- return guest_physmap_remove_page(d, _gfn(gfn), _mfn(gfn), 0);
+ return guest_physmap_remove_page(d, _gfn(dfn), _mfn(dfn), 0);
}
static const struct iommu_ops arm_smmu_iommu_ops = {
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 8f4309760a..0695eb8d7e 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -215,7 +215,7 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
page_list_for_each ( page, &d->page_list )
{
unsigned long mfn = mfn_x(page_to_mfn(page));
- unsigned long gfn = mfn_to_gmfn(d, mfn);
+ unsigned long dfn = mfn_to_gmfn(d, mfn);
unsigned int mapping = IOMMUF_readable;
int ret;
@@ -224,7 +224,7 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
== PGT_writable_page) )
mapping |= IOMMUF_writable;
- ret = hd->platform_ops->map_page(d, gfn, mfn, mapping);
+ ret = hd->platform_ops->map_page(d, dfn, mfn, mapping);
if ( !rc )
rc = ret;
@@ -285,7 +285,7 @@ void iommu_domain_destroy(struct domain *d)
arch_iommu_domain_destroy(d);
}
-int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
+int iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
unsigned int flags)
{
const struct domain_iommu *hd = dom_iommu(d);
@@ -294,13 +294,13 @@ int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->map_page(d, gfn, mfn, flags);
+ rc = hd->platform_ops->map_page(d, dfn, mfn, flags);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU mapping gfn %#lx to mfn %#lx failed: %d\n",
- d->domain_id, gfn, mfn, rc);
+ "d%d: IOMMU mapping dfn %#lx to mfn %#lx failed: %d\n",
+ d->domain_id, dfn, mfn, rc);
if ( !is_hardware_domain(d) )
domain_crash(d);
@@ -309,7 +309,7 @@ int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
return rc;
}
-int iommu_unmap_page(struct domain *d, unsigned long gfn)
+int iommu_unmap_page(struct domain *d, unsigned long dfn)
{
const struct domain_iommu *hd = dom_iommu(d);
int rc;
@@ -317,13 +317,13 @@ int iommu_unmap_page(struct domain *d, unsigned long gfn)
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->unmap_page(d, gfn);
+ rc = hd->platform_ops->unmap_page(d, dfn);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU unmapping gfn %#lx failed: %d\n",
- d->domain_id, gfn, rc);
+ "d%d: IOMMU unmapping dfn %#lx failed: %d\n",
+ d->domain_id, dfn, rc);
if ( !is_hardware_domain(d) )
domain_crash(d);
@@ -349,7 +349,7 @@ static void iommu_free_pagetables(unsigned long unused)
cpumask_cycle(smp_processor_id(), &cpu_online_map));
}
-int iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+int iommu_iotlb_flush(struct domain *d, unsigned long dfn,
unsigned int page_count)
{
const struct domain_iommu *hd = dom_iommu(d);
@@ -358,13 +358,13 @@ int iommu_iotlb_flush(struct domain *d, unsigned long gfn,
if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush )
return 0;
- rc = hd->platform_ops->iotlb_flush(d, gfn, page_count);
+ rc = hd->platform_ops->iotlb_flush(d, dfn, page_count);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU IOTLB flush failed: %d, gfn %#lx, page count %u\n",
- d->domain_id, rc, gfn, page_count);
+ "d%d: IOMMU IOTLB flush failed: %d, dfn %#lx, page count %u\n",
+ d->domain_id, rc, dfn, page_count);
if ( !is_hardware_domain(d) )
domain_crash(d);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index cf8a80d7a1..485704bcce 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -585,7 +585,7 @@ static int __must_check iommu_flush_all(void)
}
static int __must_check iommu_flush_iotlb(struct domain *d,
- unsigned long gfn,
+ unsigned long dfn,
bool_t dma_old_pte_present,
unsigned int page_count)
{
@@ -612,12 +612,12 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
if ( iommu_domid == -1 )
continue;
- if ( page_count != 1 || gfn == gfn_x(INVALID_GFN) )
+ if ( page_count != 1 || dfn == dfn_x(INVALID_DFN) )
rc = iommu_flush_iotlb_dsi(iommu, iommu_domid,
0, flush_dev_iotlb);
else
rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
- (paddr_t)gfn << PAGE_SHIFT_4K,
+ __dfn_to_daddr(dfn),
PAGE_ORDER_4K,
!dma_old_pte_present,
flush_dev_iotlb);
@@ -633,15 +633,15 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
}
static int __must_check iommu_flush_iotlb_pages(struct domain *d,
- unsigned long gfn,
+ unsigned long dfn,
unsigned int page_count)
{
- return iommu_flush_iotlb(d, gfn, 1, page_count);
+ return iommu_flush_iotlb(d, dfn, 1, page_count);
}
static int __must_check iommu_flush_iotlb_all(struct domain *d)
{
- return iommu_flush_iotlb(d, gfn_x(INVALID_GFN), 0, 0);
+ return iommu_flush_iotlb(d, dfn_x(INVALID_DFN), 0, 0);
}
/* clear one page's page table */
@@ -1763,7 +1763,7 @@ static void iommu_domain_teardown(struct domain *d)
}
static int __must_check intel_iommu_map_page(struct domain *d,
- unsigned long gfn,
+ unsigned long dfn,
unsigned long mfn,
unsigned int flags)
{
@@ -1782,14 +1782,14 @@ static int __must_check intel_iommu_map_page(struct domain *d,
spin_lock(&hd->arch.mapping_lock);
- pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K, 1);
+ pg_maddr = addr_to_dma_page_maddr(d, __dfn_to_daddr(dfn), 1);
if ( pg_maddr == 0 )
{
spin_unlock(&hd->arch.mapping_lock);
return -ENOMEM;
}
page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
- pte = page + (gfn & LEVEL_MASK);
+ pte = page + (dfn & LEVEL_MASK);
old = *pte;
dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
dma_set_pte_prot(new,
@@ -1813,22 +1813,22 @@ static int __must_check intel_iommu_map_page(struct domain *d,
unmap_vtd_domain_page(page);
if ( !this_cpu(iommu_dont_flush_iotlb) )
- rc = iommu_flush_iotlb(d, gfn, dma_pte_present(old), 1);
+ rc = iommu_flush_iotlb(d, dfn, dma_pte_present(old), 1);
return rc;
}
static int __must_check intel_iommu_unmap_page(struct domain *d,
- unsigned long gfn)
+ unsigned long dfn)
{
/* Do nothing if hardware domain and iommu supports pass thru. */
if ( iommu_hwdom_passthrough && is_hardware_domain(d) )
return 0;
- return dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
+ return dma_pte_clear_one(d, __dfn_to_daddr(dfn));
}
-int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
+int iommu_pte_flush(struct domain *d, uint64_t dfn, uint64_t *pte,
int order, int present)
{
struct acpi_drhd_unit *drhd;
@@ -1852,7 +1852,7 @@ int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
continue;
rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
- (paddr_t)gfn << PAGE_SHIFT_4K,
+ __dfn_to_daddr(dfn),
order, !present, flush_dev_iotlb);
if ( rc > 0 )
{
@@ -2622,7 +2622,7 @@ static void vtd_dump_p2m_table_level(paddr_t pt_maddr, int level, paddr_t gpa,
vtd_dump_p2m_table_level(dma_pte_addr(*pte), next_level,
address, indent + 1);
else
- printk("%*sgfn: %08lx mfn: %08lx\n",
+ printk("%*sdfn: %08lx mfn: %08lx\n",
indent, "",
(unsigned long)(address >> PAGE_SHIFT_4K),
(unsigned long)(dma_pte_addr(*pte) >> PAGE_SHIFT_4K));
diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
index 14ad0489a6..0ed4a9e86d 100644
--- a/xen/include/asm-x86/iommu.h
+++ b/xen/include/asm-x86/iommu.h
@@ -30,6 +30,18 @@ struct g2m_ioport {
unsigned int np;
};
+#define IOMMU_PAGE_SHIFT 12
+#define IOMMU_PAGE_SIZE (1 << IOMMU_PAGE_SHIFT)
+#define IOMMU_PAGE_MASK (~(IOMMU_PAGE_SIZE - 1))
+
+typedef uint64_t daddr_t;
+
+#define __dfn_to_daddr(dfn) ((daddr_t)(dfn) << IOMMU_PAGE_SHIFT)
+#define __daddr_to_dfn(daddr) ((daddr) >> IOMMU_PAGE_SHIFT)
+
+#define dfn_to_daddr(dfn) __dfn_to_daddr(dfn_x(dfn))
+#define daddr_to_dfn(daddr) _dfn(__daddr_to_dfn(daddr))
+
struct arch_iommu
{
u64 pgd_maddr; /* io page directory machine address */
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 57c4e81ec6..290e0aada6 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -23,11 +23,25 @@
#include <xen/page-defs.h>
#include <xen/spinlock.h>
#include <xen/pci.h>
+#include <xen/typesafe.h>
#include <public/hvm/ioreq.h>
#include <public/domctl.h>
#include <asm/device.h>
#include <asm/iommu.h>
+TYPE_SAFE(uint64_t, dfn);
+#define PRI_dfn PRIx64
+#define INVALID_DFN _dfn(~0ULL)
+
+#ifndef dfn_t
+#define dfn_t /* Grep fodder: dfn_t, _dfn() and dfn_x() are defined above */
+#define _dfn
+#define dfn_x
+#undef dfn_t
+#undef _dfn
+#undef dfn_x
+#endif
+
extern bool_t iommu_enable, iommu_enabled;
extern bool_t force_iommu, iommu_verbose;
extern bool_t iommu_workaround_bios_bug, iommu_igfx;
@@ -64,9 +78,9 @@ void iommu_teardown(struct domain *d);
#define IOMMUF_readable (1u<<_IOMMUF_readable)
#define _IOMMUF_writable 1
#define IOMMUF_writable (1u<<_IOMMUF_writable)
-int __must_check iommu_map_page(struct domain *d, unsigned long gfn,
+int __must_check iommu_map_page(struct domain *d, unsigned long dfn,
unsigned long mfn, unsigned int flags);
-int __must_check iommu_unmap_page(struct domain *d, unsigned long gfn);
+int __must_check iommu_unmap_page(struct domain *d, unsigned long dfn);
enum iommu_feature
{
@@ -154,9 +168,9 @@ struct iommu_ops {
#endif /* HAS_PCI */
void (*teardown)(struct domain *d);
- int __must_check (*map_page)(struct domain *d, unsigned long gfn,
+ int __must_check (*map_page)(struct domain *d, unsigned long dfn,
unsigned long mfn, unsigned int flags);
- int __must_check (*unmap_page)(struct domain *d, unsigned long gfn);
+ int __must_check (*unmap_page)(struct domain *d, unsigned long dfn);
void (*free_page_table)(struct page_info *);
#ifdef CONFIG_X86
void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned int value);
@@ -167,7 +181,7 @@ struct iommu_ops {
void (*resume)(void);
void (*share_p2m)(struct domain *d);
void (*crash_shutdown)(void);
- int __must_check (*iotlb_flush)(struct domain *d, unsigned long gfn,
+ int __must_check (*iotlb_flush)(struct domain *d, unsigned long dfn,
unsigned int page_count);
int __must_check (*iotlb_flush_all)(struct domain *d);
int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
@@ -189,7 +203,7 @@ int iommu_do_pci_domctl(struct xen_domctl *, struct domain *d,
int iommu_do_domctl(struct xen_domctl *, struct domain *d,
XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
-int __must_check iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+int __must_check iommu_iotlb_flush(struct domain *d, unsigned long dfn,
unsigned int page_count);
int __must_check iommu_iotlb_flush_all(struct domain *d);
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 9595539aee..054d02e6c0 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -26,6 +26,11 @@
* A linear idea of a guest physical address space. For an auto-translated
* guest, pfn == gfn while for a non-translated guest, pfn != gfn.
*
+ * dfn: Device DMA Frame Number (definitions in include/xen/iommu.h)
+ * The linear frame numbers of device DMA address space. All initiators for
+ * (i.e. all devices assigned to) a guest share a single DMA address space
+ * and, by default, Xen will ensure dfn == pfn.
+ *
* WARNING: Some of these terms have changed over time while others have been
* used inconsistently, meaning that a lot of existing code does not match the
* definitions above. New code should use these terms as described here, and
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v14 2/9] iommu: make use of type-safe DFN and MFN in exported functions
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
2018-10-04 10:45 ` [PATCH v14 1/9] iommu: introduce the concept of DFN Paul Durrant
@ 2018-10-04 10:45 ` Paul Durrant
2018-10-04 15:14 ` George Dunlap
2018-10-04 10:45 ` [PATCH v14 3/9] iommu: push use of type-safe DFN and MFN into iommu_ops Paul Durrant
` (6 subsequent siblings)
8 siblings, 1 reply; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, George Dunlap,
Andrew Cooper, Ian Jackson, Tim Deegan, Paul Durrant,
Jun Nakajima
This patch modifies the declaration of the entry points to the IOMMU
sub-system to use dfn_t and mfn_t in place of unsigned long. A subsequent
patch will similarly modify the methods in the iommu_ops structure.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Roger Pau Monne <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
v9:
- Re-base.
v7:
- Re-base and re-name BFN -> DFN.
- Added Jan's A-b since re-naming was purely mechanical.
v6:
- Re-base.
v3:
- Removed most of the uses of an intermediate 'frame' variable.
v2:
- Addressed comments from Jan.
- Use intermediate 'frame' variable to avoid directly encapsulating
mfn or gfn values as dfns.
---
xen/arch/arm/p2m.c | 3 ++-
xen/arch/x86/mm.c | 10 ++++----
xen/arch/x86/mm/p2m-ept.c | 10 +++++---
xen/arch/x86/mm/p2m-pt.c | 45 ++++++++++++++++++++---------------
xen/arch/x86/mm/p2m.c | 16 ++++++++-----
xen/arch/x86/x86_64/mm.c | 5 ++--
xen/common/grant_table.c | 12 +++++-----
xen/common/memory.c | 4 ++--
xen/drivers/passthrough/iommu.c | 25 ++++++++++---------
xen/drivers/passthrough/vtd/x86/vtd.c | 1 -
xen/drivers/passthrough/x86/iommu.c | 3 ++-
xen/include/xen/iommu.h | 14 +++++++----
12 files changed, 85 insertions(+), 63 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 1364e5960a..0db12b01f1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -957,7 +957,8 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
if ( need_iommu(p2m->domain) &&
(lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
- rc = iommu_iotlb_flush(p2m->domain, gfn_x(sgfn), 1UL << page_order);
+ rc = iommu_iotlb_flush(p2m->domain, _dfn(gfn_x(sgfn)),
+ 1UL << page_order);
else
rc = 0;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 02abd061be..546d98c864 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2789,14 +2789,14 @@ static int _get_page_type(struct page_info *page, unsigned long type,
struct domain *d = page_get_owner(page);
if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) )
{
- gfn_t gfn = _gfn(mfn_to_gmfn(d, mfn_x(page_to_mfn(page))));
+ mfn_t mfn = page_to_mfn(page);
if ( (x & PGT_type_mask) == PGT_writable_page )
- iommu_ret = iommu_unmap_page(d, gfn_x(gfn));
+ iommu_ret = iommu_unmap_page(d, _dfn(mfn_x(mfn)));
else if ( type == PGT_writable_page )
- iommu_ret = iommu_map_page(d, gfn_x(gfn),
- mfn_x(page_to_mfn(page)),
- IOMMUF_readable|IOMMUF_writable);
+ iommu_ret = iommu_map_page(d, _dfn(mfn_x(mfn)), mfn,
+ IOMMUF_readable |
+ IOMMUF_writable);
}
}
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index d376966560..e0eb85bc3d 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -881,15 +881,19 @@ out:
rc = iommu_pte_flush(d, gfn, &ept_entry->epte, order, vtd_pte_present);
else
{
+ dfn_t dfn = _dfn(gfn);
+
if ( iommu_flags )
for ( i = 0; i < (1 << order); i++ )
{
- rc = iommu_map_page(d, gfn + i, mfn_x(mfn) + i, iommu_flags);
+ rc = iommu_map_page(d, dfn_add(dfn, i),
+ mfn_add(mfn, i), iommu_flags);
if ( unlikely(rc) )
{
while ( i-- )
/* If statement to satisfy __must_check. */
- if ( iommu_unmap_page(p2m->domain, gfn + i) )
+ if ( iommu_unmap_page(p2m->domain,
+ dfn_add(dfn, i)) )
continue;
break;
@@ -898,7 +902,7 @@ out:
else
for ( i = 0; i < (1 << order); i++ )
{
- ret = iommu_unmap_page(d, gfn + i);
+ ret = iommu_unmap_page(d, dfn_add(dfn, i));
if ( !rc )
rc = ret;
}
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 33dd12969c..0569f1de80 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -688,29 +688,36 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
if ( iommu_old_flags )
amd_iommu_flush_pages(p2m->domain, gfn, page_order);
}
- else if ( iommu_pte_flags )
- for ( i = 0; i < (1UL << page_order); i++ )
- {
- rc = iommu_map_page(p2m->domain, gfn + i, mfn_x(mfn) + i,
- iommu_pte_flags);
- if ( unlikely(rc) )
+ else
+ {
+ dfn_t dfn = _dfn(gfn);
+
+ if ( iommu_pte_flags )
+ for ( i = 0; i < (1UL << page_order); i++ )
{
- while ( i-- )
- /* If statement to satisfy __must_check. */
- if ( iommu_unmap_page(p2m->domain, gfn + i) )
- continue;
+ rc = iommu_map_page(p2m->domain, dfn_add(dfn, i),
+ mfn_add(mfn, i), iommu_pte_flags);
+ if ( unlikely(rc) )
+ {
+ while ( i-- )
+ /* If statement to satisfy __must_check. */
+ if ( iommu_unmap_page(p2m->domain,
+ dfn_add(dfn, i)) )
+ continue;
- break;
+ break;
+ }
}
- }
- else
- for ( i = 0; i < (1UL << page_order); i++ )
- {
- int ret = iommu_unmap_page(p2m->domain, gfn + i);
+ else
+ for ( i = 0; i < (1UL << page_order); i++ )
+ {
+ int ret = iommu_unmap_page(p2m->domain,
+ dfn_add(dfn, i));
- if ( !rc )
- rc = ret;
- }
+ if ( !rc )
+ rc = ret;
+ }
+ }
}
/*
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index d6a8810c96..e5c06e22c7 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -723,9 +723,11 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn_l, unsigned long mfn,
if ( need_iommu(p2m->domain) )
{
+ dfn_t dfn = _dfn(mfn);
+
for ( i = 0; i < (1 << page_order); i++ )
{
- int ret = iommu_unmap_page(p2m->domain, mfn + i);
+ int ret = iommu_unmap_page(p2m->domain, dfn_add(dfn, i));
if ( !rc )
rc = ret;
@@ -782,16 +784,17 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
{
if ( need_iommu(d) && t == p2m_ram_rw )
{
+ dfn_t dfn = _dfn(mfn_x(mfn));
+
for ( i = 0; i < (1 << page_order); i++ )
{
- rc = iommu_map_page(d, mfn_x(mfn_add(mfn, i)),
- mfn_x(mfn_add(mfn, i)),
+ rc = iommu_map_page(d, dfn_add(dfn, i), mfn_add(mfn, i),
IOMMUF_readable|IOMMUF_writable);
if ( rc != 0 )
{
while ( i-- > 0 )
/* If statement to satisfy __must_check. */
- if ( iommu_unmap_page(d, mfn_x(mfn_add(mfn, i))) )
+ if ( iommu_unmap_page(d, dfn_add(dfn, i)) )
continue;
return rc;
@@ -1170,7 +1173,8 @@ int set_identity_p2m_entry(struct domain *d, unsigned long gfn_l,
{
if ( !need_iommu(d) )
return 0;
- return iommu_map_page(d, gfn_l, gfn_l, IOMMUF_readable|IOMMUF_writable);
+ return iommu_map_page(d, _dfn(gfn_l), _mfn(gfn_l),
+ IOMMUF_readable | IOMMUF_writable);
}
gfn_lock(p2m, gfn, 0);
@@ -1260,7 +1264,7 @@ int clear_identity_p2m_entry(struct domain *d, unsigned long gfn_l)
{
if ( !need_iommu(d) )
return 0;
- return iommu_unmap_page(d, gfn_l);
+ return iommu_unmap_page(d, _dfn(gfn_l));
}
gfn_lock(p2m, gfn, 0);
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 90a648c956..d1fce57432 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1430,13 +1430,14 @@ int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm)
!need_iommu(hardware_domain) )
{
for ( i = spfn; i < epfn; i++ )
- if ( iommu_map_page(hardware_domain, i, i, IOMMUF_readable|IOMMUF_writable) )
+ if ( iommu_map_page(hardware_domain, _dfn(i), _mfn(i),
+ IOMMUF_readable | IOMMUF_writable) )
break;
if ( i != epfn )
{
while (i-- > old_max)
/* If statement to satisfy __must_check. */
- if ( iommu_unmap_page(hardware_domain, i) )
+ if ( iommu_unmap_page(hardware_domain, _dfn(i)) )
continue;
goto destroy_m2p;
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 73d3ed3701..2d01cad176 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1143,13 +1143,13 @@ map_grant_ref(
!(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
{
if ( !(kind & MAPKIND_WRITE) )
- err = iommu_map_page(ld, mfn_x(mfn), mfn_x(mfn),
- IOMMUF_readable|IOMMUF_writable);
+ err = iommu_map_page(ld, _dfn(mfn_x(mfn)), mfn,
+ IOMMUF_readable | IOMMUF_writable);
}
else if ( act_pin && !old_pin )
{
if ( !kind )
- err = iommu_map_page(ld, mfn_x(mfn), mfn_x(mfn),
+ err = iommu_map_page(ld, _dfn(mfn_x(mfn)), mfn,
IOMMUF_readable);
}
if ( err )
@@ -1398,10 +1398,10 @@ unmap_common(
kind = mapkind(lgt, rd, op->mfn);
if ( !kind )
- err = iommu_unmap_page(ld, mfn_x(op->mfn));
+ err = iommu_unmap_page(ld, _dfn(mfn_x(op->mfn)));
else if ( !(kind & MAPKIND_WRITE) )
- err = iommu_map_page(ld, mfn_x(op->mfn),
- mfn_x(op->mfn), IOMMUF_readable);
+ err = iommu_map_page(ld, _dfn(mfn_x(op->mfn)), op->mfn,
+ IOMMUF_readable);
double_gt_unlock(lgt, rgt);
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 5c71ce13ce..238a28cabc 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -835,11 +835,11 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
this_cpu(iommu_dont_flush_iotlb) = 0;
- ret = iommu_iotlb_flush(d, xatp->idx - done, done);
+ ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done);
if ( unlikely(ret) && rc >= 0 )
rc = ret;
- ret = iommu_iotlb_flush(d, xatp->gpfn - done, done);
+ ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done);
if ( unlikely(ret) && rc >= 0 )
rc = ret;
}
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 0695eb8d7e..ef9d651317 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -285,7 +285,7 @@ void iommu_domain_destroy(struct domain *d)
arch_iommu_domain_destroy(d);
}
-int iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
+int iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
unsigned int flags)
{
const struct domain_iommu *hd = dom_iommu(d);
@@ -294,13 +294,13 @@ int iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->map_page(d, dfn, mfn, flags);
+ rc = hd->platform_ops->map_page(d, dfn_x(dfn), mfn_x(mfn), flags);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU mapping dfn %#lx to mfn %#lx failed: %d\n",
- d->domain_id, dfn, mfn, rc);
+ "d%d: IOMMU mapping dfn %"PRI_dfn" to mfn %"PRI_mfn" failed: %d\n",
+ d->domain_id, dfn_x(dfn), mfn_x(mfn), rc);
if ( !is_hardware_domain(d) )
domain_crash(d);
@@ -309,7 +309,7 @@ int iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
return rc;
}
-int iommu_unmap_page(struct domain *d, unsigned long dfn)
+int iommu_unmap_page(struct domain *d, dfn_t dfn)
{
const struct domain_iommu *hd = dom_iommu(d);
int rc;
@@ -317,13 +317,13 @@ int iommu_unmap_page(struct domain *d, unsigned long dfn)
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->unmap_page(d, dfn);
+ rc = hd->platform_ops->unmap_page(d, dfn_x(dfn));
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU unmapping dfn %#lx failed: %d\n",
- d->domain_id, dfn, rc);
+ "d%d: IOMMU unmapping dfn %"PRI_dfn" failed: %d\n",
+ d->domain_id, dfn_x(dfn), rc);
if ( !is_hardware_domain(d) )
domain_crash(d);
@@ -349,8 +349,7 @@ static void iommu_free_pagetables(unsigned long unused)
cpumask_cycle(smp_processor_id(), &cpu_online_map));
}
-int iommu_iotlb_flush(struct domain *d, unsigned long dfn,
- unsigned int page_count)
+int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int page_count)
{
const struct domain_iommu *hd = dom_iommu(d);
int rc;
@@ -358,13 +357,13 @@ int iommu_iotlb_flush(struct domain *d, unsigned long dfn,
if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush )
return 0;
- rc = hd->platform_ops->iotlb_flush(d, dfn, page_count);
+ rc = hd->platform_ops->iotlb_flush(d, dfn_x(dfn), page_count);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU IOTLB flush failed: %d, dfn %#lx, page count %u\n",
- d->domain_id, rc, dfn, page_count);
+ "d%d: IOMMU IOTLB flush failed: %d, dfn %"PRI_dfn", page count %u\n",
+ d->domain_id, rc, dfn_x(dfn), page_count);
if ( !is_hardware_domain(d) )
domain_crash(d);
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
index 48e16f956b..ff456e1e70 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -60,4 +60,3 @@ void flush_all_cache()
{
wbinvd();
}
-
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index 2de8822c59..34727b6653 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -241,7 +241,8 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
if ( paging_mode_translate(d) )
rc = set_identity_p2m_entry(d, pfn, p2m_access_rw, 0);
else
- rc = iommu_map_page(d, pfn, pfn, IOMMUF_readable|IOMMUF_writable);
+ rc = iommu_map_page(d, _dfn(pfn), _mfn(pfn),
+ IOMMUF_readable | IOMMUF_writable);
if ( rc )
printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
d->domain_id, rc);
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 290e0aada6..f9d86fc816 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -24,6 +24,7 @@
#include <xen/spinlock.h>
#include <xen/pci.h>
#include <xen/typesafe.h>
+#include <xen/mm.h>
#include <public/hvm/ioreq.h>
#include <public/domctl.h>
#include <asm/device.h>
@@ -42,6 +43,11 @@ TYPE_SAFE(uint64_t, dfn);
#undef dfn_x
#endif
+static inline dfn_t dfn_add(dfn_t dfn, unsigned long i)
+{
+ return _dfn(dfn_x(dfn) + i);
+}
+
extern bool_t iommu_enable, iommu_enabled;
extern bool_t force_iommu, iommu_verbose;
extern bool_t iommu_workaround_bios_bug, iommu_igfx;
@@ -78,9 +84,9 @@ void iommu_teardown(struct domain *d);
#define IOMMUF_readable (1u<<_IOMMUF_readable)
#define _IOMMUF_writable 1
#define IOMMUF_writable (1u<<_IOMMUF_writable)
-int __must_check iommu_map_page(struct domain *d, unsigned long dfn,
- unsigned long mfn, unsigned int flags);
-int __must_check iommu_unmap_page(struct domain *d, unsigned long dfn);
+int __must_check iommu_map_page(struct domain *d, dfn_t dfn,
+ mfn_t mfn, unsigned int flags);
+int __must_check iommu_unmap_page(struct domain *d, dfn_t dfn);
enum iommu_feature
{
@@ -203,7 +209,7 @@ int iommu_do_pci_domctl(struct xen_domctl *, struct domain *d,
int iommu_do_domctl(struct xen_domctl *, struct domain *d,
XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
-int __must_check iommu_iotlb_flush(struct domain *d, unsigned long dfn,
+int __must_check iommu_iotlb_flush(struct domain *d, dfn_t dfn,
unsigned int page_count);
int __must_check iommu_iotlb_flush_all(struct domain *d);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v14 3/9] iommu: push use of type-safe DFN and MFN into iommu_ops
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
2018-10-04 10:45 ` [PATCH v14 1/9] iommu: introduce the concept of DFN Paul Durrant
2018-10-04 10:45 ` [PATCH v14 2/9] iommu: make use of type-safe DFN and MFN in exported functions Paul Durrant
@ 2018-10-04 10:45 ` Paul Durrant
2018-10-04 13:04 ` Paul Durrant
2018-10-04 10:45 ` [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page() Paul Durrant
` (5 subsequent siblings)
8 siblings, 1 reply; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, George Dunlap
This patch modifies the methods in struct iommu_ops to use type-safe DFN
and MFN. This follows on from the prior patch that modified the functions
exported in xen/iommu.h.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Roger Pau Monne <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
v14:
- Re-base.
v9:
- Re-base.
v7:
- Re-base and re-name BFN -> DFN.
- Added Jan's A-b since re-naming was purely mechanical.
v6:
- Re-base.
v3:
- Remove some use of intermediate 'frame' variables.
v2:
- Addressed comments from Jan.
- Extend use of intermediate 'frame' variable to avoid directly
encapsulating gfn values as bfns (now dfns) or vice versa.
---
xen/drivers/passthrough/amd/iommu_map.c | 46 ++++++++++++++++-----------
xen/drivers/passthrough/arm/smmu.c | 16 +++++-----
xen/drivers/passthrough/iommu.c | 9 +++---
xen/drivers/passthrough/vtd/iommu.c | 26 +++++++--------
xen/drivers/passthrough/x86/iommu.c | 2 +-
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 8 ++---
xen/include/xen/iommu.h | 13 +++++---
7 files changed, 66 insertions(+), 54 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c
index 61ade71850..c89c54fdb6 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -631,7 +631,7 @@ static int update_paging_mode(struct domain *d, unsigned long dfn)
return 0;
}
-int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
+int amd_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
unsigned int flags)
{
bool_t need_flush = 0;
@@ -651,7 +651,8 @@ int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
if ( rc )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %lx\n", dfn);
+ AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %"PRI_dfn"\n",
+ dfn_x(dfn));
domain_crash(d);
return rc;
}
@@ -660,25 +661,27 @@ int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
* we might need a deeper page table for wider dfn now */
if ( is_hvm_domain(d) )
{
- if ( update_paging_mode(d, dfn) )
+ if ( update_paging_mode(d, dfn_x(dfn)) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn);
+ AMD_IOMMU_DEBUG("Update page mode failed dfn = %"PRI_dfn"\n",
+ dfn_x(dfn));
domain_crash(d);
return -EFAULT;
}
}
- if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) )
+ if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn);
+ AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
+ dfn_x(dfn));
domain_crash(d);
return -EFAULT;
}
/* Install 4k mapping first */
- need_flush = set_iommu_pte_present(pt_mfn[1], dfn, mfn,
+ need_flush = set_iommu_pte_present(pt_mfn[1], dfn_x(dfn), mfn_x(mfn),
IOMMU_PAGING_MODE_LEVEL_1,
!!(flags & IOMMUF_writable),
!!(flags & IOMMUF_readable));
@@ -690,7 +693,7 @@ int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
/* 4K mapping for PV guests never changes,
* no need to flush if we trust non-present bits */
if ( is_hvm_domain(d) )
- amd_iommu_flush_pages(d, dfn, 0);
+ amd_iommu_flush_pages(d, dfn_x(dfn), 0);
for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2;
merge_level <= hd->arch.paging_mode; merge_level++ )
@@ -698,15 +701,16 @@ int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long mfn,
if ( pt_mfn[merge_level] == 0 )
break;
if ( !iommu_update_pde_count(d, pt_mfn[merge_level],
- dfn, mfn, merge_level) )
+ dfn_x(dfn), mfn_x(mfn), merge_level) )
break;
- if ( iommu_merge_pages(d, pt_mfn[merge_level], dfn,
+ if ( iommu_merge_pages(d, pt_mfn[merge_level], dfn_x(dfn),
flags, merge_level) )
{
spin_unlock(&hd->arch.mapping_lock);
AMD_IOMMU_DEBUG("Merge iommu page failed at level %d, "
- "dfn = %lx mfn = %lx\n", merge_level, dfn, mfn);
+ "dfn = %"PRI_dfn" mfn = %"PRI_mfn"\n",
+ merge_level, dfn_x(dfn), mfn_x(mfn));
domain_crash(d);
return -EFAULT;
}
@@ -720,7 +724,7 @@ out:
return 0;
}
-int amd_iommu_unmap_page(struct domain *d, unsigned long dfn)
+int amd_iommu_unmap_page(struct domain *d, dfn_t dfn)
{
unsigned long pt_mfn[7];
struct domain_iommu *hd = dom_iommu(d);
@@ -742,31 +746,33 @@ int amd_iommu_unmap_page(struct domain *d, unsigned long dfn)
* we might need a deeper page table for lager dfn now */
if ( is_hvm_domain(d) )
{
- int rc = update_paging_mode(d, dfn);
+ int rc = update_paging_mode(d, dfn_x(dfn));
if ( rc )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn);
+ AMD_IOMMU_DEBUG("Update page mode failed dfn = %"PRI_dfn"\n",
+ dfn_x(dfn));
if ( rc != -EADDRNOTAVAIL )
domain_crash(d);
return rc;
}
}
- if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) )
+ if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn);
+ AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
+ dfn_x(dfn));
domain_crash(d);
return -EFAULT;
}
/* mark PTE as 'page not present' */
- clear_iommu_pte_present(pt_mfn[1], dfn);
+ clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn));
spin_unlock(&hd->arch.mapping_lock);
- amd_iommu_flush_pages(d, dfn, 0);
+ amd_iommu_flush_pages(d, dfn_x(dfn), 0);
return 0;
}
@@ -787,7 +793,9 @@ int amd_iommu_reserve_domain_unity_map(struct domain *domain,
gfn = phys_addr >> PAGE_SHIFT;
for ( i = 0; i < npages; i++ )
{
- rt = amd_iommu_map_page(domain, gfn +i, gfn +i, flags);
+ unsigned long frame = gfn + i;
+
+ rt = amd_iommu_map_page(domain, _dfn(frame), _mfn(frame), flags);
if ( rt != 0 )
return rt;
}
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 1eda96a72a..53e5823d05 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2550,8 +2550,7 @@ static int __must_check arm_smmu_iotlb_flush_all(struct domain *d)
return 0;
}
-static int __must_check arm_smmu_iotlb_flush(struct domain *d,
- unsigned long dfn,
+static int __must_check arm_smmu_iotlb_flush(struct domain *d, dfn_t dfn,
unsigned int page_count)
{
/* ARM SMMU v1 doesn't have flush by VMA and VMID */
@@ -2748,8 +2747,8 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
xfree(xen_domain);
}
-static int __must_check arm_smmu_map_page(struct domain *d, unsigned long dfn,
- unsigned long mfn, unsigned int flags)
+static int __must_check arm_smmu_map_page(struct domain *d, dfn_t dfn,
+ mfn_t mfn, unsigned int flags)
{
p2m_type_t t;
@@ -2762,7 +2761,7 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long dfn,
* function should only be used by gnttab code with gfn == mfn == dfn.
*/
BUG_ON(!is_domain_direct_mapped(d));
- BUG_ON(mfn != dfn);
+ BUG_ON(mfn_x(mfn) != dfn_x(dfn));
/* We only support readable and writable flags */
if (!(flags & (IOMMUF_readable | IOMMUF_writable)))
@@ -2774,10 +2773,11 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long dfn,
* The function guest_physmap_add_entry replaces the current mapping
* if there is already one...
*/
- return guest_physmap_add_entry(d, _gfn(dfn), _mfn(dfn), 0, t);
+ return guest_physmap_add_entry(d, _gfn(dfn_x(dfn)), _mfn(dfn_x(dfn)),
+ 0, t);
}
-static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long dfn)
+static int __must_check arm_smmu_unmap_page(struct domain *d, dfn_t dfn)
{
/*
* This function should only be used by gnttab code when the domain
@@ -2786,7 +2786,7 @@ static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long dfn)
if ( !is_domain_direct_mapped(d) )
return -EINVAL;
- return guest_physmap_remove_page(d, _gfn(dfn), _mfn(dfn), 0);
+ return guest_physmap_remove_page(d, _gfn(dfn_x(dfn)), _mfn(dfn_x(dfn)), 0);
}
static const struct iommu_ops arm_smmu_iommu_ops = {
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index ef9d651317..d61fbbf439 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -224,7 +224,8 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
== PGT_writable_page) )
mapping |= IOMMUF_writable;
- ret = hd->platform_ops->map_page(d, dfn, mfn, mapping);
+ ret = hd->platform_ops->map_page(d, _dfn(dfn), _mfn(mfn),
+ mapping);
if ( !rc )
rc = ret;
@@ -294,7 +295,7 @@ int iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->map_page(d, dfn_x(dfn), mfn_x(mfn), flags);
+ rc = hd->platform_ops->map_page(d, dfn, mfn, flags);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
@@ -317,7 +318,7 @@ int iommu_unmap_page(struct domain *d, dfn_t dfn)
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->unmap_page(d, dfn_x(dfn));
+ rc = hd->platform_ops->unmap_page(d, dfn);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
@@ -357,7 +358,7 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int page_count)
if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush )
return 0;
- rc = hd->platform_ops->iotlb_flush(d, dfn_x(dfn), page_count);
+ rc = hd->platform_ops->iotlb_flush(d, dfn, page_count);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 485704bcce..7e185c43f3 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -584,8 +584,7 @@ static int __must_check iommu_flush_all(void)
return rc;
}
-static int __must_check iommu_flush_iotlb(struct domain *d,
- unsigned long dfn,
+static int __must_check iommu_flush_iotlb(struct domain *d, dfn_t dfn,
bool_t dma_old_pte_present,
unsigned int page_count)
{
@@ -612,12 +611,12 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
if ( iommu_domid == -1 )
continue;
- if ( page_count != 1 || dfn == dfn_x(INVALID_DFN) )
+ if ( page_count != 1 || dfn_eq(dfn, INVALID_DFN) )
rc = iommu_flush_iotlb_dsi(iommu, iommu_domid,
0, flush_dev_iotlb);
else
rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
- __dfn_to_daddr(dfn),
+ dfn_to_daddr(dfn),
PAGE_ORDER_4K,
!dma_old_pte_present,
flush_dev_iotlb);
@@ -633,7 +632,7 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
}
static int __must_check iommu_flush_iotlb_pages(struct domain *d,
- unsigned long dfn,
+ dfn_t dfn,
unsigned int page_count)
{
return iommu_flush_iotlb(d, dfn, 1, page_count);
@@ -641,7 +640,7 @@ static int __must_check iommu_flush_iotlb_pages(struct domain *d,
static int __must_check iommu_flush_iotlb_all(struct domain *d)
{
- return iommu_flush_iotlb(d, dfn_x(INVALID_DFN), 0, 0);
+ return iommu_flush_iotlb(d, INVALID_DFN, 0, 0);
}
/* clear one page's page table */
@@ -676,7 +675,7 @@ static int __must_check dma_pte_clear_one(struct domain *domain, u64 addr)
iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
if ( !this_cpu(iommu_dont_flush_iotlb) )
- rc = iommu_flush_iotlb_pages(domain, addr >> PAGE_SHIFT_4K, 1);
+ rc = iommu_flush_iotlb_pages(domain, daddr_to_dfn(addr), 1);
unmap_vtd_domain_page(page);
@@ -1763,8 +1762,7 @@ static void iommu_domain_teardown(struct domain *d)
}
static int __must_check intel_iommu_map_page(struct domain *d,
- unsigned long dfn,
- unsigned long mfn,
+ dfn_t dfn, mfn_t mfn,
unsigned int flags)
{
struct domain_iommu *hd = dom_iommu(d);
@@ -1782,16 +1780,16 @@ static int __must_check intel_iommu_map_page(struct domain *d,
spin_lock(&hd->arch.mapping_lock);
- pg_maddr = addr_to_dma_page_maddr(d, __dfn_to_daddr(dfn), 1);
+ pg_maddr = addr_to_dma_page_maddr(d, dfn_to_daddr(dfn), 1);
if ( pg_maddr == 0 )
{
spin_unlock(&hd->arch.mapping_lock);
return -ENOMEM;
}
page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
- pte = page + (dfn & LEVEL_MASK);
+ pte = page + (dfn_x(dfn) & LEVEL_MASK);
old = *pte;
- dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
+ dma_set_pte_addr(new, mfn_to_maddr(mfn));
dma_set_pte_prot(new,
((flags & IOMMUF_readable) ? DMA_PTE_READ : 0) |
((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
@@ -1819,13 +1817,13 @@ static int __must_check intel_iommu_map_page(struct domain *d,
}
static int __must_check intel_iommu_unmap_page(struct domain *d,
- unsigned long dfn)
+ dfn_t dfn)
{
/* Do nothing if hardware domain and iommu supports pass thru. */
if ( iommu_hwdom_passthrough && is_hardware_domain(d) )
return 0;
- return dma_pte_clear_one(d, __dfn_to_daddr(dfn));
+ return dma_pte_clear_one(d, dfn_to_daddr(dfn));
}
int iommu_pte_flush(struct domain *d, uint64_t dfn, uint64_t *pte,
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index 34727b6653..f410717a59 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -68,7 +68,7 @@ int arch_iommu_populate_page_table(struct domain *d)
{
ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
BUG_ON(SHARED_M2P(gfn));
- rc = hd->platform_ops->map_page(d, gfn, mfn,
+ rc = hd->platform_ops->map_page(d, _dfn(gfn), _mfn(mfn),
IOMMUF_readable |
IOMMUF_writable);
}
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
index 99bc21c7b3..3083d625bd 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -52,9 +52,9 @@ int amd_iommu_init(void);
int amd_iommu_update_ivrs_mapping_acpi(void);
/* mapping functions */
-int __must_check amd_iommu_map_page(struct domain *d, unsigned long gfn,
- unsigned long mfn, unsigned int flags);
-int __must_check amd_iommu_unmap_page(struct domain *d, unsigned long gfn);
+int __must_check amd_iommu_map_page(struct domain *d, dfn_t dfn,
+ mfn_t mfn, unsigned int flags);
+int __must_check amd_iommu_unmap_page(struct domain *d, dfn_t dfn);
u64 amd_iommu_get_next_table_from_pte(u32 *entry);
int __must_check amd_iommu_alloc_root(struct domain_iommu *hd);
int amd_iommu_reserve_domain_unity_map(struct domain *domain,
@@ -77,7 +77,7 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id, u64 gcr3,
/* send cmd to iommu */
void amd_iommu_flush_all_pages(struct domain *d);
-void amd_iommu_flush_pages(struct domain *d, unsigned long gfn,
+void amd_iommu_flush_pages(struct domain *d, unsigned long dfn,
unsigned int order);
void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
uint64_t gaddr, unsigned int order);
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index f9d86fc816..7313957c81 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -48,6 +48,11 @@ static inline dfn_t dfn_add(dfn_t dfn, unsigned long i)
return _dfn(dfn_x(dfn) + i);
}
+static inline bool_t dfn_eq(dfn_t x, dfn_t y)
+{
+ return dfn_x(x) == dfn_x(y);
+}
+
extern bool_t iommu_enable, iommu_enabled;
extern bool_t force_iommu, iommu_verbose;
extern bool_t iommu_workaround_bios_bug, iommu_igfx;
@@ -174,9 +179,9 @@ struct iommu_ops {
#endif /* HAS_PCI */
void (*teardown)(struct domain *d);
- int __must_check (*map_page)(struct domain *d, unsigned long dfn,
- unsigned long mfn, unsigned int flags);
- int __must_check (*unmap_page)(struct domain *d, unsigned long dfn);
+ int __must_check (*map_page)(struct domain *d, dfn_t dfn, mfn_t mfn,
+ unsigned int flags);
+ int __must_check (*unmap_page)(struct domain *d, dfn_t dfn);
void (*free_page_table)(struct page_info *);
#ifdef CONFIG_X86
void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned int value);
@@ -187,7 +192,7 @@ struct iommu_ops {
void (*resume)(void);
void (*share_p2m)(struct domain *d);
void (*crash_shutdown)(void);
- int __must_check (*iotlb_flush)(struct domain *d, unsigned long dfn,
+ int __must_check (*iotlb_flush)(struct domain *d, dfn_t dfn,
unsigned int page_count);
int __must_check (*iotlb_flush_all)(struct domain *d);
int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
` (2 preceding siblings ...)
2018-10-04 10:45 ` [PATCH v14 3/9] iommu: push use of type-safe DFN and MFN into iommu_ops Paul Durrant
@ 2018-10-04 10:45 ` Paul Durrant
2018-10-04 16:29 ` George Dunlap
2018-10-04 10:45 ` [PATCH v14 5/9] memory: add check_get_page_from_gfn() as a wrapper Paul Durrant
` (4 subsequent siblings)
8 siblings, 1 reply; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Paul Durrant, Jun Nakajima
This patch removes the implicit domain_crash() from iommu_map(),
unmap_page() and iommu_iotlb_flush() and turns them into straightforward
wrappers that check the existence of the relevant iommu_op and call
through to it. This makes them usable by PV IOMMU code to be delivered in
future patches.
This patch adds a helper macro, domu_crash(), that will only invoke
domain_crash() if the domain is not the hardware domain and modifies
callers of iommu_map(), unmap_page() and iommu_iotlb_flush() to use this
should an operation fail.
NOTE: This patch includes one bit of clean-up in set_identity_p2m_entry()
replacing use of p2m->domain with the domain pointer passed into the
function.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Roger Pau Monne <roger.pau@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
v7:
- Re-base and re-name BFN -> DFN.
- Move domu_crash() outside double locked region in grant_table.c.
- Added Jan's R-b.
v6:
- Introduce domu_crash() (idea suggested by Kevin, name suggested by Jan)
to crash non-hardware domains.
- Dropped Wei's and George's R-b because of change.
v2:
- New in v2.
---
xen/arch/arm/p2m.c | 4 ++++
xen/arch/x86/mm.c | 3 +++
xen/arch/x86/mm/p2m-ept.c | 3 +++
xen/arch/x86/mm/p2m-pt.c | 3 +++
xen/arch/x86/mm/p2m.c | 22 ++++++++++++++++++----
xen/common/grant_table.c | 4 ++++
xen/common/memory.c | 3 +++
xen/drivers/passthrough/iommu.c | 12 ------------
xen/drivers/passthrough/x86/iommu.c | 4 ++++
xen/include/xen/sched.h | 5 +++++
10 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 0db12b01f1..1c79ff7ade 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -957,8 +957,12 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
if ( need_iommu(p2m->domain) &&
(lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
+ {
rc = iommu_iotlb_flush(p2m->domain, _dfn(gfn_x(sgfn)),
1UL << page_order);
+ if ( unlikely(rc) )
+ domu_crash(p2m->domain);
+ }
else
rc = 0;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 546d98c864..5a7c5335a2 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2797,6 +2797,9 @@ static int _get_page_type(struct page_info *page, unsigned long type,
iommu_ret = iommu_map_page(d, _dfn(mfn_x(mfn)), mfn,
IOMMUF_readable |
IOMMUF_writable);
+
+ if ( unlikely(iommu_ret) )
+ domu_crash(d);
}
}
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index e0eb85bc3d..cd4cb5752a 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -906,6 +906,9 @@ out:
if ( !rc )
rc = ret;
}
+
+ if ( unlikely(rc) )
+ domu_crash(d);
}
}
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 0569f1de80..4735934492 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -718,6 +718,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
rc = ret;
}
}
+
+ if ( unlikely(rc) )
+ domu_crash(p2m->domain);
}
/*
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index e5c06e22c7..3108fb2b27 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -732,6 +732,9 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn_l, unsigned long mfn,
if ( !rc )
rc = ret;
}
+
+ if ( unlikely(rc) )
+ domu_crash(p2m->domain);
}
return rc;
@@ -797,6 +800,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
if ( iommu_unmap_page(d, dfn_add(dfn, i)) )
continue;
+ domu_crash(d);
return rc;
}
}
@@ -1169,12 +1173,17 @@ int set_identity_p2m_entry(struct domain *d, unsigned long gfn_l,
struct p2m_domain *p2m = p2m_get_hostp2m(d);
int ret;
- if ( !paging_mode_translate(p2m->domain) )
+ if ( !paging_mode_translate(d) )
{
if ( !need_iommu(d) )
return 0;
- return iommu_map_page(d, _dfn(gfn_l), _mfn(gfn_l),
- IOMMUF_readable | IOMMUF_writable);
+
+ ret = iommu_map_page(d, _dfn(gfn_l), _mfn(gfn_l),
+ IOMMUF_readable | IOMMUF_writable);
+ if ( unlikely(ret) )
+ domu_crash(d);
+
+ return ret;
}
gfn_lock(p2m, gfn, 0);
@@ -1264,7 +1273,12 @@ int clear_identity_p2m_entry(struct domain *d, unsigned long gfn_l)
{
if ( !need_iommu(d) )
return 0;
- return iommu_unmap_page(d, _dfn(gfn_l));
+
+ ret = iommu_unmap_page(d, _dfn(gfn_l));
+ if ( unlikely(ret) )
+ domu_crash(d);
+
+ return ret;
}
gfn_lock(p2m, gfn, 0);
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 2d01cad176..0f0b7b1a49 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1155,6 +1155,7 @@ map_grant_ref(
if ( err )
{
double_gt_unlock(lgt, rgt);
+ domu_crash(ld);
rc = GNTST_general_error;
goto undo_out;
}
@@ -1406,7 +1407,10 @@ unmap_common(
double_gt_unlock(lgt, rgt);
if ( err )
+ {
+ domu_crash(ld);
rc = GNTST_general_error;
+ }
}
/* If just unmapped a writable mapping, mark as dirtied */
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 238a28cabc..b567bd46bb 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -842,6 +842,9 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done);
if ( unlikely(ret) && rc >= 0 )
rc = ret;
+
+ if ( unlikely(rc < 0) )
+ domu_crash(d);
}
#endif
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index d61fbbf439..c0dd8e494f 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -302,9 +302,6 @@ int iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
printk(XENLOG_ERR
"d%d: IOMMU mapping dfn %"PRI_dfn" to mfn %"PRI_mfn" failed: %d\n",
d->domain_id, dfn_x(dfn), mfn_x(mfn), rc);
-
- if ( !is_hardware_domain(d) )
- domain_crash(d);
}
return rc;
@@ -325,9 +322,6 @@ int iommu_unmap_page(struct domain *d, dfn_t dfn)
printk(XENLOG_ERR
"d%d: IOMMU unmapping dfn %"PRI_dfn" failed: %d\n",
d->domain_id, dfn_x(dfn), rc);
-
- if ( !is_hardware_domain(d) )
- domain_crash(d);
}
return rc;
@@ -365,9 +359,6 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int page_count)
printk(XENLOG_ERR
"d%d: IOMMU IOTLB flush failed: %d, dfn %"PRI_dfn", page count %u\n",
d->domain_id, rc, dfn_x(dfn), page_count);
-
- if ( !is_hardware_domain(d) )
- domain_crash(d);
}
return rc;
@@ -388,9 +379,6 @@ int iommu_iotlb_flush_all(struct domain *d)
printk(XENLOG_ERR
"d%d: IOMMU IOTLB flush all failed: %d\n",
d->domain_id, rc);
-
- if ( !is_hardware_domain(d) )
- domain_crash(d);
}
return rc;
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index f410717a59..bcf7a2f298 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -107,7 +107,11 @@ int arch_iommu_populate_page_table(struct domain *d)
this_cpu(iommu_dont_flush_iotlb) = 0;
if ( !rc )
+ {
rc = iommu_iotlb_flush_all(d);
+ if ( unlikely(rc) )
+ domain_crash(d);
+ }
if ( rc && rc != -ERESTART )
iommu_teardown(d);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 0ba80cb1a8..f2c594d197 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -616,6 +616,11 @@ void __domain_crash(struct domain *d);
__domain_crash(d); \
} while (0)
+#define domu_crash(d) do { \
+ if ( !is_hardware_domain(d) ) \
+ domain_crash(d); \
+} while (false)
+
/*
* Called from assembly code, with an optional address to help indicate why
* the crash occured. If addr is 0, look up address from last extable
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v14 5/9] memory: add check_get_page_from_gfn() as a wrapper...
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
` (3 preceding siblings ...)
2018-10-04 10:45 ` [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page() Paul Durrant
@ 2018-10-04 10:45 ` Paul Durrant
2018-10-04 16:02 ` George Dunlap
2018-10-04 10:45 ` [PATCH v14 6/9] vtd: add missing check for shared EPT Paul Durrant
` (3 subsequent siblings)
8 siblings, 1 reply; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Paul Durrant
...for some uses of get_page_from_gfn().
There are many occurrences of the following pattern in the code:
q = <readonly look-up> ? P2M_ALLOC : P2M_UNSHARE;
page = get_page_from_gfn(d, gfn, &p2mt, q);
if ( p2m_is_paging(p2mt) )
{
if ( page )
put_page(page);
p2m_mem_paging_populate(d, gfn);
return <-EAGAIN or equivalent>;
}
if ( (q & P2M_UNSHARE) && p2m_is_shared(p2mt) )
{
if ( page )
put_page(page);
return <-EAGAIN or equivalent>;
}
if ( !page )
return <-EINVAL or equivalent>;
There are some small differences between the exact way the occurrences
are coded but the desired semantic is the same.
This patch introduces a new common implementation of this code in
check_get_page_from_gfn() and then converts the various open-coded patterns
into calls to this new function.
NOTE: A forward declaration of p2m_type_t enum has been introduced in
p2m-common.h so that it is possible to declare
check_get_page_from_gfn() there rather than having to add
duplicate declarations in the per-architecture p2m headers.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Roger Pau Monne <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
v13:
- Expanded commit comment.
v11:
- Forward declare p2m_type_t in p2m-common.h to allow the duplicate
declarations of check_get_page_from_gfn() to be removed, and hence add
Jan's R-b.
v10:
- Expand commit comment to point out the reason for the duplicate
declarations of check_get_page_from_gfn().
v9:
- Defer P2M type checks (beyond shared or paging) to the caller.
v7:
- Fix ARM build by introducing p2m_is_readonly() predicate.
- Re-name get_paged_frame() -> check_get_page_from_gfn().
- Adjust default cases of callers switch-ing on return value.
v3:
- Addressed comments from George.
v2:
- New in v2.
---
xen/arch/x86/hvm/emulate.c | 25 +++++++++++-----------
xen/arch/x86/hvm/hvm.c | 14 +------------
xen/common/grant_table.c | 32 ++++++++++++++---------------
xen/common/memory.c | 49 +++++++++++++++++++++++++++++++++++---------
xen/include/asm-arm/p2m.h | 4 ++--
xen/include/asm-x86/p2m.h | 5 ++---
xen/include/xen/p2m-common.h | 6 ++++++
7 files changed, 77 insertions(+), 58 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index eab66eab77..cd1d9a7c57 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -356,22 +356,21 @@ static int hvmemul_acquire_page(unsigned long gmfn, struct page_info **page)
struct domain *curr_d = current->domain;
p2m_type_t p2mt;
- *page = get_page_from_gfn(curr_d, gmfn, &p2mt, P2M_UNSHARE);
-
- if ( *page == NULL )
- return X86EMUL_UNHANDLEABLE;
-
- if ( p2m_is_paging(p2mt) )
+ switch ( check_get_page_from_gfn(curr_d, _gfn(gmfn), false, &p2mt,
+ page) )
{
- put_page(*page);
- p2m_mem_paging_populate(curr_d, gmfn);
- return X86EMUL_RETRY;
- }
+ case 0:
+ break;
- if ( p2m_is_shared(p2mt) )
- {
- put_page(*page);
+ case -EAGAIN:
return X86EMUL_RETRY;
+
+ default:
+ ASSERT_UNREACHABLE();
+ /* Fallthrough */
+
+ case -EINVAL:
+ return X86EMUL_UNHANDLEABLE;
}
/* This code should not be reached if the gmfn is not RAM */
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 51fc3ec07f..fa994a36a4 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2536,20 +2536,8 @@ static void *_hvm_map_guest_frame(unsigned long gfn, bool_t permanent,
struct page_info *page;
struct domain *d = current->domain;
- page = get_page_from_gfn(d, gfn, &p2mt,
- writable ? P2M_UNSHARE : P2M_ALLOC);
- if ( (p2m_is_shared(p2mt) && writable) || !page )
- {
- if ( page )
- put_page(page);
- return NULL;
- }
- if ( p2m_is_paging(p2mt) )
- {
- put_page(page);
- p2m_mem_paging_populate(d, gfn);
+ if ( check_get_page_from_gfn(d, _gfn(gfn), !writable, &p2mt, &page) )
return NULL;
- }
if ( writable )
{
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 0f0b7b1a49..3604a8812c 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -374,25 +374,23 @@ static int get_paged_frame(unsigned long gfn, mfn_t *mfn,
struct page_info **page, bool readonly,
struct domain *rd)
{
- int rc = GNTST_okay;
p2m_type_t p2mt;
+ int rc;
- *mfn = INVALID_MFN;
- *page = get_page_from_gfn(rd, gfn, &p2mt,
- readonly ? P2M_ALLOC : P2M_UNSHARE);
- if ( !*page )
+ rc = check_get_page_from_gfn(rd, _gfn(gfn), readonly, &p2mt, page);
+ switch ( rc )
{
-#ifdef P2M_SHARED_TYPES
- if ( p2m_is_shared(p2mt) )
- return GNTST_eagain;
-#endif
-#ifdef P2M_PAGES_TYPES
- if ( p2m_is_paging(p2mt) )
- {
- p2m_mem_paging_populate(rd, gfn);
- return GNTST_eagain;
- }
-#endif
+ case 0:
+ break;
+
+ case -EAGAIN:
+ return GNTST_eagain;
+
+ default:
+ ASSERT_UNREACHABLE();
+ /* Fallthrough */
+
+ case -EINVAL:
return GNTST_bad_page;
}
@@ -406,7 +404,7 @@ static int get_paged_frame(unsigned long gfn, mfn_t *mfn,
*mfn = page_to_mfn(*page);
- return rc;
+ return GNTST_okay;
}
static inline void
diff --git a/xen/common/memory.c b/xen/common/memory.c
index b567bd46bb..52d55dfed7 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1626,37 +1626,66 @@ void destroy_ring_for_helper(
}
}
-int prepare_ring_for_helper(
- struct domain *d, unsigned long gmfn, struct page_info **_page,
- void **_va)
+/*
+ * Acquire a pointer to struct page_info for a specified doman and GFN,
+ * checking whether the page has been paged out, or needs unsharing.
+ * If the function succeeds then zero is returned, page_p is written
+ * with a pointer to the struct page_info with a reference taken, and
+ * p2mt_p it is written with the P2M type of the page. The caller is
+ * responsible for dropping the reference.
+ * If the function fails then an appropriate errno is returned and the
+ * values referenced by page_p and p2mt_p are undefined.
+ */
+int check_get_page_from_gfn(struct domain *d, gfn_t gfn, bool readonly,
+ p2m_type_t *p2mt_p, struct page_info **page_p)
{
- struct page_info *page;
+ p2m_query_t q = readonly ? P2M_ALLOC : P2M_UNSHARE;
p2m_type_t p2mt;
- void *va;
+ struct page_info *page;
- page = get_page_from_gfn(d, gmfn, &p2mt, P2M_UNSHARE);
+ page = get_page_from_gfn(d, gfn_x(gfn), &p2mt, q);
#ifdef CONFIG_HAS_MEM_PAGING
if ( p2m_is_paging(p2mt) )
{
if ( page )
put_page(page);
- p2m_mem_paging_populate(d, gmfn);
- return -ENOENT;
+
+ p2m_mem_paging_populate(d, gfn_x(gfn));
+ return -EAGAIN;
}
#endif
#ifdef CONFIG_HAS_MEM_SHARING
- if ( p2m_is_shared(p2mt) )
+ if ( (q & P2M_UNSHARE) && p2m_is_shared(p2mt) )
{
if ( page )
put_page(page);
- return -ENOENT;
+
+ return -EAGAIN;
}
#endif
if ( !page )
return -EINVAL;
+ *p2mt_p = p2mt;
+ *page_p = page;
+ return 0;
+}
+
+int prepare_ring_for_helper(
+ struct domain *d, unsigned long gmfn, struct page_info **_page,
+ void **_va)
+{
+ p2m_type_t p2mt;
+ struct page_info *page;
+ void *va;
+ int rc;
+
+ rc = check_get_page_from_gfn(d, _gfn(gmfn), false, &p2mt, &page);
+ if ( rc )
+ return (rc == -EAGAIN) ? -ENOENT : rc;
+
if ( !get_page_type(page, PGT_writable_page) )
{
put_page(page);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 8823707c17..c03557544a 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -110,7 +110,7 @@ struct p2m_domain {
* future, it's possible to use higher value for pseudo-type and don't store
* them in the p2m entry.
*/
-typedef enum {
+enum p2m_type {
p2m_invalid = 0, /* Nothing mapped here */
p2m_ram_rw, /* Normal read/write guest RAM */
p2m_ram_ro, /* Read-only; writes are silently dropped */
@@ -124,7 +124,7 @@ typedef enum {
p2m_iommu_map_rw, /* Read/write iommu mapping */
p2m_iommu_map_ro, /* Read-only iommu mapping */
p2m_max_real_type, /* Types after this won't be store in the p2m */
-} p2m_type_t;
+};
/* We use bitmaps and mask to handle groups of types */
#define p2m_to_mask(_t) (1UL << (_t))
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index be3b6fcaf0..d08c595887 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -52,7 +52,7 @@ extern bool_t opt_hap_1gb, opt_hap_2mb;
* cannot be non-zero, otherwise, hardware generates io page faults when
* device access those pages. Therefore, p2m_ram_rw has to be defined as 0.
*/
-typedef enum {
+enum p2m_type {
p2m_ram_rw = 0, /* Normal read/write guest RAM */
p2m_invalid = 1, /* Nothing mapped here */
p2m_ram_logdirty = 2, /* Temporarily read-only for log-dirty */
@@ -72,7 +72,7 @@ typedef enum {
p2m_ram_broken = 13, /* Broken page, access cause domain crash */
p2m_map_foreign = 14, /* ram pages from foreign domain */
p2m_ioreq_server = 15,
-} p2m_type_t;
+};
/* Modifiers to the query */
typedef unsigned int p2m_query_t;
@@ -503,7 +503,6 @@ static inline struct page_info *get_page_from_gfn(
return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL;
}
-
/* General conversion function from mfn to gfn */
static inline unsigned long mfn_to_gfn(struct domain *d, mfn_t mfn)
{
diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
index 74311950ad..f4d30efe5f 100644
--- a/xen/include/xen/p2m-common.h
+++ b/xen/include/xen/p2m-common.h
@@ -32,5 +32,11 @@ unsigned long
p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn,
unsigned int order);
+typedef enum p2m_type p2m_type_t;
+
+int __must_check check_get_page_from_gfn(struct domain *d, gfn_t gfn,
+ bool readonly, p2m_type_t *p2mt_p,
+ struct page_info **page_p);
+
#endif /* _XEN_P2M_COMMON_H */
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v14 6/9] vtd: add missing check for shared EPT...
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
` (4 preceding siblings ...)
2018-10-04 10:45 ` [PATCH v14 5/9] memory: add check_get_page_from_gfn() as a wrapper Paul Durrant
@ 2018-10-04 10:45 ` Paul Durrant
2018-10-04 10:45 ` [PATCH v14 7/9] vtd: add lookup_page method to iommu_ops Paul Durrant
` (2 subsequent siblings)
8 siblings, 0 replies; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Wei Liu, George Dunlap, Jan Beulich
...in intel_iommu_unmap_page().
This patch also includes some non-functional modifications in
intel_iommu_map_page().
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
---
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: George Dunlap <george.dunlap@citrix.com>
v8:
- New in v8. (Split from the next patch in the series as requested by
Kevin).
---
xen/drivers/passthrough/vtd/iommu.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7e185c43f3..9187d50730 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1766,7 +1766,7 @@ static int __must_check intel_iommu_map_page(struct domain *d,
unsigned int flags)
{
struct domain_iommu *hd = dom_iommu(d);
- struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
+ struct dma_pte *page, *pte, old, new = {};
u64 pg_maddr;
int rc = 0;
@@ -1781,14 +1781,16 @@ static int __must_check intel_iommu_map_page(struct domain *d,
spin_lock(&hd->arch.mapping_lock);
pg_maddr = addr_to_dma_page_maddr(d, dfn_to_daddr(dfn), 1);
- if ( pg_maddr == 0 )
+ if ( !pg_maddr )
{
spin_unlock(&hd->arch.mapping_lock);
return -ENOMEM;
}
+
page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
- pte = page + (dfn_x(dfn) & LEVEL_MASK);
+ pte = &page[dfn_x(dfn) & LEVEL_MASK];
old = *pte;
+
dma_set_pte_addr(new, mfn_to_maddr(mfn));
dma_set_pte_prot(new,
((flags & IOMMUF_readable) ? DMA_PTE_READ : 0) |
@@ -1804,6 +1806,7 @@ static int __must_check intel_iommu_map_page(struct domain *d,
unmap_vtd_domain_page(page);
return 0;
}
+
*pte = new;
iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
@@ -1819,6 +1822,10 @@ static int __must_check intel_iommu_map_page(struct domain *d,
static int __must_check intel_iommu_unmap_page(struct domain *d,
dfn_t dfn)
{
+ /* Do nothing if VT-d shares EPT page table */
+ if ( iommu_use_hap_pt(d) )
+ return 0;
+
/* Do nothing if hardware domain and iommu supports pass thru. */
if ( iommu_hwdom_passthrough && is_hardware_domain(d) )
return 0;
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v14 7/9] vtd: add lookup_page method to iommu_ops
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
` (5 preceding siblings ...)
2018-10-04 10:45 ` [PATCH v14 6/9] vtd: add missing check for shared EPT Paul Durrant
@ 2018-10-04 10:45 ` Paul Durrant
2018-12-24 15:15 ` Andrew Cooper
2018-10-04 10:45 ` [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt() Paul Durrant
2018-10-04 10:45 ` [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync() Paul Durrant
8 siblings, 1 reply; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Wei Liu, George Dunlap
This patch adds a new method to the VT-d IOMMU implementation to find the
MFN currently mapped by the specified DFN along with a wrapper function
in generic IOMMU code to call the implementation if it exists.
NOTE: This patch only adds a Xen-internal interface. This will be used by
a subsequent patch.
Another subsequent patch will add similar functionality for AMD
IOMMUs.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
---
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
v11:
- Fold in patch to change failure semantics (already ack-ed by Kevin).
v10:
- Adjust the locking comment.
v9:
- Add comment about locking in xen/iommu.h.
v8:
- Remove clean-up as this is now done by a prior patch.
- Make intel_iommu_lookup_page() return dfn value if using shared EPT
or iommu_passthrough is set, as requested by Kevin.
v7:
- Re-base and re-name BFN -> DFN.
- Add missing checks for shared EPT and iommu_passthrough.
- Remove unnecessary initializers and use array-style dereference.
- Drop Wei's R-b because of code churn.
v3:
- Addressed comments from George.
v2:
- Addressed some comments from Jan.
---
xen/drivers/passthrough/iommu.c | 11 ++++++++++
xen/drivers/passthrough/vtd/iommu.c | 41 +++++++++++++++++++++++++++++++++++++
xen/drivers/passthrough/vtd/iommu.h | 3 +++
xen/include/xen/iommu.h | 10 +++++++++
4 files changed, 65 insertions(+)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index c0dd8e494f..d51fffae40 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -327,6 +327,17 @@ int iommu_unmap_page(struct domain *d, dfn_t dfn)
return rc;
}
+int iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
+ unsigned int *flags)
+{
+ const struct domain_iommu *hd = dom_iommu(d);
+
+ if ( !iommu_enabled || !hd->platform_ops )
+ return -EOPNOTSUPP;
+
+ return hd->platform_ops->lookup_page(d, dfn, mfn, flags);
+}
+
static void iommu_free_pagetables(unsigned long unused)
{
do {
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 9187d50730..f94b522c73 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1833,6 +1833,46 @@ static int __must_check intel_iommu_unmap_page(struct domain *d,
return dma_pte_clear_one(d, dfn_to_daddr(dfn));
}
+static int intel_iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
+ unsigned int *flags)
+{
+ struct domain_iommu *hd = dom_iommu(d);
+ struct dma_pte *page, val;
+ u64 pg_maddr;
+
+ /*
+ * If VT-d shares EPT page table or if the domain is the hardware
+ * domain and iommu_passthrough is set then pass back the dfn.
+ */
+ if ( iommu_use_hap_pt(d) ||
+ (iommu_hwdom_passthrough && is_hardware_domain(d)) )
+ return -EOPNOTSUPP;
+
+ spin_lock(&hd->arch.mapping_lock);
+
+ pg_maddr = addr_to_dma_page_maddr(d, dfn_to_daddr(dfn), 0);
+ if ( !pg_maddr )
+ {
+ spin_unlock(&hd->arch.mapping_lock);
+ return -ENOMEM;
+ }
+
+ page = map_vtd_domain_page(pg_maddr);
+ val = page[dfn_x(dfn) & LEVEL_MASK];
+
+ unmap_vtd_domain_page(page);
+ spin_unlock(&hd->arch.mapping_lock);
+
+ if ( !dma_pte_present(val) )
+ return -ENOENT;
+
+ *mfn = maddr_to_mfn(dma_pte_addr(val));
+ *flags = dma_pte_read(val) ? IOMMUF_readable : 0;
+ *flags |= dma_pte_write(val) ? IOMMUF_writable : 0;
+
+ return 0;
+}
+
int iommu_pte_flush(struct domain *d, uint64_t dfn, uint64_t *pte,
int order, int present)
{
@@ -2658,6 +2698,7 @@ const struct iommu_ops intel_iommu_ops = {
.teardown = iommu_domain_teardown,
.map_page = intel_iommu_map_page,
.unmap_page = intel_iommu_unmap_page,
+ .lookup_page = intel_iommu_lookup_page,
.free_page_table = iommu_free_page_table,
.reassign_device = reassign_device_ownership,
.get_device_group_id = intel_iommu_group_id,
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
index 72c1a2e3cd..47bdfcb5ea 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -272,6 +272,9 @@ struct dma_pte {
#define dma_set_pte_prot(p, prot) do { \
(p).val = ((p).val & ~DMA_PTE_PROT) | ((prot) & DMA_PTE_PROT); \
} while (0)
+#define dma_pte_prot(p) ((p).val & DMA_PTE_PROT)
+#define dma_pte_read(p) (dma_pte_prot(p) & DMA_PTE_READ)
+#define dma_pte_write(p) (dma_pte_prot(p) & DMA_PTE_WRITE)
#define dma_pte_addr(p) ((p).val & PADDR_MASK & PAGE_MASK_4K)
#define dma_set_pte_addr(p, addr) do {\
(p).val |= ((addr) & PAGE_MASK_4K); } while (0)
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 7313957c81..9ae8321bb4 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -92,6 +92,8 @@ void iommu_teardown(struct domain *d);
int __must_check iommu_map_page(struct domain *d, dfn_t dfn,
mfn_t mfn, unsigned int flags);
int __must_check iommu_unmap_page(struct domain *d, dfn_t dfn);
+int __must_check iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
+ unsigned int *flags);
enum iommu_feature
{
@@ -179,9 +181,17 @@ struct iommu_ops {
#endif /* HAS_PCI */
void (*teardown)(struct domain *d);
+
+ /*
+ * This block of operations must be appropriately locked against each
+ * other by the caller in order to have meaningful results.
+ */
int __must_check (*map_page)(struct domain *d, dfn_t dfn, mfn_t mfn,
unsigned int flags);
int __must_check (*unmap_page)(struct domain *d, dfn_t dfn);
+ int __must_check (*lookup_page)(struct domain *d, dfn_t dfn, mfn_t *mfn,
+ unsigned int *flags);
+
void (*free_page_table)(struct page_info *);
#ifdef CONFIG_X86
void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned int value);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt()
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
` (6 preceding siblings ...)
2018-10-04 10:45 ` [PATCH v14 7/9] vtd: add lookup_page method to iommu_ops Paul Durrant
@ 2018-10-04 10:45 ` Paul Durrant
2018-10-04 14:02 ` Jan Beulich
` (2 more replies)
2018-10-04 10:45 ` [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync() Paul Durrant
8 siblings, 3 replies; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Jun Nakajima, George Dunlap, Andrew Cooper,
Paul Durrant, Jan Beulich
The name 'iommu_use_hap_pt' suggests that that P2M table is in use as the
domain's IOMMU pagetable which, prior to this patch, is not strictly true
since the macro did not test whether the domain actually has IOMMU
mappings.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
v11:
- Pulled in from v6 of the full PV-IOMMU series.
v4:
- New in v4.
---
xen/arch/x86/mm/p2m-ept.c | 6 +++---
xen/arch/x86/mm/p2m-pt.c | 6 +++---
xen/arch/x86/mm/p2m.c | 2 +-
xen/drivers/passthrough/iommu.c | 2 +-
xen/include/asm-arm/iommu.h | 2 +-
xen/include/asm-x86/iommu.h | 5 +++--
6 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index cd4cb5752a..f32df3eb7a 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -874,12 +874,12 @@ out:
ept_sync_domain(p2m);
/* For host p2m, may need to change VT-d page table.*/
- if ( rc == 0 && p2m_is_hostp2m(p2m) && need_iommu(d) &&
+ if ( rc == 0 && p2m_is_hostp2m(p2m) &&
need_modify_vtd_table )
{
- if ( iommu_hap_pt_share )
+ if ( iommu_use_hap_pt(d) )
rc = iommu_pte_flush(d, gfn, &ept_entry->epte, order, vtd_pte_present);
- else
+ else if ( need_iommu(d) )
{
dfn_t dfn = _dfn(gfn);
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 4735934492..bf36160993 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -678,8 +678,8 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
&& (gfn + (1UL << page_order) - 1 > p2m->max_mapped_pfn) )
p2m->max_mapped_pfn = gfn + (1UL << page_order) - 1;
- if ( iommu_enabled && need_iommu(p2m->domain) &&
- (iommu_old_flags != iommu_pte_flags || old_mfn != mfn_x(mfn)) )
+ if ( iommu_enabled && (iommu_old_flags != iommu_pte_flags ||
+ old_mfn != mfn_x(mfn)) )
{
ASSERT(rc == 0);
@@ -688,7 +688,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
if ( iommu_old_flags )
amd_iommu_flush_pages(p2m->domain, gfn, page_order);
}
- else
+ else if ( need_iommu(p2m->domain) )
{
dfn_t dfn = _dfn(gfn);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 3108fb2b27..4a98198004 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2101,7 +2101,7 @@ static unsigned int mmio_order(const struct domain *d,
* - exclude PV guests, should execution reach this code for such.
* So be careful when altering this.
*/
- if ( !need_iommu(d) || !iommu_use_hap_pt(d) ||
+ if ( !iommu_use_hap_pt(d) ||
(start_fn & ((1UL << PAGE_ORDER_2M) - 1)) || !(nr >> PAGE_ORDER_2M) )
return PAGE_ORDER_4K;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index d51fffae40..19e95720b5 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -475,7 +475,7 @@ int iommu_do_domctl(
void iommu_share_p2m_table(struct domain* d)
{
- if ( iommu_enabled && iommu_use_hap_pt(d) )
+ if ( iommu_use_hap_pt(d) )
iommu_get_ops()->share_p2m(d);
}
diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
index 57d9b1e14a..8d1506c6f7 100644
--- a/xen/include/asm-arm/iommu.h
+++ b/xen/include/asm-arm/iommu.h
@@ -21,7 +21,7 @@ struct arch_iommu
};
/* Always share P2M Table between the CPU and the IOMMU */
-#define iommu_use_hap_pt(d) (1)
+#define iommu_use_hap_pt(d) (need_iommu(d))
const struct iommu_ops *iommu_get_ops(void);
void __init iommu_set_ops(const struct iommu_ops *ops);
diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
index 0ed4a9e86d..7c3187c8ec 100644
--- a/xen/include/asm-x86/iommu.h
+++ b/xen/include/asm-x86/iommu.h
@@ -89,8 +89,9 @@ static inline int iommu_hardware_setup(void)
return -ENODEV;
}
-/* Does this domain have a P2M table we can use as its IOMMU pagetable? */
-#define iommu_use_hap_pt(d) (hap_enabled(d) && iommu_hap_pt_share)
+/* Are we using the domain P2M table as its IOMMU pagetable? */
+#define iommu_use_hap_pt(d) \
+ (hap_enabled(d) && need_iommu(d) && iommu_hap_pt_share)
void iommu_update_ire_from_apic(unsigned int apic, unsigned int reg, unsigned int value);
unsigned int iommu_read_apic_from_ire(unsigned int apic, unsigned int reg);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync()
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
` (7 preceding siblings ...)
2018-10-04 10:45 ` [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt() Paul Durrant
@ 2018-10-04 10:45 ` Paul Durrant
2018-10-04 14:11 ` Jan Beulich
` (3 more replies)
8 siblings, 4 replies; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 10:45 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Paul Durrant, Tamas K Lengyel,
Jan Beulich, Brian Woods, Suravee Suthikulpanit
The name 'need_iommu()' is a little confusing as it suggests a domain needs
to use the IOMMU but something might not be set up yet, when in fact it
represents a tri-state value (not a boolean as might be expected) where
-1 means 'IOMMU mappings being set up' and 1 means 'IOMMU mappings have
been fully set up'.
Two different meanings are also inferred from the macro it in various
places in the code:
- Some callers want to test whether a domain has IOMMU mappings at all
- Some callers want to test whether they need to synchronize the domain's
P2M and IOMMU mappings
This patch replaces the 'need_iommu' tri-state value with a defined
enumeration and adds a boolean flag 'need_sync' to separate these meanings,
and places both of these in struct domain_iommu, rather than directly in
struct domain.
This patch also creates two new boolean macros:
- 'has_iommu_pt()' evaluates to true if a domain has IOMMU mappings, even
if they are still under construction.
- 'need_iommu_pt_sync()' evaluates to true if a domain requires explicit
synchronization of the P2M and IOMMU mappings.
All callers of need_iommu() are then modified to use the macro appropriate
to what they are trying to test, except for the instance in
xen/drivers/passthrough/pci.c:assign_device() which has simply been
removed since it appears to be unnecessary.
NOTE: There are some callers of need_iommu() that strictly operate on
the hardware domain. In some of these case a more global flag is
used instead.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Brian Woods <brian.woods@amd.com>
v14:
- Re-base.
v13:
- Re-work checks in memory_add().
- Extend commit comment to note removal of need_iommu() check in
assign_device().
v12:
- Fix two mis-uses of iommu_hap_pt_share().
- Remove one use has_iommu_pt() check in passthrough/pci.c
v11:
- Pulled in from v6 of the full PV-IOMMU series.
_ Changed the condition being tested in memory_add() to be more self-
explanatory but also added a comment to explain the circumstances
under which iommu_map_page() needs to be called.
- Get rid of #ifdef CONFIG_HAS_PASSTHROUGH in memory.c since the if
clauses within will never be executed unless the option is defined
(since has_iommu_pt() will always be false otherwise). Also flushing
should be done regardless of whether the IOMMU is sharing page tables
or not.
v6:
- Deal with need_iommu being tri-state.
- Change the name of 'sync_iommu_pt' to 'need_iommu_pt_sync' and make
sure it is set as soon as the mappings are under construction.
- Not adding Razvan's A-b because of change.
v4:
- New in v4.
---
xen/arch/arm/p2m.c | 2 +-
xen/arch/x86/hvm/mtrr.c | 4 ++--
xen/arch/x86/mm.c | 2 +-
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 2 +-
xen/arch/x86/mm/p2m-pt.c | 2 +-
xen/arch/x86/mm/p2m.c | 8 ++++----
xen/arch/x86/mm/paging.c | 2 +-
xen/arch/x86/x86_64/mm.c | 10 ++++++++--
xen/common/memory.c | 10 +++-------
xen/common/vm_event.c | 2 +-
xen/drivers/passthrough/device_tree.c | 21 ++++++++++----------
xen/drivers/passthrough/iommu.c | 37 ++++++++++++++++++++++++++---------
xen/drivers/passthrough/pci.c | 11 +++++------
xen/drivers/passthrough/x86/iommu.c | 2 --
xen/include/asm-arm/grant_table.h | 2 +-
xen/include/asm-arm/iommu.h | 2 +-
xen/include/asm-x86/grant_table.h | 2 +-
xen/include/asm-x86/iommu.h | 2 +-
xen/include/xen/iommu.h | 17 ++++++++++++++++
xen/include/xen/sched.h | 9 ++++-----
21 files changed, 92 insertions(+), 59 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 1c79ff7ade..a0bec7c95c 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -955,7 +955,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
if ( lpae_is_valid(orig_pte) && entry->p2m.base != orig_pte.p2m.base )
p2m_free_entry(p2m, orig_pte, level);
- if ( need_iommu(p2m->domain) &&
+ if ( need_iommu_pt_sync(p2m->domain) &&
(lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
{
rc = iommu_iotlb_flush(p2m->domain, _dfn(gfn_x(sgfn)),
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
index 4f2f195f7d..b8fa340d5a 100644
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -783,7 +783,7 @@ HVM_REGISTER_SAVE_RESTORE(MTRR, hvm_save_mtrr_msr, hvm_load_mtrr_msr, 1,
void memory_type_changed(struct domain *d)
{
- if ( need_iommu(d) && d->vcpu && d->vcpu[0] )
+ if ( has_iommu_pt(d) && d->vcpu && d->vcpu[0] )
{
p2m_memory_type_changed(d);
flush_all(FLUSH_CACHE);
@@ -831,7 +831,7 @@ int epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn,
return MTRR_TYPE_UNCACHABLE;
}
- if ( !need_iommu(d) && !cache_flush_permitted(d) )
+ if ( !has_iommu_pt(d) && !cache_flush_permitted(d) )
{
*ipat = 1;
return MTRR_TYPE_WRBACK;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5a7c5335a2..48acdadc8c 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2787,7 +2787,7 @@ static int _get_page_type(struct page_info *page, unsigned long type,
{
/* Special pages should not be accessible from devices. */
struct domain *d = page_get_owner(page);
- if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) )
+ if ( d && is_pv_domain(d) && unlikely(need_iommu_pt_sync(d)) )
{
mfn_t mfn = page_to_mfn(page);
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 349e6fd2cf..1dab2c8cc3 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1612,7 +1612,7 @@ int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec)
case XEN_DOMCTL_MEM_SHARING_CONTROL:
{
rc = 0;
- if ( unlikely(need_iommu(d) && mec->u.enable) )
+ if ( unlikely(has_iommu_pt(d) && mec->u.enable) )
rc = -EXDEV;
else
d->arch.hvm.mem_sharing_enabled = mec->u.enable;
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index f32df3eb7a..a524ac86fa 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -879,7 +879,7 @@ out:
{
if ( iommu_use_hap_pt(d) )
rc = iommu_pte_flush(d, gfn, &ept_entry->epte, order, vtd_pte_present);
- else if ( need_iommu(d) )
+ else if ( need_iommu_pt_sync(d) )
{
dfn_t dfn = _dfn(gfn);
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index bf36160993..cc834dc95f 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -688,7 +688,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
if ( iommu_old_flags )
amd_iommu_flush_pages(p2m->domain, gfn, page_order);
}
- else if ( need_iommu(p2m->domain) )
+ else if ( need_iommu_pt_sync(p2m->domain) )
{
dfn_t dfn = _dfn(gfn);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 4a98198004..7b63e39912 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -721,7 +721,7 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn_l, unsigned long mfn,
{
int rc = 0;
- if ( need_iommu(p2m->domain) )
+ if ( need_iommu_pt_sync(p2m->domain) )
{
dfn_t dfn = _dfn(mfn);
@@ -785,7 +785,7 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
if ( !paging_mode_translate(d) )
{
- if ( need_iommu(d) && t == p2m_ram_rw )
+ if ( need_iommu_pt_sync(d) && t == p2m_ram_rw )
{
dfn_t dfn = _dfn(mfn_x(mfn));
@@ -1175,7 +1175,7 @@ int set_identity_p2m_entry(struct domain *d, unsigned long gfn_l,
if ( !paging_mode_translate(d) )
{
- if ( !need_iommu(d) )
+ if ( !need_iommu_pt_sync(d) )
return 0;
ret = iommu_map_page(d, _dfn(gfn_l), _mfn(gfn_l),
@@ -1271,7 +1271,7 @@ int clear_identity_p2m_entry(struct domain *d, unsigned long gfn_l)
if ( !paging_mode_translate(d) )
{
- if ( !need_iommu(d) )
+ if ( !need_iommu_pt_sync(d) )
return 0;
ret = iommu_unmap_page(d, _dfn(gfn_l));
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index 7f460bd321..f32a60188a 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -213,7 +213,7 @@ int paging_log_dirty_enable(struct domain *d, bool_t log_global)
{
int ret;
- if ( need_iommu(d) && log_global )
+ if ( has_iommu_pt(d) && log_global )
{
/*
* Refuse to turn on global log-dirty mode
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index d1fce57432..543ea030e3 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1426,8 +1426,14 @@ int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm)
if ( ret )
goto destroy_m2p;
- if ( iommu_enabled && !iommu_hwdom_passthrough &&
- !need_iommu(hardware_domain) )
+ /*
+ * If hardware domain has IOMMU mappings but page tables are not
+ * shared or being kept in sync then newly added memory needs to be
+ * mapped here.
+ */
+ if ( has_iommu_pt(hardware_domain) &&
+ !iommu_use_hap_pt(hardware_domain) &&
+ !need_iommu_pt_sync(hardware_domain) )
{
for ( i = spfn; i < epfn; i++ )
if ( iommu_map_page(hardware_domain, _dfn(i), _mfn(i),
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 52d55dfed7..db3334bae6 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -805,10 +805,8 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
xatp->gpfn += start;
xatp->size -= start;
-#ifdef CONFIG_HAS_PASSTHROUGH
- if ( need_iommu(d) )
- this_cpu(iommu_dont_flush_iotlb) = 1;
-#endif
+ if ( has_iommu_pt(d) )
+ this_cpu(iommu_dont_flush_iotlb) = 1;
while ( xatp->size > done )
{
@@ -828,8 +826,7 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
}
}
-#ifdef CONFIG_HAS_PASSTHROUGH
- if ( need_iommu(d) )
+ if ( has_iommu_pt(d) )
{
int ret;
@@ -846,7 +843,6 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
if ( unlikely(rc < 0) )
domu_crash(d);
}
-#endif
return rc;
}
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 100da8048c..6ffd18a448 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -642,7 +642,7 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
/* No paging if iommu is used */
rc = -EMLINK;
- if ( unlikely(need_iommu(d)) )
+ if ( unlikely(has_iommu_pt(d)) )
break;
rc = -EXDEV;
diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c
index 421f003438..b6eaae7283 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -40,17 +40,16 @@ int iommu_assign_dt_device(struct domain *d, struct dt_device_node *dev)
if ( !list_empty(&dev->domain_list) )
goto fail;
- if ( need_iommu(d) <= 0 )
- {
- /*
- * The hwdom is forced to use IOMMU for protecting assigned
- * device. Therefore the IOMMU data is already set up.
- */
- ASSERT(!is_hardware_domain(d));
- rc = iommu_construct(d);
- if ( rc )
- goto fail;
- }
+ /*
+ * The hwdom is forced to use IOMMU for protecting assigned
+ * device. Therefore the IOMMU data is already set up.
+ */
+ ASSERT(!is_hardware_domain(d) ||
+ hd->status == IOMMU_STATUS_initialized);
+
+ rc = iommu_construct(d);
+ if ( rc )
+ goto fail;
/* The flag field doesn't matter to DT device. */
rc = hd->platform_ops->assign_device(d, 0, dt_to_dev(dev), 0);
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 19e95720b5..d31facdc48 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -197,7 +197,7 @@ static void __hwdom_init check_hwdom_reqs(struct domain *d)
void __hwdom_init iommu_hwdom_init(struct domain *d)
{
- const struct domain_iommu *hd = dom_iommu(d);
+ struct domain_iommu *hd = dom_iommu(d);
check_hwdom_reqs(d);
@@ -205,8 +205,10 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
return;
register_keyhandler('o', &iommu_dump_p2m_table, "dump iommu p2m table", 0);
- d->need_iommu = iommu_hwdom_strict;
- if ( need_iommu(d) && !iommu_use_hap_pt(d) )
+
+ hd->status = IOMMU_STATUS_initializing;
+ hd->need_sync = iommu_hwdom_strict && !iommu_use_hap_pt(d);
+ if ( need_iommu_pt_sync(d) )
{
struct page_info *page;
unsigned int i = 0;
@@ -239,35 +241,51 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
}
hd->platform_ops->hwdom_init(d);
+
+ hd->status = IOMMU_STATUS_initialized;
}
void iommu_teardown(struct domain *d)
{
- const struct domain_iommu *hd = dom_iommu(d);
+ struct domain_iommu *hd = dom_iommu(d);
- d->need_iommu = 0;
+ hd->status = IOMMU_STATUS_disabled;
hd->platform_ops->teardown(d);
tasklet_schedule(&iommu_pt_cleanup_tasklet);
}
int iommu_construct(struct domain *d)
{
- if ( need_iommu(d) > 0 )
+ struct domain_iommu *hd = dom_iommu(d);
+
+ if ( hd->status == IOMMU_STATUS_initialized )
return 0;
if ( !iommu_use_hap_pt(d) )
{
int rc;
+ hd->status = IOMMU_STATUS_initializing;
+ hd->need_sync = true;
+
rc = arch_iommu_populate_page_table(d);
if ( rc )
+ {
+ if ( rc != -ERESTART )
+ {
+ hd->need_sync = false;
+ hd->status = IOMMU_STATUS_disabled;
+ }
+
return rc;
+ }
}
- d->need_iommu = 1;
+ hd->status = IOMMU_STATUS_initialized;
+
/*
* There may be dirty cache lines when a device is assigned
- * and before need_iommu(d) becoming true, this will cause
+ * and before has_iommu_pt(d) becoming true, this will cause
* memory_type_changed lose effect if memory type changes.
* Call memory_type_changed here to amend this.
*/
@@ -522,7 +540,8 @@ static void iommu_dump_p2m_table(unsigned char key)
ops = iommu_get_ops();
for_each_domain(d)
{
- if ( is_hardware_domain(d) || need_iommu(d) <= 0 )
+ if ( is_hardware_domain(d) ||
+ dom_iommu(d)->status < IOMMU_STATUS_initialized )
continue;
if ( iommu_use_hap_pt(d) )
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 9695cf566d..e5b9602762 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1416,10 +1416,9 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
- if ( unlikely(!need_iommu(d) &&
- (d->arch.hvm.mem_sharing_enabled ||
- vm_event_check_ring(d->vm_event_paging) ||
- p2m_get_hostp2m(d)->global_logdirty)) )
+ if ( unlikely(d->arch.hvm.mem_sharing_enabled ||
+ vm_event_check_ring(d->vm_event_paging) ||
+ p2m_get_hostp2m(d)->global_logdirty) )
return -EXDEV;
if ( !pcidevs_trylock() )
@@ -1460,7 +1459,7 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
}
done:
- if ( !has_arch_pdevs(d) && need_iommu(d) )
+ if ( !has_arch_pdevs(d) && has_iommu_pt(d) )
iommu_teardown(d);
pcidevs_unlock();
@@ -1510,7 +1509,7 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
pdev->fault.count = 0;
- if ( !has_arch_pdevs(d) && need_iommu(d) )
+ if ( !has_arch_pdevs(d) && has_iommu_pt(d) )
iommu_teardown(d);
return ret;
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index bcf7a2f298..d2a7a952d9 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -48,8 +48,6 @@ int arch_iommu_populate_page_table(struct domain *d)
struct page_info *page;
int rc = 0, n = 0;
- d->need_iommu = -1;
-
this_cpu(iommu_dont_flush_iotlb) = 1;
spin_lock(&d->page_alloc_lock);
diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index d8fde01651..37415b7821 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -90,7 +90,7 @@ void gnttab_mark_dirty(struct domain *d, mfn_t mfn);
gfn_x(((i) >= nr_status_frames(t)) ? INVALID_GFN : (t)->arch.status_gfn[i])
#define gnttab_need_iommu_mapping(d) \
- (is_domain_direct_mapped(d) && need_iommu(d))
+ (is_domain_direct_mapped(d) && need_iommu_pt_sync(d))
#endif /* __ASM_GRANT_TABLE_H__ */
/*
diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
index 8d1506c6f7..f6df32f860 100644
--- a/xen/include/asm-arm/iommu.h
+++ b/xen/include/asm-arm/iommu.h
@@ -21,7 +21,7 @@ struct arch_iommu
};
/* Always share P2M Table between the CPU and the IOMMU */
-#define iommu_use_hap_pt(d) (need_iommu(d))
+#define iommu_use_hap_pt(d) (has_iommu_pt(d))
const struct iommu_ops *iommu_get_ops(void);
void __init iommu_set_ops(const struct iommu_ops *ops);
diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h
index 761a8c33a5..1e6a98813d 100644
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -94,6 +94,6 @@ static inline void gnttab_clear_flag(unsigned int nr, uint16_t *st)
#define gnttab_release_host_mappings(domain) ( paging_mode_external(domain) )
#define gnttab_need_iommu_mapping(d) \
- (!paging_mode_translate(d) && need_iommu(d))
+ (!paging_mode_translate(d) && need_iommu_pt_sync(d))
#endif /* __ASM_GRANT_TABLE_H__ */
diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
index 7c3187c8ec..fa37b0539b 100644
--- a/xen/include/asm-x86/iommu.h
+++ b/xen/include/asm-x86/iommu.h
@@ -91,7 +91,7 @@ static inline int iommu_hardware_setup(void)
/* Are we using the domain P2M table as its IOMMU pagetable? */
#define iommu_use_hap_pt(d) \
- (hap_enabled(d) && need_iommu(d) && iommu_hap_pt_share)
+ (hap_enabled(d) && has_iommu_pt(d) && iommu_hap_pt_share)
void iommu_update_ire_from_apic(unsigned int apic, unsigned int reg, unsigned int value);
unsigned int iommu_read_apic_from_ire(unsigned int apic, unsigned int reg);
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 9ae8321bb4..7ebaca5ca4 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -103,6 +103,13 @@ enum iommu_feature
bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature);
+enum iommu_status
+{
+ IOMMU_STATUS_disabled,
+ IOMMU_STATUS_initializing,
+ IOMMU_STATUS_initialized
+};
+
struct domain_iommu {
struct arch_iommu arch;
@@ -116,6 +123,16 @@ struct domain_iommu {
/* Features supported by the IOMMU */
DECLARE_BITMAP(features, IOMMU_FEAT_count);
+
+ /* Status of guest IOMMU mappings */
+ enum iommu_status status;
+
+ /*
+ * Does the guest reqire mappings to be synchonized, to maintain
+ * the default dfn == pfn map. (See comment on dfn at the top of
+ * include/xen/mm.h).
+ */
+ bool need_sync;
};
#define dom_iommu(d) (&(d)->iommu)
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index f2c594d197..63fb70bcdc 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -371,9 +371,6 @@ struct domain
#ifdef CONFIG_HAS_PASSTHROUGH
struct domain_iommu iommu;
-
- /* Does this guest need iommu mappings (-1 meaning "being set up")? */
- s8 need_iommu;
#endif
/* is node-affinity automatically computed? */
bool auto_node_affinity;
@@ -898,9 +895,11 @@ static inline bool is_hvm_vcpu(const struct vcpu *v)
#define is_pinned_vcpu(v) ((v)->domain->is_pinned || \
cpumask_weight((v)->cpu_hard_affinity) == 1)
#ifdef CONFIG_HAS_PASSTHROUGH
-#define need_iommu(d) ((d)->need_iommu)
+#define has_iommu_pt(d) (dom_iommu(d)->status != IOMMU_STATUS_disabled)
+#define need_iommu_pt_sync(d) (dom_iommu(d)->need_sync)
#else
-#define need_iommu(d) (0)
+#define has_iommu_pt(d) false
+#define need_iommu_pt_sync(d) false
#endif
static inline bool is_vcpu_online(const struct vcpu *v)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v14 3/9] iommu: push use of type-safe DFN and MFN into iommu_ops
2018-10-04 10:45 ` [PATCH v14 3/9] iommu: push use of type-safe DFN and MFN into iommu_ops Paul Durrant
@ 2018-10-04 13:04 ` Paul Durrant
2018-10-04 13:07 ` Julien Grall
0 siblings, 1 reply; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 13:04 UTC (permalink / raw)
To: Julien Grall (julien.grall@arm.com)
Cc: Andrew Cooper, George Dunlap, xen-devel
> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 04 October 2018 11:46
> To: xen-devel@lists.xenproject.org
> Cc: Paul Durrant <Paul.Durrant@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>
> Subject: [PATCH v14 3/9] iommu: push use of type-safe DFN and MFN into
> iommu_ops
>
> This patch modifies the methods in struct iommu_ops to use type-safe DFN
> and MFN. This follows on from the prior patch that modified the functions
> exported in xen/iommu.h.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Julien, you should have been cc-ed on this one too. Not sure why I missed you off the list... sorry about that. Could I get an ack or otherwise please?
Paul
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Reviewed-by: Roger Pau Monne <roger.pau@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <george.dunlap@citrix.com>
>
> v14:
> - Re-base.
>
> v9:
> - Re-base.
>
> v7:
> - Re-base and re-name BFN -> DFN.
> - Added Jan's A-b since re-naming was purely mechanical.
>
> v6:
> - Re-base.
>
> v3:
> - Remove some use of intermediate 'frame' variables.
>
> v2:
> - Addressed comments from Jan.
> - Extend use of intermediate 'frame' variable to avoid directly
> encapsulating gfn values as bfns (now dfns) or vice versa.
> ---
> xen/drivers/passthrough/amd/iommu_map.c | 46 ++++++++++++++++------
> -----
> xen/drivers/passthrough/arm/smmu.c | 16 +++++-----
> xen/drivers/passthrough/iommu.c | 9 +++---
> xen/drivers/passthrough/vtd/iommu.c | 26 +++++++--------
> xen/drivers/passthrough/x86/iommu.c | 2 +-
> xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 8 ++---
> xen/include/xen/iommu.h | 13 +++++---
> 7 files changed, 66 insertions(+), 54 deletions(-)
>
> diff --git a/xen/drivers/passthrough/amd/iommu_map.c
> b/xen/drivers/passthrough/amd/iommu_map.c
> index 61ade71850..c89c54fdb6 100644
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -631,7 +631,7 @@ static int update_paging_mode(struct domain *d,
> unsigned long dfn)
> return 0;
> }
>
> -int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long
> mfn,
> +int amd_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
> unsigned int flags)
> {
> bool_t need_flush = 0;
> @@ -651,7 +651,8 @@ int amd_iommu_map_page(struct domain *d, unsigned long
> dfn, unsigned long mfn,
> if ( rc )
> {
> spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %lx\n", dfn);
> + AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %"PRI_dfn"\n",
> + dfn_x(dfn));
> domain_crash(d);
> return rc;
> }
> @@ -660,25 +661,27 @@ int amd_iommu_map_page(struct domain *d, unsigned
> long dfn, unsigned long mfn,
> * we might need a deeper page table for wider dfn now */
> if ( is_hvm_domain(d) )
> {
> - if ( update_paging_mode(d, dfn) )
> + if ( update_paging_mode(d, dfn_x(dfn)) )
> {
> spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn);
> + AMD_IOMMU_DEBUG("Update page mode failed dfn = %"PRI_dfn"\n",
> + dfn_x(dfn));
> domain_crash(d);
> return -EFAULT;
> }
> }
>
> - if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) )
> + if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) )
> {
> spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn);
> + AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
> + dfn_x(dfn));
> domain_crash(d);
> return -EFAULT;
> }
>
> /* Install 4k mapping first */
> - need_flush = set_iommu_pte_present(pt_mfn[1], dfn, mfn,
> + need_flush = set_iommu_pte_present(pt_mfn[1], dfn_x(dfn), mfn_x(mfn),
> IOMMU_PAGING_MODE_LEVEL_1,
> !!(flags & IOMMUF_writable),
> !!(flags & IOMMUF_readable));
> @@ -690,7 +693,7 @@ int amd_iommu_map_page(struct domain *d, unsigned long
> dfn, unsigned long mfn,
> /* 4K mapping for PV guests never changes,
> * no need to flush if we trust non-present bits */
> if ( is_hvm_domain(d) )
> - amd_iommu_flush_pages(d, dfn, 0);
> + amd_iommu_flush_pages(d, dfn_x(dfn), 0);
>
> for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2;
> merge_level <= hd->arch.paging_mode; merge_level++ )
> @@ -698,15 +701,16 @@ int amd_iommu_map_page(struct domain *d, unsigned
> long dfn, unsigned long mfn,
> if ( pt_mfn[merge_level] == 0 )
> break;
> if ( !iommu_update_pde_count(d, pt_mfn[merge_level],
> - dfn, mfn, merge_level) )
> + dfn_x(dfn), mfn_x(mfn), merge_level)
> )
> break;
>
> - if ( iommu_merge_pages(d, pt_mfn[merge_level], dfn,
> + if ( iommu_merge_pages(d, pt_mfn[merge_level], dfn_x(dfn),
> flags, merge_level) )
> {
> spin_unlock(&hd->arch.mapping_lock);
> AMD_IOMMU_DEBUG("Merge iommu page failed at level %d, "
> - "dfn = %lx mfn = %lx\n", merge_level, dfn,
> mfn);
> + "dfn = %"PRI_dfn" mfn = %"PRI_mfn"\n",
> + merge_level, dfn_x(dfn), mfn_x(mfn));
> domain_crash(d);
> return -EFAULT;
> }
> @@ -720,7 +724,7 @@ out:
> return 0;
> }
>
> -int amd_iommu_unmap_page(struct domain *d, unsigned long dfn)
> +int amd_iommu_unmap_page(struct domain *d, dfn_t dfn)
> {
> unsigned long pt_mfn[7];
> struct domain_iommu *hd = dom_iommu(d);
> @@ -742,31 +746,33 @@ int amd_iommu_unmap_page(struct domain *d, unsigned
> long dfn)
> * we might need a deeper page table for lager dfn now */
> if ( is_hvm_domain(d) )
> {
> - int rc = update_paging_mode(d, dfn);
> + int rc = update_paging_mode(d, dfn_x(dfn));
>
> if ( rc )
> {
> spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn);
> + AMD_IOMMU_DEBUG("Update page mode failed dfn = %"PRI_dfn"\n",
> + dfn_x(dfn));
> if ( rc != -EADDRNOTAVAIL )
> domain_crash(d);
> return rc;
> }
> }
>
> - if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) )
> + if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) )
> {
> spin_unlock(&hd->arch.mapping_lock);
> - AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn);
> + AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
> + dfn_x(dfn));
> domain_crash(d);
> return -EFAULT;
> }
>
> /* mark PTE as 'page not present' */
> - clear_iommu_pte_present(pt_mfn[1], dfn);
> + clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn));
> spin_unlock(&hd->arch.mapping_lock);
>
> - amd_iommu_flush_pages(d, dfn, 0);
> + amd_iommu_flush_pages(d, dfn_x(dfn), 0);
>
> return 0;
> }
> @@ -787,7 +793,9 @@ int amd_iommu_reserve_domain_unity_map(struct domain
> *domain,
> gfn = phys_addr >> PAGE_SHIFT;
> for ( i = 0; i < npages; i++ )
> {
> - rt = amd_iommu_map_page(domain, gfn +i, gfn +i, flags);
> + unsigned long frame = gfn + i;
> +
> + rt = amd_iommu_map_page(domain, _dfn(frame), _mfn(frame), flags);
> if ( rt != 0 )
> return rt;
> }
> diff --git a/xen/drivers/passthrough/arm/smmu.c
> b/xen/drivers/passthrough/arm/smmu.c
> index 1eda96a72a..53e5823d05 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -2550,8 +2550,7 @@ static int __must_check
> arm_smmu_iotlb_flush_all(struct domain *d)
> return 0;
> }
>
> -static int __must_check arm_smmu_iotlb_flush(struct domain *d,
> - unsigned long dfn,
> +static int __must_check arm_smmu_iotlb_flush(struct domain *d, dfn_t dfn,
> unsigned int page_count)
> {
> /* ARM SMMU v1 doesn't have flush by VMA and VMID */
> @@ -2748,8 +2747,8 @@ static void arm_smmu_iommu_domain_teardown(struct
> domain *d)
> xfree(xen_domain);
> }
>
> -static int __must_check arm_smmu_map_page(struct domain *d, unsigned long
> dfn,
> - unsigned long mfn, unsigned int flags)
> +static int __must_check arm_smmu_map_page(struct domain *d, dfn_t dfn,
> + mfn_t mfn, unsigned int flags)
> {
> p2m_type_t t;
>
> @@ -2762,7 +2761,7 @@ static int __must_check arm_smmu_map_page(struct
> domain *d, unsigned long dfn,
> * function should only be used by gnttab code with gfn == mfn ==
> dfn.
> */
> BUG_ON(!is_domain_direct_mapped(d));
> - BUG_ON(mfn != dfn);
> + BUG_ON(mfn_x(mfn) != dfn_x(dfn));
>
> /* We only support readable and writable flags */
> if (!(flags & (IOMMUF_readable | IOMMUF_writable)))
> @@ -2774,10 +2773,11 @@ static int __must_check arm_smmu_map_page(struct
> domain *d, unsigned long dfn,
> * The function guest_physmap_add_entry replaces the current mapping
> * if there is already one...
> */
> - return guest_physmap_add_entry(d, _gfn(dfn), _mfn(dfn), 0, t);
> + return guest_physmap_add_entry(d, _gfn(dfn_x(dfn)),
> _mfn(dfn_x(dfn)),
> + 0, t);
> }
>
> -static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned
> long dfn)
> +static int __must_check arm_smmu_unmap_page(struct domain *d, dfn_t dfn)
> {
> /*
> * This function should only be used by gnttab code when the domain
> @@ -2786,7 +2786,7 @@ static int __must_check arm_smmu_unmap_page(struct
> domain *d, unsigned long dfn)
> if ( !is_domain_direct_mapped(d) )
> return -EINVAL;
>
> - return guest_physmap_remove_page(d, _gfn(dfn), _mfn(dfn), 0);
> + return guest_physmap_remove_page(d, _gfn(dfn_x(dfn)),
> _mfn(dfn_x(dfn)), 0);
> }
>
> static const struct iommu_ops arm_smmu_iommu_ops = {
> diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> index ef9d651317..d61fbbf439 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -224,7 +224,8 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
> == PGT_writable_page) )
> mapping |= IOMMUF_writable;
>
> - ret = hd->platform_ops->map_page(d, dfn, mfn, mapping);
> + ret = hd->platform_ops->map_page(d, _dfn(dfn), _mfn(mfn),
> + mapping);
> if ( !rc )
> rc = ret;
>
> @@ -294,7 +295,7 @@ int iommu_map_page(struct domain *d, dfn_t dfn, mfn_t
> mfn,
> if ( !iommu_enabled || !hd->platform_ops )
> return 0;
>
> - rc = hd->platform_ops->map_page(d, dfn_x(dfn), mfn_x(mfn), flags);
> + rc = hd->platform_ops->map_page(d, dfn, mfn, flags);
> if ( unlikely(rc) )
> {
> if ( !d->is_shutting_down && printk_ratelimit() )
> @@ -317,7 +318,7 @@ int iommu_unmap_page(struct domain *d, dfn_t dfn)
> if ( !iommu_enabled || !hd->platform_ops )
> return 0;
>
> - rc = hd->platform_ops->unmap_page(d, dfn_x(dfn));
> + rc = hd->platform_ops->unmap_page(d, dfn);
> if ( unlikely(rc) )
> {
> if ( !d->is_shutting_down && printk_ratelimit() )
> @@ -357,7 +358,7 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn,
> unsigned int page_count)
> if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops-
> >iotlb_flush )
> return 0;
>
> - rc = hd->platform_ops->iotlb_flush(d, dfn_x(dfn), page_count);
> + rc = hd->platform_ops->iotlb_flush(d, dfn, page_count);
> if ( unlikely(rc) )
> {
> if ( !d->is_shutting_down && printk_ratelimit() )
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 485704bcce..7e185c43f3 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -584,8 +584,7 @@ static int __must_check iommu_flush_all(void)
> return rc;
> }
>
> -static int __must_check iommu_flush_iotlb(struct domain *d,
> - unsigned long dfn,
> +static int __must_check iommu_flush_iotlb(struct domain *d, dfn_t dfn,
> bool_t dma_old_pte_present,
> unsigned int page_count)
> {
> @@ -612,12 +611,12 @@ static int __must_check iommu_flush_iotlb(struct
> domain *d,
> if ( iommu_domid == -1 )
> continue;
>
> - if ( page_count != 1 || dfn == dfn_x(INVALID_DFN) )
> + if ( page_count != 1 || dfn_eq(dfn, INVALID_DFN) )
> rc = iommu_flush_iotlb_dsi(iommu, iommu_domid,
> 0, flush_dev_iotlb);
> else
> rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
> - __dfn_to_daddr(dfn),
> + dfn_to_daddr(dfn),
> PAGE_ORDER_4K,
> !dma_old_pte_present,
> flush_dev_iotlb);
> @@ -633,7 +632,7 @@ static int __must_check iommu_flush_iotlb(struct
> domain *d,
> }
>
> static int __must_check iommu_flush_iotlb_pages(struct domain *d,
> - unsigned long dfn,
> + dfn_t dfn,
> unsigned int page_count)
> {
> return iommu_flush_iotlb(d, dfn, 1, page_count);
> @@ -641,7 +640,7 @@ static int __must_check iommu_flush_iotlb_pages(struct
> domain *d,
>
> static int __must_check iommu_flush_iotlb_all(struct domain *d)
> {
> - return iommu_flush_iotlb(d, dfn_x(INVALID_DFN), 0, 0);
> + return iommu_flush_iotlb(d, INVALID_DFN, 0, 0);
> }
>
> /* clear one page's page table */
> @@ -676,7 +675,7 @@ static int __must_check dma_pte_clear_one(struct
> domain *domain, u64 addr)
> iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
>
> if ( !this_cpu(iommu_dont_flush_iotlb) )
> - rc = iommu_flush_iotlb_pages(domain, addr >> PAGE_SHIFT_4K, 1);
> + rc = iommu_flush_iotlb_pages(domain, daddr_to_dfn(addr), 1);
>
> unmap_vtd_domain_page(page);
>
> @@ -1763,8 +1762,7 @@ static void iommu_domain_teardown(struct domain *d)
> }
>
> static int __must_check intel_iommu_map_page(struct domain *d,
> - unsigned long dfn,
> - unsigned long mfn,
> + dfn_t dfn, mfn_t mfn,
> unsigned int flags)
> {
> struct domain_iommu *hd = dom_iommu(d);
> @@ -1782,16 +1780,16 @@ static int __must_check
> intel_iommu_map_page(struct domain *d,
>
> spin_lock(&hd->arch.mapping_lock);
>
> - pg_maddr = addr_to_dma_page_maddr(d, __dfn_to_daddr(dfn), 1);
> + pg_maddr = addr_to_dma_page_maddr(d, dfn_to_daddr(dfn), 1);
> if ( pg_maddr == 0 )
> {
> spin_unlock(&hd->arch.mapping_lock);
> return -ENOMEM;
> }
> page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
> - pte = page + (dfn & LEVEL_MASK);
> + pte = page + (dfn_x(dfn) & LEVEL_MASK);
> old = *pte;
> - dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
> + dma_set_pte_addr(new, mfn_to_maddr(mfn));
> dma_set_pte_prot(new,
> ((flags & IOMMUF_readable) ? DMA_PTE_READ : 0) |
> ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
> @@ -1819,13 +1817,13 @@ static int __must_check
> intel_iommu_map_page(struct domain *d,
> }
>
> static int __must_check intel_iommu_unmap_page(struct domain *d,
> - unsigned long dfn)
> + dfn_t dfn)
> {
> /* Do nothing if hardware domain and iommu supports pass thru. */
> if ( iommu_hwdom_passthrough && is_hardware_domain(d) )
> return 0;
>
> - return dma_pte_clear_one(d, __dfn_to_daddr(dfn));
> + return dma_pte_clear_one(d, dfn_to_daddr(dfn));
> }
>
> int iommu_pte_flush(struct domain *d, uint64_t dfn, uint64_t *pte,
> diff --git a/xen/drivers/passthrough/x86/iommu.c
> b/xen/drivers/passthrough/x86/iommu.c
> index 34727b6653..f410717a59 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -68,7 +68,7 @@ int arch_iommu_populate_page_table(struct domain *d)
> {
> ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
> BUG_ON(SHARED_M2P(gfn));
> - rc = hd->platform_ops->map_page(d, gfn, mfn,
> + rc = hd->platform_ops->map_page(d, _dfn(gfn), _mfn(mfn),
> IOMMUF_readable |
> IOMMUF_writable);
> }
> diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> index 99bc21c7b3..3083d625bd 100644
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -52,9 +52,9 @@ int amd_iommu_init(void);
> int amd_iommu_update_ivrs_mapping_acpi(void);
>
> /* mapping functions */
> -int __must_check amd_iommu_map_page(struct domain *d, unsigned long gfn,
> - unsigned long mfn, unsigned int
> flags);
> -int __must_check amd_iommu_unmap_page(struct domain *d, unsigned long
> gfn);
> +int __must_check amd_iommu_map_page(struct domain *d, dfn_t dfn,
> + mfn_t mfn, unsigned int flags);
> +int __must_check amd_iommu_unmap_page(struct domain *d, dfn_t dfn);
> u64 amd_iommu_get_next_table_from_pte(u32 *entry);
> int __must_check amd_iommu_alloc_root(struct domain_iommu *hd);
> int amd_iommu_reserve_domain_unity_map(struct domain *domain,
> @@ -77,7 +77,7 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id, u64
> gcr3,
>
> /* send cmd to iommu */
> void amd_iommu_flush_all_pages(struct domain *d);
> -void amd_iommu_flush_pages(struct domain *d, unsigned long gfn,
> +void amd_iommu_flush_pages(struct domain *d, unsigned long dfn,
> unsigned int order);
> void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
> uint64_t gaddr, unsigned int order);
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index f9d86fc816..7313957c81 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -48,6 +48,11 @@ static inline dfn_t dfn_add(dfn_t dfn, unsigned long i)
> return _dfn(dfn_x(dfn) + i);
> }
>
> +static inline bool_t dfn_eq(dfn_t x, dfn_t y)
> +{
> + return dfn_x(x) == dfn_x(y);
> +}
> +
> extern bool_t iommu_enable, iommu_enabled;
> extern bool_t force_iommu, iommu_verbose;
> extern bool_t iommu_workaround_bios_bug, iommu_igfx;
> @@ -174,9 +179,9 @@ struct iommu_ops {
> #endif /* HAS_PCI */
>
> void (*teardown)(struct domain *d);
> - int __must_check (*map_page)(struct domain *d, unsigned long dfn,
> - unsigned long mfn, unsigned int flags);
> - int __must_check (*unmap_page)(struct domain *d, unsigned long dfn);
> + int __must_check (*map_page)(struct domain *d, dfn_t dfn, mfn_t mfn,
> + unsigned int flags);
> + int __must_check (*unmap_page)(struct domain *d, dfn_t dfn);
> void (*free_page_table)(struct page_info *);
> #ifdef CONFIG_X86
> void (*update_ire_from_apic)(unsigned int apic, unsigned int reg,
> unsigned int value);
> @@ -187,7 +192,7 @@ struct iommu_ops {
> void (*resume)(void);
> void (*share_p2m)(struct domain *d);
> void (*crash_shutdown)(void);
> - int __must_check (*iotlb_flush)(struct domain *d, unsigned long dfn,
> + int __must_check (*iotlb_flush)(struct domain *d, dfn_t dfn,
> unsigned int page_count);
> int __must_check (*iotlb_flush_all)(struct domain *d);
> int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
> --
> 2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 3/9] iommu: push use of type-safe DFN and MFN into iommu_ops
2018-10-04 13:04 ` Paul Durrant
@ 2018-10-04 13:07 ` Julien Grall
0 siblings, 0 replies; 34+ messages in thread
From: Julien Grall @ 2018-10-04 13:07 UTC (permalink / raw)
To: Paul Durrant; +Cc: Andrew Cooper, George Dunlap, xen-devel
Hi Paul,
On 04/10/2018 14:04, Paul Durrant wrote:
>> -----Original Message-----
>> From: Paul Durrant [mailto:paul.durrant@citrix.com]
>> Sent: 04 October 2018 11:46
>> To: xen-devel@lists.xenproject.org
>> Cc: Paul Durrant <Paul.Durrant@citrix.com>; Andrew Cooper
>> <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>
>> Subject: [PATCH v14 3/9] iommu: push use of type-safe DFN and MFN into
>> iommu_ops
>>
>> This patch modifies the methods in struct iommu_ops to use type-safe DFN
>> and MFN. This follows on from the prior patch that modified the functions
>> exported in xen/iommu.h.
>>
>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>
> Julien, you should have been cc-ed on this one too. Not sure why I missed you off the list... sorry about that. Could I get an ack or otherwise please?
The patch looks good to me:
Acked-by: Julien Grall <julien.grall@arm.com>
Cheers,
>
> Paul
>
>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
>> Reviewed-by: Roger Pau Monne <roger.pau@citrix.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> ---
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: George Dunlap <george.dunlap@citrix.com>
>>
>> v14:
>> - Re-base.
>>
>> v9:
>> - Re-base.
>>
>> v7:
>> - Re-base and re-name BFN -> DFN.
>> - Added Jan's A-b since re-naming was purely mechanical.
>>
>> v6:
>> - Re-base.
>>
>> v3:
>> - Remove some use of intermediate 'frame' variables.
>>
>> v2:
>> - Addressed comments from Jan.
>> - Extend use of intermediate 'frame' variable to avoid directly
>> encapsulating gfn values as bfns (now dfns) or vice versa.
>> ---
>> xen/drivers/passthrough/amd/iommu_map.c | 46 ++++++++++++++++------
>> -----
>> xen/drivers/passthrough/arm/smmu.c | 16 +++++-----
>> xen/drivers/passthrough/iommu.c | 9 +++---
>> xen/drivers/passthrough/vtd/iommu.c | 26 +++++++--------
>> xen/drivers/passthrough/x86/iommu.c | 2 +-
>> xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 8 ++---
>> xen/include/xen/iommu.h | 13 +++++---
>> 7 files changed, 66 insertions(+), 54 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/amd/iommu_map.c
>> b/xen/drivers/passthrough/amd/iommu_map.c
>> index 61ade71850..c89c54fdb6 100644
>> --- a/xen/drivers/passthrough/amd/iommu_map.c
>> +++ b/xen/drivers/passthrough/amd/iommu_map.c
>> @@ -631,7 +631,7 @@ static int update_paging_mode(struct domain *d,
>> unsigned long dfn)
>> return 0;
>> }
>>
>> -int amd_iommu_map_page(struct domain *d, unsigned long dfn, unsigned long
>> mfn,
>> +int amd_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
>> unsigned int flags)
>> {
>> bool_t need_flush = 0;
>> @@ -651,7 +651,8 @@ int amd_iommu_map_page(struct domain *d, unsigned long
>> dfn, unsigned long mfn,
>> if ( rc )
>> {
>> spin_unlock(&hd->arch.mapping_lock);
>> - AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %lx\n", dfn);
>> + AMD_IOMMU_DEBUG("Root table alloc failed, dfn = %"PRI_dfn"\n",
>> + dfn_x(dfn));
>> domain_crash(d);
>> return rc;
>> }
>> @@ -660,25 +661,27 @@ int amd_iommu_map_page(struct domain *d, unsigned
>> long dfn, unsigned long mfn,
>> * we might need a deeper page table for wider dfn now */
>> if ( is_hvm_domain(d) )
>> {
>> - if ( update_paging_mode(d, dfn) )
>> + if ( update_paging_mode(d, dfn_x(dfn)) )
>> {
>> spin_unlock(&hd->arch.mapping_lock);
>> - AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn);
>> + AMD_IOMMU_DEBUG("Update page mode failed dfn = %"PRI_dfn"\n",
>> + dfn_x(dfn));
>> domain_crash(d);
>> return -EFAULT;
>> }
>> }
>>
>> - if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) )
>> + if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) )
>> {
>> spin_unlock(&hd->arch.mapping_lock);
>> - AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn);
>> + AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
>> + dfn_x(dfn));
>> domain_crash(d);
>> return -EFAULT;
>> }
>>
>> /* Install 4k mapping first */
>> - need_flush = set_iommu_pte_present(pt_mfn[1], dfn, mfn,
>> + need_flush = set_iommu_pte_present(pt_mfn[1], dfn_x(dfn), mfn_x(mfn),
>> IOMMU_PAGING_MODE_LEVEL_1,
>> !!(flags & IOMMUF_writable),
>> !!(flags & IOMMUF_readable));
>> @@ -690,7 +693,7 @@ int amd_iommu_map_page(struct domain *d, unsigned long
>> dfn, unsigned long mfn,
>> /* 4K mapping for PV guests never changes,
>> * no need to flush if we trust non-present bits */
>> if ( is_hvm_domain(d) )
>> - amd_iommu_flush_pages(d, dfn, 0);
>> + amd_iommu_flush_pages(d, dfn_x(dfn), 0);
>>
>> for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2;
>> merge_level <= hd->arch.paging_mode; merge_level++ )
>> @@ -698,15 +701,16 @@ int amd_iommu_map_page(struct domain *d, unsigned
>> long dfn, unsigned long mfn,
>> if ( pt_mfn[merge_level] == 0 )
>> break;
>> if ( !iommu_update_pde_count(d, pt_mfn[merge_level],
>> - dfn, mfn, merge_level) )
>> + dfn_x(dfn), mfn_x(mfn), merge_level)
>> )
>> break;
>>
>> - if ( iommu_merge_pages(d, pt_mfn[merge_level], dfn,
>> + if ( iommu_merge_pages(d, pt_mfn[merge_level], dfn_x(dfn),
>> flags, merge_level) )
>> {
>> spin_unlock(&hd->arch.mapping_lock);
>> AMD_IOMMU_DEBUG("Merge iommu page failed at level %d, "
>> - "dfn = %lx mfn = %lx\n", merge_level, dfn,
>> mfn);
>> + "dfn = %"PRI_dfn" mfn = %"PRI_mfn"\n",
>> + merge_level, dfn_x(dfn), mfn_x(mfn));
>> domain_crash(d);
>> return -EFAULT;
>> }
>> @@ -720,7 +724,7 @@ out:
>> return 0;
>> }
>>
>> -int amd_iommu_unmap_page(struct domain *d, unsigned long dfn)
>> +int amd_iommu_unmap_page(struct domain *d, dfn_t dfn)
>> {
>> unsigned long pt_mfn[7];
>> struct domain_iommu *hd = dom_iommu(d);
>> @@ -742,31 +746,33 @@ int amd_iommu_unmap_page(struct domain *d, unsigned
>> long dfn)
>> * we might need a deeper page table for lager dfn now */
>> if ( is_hvm_domain(d) )
>> {
>> - int rc = update_paging_mode(d, dfn);
>> + int rc = update_paging_mode(d, dfn_x(dfn));
>>
>> if ( rc )
>> {
>> spin_unlock(&hd->arch.mapping_lock);
>> - AMD_IOMMU_DEBUG("Update page mode failed dfn = %lx\n", dfn);
>> + AMD_IOMMU_DEBUG("Update page mode failed dfn = %"PRI_dfn"\n",
>> + dfn_x(dfn));
>> if ( rc != -EADDRNOTAVAIL )
>> domain_crash(d);
>> return rc;
>> }
>> }
>>
>> - if ( iommu_pde_from_dfn(d, dfn, pt_mfn) || (pt_mfn[1] == 0) )
>> + if ( iommu_pde_from_dfn(d, dfn_x(dfn), pt_mfn) || (pt_mfn[1] == 0) )
>> {
>> spin_unlock(&hd->arch.mapping_lock);
>> - AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %lx\n", dfn);
>> + AMD_IOMMU_DEBUG("Invalid IO pagetable entry dfn = %"PRI_dfn"\n",
>> + dfn_x(dfn));
>> domain_crash(d);
>> return -EFAULT;
>> }
>>
>> /* mark PTE as 'page not present' */
>> - clear_iommu_pte_present(pt_mfn[1], dfn);
>> + clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn));
>> spin_unlock(&hd->arch.mapping_lock);
>>
>> - amd_iommu_flush_pages(d, dfn, 0);
>> + amd_iommu_flush_pages(d, dfn_x(dfn), 0);
>>
>> return 0;
>> }
>> @@ -787,7 +793,9 @@ int amd_iommu_reserve_domain_unity_map(struct domain
>> *domain,
>> gfn = phys_addr >> PAGE_SHIFT;
>> for ( i = 0; i < npages; i++ )
>> {
>> - rt = amd_iommu_map_page(domain, gfn +i, gfn +i, flags);
>> + unsigned long frame = gfn + i;
>> +
>> + rt = amd_iommu_map_page(domain, _dfn(frame), _mfn(frame), flags);
>> if ( rt != 0 )
>> return rt;
>> }
>> diff --git a/xen/drivers/passthrough/arm/smmu.c
>> b/xen/drivers/passthrough/arm/smmu.c
>> index 1eda96a72a..53e5823d05 100644
>> --- a/xen/drivers/passthrough/arm/smmu.c
>> +++ b/xen/drivers/passthrough/arm/smmu.c
>> @@ -2550,8 +2550,7 @@ static int __must_check
>> arm_smmu_iotlb_flush_all(struct domain *d)
>> return 0;
>> }
>>
>> -static int __must_check arm_smmu_iotlb_flush(struct domain *d,
>> - unsigned long dfn,
>> +static int __must_check arm_smmu_iotlb_flush(struct domain *d, dfn_t dfn,
>> unsigned int page_count)
>> {
>> /* ARM SMMU v1 doesn't have flush by VMA and VMID */
>> @@ -2748,8 +2747,8 @@ static void arm_smmu_iommu_domain_teardown(struct
>> domain *d)
>> xfree(xen_domain);
>> }
>>
>> -static int __must_check arm_smmu_map_page(struct domain *d, unsigned long
>> dfn,
>> - unsigned long mfn, unsigned int flags)
>> +static int __must_check arm_smmu_map_page(struct domain *d, dfn_t dfn,
>> + mfn_t mfn, unsigned int flags)
>> {
>> p2m_type_t t;
>>
>> @@ -2762,7 +2761,7 @@ static int __must_check arm_smmu_map_page(struct
>> domain *d, unsigned long dfn,
>> * function should only be used by gnttab code with gfn == mfn ==
>> dfn.
>> */
>> BUG_ON(!is_domain_direct_mapped(d));
>> - BUG_ON(mfn != dfn);
>> + BUG_ON(mfn_x(mfn) != dfn_x(dfn));
>>
>> /* We only support readable and writable flags */
>> if (!(flags & (IOMMUF_readable | IOMMUF_writable)))
>> @@ -2774,10 +2773,11 @@ static int __must_check arm_smmu_map_page(struct
>> domain *d, unsigned long dfn,
>> * The function guest_physmap_add_entry replaces the current mapping
>> * if there is already one...
>> */
>> - return guest_physmap_add_entry(d, _gfn(dfn), _mfn(dfn), 0, t);
>> + return guest_physmap_add_entry(d, _gfn(dfn_x(dfn)),
>> _mfn(dfn_x(dfn)),
>> + 0, t);
>> }
>>
>> -static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned
>> long dfn)
>> +static int __must_check arm_smmu_unmap_page(struct domain *d, dfn_t dfn)
>> {
>> /*
>> * This function should only be used by gnttab code when the domain
>> @@ -2786,7 +2786,7 @@ static int __must_check arm_smmu_unmap_page(struct
>> domain *d, unsigned long dfn)
>> if ( !is_domain_direct_mapped(d) )
>> return -EINVAL;
>>
>> - return guest_physmap_remove_page(d, _gfn(dfn), _mfn(dfn), 0);
>> + return guest_physmap_remove_page(d, _gfn(dfn_x(dfn)),
>> _mfn(dfn_x(dfn)), 0);
>> }
>>
>> static const struct iommu_ops arm_smmu_iommu_ops = {
>> diff --git a/xen/drivers/passthrough/iommu.c
>> b/xen/drivers/passthrough/iommu.c
>> index ef9d651317..d61fbbf439 100644
>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -224,7 +224,8 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>> == PGT_writable_page) )
>> mapping |= IOMMUF_writable;
>>
>> - ret = hd->platform_ops->map_page(d, dfn, mfn, mapping);
>> + ret = hd->platform_ops->map_page(d, _dfn(dfn), _mfn(mfn),
>> + mapping);
>> if ( !rc )
>> rc = ret;
>>
>> @@ -294,7 +295,7 @@ int iommu_map_page(struct domain *d, dfn_t dfn, mfn_t
>> mfn,
>> if ( !iommu_enabled || !hd->platform_ops )
>> return 0;
>>
>> - rc = hd->platform_ops->map_page(d, dfn_x(dfn), mfn_x(mfn), flags);
>> + rc = hd->platform_ops->map_page(d, dfn, mfn, flags);
>> if ( unlikely(rc) )
>> {
>> if ( !d->is_shutting_down && printk_ratelimit() )
>> @@ -317,7 +318,7 @@ int iommu_unmap_page(struct domain *d, dfn_t dfn)
>> if ( !iommu_enabled || !hd->platform_ops )
>> return 0;
>>
>> - rc = hd->platform_ops->unmap_page(d, dfn_x(dfn));
>> + rc = hd->platform_ops->unmap_page(d, dfn);
>> if ( unlikely(rc) )
>> {
>> if ( !d->is_shutting_down && printk_ratelimit() )
>> @@ -357,7 +358,7 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn,
>> unsigned int page_count)
>> if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops-
>>> iotlb_flush )
>> return 0;
>>
>> - rc = hd->platform_ops->iotlb_flush(d, dfn_x(dfn), page_count);
>> + rc = hd->platform_ops->iotlb_flush(d, dfn, page_count);
>> if ( unlikely(rc) )
>> {
>> if ( !d->is_shutting_down && printk_ratelimit() )
>> diff --git a/xen/drivers/passthrough/vtd/iommu.c
>> b/xen/drivers/passthrough/vtd/iommu.c
>> index 485704bcce..7e185c43f3 100644
>> --- a/xen/drivers/passthrough/vtd/iommu.c
>> +++ b/xen/drivers/passthrough/vtd/iommu.c
>> @@ -584,8 +584,7 @@ static int __must_check iommu_flush_all(void)
>> return rc;
>> }
>>
>> -static int __must_check iommu_flush_iotlb(struct domain *d,
>> - unsigned long dfn,
>> +static int __must_check iommu_flush_iotlb(struct domain *d, dfn_t dfn,
>> bool_t dma_old_pte_present,
>> unsigned int page_count)
>> {
>> @@ -612,12 +611,12 @@ static int __must_check iommu_flush_iotlb(struct
>> domain *d,
>> if ( iommu_domid == -1 )
>> continue;
>>
>> - if ( page_count != 1 || dfn == dfn_x(INVALID_DFN) )
>> + if ( page_count != 1 || dfn_eq(dfn, INVALID_DFN) )
>> rc = iommu_flush_iotlb_dsi(iommu, iommu_domid,
>> 0, flush_dev_iotlb);
>> else
>> rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
>> - __dfn_to_daddr(dfn),
>> + dfn_to_daddr(dfn),
>> PAGE_ORDER_4K,
>> !dma_old_pte_present,
>> flush_dev_iotlb);
>> @@ -633,7 +632,7 @@ static int __must_check iommu_flush_iotlb(struct
>> domain *d,
>> }
>>
>> static int __must_check iommu_flush_iotlb_pages(struct domain *d,
>> - unsigned long dfn,
>> + dfn_t dfn,
>> unsigned int page_count)
>> {
>> return iommu_flush_iotlb(d, dfn, 1, page_count);
>> @@ -641,7 +640,7 @@ static int __must_check iommu_flush_iotlb_pages(struct
>> domain *d,
>>
>> static int __must_check iommu_flush_iotlb_all(struct domain *d)
>> {
>> - return iommu_flush_iotlb(d, dfn_x(INVALID_DFN), 0, 0);
>> + return iommu_flush_iotlb(d, INVALID_DFN, 0, 0);
>> }
>>
>> /* clear one page's page table */
>> @@ -676,7 +675,7 @@ static int __must_check dma_pte_clear_one(struct
>> domain *domain, u64 addr)
>> iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
>>
>> if ( !this_cpu(iommu_dont_flush_iotlb) )
>> - rc = iommu_flush_iotlb_pages(domain, addr >> PAGE_SHIFT_4K, 1);
>> + rc = iommu_flush_iotlb_pages(domain, daddr_to_dfn(addr), 1);
>>
>> unmap_vtd_domain_page(page);
>>
>> @@ -1763,8 +1762,7 @@ static void iommu_domain_teardown(struct domain *d)
>> }
>>
>> static int __must_check intel_iommu_map_page(struct domain *d,
>> - unsigned long dfn,
>> - unsigned long mfn,
>> + dfn_t dfn, mfn_t mfn,
>> unsigned int flags)
>> {
>> struct domain_iommu *hd = dom_iommu(d);
>> @@ -1782,16 +1780,16 @@ static int __must_check
>> intel_iommu_map_page(struct domain *d,
>>
>> spin_lock(&hd->arch.mapping_lock);
>>
>> - pg_maddr = addr_to_dma_page_maddr(d, __dfn_to_daddr(dfn), 1);
>> + pg_maddr = addr_to_dma_page_maddr(d, dfn_to_daddr(dfn), 1);
>> if ( pg_maddr == 0 )
>> {
>> spin_unlock(&hd->arch.mapping_lock);
>> return -ENOMEM;
>> }
>> page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
>> - pte = page + (dfn & LEVEL_MASK);
>> + pte = page + (dfn_x(dfn) & LEVEL_MASK);
>> old = *pte;
>> - dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
>> + dma_set_pte_addr(new, mfn_to_maddr(mfn));
>> dma_set_pte_prot(new,
>> ((flags & IOMMUF_readable) ? DMA_PTE_READ : 0) |
>> ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
>> @@ -1819,13 +1817,13 @@ static int __must_check
>> intel_iommu_map_page(struct domain *d,
>> }
>>
>> static int __must_check intel_iommu_unmap_page(struct domain *d,
>> - unsigned long dfn)
>> + dfn_t dfn)
>> {
>> /* Do nothing if hardware domain and iommu supports pass thru. */
>> if ( iommu_hwdom_passthrough && is_hardware_domain(d) )
>> return 0;
>>
>> - return dma_pte_clear_one(d, __dfn_to_daddr(dfn));
>> + return dma_pte_clear_one(d, dfn_to_daddr(dfn));
>> }
>>
>> int iommu_pte_flush(struct domain *d, uint64_t dfn, uint64_t *pte,
>> diff --git a/xen/drivers/passthrough/x86/iommu.c
>> b/xen/drivers/passthrough/x86/iommu.c
>> index 34727b6653..f410717a59 100644
>> --- a/xen/drivers/passthrough/x86/iommu.c
>> +++ b/xen/drivers/passthrough/x86/iommu.c
>> @@ -68,7 +68,7 @@ int arch_iommu_populate_page_table(struct domain *d)
>> {
>> ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
>> BUG_ON(SHARED_M2P(gfn));
>> - rc = hd->platform_ops->map_page(d, gfn, mfn,
>> + rc = hd->platform_ops->map_page(d, _dfn(gfn), _mfn(mfn),
>> IOMMUF_readable |
>> IOMMUF_writable);
>> }
>> diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
>> b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
>> index 99bc21c7b3..3083d625bd 100644
>> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
>> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
>> @@ -52,9 +52,9 @@ int amd_iommu_init(void);
>> int amd_iommu_update_ivrs_mapping_acpi(void);
>>
>> /* mapping functions */
>> -int __must_check amd_iommu_map_page(struct domain *d, unsigned long gfn,
>> - unsigned long mfn, unsigned int
>> flags);
>> -int __must_check amd_iommu_unmap_page(struct domain *d, unsigned long
>> gfn);
>> +int __must_check amd_iommu_map_page(struct domain *d, dfn_t dfn,
>> + mfn_t mfn, unsigned int flags);
>> +int __must_check amd_iommu_unmap_page(struct domain *d, dfn_t dfn);
>> u64 amd_iommu_get_next_table_from_pte(u32 *entry);
>> int __must_check amd_iommu_alloc_root(struct domain_iommu *hd);
>> int amd_iommu_reserve_domain_unity_map(struct domain *domain,
>> @@ -77,7 +77,7 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id, u64
>> gcr3,
>>
>> /* send cmd to iommu */
>> void amd_iommu_flush_all_pages(struct domain *d);
>> -void amd_iommu_flush_pages(struct domain *d, unsigned long gfn,
>> +void amd_iommu_flush_pages(struct domain *d, unsigned long dfn,
>> unsigned int order);
>> void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
>> uint64_t gaddr, unsigned int order);
>> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
>> index f9d86fc816..7313957c81 100644
>> --- a/xen/include/xen/iommu.h
>> +++ b/xen/include/xen/iommu.h
>> @@ -48,6 +48,11 @@ static inline dfn_t dfn_add(dfn_t dfn, unsigned long i)
>> return _dfn(dfn_x(dfn) + i);
>> }
>>
>> +static inline bool_t dfn_eq(dfn_t x, dfn_t y)
>> +{
>> + return dfn_x(x) == dfn_x(y);
>> +}
>> +
>> extern bool_t iommu_enable, iommu_enabled;
>> extern bool_t force_iommu, iommu_verbose;
>> extern bool_t iommu_workaround_bios_bug, iommu_igfx;
>> @@ -174,9 +179,9 @@ struct iommu_ops {
>> #endif /* HAS_PCI */
>>
>> void (*teardown)(struct domain *d);
>> - int __must_check (*map_page)(struct domain *d, unsigned long dfn,
>> - unsigned long mfn, unsigned int flags);
>> - int __must_check (*unmap_page)(struct domain *d, unsigned long dfn);
>> + int __must_check (*map_page)(struct domain *d, dfn_t dfn, mfn_t mfn,
>> + unsigned int flags);
>> + int __must_check (*unmap_page)(struct domain *d, dfn_t dfn);
>> void (*free_page_table)(struct page_info *);
>> #ifdef CONFIG_X86
>> void (*update_ire_from_apic)(unsigned int apic, unsigned int reg,
>> unsigned int value);
>> @@ -187,7 +192,7 @@ struct iommu_ops {
>> void (*resume)(void);
>> void (*share_p2m)(struct domain *d);
>> void (*crash_shutdown)(void);
>> - int __must_check (*iotlb_flush)(struct domain *d, unsigned long dfn,
>> + int __must_check (*iotlb_flush)(struct domain *d, dfn_t dfn,
>> unsigned int page_count);
>> int __must_check (*iotlb_flush_all)(struct domain *d);
>> int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
>> --
>> 2.11.0
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt()
2018-10-04 10:45 ` [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt() Paul Durrant
@ 2018-10-04 14:02 ` Jan Beulich
2018-10-04 16:23 ` George Dunlap
2018-10-05 16:03 ` Wei Liu
2 siblings, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2018-10-04 14:02 UTC (permalink / raw)
To: Paul Durrant
Cc: George Dunlap, Andrew Cooper, Stefano Stabellini, Jun Nakajima,
xen-devel
>>> On 04.10.18 at 12:45, <paul.durrant@citrix.com> wrote:
> The name 'iommu_use_hap_pt' suggests that that P2M table is in use as the
> domain's IOMMU pagetable which, prior to this patch, is not strictly true
> since the macro did not test whether the domain actually has IOMMU
> mappings.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync()
2018-10-04 10:45 ` [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync() Paul Durrant
@ 2018-10-04 14:11 ` Jan Beulich
2018-10-04 16:23 ` George Dunlap
` (2 subsequent siblings)
3 siblings, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2018-10-04 14:11 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Tamas K Lengyel, Suravee Suthikulpanit,
xen-devel, Brian Woods
>>> On 04.10.18 at 12:45, <paul.durrant@citrix.com> wrote:
> The name 'need_iommu()' is a little confusing as it suggests a domain needs
> to use the IOMMU but something might not be set up yet, when in fact it
> represents a tri-state value (not a boolean as might be expected) where
> -1 means 'IOMMU mappings being set up' and 1 means 'IOMMU mappings have
> been fully set up'.
>
> Two different meanings are also inferred from the macro it in various
> places in the code:
>
> - Some callers want to test whether a domain has IOMMU mappings at all
> - Some callers want to test whether they need to synchronize the domain's
> P2M and IOMMU mappings
>
> This patch replaces the 'need_iommu' tri-state value with a defined
> enumeration and adds a boolean flag 'need_sync' to separate these meanings,
> and places both of these in struct domain_iommu, rather than directly in
> struct domain.
> This patch also creates two new boolean macros:
>
> - 'has_iommu_pt()' evaluates to true if a domain has IOMMU mappings, even
> if they are still under construction.
> - 'need_iommu_pt_sync()' evaluates to true if a domain requires explicit
> synchronization of the P2M and IOMMU mappings.
>
> All callers of need_iommu() are then modified to use the macro appropriate
> to what they are trying to test, except for the instance in
> xen/drivers/passthrough/pci.c:assign_device() which has simply been
> removed since it appears to be unnecessary.
>
> NOTE: There are some callers of need_iommu() that strictly operate on
> the hardware domain. In some of these case a more global flag is
> used instead.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Except for the Arm parts
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 2/9] iommu: make use of type-safe DFN and MFN in exported functions
2018-10-04 10:45 ` [PATCH v14 2/9] iommu: make use of type-safe DFN and MFN in exported functions Paul Durrant
@ 2018-10-04 15:14 ` George Dunlap
0 siblings, 0 replies; 34+ messages in thread
From: George Dunlap @ 2018-10-04 15:14 UTC (permalink / raw)
To: Paul Durrant, xen-devel
Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, George Dunlap,
Andrew Cooper, Tim Deegan, Jun Nakajima, Ian Jackson
On 10/04/2018 11:45 AM, Paul Durrant wrote:
> This patch modifies the declaration of the entry points to the IOMMU
> sub-system to use dfn_t and mfn_t in place of unsigned long. A subsequent
> patch will similarly modify the methods in the iommu_ops structure.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Reviewed-by: Roger Pau Monne <roger.pau@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 5/9] memory: add check_get_page_from_gfn() as a wrapper...
2018-10-04 10:45 ` [PATCH v14 5/9] memory: add check_get_page_from_gfn() as a wrapper Paul Durrant
@ 2018-10-04 16:02 ` George Dunlap
0 siblings, 0 replies; 34+ messages in thread
From: George Dunlap @ 2018-10-04 16:02 UTC (permalink / raw)
To: Paul Durrant, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Tim Deegan, Ian Jackson
On 10/04/2018 11:45 AM, Paul Durrant wrote:
> ...for some uses of get_page_from_gfn().
>
> There are many occurrences of the following pattern in the code:
>
> q = <readonly look-up> ? P2M_ALLOC : P2M_UNSHARE;
> page = get_page_from_gfn(d, gfn, &p2mt, q);
>
> if ( p2m_is_paging(p2mt) )
> {
> if ( page )
> put_page(page);
>
> p2m_mem_paging_populate(d, gfn);
> return <-EAGAIN or equivalent>;
> }
>
> if ( (q & P2M_UNSHARE) && p2m_is_shared(p2mt) )
> {
> if ( page )
> put_page(page);
>
> return <-EAGAIN or equivalent>;
> }
>
> if ( !page )
> return <-EINVAL or equivalent>;
>
> There are some small differences between the exact way the occurrences
> are coded but the desired semantic is the same.
>
> This patch introduces a new common implementation of this code in
> check_get_page_from_gfn() and then converts the various open-coded patterns
> into calls to this new function.
>
> NOTE: A forward declaration of p2m_type_t enum has been introduced in
> p2m-common.h so that it is possible to declare
> check_get_page_from_gfn() there rather than having to add
> duplicate declarations in the per-architecture p2m headers.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: Roger Pau Monne <roger.pau@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
>
> v13:
> - Expanded commit comment.
>
> v11:
> - Forward declare p2m_type_t in p2m-common.h to allow the duplicate
> declarations of check_get_page_from_gfn() to be removed, and hence add
> Jan's R-b.
>
> v10:
> - Expand commit comment to point out the reason for the duplicate
> declarations of check_get_page_from_gfn().
>
> v9:
> - Defer P2M type checks (beyond shared or paging) to the caller.
>
> v7:
> - Fix ARM build by introducing p2m_is_readonly() predicate.
> - Re-name get_paged_frame() -> check_get_page_from_gfn().
> - Adjust default cases of callers switch-ing on return value.
>
> v3:
> - Addressed comments from George.
>
> v2:
> - New in v2.
> ---
> xen/arch/x86/hvm/emulate.c | 25 +++++++++++-----------
> xen/arch/x86/hvm/hvm.c | 14 +------------
> xen/common/grant_table.c | 32 ++++++++++++++---------------
> xen/common/memory.c | 49 +++++++++++++++++++++++++++++++++++---------
> xen/include/asm-arm/p2m.h | 4 ++--
> xen/include/asm-x86/p2m.h | 5 ++---
> xen/include/xen/p2m-common.h | 6 ++++++
> 7 files changed, 77 insertions(+), 58 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index eab66eab77..cd1d9a7c57 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -356,22 +356,21 @@ static int hvmemul_acquire_page(unsigned long gmfn, struct page_info **page)
> struct domain *curr_d = current->domain;
> p2m_type_t p2mt;
>
> - *page = get_page_from_gfn(curr_d, gmfn, &p2mt, P2M_UNSHARE);
> -
> - if ( *page == NULL )
> - return X86EMUL_UNHANDLEABLE;
> -
> - if ( p2m_is_paging(p2mt) )
> + switch ( check_get_page_from_gfn(curr_d, _gfn(gmfn), false, &p2mt,
> + page) )
> {
> - put_page(*page);
> - p2m_mem_paging_populate(curr_d, gmfn);
> - return X86EMUL_RETRY;
> - }
> + case 0:
> + break;
>
> - if ( p2m_is_shared(p2mt) )
> - {
> - put_page(*page);
> + case -EAGAIN:
> return X86EMUL_RETRY;
> +
> + default:
> + ASSERT_UNREACHABLE();
> + /* Fallthrough */
> +
> + case -EINVAL:
> + return X86EMUL_UNHANDLEABLE;
> }
>
> /* This code should not be reached if the gmfn is not RAM */
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 51fc3ec07f..fa994a36a4 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2536,20 +2536,8 @@ static void *_hvm_map_guest_frame(unsigned long gfn, bool_t permanent,
> struct page_info *page;
> struct domain *d = current->domain;
>
> - page = get_page_from_gfn(d, gfn, &p2mt,
> - writable ? P2M_UNSHARE : P2M_ALLOC);
> - if ( (p2m_is_shared(p2mt) && writable) || !page )
> - {
> - if ( page )
> - put_page(page);
> - return NULL;
> - }
> - if ( p2m_is_paging(p2mt) )
> - {
> - put_page(page);
> - p2m_mem_paging_populate(d, gfn);
> + if ( check_get_page_from_gfn(d, _gfn(gfn), !writable, &p2mt, &page) )
> return NULL;
> - }
>
> if ( writable )
> {
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 0f0b7b1a49..3604a8812c 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -374,25 +374,23 @@ static int get_paged_frame(unsigned long gfn, mfn_t *mfn,
> struct page_info **page, bool readonly,
> struct domain *rd)
> {
> - int rc = GNTST_okay;
> p2m_type_t p2mt;
> + int rc;
>
> - *mfn = INVALID_MFN;
> - *page = get_page_from_gfn(rd, gfn, &p2mt,
> - readonly ? P2M_ALLOC : P2M_UNSHARE);
> - if ( !*page )
> + rc = check_get_page_from_gfn(rd, _gfn(gfn), readonly, &p2mt, page);
> + switch ( rc )
> {
> -#ifdef P2M_SHARED_TYPES
> - if ( p2m_is_shared(p2mt) )
> - return GNTST_eagain;
> -#endif
> -#ifdef P2M_PAGES_TYPES
> - if ( p2m_is_paging(p2mt) )
> - {
> - p2m_mem_paging_populate(rd, gfn);
> - return GNTST_eagain;
> - }
> -#endif
> + case 0:
> + break;
> +
> + case -EAGAIN:
> + return GNTST_eagain;
> +
> + default:
> + ASSERT_UNREACHABLE();
> + /* Fallthrough */
> +
> + case -EINVAL:
> return GNTST_bad_page;
> }
>
> @@ -406,7 +404,7 @@ static int get_paged_frame(unsigned long gfn, mfn_t *mfn,
>
> *mfn = page_to_mfn(*page);
>
> - return rc;
> + return GNTST_okay;
> }
>
> static inline void
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index b567bd46bb..52d55dfed7 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -1626,37 +1626,66 @@ void destroy_ring_for_helper(
> }
> }
>
> -int prepare_ring_for_helper(
> - struct domain *d, unsigned long gmfn, struct page_info **_page,
> - void **_va)
> +/*
> + * Acquire a pointer to struct page_info for a specified doman and GFN,
> + * checking whether the page has been paged out, or needs unsharing.
> + * If the function succeeds then zero is returned, page_p is written
> + * with a pointer to the struct page_info with a reference taken, and
> + * p2mt_p it is written with the P2M type of the page. The caller is
> + * responsible for dropping the reference.
> + * If the function fails then an appropriate errno is returned and the
> + * values referenced by page_p and p2mt_p are undefined.
> + */
> +int check_get_page_from_gfn(struct domain *d, gfn_t gfn, bool readonly,
> + p2m_type_t *p2mt_p, struct page_info **page_p)
> {
> - struct page_info *page;
> + p2m_query_t q = readonly ? P2M_ALLOC : P2M_UNSHARE;
> p2m_type_t p2mt;
> - void *va;
> + struct page_info *page;
>
> - page = get_page_from_gfn(d, gmfn, &p2mt, P2M_UNSHARE);
> + page = get_page_from_gfn(d, gfn_x(gfn), &p2mt, q);
>
> #ifdef CONFIG_HAS_MEM_PAGING
> if ( p2m_is_paging(p2mt) )
> {
> if ( page )
> put_page(page);
> - p2m_mem_paging_populate(d, gmfn);
> - return -ENOENT;
> +
> + p2m_mem_paging_populate(d, gfn_x(gfn));
> + return -EAGAIN;
> }
> #endif
> #ifdef CONFIG_HAS_MEM_SHARING
> - if ( p2m_is_shared(p2mt) )
> + if ( (q & P2M_UNSHARE) && p2m_is_shared(p2mt) )
> {
> if ( page )
> put_page(page);
> - return -ENOENT;
> +
> + return -EAGAIN;
> }
> #endif
>
> if ( !page )
> return -EINVAL;
>
> + *p2mt_p = p2mt;
> + *page_p = page;
> + return 0;
> +}
> +
> +int prepare_ring_for_helper(
> + struct domain *d, unsigned long gmfn, struct page_info **_page,
> + void **_va)
> +{
> + p2m_type_t p2mt;
> + struct page_info *page;
> + void *va;
> + int rc;
> +
> + rc = check_get_page_from_gfn(d, _gfn(gmfn), false, &p2mt, &page);
> + if ( rc )
> + return (rc == -EAGAIN) ? -ENOENT : rc;
> +
> if ( !get_page_type(page, PGT_writable_page) )
> {
> put_page(page);
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 8823707c17..c03557544a 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -110,7 +110,7 @@ struct p2m_domain {
> * future, it's possible to use higher value for pseudo-type and don't store
> * them in the p2m entry.
> */
> -typedef enum {
> +enum p2m_type {
> p2m_invalid = 0, /* Nothing mapped here */
> p2m_ram_rw, /* Normal read/write guest RAM */
> p2m_ram_ro, /* Read-only; writes are silently dropped */
> @@ -124,7 +124,7 @@ typedef enum {
> p2m_iommu_map_rw, /* Read/write iommu mapping */
> p2m_iommu_map_ro, /* Read-only iommu mapping */
> p2m_max_real_type, /* Types after this won't be store in the p2m */
> -} p2m_type_t;
> +};
>
> /* We use bitmaps and mask to handle groups of types */
> #define p2m_to_mask(_t) (1UL << (_t))
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index be3b6fcaf0..d08c595887 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -52,7 +52,7 @@ extern bool_t opt_hap_1gb, opt_hap_2mb;
> * cannot be non-zero, otherwise, hardware generates io page faults when
> * device access those pages. Therefore, p2m_ram_rw has to be defined as 0.
> */
> -typedef enum {
> +enum p2m_type {
> p2m_ram_rw = 0, /* Normal read/write guest RAM */
> p2m_invalid = 1, /* Nothing mapped here */
> p2m_ram_logdirty = 2, /* Temporarily read-only for log-dirty */
> @@ -72,7 +72,7 @@ typedef enum {
> p2m_ram_broken = 13, /* Broken page, access cause domain crash */
> p2m_map_foreign = 14, /* ram pages from foreign domain */
> p2m_ioreq_server = 15,
> -} p2m_type_t;
> +};
>
> /* Modifiers to the query */
> typedef unsigned int p2m_query_t;
> @@ -503,7 +503,6 @@ static inline struct page_info *get_page_from_gfn(
> return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL;
> }
>
> -
> /* General conversion function from mfn to gfn */
> static inline unsigned long mfn_to_gfn(struct domain *d, mfn_t mfn)
> {
> diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
> index 74311950ad..f4d30efe5f 100644
> --- a/xen/include/xen/p2m-common.h
> +++ b/xen/include/xen/p2m-common.h
> @@ -32,5 +32,11 @@ unsigned long
> p2m_pod_decrease_reservation(struct domain *d, gfn_t gfn,
> unsigned int order);
>
> +typedef enum p2m_type p2m_type_t;
> +
> +int __must_check check_get_page_from_gfn(struct domain *d, gfn_t gfn,
> + bool readonly, p2m_type_t *p2mt_p,
> + struct page_info **page_p);
> +
>
> #endif /* _XEN_P2M_COMMON_H */
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync()
2018-10-04 10:45 ` [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync() Paul Durrant
2018-10-04 14:11 ` Jan Beulich
@ 2018-10-04 16:23 ` George Dunlap
2018-10-04 17:15 ` Julien Grall
2018-10-05 14:50 ` Jan Beulich
3 siblings, 0 replies; 34+ messages in thread
From: George Dunlap @ 2018-10-04 16:23 UTC (permalink / raw)
To: Paul Durrant, xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Tamas K Lengyel, Jan Beulich,
Brian Woods, Suravee Suthikulpanit
On 10/04/2018 11:45 AM, Paul Durrant wrote:
> The name 'need_iommu()' is a little confusing as it suggests a domain needs
> to use the IOMMU but something might not be set up yet, when in fact it
> represents a tri-state value (not a boolean as might be expected) where
> -1 means 'IOMMU mappings being set up' and 1 means 'IOMMU mappings have
> been fully set up'.
>
> Two different meanings are also inferred from the macro it in various
> places in the code:
>
> - Some callers want to test whether a domain has IOMMU mappings at all
> - Some callers want to test whether they need to synchronize the domain's
> P2M and IOMMU mappings
>
> This patch replaces the 'need_iommu' tri-state value with a defined
> enumeration and adds a boolean flag 'need_sync' to separate these meanings,
> and places both of these in struct domain_iommu, rather than directly in
> struct domain.
> This patch also creates two new boolean macros:
>
> - 'has_iommu_pt()' evaluates to true if a domain has IOMMU mappings, even
> if they are still under construction.
> - 'need_iommu_pt_sync()' evaluates to true if a domain requires explicit
> synchronization of the P2M and IOMMU mappings.
>
> All callers of need_iommu() are then modified to use the macro appropriate
> to what they are trying to test, except for the instance in
> xen/drivers/passthrough/pci.c:assign_device() which has simply been
> removed since it appears to be unnecessary.
>
> NOTE: There are some callers of need_iommu() that strictly operate on
> the hardware domain. In some of these case a more global flag is
> used instead.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Wow -- looks like good work:
x86/p2m bits:
Acked-by: George Dunlap <george.dunlap@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt()
2018-10-04 10:45 ` [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt() Paul Durrant
2018-10-04 14:02 ` Jan Beulich
@ 2018-10-04 16:23 ` George Dunlap
2018-10-05 16:03 ` Wei Liu
2 siblings, 0 replies; 34+ messages in thread
From: George Dunlap @ 2018-10-04 16:23 UTC (permalink / raw)
To: Paul Durrant, xen-devel
Cc: George Dunlap, Andrew Cooper, Stefano Stabellini, Jan Beulich,
Jun Nakajima
On 10/04/2018 11:45 AM, Paul Durrant wrote:
> The name 'iommu_use_hap_pt' suggests that that P2M table is in use as the
> domain's IOMMU pagetable which, prior to this patch, is not strictly true
> since the macro did not test whether the domain actually has IOMMU
> mappings.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Acked-by: Julien Grall <julien.grall@arm.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
2018-10-04 10:45 ` [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page() Paul Durrant
@ 2018-10-04 16:29 ` George Dunlap
2018-10-04 16:36 ` Paul Durrant
0 siblings, 1 reply; 34+ messages in thread
From: George Dunlap @ 2018-10-04 16:29 UTC (permalink / raw)
To: Paul Durrant, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Tim Deegan, Jun Nakajima,
Ian Jackson
On 10/04/2018 11:45 AM, Paul Durrant wrote:
> This patch removes the implicit domain_crash() from iommu_map(),
> unmap_page() and iommu_iotlb_flush() and turns them into straightforward
> wrappers that check the existence of the relevant iommu_op and call
> through to it. This makes them usable by PV IOMMU code to be delivered in
> future patches.
Hmm, apparently I gave this an R-b before, but now I'm not totally happy
with it. The point of putting the domain_crash() inside those functions
was because forgetting to crash the domain, particularly in the event of
an unmap or a flush, was very likely to be a security issue.
Would it be possible to either add a `mayfail` parameter, or a new
function (iommu_map_mayfail() or something), that the PV IOMMU code
could use instead?
It looks like git is comfortable putting this patch at the end; all the
other patches look like they probably have enough acks to go in while we
discuss this one.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
2018-10-04 16:29 ` George Dunlap
@ 2018-10-04 16:36 ` Paul Durrant
2018-10-05 7:33 ` Jan Beulich
0 siblings, 1 reply; 34+ messages in thread
From: Paul Durrant @ 2018-10-04 16:36 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Jun Nakajima, Ian Jackson
> -----Original Message-----
> From: George Dunlap [mailto:george.dunlap@citrix.com]
> Sent: 04 October 2018 17:30
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Wei Liu <wei.liu2@citrix.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian
> Jackson <Ian.Jackson@citrix.com>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Jun Nakajima
> <jun.nakajima@intel.com>; George Dunlap <George.Dunlap@citrix.com>
> Subject: Re: [PATCH v14 4/9] iommu: don't domain_crash() inside
> iommu_map/unmap_page()
>
> On 10/04/2018 11:45 AM, Paul Durrant wrote:
> > This patch removes the implicit domain_crash() from iommu_map(),
> > unmap_page() and iommu_iotlb_flush() and turns them into straightforward
> > wrappers that check the existence of the relevant iommu_op and call
> > through to it. This makes them usable by PV IOMMU code to be delivered
> in
> > future patches.
>
> Hmm, apparently I gave this an R-b before, but now I'm not totally happy
> with it. The point of putting the domain_crash() inside those functions
> was because forgetting to crash the domain, particularly in the event of
> an unmap or a flush, was very likely to be a security issue.
>
> Would it be possible to either add a `mayfail` parameter, or a new
> function (iommu_map_mayfail() or something), that the PV IOMMU code
> could use instead?
>
> It looks like git is comfortable putting this patch at the end; all the
> other patches look like they probably have enough acks to go in while we
> discuss this one.
>
I still think an implicit domain_crash() doesn't really belong in something that looks like a straightforward wrapper around a per-implementation jump table. How about iommu_map/unmap_may_crash() instead to highlight the semantic?
Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync()
2018-10-04 10:45 ` [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync() Paul Durrant
2018-10-04 14:11 ` Jan Beulich
2018-10-04 16:23 ` George Dunlap
@ 2018-10-04 17:15 ` Julien Grall
2018-10-05 14:50 ` Jan Beulich
3 siblings, 0 replies; 34+ messages in thread
From: Julien Grall @ 2018-10-04 17:15 UTC (permalink / raw)
To: Paul Durrant, xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Tamas K Lengyel, Jan Beulich, Brian Woods,
Suravee Suthikulpanit
Hi Paul,
On 04/10/2018 11:45, Paul Durrant wrote:
> The name 'need_iommu()' is a little confusing as it suggests a domain needs
> to use the IOMMU but something might not be set up yet, when in fact it
> represents a tri-state value (not a boolean as might be expected) where
> -1 means 'IOMMU mappings being set up' and 1 means 'IOMMU mappings have
> been fully set up'.
>
> Two different meanings are also inferred from the macro it in various
> places in the code:
>
> - Some callers want to test whether a domain has IOMMU mappings at all
> - Some callers want to test whether they need to synchronize the domain's
> P2M and IOMMU mappings
>
> This patch replaces the 'need_iommu' tri-state value with a defined
> enumeration and adds a boolean flag 'need_sync' to separate these meanings,
> and places both of these in struct domain_iommu, rather than directly in
> struct domain.
> This patch also creates two new boolean macros:
>
> - 'has_iommu_pt()' evaluates to true if a domain has IOMMU mappings, even
> if they are still under construction.
> - 'need_iommu_pt_sync()' evaluates to true if a domain requires explicit
> synchronization of the P2M and IOMMU mappings.
>
> All callers of need_iommu() are then modified to use the macro appropriate
> to what they are trying to test, except for the instance in
> xen/drivers/passthrough/pci.c:assign_device() which has simply been
> removed since it appears to be unnecessary.
>
> NOTE: There are some callers of need_iommu() that strictly operate on
> the hardware domain. In some of these case a more global flag is
> used instead.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
For the Arm bits:
Acked-by: Julien Grall <julien.grall@arm.com>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
2018-10-04 16:36 ` Paul Durrant
@ 2018-10-05 7:33 ` Jan Beulich
2018-10-05 9:02 ` Paul Durrant
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2018-10-05 7:33 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
Andrew Cooper, Tim Deegan, george.dunlap, Jun Nakajima,
xen-devel, Ian Jackson
>>> On 04.10.18 at 18:36, <Paul.Durrant@citrix.com> wrote:
> I still think an implicit domain_crash() doesn't really belong in something
> that looks like a straightforward wrapper around a per-implementation jump
> table. How about iommu_map/unmap_may_crash() instead to highlight the
> semantic?
If anything then the other way around, i.e. iommu_unmap_no_crash(),
such that only callers who explicitly mean to deal with the crashing
themselves would use the otherwise insecure variant.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
2018-10-05 7:33 ` Jan Beulich
@ 2018-10-05 9:02 ` Paul Durrant
[not found] ` <F5DD545D-610B-40C7-827C-4CEAA2DF5F04@citrix.com>
0 siblings, 1 reply; 34+ messages in thread
From: Paul Durrant @ 2018-10-05 9:02 UTC (permalink / raw)
To: 'Jan Beulich'
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Jun Nakajima, xen-devel, Ian Jackson
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 05 October 2018 08:33
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Wei Liu
> <wei.liu2@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>; Stefano
> Stabellini <sstabellini@kernel.org>; xen-devel <xen-
> devel@lists.xenproject.org>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>
> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
> inside iommu_map/unmap_page()
>
> >>> On 04.10.18 at 18:36, <Paul.Durrant@citrix.com> wrote:
> > I still think an implicit domain_crash() doesn't really belong in
> something
> > that looks like a straightforward wrapper around a per-implementation
> jump
> > table. How about iommu_map/unmap_may_crash() instead to highlight the
> > semantic?
>
> If anything then the other way around, i.e. iommu_unmap_no_crash(),
> such that only callers who explicitly mean to deal with the crashing
> themselves would use the otherwise insecure variant.
>
Ok. George, what is your preference?
At this point my proposal is to drop this patch and replace it with one that removes the implicit crash from from everything except the unmap. I can then introduce a 'nocrash' variant of unmap if I need it... although I'm no longer convinced I can really do anything else if a PV-IOMMU unmap fails.
Paul
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
[not found] ` <b6add5ff3b6647eaab63265dbaf923bd@AMSPEX02CL03.citrite.net>
@ 2018-10-05 10:31 ` Paul Durrant
[not found] ` <28341D09-548F-460C-8B22-43CA61EA1308@citrix.com>
1 sibling, 0 replies; 34+ messages in thread
From: Paul Durrant @ 2018-10-05 10:31 UTC (permalink / raw)
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
Andrew Cooper, Tim (Xen.org),
George Dunlap, Jun Nakajima, Ian Jackson, xen-devel
Re-adding the cc-list...
> -----Original Message-----
> From: Paul Durrant
> Sent: 05 October 2018 11:27
> To: George Dunlap <George.Dunlap@citrix.com>
> Subject: RE: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
> inside iommu_map/unmap_page()
>
> > -----Original Message-----
> > From: George Dunlap
> > Sent: 05 October 2018 11:25
> > To: Paul Durrant <Paul.Durrant@citrix.com>
> > Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
> > inside iommu_map/unmap_page()
> >
> > [Sorry, my mail client crashed and I can’t figure out how to make it re-
> > edit this draft, so I’m replying to it instead.]
> >
> > > On Oct 5, 2018, at 11:22 AM, George Dunlap <George.Dunlap@citrix.com>
> > wrote:
> > >
> > >
> > >
> > >> On Oct 5, 2018, at 10:02 AM, Paul Durrant <Paul.Durrant@citrix.com>
> > wrote:
> > >>
> > >>> -----Original Message-----
> > >>> From: Jan Beulich [mailto:JBeulich@suse.com]
> > >>> Sent: 05 October 2018 08:33
> > >>> To: Paul Durrant <Paul.Durrant@citrix.com>
> > >>> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> > >>> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> Wei
> > Liu
> > >>> <wei.liu2@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>;
> Stefano
> > >>> Stabellini <sstabellini@kernel.org>; xen-devel <xen-
> > >>> devel@lists.xenproject.org>; Konrad Rzeszutek Wilk
> > >>> <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>
> > >>> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
> > >>> inside iommu_map/unmap_page()
> > >>>
> > >>>>>> On 04.10.18 at 18:36, <Paul.Durrant@citrix.com> wrote:
> > >>>> I still think an implicit domain_crash() doesn't really belong in
> > >>> something
> > >>>> that looks like a straightforward wrapper around a per-
> implementation
> > >>> jump
> > >>>> table. How about iommu_map/unmap_may_crash() instead to highlight
> the
> > >>>> semantic?
> > >>>
> > >>> If anything then the other way around, i.e. iommu_unmap_no_crash(),
> > >>> such that only callers who explicitly mean to deal with the crashing
> > >>> themselves would use the otherwise insecure variant.
> > >>>
> > >>
> > >> Ok. George, what is your preference?
> > >>
> > >> At this point my proposal is to drop this patch and replace it with
> one
> > that removes the implicit crash from from everything except the unmap. I
> > can then introduce a 'nocrash' variant of unmap if I need it... although
> > I'm no longer convinced I can really do anything else if a PV-IOMMU
> unmap
> > fails.
> > >
> > > Sorry, ‘mayfail’ was meant to be short for “may fail [without crashing
> > the guest]”; as opposed to “must succeed [or crash the guest]”. IOW, I
> > agree with Jan that the default should be to crash the guest unless the
> > caller explicitly opts to handle the failure themselves. Don’t have a
> > strong opinion on the name.
>
> But for mapping too? It seems unnecessary to crash the domain in that
> case.
>
> Paul
>
> > >
> > > -George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
[not found] ` <28341D09-548F-460C-8B22-43CA61EA1308@citrix.com>
@ 2018-10-05 10:37 ` George Dunlap
2018-10-05 10:38 ` Paul Durrant
1 sibling, 0 replies; 34+ messages in thread
From: George Dunlap @ 2018-10-05 10:37 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
Andrew Cooper, Tim (Xen.org),
Jan Beulich, Ian Jackson, xen-devel
Re-cc’ing xen-devel...
> On Oct 5, 2018, at 11:34 AM, George Dunlap <george.dunlap@citrix.com> wrote:
>
>
>
>> On Oct 5, 2018, at 11:27 AM, Paul Durrant <Paul.Durrant@citrix.com> wrote:
>>
>>> -----Original Message-----
>>> From: George Dunlap
>>> Sent: 05 October 2018 11:25
>>> To: Paul Durrant <Paul.Durrant@citrix.com>
>>> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
>>> inside iommu_map/unmap_page()
>>>
>>> [Sorry, my mail client crashed and I can’t figure out how to make it re-
>>> edit this draft, so I’m replying to it instead.]
>>>
>>>> On Oct 5, 2018, at 11:22 AM, George Dunlap <George.Dunlap@citrix.com>
>>> wrote:
>>>>
>>>>
>>>>
>>>>> On Oct 5, 2018, at 10:02 AM, Paul Durrant <Paul.Durrant@citrix.com>
>>> wrote:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>>>>> Sent: 05 October 2018 08:33
>>>>>> To: Paul Durrant <Paul.Durrant@citrix.com>
>>>>>> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
>>>>>> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Wei
>>> Liu
>>>>>> <wei.liu2@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>; Stefano
>>>>>> Stabellini <sstabellini@kernel.org>; xen-devel <xen-
>>>>>> devel@lists.xenproject.org>; Konrad Rzeszutek Wilk
>>>>>> <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>
>>>>>> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
>>>>>> inside iommu_map/unmap_page()
>>>>>>
>>>>>>>>> On 04.10.18 at 18:36, <Paul.Durrant@citrix.com> wrote:
>>>>>>> I still think an implicit domain_crash() doesn't really belong in
>>>>>> something
>>>>>>> that looks like a straightforward wrapper around a per-implementation
>>>>>> jump
>>>>>>> table. How about iommu_map/unmap_may_crash() instead to highlight the
>>>>>>> semantic?
>>>>>>
>>>>>> If anything then the other way around, i.e. iommu_unmap_no_crash(),
>>>>>> such that only callers who explicitly mean to deal with the crashing
>>>>>> themselves would use the otherwise insecure variant.
>>>>>>
>>>>>
>>>>> Ok. George, what is your preference?
>>>>>
>>>>> At this point my proposal is to drop this patch and replace it with one
>>> that removes the implicit crash from from everything except the unmap. I
>>> can then introduce a 'nocrash' variant of unmap if I need it... although
>>> I'm no longer convinced I can really do anything else if a PV-IOMMU unmap
>>> fails.
>>>>
>>>> Sorry, ‘mayfail’ was meant to be short for “may fail [without crashing
>>> the guest]”; as opposed to “must succeed [or crash the guest]”. IOW, I
>>> agree with Jan that the default should be to crash the guest unless the
>>> caller explicitly opts to handle the failure themselves. Don’t have a
>>> strong opinion on the name.
>>
>> But for mapping too? It seems unnecessary to crash the domain in that case.
>
> ISTR that the domain_crash() was added only a few years ago; I’d have to go back and see the reasoning for it being added in the first place. I’ll do that Monday if Jan doesn’t beat me to it.
>
> -George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
[not found] ` <28341D09-548F-460C-8B22-43CA61EA1308@citrix.com>
2018-10-05 10:37 ` George Dunlap
@ 2018-10-05 10:38 ` Paul Durrant
2018-10-05 11:17 ` Jan Beulich
1 sibling, 1 reply; 34+ messages in thread
From: Paul Durrant @ 2018-10-05 10:38 UTC (permalink / raw)
To: George Dunlap
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
Andrew Cooper, Tim (Xen.org),
Jan Beulich, Ian Jackson, xen-devel
> -----Original Message-----
> From: George Dunlap
> Sent: 05 October 2018 11:35
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
> inside iommu_map/unmap_page()
>
>
>
> > On Oct 5, 2018, at 11:27 AM, Paul Durrant <Paul.Durrant@citrix.com>
> wrote:
> >
> >> -----Original Message-----
> >> From: George Dunlap
> >> Sent: 05 October 2018 11:25
> >> To: Paul Durrant <Paul.Durrant@citrix.com>
> >> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
> >> inside iommu_map/unmap_page()
> >>
> >> [Sorry, my mail client crashed and I can’t figure out how to make it
> re-
> >> edit this draft, so I’m replying to it instead.]
> >>
> >>> On Oct 5, 2018, at 11:22 AM, George Dunlap <George.Dunlap@citrix.com>
> >> wrote:
> >>>
> >>>
> >>>
> >>>> On Oct 5, 2018, at 10:02 AM, Paul Durrant <Paul.Durrant@citrix.com>
> >> wrote:
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Jan Beulich [mailto:JBeulich@suse.com]
> >>>>> Sent: 05 October 2018 08:33
> >>>>> To: Paul Durrant <Paul.Durrant@citrix.com>
> >>>>> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> >>>>> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> Wei
> >> Liu
> >>>>> <wei.liu2@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>;
> Stefano
> >>>>> Stabellini <sstabellini@kernel.org>; xen-devel <xen-
> >>>>> devel@lists.xenproject.org>; Konrad Rzeszutek Wilk
> >>>>> <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>
> >>>>> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
> >>>>> inside iommu_map/unmap_page()
> >>>>>
> >>>>>>>> On 04.10.18 at 18:36, <Paul.Durrant@citrix.com> wrote:
> >>>>>> I still think an implicit domain_crash() doesn't really belong in
> >>>>> something
> >>>>>> that looks like a straightforward wrapper around a per-
> implementation
> >>>>> jump
> >>>>>> table. How about iommu_map/unmap_may_crash() instead to highlight
> the
> >>>>>> semantic?
> >>>>>
> >>>>> If anything then the other way around, i.e. iommu_unmap_no_crash(),
> >>>>> such that only callers who explicitly mean to deal with the crashing
> >>>>> themselves would use the otherwise insecure variant.
> >>>>>
> >>>>
> >>>> Ok. George, what is your preference?
> >>>>
> >>>> At this point my proposal is to drop this patch and replace it with
> one
> >> that removes the implicit crash from from everything except the unmap.
> I
> >> can then introduce a 'nocrash' variant of unmap if I need it...
> although
> >> I'm no longer convinced I can really do anything else if a PV-IOMMU
> unmap
> >> fails.
> >>>
> >>> Sorry, ‘mayfail’ was meant to be short for “may fail [without crashing
> >> the guest]”; as opposed to “must succeed [or crash the guest]”. IOW, I
> >> agree with Jan that the default should be to crash the guest unless the
> >> caller explicitly opts to handle the failure themselves. Don’t have a
> >> strong opinion on the name.
> >
> > But for mapping too? It seems unnecessary to crash the domain in that
> case.
>
> ISTR that the domain_crash() was added only a few years ago; I’d have to
> go back and see the reasoning for it being added in the first place. I’ll
> do that Monday if Jan doesn’t beat me to it.
>
I was added by the following commit:
commit 834c97baebb3743c54bcae228e984ae1b9692e6a
Author: Quan Xu <quan.xu@intel.com>
Date: Tue Jun 14 15:10:57 2016 +0200
IOMMU: handle IOMMU mapping and unmapping failures
Treat IOMMU mapping and unmapping failures as a fatal to the DomU
If IOMMU mapping and unmapping failed, crash the DomU and propagate
the error up to the call trees.
No spamming of the log can occur. For DomU, we avoid logging any
message for already dying domains. For Dom0, that'll still be more
verbose than we'd really like, but it at least wouldn't outright
flood the console.
Signed-off-by: Quan Xu <quan.xu@intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
So the justification appears to be to avoid log spam.
Paul
> -George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
2018-10-05 10:38 ` Paul Durrant
@ 2018-10-05 11:17 ` Jan Beulich
2018-10-05 11:44 ` Paul Durrant
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2018-10-05 11:17 UTC (permalink / raw)
To: george.dunlap, Paul Durrant
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
Andrew Cooper, Tim Deegan, Ian Jackson, xen-devel
>>> On 05.10.18 at 12:38, <Paul.Durrant@citrix.com> wrote:
>> From: George Dunlap
>> Sent: 05 October 2018 11:35
>>
>> > On Oct 5, 2018, at 11:27 AM, Paul Durrant <Paul.Durrant@citrix.com>
>> wrote:
>> > But for mapping too? It seems unnecessary to crash the domain in that
>> case.
>>
>> ISTR that the domain_crash() was added only a few years ago; I’d have to
>> go back and see the reasoning for it being added in the first place. I’ll
>> do that Monday if Jan doesn’t beat me to it.
>>
>
> I was added by the following commit:
>
> commit 834c97baebb3743c54bcae228e984ae1b9692e6a
> Author: Quan Xu <quan.xu@intel.com>
> Date: Tue Jun 14 15:10:57 2016 +0200
>
> IOMMU: handle IOMMU mapping and unmapping failures
>
> Treat IOMMU mapping and unmapping failures as a fatal to the DomU
> If IOMMU mapping and unmapping failed, crash the DomU and propagate
> the error up to the call trees.
>
> No spamming of the log can occur. For DomU, we avoid logging any
> message for already dying domains. For Dom0, that'll still be more
> verbose than we'd really like, but it at least wouldn't outright
> flood the console.
>
> Signed-off-by: Quan Xu <quan.xu@intel.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> So the justification appears to be to avoid log spam.
Iirc that part of the description only exists because early version of
that patch did introduce log spam.
The problem iirc is mainly proper error handling, in particular proper
unwinding of earlier mappings that may have got installed
successfully in the context of the same hypercall (or whatever).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()
2018-10-05 11:17 ` Jan Beulich
@ 2018-10-05 11:44 ` Paul Durrant
0 siblings, 0 replies; 34+ messages in thread
From: Paul Durrant @ 2018-10-05 11:44 UTC (permalink / raw)
To: 'Jan Beulich', George Dunlap
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
Andrew Cooper, Tim (Xen.org),
Ian Jackson, xen-devel
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 05 October 2018 12:18
> To: George Dunlap <George.Dunlap@citrix.com>; Paul Durrant
> <Paul.Durrant@citrix.com>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Stefano
> Stabellini <sstabellini@kernel.org>; xen-devel <xen-
> devel@lists.xenproject.org>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>
> Subject: RE: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
> inside iommu_map/unmap_page()
>
> >>> On 05.10.18 at 12:38, <Paul.Durrant@citrix.com> wrote:
> >> From: George Dunlap
> >> Sent: 05 October 2018 11:35
> >>
> >> > On Oct 5, 2018, at 11:27 AM, Paul Durrant <Paul.Durrant@citrix.com>
> >> wrote:
> >> > But for mapping too? It seems unnecessary to crash the domain in that
> >> case.
> >>
> >> ISTR that the domain_crash() was added only a few years ago; I’d have
> to
> >> go back and see the reasoning for it being added in the first place.
> I’ll
> >> do that Monday if Jan doesn’t beat me to it.
> >>
> >
> > I was added by the following commit:
> >
> > commit 834c97baebb3743c54bcae228e984ae1b9692e6a
> > Author: Quan Xu <quan.xu@intel.com>
> > Date: Tue Jun 14 15:10:57 2016 +0200
> >
> > IOMMU: handle IOMMU mapping and unmapping failures
> >
> > Treat IOMMU mapping and unmapping failures as a fatal to the DomU
> > If IOMMU mapping and unmapping failed, crash the DomU and propagate
> > the error up to the call trees.
> >
> > No spamming of the log can occur. For DomU, we avoid logging any
> > message for already dying domains. For Dom0, that'll still be more
> > verbose than we'd really like, but it at least wouldn't outright
> > flood the console.
> >
> > Signed-off-by: Quan Xu <quan.xu@intel.com>
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > Reviewed-by: Jan Beulich <jbeulich@suse.com>
> >
> > So the justification appears to be to avoid log spam.
>
> Iirc that part of the description only exists because early version of
> that patch did introduce log spam.
>
> The problem iirc is mainly proper error handling, in particular proper
> unwinding of earlier mappings that may have got installed
> successfully in the context of the same hypercall (or whatever).
>
Ok. In the interest of making progress let's just drop this patch altogether. I'll add a patch to introduce a no-crash variant for map into my series implementing PV-IOMMU.
Paul
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync()
2018-10-04 10:45 ` [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync() Paul Durrant
` (2 preceding siblings ...)
2018-10-04 17:15 ` Julien Grall
@ 2018-10-05 14:50 ` Jan Beulich
2018-10-05 15:19 ` Paul Durrant
3 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2018-10-05 14:50 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Tamas K Lengyel, Suravee Suthikulpanit,
xen-devel, Brian Woods
>>> On 04.10.18 at 12:45, <paul.durrant@citrix.com> wrote:
> The name 'need_iommu()' is a little confusing as it suggests a domain needs
> to use the IOMMU but something might not be set up yet, when in fact it
> represents a tri-state value (not a boolean as might be expected) where
> -1 means 'IOMMU mappings being set up' and 1 means 'IOMMU mappings have
> been fully set up'.
>
> Two different meanings are also inferred from the macro it in various
> places in the code:
>
> - Some callers want to test whether a domain has IOMMU mappings at all
> - Some callers want to test whether they need to synchronize the domain's
> P2M and IOMMU mappings
>
> This patch replaces the 'need_iommu' tri-state value with a defined
> enumeration and adds a boolean flag 'need_sync' to separate these meanings,
> and places both of these in struct domain_iommu, rather than directly in
> struct domain.
> This patch also creates two new boolean macros:
>
> - 'has_iommu_pt()' evaluates to true if a domain has IOMMU mappings, even
> if they are still under construction.
> - 'need_iommu_pt_sync()' evaluates to true if a domain requires explicit
> synchronization of the P2M and IOMMU mappings.
>
> All callers of need_iommu() are then modified to use the macro appropriate
> to what they are trying to test, except for the instance in
> xen/drivers/passthrough/pci.c:assign_device() which has simply been
> removed since it appears to be unnecessary.
>
> NOTE: There are some callers of need_iommu() that strictly operate on
> the hardware domain. In some of these case a more global flag is
> used instead.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Presumably as a result of leaving out patch 4, this patch had quite a
bit of fuzz when applying. Would you please double check that I
didn't screw things up?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync()
2018-10-05 14:50 ` Jan Beulich
@ 2018-10-05 15:19 ` Paul Durrant
0 siblings, 0 replies; 34+ messages in thread
From: Paul Durrant @ 2018-10-05 15:19 UTC (permalink / raw)
To: 'Jan Beulich'
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
Konrad Rzeszutek Wilk, Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Tamas K Lengyel,
Suravee Suthikulpanit, xen-devel, Ian Jackson, Brian Woods
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 05 October 2018 15:51
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Brian Woods <brian.woods@amd.com>; Suravee Suthikulpanit
> <suravee.suthikulpanit@amd.com>; Julien Grall <julien.grall@arm.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wei.liu2@citrix.com>;
> George Dunlap <George.Dunlap@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>; Kevin
> Tian <kevin.tian@intel.com>; Stefano Stabellini <sstabellini@kernel.org>;
> xen-devel <xen-devel@lists.xenproject.org>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Tamas K Lengyel <tamas@tklengyel.com>; Tim
> (Xen.org) <tim@xen.org>
> Subject: Re: [PATCH v14 9/9] mm / iommu: split need_iommu() into
> has_iommu_pt() and need_iommu_pt_sync()
>
> >>> On 04.10.18 at 12:45, <paul.durrant@citrix.com> wrote:
> > The name 'need_iommu()' is a little confusing as it suggests a domain
> needs
> > to use the IOMMU but something might not be set up yet, when in fact it
> > represents a tri-state value (not a boolean as might be expected) where
> > -1 means 'IOMMU mappings being set up' and 1 means 'IOMMU mappings have
> > been fully set up'.
> >
> > Two different meanings are also inferred from the macro it in various
> > places in the code:
> >
> > - Some callers want to test whether a domain has IOMMU mappings at all
> > - Some callers want to test whether they need to synchronize the
> domain's
> > P2M and IOMMU mappings
> >
> > This patch replaces the 'need_iommu' tri-state value with a defined
> > enumeration and adds a boolean flag 'need_sync' to separate these
> meanings,
> > and places both of these in struct domain_iommu, rather than directly in
> > struct domain.
> > This patch also creates two new boolean macros:
> >
> > - 'has_iommu_pt()' evaluates to true if a domain has IOMMU mappings,
> even
> > if they are still under construction.
> > - 'need_iommu_pt_sync()' evaluates to true if a domain requires explicit
> > synchronization of the P2M and IOMMU mappings.
> >
> > All callers of need_iommu() are then modified to use the macro
> appropriate
> > to what they are trying to test, except for the instance in
> > xen/drivers/passthrough/pci.c:assign_device() which has simply been
> > removed since it appears to be unnecessary.
> >
> > NOTE: There are some callers of need_iommu() that strictly operate on
> > the hardware domain. In some of these case a more global flag is
> > used instead.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>
> Presumably as a result of leaving out patch 4, this patch had quite a
> bit of fuzz when applying. Would you please double check that I
> didn't screw things up?
>
Certainly the commit looks fine. I'll re-base my PV-IOMMU series and re-test it... probably not until next week though.
Paul
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt()
2018-10-04 10:45 ` [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt() Paul Durrant
2018-10-04 14:02 ` Jan Beulich
2018-10-04 16:23 ` George Dunlap
@ 2018-10-05 16:03 ` Wei Liu
2018-10-05 16:13 ` Paul Durrant
2 siblings, 1 reply; 34+ messages in thread
From: Wei Liu @ 2018-10-05 16:03 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, Jun Nakajima, George Dunlap,
Andrew Cooper, Jan Beulich, xen-devel
This patch has broken my PVH Dom0 setup.
[ 2.515159] igb 0000:02:00.1: added PHC on eth1
[ 2.519539] igb 0000:02:00.1: Intel(R) Gigabit Ethernet Network Connection
[ 2.526469] igb 0000:02:00.1: eth1: (PCIe:5.0Gb/s:Width x4) 0c:c4:7a:e7:b6:53
[ 2.533733] igb 0000:02:00.1: eth1: PBA No: 010A00-000
[ 2.538860] igb 0000:02:00.1: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s)
[ 2.649191] ata8: SATA link down (SStatus 0 SControl 300)
[ 2.654467] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[ 2.660730] ata7: SATA link down (SStatus 0 SControl 300)
[ 2.666166] ata5: SATA link down (SStatus 0 SControl 300)
[ 2.671619] ata3: SATA link down (SStatus 0 SControl 300)
[ 2.677073] ata6: SATA link down (SStatus 0 SControl 300)
[ 2.682538] ata2: SATA link down (SStatus 0 SControl 300)
[ 2.687988] ata4: SATA link down (SStatus 0 SControl 300)
[ 2.694632] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x100)
[ 8.215728] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[ 8.222945] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x100)
[ 8.229052] ata1: limiting SATA link speed to 1.5 Gbps
[ 13.590991] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
[ 13.597142] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x100)
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt()
2018-10-05 16:03 ` Wei Liu
@ 2018-10-05 16:13 ` Paul Durrant
0 siblings, 0 replies; 34+ messages in thread
From: Paul Durrant @ 2018-10-05 16:13 UTC (permalink / raw)
Cc: Stefano Stabellini, Wei Liu, Jun Nakajima, Andrew Cooper,
George Dunlap, Jan Beulich, xen-devel
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 05 October 2018 17:04
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Stefano Stabellini
> <sstabellini@kernel.org>; Jun Nakajima <jun.nakajima@intel.com>; George
> Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Jan Beulich <jbeulich@suse.com>; Wei Liu
> <wei.liu2@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v14 8/9] mm / iommu: include need_iommu()
> test in iommu_use_hap_pt()
>
> This patch has broken my PVH Dom0 setup.
Hmm. This is probably a bug in the need_iommu() setting for dom0. Do you still see the problem with the subsequent patch " mm / iommu: split need_iommu() into has_iommu_pt()..." applied too?
Paul
>
> [ 2.515159] igb 0000:02:00.1: added PHC on eth1
> [ 2.519539] igb 0000:02:00.1: Intel(R) Gigabit Ethernet Network
> Connection
> [ 2.526469] igb 0000:02:00.1: eth1: (PCIe:5.0Gb/s:Width x4)
> 0c:c4:7a:e7:b6:53
> [ 2.533733] igb 0000:02:00.1: eth1: PBA No: 010A00-000
> [ 2.538860] igb 0000:02:00.1: Using MSI-X interrupts. 8 rx queue(s), 8
> tx queue(s)
> [ 2.649191] ata8: SATA link down (SStatus 0 SControl 300)
> [ 2.654467] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
> [ 2.660730] ata7: SATA link down (SStatus 0 SControl 300)
> [ 2.666166] ata5: SATA link down (SStatus 0 SControl 300)
> [ 2.671619] ata3: SATA link down (SStatus 0 SControl 300)
> [ 2.677073] ata6: SATA link down (SStatus 0 SControl 300)
> [ 2.682538] ata2: SATA link down (SStatus 0 SControl 300)
> [ 2.687988] ata4: SATA link down (SStatus 0 SControl 300)
> [ 2.694632] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x100)
> [ 8.215728] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
> [ 8.222945] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x100)
> [ 8.229052] ata1: limiting SATA link speed to 1.5 Gbps
> [ 13.590991] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
> [ 13.597142] ata1.00: failed to IDENTIFY (I/O error, err_mask=0x100)
>
> Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 7/9] vtd: add lookup_page method to iommu_ops
2018-10-04 10:45 ` [PATCH v14 7/9] vtd: add lookup_page method to iommu_ops Paul Durrant
@ 2018-12-24 15:15 ` Andrew Cooper
2019-01-03 8:58 ` Paul Durrant
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-12-24 15:15 UTC (permalink / raw)
To: Paul Durrant, xen-devel; +Cc: Wei Liu, George Dunlap
On 04/10/2018 11:45, Paul Durrant wrote:
> This patch adds a new method to the VT-d IOMMU implementation to find the
> MFN currently mapped by the specified DFN along with a wrapper function
> in generic IOMMU code to call the implementation if it exists.
>
> NOTE: This patch only adds a Xen-internal interface. This will be used by
> a subsequent patch.
> Another subsequent patch will add similar functionality for AMD
> IOMMUs.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> ---
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: George Dunlap <george.dunlap@citrix.com>
>
> v11:
> - Fold in patch to change failure semantics (already ack-ed by Kevin).
>
> v10:
> - Adjust the locking comment.
>
> v9:
> - Add comment about locking in xen/iommu.h.
>
> v8:
> - Remove clean-up as this is now done by a prior patch.
> - Make intel_iommu_lookup_page() return dfn value if using shared EPT
> or iommu_passthrough is set, as requested by Kevin.
>
> v7:
> - Re-base and re-name BFN -> DFN.
> - Add missing checks for shared EPT and iommu_passthrough.
> - Remove unnecessary initializers and use array-style dereference.
> - Drop Wei's R-b because of code churn.
>
> v3:
> - Addressed comments from George.
>
> v2:
> - Addressed some comments from Jan.
> ---
> xen/drivers/passthrough/iommu.c | 11 ++++++++++
> xen/drivers/passthrough/vtd/iommu.c | 41 +++++++++++++++++++++++++++++++++++++
> xen/drivers/passthrough/vtd/iommu.h | 3 +++
> xen/include/xen/iommu.h | 10 +++++++++
> 4 files changed, 65 insertions(+)
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index 9187d50730..f94b522c73 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1833,6 +1833,46 @@ static int __must_check intel_iommu_unmap_page(struct domain *d,
> return dma_pte_clear_one(d, dfn_to_daddr(dfn));
> }
>
> +static int intel_iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
> + unsigned int *flags)
> +{
> + struct domain_iommu *hd = dom_iommu(d);
> + struct dma_pte *page, val;
> + u64 pg_maddr;
> +
> + /*
> + * If VT-d shares EPT page table or if the domain is the hardware
> + * domain and iommu_passthrough is set then pass back the dfn.
> + */
> + if ( iommu_use_hap_pt(d) ||
> + (iommu_hwdom_passthrough && is_hardware_domain(d)) )
> + return -EOPNOTSUPP;
The patch as commented no longer behaves in the way described in the
comment, or the v8 requested change.
What is the verdict WRT returning dfn ?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v14 7/9] vtd: add lookup_page method to iommu_ops
2018-12-24 15:15 ` Andrew Cooper
@ 2019-01-03 8:58 ` Paul Durrant
0 siblings, 0 replies; 34+ messages in thread
From: Paul Durrant @ 2019-01-03 8:58 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: Wei Liu, George Dunlap
> -----Original Message-----
> From: Andrew Cooper
> Sent: 24 December 2018 15:15
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Wei Liu <wei.liu2@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v14 7/9] vtd: add lookup_page method to
> iommu_ops
>
> On 04/10/2018 11:45, Paul Durrant wrote:
> > This patch adds a new method to the VT-d IOMMU implementation to find
> the
> > MFN currently mapped by the specified DFN along with a wrapper function
> > in generic IOMMU code to call the implementation if it exists.
> >
> > NOTE: This patch only adds a Xen-internal interface. This will be used
> by
> > a subsequent patch.
> > Another subsequent patch will add similar functionality for AMD
> > IOMMUs.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Reviewed-by: Jan Beulich <jbeulich@suse.com>
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > ---
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: George Dunlap <george.dunlap@citrix.com>
> >
> > v11:
> > - Fold in patch to change failure semantics (already ack-ed by Kevin).
> >
> > v10:
> > - Adjust the locking comment.
> >
> > v9:
> > - Add comment about locking in xen/iommu.h.
> >
> > v8:
> > - Remove clean-up as this is now done by a prior patch.
> > - Make intel_iommu_lookup_page() return dfn value if using shared EPT
> > or iommu_passthrough is set, as requested by Kevin.
> >
> > v7:
> > - Re-base and re-name BFN -> DFN.
> > - Add missing checks for shared EPT and iommu_passthrough.
> > - Remove unnecessary initializers and use array-style dereference.
> > - Drop Wei's R-b because of code churn.
> >
> > v3:
> > - Addressed comments from George.
> >
> > v2:
> > - Addressed some comments from Jan.
> > ---
> > xen/drivers/passthrough/iommu.c | 11 ++++++++++
> > xen/drivers/passthrough/vtd/iommu.c | 41
> +++++++++++++++++++++++++++++++++++++
> > xen/drivers/passthrough/vtd/iommu.h | 3 +++
> > xen/include/xen/iommu.h | 10 +++++++++
> > 4 files changed, 65 insertions(+)
> >
> > diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> > index 9187d50730..f94b522c73 100644
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -1833,6 +1833,46 @@ static int __must_check
> intel_iommu_unmap_page(struct domain *d,
> > return dma_pte_clear_one(d, dfn_to_daddr(dfn));
> > }
> >
> > +static int intel_iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t
> *mfn,
> > + unsigned int *flags)
> > +{
> > + struct domain_iommu *hd = dom_iommu(d);
> > + struct dma_pte *page, val;
> > + u64 pg_maddr;
> > +
> > + /*
> > + * If VT-d shares EPT page table or if the domain is the hardware
> > + * domain and iommu_passthrough is set then pass back the dfn.
> > + */
> > + if ( iommu_use_hap_pt(d) ||
> > + (iommu_hwdom_passthrough && is_hardware_domain(d)) )
> > + return -EOPNOTSUPP;
>
> The patch as commented no longer behaves in the way described in the
> comment, or the v8 requested change.
The landscape is shifting too. I think this is going to need to pass back a base dfn and order once the the map and unmap ops gain an order parameter.
>
> What is the verdict WRT returning dfn ?
>
Passing back the dfn as the mfn is wrong for shared EPT as dfn == gfn and the code at this level does not have that information. I think it is much better to fail in some way... EOPNOTSUPP may not be the best errno but I can't think of a better one.
Paul
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2019-01-03 8:58 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-04 10:45 [PATCH v14 0/9] paravirtual IOMMU pre-requisites and clean-up Paul Durrant
2018-10-04 10:45 ` [PATCH v14 1/9] iommu: introduce the concept of DFN Paul Durrant
2018-10-04 10:45 ` [PATCH v14 2/9] iommu: make use of type-safe DFN and MFN in exported functions Paul Durrant
2018-10-04 15:14 ` George Dunlap
2018-10-04 10:45 ` [PATCH v14 3/9] iommu: push use of type-safe DFN and MFN into iommu_ops Paul Durrant
2018-10-04 13:04 ` Paul Durrant
2018-10-04 13:07 ` Julien Grall
2018-10-04 10:45 ` [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page() Paul Durrant
2018-10-04 16:29 ` George Dunlap
2018-10-04 16:36 ` Paul Durrant
2018-10-05 7:33 ` Jan Beulich
2018-10-05 9:02 ` Paul Durrant
[not found] ` <F5DD545D-610B-40C7-827C-4CEAA2DF5F04@citrix.com>
[not found] ` <8B147463-7834-4525-AC10-36922338876F@citrix.com>
[not found] ` <b6add5ff3b6647eaab63265dbaf923bd@AMSPEX02CL03.citrite.net>
2018-10-05 10:31 ` Paul Durrant
[not found] ` <28341D09-548F-460C-8B22-43CA61EA1308@citrix.com>
2018-10-05 10:37 ` George Dunlap
2018-10-05 10:38 ` Paul Durrant
2018-10-05 11:17 ` Jan Beulich
2018-10-05 11:44 ` Paul Durrant
2018-10-04 10:45 ` [PATCH v14 5/9] memory: add check_get_page_from_gfn() as a wrapper Paul Durrant
2018-10-04 16:02 ` George Dunlap
2018-10-04 10:45 ` [PATCH v14 6/9] vtd: add missing check for shared EPT Paul Durrant
2018-10-04 10:45 ` [PATCH v14 7/9] vtd: add lookup_page method to iommu_ops Paul Durrant
2018-12-24 15:15 ` Andrew Cooper
2019-01-03 8:58 ` Paul Durrant
2018-10-04 10:45 ` [PATCH v14 8/9] mm / iommu: include need_iommu() test in iommu_use_hap_pt() Paul Durrant
2018-10-04 14:02 ` Jan Beulich
2018-10-04 16:23 ` George Dunlap
2018-10-05 16:03 ` Wei Liu
2018-10-05 16:13 ` Paul Durrant
2018-10-04 10:45 ` [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync() Paul Durrant
2018-10-04 14:11 ` Jan Beulich
2018-10-04 16:23 ` George Dunlap
2018-10-04 17:15 ` Julien Grall
2018-10-05 14:50 ` Jan Beulich
2018-10-05 15:19 ` Paul Durrant
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.