* [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn
@ 2016-06-14 12:06 Julien Grall
2016-06-14 12:07 ` [PATCH 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe Julien Grall
` (7 more replies)
0 siblings, 8 replies; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:06 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Paul Durrant, Jan Beulich, wei.chen
Hello all,
Some of the ARM functions are mixing gfn vs mfn and even physical vs frame.
To avoid more confusion, this patch series makes use of the terminology
described in xen/include/xen/mm.h and the associated typesafe.
This series is based on staging + the branch next-4.8 from Stefano merge.
I have pushed a branch with the prerequisites and this series on xenbits:
git://xenbits.xen.org/people/julieng/xen-unstable.git branch typesafe-v1
Yours sincerely,
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Paul Durrant <paul.durrant@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: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Julien Grall (8):
xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe
xen: Use typesafe gfn/mfn in guest_physmap_* helpers
xen: Use typesafe gfn in xenmem_add_to_physmap_one
xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the
typesafe gfn
xen: Use the typesafe mfn and gfn in map_mmio_regions...
xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and
mfn
xen/mm: Introduce max_gfn and min_gfn
xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn
xen/arch/arm/domain.c | 4 +-
xen/arch/arm/domain_build.c | 6 +--
xen/arch/arm/domctl.c | 2 +-
xen/arch/arm/gic-v2.c | 4 +-
xen/arch/arm/mm.c | 18 ++++----
xen/arch/arm/p2m.c | 91 ++++++++++++++++++++------------------
xen/arch/arm/platforms/exynos5.c | 8 ++--
xen/arch/arm/platforms/omap5.c | 16 +++----
xen/arch/arm/traps.c | 21 ++++-----
xen/arch/arm/vgic-v2.c | 4 +-
xen/arch/x86/domain.c | 5 ++-
xen/arch/x86/domain_build.c | 6 +--
xen/arch/x86/hvm/ioreq.c | 8 ++--
xen/arch/x86/mm.c | 21 ++++-----
xen/arch/x86/mm/p2m.c | 50 ++++++++++++---------
xen/common/domctl.c | 4 +-
xen/common/grant_table.c | 11 ++---
xen/common/memory.c | 40 +++++++++--------
xen/drivers/passthrough/arm/smmu.c | 4 +-
xen/include/asm-arm/domain.h | 2 +-
xen/include/asm-arm/grant_table.h | 2 +-
xen/include/asm-arm/p2m.h | 23 +++++-----
xen/include/asm-x86/p2m.h | 11 +++--
xen/include/xen/mm.h | 7 ++-
xen/include/xen/p2m-common.h | 8 ++--
25 files changed, 198 insertions(+), 178 deletions(-)
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe
2016-06-14 12:06 [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn Julien Grall
@ 2016-06-14 12:07 ` Julien Grall
2016-06-14 12:44 ` Jan Beulich
2016-06-14 12:07 ` [PATCH 2/8] xen: Use typesafe gfn/mfn in guest_physmap_* helpers Julien Grall
` (6 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:07 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini, wei.chen
The correct acronym for a guest physical frame is gfn. Also use
the recently introduced typesafe gfn/mfn to avoid mixing the two
different kind of frame.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/p2m.c | 6 +++---
xen/common/grant_table.c | 4 ++--
xen/common/memory.c | 4 ++--
xen/include/asm-arm/p2m.h | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 6a19c57..867e294 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1479,10 +1479,10 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn)
d->arch.p2m.default_access);
}
-unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)
+mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn)
{
- paddr_t p = p2m_lookup(d, pfn_to_paddr(gpfn), NULL);
- return p >> PAGE_SHIFT;
+ paddr_t p = p2m_lookup(d, pfn_to_paddr(gfn_x(gfn)), NULL);
+ return _mfn(p >> PAGE_SHIFT);
}
/*
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 8b22299..3c304f4 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -256,7 +256,7 @@ static int __get_paged_frame(unsigned long gfn, unsigned long *frame, struct pag
}
*frame = page_to_mfn(*page);
#else
- *frame = gmfn_to_mfn(rd, gfn);
+ *frame = mfn_x(gfn_to_mfn(rd, _gfn(gfn)));
*page = mfn_valid(*frame) ? mfn_to_page(*frame) : NULL;
if ( (!(*page)) || (!get_page(*page, rd)) )
{
@@ -1788,7 +1788,7 @@ gnttab_transfer(
mfn = INVALID_MFN;
}
#else
- mfn = gmfn_to_mfn(d, gop.mfn);
+ mfn = mfn_x(gfn_to_mfn(d, _gfn(gop.mfn)));
#endif
/* Check the passed page frame for basic validity. */
diff --git a/xen/common/memory.c b/xen/common/memory.c
index ccc6436..3149f26 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -264,7 +264,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
return 1;
}
#else
- mfn = gmfn_to_mfn(d, gmfn);
+ mfn = mfn_x(gfn_to_mfn(d, _gfn(gmfn)));
#endif
if ( unlikely(!mfn_valid(mfn)) )
{
@@ -488,7 +488,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
goto fail;
}
#else /* !CONFIG_X86 */
- mfn = gmfn_to_mfn(d, gmfn + k);
+ mfn = mfn_x(gfn_to_mfn(d, _gfn(gmfn + k)));
#endif
if ( unlikely(!mfn_valid(mfn)) )
{
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index d240d1e..75c65a8 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -178,7 +178,7 @@ void guest_physmap_remove_page(struct domain *d,
unsigned long gpfn,
unsigned long mfn, unsigned int page_order);
-unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
+mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
/*
* Populate-on-demand
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/8] xen: Use typesafe gfn/mfn in guest_physmap_* helpers
2016-06-14 12:06 [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn Julien Grall
2016-06-14 12:07 ` [PATCH 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe Julien Grall
@ 2016-06-14 12:07 ` Julien Grall
2016-06-14 12:56 ` Jan Beulich
2016-06-14 12:07 ` [PATCH 3/8] xen: Use typesafe gfn in xenmem_add_to_physmap_one Julien Grall
` (5 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:07 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Paul Durrant, Jan Beulich, wei.chen
Also rename some variables to gfn or mfn when it does not require much
rework.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Paul Durrant <paul.durrant@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: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/arch/arm/domain_build.c | 2 +-
xen/arch/arm/mm.c | 10 +++++-----
xen/arch/arm/p2m.c | 20 ++++++++++----------
xen/arch/x86/domain.c | 5 +++--
xen/arch/x86/domain_build.c | 6 +++---
xen/arch/x86/hvm/ioreq.c | 8 ++++----
xen/arch/x86/mm.c | 12 +++++++-----
xen/arch/x86/mm/p2m.c | 32 ++++++++++++++++++++------------
xen/common/grant_table.c | 7 ++++---
xen/common/memory.c | 32 +++++++++++++++++---------------
xen/drivers/passthrough/arm/smmu.c | 4 ++--
xen/include/asm-arm/p2m.h | 12 ++++++------
xen/include/asm-x86/p2m.h | 11 +++++------
xen/include/xen/mm.h | 2 +-
14 files changed, 88 insertions(+), 75 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2e4c295..02b4539 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -125,7 +125,7 @@ static bool_t insert_11_bank(struct domain *d,
goto fail;
}
- res = guest_physmap_add_page(d, spfn, spfn, order);
+ res = guest_physmap_add_page(d, _gfn(spfn), _mfn(spfn), order);
if ( res )
panic("Failed map pages to DOM0: %d", res);
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 2ec211b..5ab9b75 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1153,7 +1153,7 @@ int xenmem_add_to_physmap_one(
}
/* Map at new location. */
- rc = guest_physmap_add_entry(d, gpfn, mfn, 0, t);
+ rc = guest_physmap_add_entry(d, _gfn(gpfn), _mfn(mfn), 0, t);
/* If we fail to add the mapping, we need to drop the reference we
* took earlier on foreign pages */
@@ -1282,8 +1282,8 @@ int create_grant_host_mapping(unsigned long addr, unsigned long frame,
if ( flags & GNTMAP_readonly )
t = p2m_grant_map_ro;
- rc = guest_physmap_add_entry(current->domain, addr >> PAGE_SHIFT,
- frame, 0, t);
+ rc = guest_physmap_add_entry(current->domain, _gfn(addr >> PAGE_SHIFT),
+ _mfn(frame), 0, t);
if ( rc )
return GNTST_general_error;
@@ -1294,13 +1294,13 @@ int create_grant_host_mapping(unsigned long addr, unsigned long frame,
int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
unsigned long new_addr, unsigned int flags)
{
- unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
+ gfn_t gfn = _gfn(addr >> PAGE_SHIFT);
struct domain *d = current->domain;
if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
return GNTST_general_error;
- guest_physmap_remove_page(d, gfn, mfn, 0);
+ guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
return GNTST_okay;
}
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 867e294..decec0d 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1290,26 +1290,26 @@ int map_dev_mmio_region(struct domain *d,
}
int guest_physmap_add_entry(struct domain *d,
- unsigned long gpfn,
- unsigned long mfn,
+ gfn_t gfn,
+ mfn_t mfn,
unsigned long page_order,
p2m_type_t t)
{
return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(gpfn),
- pfn_to_paddr(gpfn + (1 << page_order)),
- pfn_to_paddr(mfn), MATTR_MEM, 0, t,
+ pfn_to_paddr(gfn_x(gfn)),
+ pfn_to_paddr(gfn_x(gfn) + (1 << page_order)),
+ pfn_to_paddr(mfn_x(mfn)), MATTR_MEM, 0, t,
d->arch.p2m.default_access);
}
void guest_physmap_remove_page(struct domain *d,
- unsigned long gpfn,
- unsigned long mfn, unsigned int page_order)
+ gfn_t gfn,
+ mfn_t mfn, unsigned int page_order)
{
apply_p2m_changes(d, REMOVE,
- pfn_to_paddr(gpfn),
- pfn_to_paddr(gpfn + (1<<page_order)),
- pfn_to_paddr(mfn), MATTR_MEM, 0, p2m_invalid,
+ pfn_to_paddr(gfn_x(gfn)),
+ pfn_to_paddr(gfn_x(gfn) + (1<<page_order)),
+ pfn_to_paddr(mfn_x(mfn)), MATTR_MEM, 0, p2m_invalid,
d->arch.p2m.default_access);
}
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 989bc74..ff82818 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -802,9 +802,10 @@ int arch_domain_soft_reset(struct domain *d)
ret = -ENOMEM;
goto exit_put_gfn;
}
- guest_physmap_remove_page(d, gfn, mfn, PAGE_ORDER_4K);
+ guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), PAGE_ORDER_4K);
- ret = guest_physmap_add_page(d, gfn, page_to_mfn(new_page), PAGE_ORDER_4K);
+ ret = guest_physmap_add_page(d, _gfn(gfn), _mfn(page_to_mfn(new_page)),
+ PAGE_ORDER_4K);
if ( ret )
{
printk(XENLOG_G_ERR "Failed to add a page to replace"
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index b29c377..0a02d65 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -427,7 +427,7 @@ static __init void pvh_add_mem_mapping(struct domain *d, unsigned long gfn,
if ( !iomem_access_permitted(d, mfn + i, mfn + i) )
{
omfn = get_gfn_query_unlocked(d, gfn + i, &t);
- guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), PAGE_ORDER_4K);
+ guest_physmap_remove_page(d, _gfn(gfn + i), omfn, PAGE_ORDER_4K);
continue;
}
@@ -530,7 +530,7 @@ static __init void pvh_map_all_iomem(struct domain *d, unsigned long nr_pages)
if ( get_gpfn_from_mfn(mfn) != INVALID_M2P_ENTRY )
continue;
- rc = guest_physmap_add_page(d, start_pfn, mfn, 0);
+ rc = guest_physmap_add_page(d, _gfn(start_pfn), _mfn(mfn), 0);
if ( rc != 0 )
panic("Unable to add gpfn %#lx mfn %#lx to Dom0 physmap: %d",
start_pfn, mfn, rc);
@@ -605,7 +605,7 @@ static __init void dom0_update_physmap(struct domain *d, unsigned long pfn,
{
if ( is_pvh_domain(d) )
{
- int rc = guest_physmap_add_page(d, pfn, mfn, 0);
+ int rc = guest_physmap_add_page(d, _gfn(pfn), _mfn(mfn), 0);
BUG_ON(rc);
return;
}
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 333ce14..7148ac4 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -267,8 +267,8 @@ bool_t is_ioreq_server_page(struct domain *d, const struct page_info *page)
static void hvm_remove_ioreq_gmfn(
struct domain *d, struct hvm_ioreq_page *iorp)
{
- guest_physmap_remove_page(d, iorp->gmfn,
- page_to_mfn(iorp->page), 0);
+ guest_physmap_remove_page(d, _gfn(iorp->gmfn),
+ _mfn(page_to_mfn(iorp->page)), 0);
clear_page(iorp->va);
}
@@ -279,8 +279,8 @@ static int hvm_add_ioreq_gmfn(
clear_page(iorp->va);
- rc = guest_physmap_add_page(d, iorp->gmfn,
- page_to_mfn(iorp->page), 0);
+ rc = guest_physmap_add_page(d, _gfn(iorp->gmfn),
+ _mfn(page_to_mfn(iorp->page)), 0);
if ( rc == 0 )
paging_mark_dirty(d, page_to_mfn(iorp->page));
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 8d10a3e..a6e07f2 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4208,7 +4208,8 @@ static int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
else
p2mt = p2m_grant_map_rw;
rc = guest_physmap_add_entry(current->domain,
- addr >> PAGE_SHIFT, frame, PAGE_ORDER_4K, p2mt);
+ _gfn(addr >> PAGE_SHIFT),
+ _mfn(frame), PAGE_ORDER_4K, p2mt);
if ( rc )
return GNTST_general_error;
else
@@ -4265,7 +4266,7 @@ static int replace_grant_p2m_mapping(
type, mfn_x(old_mfn), frame);
return GNTST_general_error;
}
- guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K);
+ guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K);
put_gfn(d, gfn);
return GNTST_okay;
@@ -4850,7 +4851,8 @@ int xenmem_add_to_physmap_one(
{
if ( is_xen_heap_mfn(prev_mfn) )
/* Xen heap frames are simply unhooked from this phys slot. */
- guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K);
+ guest_physmap_remove_page(d, _gfn(gpfn), _mfn(prev_mfn),
+ PAGE_ORDER_4K);
else
/* Normal domain memory is freed, to avoid leaking memory. */
guest_remove_page(d, gpfn);
@@ -4864,10 +4866,10 @@ int xenmem_add_to_physmap_one(
if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
ASSERT( old_gpfn == gfn );
if ( old_gpfn != INVALID_M2P_ENTRY )
- guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K);
+ guest_physmap_remove_page(d, _gfn(old_gpfn), _mfn(mfn), PAGE_ORDER_4K);
/* Map at new location. */
- rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K);
+ rc = guest_physmap_add_page(d, _gfn(gpfn), _mfn(mfn), PAGE_ORDER_4K);
/* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 9b19769..faf6e13 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -665,21 +665,21 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn, unsigned long mfn,
}
int
-guest_physmap_remove_page(struct domain *d, unsigned long gfn,
- unsigned long mfn, unsigned int page_order)
+guest_physmap_remove_page(struct domain *d, gfn_t gfn,
+ mfn_t mfn, unsigned int page_order)
{
struct p2m_domain *p2m = p2m_get_hostp2m(d);
int rc;
gfn_lock(p2m, gfn, page_order);
- rc = p2m_remove_page(p2m, gfn, mfn, page_order);
+ rc = p2m_remove_page(p2m, gfn_x(gfn), mfn_x(mfn), page_order);
gfn_unlock(p2m, gfn, page_order);
return rc;
}
-int
-guest_physmap_add_entry(struct domain *d, unsigned long gfn,
- unsigned long mfn, unsigned int page_order,
- p2m_type_t t)
+static int
+__guest_physmap_add_entry(struct domain *d, unsigned long gfn,
+ unsigned long mfn, unsigned int page_order,
+ p2m_type_t t)
{
struct p2m_domain *p2m = p2m_get_hostp2m(d);
unsigned long i, ogfn;
@@ -838,6 +838,13 @@ out:
return rc;
}
+/* XXX: To be removed when __guest_physmap_add_entry will use typesafe */
+int
+guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
+ unsigned int page_order, p2m_type_t t)
+{
+ return __guest_physmap_add_entry(d, gfn_x(gfn), mfn_x(mfn), page_order, t);
+}
/*
* Modify the p2m type of a single gfn from ot to nt.
@@ -2785,7 +2792,8 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
unsigned long gpfn, domid_t foreigndom)
{
p2m_type_t p2mt, p2mt_prev;
- unsigned long prev_mfn, mfn;
+ mfn_t prev_mfn;
+ unsigned long mfn;
struct page_info *page;
int rc;
struct domain *fdom;
@@ -2831,12 +2839,12 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
mfn = mfn_x(page_to_mfn(page));
/* Remove previously mapped page if it is present. */
- prev_mfn = mfn_x(get_gfn(tdom, gpfn, &p2mt_prev));
- if ( mfn_valid(_mfn(prev_mfn)) )
+ prev_mfn = get_gfn(tdom, gpfn, &p2mt_prev);
+ if ( mfn_valid(prev_mfn) )
{
- if ( is_xen_heap_mfn(prev_mfn) )
+ if ( is_xen_heap_mfn(mfn_x(prev_mfn)) )
/* Xen heap frames are simply unhooked from this phys slot */
- guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
+ guest_physmap_remove_page(tdom, _gfn(gpfn), prev_mfn, 0);
else
/* Normal domain memory is freed, to avoid leaking memory. */
guest_remove_page(tdom, gpfn);
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 3c304f4..3f15543 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1818,7 +1818,7 @@ gnttab_transfer(
goto copyback;
}
- guest_physmap_remove_page(d, gop.mfn, mfn, 0);
+ guest_physmap_remove_page(d, _gfn(gop.mfn), _mfn(mfn), 0);
gnttab_flush_tlb(d);
/* Find the target domain. */
@@ -1946,7 +1946,7 @@ gnttab_transfer(
{
grant_entry_v1_t *sha = &shared_entry_v1(e->grant_table, gop.ref);
- guest_physmap_add_page(e, sha->frame, mfn, 0);
+ guest_physmap_add_page(e, _gfn(sha->frame), _mfn(mfn), 0);
if ( !paging_mode_translate(e) )
sha->frame = mfn;
}
@@ -1954,7 +1954,8 @@ gnttab_transfer(
{
grant_entry_v2_t *sha = &shared_entry_v2(e->grant_table, gop.ref);
- guest_physmap_add_page(e, sha->full_page.frame, mfn, 0);
+ guest_physmap_add_page(e, _gfn(sha->full_page.frame),
+ _mfn(mfn), 0);
if ( !paging_mode_translate(e) )
sha->full_page.frame = mfn;
}
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 3149f26..224a7b2 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -213,7 +213,7 @@ static void populate_physmap(struct memop_args *a)
mfn = page_to_mfn(page);
}
- guest_physmap_add_page(d, gpfn, mfn, a->extent_order);
+ guest_physmap_add_page(d, _gfn(gpfn), _mfn(mfn), a->extent_order);
if ( !paging_mode_translate(d) )
{
@@ -237,20 +237,20 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
#ifdef CONFIG_X86
p2m_type_t p2mt;
#endif
- unsigned long mfn;
+ mfn_t mfn;
#ifdef CONFIG_X86
- mfn = mfn_x(get_gfn_query(d, gmfn, &p2mt));
+ mfn = get_gfn_query(d, gmfn, &p2mt);
if ( unlikely(p2m_is_paging(p2mt)) )
{
- guest_physmap_remove_page(d, gmfn, mfn, 0);
+ guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
put_gfn(d, gmfn);
/* If the page hasn't yet been paged out, there is an
* actual page that needs to be released. */
if ( p2mt == p2m_ram_paging_out )
{
- ASSERT(mfn_valid(mfn));
- page = mfn_to_page(mfn);
+ ASSERT(mfn_valid(mfn_x(mfn)));
+ page = mfn_to_page(mfn_x(mfn));
if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
put_page(page);
}
@@ -259,14 +259,14 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
}
if ( p2mt == p2m_mmio_direct )
{
- clear_mmio_p2m_entry(d, gmfn, _mfn(mfn), 0);
+ clear_mmio_p2m_entry(d, gmfn, mfn, 0);
put_gfn(d, gmfn);
return 1;
}
#else
- mfn = mfn_x(gfn_to_mfn(d, _gfn(gmfn)));
+ mfn = gfn_to_mfn(d, _gfn(gmfn));
#endif
- if ( unlikely(!mfn_valid(mfn)) )
+ if ( unlikely(!mfn_valid(mfn_x(mfn))) )
{
put_gfn(d, gmfn);
gdprintk(XENLOG_INFO, "Domain %u page number %lx invalid\n",
@@ -288,12 +288,12 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
return 0;
}
/* Maybe the mfn changed */
- mfn = mfn_x(get_gfn_query_unlocked(d, gmfn, &p2mt));
+ mfn = get_gfn_query_unlocked(d, gmfn, &p2mt);
ASSERT(!p2m_is_shared(p2mt));
}
#endif /* CONFIG_X86 */
- page = mfn_to_page(mfn);
+ page = mfn_to_page(mfn_x(mfn));
if ( unlikely(!get_page(page, d)) )
{
put_gfn(d, gmfn);
@@ -316,7 +316,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
test_and_clear_bit(_PGC_allocated, &page->count_info) )
put_page(page);
- guest_physmap_remove_page(d, gmfn, mfn, 0);
+ guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
put_page(page);
put_gfn(d, gmfn);
@@ -540,7 +540,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
gfn = mfn_to_gmfn(d, mfn);
/* Pages were unshared above */
BUG_ON(SHARED_M2P(gfn));
- guest_physmap_remove_page(d, gfn, mfn, 0);
+ guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), 0);
put_page(page);
}
@@ -584,7 +584,8 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
}
mfn = page_to_mfn(page);
- guest_physmap_add_page(d, gpfn, mfn, exch.out.extent_order);
+ guest_physmap_add_page(d, _gfn(gpfn), _mfn(mfn),
+ exch.out.extent_order);
if ( !paging_mode_translate(d) )
{
@@ -1087,7 +1088,8 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
if ( page )
{
- guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
+ guest_physmap_remove_page(d, _gfn(xrfp.gpfn),
+ _mfn(page_to_mfn(page)), 0);
put_page(page);
}
else
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 54a03a6..90ee2e9 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2771,7 +2771,7 @@ static int 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, mfn, 0, t);
+ return guest_physmap_add_entry(d, _gfn(gfn), _mfn(mfn), 0, t);
}
static int arm_smmu_unmap_page(struct domain *d, unsigned long gfn)
@@ -2783,7 +2783,7 @@ static int arm_smmu_unmap_page(struct domain *d, unsigned long gfn)
if ( !is_domain_direct_mapped(d) )
return -EINVAL;
- guest_physmap_remove_page(d, gfn, gfn, 0);
+ guest_physmap_remove_page(d, _gfn(gfn), _mfn(gfn), 0);
return 0;
}
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 75c65a8..0d1e61e 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -160,23 +160,23 @@ int map_dev_mmio_region(struct domain *d,
unsigned long mfn);
int guest_physmap_add_entry(struct domain *d,
- unsigned long gfn,
- unsigned long mfn,
+ gfn_t gfn,
+ mfn_t mfn,
unsigned long page_order,
p2m_type_t t);
/* Untyped version for RAM only, for compatibility */
static inline int guest_physmap_add_page(struct domain *d,
- unsigned long gfn,
- unsigned long mfn,
+ gfn_t gfn,
+ mfn_t mfn,
unsigned int page_order)
{
return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
}
void guest_physmap_remove_page(struct domain *d,
- unsigned long gpfn,
- unsigned long mfn, unsigned int page_order);
+ gfn_t gfn,
+ mfn_t mfn, unsigned int page_order);
mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 65675a2..4ab3574 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -545,14 +545,14 @@ void p2m_teardown(struct p2m_domain *p2m);
void p2m_final_teardown(struct domain *d);
/* Add a page to a domain's p2m table */
-int guest_physmap_add_entry(struct domain *d, unsigned long gfn,
- unsigned long mfn, unsigned int page_order,
+int guest_physmap_add_entry(struct domain *d, gfn_t gfn,
+ mfn_t mfn, unsigned int page_order,
p2m_type_t t);
/* Untyped version for RAM only, for compatibility */
static inline int guest_physmap_add_page(struct domain *d,
- unsigned long gfn,
- unsigned long mfn,
+ gfn_t gfn,
+ mfn_t mfn,
unsigned int page_order)
{
return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
@@ -560,8 +560,7 @@ static inline int guest_physmap_add_page(struct domain *d,
/* Remove a page from a domain's p2m table */
int guest_physmap_remove_page(struct domain *d,
- unsigned long gfn,
- unsigned long mfn, unsigned int page_order);
+ gfn_t gfn, mfn_t mfn, unsigned int page_order);
/* Set a p2m range as populate-on-demand */
int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn,
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 3cf646a..1682d1f 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -511,7 +511,7 @@ int xenmem_add_to_physmap_one(struct domain *d, unsigned int space,
/* Returns 1 on success, 0 on error, negative if the ring
* for event propagation is full in the presence of paging */
-int guest_remove_page(struct domain *d, unsigned long gmfn);
+int guest_remove_page(struct domain *d, unsigned long gfn);
#define RAM_TYPE_CONVENTIONAL 0x00000001
#define RAM_TYPE_RESERVED 0x00000002
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/8] xen: Use typesafe gfn in xenmem_add_to_physmap_one
2016-06-14 12:06 [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn Julien Grall
2016-06-14 12:07 ` [PATCH 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe Julien Grall
2016-06-14 12:07 ` [PATCH 2/8] xen: Use typesafe gfn/mfn in guest_physmap_* helpers Julien Grall
@ 2016-06-14 12:07 ` Julien Grall
2016-06-14 12:58 ` Jan Beulich
2016-06-14 12:07 ` [PATCH 4/8] xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the typesafe gfn Julien Grall
` (4 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:07 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Jan Beulich, wei.chen
The x86 version of the function xenmem_add_to_physmap_one contains
variable name gpfn and gfn which make the code very confusing.
I have left unchanged for now.
Also, rename gpfn to gfn in the ARM version as the latter is the correct
acronym for a guest physical frame.
Finally, remove the trailing whitespace around the changes.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.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: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/arch/arm/mm.c | 10 +++++-----
xen/arch/x86/mm.c | 15 +++++++--------
xen/common/memory.c | 6 +++---
xen/include/xen/mm.h | 2 +-
4 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 5ab9b75..6882d54 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1046,7 +1046,7 @@ int xenmem_add_to_physmap_one(
unsigned int space,
union xen_add_to_physmap_batch_extra extra,
unsigned long idx,
- xen_pfn_t gpfn)
+ gfn_t gfn)
{
unsigned long mfn = 0;
int rc;
@@ -1081,8 +1081,8 @@ int xenmem_add_to_physmap_one(
else
return -EINVAL;
}
-
- d->arch.grant_table_gpfn[idx] = gpfn;
+
+ d->arch.grant_table_gpfn[idx] = gfn_x(gfn);
t = p2m_ram_rw;
@@ -1145,7 +1145,7 @@ int xenmem_add_to_physmap_one(
if ( extra.res0 )
return -EOPNOTSUPP;
- rc = map_dev_mmio_region(d, gpfn, 1, idx);
+ rc = map_dev_mmio_region(d, gfn_x(gfn), 1, idx);
return rc;
default:
@@ -1153,7 +1153,7 @@ int xenmem_add_to_physmap_one(
}
/* Map at new location. */
- rc = guest_physmap_add_entry(d, _gfn(gpfn), _mfn(mfn), 0, t);
+ rc = guest_physmap_add_entry(d, gfn, _mfn(mfn), 0, t);
/* If we fail to add the mapping, we need to drop the reference we
* took earlier on foreign pages */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index a6e07f2..a9498ec 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4772,7 +4772,7 @@ int xenmem_add_to_physmap_one(
unsigned int space,
union xen_add_to_physmap_batch_extra extra,
unsigned long idx,
- xen_pfn_t gpfn)
+ gfn_t gpfn)
{
struct page_info *page = NULL;
unsigned long gfn = 0; /* gcc ... */
@@ -4831,7 +4831,7 @@ int xenmem_add_to_physmap_one(
break;
}
case XENMAPSPACE_gmfn_foreign:
- return p2m_add_foreign(d, idx, gpfn, extra.foreign_domid);
+ return p2m_add_foreign(d, idx, gfn_x(gpfn), extra.foreign_domid);
default:
break;
}
@@ -4846,19 +4846,18 @@ int xenmem_add_to_physmap_one(
}
/* Remove previously mapped page if it was present. */
- prev_mfn = mfn_x(get_gfn(d, gpfn, &p2mt));
+ prev_mfn = mfn_x(get_gfn(d, gfn_x(gpfn), &p2mt));
if ( mfn_valid(prev_mfn) )
{
if ( is_xen_heap_mfn(prev_mfn) )
/* Xen heap frames are simply unhooked from this phys slot. */
- guest_physmap_remove_page(d, _gfn(gpfn), _mfn(prev_mfn),
- PAGE_ORDER_4K);
+ guest_physmap_remove_page(d, gpfn, _mfn(prev_mfn), PAGE_ORDER_4K);
else
/* Normal domain memory is freed, to avoid leaking memory. */
- guest_remove_page(d, gpfn);
+ guest_remove_page(d, gfn_x(gpfn));
}
/* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */
- put_gfn(d, gpfn);
+ put_gfn(d, gfn_x(gpfn));
/* Unmap from old location, if any. */
old_gpfn = get_gpfn_from_mfn(mfn);
@@ -4869,7 +4868,7 @@ int xenmem_add_to_physmap_one(
guest_physmap_remove_page(d, _gfn(old_gpfn), _mfn(mfn), PAGE_ORDER_4K);
/* Map at new location. */
- rc = guest_physmap_add_page(d, _gfn(gpfn), _mfn(mfn), PAGE_ORDER_4K);
+ rc = guest_physmap_add_page(d, gpfn, _mfn(mfn), PAGE_ORDER_4K);
/* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 224a7b2..0b73c06 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -649,7 +649,7 @@ static int xenmem_add_to_physmap(struct domain *d,
if ( xatp->space != XENMAPSPACE_gmfn_range )
return xenmem_add_to_physmap_one(d, xatp->space, extra,
- xatp->idx, xatp->gpfn);
+ xatp->idx, _gfn(xatp->gpfn));
if ( xatp->size < start )
return -EILSEQ;
@@ -666,7 +666,7 @@ static int xenmem_add_to_physmap(struct domain *d,
while ( xatp->size > done )
{
rc = xenmem_add_to_physmap_one(d, xatp->space, extra,
- xatp->idx, xatp->gpfn);
+ xatp->idx, _gfn(xatp->gpfn));
if ( rc < 0 )
break;
@@ -727,7 +727,7 @@ static int xenmem_add_to_physmap_batch(struct domain *d,
rc = xenmem_add_to_physmap_one(d, xatpb->space,
xatpb->u,
- idx, gpfn);
+ idx, _gfn(gpfn));
if ( unlikely(__copy_to_guest_offset(xatpb->errs, 0, &rc, 1)) )
{
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 1682d1f..a22c4c2 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -507,7 +507,7 @@ void scrub_one_page(struct page_info *);
int xenmem_add_to_physmap_one(struct domain *d, unsigned int space,
union xen_add_to_physmap_batch_extra extra,
- unsigned long idx, xen_pfn_t gpfn);
+ unsigned long idx, gfn_t gfn);
/* Returns 1 on success, 0 on error, negative if the ring
* for event propagation is full in the presence of paging */
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/8] xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the typesafe gfn
2016-06-14 12:06 [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn Julien Grall
` (2 preceding siblings ...)
2016-06-14 12:07 ` [PATCH 3/8] xen: Use typesafe gfn in xenmem_add_to_physmap_one Julien Grall
@ 2016-06-14 12:07 ` Julien Grall
2016-06-14 13:00 ` Andrew Cooper
2016-06-14 12:07 ` [PATCH 5/8] xen: Use the typesafe mfn and gfn in map_mmio_regions Julien Grall
` (3 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:07 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini, wei.chen
The correct acronym for a guest physical frame is gfn. Also use
the typesafe gfn to ensure that a guest frame is effectively used.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/domain.c | 4 ++--
xen/arch/arm/mm.c | 2 +-
xen/include/asm-arm/domain.h | 2 +-
xen/include/asm-arm/grant_table.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 1365b4a..008747c 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -444,13 +444,13 @@ struct domain *alloc_domain_struct(void)
return NULL;
clear_page(d);
- d->arch.grant_table_gpfn = xzalloc_array(xen_pfn_t, max_grant_frames);
+ d->arch.grant_table_gfn = xzalloc_array(gfn_t, max_grant_frames);
return d;
}
void free_domain_struct(struct domain *d)
{
- xfree(d->arch.grant_table_gpfn);
+ xfree(d->arch.grant_table_gfn);
free_xenheap_page(d);
}
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6882d54..0e408f8 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1082,7 +1082,7 @@ int xenmem_add_to_physmap_one(
return -EINVAL;
}
- d->arch.grant_table_gpfn[idx] = gfn_x(gfn);
+ d->arch.grant_table_gfn[idx] = gfn;
t = p2m_ram_rw;
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 370cdeb..979f7de 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -51,7 +51,7 @@ struct arch_domain
uint64_t vttbr;
struct hvm_domain hvm_domain;
- xen_pfn_t *grant_table_gpfn;
+ gfn_t *grant_table_gfn;
struct vmmio vmmio;
diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index 5e076cc..46cfe24 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -30,7 +30,7 @@ static inline int replace_grant_supported(void)
#define gnttab_shared_gmfn(d, t, i) \
( ((i >= nr_grant_frames(d->grant_table)) && \
- (i < max_grant_frames)) ? 0 : (d->arch.grant_table_gpfn[i]))
+ (i < max_grant_frames)) ? 0 : gfn_x((d->arch.grant_table_gfn[i])))
#define gnttab_need_iommu_mapping(d) \
(is_domain_direct_mapped(d) && need_iommu(d))
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/8] xen: Use the typesafe mfn and gfn in map_mmio_regions...
2016-06-14 12:06 [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn Julien Grall
` (3 preceding siblings ...)
2016-06-14 12:07 ` [PATCH 4/8] xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the typesafe gfn Julien Grall
@ 2016-06-14 12:07 ` Julien Grall
2016-06-14 13:02 ` Jan Beulich
2016-06-14 12:07 ` [PATCH 6/8] xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn Julien Grall
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:07 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Jan Beulich, wei.chen
to avoid mixing machine frame with guest frame.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.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: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/arch/arm/domain_build.c | 4 ++--
xen/arch/arm/gic-v2.c | 4 ++--
xen/arch/arm/p2m.c | 22 +++++++++++-----------
xen/arch/arm/platforms/exynos5.c | 8 ++++----
xen/arch/arm/platforms/omap5.c | 16 ++++++++--------
xen/arch/arm/vgic-v2.c | 4 ++--
xen/arch/x86/mm/p2m.c | 18 ++++++++++--------
xen/common/domctl.c | 4 ++--
xen/include/xen/p2m-common.h | 8 ++++----
9 files changed, 45 insertions(+), 43 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 02b4539..865539a 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1042,9 +1042,9 @@ static int map_range_to_domain(const struct dt_device_node *dev,
if ( need_mapping )
{
res = map_mmio_regions(d,
- paddr_to_pfn(addr),
+ _gfn(paddr_to_pfn(addr)),
DIV_ROUND_UP(len, PAGE_SIZE),
- paddr_to_pfn(addr));
+ _mfn(paddr_to_pfn(addr)));
if ( res < 0 )
{
printk(XENLOG_ERR "Unable to map 0x%"PRIx64
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 2c1c0ba..58917c8 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -601,9 +601,9 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
d->domain_id, v2m_data->addr, v2m_data->size,
v2m_data->spi_start, v2m_data->nr_spis);
- ret = map_mmio_regions(d, paddr_to_pfn(v2m_data->addr),
+ ret = map_mmio_regions(d, _gfn(paddr_to_pfn(v2m_data->addr)),
DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
- paddr_to_pfn(v2m_data->addr));
+ _mfn(paddr_to_pfn(v2m_data->addr)));
if ( ret )
{
printk(XENLOG_ERR "GICv2: Map v2m frame to d%d failed.\n",
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index decec0d..4831cf1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1243,27 +1243,27 @@ int unmap_regions_rw_cache(struct domain *d,
}
int map_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
{
return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr),
- pfn_to_paddr(mfn),
+ pfn_to_paddr(gfn_x(start_gfn)),
+ pfn_to_paddr(gfn_x(start_gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
MATTR_DEV, 0, p2m_mmio_direct,
d->arch.p2m.default_access);
}
int unmap_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
{
return apply_p2m_changes(d, REMOVE,
- pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr),
- pfn_to_paddr(mfn),
+ pfn_to_paddr(gfn_x(start_gfn)),
+ pfn_to_paddr(gfn_x(start_gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
MATTR_DEV, 0, p2m_invalid,
d->arch.p2m.default_access);
}
@@ -1278,7 +1278,7 @@ int map_dev_mmio_region(struct domain *d,
if ( !(nr && iomem_access_permitted(d, start_gfn, start_gfn + nr - 1)) )
return 0;
- res = map_mmio_regions(d, start_gfn, nr, mfn);
+ res = map_mmio_regions(d, _gfn(start_gfn), nr, _mfn(mfn));
if ( res < 0 )
{
printk(XENLOG_G_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index bf4964d..c43934f 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -83,12 +83,12 @@ static int exynos5_init_time(void)
static int exynos5250_specific_mapping(struct domain *d)
{
/* Map the chip ID */
- map_mmio_regions(d, paddr_to_pfn(EXYNOS5_PA_CHIPID), 1,
- paddr_to_pfn(EXYNOS5_PA_CHIPID));
+ map_mmio_regions(d, _gfn(paddr_to_pfn(EXYNOS5_PA_CHIPID)), 1,
+ _mfn(paddr_to_pfn(EXYNOS5_PA_CHIPID)));
/* Map the PWM region */
- map_mmio_regions(d, paddr_to_pfn(EXYNOS5_PA_TIMER), 2,
- paddr_to_pfn(EXYNOS5_PA_TIMER));
+ map_mmio_regions(d, _gfn(paddr_to_pfn(EXYNOS5_PA_TIMER)), 2,
+ _mfn(paddr_to_pfn(EXYNOS5_PA_TIMER)));
return 0;
}
diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
index a49ba62..539588e 100644
--- a/xen/arch/arm/platforms/omap5.c
+++ b/xen/arch/arm/platforms/omap5.c
@@ -102,20 +102,20 @@ static int omap5_init_time(void)
static int omap5_specific_mapping(struct domain *d)
{
/* Map the PRM module */
- map_mmio_regions(d, paddr_to_pfn(OMAP5_PRM_BASE), 2,
- paddr_to_pfn(OMAP5_PRM_BASE));
+ map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_PRM_BASE)), 2,
+ _mfn(paddr_to_pfn(OMAP5_PRM_BASE)));
/* Map the PRM_MPU */
- map_mmio_regions(d, paddr_to_pfn(OMAP5_PRCM_MPU_BASE), 1,
- paddr_to_pfn(OMAP5_PRCM_MPU_BASE));
+ map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_PRCM_MPU_BASE)), 1,
+ _mfn(paddr_to_pfn(OMAP5_PRCM_MPU_BASE)));
/* Map the Wakeup Gen */
- map_mmio_regions(d, paddr_to_pfn(OMAP5_WKUPGEN_BASE), 1,
- paddr_to_pfn(OMAP5_WKUPGEN_BASE));
+ map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_WKUPGEN_BASE)), 1,
+ _mfn(paddr_to_pfn(OMAP5_WKUPGEN_BASE)));
/* Map the on-chip SRAM */
- map_mmio_regions(d, paddr_to_pfn(OMAP5_SRAM_PA), 32,
- paddr_to_pfn(OMAP5_SRAM_PA));
+ map_mmio_regions(d, _gfn(paddr_to_pfn(OMAP5_SRAM_PA)), 32,
+ _mfn(paddr_to_pfn(OMAP5_SRAM_PA)));
return 0;
}
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 9adb4a9..cbe61cf 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -688,8 +688,8 @@ static int vgic_v2_domain_init(struct domain *d)
* Map the gic virtual cpu interface in the gic cpu interface
* region of the guest.
*/
- ret = map_mmio_regions(d, paddr_to_pfn(cbase), csize / PAGE_SIZE,
- paddr_to_pfn(vbase));
+ ret = map_mmio_regions(d, _gfn(paddr_to_pfn(cbase)), csize / PAGE_SIZE,
+ _mfn(paddr_to_pfn(vbase)));
if ( ret )
return ret;
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index faf6e13..070efd4 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2202,9 +2202,9 @@ static unsigned int mmio_order(const struct domain *d,
#define MAP_MMIO_MAX_ITER 64 /* pretty arbitrary */
int map_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
{
int ret = 0;
unsigned long i;
@@ -2217,10 +2217,11 @@ int map_mmio_regions(struct domain *d,
i += 1UL << order, ++iter )
{
/* OR'ing gfn and mfn values will return an order suitable to both. */
- for ( order = mmio_order(d, (start_gfn + i) | (mfn + i), nr - i); ;
+ for ( order = mmio_order(d, (gfn_x(start_gfn) + i) | (mfn_x(mfn) + i), nr - i); ;
order = ret - 1 )
{
- ret = set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i), order,
+ ret = set_mmio_p2m_entry(d, gfn_x(start_gfn) + i,
+ _mfn(mfn_x(mfn) + i), order,
p2m_get_hostp2m(d)->default_access);
if ( ret <= 0 )
break;
@@ -2234,9 +2235,9 @@ int map_mmio_regions(struct domain *d,
}
int unmap_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
{
int ret = 0;
unsigned long i;
@@ -2249,10 +2250,11 @@ int unmap_mmio_regions(struct domain *d,
i += 1UL << order, ++iter )
{
/* OR'ing gfn and mfn values will return an order suitable to both. */
- for ( order = mmio_order(d, (start_gfn + i) | (mfn + i), nr - i); ;
+ for ( order = mmio_order(d, (gfn_x(start_gfn) + i) | (mfn_x(mfn) + i), nr - i); ;
order = ret - 1 )
{
- ret = clear_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i), order);
+ ret = clear_mmio_p2m_entry(d, gfn_x(start_gfn) + i,
+ _mfn(mfn_x(mfn) + i), order);
if ( ret <= 0 )
break;
ASSERT(ret <= order);
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index e43904e..b784e6c 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -1074,7 +1074,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
"memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
+ ret = map_mmio_regions(d, _gfn(gfn), nr_mfns, _mfn(mfn));
if ( ret < 0 )
printk(XENLOG_G_WARNING
"memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n",
@@ -1086,7 +1086,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
"memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- ret = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
+ ret = unmap_mmio_regions(d, _gfn(gfn), nr_mfns, _mfn(mfn));
if ( ret < 0 && is_hardware_domain(current->domain) )
printk(XENLOG_ERR
"memory_map: error %ld removing dom%d access to [%lx,%lx]\n",
diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
index 6374a5b..b4f9077 100644
--- a/xen/include/xen/p2m-common.h
+++ b/xen/include/xen/p2m-common.h
@@ -37,13 +37,13 @@ typedef enum {
* * the guest physical address space to map, starting from the machine
* * frame number mfn. */
int map_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
unsigned long nr,
- unsigned long mfn);
+ mfn_t mfn);
int unmap_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
unsigned long nr,
- unsigned long mfn);
+ mfn_t mfn);
/*
* Set access type for a region of gfns.
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 6/8] xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn
2016-06-14 12:06 [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn Julien Grall
` (4 preceding siblings ...)
2016-06-14 12:07 ` [PATCH 5/8] xen: Use the typesafe mfn and gfn in map_mmio_regions Julien Grall
@ 2016-06-14 12:07 ` Julien Grall
2016-06-14 12:07 ` [PATCH 7/8] xen/mm: Introduce max_gfn and min_gfn Julien Grall
2016-06-14 12:07 ` [PATCH 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn Julien Grall
7 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:07 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini, wei.chen
The prototype and the declaration of p2m_lookup disagree on how the
function should be used. One expect a frame number whilst the other
an address.
Thankfully, everyone is using with an address today. However, most of
the callers have to convert a guest frame to an address. Modify
the interface to take a guest physical frame in parameter and return
a machine frame.
Whilst modifying the interface, use typesafe gfn and mfn for clarity
and catching possible misusage.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/p2m.c | 37 ++++++++++++++++++++-----------------
xen/arch/arm/traps.c | 21 +++++++++++----------
xen/include/asm-arm/p2m.h | 7 +++----
3 files changed, 34 insertions(+), 31 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 4831cf1..135d032 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -140,14 +140,15 @@ void flush_tlb_domain(struct domain *d)
}
/*
- * Lookup the MFN corresponding to a domain's PFN.
+ * Lookup the MFN corresponding to a domain's GFN.
*
* There are no processor functions to do a stage 2 only lookup therefore we
* do a a software walk.
*/
-static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
+static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
{
struct p2m_domain *p2m = &d->arch.p2m;
+ const paddr_t paddr = pfn_to_paddr(gfn_x(gfn));
const unsigned int offsets[4] = {
zeroeth_table_offset(paddr),
first_table_offset(paddr),
@@ -158,7 +159,7 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK
};
lpae_t pte, *map;
- paddr_t maddr = INVALID_PADDR;
+ mfn_t mfn = _mfn(INVALID_MFN);
paddr_t mask = 0;
p2m_type_t _t;
unsigned int level, root_table;
@@ -216,21 +217,22 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
{
ASSERT(mask);
ASSERT(pte.p2m.type != p2m_invalid);
- maddr = (pte.bits & PADDR_MASK & mask) | (paddr & ~mask);
+ mfn = _mfn(paddr_to_pfn((pte.bits & PADDR_MASK & mask) |
+ (paddr & ~mask)));
*t = pte.p2m.type;
}
err:
- return maddr;
+ return mfn;
}
-paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
+mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
{
- paddr_t ret;
+ mfn_t ret;
struct p2m_domain *p2m = &d->arch.p2m;
spin_lock(&p2m->lock);
- ret = __p2m_lookup(d, paddr, t);
+ ret = __p2m_lookup(d, gfn, t);
spin_unlock(&p2m->lock);
return ret;
@@ -493,8 +495,9 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
* No setting was found in the Radix tree. Check if the
* entry exists in the page-tables.
*/
- paddr_t maddr = __p2m_lookup(d, gfn_x(gfn) << PAGE_SHIFT, NULL);
- if ( INVALID_PADDR == maddr )
+ mfn_t mfn = __p2m_lookup(d, gfn, NULL);
+
+ if ( mfn_x(mfn) == INVALID_MFN )
return -ESRCH;
/* If entry exists then its rwx. */
@@ -1481,8 +1484,7 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn)
mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn)
{
- paddr_t p = p2m_lookup(d, pfn_to_paddr(gfn_x(gfn)), NULL);
- return _mfn(p >> PAGE_SHIFT);
+ return p2m_lookup(d, gfn, NULL);
}
/*
@@ -1496,7 +1498,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag)
{
long rc;
paddr_t ipa;
- unsigned long maddr;
+ gfn_t gfn;
unsigned long mfn;
xenmem_access_t xma;
p2m_type_t t;
@@ -1506,11 +1508,13 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag)
if ( rc < 0 )
goto err;
+ gfn = _gfn(paddr_to_pfn(ipa));
+
/*
* We do this first as this is faster in the default case when no
* permission is set on the page.
*/
- rc = __p2m_get_mem_access(current->domain, _gfn(paddr_to_pfn(ipa)), &xma);
+ rc = __p2m_get_mem_access(current->domain, gfn, &xma);
if ( rc < 0 )
goto err;
@@ -1559,11 +1563,10 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag)
* We had a mem_access permission limiting the access, but the page type
* could also be limiting, so we need to check that as well.
*/
- maddr = __p2m_lookup(current->domain, ipa, &t);
- if ( maddr == INVALID_PADDR )
+ mfn = mfn_x(__p2m_lookup(current->domain, gfn, &t));
+ if ( mfn == INVALID_MFN )
goto err;
- mfn = maddr >> PAGE_SHIFT;
if ( !mfn_valid(mfn) )
goto err;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index aa3e3c2..f1737e4 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2318,14 +2318,16 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
{
register_t ttbcr = READ_SYSREG(TCR_EL1);
uint64_t ttbr0 = READ_SYSREG64(TTBR0_EL1);
- paddr_t paddr;
uint32_t offset;
uint32_t *first = NULL, *second = NULL;
+ mfn_t mfn;
+
+ mfn = p2m_lookup(d, _gfn(paddr_to_pfn(ttbr0)), NULL);
printk("dom%d VA 0x%08"PRIvaddr"\n", d->domain_id, addr);
printk(" TTBCR: 0x%08"PRIregister"\n", ttbcr);
printk(" TTBR0: 0x%016"PRIx64" = 0x%"PRIpaddr"\n",
- ttbr0, p2m_lookup(d, ttbr0 & PAGE_MASK, NULL));
+ ttbr0, pfn_to_paddr(mfn_x(mfn)));
if ( ttbcr & TTBCR_EAE )
{
@@ -2338,32 +2340,31 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
return;
}
- paddr = p2m_lookup(d, ttbr0 & PAGE_MASK, NULL);
- if ( paddr == INVALID_PADDR )
+ if ( mfn_x(mfn) == INVALID_MFN )
{
printk("Failed TTBR0 maddr lookup\n");
goto done;
}
- first = map_domain_page(_mfn(paddr_to_pfn(paddr)));
+ first = map_domain_page(mfn);
offset = addr >> (12+10);
printk("1ST[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
- offset, paddr, first[offset]);
+ offset, pfn_to_paddr(mfn_x(mfn)), first[offset]);
if ( !(first[offset] & 0x1) ||
!(first[offset] & 0x2) )
goto done;
- paddr = p2m_lookup(d, first[offset] & PAGE_MASK, NULL);
+ mfn = p2m_lookup(d, _gfn(paddr_to_pfn(first[offset])), NULL);
- if ( paddr == INVALID_PADDR )
+ if ( mfn_x(mfn) == INVALID_MFN )
{
printk("Failed L1 entry maddr lookup\n");
goto done;
}
- second = map_domain_page(_mfn(paddr_to_pfn(paddr)));
+ second = map_domain_page(mfn);
offset = (addr >> 12) & 0x3FF;
printk("2ND[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
- offset, paddr, second[offset]);
+ offset, pfn_to_paddr(mfn_x(mfn)), second[offset]);
done:
if (second) unmap_domain_page(second);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 0d1e61e..f204482 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -135,8 +135,8 @@ void p2m_restore_state(struct vcpu *n);
/* Print debugging/statistial info about a domain's p2m */
void p2m_dump_info(struct domain *d);
-/* Look up the MFN corresponding to a domain's PFN. */
-paddr_t p2m_lookup(struct domain *d, paddr_t gpfn, p2m_type_t *t);
+/* Look up the MFN corresponding to a domain's GFN. */
+mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t);
/* Clean & invalidate caches corresponding to a region of guest address space */
int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
@@ -201,8 +201,7 @@ static inline struct page_info *get_page_from_gfn(
{
struct page_info *page;
p2m_type_t p2mt;
- paddr_t maddr = p2m_lookup(d, pfn_to_paddr(gfn), &p2mt);
- unsigned long mfn = maddr >> PAGE_SHIFT;
+ unsigned long mfn = mfn_x(p2m_lookup(d, _gfn(gfn), &p2mt));
if (t)
*t = p2mt;
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 7/8] xen/mm: Introduce max_gfn and min_gfn
2016-06-14 12:06 [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn Julien Grall
` (5 preceding siblings ...)
2016-06-14 12:07 ` [PATCH 6/8] xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn Julien Grall
@ 2016-06-14 12:07 ` Julien Grall
2016-06-14 13:05 ` Jan Beulich
2016-06-14 12:07 ` [PATCH 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn Julien Grall
7 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:07 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Jan Beulich, wei.chen
Those helpers will be useful to find the maximum/minimum between two
GFNs without having to unbox/box manually.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.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: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/include/xen/mm.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index a22c4c2..6fddb6f 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -70,6 +70,9 @@ TYPE_SAFE(unsigned long, gfn);
#undef gfn_t
#endif
+#define max_gfn(x, y) _gfn(max(gfn_x(x), gfn_x(y)))
+#define min_gfn(x, y) _gfn(min(gfn_x(x), gfn_x(y)))
+
TYPE_SAFE(unsigned long, pfn);
#define PRI_pfn "05lx"
#define INVALID_PFN (~0UL)
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn
2016-06-14 12:06 [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn Julien Grall
` (6 preceding siblings ...)
2016-06-14 12:07 ` [PATCH 7/8] xen/mm: Introduce max_gfn and min_gfn Julien Grall
@ 2016-06-14 12:07 ` Julien Grall
2016-06-14 13:13 ` Andrew Cooper
7 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:07 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, sstabellini, wei.chen
p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename
the variable to *gfn* and use typesafe to avoid possible misusage.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/domctl.c | 2 +-
xen/arch/arm/p2m.c | 10 +++++-----
xen/include/asm-arm/p2m.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 30453d8..b94e97c 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -30,7 +30,7 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
if ( e < s )
return -EINVAL;
- return p2m_cache_flush(d, s, e);
+ return p2m_cache_flush(d, _gfn(s), _gfn(e));
}
case XEN_DOMCTL_bind_pt_irq:
{
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 135d032..d50eda3 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1467,16 +1467,16 @@ int relinquish_p2m_mapping(struct domain *d)
d->arch.p2m.default_access);
}
-int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn)
+int p2m_cache_flush(struct domain *d, gfn_t start_gfn, gfn_t end_gfn)
{
struct p2m_domain *p2m = &d->arch.p2m;
- start_mfn = MAX(start_mfn, p2m->lowest_mapped_gfn);
- end_mfn = MIN(end_mfn, p2m->max_mapped_gfn);
+ start_gfn = max_gfn(start_gfn, _gfn(p2m->lowest_mapped_gfn));
+ end_gfn = min_gfn(end_gfn, _gfn(p2m->max_mapped_gfn));
return apply_p2m_changes(d, CACHEFLUSH,
- pfn_to_paddr(start_mfn),
- pfn_to_paddr(end_mfn),
+ pfn_to_paddr(gfn_x(start_gfn)),
+ pfn_to_paddr(gfn_x(end_gfn)),
pfn_to_paddr(INVALID_MFN),
MATTR_MEM, 0, p2m_invalid,
d->arch.p2m.default_access);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index f204482..03a5cce 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -139,7 +139,7 @@ void p2m_dump_info(struct domain *d);
mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t);
/* Clean & invalidate caches corresponding to a region of guest address space */
-int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
+int p2m_cache_flush(struct domain *d, gfn_t start_gfn, gfn_t end_gfn);
/* Setup p2m RAM mapping for domain d from start-end. */
int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe
2016-06-14 12:07 ` [PATCH 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe Julien Grall
@ 2016-06-14 12:44 ` Jan Beulich
2016-06-14 12:54 ` Julien Grall
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2016-06-14 12:44 UTC (permalink / raw)
To: Julien Grall; +Cc: sstabellini, xen-devel, wei.chen
>>> On 14.06.16 at 14:07, <julien.grall@arm.com> wrote:
> The correct acronym for a guest physical frame is gfn. Also use
> the recently introduced typesafe gfn/mfn to avoid mixing the two
> different kind of frame.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
> xen/arch/arm/p2m.c | 6 +++---
> xen/common/grant_table.c | 4 ++--
> xen/common/memory.c | 4 ++--
> xen/include/asm-arm/p2m.h | 2 +-
> 4 files changed, 8 insertions(+), 8 deletions(-)
Acked-by: Jan Beulich <jbeulich@suse.com>
(please don't forget to always Cc all relevant maintainers)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe
2016-06-14 12:44 ` Jan Beulich
@ 2016-06-14 12:54 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2016-06-14 12:54 UTC (permalink / raw)
To: Jan Beulich; +Cc: sstabellini, xen-devel, wei.chen
On 14/06/16 13:44, Jan Beulich wrote:
>>>> On 14.06.16 at 14:07, <julien.grall@arm.com> wrote:
>> The correct acronym for a guest physical frame is gfn. Also use
>> the recently introduced typesafe gfn/mfn to avoid mixing the two
>> different kind of frame.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>> xen/arch/arm/p2m.c | 6 +++---
>> xen/common/grant_table.c | 4 ++--
>> xen/common/memory.c | 4 ++--
>> xen/include/asm-arm/p2m.h | 2 +-
>> 4 files changed, 8 insertions(+), 8 deletions(-)
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
>
> (please don't forget to always Cc all relevant maintainers)
Sorry I forgot to double check what was modified by the patch. I saw
"xen/arm:" in the title and thought it was ARM only.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/8] xen: Use typesafe gfn/mfn in guest_physmap_* helpers
2016-06-14 12:07 ` [PATCH 2/8] xen: Use typesafe gfn/mfn in guest_physmap_* helpers Julien Grall
@ 2016-06-14 12:56 ` Jan Beulich
2016-06-20 11:28 ` Julien Grall
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2016-06-14 12:56 UTC (permalink / raw)
To: Julien Grall
Cc: Tim Deegan, sstabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, xen-devel, Paul Durrant, wei.chen
>>> On 14.06.16 at 14:07, <julien.grall@arm.com> wrote:
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -665,21 +665,21 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn, unsigned long mfn,
> }
>
> int
> -guest_physmap_remove_page(struct domain *d, unsigned long gfn,
> - unsigned long mfn, unsigned int page_order)
> +guest_physmap_remove_page(struct domain *d, gfn_t gfn,
> + mfn_t mfn, unsigned int page_order)
> {
> struct p2m_domain *p2m = p2m_get_hostp2m(d);
> int rc;
> gfn_lock(p2m, gfn, page_order);
> - rc = p2m_remove_page(p2m, gfn, mfn, page_order);
> + rc = p2m_remove_page(p2m, gfn_x(gfn), mfn_x(mfn), page_order);
> gfn_unlock(p2m, gfn, page_order);
> return rc;
> }
>
> -int
> -guest_physmap_add_entry(struct domain *d, unsigned long gfn,
> - unsigned long mfn, unsigned int page_order,
> - p2m_type_t t)
> +static int
> +__guest_physmap_add_entry(struct domain *d, unsigned long gfn,
At the very least only a single underscore please.
> + unsigned long mfn, unsigned int page_order,
> + p2m_type_t t)
> {
> struct p2m_domain *p2m = p2m_get_hostp2m(d);
> unsigned long i, ogfn;
> @@ -838,6 +838,13 @@ out:
> return rc;
> }
>
> +/* XXX: To be removed when __guest_physmap_add_entry will use typesafe */
But not just because of this (misleading) comment I really wonder
what the point here is: Is it really that much more intrusive to
change the function right away instead of introducing a wrapper?
(The comment is misleading because __guest_physmap_add_entry
shouldn't survive after the conversion to proper types, i.e. it is
not the function below which is supposed to get removed.)
> +int
> +guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
> + unsigned int page_order, p2m_type_t t)
> +{
> + return __guest_physmap_add_entry(d, gfn_x(gfn), mfn_x(mfn), page_order, t);
> +}
Perhaps a better (wrapper-less) approach (if full conversion is indeed
beyond scope) would be to simply rename the function parameters
and have local variables named "gfn" and "mfn"?
> @@ -2785,7 +2792,8 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
> unsigned long gpfn, domid_t foreigndom)
> {
> p2m_type_t p2mt, p2mt_prev;
> - unsigned long prev_mfn, mfn;
> + mfn_t prev_mfn;
> + unsigned long mfn;
This looks to make things more inconsistent rather than more consistent.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/8] xen: Use typesafe gfn in xenmem_add_to_physmap_one
2016-06-14 12:07 ` [PATCH 3/8] xen: Use typesafe gfn in xenmem_add_to_physmap_one Julien Grall
@ 2016-06-14 12:58 ` Jan Beulich
0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2016-06-14 12:58 UTC (permalink / raw)
To: Julien Grall
Cc: Tim Deegan, sstabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, xen-devel, wei.chen
>>> On 14.06.16 at 14:07, <julien.grall@arm.com> wrote:
> The x86 version of the function xenmem_add_to_physmap_one contains
> variable name gpfn and gfn which make the code very confusing.
> I have left unchanged for now.
>
> Also, rename gpfn to gfn in the ARM version as the latter is the correct
> acronym for a guest physical frame.
>
> Finally, remove the trailing whitespace around the changes.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Non-ARM bits:
Acked-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/8] xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the typesafe gfn
2016-06-14 12:07 ` [PATCH 4/8] xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the typesafe gfn Julien Grall
@ 2016-06-14 13:00 ` Andrew Cooper
2016-06-20 12:40 ` Julien Grall
0 siblings, 1 reply; 21+ messages in thread
From: Andrew Cooper @ 2016-06-14 13:00 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: sstabellini, wei.chen
On 14/06/16 13:07, Julien Grall wrote:
> diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
> index 5e076cc..46cfe24 100644
> --- a/xen/include/asm-arm/grant_table.h
> +++ b/xen/include/asm-arm/grant_table.h
> @@ -30,7 +30,7 @@ static inline int replace_grant_supported(void)
>
> #define gnttab_shared_gmfn(d, t, i) \
> ( ((i >= nr_grant_frames(d->grant_table)) && \
> - (i < max_grant_frames)) ? 0 : (d->arch.grant_table_gpfn[i]))
> + (i < max_grant_frames)) ? 0 : gfn_x((d->arch.grant_table_gfn[i])))
One small nit. You can drop one pair of brackets inside the gfn_x() call.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/8] xen: Use the typesafe mfn and gfn in map_mmio_regions...
2016-06-14 12:07 ` [PATCH 5/8] xen: Use the typesafe mfn and gfn in map_mmio_regions Julien Grall
@ 2016-06-14 13:02 ` Jan Beulich
2016-06-20 12:41 ` Julien Grall
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2016-06-14 13:02 UTC (permalink / raw)
To: Julien Grall
Cc: Tim Deegan, sstabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, xen-devel, wei.chen
>>> On 14.06.16 at 14:07, <julien.grall@arm.com> wrote:
> to avoid mixing machine frame with guest frame.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Non-ARM bits:
Acked-by: Jan Beulich <jbeulich@suse.com>
> @@ -2217,10 +2217,11 @@ int map_mmio_regions(struct domain *d,
> i += 1UL << order, ++iter )
> {
> /* OR'ing gfn and mfn values will return an order suitable to both. */
> - for ( order = mmio_order(d, (start_gfn + i) | (mfn + i), nr - i); ;
> + for ( order = mmio_order(d, (gfn_x(start_gfn) + i) | (mfn_x(mfn) + i), nr - i); ;
> order = ret - 1 )
> {
> - ret = set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i), order,
> + ret = set_mmio_p2m_entry(d, gfn_x(start_gfn) + i,
> + _mfn(mfn_x(mfn) + i), order,
Considering this new instance I realize we really should have
constructs like mfn_add() to avoid such ugly to read expressions.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 7/8] xen/mm: Introduce max_gfn and min_gfn
2016-06-14 12:07 ` [PATCH 7/8] xen/mm: Introduce max_gfn and min_gfn Julien Grall
@ 2016-06-14 13:05 ` Jan Beulich
2016-06-14 13:11 ` Andrew Cooper
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2016-06-14 13:05 UTC (permalink / raw)
To: Julien Grall
Cc: Tim Deegan, sstabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, xen-devel, wei.chen
>>> On 14.06.16 at 14:07, <julien.grall@arm.com> wrote:
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -70,6 +70,9 @@ TYPE_SAFE(unsigned long, gfn);
> #undef gfn_t
> #endif
>
> +#define max_gfn(x, y) _gfn(max(gfn_x(x), gfn_x(y)))
> +#define min_gfn(x, y) _gfn(min(gfn_x(x), gfn_x(y)))
With my reply to an earlier patch in mind I think these would better
be gfn_min() and gfn_max() (to be extended with gfn_add() and
mfn_add() and maybe some others).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 7/8] xen/mm: Introduce max_gfn and min_gfn
2016-06-14 13:05 ` Jan Beulich
@ 2016-06-14 13:11 ` Andrew Cooper
0 siblings, 0 replies; 21+ messages in thread
From: Andrew Cooper @ 2016-06-14 13:11 UTC (permalink / raw)
To: Jan Beulich, Julien Grall
Cc: sstabellini, Wei Liu, George Dunlap, Tim Deegan, Ian Jackson,
xen-devel, wei.chen
On 14/06/16 14:05, Jan Beulich wrote:
>>>> On 14.06.16 at 14:07, <julien.grall@arm.com> wrote:
>> --- a/xen/include/xen/mm.h
>> +++ b/xen/include/xen/mm.h
>> @@ -70,6 +70,9 @@ TYPE_SAFE(unsigned long, gfn);
>> #undef gfn_t
>> #endif
>>
>> +#define max_gfn(x, y) _gfn(max(gfn_x(x), gfn_x(y)))
>> +#define min_gfn(x, y) _gfn(min(gfn_x(x), gfn_x(y)))
> With my reply to an earlier patch in mind I think these would better
> be gfn_min() and gfn_max() (to be extended with gfn_add() and
> mfn_add() and maybe some others).
+1 to the alternative naming suggestion.
I would also suggest making these static inlines rather than defines.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn
2016-06-14 12:07 ` [PATCH 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn Julien Grall
@ 2016-06-14 13:13 ` Andrew Cooper
0 siblings, 0 replies; 21+ messages in thread
From: Andrew Cooper @ 2016-06-14 13:13 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: sstabellini, wei.chen
On 14/06/16 13:07, Julien Grall wrote:
> p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename
> the variable to *gfn* and use typesafe to avoid possible misusage.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
> xen/arch/arm/domctl.c | 2 +-
> xen/arch/arm/p2m.c | 10 +++++-----
> xen/include/asm-arm/p2m.h | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> index 30453d8..b94e97c 100644
> --- a/xen/arch/arm/domctl.c
> +++ b/xen/arch/arm/domctl.c
> @@ -30,7 +30,7 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
> if ( e < s )
> return -EINVAL;
>
> - return p2m_cache_flush(d, s, e);
> + return p2m_cache_flush(d, _gfn(s), _gfn(e));
> }
> case XEN_DOMCTL_bind_pt_irq:
> {
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 135d032..d50eda3 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1467,16 +1467,16 @@ int relinquish_p2m_mapping(struct domain *d)
> d->arch.p2m.default_access);
> }
>
> -int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn)
> +int p2m_cache_flush(struct domain *d, gfn_t start_gfn, gfn_t end_gfn)
I would recommend dropping the _gfn suffix from start and end. It only
serves to make the code longer, as the numbers are of gfn_t type.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/8] xen: Use typesafe gfn/mfn in guest_physmap_* helpers
2016-06-14 12:56 ` Jan Beulich
@ 2016-06-20 11:28 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2016-06-20 11:28 UTC (permalink / raw)
To: Jan Beulich
Cc: Tim Deegan, sstabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, xen-devel, Paul Durrant, wei.chen
Hi Jan,
On 14/06/16 13:56, Jan Beulich wrote:
>>>> On 14.06.16 at 14:07, <julien.grall@arm.com> wrote:
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -665,21 +665,21 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn, unsigned long mfn,
>> }
>>
>> int
>> -guest_physmap_remove_page(struct domain *d, unsigned long gfn,
>> - unsigned long mfn, unsigned int page_order)
>> +guest_physmap_remove_page(struct domain *d, gfn_t gfn,
>> + mfn_t mfn, unsigned int page_order)
>> {
>> struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> int rc;
>> gfn_lock(p2m, gfn, page_order);
>> - rc = p2m_remove_page(p2m, gfn, mfn, page_order);
>> + rc = p2m_remove_page(p2m, gfn_x(gfn), mfn_x(mfn), page_order);
>> gfn_unlock(p2m, gfn, page_order);
>> return rc;
>> }
>>
>> -int
>> -guest_physmap_add_entry(struct domain *d, unsigned long gfn,
>> - unsigned long mfn, unsigned int page_order,
>> - p2m_type_t t)
>> +static int
>> +__guest_physmap_add_entry(struct domain *d, unsigned long gfn,
>
> At the very least only a single underscore please.
>
>> + unsigned long mfn, unsigned int page_order,
>> + p2m_type_t t)
>> {
>> struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> unsigned long i, ogfn;
>> @@ -838,6 +838,13 @@ out:
>> return rc;
>> }
>>
>> +/* XXX: To be removed when __guest_physmap_add_entry will use typesafe */
>
> But not just because of this (misleading) comment I really wonder
> what the point here is: Is it really that much more intrusive to
> change the function right away instead of introducing a wrapper?
I though it was intrusive because there are multiple place using adding
a value to the mfn/gfn.
It might be less intrusive with your suggestion to add mfn_* wrappers
for common operation. I will give another look.
> (The comment is misleading because __guest_physmap_add_entry
> shouldn't survive after the conversion to proper types, i.e. it is
> not the function below which is supposed to get removed.)
>
>> +int
>> +guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
>> + unsigned int page_order, p2m_type_t t)
>> +{
>> + return __guest_physmap_add_entry(d, gfn_x(gfn), mfn_x(mfn), page_order, t);
>> +}
>
> Perhaps a better (wrapper-less) approach (if full conversion is indeed
> beyond scope) would be to simply rename the function parameters
> and have local variables named "gfn" and "mfn"?
>
>> @@ -2785,7 +2792,8 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
>> unsigned long gpfn, domid_t foreigndom)
>> {
>> p2m_type_t p2mt, p2mt_prev;
>> - unsigned long prev_mfn, mfn;
>> + mfn_t prev_mfn;
>> + unsigned long mfn;
>
> This looks to make things more inconsistent rather than more consistent.
The usage of the variable in p2m_add_foreign seems limited to a couple
of place. I will use mfn_t for it too.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/8] xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the typesafe gfn
2016-06-14 13:00 ` Andrew Cooper
@ 2016-06-20 12:40 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2016-06-20 12:40 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: sstabellini, wei.chen
Hi Andrew,
On 14/06/16 14:00, Andrew Cooper wrote:
> On 14/06/16 13:07, Julien Grall wrote:
>> diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
>> index 5e076cc..46cfe24 100644
>> --- a/xen/include/asm-arm/grant_table.h
>> +++ b/xen/include/asm-arm/grant_table.h
>> @@ -30,7 +30,7 @@ static inline int replace_grant_supported(void)
>>
>> #define gnttab_shared_gmfn(d, t, i) \
>> ( ((i >= nr_grant_frames(d->grant_table)) && \
>> - (i < max_grant_frames)) ? 0 : (d->arch.grant_table_gpfn[i]))
>> + (i < max_grant_frames)) ? 0 : gfn_x((d->arch.grant_table_gfn[i])))
>
> One small nit. You can drop one pair of brackets inside the gfn_x() call.
I will drop them in the next version.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/8] xen: Use the typesafe mfn and gfn in map_mmio_regions...
2016-06-14 13:02 ` Jan Beulich
@ 2016-06-20 12:41 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2016-06-20 12:41 UTC (permalink / raw)
To: Jan Beulich
Cc: Tim Deegan, sstabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, xen-devel, wei.chen
Hi Jan,
On 14/06/16 14:02, Jan Beulich wrote:
>>>> On 14.06.16 at 14:07, <julien.grall@arm.com> wrote:
>> to avoid mixing machine frame with guest frame.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> Non-ARM bits:
> Acked-by: Jan Beulich <jbeulich@suse.com>
>
>> @@ -2217,10 +2217,11 @@ int map_mmio_regions(struct domain *d,
>> i += 1UL << order, ++iter )
>> {
>> /* OR'ing gfn and mfn values will return an order suitable to both. */
>> - for ( order = mmio_order(d, (start_gfn + i) | (mfn + i), nr - i); ;
>> + for ( order = mmio_order(d, (gfn_x(start_gfn) + i) | (mfn_x(mfn) + i), nr - i); ;
>> order = ret - 1 )
>> {
>> - ret = set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i), order,
>> + ret = set_mmio_p2m_entry(d, gfn_x(start_gfn) + i,
>> + _mfn(mfn_x(mfn) + i), order,
>
> Considering this new instance I realize we really should have
> constructs like mfn_add() to avoid such ugly to read expressions.
Good point. I will introduce {gfn,mfn}_add.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2016-06-20 12:41 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-14 12:06 [PATCH 0/8] xen/arm: Use the typesafes gfn and mfn Julien Grall
2016-06-14 12:07 ` [PATCH 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe Julien Grall
2016-06-14 12:44 ` Jan Beulich
2016-06-14 12:54 ` Julien Grall
2016-06-14 12:07 ` [PATCH 2/8] xen: Use typesafe gfn/mfn in guest_physmap_* helpers Julien Grall
2016-06-14 12:56 ` Jan Beulich
2016-06-20 11:28 ` Julien Grall
2016-06-14 12:07 ` [PATCH 3/8] xen: Use typesafe gfn in xenmem_add_to_physmap_one Julien Grall
2016-06-14 12:58 ` Jan Beulich
2016-06-14 12:07 ` [PATCH 4/8] xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the typesafe gfn Julien Grall
2016-06-14 13:00 ` Andrew Cooper
2016-06-20 12:40 ` Julien Grall
2016-06-14 12:07 ` [PATCH 5/8] xen: Use the typesafe mfn and gfn in map_mmio_regions Julien Grall
2016-06-14 13:02 ` Jan Beulich
2016-06-20 12:41 ` Julien Grall
2016-06-14 12:07 ` [PATCH 6/8] xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn Julien Grall
2016-06-14 12:07 ` [PATCH 7/8] xen/mm: Introduce max_gfn and min_gfn Julien Grall
2016-06-14 13:05 ` Jan Beulich
2016-06-14 13:11 ` Andrew Cooper
2016-06-14 12:07 ` [PATCH 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn Julien Grall
2016-06-14 13:13 ` Andrew Cooper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).