* [PATCH 0/8] xen/arm: Add xentrace support @ 2018-11-06 19:14 Julien Grall 2018-11-06 19:14 ` [PATCH 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code Julien Grall ` (7 more replies) 0 siblings, 8 replies; 44+ messages in thread From: Julien Grall @ 2018-11-06 19:14 UTC (permalink / raw) To: sstabellini, xen-devel Cc: Jun Nakajima, Kevin Tian, Wei Liu, Suravee Suthikulpanit, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, Paul Durrant, Jan Beulich, Boris Ostrovsky, Brian Woods Hi all, This patch series is a rework of the series sent by Benjamin Sanda in April 2016 [1]. It finally adds support for xentrace/xenanalyze on Arm. Cheers, [1] https://lists.xenproject.org/archives/html/xen-devel/2016-04/msg00464.html Benjamin Sanda (3): xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code xen/arm: Initialize trace buffer xenalyze: Build for Both ARM and x86 Platforms Julien Grall (5): xen/arm: p2m: Introduce p2m_get_page_from_gfn xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw xen/arm: Add support for read-only foreign mappings xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN xen: Swich parameter in get_page_from_gfn to use typesafe gfn tools/xentrace/Makefile | 3 +- xen/arch/arm/guestcopy.c | 2 +- xen/arch/arm/mm.c | 16 ++++----- xen/arch/arm/p2m.c | 35 ++++++++++++++++++- xen/arch/arm/setup.c | 3 ++ xen/arch/x86/cpu/vpmu.c | 2 +- xen/arch/x86/domain.c | 12 +++---- xen/arch/x86/domctl.c | 6 ++-- xen/arch/x86/hvm/dm.c | 2 +- xen/arch/x86/hvm/domain.c | 2 +- xen/arch/x86/hvm/hvm.c | 9 ++--- xen/arch/x86/hvm/svm/svm.c | 8 ++--- xen/arch/x86/hvm/viridian/viridian.c | 24 ++++++------- xen/arch/x86/hvm/vmx/vmx.c | 4 +-- xen/arch/x86/hvm/vmx/vvmx.c | 12 +++---- xen/arch/x86/mm.c | 66 ++++++++---------------------------- xen/arch/x86/mm/p2m.c | 2 +- xen/arch/x86/mm/shadow/hvm.c | 6 ++-- xen/arch/x86/physdev.c | 3 +- xen/arch/x86/pv/descriptor-tables.c | 5 ++- xen/arch/x86/pv/emul-priv-op.c | 6 ++-- xen/arch/x86/pv/mm.c | 2 +- xen/arch/x86/traps.c | 11 +++--- xen/common/domain.c | 2 +- xen/common/event_fifo.c | 12 +++---- xen/common/memory.c | 4 +-- xen/common/page_alloc.c | 38 +++++++++++++++++++++ xen/common/tmem_xen.c | 2 +- xen/include/asm-arm/p2m.h | 57 ++++++++++++++++++------------- xen/include/asm-x86/p2m.h | 11 +++--- xen/include/xen/mm.h | 3 ++ 31 files changed, 212 insertions(+), 158 deletions(-) -- 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] 44+ messages in thread
* [PATCH 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code 2018-11-06 19:14 [PATCH 0/8] xen/arm: Add xentrace support Julien Grall @ 2018-11-06 19:14 ` Julien Grall 2018-11-07 9:28 ` Jan Beulich 2018-11-06 19:14 ` [PATCH 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn Julien Grall ` (6 subsequent siblings) 7 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-06 19:14 UTC (permalink / raw) To: sstabellini, xen-devel Cc: Wei Liu, Benjamin Sanda, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich From: Benjamin Sanda <ben.sanda@dornerworks.com> get_pg_owner() and put_pg_owner() will be necessary in a follow-up commit to support xentrace on Arm. So move the helper to common code. Note that put_pg_owner() has been turned to a macro rather than static inline because of inter-dependency between includes. Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com> [julien: Rework commit title / turn put_pg_owner to a macro] Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/x86/mm.c | 42 ------------------------------------------ xen/common/page_alloc.c | 38 ++++++++++++++++++++++++++++++++++++++ xen/include/xen/mm.h | 3 +++ 3 files changed, 41 insertions(+), 42 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index f043e43ac7..9363e9bd96 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -3100,48 +3100,6 @@ static int vcpumask_to_pcpumask( } } -static struct domain *get_pg_owner(domid_t domid) -{ - struct domain *pg_owner = NULL, *curr = current->domain; - - if ( likely(domid == DOMID_SELF) ) - { - pg_owner = rcu_lock_current_domain(); - goto out; - } - - if ( unlikely(domid == curr->domain_id) ) - { - gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n"); - goto out; - } - - switch ( domid ) - { - case DOMID_IO: - pg_owner = rcu_lock_domain(dom_io); - break; - case DOMID_XEN: - pg_owner = rcu_lock_domain(dom_xen); - break; - default: - if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL ) - { - gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid); - break; - } - break; - } - - out: - return pg_owner; -} - -static void put_pg_owner(struct domain *pg_owner) -{ - rcu_unlock_domain(pg_owner); -} - long do_mmuext_op( XEN_GUEST_HANDLE_PARAM(mmuext_op_t) uops, unsigned int count, diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 16e1b0c357..ef1b4f596a 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -146,6 +146,7 @@ #include <asm/guest.h> #include <asm/p2m.h> #include <asm/setup.h> /* for highmem_start only */ +#include <asm/paging.h> #else #define p2m_pod_offline_or_broken_hit(pg) 0 #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL) @@ -2447,6 +2448,43 @@ static __init int register_heap_trigger(void) } __initcall(register_heap_trigger); +struct domain *get_pg_owner(domid_t domid) +{ + struct domain *pg_owner = NULL, *curr = current->domain; + + if ( likely(domid == DOMID_SELF) ) + { + pg_owner = rcu_lock_current_domain(); + goto out; + } + + if ( unlikely(domid == curr->domain_id) ) + { + gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n"); + goto out; + } + + switch ( domid ) + { + case DOMID_IO: + pg_owner = rcu_lock_domain(dom_io); + break; + case DOMID_XEN: + pg_owner = rcu_lock_domain(dom_xen); + break; + default: + if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL ) + { + gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid); + break; + } + break; + } + + out: + return pg_owner; +} + /* * Local variables: * mode: C diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index 054d02e6c0..dd4d990ae3 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -589,6 +589,9 @@ int xenmem_add_to_physmap_one(struct domain *d, unsigned int space, int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp, unsigned int start); +struct domain *get_pg_owner(domid_t domid); +#define put_pg_owner(pg_owner) rcu_unlock_domain(pg_owner) + /* Return 0 on success, or negative on error. */ int __must_check guest_remove_page(struct domain *d, unsigned long gmfn); int __must_check steal_page(struct domain *d, struct page_info *page, -- 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] 44+ messages in thread
* Re: [PATCH 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code 2018-11-06 19:14 ` [PATCH 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code Julien Grall @ 2018-11-07 9:28 ` Jan Beulich 2018-11-09 18:06 ` Julien Grall 0 siblings, 1 reply; 44+ messages in thread From: Jan Beulich @ 2018-11-07 9:28 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Wei Liu, Benjamin Sanda, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, xen-devel >>> On 06.11.18 at 20:14, <julien.grall@arm.com> wrote: > From: Benjamin Sanda <ben.sanda@dornerworks.com> > > get_pg_owner() and put_pg_owner() will be necessary in a follow-up > commit to support xentrace on Arm. So move the helper to common code. > > Note that put_pg_owner() has been turned to a macro rather than static > inline because of inter-dependency between includes. Could this be avoided by placing both in a different header, e.g. sched.h? Mid-term we should probably try to split out type (structure) declarations from sched.h, to make it easier to have full types available without having to respect other include dependencies. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code 2018-11-07 9:28 ` Jan Beulich @ 2018-11-09 18:06 ` Julien Grall 0 siblings, 0 replies; 44+ messages in thread From: Julien Grall @ 2018-11-09 18:06 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Benjamin Sanda, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, xen-devel Hi Jan, On 07/11/2018 09:28, Jan Beulich wrote: >>>> On 06.11.18 at 20:14, <julien.grall@arm.com> wrote: >> From: Benjamin Sanda <ben.sanda@dornerworks.com> >> >> get_pg_owner() and put_pg_owner() will be necessary in a follow-up >> commit to support xentrace on Arm. So move the helper to common code. >> >> Note that put_pg_owner() has been turned to a macro rather than static >> inline because of inter-dependency between includes. > > Could this be avoided by placing both in a different header, > e.g. sched.h? This seems to work. I will use that in the next version. > Mid-term we should probably try to split out > type (structure) declarations from sched.h, to make it easier > to have full types available without having to respect other > include dependencies. The include dependencies on Xen is a bit of a nightmare. I have started to clean it up on Arm and I will support any changes in the common headers. 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] 44+ messages in thread
* [PATCH 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn 2018-11-06 19:14 [PATCH 0/8] xen/arm: Add xentrace support Julien Grall 2018-11-06 19:14 ` [PATCH 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code Julien Grall @ 2018-11-06 19:14 ` Julien Grall 2018-11-15 13:31 ` Andrii Anisov 2018-11-06 19:14 ` [PATCH 3/8] xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw Julien Grall ` (5 subsequent siblings) 7 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-06 19:14 UTC (permalink / raw) To: sstabellini, xen-devel; +Cc: Julien Grall In a follow-up patches, we will need to handle get_page_from_gfn differently for DOMID_XEN. To keep the code simple move the current content in a new separate helper p2m_get_page_from_gfn. Note the new helper is a not anymore a static inline function as the helper is quite complex. Finally, take the opportunity to use typesafe gfn as the change is minor. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/p2m.c | 32 ++++++++++++++++++++++++++++++++ xen/include/asm-arm/p2m.h | 33 ++++----------------------------- 2 files changed, 36 insertions(+), 29 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 30cfb01498..04c8718e9f 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -379,6 +379,38 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t) return mfn; } +struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn, + p2m_type_t *t) +{ + struct page_info *page; + p2m_type_t p2mt; + mfn_t mfn = p2m_lookup(d, gfn, &p2mt); + + if (t) + *t = p2mt; + + if ( !p2m_is_any_ram(p2mt) ) + return NULL; + + if ( !mfn_valid(mfn) ) + return NULL; + page = mfn_to_page(mfn); + + /* + * get_page won't work on foreign mapping because the page doesn't + * belong to the current domain. + */ + if ( p2m_is_foreign(p2mt) ) + { + struct domain *fdom = page_get_owner_and_reference(page); + ASSERT(fdom != NULL); + ASSERT(fdom != d); + return page; + } + + return (get_page(page, d) ? page: NULL); +} + int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn, unsigned int order) diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index c03557544a..a5914136e3 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -269,38 +269,13 @@ typedef unsigned int p2m_query_t; #define P2M_ALLOC (1u<<0) /* Populate PoD and paged-out entries */ #define P2M_UNSHARE (1u<<1) /* Break CoW sharing */ +struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn, + p2m_type_t *t); + static inline struct page_info *get_page_from_gfn( struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) { - struct page_info *page; - p2m_type_t p2mt; - mfn_t mfn = p2m_lookup(d, _gfn(gfn), &p2mt); - - if (t) - *t = p2mt; - - if ( !p2m_is_any_ram(p2mt) ) - return NULL; - - if ( !mfn_valid(mfn) ) - return NULL; - page = mfn_to_page(mfn); - - /* - * get_page won't work on foreign mapping because the page doesn't - * belong to the current domain. - */ - if ( p2m_is_foreign(p2mt) ) - { - struct domain *fdom = page_get_owner_and_reference(page); - ASSERT(fdom != NULL); - ASSERT(fdom != d); - return page; - } - - if ( !get_page(page, d) ) - return NULL; - return page; + return p2m_get_page_from_gfn(d, _gfn(gfn), t); } int get_page_type(struct page_info *page, unsigned long type); -- 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] 44+ messages in thread
* Re: [PATCH 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn 2018-11-06 19:14 ` [PATCH 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn Julien Grall @ 2018-11-15 13:31 ` Andrii Anisov 2018-11-15 13:35 ` Andrii Anisov 2018-11-15 15:22 ` Julien Grall 0 siblings, 2 replies; 44+ messages in thread From: Andrii Anisov @ 2018-11-15 13:31 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini Hello Julien, вт, 6 лист. 2018 о 21:16 Julien Grall <julien.grall@arm.com> пише: > > In a follow-up patches, we will need to handle get_page_from_gfn > differently for DOMID_XEN. To keep the code simple move the current > content in a new separate helper p2m_get_page_from_gfn. > > Note the new helper is a not anymore a static inline function as the helper > is quite complex. In the patch "[PATCH 4/8] xen/arm: Add support for read-only foreign mappings" you make p2m_get_page_from_gfn() comparingly complex, but still keep it as static inline. Could you please be consistent (making both of those functions inline or non-inline) Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn 2018-11-15 13:31 ` Andrii Anisov @ 2018-11-15 13:35 ` Andrii Anisov 2018-11-15 15:22 ` Julien Grall 1 sibling, 0 replies; 44+ messages in thread From: Andrii Anisov @ 2018-11-15 13:35 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini Sorry, Not "comparingly complex", but "equally complex". ;) Sincerely, Andrii Anisov. чт, 15 лист. 2018 о 15:31 Andrii Anisov <andrii.anisov@gmail.com> пише: > > Hello Julien, > > вт, 6 лист. 2018 о 21:16 Julien Grall <julien.grall@arm.com> пише: > > > > In a follow-up patches, we will need to handle get_page_from_gfn > > differently for DOMID_XEN. To keep the code simple move the current > > content in a new separate helper p2m_get_page_from_gfn. > > > > Note the new helper is a not anymore a static inline function as the helper > > is quite complex. > > In the patch "[PATCH 4/8] xen/arm: Add support for read-only foreign > mappings" you make p2m_get_page_from_gfn() comparingly complex, but > still keep it as static inline. Could you please be consistent (making > both of those functions inline or non-inline) > > Sincerely, > Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn 2018-11-15 13:31 ` Andrii Anisov 2018-11-15 13:35 ` Andrii Anisov @ 2018-11-15 15:22 ` Julien Grall 2018-12-20 11:20 ` Andrii Anisov 1 sibling, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-15 15:22 UTC (permalink / raw) To: Andrii Anisov; +Cc: xen-devel, Stefano Stabellini On 11/15/18 1:31 PM, Andrii Anisov wrote: > Hello Julien, Hi, > > вт, 6 лист. 2018 о 21:16 Julien Grall <julien.grall@arm.com> пише: >> >> In a follow-up patches, we will need to handle get_page_from_gfn >> differently for DOMID_XEN. To keep the code simple move the current >> content in a new separate helper p2m_get_page_from_gfn. >> >> Note the new helper is a not anymore a static inline function as the helper >> is quite complex. > > In the patch "[PATCH 4/8] xen/arm: Add support for read-only foreign > mappings" you make p2m_get_page_from_gfn() comparingly complex, but > still keep it as static inline. Could you please be consistent (making > both of those functions inline or non-inline) The reason I didn't move the other one in p2m.c is because so far p2m.c is only dealing with auto-translated guest. I didn't want to add function related with non-auto translated guest in it. I also don't think introduce a new file for one 10 line function is really useful. So that was the best solution. I am open to other suggestion. 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] 44+ messages in thread
* Re: [PATCH 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn 2018-11-15 15:22 ` Julien Grall @ 2018-12-20 11:20 ` Andrii Anisov 0 siblings, 0 replies; 44+ messages in thread From: Andrii Anisov @ 2018-12-20 11:20 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini On 15.11.18 17:22, Julien Grall wrote: > The reason I didn't move the other one in p2m.c is because so far p2m.c is only dealing with auto-translated guest. I didn't want to add function related with non-auto translated guest in it. > > I also don't think introduce a new file for one 10 line function is really useful. > > So that was the best solution. I am open to other suggestion. > > Cheers, Reviewed-by: Andrii Anisov <andrii_anisov@epam.com> -- Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 3/8] xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw 2018-11-06 19:14 [PATCH 0/8] xen/arm: Add xentrace support Julien Grall 2018-11-06 19:14 ` [PATCH 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code Julien Grall 2018-11-06 19:14 ` [PATCH 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn Julien Grall @ 2018-11-06 19:14 ` Julien Grall 2018-11-15 11:42 ` Andrii Anisov 2018-11-06 19:14 ` [PATCH 4/8] xen/arm: Add support for read-only foreign mappings Julien Grall ` (4 subsequent siblings) 7 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-06 19:14 UTC (permalink / raw) To: sstabellini, xen-devel; +Cc: Julien Grall A follow-up patch will introduce another type of foreign mapping. Rename the type to make clear it is only used for read-write mapping. No functional changes intended. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/mm.c | 2 +- xen/arch/arm/p2m.c | 2 +- xen/include/asm-arm/p2m.h | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 7a06a33e21..cec821c3a3 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1283,7 +1283,7 @@ int xenmem_add_to_physmap_one( } mfn = page_to_mfn(page); - t = p2m_map_foreign; + t = p2m_map_foreign_rw; rcu_unlock_domain(od); break; diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 04c8718e9f..b32b16cd94 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -440,7 +440,7 @@ static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a) break; case p2m_iommu_map_rw: - case p2m_map_foreign: + case p2m_map_foreign_rw: case p2m_grant_map_rw: case p2m_mmio_direct_dev: case p2m_mmio_direct_nc: diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index a5914136e3..5f7f31186d 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -117,7 +117,7 @@ enum p2m_type { p2m_mmio_direct_dev,/* Read/write mapping of genuine Device MMIO area */ p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area non-cacheable */ p2m_mmio_direct_c, /* Read/write mapping of genuine MMIO area cacheable */ - p2m_map_foreign, /* Ram pages from foreign domain */ + p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */ p2m_grant_map_rw, /* Read/write grant mapping */ p2m_grant_map_ro, /* Read-only grant mapping */ /* The types below are only used to decide the page attribute in the P2M */ @@ -139,10 +139,10 @@ enum p2m_type { /* Useful predicates */ #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES) -#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign)) +#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign_rw)) #define p2m_is_any_ram(_t) (p2m_to_mask(_t) & \ (P2M_RAM_TYPES | P2M_GRANT_TYPES | \ - p2m_to_mask(p2m_map_foreign))) + p2m_to_mask(p2m_map_foreign_rw))) static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx) -- 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] 44+ messages in thread
* Re: [PATCH 3/8] xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw 2018-11-06 19:14 ` [PATCH 3/8] xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw Julien Grall @ 2018-11-15 11:42 ` Andrii Anisov 2018-11-15 12:07 ` Julien Grall 0 siblings, 1 reply; 44+ messages in thread From: Andrii Anisov @ 2018-11-15 11:42 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini Hello Julien, I'm not sure why do you need this patch to be separated from "[PATCH 4/8] xen/arm: Add support for read-only foreign mappings". But I would not argue for that. Reviewed-by: Andrii Anisov <andrii_anisov@epam.com> Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 3/8] xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw 2018-11-15 11:42 ` Andrii Anisov @ 2018-11-15 12:07 ` Julien Grall 0 siblings, 0 replies; 44+ messages in thread From: Julien Grall @ 2018-11-15 12:07 UTC (permalink / raw) To: Andrii Anisov; +Cc: xen-devel, Stefano Stabellini On 11/15/18 11:42 AM, Andrii Anisov wrote: > Hello Julien, Hi Andrii, > > I'm not sure why do you need this patch to be separated from "[PATCH > 4/8] xen/arm: Add support for read-only foreign mappings". > But I would not argue for that. > Reviewed-by: Andrii Anisov <andrii_anisov@epam.com> We tend to separate renaming/clean-up from new feature. Thank you for the review. 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] 44+ messages in thread
* [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-06 19:14 [PATCH 0/8] xen/arm: Add xentrace support Julien Grall ` (2 preceding siblings ...) 2018-11-06 19:14 ` [PATCH 3/8] xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw Julien Grall @ 2018-11-06 19:14 ` Julien Grall 2018-11-15 11:33 ` Andrii Anisov 2018-11-06 19:14 ` [PATCH 5/8] xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN Julien Grall ` (3 subsequent siblings) 7 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-06 19:14 UTC (permalink / raw) To: sstabellini, xen-devel; +Cc: Julien Grall Current, foreign mappings can only be read-write. A follow-up patch will extend foreign mapping for Xen backend memory (via XEN_DOMID), some of that memory should only be read accessible for the mapping domain. Introduce a new p2m_type to cater read-only foreign mappings. For now, the decision between the two foreign mapping type is based on the type of the guest page mapped. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/mm.c | 2 +- xen/arch/arm/p2m.c | 1 + xen/include/asm-arm/p2m.h | 42 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index cec821c3a3..e31b47a394 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1283,7 +1283,7 @@ int xenmem_add_to_physmap_one( } mfn = page_to_mfn(page); - t = p2m_map_foreign_rw; + t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro; rcu_unlock_domain(od); break; diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index b32b16cd94..0941be9e41 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -450,6 +450,7 @@ static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a) break; case p2m_iommu_map_ro: + case p2m_map_foreign_ro: case p2m_grant_map_ro: case p2m_invalid: e->p2m.xn = 1; diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index 5f7f31186d..7c67806056 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -118,6 +118,7 @@ enum p2m_type { p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area non-cacheable */ p2m_mmio_direct_c, /* Read/write mapping of genuine MMIO area cacheable */ p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */ + p2m_map_foreign_ro, /* Read-only RAM pages from foreign domain */ p2m_grant_map_rw, /* Read/write grant mapping */ p2m_grant_map_ro, /* Read-only grant mapping */ /* The types below are only used to decide the page attribute in the P2M */ @@ -137,12 +138,16 @@ enum p2m_type { #define P2M_GRANT_TYPES (p2m_to_mask(p2m_grant_map_rw) | \ p2m_to_mask(p2m_grant_map_ro)) +/* Foreign mappings types */ +#define P2M_FOREIGN_TYPES (p2m_to_mask(p2m_map_foreign_rw) | \ + p2m_to_mask(p2m_map_foreign_ro)) + /* Useful predicates */ #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES) -#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign_rw)) +#define p2m_is_foreign(_t) (p2m_to_mask(_t) & P2M_FOREIGN_TYPES) #define p2m_is_any_ram(_t) (p2m_to_mask(_t) & \ (P2M_RAM_TYPES | P2M_GRANT_TYPES | \ - p2m_to_mask(p2m_map_foreign_rw))) + P2M_FOREIGN_TYPES)) static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx) @@ -275,7 +280,38 @@ struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn, static inline struct page_info *get_page_from_gfn( struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) { - return p2m_get_page_from_gfn(d, _gfn(gfn), t); + mfn_t mfn; + p2m_type_t _t; + struct page_info *page; + + /* + * Special case for DOMID_XEN as it is the only domain so far that is + * not auto-translated. + */ + if ( unlikely(d != dom_xen) ) + return p2m_get_page_from_gfn(d, _gfn(gfn), t); + + if ( !t ) + t = &_t; + + *t = p2m_invalid; + + /* + * DOMID_XEN see 1-1 RAM. The p2m_type is based on the type of the + * page. + */ + mfn = _mfn(gfn); + page = mfn_to_page(mfn); + + if ( !mfn_valid(mfn) || !get_page(page, d) ) + return NULL; + + if ( page->u.inuse.type_info & PGT_writable_page ) + *t = p2m_ram_rw; + else + *t = p2m_ram_ro; + + return page; } int get_page_type(struct page_info *page, unsigned long type); -- 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] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-06 19:14 ` [PATCH 4/8] xen/arm: Add support for read-only foreign mappings Julien Grall @ 2018-11-15 11:33 ` Andrii Anisov 2018-11-15 11:40 ` Julien Grall 0 siblings, 1 reply; 44+ messages in thread From: Andrii Anisov @ 2018-11-15 11:33 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini Hello Julien, вт, 6 лист. 2018 о 21:16 Julien Grall <julien.grall@arm.com> пише: > @@ -275,7 +280,38 @@ struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn, > static inline struct page_info *get_page_from_gfn( > struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) > { > - return p2m_get_page_from_gfn(d, _gfn(gfn), t); > + mfn_t mfn; > + p2m_type_t _t; Me personally, do not like introducing intermediate `_t` variable. IMO, constructs like following: > + if ( !t ) > + t = &_t; > + > + *t = p2m_invalid; make the code harder to understand than simple checking `t` for nul before assigning it a value. Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 11:33 ` Andrii Anisov @ 2018-11-15 11:40 ` Julien Grall 2018-11-15 12:02 ` Andrii Anisov 0 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-15 11:40 UTC (permalink / raw) To: Andrii Anisov; +Cc: xen-devel, Stefano Stabellini On 11/15/18 11:33 AM, Andrii Anisov wrote: > Hello Julien, Hi, > > вт, 6 лист. 2018 о 21:16 Julien Grall <julien.grall@arm.com> пише: >> @@ -275,7 +280,38 @@ struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn, >> static inline struct page_info *get_page_from_gfn( >> struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) >> { >> - return p2m_get_page_from_gfn(d, _gfn(gfn), t); >> + mfn_t mfn; >> + p2m_type_t _t; > > Me personally, do not like introducing intermediate `_t` variable. > IMO, constructs like following: > >> + if ( !t ) >> + t = &_t; >> + >> + *t = p2m_invalid; > > make the code harder to understand than simple checking `t` for nul > before assigning it a > value. If I drop _t then I need to add if ( *t ) in 3 places in that code. So I don't think the approach is any better. Feel free to suggest a solution that does not require to check 't' everywhere. 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] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 11:40 ` Julien Grall @ 2018-11-15 12:02 ` Andrii Anisov 2018-11-15 12:09 ` Julien Grall 0 siblings, 1 reply; 44+ messages in thread From: Andrii Anisov @ 2018-11-15 12:02 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini чт, 15 лист. 2018 о 13:40 Julien Grall <julien.grall@arm.com> пише: > If I drop _t then I need to add if ( *t ) in 3 places in that code. So I > don't think the approach is any better. Ouch, I kept in my mind two places. Something like: if ( t ) *t = p2m_invalid; .... if ( t ) *t = ( page->u.inuse.type_info & PGT_writable_page ) ? p2m_ram_rw : p2m_ram_ro; ..... Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 12:02 ` Andrii Anisov @ 2018-11-15 12:09 ` Julien Grall 2018-11-15 13:19 ` Andrii Anisov 0 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-15 12:09 UTC (permalink / raw) To: Andrii Anisov; +Cc: xen-devel, Stefano Stabellini On 11/15/18 12:02 PM, Andrii Anisov wrote: > чт, 15 лист. 2018 о 13:40 Julien Grall <julien.grall@arm.com> пише: >> If I drop _t then I need to add if ( *t ) in 3 places in that code. So I >> don't think the approach is any better. > Ouch, I kept in my mind two places. > > Something like: > > if ( t ) > *t = p2m_invalid; > .... > if ( t ) > *t = ( page->u.inuse.type_info & PGT_writable_page ) ? p2m_ram_rw > : p2m_ram_ro; If it was fitting in a single line maybe. In that context, I find it more difficult to read. So I would prefer to stick with _t which is quite common within the p2m code base so far. 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] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 12:09 ` Julien Grall @ 2018-11-15 13:19 ` Andrii Anisov 2018-11-15 15:05 ` Julien Grall 0 siblings, 1 reply; 44+ messages in thread From: Andrii Anisov @ 2018-11-15 13:19 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini > So I would prefer to stick with _t which is quite common within the p2m > code base so far. I've found a similar code only in one place - p2m_get_entry() function. And it is, at least, somehow commented there: ... /* Allow t to be NULL */ t = t ?: &_t; *t = p2m_invalid; ... But IMO, it is really confusing to write a code to calculate and store a value which clearly is not needed by a caller. From another hand, I'm not sure if a compiler would be intelligent enough to factor out the odd code from execution pass on the incoming null pointer. I'm sorry, but I can't pass my RB for `_t`. Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 13:19 ` Andrii Anisov @ 2018-11-15 15:05 ` Julien Grall 2018-11-15 18:44 ` Stefano Stabellini ` (3 more replies) 0 siblings, 4 replies; 44+ messages in thread From: Julien Grall @ 2018-11-15 15:05 UTC (permalink / raw) To: Andrii Anisov; +Cc: xen-devel, Stefano Stabellini Hi, On 11/15/18 1:19 PM, Andrii Anisov wrote: >> So I would prefer to stick with _t which is quite common within the p2m >> code base so far. > > I've found a similar code only in one place - p2m_get_entry() > function. And it is, at least, somehow commented there: > ... > /* Allow t to be NULL */ > t = t ?: &_t; > > *t = p2m_invalid; > ... And in x86 code... > > But IMO, it is really confusing to write a code to calculate and store > a value which clearly is not needed by a caller. I disagree, you directly know that t can be NULL. Although, I agree that a comment would help to understand. > From another hand, I'm not sure if a compiler would be intelligent > enough to factor out the odd code from execution pass on the incoming > null pointer. You can't assume the compiler will inline even with the inline keyword. > > I'm sorry, but I can't pass my RB for `_t`. I think this request is unreasonable because this is a matter of taste. While this is a nice feature to have in Xen 4.12 and address a long standing open issue on Arm. I don't require it and I am not inclined to address bikeshedding. So I will keep aside for now until someone cares about it. 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] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 15:05 ` Julien Grall @ 2018-11-15 18:44 ` Stefano Stabellini 2018-11-15 19:06 ` Julien Grall 2018-11-16 8:37 ` Andrii Anisov ` (2 subsequent siblings) 3 siblings, 1 reply; 44+ messages in thread From: Stefano Stabellini @ 2018-11-15 18:44 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, lars.kurth, Stefano Stabellini, george.dunlap, Andrii Anisov On Thu, 15 Nov 2018, Julien Grall wrote: > Hi, > > On 11/15/18 1:19 PM, Andrii Anisov wrote: > > > So I would prefer to stick with _t which is quite common within the p2m > > > code base so far. > > > > I've found a similar code only in one place - p2m_get_entry() > > function. And it is, at least, somehow commented there: > > ... > > /* Allow t to be NULL */ > > t = t ?: &_t; > > > > *t = p2m_invalid; > > ... > > And in x86 code... > > > > > But IMO, it is really confusing to write a code to calculate and store > > a value which clearly is not needed by a caller. > > I disagree, you directly know that t can be NULL. Although, I agree that a > comment would help to understand. > > > From another hand, I'm not sure if a compiler would be intelligent > > enough to factor out the odd code from execution pass on the incoming > > null pointer. > > You can't assume the compiler will inline even with the inline keyword. > > > > > I'm sorry, but I can't pass my RB for `_t`. > > I think this request is unreasonable because this is a matter of taste. > > While this is a nice feature to have in Xen 4.12 and address a long standing > open issue on Arm. I don't require it and I am not inclined to address > bikeshedding. > > So I will keep aside for now until someone cares about it. Let's try to be reasonable and have constructive conversations. If we do have to disagree, let's disagree on meaningful design decisions, not silly code style issues :-) Andrii, when something can be done equally well in two different ways, especially when it is a matter of style, I think it is best to express our preference, but not to force a contributor in a particular direction, unless specifically stated in CODING_STYLE or equivalent docs. We don't have written rules about code reviews, but if we had, I think this should be one of them. (I would love to have written rules about code reviews.) Julien, usually in situations like this, it is quicker to change the code than to argue. I personally don't care and wouldn't ask you to change it, but if a member of the community reads the code and has an adverse reaction, it is always worth reconsidering the approach, because others might find it antagonizing too. Given the choice, it is best to go for something obvious to most, so if a new contributor sends a patch to change, it is more likely to be correct from the start. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 18:44 ` Stefano Stabellini @ 2018-11-15 19:06 ` Julien Grall 2018-11-15 19:48 ` Stefano Stabellini 0 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-15 19:06 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, lars.kurth, george.dunlap, Andrii Anisov Hi Stefano, On 11/15/18 6:44 PM, Stefano Stabellini wrote: > On Thu, 15 Nov 2018, Julien Grall wrote: >> Hi, >> >> On 11/15/18 1:19 PM, Andrii Anisov wrote: >>>> So I would prefer to stick with _t which is quite common within the p2m >>>> code base so far. >>> >>> I've found a similar code only in one place - p2m_get_entry() >>> function. And it is, at least, somehow commented there: >>> ... >>> /* Allow t to be NULL */ >>> t = t ?: &_t; >>> >>> *t = p2m_invalid; >>> ... >> >> And in x86 code... >> >>> >>> But IMO, it is really confusing to write a code to calculate and store >>> a value which clearly is not needed by a caller. >> >> I disagree, you directly know that t can be NULL. Although, I agree that a >> comment would help to understand. >> >>> From another hand, I'm not sure if a compiler would be intelligent >>> enough to factor out the odd code from execution pass on the incoming >>> null pointer. >> >> You can't assume the compiler will inline even with the inline keyword. >> >>> >>> I'm sorry, but I can't pass my RB for `_t`. >> >> I think this request is unreasonable because this is a matter of taste. >> >> While this is a nice feature to have in Xen 4.12 and address a long standing >> open issue on Arm. I don't require it and I am not inclined to address >> bikeshedding. >> >> So I will keep aside for now until someone cares about it. > > Let's try to be reasonable and have constructive conversations. If we do > have to disagree, let's disagree on meaningful design decisions, not > silly code style issues :-) > > Andrii, when something can be done equally well in two different ways, > especially when it is a matter of style, I think it is best to express > our preference, but not to force a contributor in a particular > direction, unless specifically stated in CODING_STYLE or equivalent > docs. We don't have written rules about code reviews, but if we had, > I think this should be one of them. (I would love to have written rules > about code reviews.) > > Julien, usually in situations like this, it is quicker to change the > code than to argue. I personally don't care and wouldn't ask you to > change it, but if a member of the community reads the code and has an > adverse reaction, it is always worth reconsidering the approach, because > others might find it antagonizing too. Given the choice, it is best to > go for something obvious to most, so if a new contributor sends a patch > to change, it is more likely to be correct from the start. I agree with your point here but this is also valid in the other way. If the suggested alternative provokes an adverse reaction to the contributor, then why should he use it? As I wrote earlier one trying to use ternary condition split over 2 lines is not making the code more obvious. So I am not entirely sure why I should be forced to write such code? 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] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 19:06 ` Julien Grall @ 2018-11-15 19:48 ` Stefano Stabellini 2018-11-15 20:20 ` Julien Grall 0 siblings, 1 reply; 44+ messages in thread From: Stefano Stabellini @ 2018-11-15 19:48 UTC (permalink / raw) To: Julien Grall Cc: xen-devel, lars.kurth, Stefano Stabellini, george.dunlap, Andrii Anisov On Thu, 15 Nov 2018, Julien Grall wrote: > Hi Stefano, > > On 11/15/18 6:44 PM, Stefano Stabellini wrote: > > On Thu, 15 Nov 2018, Julien Grall wrote: > > > Hi, > > > > > > On 11/15/18 1:19 PM, Andrii Anisov wrote: > > > > > So I would prefer to stick with _t which is quite common within the > > > > > p2m > > > > > code base so far. > > > > > > > > I've found a similar code only in one place - p2m_get_entry() > > > > function. And it is, at least, somehow commented there: > > > > ... > > > > /* Allow t to be NULL */ > > > > t = t ?: &_t; > > > > > > > > *t = p2m_invalid; > > > > ... > > > > > > And in x86 code... > > > > > > > > > > > But IMO, it is really confusing to write a code to calculate and store > > > > a value which clearly is not needed by a caller. > > > > > > I disagree, you directly know that t can be NULL. Although, I agree that a > > > comment would help to understand. > > > > > > > From another hand, I'm not sure if a compiler would be intelligent > > > > enough to factor out the odd code from execution pass on the incoming > > > > null pointer. > > > > > > You can't assume the compiler will inline even with the inline keyword. > > > > > > > > > > > I'm sorry, but I can't pass my RB for `_t`. > > > > > > I think this request is unreasonable because this is a matter of taste. > > > > > > While this is a nice feature to have in Xen 4.12 and address a long > > > standing > > > open issue on Arm. I don't require it and I am not inclined to address > > > bikeshedding. > > > > > > So I will keep aside for now until someone cares about it. > > > > Let's try to be reasonable and have constructive conversations. If we do > > have to disagree, let's disagree on meaningful design decisions, not > > silly code style issues :-) > > > > Andrii, when something can be done equally well in two different ways, > > especially when it is a matter of style, I think it is best to express > > our preference, but not to force a contributor in a particular > > direction, unless specifically stated in CODING_STYLE or equivalent > > docs. We don't have written rules about code reviews, but if we had, > > I think this should be one of them. (I would love to have written rules > > about code reviews.) > > > > Julien, usually in situations like this, it is quicker to change the > > code than to argue. I personally don't care and wouldn't ask you to > > change it, but if a member of the community reads the code and has an > > adverse reaction, it is always worth reconsidering the approach, because > > others might find it antagonizing too. Given the choice, it is best to > > go for something obvious to most, so if a new contributor sends a patch > > to change, it is more likely to be correct from the start. > > I agree with your point here but this is also valid in the other way. If the > suggested alternative provokes an adverse reaction to the contributor, then > why should he use it? > > As I wrote earlier one trying to use ternary condition split over 2 lines is > not making the code more obvious. So I am not entirely sure why I should be > forced to write such code? I don't think you should be. You can find another way. For instance, the whole thing could be reduce to one more "if" condition: if ( t != NULL ) { *t = p2m_invalid; if ( page->u.inuse.type_info & PGT_writable_page ) *t = p2m_ram_rw; else *t = p2m_ram_ro; } be creative :-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 19:48 ` Stefano Stabellini @ 2018-11-15 20:20 ` Julien Grall 2018-11-16 9:05 ` Andrii Anisov 0 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-15 20:20 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, lars.kurth, george.dunlap, Andrii Anisov Hi, On 11/15/18 7:48 PM, Stefano Stabellini wrote: > On Thu, 15 Nov 2018, Julien Grall wrote: >> Hi Stefano, >> >> On 11/15/18 6:44 PM, Stefano Stabellini wrote: >>> On Thu, 15 Nov 2018, Julien Grall wrote: >>>> Hi, >>>> >>>> On 11/15/18 1:19 PM, Andrii Anisov wrote: >>>>>> So I would prefer to stick with _t which is quite common within the >>>>>> p2m >>>>>> code base so far. >>>>> >>>>> I've found a similar code only in one place - p2m_get_entry() >>>>> function. And it is, at least, somehow commented there: >>>>> ... >>>>> /* Allow t to be NULL */ >>>>> t = t ?: &_t; >>>>> >>>>> *t = p2m_invalid; >>>>> ... >>>> >>>> And in x86 code... >>>> >>>>> >>>>> But IMO, it is really confusing to write a code to calculate and store >>>>> a value which clearly is not needed by a caller. >>>> >>>> I disagree, you directly know that t can be NULL. Although, I agree that a >>>> comment would help to understand. >>>> >>>>> From another hand, I'm not sure if a compiler would be intelligent >>>>> enough to factor out the odd code from execution pass on the incoming >>>>> null pointer. >>>> >>>> You can't assume the compiler will inline even with the inline keyword. >>>> >>>>> >>>>> I'm sorry, but I can't pass my RB for `_t`. >>>> >>>> I think this request is unreasonable because this is a matter of taste. >>>> >>>> While this is a nice feature to have in Xen 4.12 and address a long >>>> standing >>>> open issue on Arm. I don't require it and I am not inclined to address >>>> bikeshedding. >>>> >>>> So I will keep aside for now until someone cares about it. >>> >>> Let's try to be reasonable and have constructive conversations. If we do >>> have to disagree, let's disagree on meaningful design decisions, not >>> silly code style issues :-) >>> >>> Andrii, when something can be done equally well in two different ways, >>> especially when it is a matter of style, I think it is best to express >>> our preference, but not to force a contributor in a particular >>> direction, unless specifically stated in CODING_STYLE or equivalent >>> docs. We don't have written rules about code reviews, but if we had, >>> I think this should be one of them. (I would love to have written rulesAs >>> about code reviews.) >>> >>> Julien, usually in situations like this, it is quicker to change the >>> code than to argue. I personally don't care and wouldn't ask you to >>> change it, but if a member of the community reads the code and has an >>> adverse reaction, it is always worth reconsidering the approach, because >>> others might find it antagonizing too. Given the choice, it is best to >>> go for something obvious to most, so if a new contributor sends a patch >>> to change, it is more likely to be correct from the start. >> >> I agree with your point here but this is also valid in the other way. If the >> suggested alternative provokes an adverse reaction to the contributor, then >> why should he use it? >> >> As I wrote earlier one trying to use ternary condition split over 2 lines is >> not making the code more obvious. So I am not entirely sure why I should be >> forced to write such code? > > I don't think you should be. You can find another way. For instance, the > whole thing could be reduce to one more "if" condition: > > if ( t != NULL ) > { > *t = p2m_invalid; > if ( page->u.inuse.type_info & PGT_writable_page ) > *t = p2m_ram_rw; > else > *t = p2m_ram_ro; > } > > be creative :-) Well the code suggested is different from what I intended :). t should be set to invalid before the check mfn_valid/get_page. So this needs at least 2 checks. 2 places were t != NULL needs to be explained. As you said this is a matter of taste. If someone disagree on the coding style, then he should suggest an alternative way fitting the requirement. 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] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 20:20 ` Julien Grall @ 2018-11-16 9:05 ` Andrii Anisov 0 siblings, 0 replies; 44+ messages in thread From: Andrii Anisov @ 2018-11-16 9:05 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, George Dunlap, lars.kurth Hello Julien, чт, 15 лист. 2018 о 22:20 Julien Grall <julien.grall@arm.com> пише: > Well the code suggested is different from what I intended :). t should > be set to invalid before the check mfn_valid/get_page. So this needs at > least 2 checks. 2 places were t != NULL needs to be explained. Well, I guess checking a pointer in order to avoid null pointer dereference is pretty self-explaining. Even if it is done twice. And the code itself would give a clear idea that we do calculate (and assign) type value only in case a caller provided us a valid pointer. For sure, I do not insist on a ternary operator usage. Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 15:05 ` Julien Grall 2018-11-15 18:44 ` Stefano Stabellini @ 2018-11-16 8:37 ` Andrii Anisov 2018-11-20 15:27 ` Andrii Anisov 2018-12-20 11:21 ` Andrii Anisov 3 siblings, 0 replies; 44+ messages in thread From: Andrii Anisov @ 2018-11-16 8:37 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini чт, 15 лист. 2018 о 17:05 Julien Grall <julien.grall@arm.com> пише: > And in x86 code... Sorry, did not check. > You can't assume the compiler will inline even with the inline keyword. For sure! > > I'm sorry, but I can't pass my RB for `_t`. > > I think this request is unreasonable because this is a matter of taste. I would not say it is a request. I'm just saying that I personally do not like that code, it confuses me, so I can't pass RB for it. Sorry for that :( I really hope someone else can come up with RB for it. Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 15:05 ` Julien Grall 2018-11-15 18:44 ` Stefano Stabellini 2018-11-16 8:37 ` Andrii Anisov @ 2018-11-20 15:27 ` Andrii Anisov 2018-12-20 11:21 ` Andrii Anisov 3 siblings, 0 replies; 44+ messages in thread From: Andrii Anisov @ 2018-11-20 15:27 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini Hello Julien, It is me again. On 15.11.18 17:05, Julien Grall wrote: > On 11/15/18 1:19 PM, Andrii Anisov wrote: >>> So I would prefer to stick with _t which is quite common within the p2m >>> code base so far. >> >> I've found a similar code only in one place - p2m_get_entry() >> function. And it is, at least, somehow commented there: >> ... >> /* Allow t to be NULL */ >> t = t ?: &_t; >> >> *t = p2m_invalid; >> ... > > And in x86 code... I've taken care to look into the x86 p2m code about that construction. I've found it in x86's `p2m_get_page_from_gfn` function. And it looks reasonable over there. Because that value is not only might be returned to a caller. But is needed by the function code itself to take decisions and passing it to called functions (which do not do a check for the null pointer). Your code (as well as `p2m_get_entry`) does not actually use that value. So I see no need to calculate or keep it if it is not needed by a caller. -- Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 4/8] xen/arm: Add support for read-only foreign mappings 2018-11-15 15:05 ` Julien Grall ` (2 preceding siblings ...) 2018-11-20 15:27 ` Andrii Anisov @ 2018-12-20 11:21 ` Andrii Anisov 3 siblings, 0 replies; 44+ messages in thread From: Andrii Anisov @ 2018-12-20 11:21 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini Reviewed-by: Andrii Anisov <andrii_anisov@epam.com> -- Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 5/8] xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN 2018-11-06 19:14 [PATCH 0/8] xen/arm: Add xentrace support Julien Grall ` (3 preceding siblings ...) 2018-11-06 19:14 ` [PATCH 4/8] xen/arm: Add support for read-only foreign mappings Julien Grall @ 2018-11-06 19:14 ` Julien Grall 2018-11-15 13:45 ` Andrii Anisov 2018-11-06 19:14 ` [PATCH 6/8] xen/arm: Initialize trace buffer Julien Grall ` (2 subsequent siblings) 7 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-06 19:14 UTC (permalink / raw) To: sstabellini, xen-devel; +Cc: Julien Grall For auto-translated domain, the only way to map page to itself is the using foreign map API. The current code does not allow mapping page from special page (such as DOMID_XEN). As xentrace buffer are shared using DOMID_XEN, it is not possible to use tracing for Arm. This could be solved by using the helper get_pg_owner(). This helper will be able to get a reference on DOMID_XEN and therefore allow mapping for privileged domain. This patch replace the call to rcu_lock_domain_by_any_id() with get_pg_owner(). For consistency, all the call to rcu_unlock_domain are replaced by put_pg_owner(). Signed-off-by: Julien grall <julien.grall@arm.com> --- xen/arch/arm/mm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index e31b47a394..72d0285768 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1249,20 +1249,20 @@ int xenmem_add_to_physmap_one( struct domain *od; p2m_type_t p2mt; - od = rcu_lock_domain_by_any_id(extra.foreign_domid); + od = get_pg_owner(extra.foreign_domid); if ( od == NULL ) return -ESRCH; if ( od == d ) { - rcu_unlock_domain(od); + put_pg_owner(od); return -EINVAL; } rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od); if ( rc ) { - rcu_unlock_domain(od); + put_pg_owner(od); return rc; } @@ -1271,21 +1271,21 @@ int xenmem_add_to_physmap_one( page = get_page_from_gfn(od, idx, &p2mt, P2M_ALLOC); if ( !page ) { - rcu_unlock_domain(od); + put_pg_owner(od); return -EINVAL; } if ( !p2m_is_ram(p2mt) ) { put_page(page); - rcu_unlock_domain(od); + put_pg_owner(od); return -EINVAL; } mfn = page_to_mfn(page); t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro; - rcu_unlock_domain(od); + put_pg_owner(od); break; } case XENMAPSPACE_dev_mmio: -- 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] 44+ messages in thread
* Re: [PATCH 5/8] xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN 2018-11-06 19:14 ` [PATCH 5/8] xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN Julien Grall @ 2018-11-15 13:45 ` Andrii Anisov 0 siblings, 0 replies; 44+ messages in thread From: Andrii Anisov @ 2018-11-15 13:45 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini Reviewed-by: Andrii Anisov <andrii_anisov@epam.com> Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 6/8] xen/arm: Initialize trace buffer 2018-11-06 19:14 [PATCH 0/8] xen/arm: Add xentrace support Julien Grall ` (4 preceding siblings ...) 2018-11-06 19:14 ` [PATCH 5/8] xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN Julien Grall @ 2018-11-06 19:14 ` Julien Grall 2018-11-15 13:49 ` Andrii Anisov 2018-11-06 19:14 ` [PATCH 7/8] xenalyze: Build for Both ARM and x86 Platforms Julien Grall 2018-11-06 19:14 ` [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn Julien Grall 7 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-06 19:14 UTC (permalink / raw) To: sstabellini, xen-devel; +Cc: Julien Grall, Benjamin Sanda From: Benjamin Sanda <ben.sanda@dornerworks.com> Now that we allow a privileged domain to map tracing buffer, initialize them so a user can effectively trace Xen. Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com> [julien: rework commit message] Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/setup.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 80f00286d3..7022904cc3 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -36,6 +36,7 @@ #include <xen/pfn.h> #include <xen/virtual_region.h> #include <xen/vmap.h> +#include <xen/trace.h> #include <xen/libfdt/libfdt.h> #include <xen/acpi.h> #include <asm/alternative.h> @@ -863,6 +864,8 @@ void __init start_xen(unsigned long boot_phys_offset, heap_init_late(); + init_trace_bufs(); + init_constructors(); console_endboot(); -- 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] 44+ messages in thread
* Re: [PATCH 6/8] xen/arm: Initialize trace buffer 2018-11-06 19:14 ` [PATCH 6/8] xen/arm: Initialize trace buffer Julien Grall @ 2018-11-15 13:49 ` Andrii Anisov 0 siblings, 0 replies; 44+ messages in thread From: Andrii Anisov @ 2018-11-15 13:49 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, ben.sanda Reviewed-by: Andrii Anisov <andrii_anisov@epam.com> Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 7/8] xenalyze: Build for Both ARM and x86 Platforms 2018-11-06 19:14 [PATCH 0/8] xen/arm: Add xentrace support Julien Grall ` (5 preceding siblings ...) 2018-11-06 19:14 ` [PATCH 6/8] xen/arm: Initialize trace buffer Julien Grall @ 2018-11-06 19:14 ` Julien Grall 2018-11-07 13:04 ` Wei Liu 2018-11-06 19:14 ` [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn Julien Grall 7 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-06 19:14 UTC (permalink / raw) To: sstabellini, xen-devel Cc: George Dunlap, Wei Liu, Julien Grall, Ian Jackson, Benjamin Sanda From: Benjamin Sanda <ben.sanda@dornerworks.com> Modified to provide building of the xenalyze binary for both ARM and x86 platforms. The xenalyze binary is now built as part of the BIN list for both platforms. Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com> Signed-off-by: Julien Grall <julien.grall@arm.com> --- tools/xentrace/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/xentrace/Makefile b/tools/xentrace/Makefile index 0bad942bdf..9fb7fc96e7 100644 --- a/tools/xentrace/Makefile +++ b/tools/xentrace/Makefile @@ -9,8 +9,7 @@ LDLIBS += $(LDLIBS_libxenevtchn) LDLIBS += $(LDLIBS_libxenctrl) LDLIBS += $(ARGP_LDFLAGS) -BIN-$(CONFIG_X86) = xenalyze -BIN = $(BIN-y) +BIN = xenalyze SBIN = xentrace xentrace_setsize LIBBIN = xenctx SCRIPTS = xentrace_format -- 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] 44+ messages in thread
* Re: [PATCH 7/8] xenalyze: Build for Both ARM and x86 Platforms 2018-11-06 19:14 ` [PATCH 7/8] xenalyze: Build for Both ARM and x86 Platforms Julien Grall @ 2018-11-07 13:04 ` Wei Liu 0 siblings, 0 replies; 44+ messages in thread From: Wei Liu @ 2018-11-07 13:04 UTC (permalink / raw) To: Julien Grall Cc: sstabellini, Wei Liu, Benjamin Sanda, George Dunlap, Ian Jackson, xen-devel On Tue, Nov 06, 2018 at 07:14:53PM +0000, Julien Grall wrote: > From: Benjamin Sanda <ben.sanda@dornerworks.com> > > Modified to provide building of the xenalyze binary for both ARM and > x86 platforms. The xenalyze binary is now built as part of the BIN > list for both platforms. > > Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com> > Signed-off-by: Julien Grall <julien.grall@arm.com> Acked-by: Wei Liu <wei.liu2@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-06 19:14 [PATCH 0/8] xen/arm: Add xentrace support Julien Grall ` (6 preceding siblings ...) 2018-11-06 19:14 ` [PATCH 7/8] xenalyze: Build for Both ARM and x86 Platforms Julien Grall @ 2018-11-06 19:14 ` Julien Grall 2018-11-06 20:11 ` Andrew Cooper ` (2 more replies) 7 siblings, 3 replies; 44+ messages in thread From: Julien Grall @ 2018-11-06 19:14 UTC (permalink / raw) To: sstabellini, xen-devel Cc: Jun Nakajima, Kevin Tian, Wei Liu, Suravee Suthikulpanit, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, Julien Grall, Paul Durrant, Jan Beulich, Boris Ostrovsky, Brian Woods No functional change intended. Only reasonable clean-ups are done in this patch. The rest will use _gfn for the time being. Signed-off-by: Julien Grall <julie.grall@arm.com> --- xen/arch/arm/guestcopy.c | 2 +- xen/arch/arm/mm.c | 2 +- xen/arch/x86/cpu/vpmu.c | 2 +- xen/arch/x86/domain.c | 12 ++++++------ xen/arch/x86/domctl.c | 6 +++--- xen/arch/x86/hvm/dm.c | 2 +- xen/arch/x86/hvm/domain.c | 2 +- xen/arch/x86/hvm/hvm.c | 9 +++++---- xen/arch/x86/hvm/svm/svm.c | 8 ++++---- xen/arch/x86/hvm/viridian/viridian.c | 24 ++++++++++++------------ xen/arch/x86/hvm/vmx/vmx.c | 4 ++-- xen/arch/x86/hvm/vmx/vvmx.c | 12 ++++++------ xen/arch/x86/mm.c | 24 ++++++++++++++---------- xen/arch/x86/mm/p2m.c | 2 +- xen/arch/x86/mm/shadow/hvm.c | 6 +++--- xen/arch/x86/physdev.c | 3 ++- xen/arch/x86/pv/descriptor-tables.c | 5 ++--- xen/arch/x86/pv/emul-priv-op.c | 6 +++--- xen/arch/x86/pv/mm.c | 2 +- xen/arch/x86/traps.c | 11 ++++++----- xen/common/domain.c | 2 +- xen/common/event_fifo.c | 12 ++++++------ xen/common/memory.c | 4 ++-- xen/common/tmem_xen.c | 2 +- xen/include/asm-arm/p2m.h | 6 +++--- xen/include/asm-x86/p2m.h | 11 +++++++---- 26 files changed, 95 insertions(+), 86 deletions(-) diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c index 7a0f3e9d5f..55892062bb 100644 --- a/xen/arch/arm/guestcopy.c +++ b/xen/arch/arm/guestcopy.c @@ -37,7 +37,7 @@ static struct page_info *translate_get_page(copy_info_t info, uint64_t addr, return get_page_from_gva(info.gva.v, addr, write ? GV2M_WRITE : GV2M_READ); - page = get_page_from_gfn(info.gpa.d, paddr_to_pfn(addr), &p2mt, P2M_ALLOC); + page = get_page_from_gfn(info.gpa.d, gaddr_to_gfn(addr), &p2mt, P2M_ALLOC); if ( !page ) return NULL; diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 72d0285768..88711096ef 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1268,7 +1268,7 @@ int xenmem_add_to_physmap_one( /* Take reference to the foreign domain page. * Reference will be released in XENMEM_remove_from_physmap */ - page = get_page_from_gfn(od, idx, &p2mt, P2M_ALLOC); + page = get_page_from_gfn(od, _gfn(idx), &p2mt, P2M_ALLOC); if ( !page ) { put_pg_owner(od); diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c index 8a4f753eae..4d8f153031 100644 --- a/xen/arch/x86/cpu/vpmu.c +++ b/xen/arch/x86/cpu/vpmu.c @@ -607,7 +607,7 @@ static int pvpmu_init(struct domain *d, xen_pmu_params_t *params) struct vcpu *v; struct vpmu_struct *vpmu; struct page_info *page; - uint64_t gfn = params->val; + gfn_t gfn = _gfn(params->val); if ( (params->vcpu >= d->max_vcpus) || (d->vcpu[params->vcpu] == NULL) ) return -EINVAL; diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index f6fe954313..c5cce4b38d 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -797,7 +797,7 @@ int arch_set_info_guest( unsigned long flags; bool compat; #ifdef CONFIG_PV - unsigned long cr3_gfn; + gfn_t cr3_gfn; struct page_info *cr3_page; unsigned long cr4; int rc = 0; @@ -1061,9 +1061,9 @@ int arch_set_info_guest( set_bit(_VPF_in_reset, &v->pause_flags); if ( !compat ) - cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[3]); + cr3_gfn = _gfn(xen_cr3_to_pfn(c.nat->ctrlreg[3])); else - cr3_gfn = compat_cr3_to_pfn(c.cmp->ctrlreg[3]); + cr3_gfn = _gfn(compat_cr3_to_pfn(c.cmp->ctrlreg[3])); cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC); if ( !cr3_page ) @@ -1092,7 +1092,7 @@ int arch_set_info_guest( case 0: if ( !compat && !VM_ASSIST(d, m2p_strict) && !paging_mode_refcounts(d) ) - fill_ro_mpt(_mfn(cr3_gfn)); + fill_ro_mpt(_mfn(gfn_x(cr3_gfn))); break; default: if ( cr3_page == current->arch.old_guest_table ) @@ -1107,7 +1107,7 @@ int arch_set_info_guest( v->arch.guest_table = pagetable_from_page(cr3_page); if ( c.nat->ctrlreg[1] ) { - cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[1]); + cr3_gfn = _gfn(xen_cr3_to_pfn(c.nat->ctrlreg[1])); cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC); if ( !cr3_page ) @@ -1132,7 +1132,7 @@ int arch_set_info_guest( break; case 0: if ( VM_ASSIST(d, m2p_strict) ) - zap_ro_mpt(_mfn(cr3_gfn)); + zap_ro_mpt(_mfn(gfn_x(cr3_gfn))); break; } } diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 33f9a869c0..6b0d8075cd 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -448,7 +448,7 @@ long arch_do_domctl( break; } - page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC); + page = get_page_from_gfn(d, _gfn(gfn), &t, P2M_ALLOC); if ( unlikely(!page) || unlikely(is_xen_heap_page(page)) ) @@ -498,11 +498,11 @@ long arch_do_domctl( case XEN_DOMCTL_hypercall_init: { - unsigned long gmfn = domctl->u.hypercall_init.gmfn; + gfn_t gfn = _gfn(domctl->u.hypercall_init.gmfn); struct page_info *page; void *hypercall_page; - page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); + page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); if ( !page || !get_page_type(page, PGT_writable_page) ) { diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c index d6d0e8be89..3b3ad27938 100644 --- a/xen/arch/x86/hvm/dm.c +++ b/xen/arch/x86/hvm/dm.c @@ -186,7 +186,7 @@ static int modified_memory(struct domain *d, { struct page_info *page; - page = get_page_from_gfn(d, pfn, NULL, P2M_UNSHARE); + page = get_page_from_gfn(d, _gfn(pfn), NULL, P2M_UNSHARE); if ( page ) { paging_mark_pfn_dirty(d, _pfn(pfn)); diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c index 5d5a746a25..73d2da8441 100644 --- a/xen/arch/x86/hvm/domain.c +++ b/xen/arch/x86/hvm/domain.c @@ -297,7 +297,7 @@ int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx) { /* Shadow-mode CR3 change. Check PDBR and update refcounts. */ struct page_info *page = get_page_from_gfn(v->domain, - v->arch.hvm.guest_cr[3] >> PAGE_SHIFT, + gaddr_to_gfn(v->arch.hvm.guest_cr[3]), NULL, P2M_ALLOC); if ( !page ) { diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 7be9cf4454..be262e5a1d 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -2146,7 +2146,7 @@ int hvm_set_cr0(unsigned long value, bool may_defer) { struct vcpu *v = current; struct domain *d = v->domain; - unsigned long gfn, old_value = v->arch.hvm.guest_cr[0]; + unsigned long old_value = v->arch.hvm.guest_cr[0]; struct page_info *page; HVM_DBG_LOG(DBG_LEVEL_VMMU, "Update CR0 value = %lx", value); @@ -2201,7 +2201,8 @@ int hvm_set_cr0(unsigned long value, bool may_defer) if ( !paging_mode_hap(d) ) { /* The guest CR3 must be pointing to the guest physical. */ - gfn = v->arch.hvm.guest_cr[3] >> PAGE_SHIFT; + gfn_t gfn = gaddr_to_gfn(v->arch.hvm.guest_cr[3]); + page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); if ( !page ) { @@ -2293,7 +2294,7 @@ int hvm_set_cr3(unsigned long value, bool may_defer) { /* Shadow-mode CR3 change. Check PDBR and update refcounts. */ HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value); - page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT, + page = get_page_from_gfn(v->domain, gaddr_to_gfn(value), NULL, P2M_ALLOC); if ( !page ) goto bad_cr3; @@ -3120,7 +3121,7 @@ enum hvm_translation_result hvm_translate_get_page( && hvm_mmio_internal(gfn_to_gaddr(gfn)) ) return HVMTRANS_bad_gfn_to_mfn; - page = get_page_from_gfn(v->domain, gfn_x(gfn), &p2mt, P2M_UNSHARE); + page = get_page_from_gfn(v->domain, gfn, &p2mt, P2M_UNSHARE); if ( !page ) return HVMTRANS_bad_gfn_to_mfn; diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 5d00256aaa..a7419bd444 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -317,7 +317,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct hvm_hw_cpu *c) { if ( c->cr0 & X86_CR0_PG ) { - page = get_page_from_gfn(v->domain, c->cr3 >> PAGE_SHIFT, + page = get_page_from_gfn(v->domain, gaddr_to_gfn(c->cr3), NULL, P2M_ALLOC); if ( !page ) { @@ -2412,9 +2412,9 @@ nsvm_get_nvmcb_page(struct vcpu *v, uint64_t vmcbaddr) return NULL; /* Need to translate L1-GPA to MPA */ - page = get_page_from_gfn(v->domain, - nv->nv_vvmcxaddr >> PAGE_SHIFT, - &p2mt, P2M_ALLOC | P2M_UNSHARE); + page = get_page_from_gfn(v->domain, + gaddr_to_gfn(nv->nv_vvmcxaddr >> PAGE_SHIFT), + &p2mt, P2M_ALLOC | P2M_UNSHARE); if ( !page ) return NULL; diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c index 2dc86dd0f3..1d3be156db 100644 --- a/xen/arch/x86/hvm/viridian/viridian.c +++ b/xen/arch/x86/hvm/viridian/viridian.c @@ -332,16 +332,16 @@ static void dump_reference_tsc(const struct domain *d) static void enable_hypercall_page(struct domain *d) { - unsigned long gmfn = d->arch.hvm.viridian.hypercall_gpa.fields.pfn; - struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); + gfn_t gfn = _gfn(d->arch.hvm.viridian.hypercall_gpa.fields.pfn); + struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); uint8_t *p; if ( !page || !get_page_type(page, PGT_writable_page) ) { if ( page ) put_page(page); - gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); + gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); return; } @@ -367,8 +367,8 @@ static void enable_hypercall_page(struct domain *d) static void initialize_vp_assist(struct vcpu *v) { struct domain *d = v->domain; - unsigned long gmfn = v->arch.hvm.viridian.vp_assist.msr.fields.pfn; - struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); + gfn_t gfn = _gfn(v->arch.hvm.viridian.vp_assist.msr.fields.pfn); + struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); void *va; ASSERT(!v->arch.hvm.viridian.vp_assist.va); @@ -395,8 +395,8 @@ static void initialize_vp_assist(struct vcpu *v) return; fail: - gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); + gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); } static void teardown_vp_assist(struct vcpu *v) @@ -465,16 +465,16 @@ void viridian_apic_assist_clear(struct vcpu *v) static void update_reference_tsc(struct domain *d, bool_t initialize) { - unsigned long gmfn = d->arch.hvm.viridian.reference_tsc.fields.pfn; - struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); + gfn_t gfn = _gfn(d->arch.hvm.viridian.reference_tsc.fields.pfn); + struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); HV_REFERENCE_TSC_PAGE *p; if ( !page || !get_page_type(page, PGT_writable_page) ) { if ( page ) put_page(page); - gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); + gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); return; } diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index e065f8bbdb..2070e78358 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -674,7 +674,7 @@ static int vmx_restore_cr0_cr3( { if ( cr0 & X86_CR0_PG ) { - page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT, + page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3), NULL, P2M_ALLOC); if ( !page ) { @@ -1373,7 +1373,7 @@ static void vmx_load_pdptrs(struct vcpu *v) if ( (cr3 & 0x1fUL) && !hvm_pcid_enabled(v) ) goto crash; - page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt, P2M_UNSHARE); + page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3), &p2mt, P2M_UNSHARE); if ( !page ) { /* Ideally you don't want to crash but rather go into a wait diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index dfd08e2d0a..2953d05a17 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -649,11 +649,11 @@ static void nvmx_update_apic_access_address(struct vcpu *v) if ( ctrl & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES ) { p2m_type_t p2mt; - unsigned long apic_gpfn; + gfn_t apic_gfn; struct page_info *apic_pg; - apic_gpfn = get_vvmcs(v, APIC_ACCESS_ADDR) >> PAGE_SHIFT; - apic_pg = get_page_from_gfn(v->domain, apic_gpfn, &p2mt, P2M_ALLOC); + apic_gfn = gaddr_to_gfn(get_vvmcs(v, APIC_ACCESS_ADDR)); + apic_pg = get_page_from_gfn(v->domain, apic_gfn, &p2mt, P2M_ALLOC); ASSERT(apic_pg && !p2m_is_paging(p2mt)); __vmwrite(APIC_ACCESS_ADDR, page_to_maddr(apic_pg)); put_page(apic_pg); @@ -670,11 +670,11 @@ static void nvmx_update_virtual_apic_address(struct vcpu *v) if ( ctrl & CPU_BASED_TPR_SHADOW ) { p2m_type_t p2mt; - unsigned long vapic_gpfn; + gfn_t vapic_gfn; struct page_info *vapic_pg; - vapic_gpfn = get_vvmcs(v, VIRTUAL_APIC_PAGE_ADDR) >> PAGE_SHIFT; - vapic_pg = get_page_from_gfn(v->domain, vapic_gpfn, &p2mt, P2M_ALLOC); + vapic_gfn = gaddr_to_gfn(get_vvmcs(v, VIRTUAL_APIC_PAGE_ADDR)); + vapic_pg = get_page_from_gfn(v->domain, vapic_gfn, &p2mt, P2M_ALLOC); ASSERT(vapic_pg && !p2m_is_paging(p2mt)); __vmwrite(VIRTUAL_APIC_PAGE_ADDR, page_to_maddr(vapic_pg)); put_page(vapic_pg); diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 9363e9bd96..e3462f8a77 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -2052,7 +2052,7 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, l1_pgentry_t nl1e, p2m_query_t q = l1e_get_flags(nl1e) & _PAGE_RW ? P2M_ALLOC | P2M_UNSHARE : P2M_ALLOC; - page = get_page_from_gfn(pg_dom, l1e_get_pfn(nl1e), &p2mt, q); + page = get_page_from_gfn(pg_dom, _gfn(l1e_get_pfn(nl1e)), &p2mt, q); if ( p2m_is_paged(p2mt) ) { @@ -3223,7 +3223,8 @@ long do_mmuext_op( if ( paging_mode_refcounts(pg_owner) ) break; - page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC); + page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), NULL, + P2M_ALLOC); if ( unlikely(!page) ) { rc = -EINVAL; @@ -3288,7 +3289,8 @@ long do_mmuext_op( if ( paging_mode_refcounts(pg_owner) ) break; - page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC); + page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), NULL, + P2M_ALLOC); if ( unlikely(!page) ) { gdprintk(XENLOG_WARNING, @@ -3504,7 +3506,8 @@ long do_mmuext_op( } case MMUEXT_CLEAR_PAGE: - page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, P2M_ALLOC); + page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), &p2mt, + P2M_ALLOC); if ( unlikely(p2mt != p2m_ram_rw) && page ) { put_page(page); @@ -3532,7 +3535,7 @@ long do_mmuext_op( { struct page_info *src_page, *dst_page; - src_page = get_page_from_gfn(pg_owner, op.arg2.src_mfn, &p2mt, + src_page = get_page_from_gfn(pg_owner, _gfn(op.arg2.src_mfn), &p2mt, P2M_ALLOC); if ( unlikely(p2mt != p2m_ram_rw) && src_page ) { @@ -3548,7 +3551,7 @@ long do_mmuext_op( break; } - dst_page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, + dst_page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), &p2mt, P2M_ALLOC); if ( unlikely(p2mt != p2m_ram_rw) && dst_page ) { @@ -3636,7 +3639,8 @@ long do_mmu_update( { struct mmu_update req; void *va = NULL; - unsigned long gpfn, gmfn, mfn; + unsigned long gpfn, mfn; + gfn_t gfn; struct page_info *page; unsigned int cmd, i = 0, done = 0, pt_dom; struct vcpu *curr = current, *v = curr; @@ -3749,8 +3753,8 @@ long do_mmu_update( rc = -EINVAL; req.ptr -= cmd; - gmfn = req.ptr >> PAGE_SHIFT; - page = get_page_from_gfn(pt_owner, gmfn, &p2mt, P2M_ALLOC); + gfn = gaddr_to_gfn(req.ptr); + page = get_page_from_gfn(pt_owner, gfn, &p2mt, P2M_ALLOC); if ( unlikely(!page) || p2mt != p2m_ram_rw ) { @@ -3758,7 +3762,7 @@ long do_mmu_update( put_page(page); if ( p2m_is_paged(p2mt) ) { - p2m_mem_paging_populate(pt_owner, gmfn); + p2m_mem_paging_populate(pt_owner, gfn_x(gfn)); rc = -ENOENT; } else diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index a00a3c1bff..3b2aac8804 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -2718,7 +2718,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn, * Take a refcnt on the mfn. NB: following supported for foreign mapping: * ram_rw | ram_logdirty | ram_ro | paging_out. */ - page = get_page_from_gfn(fdom, fgfn, &p2mt, P2M_ALLOC); + page = get_page_from_gfn(fdom, _gfn(fgfn), &p2mt, P2M_ALLOC); if ( !page || !p2m_is_ram(p2mt) || p2m_is_shared(p2mt) || p2m_is_hole(p2mt) ) { diff --git a/xen/arch/x86/mm/shadow/hvm.c b/xen/arch/x86/mm/shadow/hvm.c index 4cc75916b8..9275ba476c 100644 --- a/xen/arch/x86/mm/shadow/hvm.c +++ b/xen/arch/x86/mm/shadow/hvm.c @@ -313,15 +313,15 @@ const struct x86_emulate_ops hvm_shadow_emulator_ops = { static mfn_t emulate_gva_to_mfn(struct vcpu *v, unsigned long vaddr, struct sh_emulate_ctxt *sh_ctxt) { - unsigned long gfn; + gfn_t gfn; struct page_info *page; mfn_t mfn; p2m_type_t p2mt; uint32_t pfec = PFEC_page_present | PFEC_write_access; /* Translate the VA to a GFN. */ - gfn = paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, &pfec); - if ( gfn == gfn_x(INVALID_GFN) ) + gfn = _gfn(paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, &pfec)); + if ( gfn_eq(gfn, INVALID_GFN) ) { x86_emul_pagefault(pfec, vaddr, &sh_ctxt->ctxt); diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c index 3a3c15890b..4f3f438614 100644 --- a/xen/arch/x86/physdev.c +++ b/xen/arch/x86/physdev.c @@ -229,7 +229,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) break; ret = -EINVAL; - page = get_page_from_gfn(current->domain, info.gmfn, NULL, P2M_ALLOC); + page = get_page_from_gfn(current->domain, _gfn(info.gmfn), + NULL, P2M_ALLOC); if ( !page ) break; if ( !get_page_type(page, PGT_writable_page) ) diff --git a/xen/arch/x86/pv/descriptor-tables.c b/xen/arch/x86/pv/descriptor-tables.c index 8b2d55fc2e..7e8f41d3fd 100644 --- a/xen/arch/x86/pv/descriptor-tables.c +++ b/xen/arch/x86/pv/descriptor-tables.c @@ -112,7 +112,7 @@ long pv_set_gdt(struct vcpu *v, unsigned long *frames, unsigned int entries) { struct page_info *page; - page = get_page_from_gfn(d, frames[i], NULL, P2M_ALLOC); + page = get_page_from_gfn(d, _gfn(frames[i]), NULL, P2M_ALLOC); if ( !page ) goto fail; if ( !get_page_type(page, PGT_seg_desc_page) ) @@ -209,7 +209,6 @@ int compat_set_gdt(XEN_GUEST_HANDLE_PARAM(uint) frame_list, long do_update_descriptor(uint64_t pa, uint64_t desc) { struct domain *currd = current->domain; - unsigned long gmfn = pa >> PAGE_SHIFT; unsigned long mfn; unsigned int offset; struct desc_struct *gdt_pent, d; @@ -220,7 +219,7 @@ long do_update_descriptor(uint64_t pa, uint64_t desc) *(uint64_t *)&d = desc; - page = get_page_from_gfn(currd, gmfn, NULL, P2M_ALLOC); + page = get_page_from_gfn(currd, gaddr_to_gfn(pa), NULL, P2M_ALLOC); if ( (((unsigned int)pa % sizeof(struct desc_struct)) != 0) || !page || !check_descriptor(currd, &d) ) diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c index f73ea4a163..a529ebcc3f 100644 --- a/xen/arch/x86/pv/emul-priv-op.c +++ b/xen/arch/x86/pv/emul-priv-op.c @@ -760,12 +760,12 @@ static int write_cr(unsigned int reg, unsigned long val, case 3: /* Write CR3 */ { struct domain *currd = curr->domain; - unsigned long gfn; + gfn_t gfn; struct page_info *page; int rc; - gfn = !is_pv_32bit_domain(currd) - ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val); + gfn = _gfn(!is_pv_32bit_domain(currd) + ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val)); page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC); if ( !page ) break; diff --git a/xen/arch/x86/pv/mm.c b/xen/arch/x86/pv/mm.c index f5ea00ca4e..c9ad1152b4 100644 --- a/xen/arch/x86/pv/mm.c +++ b/xen/arch/x86/pv/mm.c @@ -106,7 +106,7 @@ bool pv_map_ldt_shadow_page(unsigned int offset) if ( unlikely(!(l1e_get_flags(gl1e) & _PAGE_PRESENT)) ) return false; - page = get_page_from_gfn(currd, l1e_get_pfn(gl1e), NULL, P2M_ALLOC); + page = get_page_from_gfn(currd, _gfn(l1e_get_pfn(gl1e)), NULL, P2M_ALLOC); if ( unlikely(!page) ) return false; diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 9471d89022..d967e49432 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -795,7 +795,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val) case 0: /* Write hypercall page */ { void *hypercall_page; - unsigned long gmfn = val >> PAGE_SHIFT; + gfn_t gfn = gaddr_to_gfn(val); unsigned int page_index = val & (PAGE_SIZE - 1); struct page_info *page; p2m_type_t t; @@ -808,7 +808,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val) return X86EMUL_EXCEPTION; } - page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC); + page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC); if ( !page || !get_page_type(page, PGT_writable_page) ) { @@ -817,13 +817,14 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val) if ( p2m_is_paging(t) ) { - p2m_mem_paging_populate(d, gmfn); + p2m_mem_paging_populate(d, gfn_x(gfn)); return X86EMUL_RETRY; } gdprintk(XENLOG_WARNING, - "Bad GMFN %lx (MFN %#"PRI_mfn") to MSR %08x\n", - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN), base); + "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn") to MSR %08x\n", + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN), + base); return X86EMUL_EXCEPTION; } diff --git a/xen/common/domain.c b/xen/common/domain.c index d6650f0656..5e3c05b96c 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -1250,7 +1250,7 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset) if ( (v != current) && !(v->pause_flags & VPF_down) ) return -EINVAL; - page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); + page = get_page_from_gfn(d, _gfn(gfn), NULL, P2M_ALLOC); if ( !page ) return -EINVAL; diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c index c49f446754..71a6f673b2 100644 --- a/xen/common/event_fifo.c +++ b/xen/common/event_fifo.c @@ -358,7 +358,7 @@ static const struct evtchn_port_ops evtchn_port_ops_fifo = .print_state = evtchn_fifo_print_state, }; -static int map_guest_page(struct domain *d, uint64_t gfn, void **virt) +static int map_guest_page(struct domain *d, gfn_t gfn, void **virt) { struct page_info *p; @@ -419,7 +419,7 @@ static int setup_control_block(struct vcpu *v) return 0; } -static int map_control_block(struct vcpu *v, uint64_t gfn, uint32_t offset) +static int map_control_block(struct vcpu *v, gfn_t gfn, uint32_t offset) { void *virt; unsigned int i; @@ -505,7 +505,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control) { struct domain *d = current->domain; uint32_t vcpu_id; - uint64_t gfn; + gfn_t gfn; uint32_t offset; struct vcpu *v; int rc; @@ -513,7 +513,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control) init_control->link_bits = EVTCHN_FIFO_LINK_BITS; vcpu_id = init_control->vcpu; - gfn = init_control->control_gfn; + gfn = _gfn(init_control->control_gfn); offset = init_control->offset; if ( vcpu_id >= d->max_vcpus || !d->vcpu[vcpu_id] ) @@ -569,7 +569,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control) return rc; } -static int add_page_to_event_array(struct domain *d, unsigned long gfn) +static int add_page_to_event_array(struct domain *d, gfn_t gfn) { void *virt; unsigned int slot; @@ -619,7 +619,7 @@ int evtchn_fifo_expand_array(const struct evtchn_expand_array *expand_array) return -EOPNOTSUPP; spin_lock(&d->event_lock); - rc = add_page_to_event_array(d, expand_array->array_gfn); + rc = add_page_to_event_array(d, _gfn(expand_array->array_gfn)); spin_unlock(&d->event_lock); return rc; diff --git a/xen/common/memory.c b/xen/common/memory.c index 987395fbb3..e02733dba0 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -1365,7 +1365,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) return rc; } - page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC); + page = get_page_from_gfn(d, _gfn(xrfp.gpfn), NULL, P2M_ALLOC); if ( page ) { rc = guest_physmap_remove_page(d, _gfn(xrfp.gpfn), @@ -1636,7 +1636,7 @@ int check_get_page_from_gfn(struct domain *d, gfn_t gfn, bool readonly, p2m_type_t p2mt; struct page_info *page; - page = get_page_from_gfn(d, gfn_x(gfn), &p2mt, q); + page = get_page_from_gfn(d, gfn, &p2mt, q); #ifdef CONFIG_HAS_MEM_PAGING if ( p2m_is_paging(p2mt) ) diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c index bf7b14f79a..72cba7f10c 100644 --- a/xen/common/tmem_xen.c +++ b/xen/common/tmem_xen.c @@ -52,7 +52,7 @@ static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t *pcli_mfn, p2m_type_t t; struct page_info *page; - page = get_page_from_gfn(current->domain, cmfn, &t, P2M_ALLOC); + page = get_page_from_gfn(current->domain, _gfn(cmfn), &t, P2M_ALLOC); if ( !page || t != p2m_ram_rw ) { if ( page ) diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index 7c67806056..5e598a0b37 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -278,7 +278,7 @@ struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn, p2m_type_t *t); static inline struct page_info *get_page_from_gfn( - struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) + struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q) { mfn_t mfn; p2m_type_t _t; @@ -289,7 +289,7 @@ static inline struct page_info *get_page_from_gfn( * not auto-translated. */ if ( unlikely(d != dom_xen) ) - return p2m_get_page_from_gfn(d, _gfn(gfn), t); + return p2m_get_page_from_gfn(d, gfn, t); if ( !t ) t = &_t; @@ -300,7 +300,7 @@ static inline struct page_info *get_page_from_gfn( * DOMID_XEN see 1-1 RAM. The p2m_type is based on the type of the * page. */ - mfn = _mfn(gfn); + mfn = _mfn(gfn_x(gfn)); page = mfn_to_page(mfn); if ( !mfn_valid(mfn) || !get_page(page, d) ) diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h index d08c595887..db1ec37610 100644 --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -489,18 +489,21 @@ struct page_info *p2m_get_page_from_gfn(struct p2m_domain *p2m, gfn_t gfn, p2m_query_t q); static inline struct page_info *get_page_from_gfn( - struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) + struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q) { struct page_info *page; + mfn_t mfn; if ( paging_mode_translate(d) ) - return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, NULL, q); + return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL, q); /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */ if ( t ) *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct; - page = mfn_to_page(_mfn(gfn)); - return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL; + + mfn = _mfn(gfn_x(gfn)); + page = mfn_to_page(mfn); + return mfn_valid(mfn) && get_page(page, d) ? page : NULL; } /* General conversion function from mfn to gfn */ -- 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] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-06 19:14 ` [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn Julien Grall @ 2018-11-06 20:11 ` Andrew Cooper 2018-11-07 8:57 ` Paul Durrant 2018-11-07 13:08 ` Julien Grall 2018-11-07 9:24 ` Paul Durrant 2018-11-12 16:49 ` Andrii Anisov 2 siblings, 2 replies; 44+ messages in thread From: Andrew Cooper @ 2018-11-06 20:11 UTC (permalink / raw) To: Julien Grall, sstabellini, xen-devel Cc: Jun Nakajima, Kevin Tian, Wei Liu, Suravee Suthikulpanit, Konrad Rzeszutek Wilk, George Dunlap, Ian Jackson, Tim Deegan, Julien Grall, Paul Durrant, Jan Beulich, Boris Ostrovsky, Brian Woods Hi - just some cosmetic suggestions. Subject s/Swich/Switch/ > diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c > index f73ea4a163..a529ebcc3f 100644 > --- a/xen/arch/x86/pv/emul-priv-op.c > +++ b/xen/arch/x86/pv/emul-priv-op.c > @@ -760,12 +760,12 @@ static int write_cr(unsigned int reg, unsigned long val, > case 3: /* Write CR3 */ > { > struct domain *currd = curr->domain; > - unsigned long gfn; > + gfn_t gfn; > struct page_info *page; > int rc; > > - gfn = !is_pv_32bit_domain(currd) > - ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val); > + gfn = _gfn(!is_pv_32bit_domain(currd) > + ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val)); Please re-indent. > page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC); > if ( !page ) > break; > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index 9471d89022..d967e49432 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -817,13 +817,14 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val) > > if ( p2m_is_paging(t) ) > { > - p2m_mem_paging_populate(d, gmfn); > + p2m_mem_paging_populate(d, gfn_x(gfn)); > return X86EMUL_RETRY; > } > > gdprintk(XENLOG_WARNING, > - "Bad GMFN %lx (MFN %#"PRI_mfn") to MSR %08x\n", > - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN), base); > + "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn") to MSR %08x\n", GMFN => GFN. > + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN), > + base); > return X86EMUL_EXCEPTION; > } > > diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h > index d08c595887..db1ec37610 100644 > --- a/xen/include/asm-x86/p2m.h > +++ b/xen/include/asm-x86/p2m.h > @@ -489,18 +489,21 @@ struct page_info *p2m_get_page_from_gfn(struct p2m_domain *p2m, gfn_t gfn, > p2m_query_t q); > > static inline struct page_info *get_page_from_gfn( > - struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) > + struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q) > { > struct page_info *page; > + mfn_t mfn; > > if ( paging_mode_translate(d) ) > - return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, NULL, q); > + return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL, q); > > /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */ > if ( t ) > *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct; > - page = mfn_to_page(_mfn(gfn)); > - return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL; > + > + mfn = _mfn(gfn_x(gfn)); > + page = mfn_to_page(mfn); > + return mfn_valid(mfn) && get_page(page, d) ? page : NULL; This looks like it would be cleaner by not splitting mfn out into a separate variable. page = mfn_to_page(_mfn(gfn_x(gfn))); return mfn_valid(mfn) && get_page(page, d) ? page : NULL; The only reason this looks odd is because of the mfn => gfn equality, but we are just beside a comment explaining that we are non-translated. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-06 20:11 ` Andrew Cooper @ 2018-11-07 8:57 ` Paul Durrant 2018-11-07 13:08 ` Andrew Cooper 2018-11-07 13:08 ` Julien Grall 1 sibling, 1 reply; 44+ messages in thread From: Paul Durrant @ 2018-11-07 8:57 UTC (permalink / raw) To: Andrew Cooper, Julien Grall, sstabellini, xen-devel Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit, Konrad Rzeszutek Wilk, Jun Nakajima, Tim (Xen.org), George Dunlap, Julien Grall, Jan Beulich, Ian Jackson, Boris Ostrovsky, Brian Woods > -----Original Message----- > From: Andrew Cooper > Sent: 06 November 2018 20:12 > To: Julien Grall <julien.grall@arm.com>; sstabellini@kernel.org; xen- > devel@lists.xenproject.org > Cc: George Dunlap <George.Dunlap@citrix.com>; Ian Jackson > <Ian.Jackson@citrix.com>; Jan Beulich <jbeulich@suse.com>; Konrad > Rzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Wei > Liu <wei.liu2@citrix.com>; Boris Ostrovsky <boris.ostrovsky@oracle.com>; > Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Brian Woods > <brian.woods@amd.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jun > Nakajima <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>; > Julien Grall <julie.grall@arm.com> > Subject: Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use > typesafe gfn > > Hi - just some cosmetic suggestions. > > Subject s/Swich/Switch/ > > > diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv- > op.c > > index f73ea4a163..a529ebcc3f 100644 > > --- a/xen/arch/x86/pv/emul-priv-op.c > > +++ b/xen/arch/x86/pv/emul-priv-op.c > > @@ -760,12 +760,12 @@ static int write_cr(unsigned int reg, unsigned > long val, > > case 3: /* Write CR3 */ > > { > > struct domain *currd = curr->domain; > > - unsigned long gfn; > > + gfn_t gfn; > > struct page_info *page; > > int rc; > > > > - gfn = !is_pv_32bit_domain(currd) > > - ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val); > > + gfn = _gfn(!is_pv_32bit_domain(currd) > > + ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val)); > > Please re-indent. > > > page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC); > > if ( !page ) > > break; > > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > > index 9471d89022..d967e49432 100644 > > --- a/xen/arch/x86/traps.c > > +++ b/xen/arch/x86/traps.c > > @@ -817,13 +817,14 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, > uint64_t val) > > > > if ( p2m_is_paging(t) ) > > { > > - p2m_mem_paging_populate(d, gmfn); > > + p2m_mem_paging_populate(d, gfn_x(gfn)); > > return X86EMUL_RETRY; > > } > > > > gdprintk(XENLOG_WARNING, > > - "Bad GMFN %lx (MFN %#"PRI_mfn") to MSR %08x\n", > > - gmfn, mfn_x(page ? page_to_mfn(page) : > INVALID_MFN), base); > > + "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn") to MSR > %08x\n", > > GMFN => GFN. > > > + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : > INVALID_MFN), > > + base); > > return X86EMUL_EXCEPTION; > > } > > > > diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h > > index d08c595887..db1ec37610 100644 > > --- a/xen/include/asm-x86/p2m.h > > +++ b/xen/include/asm-x86/p2m.h > > @@ -489,18 +489,21 @@ struct page_info *p2m_get_page_from_gfn(struct > p2m_domain *p2m, gfn_t gfn, > > p2m_query_t q); > > > > static inline struct page_info *get_page_from_gfn( > > - struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) > > + struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q) > > { > > struct page_info *page; > > + mfn_t mfn; > > > > if ( paging_mode_translate(d) ) > > - return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, > NULL, q); > > + return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL, > q); > > > > /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */ > > if ( t ) > > *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct; > > - page = mfn_to_page(_mfn(gfn)); > > - return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL; > > + > > + mfn = _mfn(gfn_x(gfn)); > > + page = mfn_to_page(mfn); > > + return mfn_valid(mfn) && get_page(page, d) ? page : NULL; > > This looks like it would be cleaner by not splitting mfn out into a > separate variable. > > page = mfn_to_page(_mfn(gfn_x(gfn))); > > return mfn_valid(mfn) && get_page(page, d) ? page : NULL; ^^ er... how's that mfn_valid() going to work? You'd need mfn_valid(page_to_mfn(pahge)), or somesuch. Paul > > The only reason this looks odd is because of the mfn => gfn equality, > but we are just beside a comment explaining that we are non-translated. > > ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-07 8:57 ` Paul Durrant @ 2018-11-07 13:08 ` Andrew Cooper 0 siblings, 0 replies; 44+ messages in thread From: Andrew Cooper @ 2018-11-07 13:08 UTC (permalink / raw) To: Paul Durrant, Julien Grall, sstabellini, xen-devel Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit, Konrad Rzeszutek Wilk, Jun Nakajima, Tim (Xen.org), George Dunlap, Julien Grall, Jan Beulich, Ian Jackson, Boris Ostrovsky, Brian Woods On 07/11/18 08:57, Paul Durrant wrote: >> >>> static inline struct page_info *get_page_from_gfn( >>> - struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) >>> + struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q) >>> { >>> struct page_info *page; >>> + mfn_t mfn; >>> >>> if ( paging_mode_translate(d) ) >>> - return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, >> NULL, q); >>> + return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL, >> q); >>> /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */ >>> if ( t ) >>> *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct; >>> - page = mfn_to_page(_mfn(gfn)); >>> - return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL; >>> + >>> + mfn = _mfn(gfn_x(gfn)); >>> + page = mfn_to_page(mfn); >>> + return mfn_valid(mfn) && get_page(page, d) ? page : NULL; >> This looks like it would be cleaner by not splitting mfn out into a >> separate variable. >> >> page = mfn_to_page(_mfn(gfn_x(gfn))); >> >> return mfn_valid(mfn) && get_page(page, d) ? page : NULL; > ^^ er... how's that mfn_valid() going to work? You'd need mfn_valid(page_to_mfn(pahge)), or somesuch. Oops - I'm blind. Sorry for the noise. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-06 20:11 ` Andrew Cooper 2018-11-07 8:57 ` Paul Durrant @ 2018-11-07 13:08 ` Julien Grall 1 sibling, 0 replies; 44+ messages in thread From: Julien Grall @ 2018-11-07 13:08 UTC (permalink / raw) To: Andrew Cooper, sstabellini, xen-devel Cc: Jun Nakajima, Kevin Tian, Wei Liu, Suravee Suthikulpanit, Konrad Rzeszutek Wilk, George Dunlap, Ian Jackson, Tim Deegan, Paul Durrant, Jan Beulich, Boris Ostrovsky, Brian Woods Hi Andrew, On 06/11/2018 20:11, Andrew Cooper wrote: > This looks like it would be cleaner by not splitting mfn out into a > separate variable. > > page = mfn_to_page(_mfn(gfn_x(gfn))); > > return mfn_valid(mfn) && get_page(page, d) ? page : NULL; > > The only reason this looks odd is because of the mfn => gfn equality, > but we are just beside a comment explaining that we are non-translated. I introduced the mfn variable to avoid duplicating _mfn(gfn_x(gfn)) in the two places where gfn was used. I am happy to duplicated if you prefer. 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] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-06 19:14 ` [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn Julien Grall 2018-11-06 20:11 ` Andrew Cooper @ 2018-11-07 9:24 ` Paul Durrant 2018-11-07 13:05 ` Julien Grall 2018-11-12 16:49 ` Andrii Anisov 2 siblings, 1 reply; 44+ messages in thread From: Paul Durrant @ 2018-11-07 9:24 UTC (permalink / raw) To: 'Julien Grall', sstabellini, xen-devel Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit, Konrad Rzeszutek Wilk, Jun Nakajima, Andrew Cooper, Tim (Xen.org), George Dunlap, Julien Grall, Jan Beulich, Ian Jackson, Boris Ostrovsky, Brian Woods > -----Original Message----- > From: Julien Grall [mailto:julien.grall@arm.com] > Sent: 06 November 2018 19:15 > To: sstabellini@kernel.org; xen-devel@lists.xenproject.org > Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Ian > Jackson <Ian.Jackson@citrix.com>; Jan Beulich <jbeulich@suse.com>; Konrad > Rzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Wei > Liu <wei.liu2@citrix.com>; Boris Ostrovsky <boris.ostrovsky@oracle.com>; > Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Brian Woods > <brian.woods@amd.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jun > Nakajima <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>; > Julien Grall <julie.grall@arm.com> > Subject: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use > typesafe gfn > > No functional change intended. > > Only reasonable clean-ups are done in this patch. The rest will use _gfn > for the time being. > > Signed-off-by: Julien Grall <julie.grall@arm.com> > --- > xen/arch/arm/guestcopy.c | 2 +- > xen/arch/arm/mm.c | 2 +- > xen/arch/x86/cpu/vpmu.c | 2 +- > xen/arch/x86/domain.c | 12 ++++++------ > xen/arch/x86/domctl.c | 6 +++--- > xen/arch/x86/hvm/dm.c | 2 +- > xen/arch/x86/hvm/domain.c | 2 +- > xen/arch/x86/hvm/hvm.c | 9 +++++---- > xen/arch/x86/hvm/svm/svm.c | 8 ++++---- > xen/arch/x86/hvm/viridian/viridian.c | 24 ++++++++++++------------ > xen/arch/x86/hvm/vmx/vmx.c | 4 ++-- > xen/arch/x86/hvm/vmx/vvmx.c | 12 ++++++------ > xen/arch/x86/mm.c | 24 ++++++++++++++---------- > xen/arch/x86/mm/p2m.c | 2 +- > xen/arch/x86/mm/shadow/hvm.c | 6 +++--- > xen/arch/x86/physdev.c | 3 ++- > xen/arch/x86/pv/descriptor-tables.c | 5 ++--- > xen/arch/x86/pv/emul-priv-op.c | 6 +++--- > xen/arch/x86/pv/mm.c | 2 +- > xen/arch/x86/traps.c | 11 ++++++----- > xen/common/domain.c | 2 +- > xen/common/event_fifo.c | 12 ++++++------ > xen/common/memory.c | 4 ++-- > xen/common/tmem_xen.c | 2 +- > xen/include/asm-arm/p2m.h | 6 +++--- > xen/include/asm-x86/p2m.h | 11 +++++++---- > 26 files changed, 95 insertions(+), 86 deletions(-) > [snip] > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 5d00256aaa..a7419bd444 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -317,7 +317,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct > hvm_hw_cpu *c) > { > if ( c->cr0 & X86_CR0_PG ) > { > - page = get_page_from_gfn(v->domain, c->cr3 >> PAGE_SHIFT, > + page = get_page_from_gfn(v->domain, gaddr_to_gfn(c->cr3), > NULL, P2M_ALLOC); > if ( !page ) > { > @@ -2412,9 +2412,9 @@ nsvm_get_nvmcb_page(struct vcpu *v, uint64_t > vmcbaddr) > return NULL; > > /* Need to translate L1-GPA to MPA */ > - page = get_page_from_gfn(v->domain, > - nv->nv_vvmcxaddr >> PAGE_SHIFT, > - &p2mt, P2M_ALLOC | P2M_UNSHARE); > + page = get_page_from_gfn(v->domain, > + gaddr_to_gfn(nv->nv_vvmcxaddr >> > PAGE_SHIFT), Don't you need to lose the '>> PAGE_SHIFT' now? Paul > + &p2mt, P2M_ALLOC | P2M_UNSHARE); > if ( !page ) > return NULL; > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-07 9:24 ` Paul Durrant @ 2018-11-07 13:05 ` Julien Grall 0 siblings, 0 replies; 44+ messages in thread From: Julien Grall @ 2018-11-07 13:05 UTC (permalink / raw) To: Paul Durrant, sstabellini, xen-devel Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit, Konrad Rzeszutek Wilk, Jun Nakajima, Andrew Cooper, Tim (Xen.org), George Dunlap, Julien Grall, Jan Beulich, Ian Jackson, Boris Ostrovsky, Brian Woods Hi Paul, On 07/11/2018 09:24, Paul Durrant wrote: >> -----Original Message----- >> From: Julien Grall [mailto:julien.grall@arm.com] >> Sent: 06 November 2018 19:15 >> To: sstabellini@kernel.org; xen-devel@lists.xenproject.org >> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper >> <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Ian >> Jackson <Ian.Jackson@citrix.com>; Jan Beulich <jbeulich@suse.com>; Konrad >> Rzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Wei >> Liu <wei.liu2@citrix.com>; Boris Ostrovsky <boris.ostrovsky@oracle.com>; >> Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Brian Woods >> <brian.woods@amd.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jun >> Nakajima <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>; >> Julien Grall <julie.grall@arm.com> >> Subject: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use >> typesafe gfn >> >> No functional change intended. >> >> Only reasonable clean-ups are done in this patch. The rest will use _gfn >> for the time being. >> >> Signed-off-by: Julien Grall <julie.grall@arm.com> >> --- >> xen/arch/arm/guestcopy.c | 2 +- >> xen/arch/arm/mm.c | 2 +- >> xen/arch/x86/cpu/vpmu.c | 2 +- >> xen/arch/x86/domain.c | 12 ++++++------ >> xen/arch/x86/domctl.c | 6 +++--- >> xen/arch/x86/hvm/dm.c | 2 +- >> xen/arch/x86/hvm/domain.c | 2 +- >> xen/arch/x86/hvm/hvm.c | 9 +++++---- >> xen/arch/x86/hvm/svm/svm.c | 8 ++++---- >> xen/arch/x86/hvm/viridian/viridian.c | 24 ++++++++++++------------ >> xen/arch/x86/hvm/vmx/vmx.c | 4 ++-- >> xen/arch/x86/hvm/vmx/vvmx.c | 12 ++++++------ >> xen/arch/x86/mm.c | 24 ++++++++++++++---------- >> xen/arch/x86/mm/p2m.c | 2 +- >> xen/arch/x86/mm/shadow/hvm.c | 6 +++--- >> xen/arch/x86/physdev.c | 3 ++- >> xen/arch/x86/pv/descriptor-tables.c | 5 ++--- >> xen/arch/x86/pv/emul-priv-op.c | 6 +++--- >> xen/arch/x86/pv/mm.c | 2 +- >> xen/arch/x86/traps.c | 11 ++++++----- >> xen/common/domain.c | 2 +- >> xen/common/event_fifo.c | 12 ++++++------ >> xen/common/memory.c | 4 ++-- >> xen/common/tmem_xen.c | 2 +- >> xen/include/asm-arm/p2m.h | 6 +++--- >> xen/include/asm-x86/p2m.h | 11 +++++++---- >> 26 files changed, 95 insertions(+), 86 deletions(-) >> > [snip] >> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c >> index 5d00256aaa..a7419bd444 100644 >> --- a/xen/arch/x86/hvm/svm/svm.c >> +++ b/xen/arch/x86/hvm/svm/svm.c >> @@ -317,7 +317,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct >> hvm_hw_cpu *c) >> { >> if ( c->cr0 & X86_CR0_PG ) >> { >> - page = get_page_from_gfn(v->domain, c->cr3 >> PAGE_SHIFT, >> + page = get_page_from_gfn(v->domain, gaddr_to_gfn(c->cr3), >> NULL, P2M_ALLOC); >> if ( !page ) >> { >> @@ -2412,9 +2412,9 @@ nsvm_get_nvmcb_page(struct vcpu *v, uint64_t >> vmcbaddr) >> return NULL; >> >> /* Need to translate L1-GPA to MPA */ >> - page = get_page_from_gfn(v->domain, >> - nv->nv_vvmcxaddr >> PAGE_SHIFT, >> - &p2mt, P2M_ALLOC | P2M_UNSHARE); >> + page = get_page_from_gfn(v->domain, >> + gaddr_to_gfn(nv->nv_vvmcxaddr >> >> PAGE_SHIFT), > > Don't you need to lose the '>> PAGE_SHIFT' now? Yes. Brian reported on IRC and now it is fixed. Thank you for the review. 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] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-06 19:14 ` [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn Julien Grall 2018-11-06 20:11 ` Andrew Cooper 2018-11-07 9:24 ` Paul Durrant @ 2018-11-12 16:49 ` Andrii Anisov 2018-11-12 16:52 ` Julien Grall 2 siblings, 1 reply; 44+ messages in thread From: Andrii Anisov @ 2018-11-12 16:49 UTC (permalink / raw) To: Julien Grall Cc: kevin.tian, Stefano Stabellini, Wei Liu, jun.nakajima, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim (Xen.org), julie.grall, Jan Beulich, Paul Durrant, suravee.suthikulpanit, xen-devel, Boris Ostrovsky, brian.woods [-- Attachment #1.1: Type: text/plain, Size: 34146 bytes --] Hello Julien, I'm just wondering if this patch really belongs to xentrace series. It rather looks like a separate cleanup patch. Sincerely, Andrii Anisov. вт, 6 лист. 2018 о 21:16 Julien Grall <julien.grall@arm.com> пише: > No functional change intended. > > Only reasonable clean-ups are done in this patch. The rest will use _gfn > for the time being. > > Signed-off-by: Julien Grall <julie.grall@arm.com> > --- > xen/arch/arm/guestcopy.c | 2 +- > xen/arch/arm/mm.c | 2 +- > xen/arch/x86/cpu/vpmu.c | 2 +- > xen/arch/x86/domain.c | 12 ++++++------ > xen/arch/x86/domctl.c | 6 +++--- > xen/arch/x86/hvm/dm.c | 2 +- > xen/arch/x86/hvm/domain.c | 2 +- > xen/arch/x86/hvm/hvm.c | 9 +++++---- > xen/arch/x86/hvm/svm/svm.c | 8 ++++---- > xen/arch/x86/hvm/viridian/viridian.c | 24 ++++++++++++------------ > xen/arch/x86/hvm/vmx/vmx.c | 4 ++-- > xen/arch/x86/hvm/vmx/vvmx.c | 12 ++++++------ > xen/arch/x86/mm.c | 24 ++++++++++++++---------- > xen/arch/x86/mm/p2m.c | 2 +- > xen/arch/x86/mm/shadow/hvm.c | 6 +++--- > xen/arch/x86/physdev.c | 3 ++- > xen/arch/x86/pv/descriptor-tables.c | 5 ++--- > xen/arch/x86/pv/emul-priv-op.c | 6 +++--- > xen/arch/x86/pv/mm.c | 2 +- > xen/arch/x86/traps.c | 11 ++++++----- > xen/common/domain.c | 2 +- > xen/common/event_fifo.c | 12 ++++++------ > xen/common/memory.c | 4 ++-- > xen/common/tmem_xen.c | 2 +- > xen/include/asm-arm/p2m.h | 6 +++--- > xen/include/asm-x86/p2m.h | 11 +++++++---- > 26 files changed, 95 insertions(+), 86 deletions(-) > > diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c > index 7a0f3e9d5f..55892062bb 100644 > --- a/xen/arch/arm/guestcopy.c > +++ b/xen/arch/arm/guestcopy.c > @@ -37,7 +37,7 @@ static struct page_info *translate_get_page(copy_info_t > info, uint64_t addr, > return get_page_from_gva(info.gva.v, addr, > write ? GV2M_WRITE : GV2M_READ); > > - page = get_page_from_gfn(info.gpa.d, paddr_to_pfn(addr), &p2mt, > P2M_ALLOC); > + page = get_page_from_gfn(info.gpa.d, gaddr_to_gfn(addr), &p2mt, > P2M_ALLOC); > > if ( !page ) > return NULL; > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 72d0285768..88711096ef 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -1268,7 +1268,7 @@ int xenmem_add_to_physmap_one( > > /* Take reference to the foreign domain page. > * Reference will be released in XENMEM_remove_from_physmap */ > - page = get_page_from_gfn(od, idx, &p2mt, P2M_ALLOC); > + page = get_page_from_gfn(od, _gfn(idx), &p2mt, P2M_ALLOC); > if ( !page ) > { > put_pg_owner(od); > diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c > index 8a4f753eae..4d8f153031 100644 > --- a/xen/arch/x86/cpu/vpmu.c > +++ b/xen/arch/x86/cpu/vpmu.c > @@ -607,7 +607,7 @@ static int pvpmu_init(struct domain *d, > xen_pmu_params_t *params) > struct vcpu *v; > struct vpmu_struct *vpmu; > struct page_info *page; > - uint64_t gfn = params->val; > + gfn_t gfn = _gfn(params->val); > > if ( (params->vcpu >= d->max_vcpus) || (d->vcpu[params->vcpu] == > NULL) ) > return -EINVAL; > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index f6fe954313..c5cce4b38d 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -797,7 +797,7 @@ int arch_set_info_guest( > unsigned long flags; > bool compat; > #ifdef CONFIG_PV > - unsigned long cr3_gfn; > + gfn_t cr3_gfn; > struct page_info *cr3_page; > unsigned long cr4; > int rc = 0; > @@ -1061,9 +1061,9 @@ int arch_set_info_guest( > set_bit(_VPF_in_reset, &v->pause_flags); > > if ( !compat ) > - cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[3]); > + cr3_gfn = _gfn(xen_cr3_to_pfn(c.nat->ctrlreg[3])); > else > - cr3_gfn = compat_cr3_to_pfn(c.cmp->ctrlreg[3]); > + cr3_gfn = _gfn(compat_cr3_to_pfn(c.cmp->ctrlreg[3])); > cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC); > > if ( !cr3_page ) > @@ -1092,7 +1092,7 @@ int arch_set_info_guest( > case 0: > if ( !compat && !VM_ASSIST(d, m2p_strict) && > !paging_mode_refcounts(d) ) > - fill_ro_mpt(_mfn(cr3_gfn)); > + fill_ro_mpt(_mfn(gfn_x(cr3_gfn))); > break; > default: > if ( cr3_page == current->arch.old_guest_table ) > @@ -1107,7 +1107,7 @@ int arch_set_info_guest( > v->arch.guest_table = pagetable_from_page(cr3_page); > if ( c.nat->ctrlreg[1] ) > { > - cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[1]); > + cr3_gfn = _gfn(xen_cr3_to_pfn(c.nat->ctrlreg[1])); > cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC); > > if ( !cr3_page ) > @@ -1132,7 +1132,7 @@ int arch_set_info_guest( > break; > case 0: > if ( VM_ASSIST(d, m2p_strict) ) > - zap_ro_mpt(_mfn(cr3_gfn)); > + zap_ro_mpt(_mfn(gfn_x(cr3_gfn))); > break; > } > } > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c > index 33f9a869c0..6b0d8075cd 100644 > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -448,7 +448,7 @@ long arch_do_domctl( > break; > } > > - page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC); > + page = get_page_from_gfn(d, _gfn(gfn), &t, P2M_ALLOC); > > if ( unlikely(!page) || > unlikely(is_xen_heap_page(page)) ) > @@ -498,11 +498,11 @@ long arch_do_domctl( > > case XEN_DOMCTL_hypercall_init: > { > - unsigned long gmfn = domctl->u.hypercall_init.gmfn; > + gfn_t gfn = _gfn(domctl->u.hypercall_init.gmfn); > struct page_info *page; > void *hypercall_page; > > - page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); > + page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); > > if ( !page || !get_page_type(page, PGT_writable_page) ) > { > diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c > index d6d0e8be89..3b3ad27938 100644 > --- a/xen/arch/x86/hvm/dm.c > +++ b/xen/arch/x86/hvm/dm.c > @@ -186,7 +186,7 @@ static int modified_memory(struct domain *d, > { > struct page_info *page; > > - page = get_page_from_gfn(d, pfn, NULL, P2M_UNSHARE); > + page = get_page_from_gfn(d, _gfn(pfn), NULL, P2M_UNSHARE); > if ( page ) > { > paging_mark_pfn_dirty(d, _pfn(pfn)); > diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c > index 5d5a746a25..73d2da8441 100644 > --- a/xen/arch/x86/hvm/domain.c > +++ b/xen/arch/x86/hvm/domain.c > @@ -297,7 +297,7 @@ int arch_set_info_hvm_guest(struct vcpu *v, const > vcpu_hvm_context_t *ctx) > { > /* Shadow-mode CR3 change. Check PDBR and update refcounts. */ > struct page_info *page = get_page_from_gfn(v->domain, > - v->arch.hvm.guest_cr[3] >> PAGE_SHIFT, > + gaddr_to_gfn(v->arch.hvm.guest_cr[3]), > NULL, P2M_ALLOC); > if ( !page ) > { > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 7be9cf4454..be262e5a1d 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -2146,7 +2146,7 @@ int hvm_set_cr0(unsigned long value, bool may_defer) > { > struct vcpu *v = current; > struct domain *d = v->domain; > - unsigned long gfn, old_value = v->arch.hvm.guest_cr[0]; > + unsigned long old_value = v->arch.hvm.guest_cr[0]; > struct page_info *page; > > HVM_DBG_LOG(DBG_LEVEL_VMMU, "Update CR0 value = %lx", value); > @@ -2201,7 +2201,8 @@ int hvm_set_cr0(unsigned long value, bool may_defer) > if ( !paging_mode_hap(d) ) > { > /* The guest CR3 must be pointing to the guest physical. */ > - gfn = v->arch.hvm.guest_cr[3] >> PAGE_SHIFT; > + gfn_t gfn = gaddr_to_gfn(v->arch.hvm.guest_cr[3]); > + > page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); > if ( !page ) > { > @@ -2293,7 +2294,7 @@ int hvm_set_cr3(unsigned long value, bool may_defer) > { > /* Shadow-mode CR3 change. Check PDBR and update refcounts. */ > HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value); > - page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT, > + page = get_page_from_gfn(v->domain, gaddr_to_gfn(value), > NULL, P2M_ALLOC); > if ( !page ) > goto bad_cr3; > @@ -3120,7 +3121,7 @@ enum hvm_translation_result hvm_translate_get_page( > && hvm_mmio_internal(gfn_to_gaddr(gfn)) ) > return HVMTRANS_bad_gfn_to_mfn; > > - page = get_page_from_gfn(v->domain, gfn_x(gfn), &p2mt, P2M_UNSHARE); > + page = get_page_from_gfn(v->domain, gfn, &p2mt, P2M_UNSHARE); > > if ( !page ) > return HVMTRANS_bad_gfn_to_mfn; > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 5d00256aaa..a7419bd444 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -317,7 +317,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct > hvm_hw_cpu *c) > { > if ( c->cr0 & X86_CR0_PG ) > { > - page = get_page_from_gfn(v->domain, c->cr3 >> PAGE_SHIFT, > + page = get_page_from_gfn(v->domain, gaddr_to_gfn(c->cr3), > NULL, P2M_ALLOC); > if ( !page ) > { > @@ -2412,9 +2412,9 @@ nsvm_get_nvmcb_page(struct vcpu *v, uint64_t > vmcbaddr) > return NULL; > > /* Need to translate L1-GPA to MPA */ > - page = get_page_from_gfn(v->domain, > - nv->nv_vvmcxaddr >> PAGE_SHIFT, > - &p2mt, P2M_ALLOC | P2M_UNSHARE); > + page = get_page_from_gfn(v->domain, > + gaddr_to_gfn(nv->nv_vvmcxaddr >> PAGE_SHIFT), > + &p2mt, P2M_ALLOC | P2M_UNSHARE); > if ( !page ) > return NULL; > > diff --git a/xen/arch/x86/hvm/viridian/viridian.c > b/xen/arch/x86/hvm/viridian/viridian.c > index 2dc86dd0f3..1d3be156db 100644 > --- a/xen/arch/x86/hvm/viridian/viridian.c > +++ b/xen/arch/x86/hvm/viridian/viridian.c > @@ -332,16 +332,16 @@ static void dump_reference_tsc(const struct domain > *d) > > static void enable_hypercall_page(struct domain *d) > { > - unsigned long gmfn = d->arch.hvm.viridian.hypercall_gpa.fields.pfn; > - struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); > + gfn_t gfn = _gfn(d->arch.hvm.viridian.hypercall_gpa.fields.pfn); > + struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); > uint8_t *p; > > if ( !page || !get_page_type(page, PGT_writable_page) ) > { > if ( page ) > put_page(page); > - gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN > %#"PRI_mfn")\n", > - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); > + gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN > %#"PRI_mfn")\n", > + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : > INVALID_MFN)); > return; > } > > @@ -367,8 +367,8 @@ static void enable_hypercall_page(struct domain *d) > static void initialize_vp_assist(struct vcpu *v) > { > struct domain *d = v->domain; > - unsigned long gmfn = v->arch.hvm.viridian.vp_assist.msr.fields.pfn; > - struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); > + gfn_t gfn = _gfn(v->arch.hvm.viridian.vp_assist.msr.fields.pfn); > + struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); > void *va; > > ASSERT(!v->arch.hvm.viridian.vp_assist.va); > @@ -395,8 +395,8 @@ static void initialize_vp_assist(struct vcpu *v) > return; > > fail: > - gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", > - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); > + gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n", > + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); > } > > static void teardown_vp_assist(struct vcpu *v) > @@ -465,16 +465,16 @@ void viridian_apic_assist_clear(struct vcpu *v) > > static void update_reference_tsc(struct domain *d, bool_t initialize) > { > - unsigned long gmfn = d->arch.hvm.viridian.reference_tsc.fields.pfn; > - struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); > + gfn_t gfn = _gfn(d->arch.hvm.viridian.reference_tsc.fields.pfn); > + struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); > HV_REFERENCE_TSC_PAGE *p; > > if ( !page || !get_page_type(page, PGT_writable_page) ) > { > if ( page ) > put_page(page); > - gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN > %#"PRI_mfn")\n", > - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN)); > + gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN > %#"PRI_mfn")\n", > + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : > INVALID_MFN)); > return; > } > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index e065f8bbdb..2070e78358 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -674,7 +674,7 @@ static int vmx_restore_cr0_cr3( > { > if ( cr0 & X86_CR0_PG ) > { > - page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT, > + page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3), > NULL, P2M_ALLOC); > if ( !page ) > { > @@ -1373,7 +1373,7 @@ static void vmx_load_pdptrs(struct vcpu *v) > if ( (cr3 & 0x1fUL) && !hvm_pcid_enabled(v) ) > goto crash; > > - page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt, > P2M_UNSHARE); > + page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3), &p2mt, > P2M_UNSHARE); > if ( !page ) > { > /* Ideally you don't want to crash but rather go into a wait > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > index dfd08e2d0a..2953d05a17 100644 > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -649,11 +649,11 @@ static void nvmx_update_apic_access_address(struct > vcpu *v) > if ( ctrl & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES ) > { > p2m_type_t p2mt; > - unsigned long apic_gpfn; > + gfn_t apic_gfn; > struct page_info *apic_pg; > > - apic_gpfn = get_vvmcs(v, APIC_ACCESS_ADDR) >> PAGE_SHIFT; > - apic_pg = get_page_from_gfn(v->domain, apic_gpfn, &p2mt, > P2M_ALLOC); > + apic_gfn = gaddr_to_gfn(get_vvmcs(v, APIC_ACCESS_ADDR)); > + apic_pg = get_page_from_gfn(v->domain, apic_gfn, &p2mt, > P2M_ALLOC); > ASSERT(apic_pg && !p2m_is_paging(p2mt)); > __vmwrite(APIC_ACCESS_ADDR, page_to_maddr(apic_pg)); > put_page(apic_pg); > @@ -670,11 +670,11 @@ static void nvmx_update_virtual_apic_address(struct > vcpu *v) > if ( ctrl & CPU_BASED_TPR_SHADOW ) > { > p2m_type_t p2mt; > - unsigned long vapic_gpfn; > + gfn_t vapic_gfn; > struct page_info *vapic_pg; > > - vapic_gpfn = get_vvmcs(v, VIRTUAL_APIC_PAGE_ADDR) >> PAGE_SHIFT; > - vapic_pg = get_page_from_gfn(v->domain, vapic_gpfn, &p2mt, > P2M_ALLOC); > + vapic_gfn = gaddr_to_gfn(get_vvmcs(v, VIRTUAL_APIC_PAGE_ADDR)); > + vapic_pg = get_page_from_gfn(v->domain, vapic_gfn, &p2mt, > P2M_ALLOC); > ASSERT(vapic_pg && !p2m_is_paging(p2mt)); > __vmwrite(VIRTUAL_APIC_PAGE_ADDR, page_to_maddr(vapic_pg)); > put_page(vapic_pg); > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > index 9363e9bd96..e3462f8a77 100644 > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -2052,7 +2052,7 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, > l1_pgentry_t nl1e, > p2m_query_t q = l1e_get_flags(nl1e) & _PAGE_RW ? > P2M_ALLOC | P2M_UNSHARE : P2M_ALLOC; > > - page = get_page_from_gfn(pg_dom, l1e_get_pfn(nl1e), &p2mt, q); > + page = get_page_from_gfn(pg_dom, _gfn(l1e_get_pfn(nl1e)), > &p2mt, q); > > if ( p2m_is_paged(p2mt) ) > { > @@ -3223,7 +3223,8 @@ long do_mmuext_op( > if ( paging_mode_refcounts(pg_owner) ) > break; > > - page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, > P2M_ALLOC); > + page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), NULL, > + P2M_ALLOC); > if ( unlikely(!page) ) > { > rc = -EINVAL; > @@ -3288,7 +3289,8 @@ long do_mmuext_op( > if ( paging_mode_refcounts(pg_owner) ) > break; > > - page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, > P2M_ALLOC); > + page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), NULL, > + P2M_ALLOC); > if ( unlikely(!page) ) > { > gdprintk(XENLOG_WARNING, > @@ -3504,7 +3506,8 @@ long do_mmuext_op( > } > > case MMUEXT_CLEAR_PAGE: > - page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, > P2M_ALLOC); > + page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), &p2mt, > + P2M_ALLOC); > if ( unlikely(p2mt != p2m_ram_rw) && page ) > { > put_page(page); > @@ -3532,7 +3535,7 @@ long do_mmuext_op( > { > struct page_info *src_page, *dst_page; > > - src_page = get_page_from_gfn(pg_owner, op.arg2.src_mfn, &p2mt, > + src_page = get_page_from_gfn(pg_owner, _gfn(op.arg2.src_mfn), > &p2mt, > P2M_ALLOC); > if ( unlikely(p2mt != p2m_ram_rw) && src_page ) > { > @@ -3548,7 +3551,7 @@ long do_mmuext_op( > break; > } > > - dst_page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, > + dst_page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), > &p2mt, > P2M_ALLOC); > if ( unlikely(p2mt != p2m_ram_rw) && dst_page ) > { > @@ -3636,7 +3639,8 @@ long do_mmu_update( > { > struct mmu_update req; > void *va = NULL; > - unsigned long gpfn, gmfn, mfn; > + unsigned long gpfn, mfn; > + gfn_t gfn; > struct page_info *page; > unsigned int cmd, i = 0, done = 0, pt_dom; > struct vcpu *curr = current, *v = curr; > @@ -3749,8 +3753,8 @@ long do_mmu_update( > rc = -EINVAL; > > req.ptr -= cmd; > - gmfn = req.ptr >> PAGE_SHIFT; > - page = get_page_from_gfn(pt_owner, gmfn, &p2mt, P2M_ALLOC); > + gfn = gaddr_to_gfn(req.ptr); > + page = get_page_from_gfn(pt_owner, gfn, &p2mt, P2M_ALLOC); > > if ( unlikely(!page) || p2mt != p2m_ram_rw ) > { > @@ -3758,7 +3762,7 @@ long do_mmu_update( > put_page(page); > if ( p2m_is_paged(p2mt) ) > { > - p2m_mem_paging_populate(pt_owner, gmfn); > + p2m_mem_paging_populate(pt_owner, gfn_x(gfn)); > rc = -ENOENT; > } > else > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c > index a00a3c1bff..3b2aac8804 100644 > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -2718,7 +2718,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned > long fgfn, > * Take a refcnt on the mfn. NB: following supported for foreign > mapping: > * ram_rw | ram_logdirty | ram_ro | paging_out. > */ > - page = get_page_from_gfn(fdom, fgfn, &p2mt, P2M_ALLOC); > + page = get_page_from_gfn(fdom, _gfn(fgfn), &p2mt, P2M_ALLOC); > if ( !page || > !p2m_is_ram(p2mt) || p2m_is_shared(p2mt) || p2m_is_hole(p2mt) ) > { > diff --git a/xen/arch/x86/mm/shadow/hvm.c b/xen/arch/x86/mm/shadow/hvm.c > index 4cc75916b8..9275ba476c 100644 > --- a/xen/arch/x86/mm/shadow/hvm.c > +++ b/xen/arch/x86/mm/shadow/hvm.c > @@ -313,15 +313,15 @@ const struct x86_emulate_ops hvm_shadow_emulator_ops > = { > static mfn_t emulate_gva_to_mfn(struct vcpu *v, unsigned long vaddr, > struct sh_emulate_ctxt *sh_ctxt) > { > - unsigned long gfn; > + gfn_t gfn; > struct page_info *page; > mfn_t mfn; > p2m_type_t p2mt; > uint32_t pfec = PFEC_page_present | PFEC_write_access; > > /* Translate the VA to a GFN. */ > - gfn = paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, &pfec); > - if ( gfn == gfn_x(INVALID_GFN) ) > + gfn = _gfn(paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, &pfec)); > + if ( gfn_eq(gfn, INVALID_GFN) ) > { > x86_emul_pagefault(pfec, vaddr, &sh_ctxt->ctxt); > > diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c > index 3a3c15890b..4f3f438614 100644 > --- a/xen/arch/x86/physdev.c > +++ b/xen/arch/x86/physdev.c > @@ -229,7 +229,8 @@ ret_t do_physdev_op(int cmd, > XEN_GUEST_HANDLE_PARAM(void) arg) > break; > > ret = -EINVAL; > - page = get_page_from_gfn(current->domain, info.gmfn, NULL, > P2M_ALLOC); > + page = get_page_from_gfn(current->domain, _gfn(info.gmfn), > + NULL, P2M_ALLOC); > if ( !page ) > break; > if ( !get_page_type(page, PGT_writable_page) ) > diff --git a/xen/arch/x86/pv/descriptor-tables.c > b/xen/arch/x86/pv/descriptor-tables.c > index 8b2d55fc2e..7e8f41d3fd 100644 > --- a/xen/arch/x86/pv/descriptor-tables.c > +++ b/xen/arch/x86/pv/descriptor-tables.c > @@ -112,7 +112,7 @@ long pv_set_gdt(struct vcpu *v, unsigned long *frames, > unsigned int entries) > { > struct page_info *page; > > - page = get_page_from_gfn(d, frames[i], NULL, P2M_ALLOC); > + page = get_page_from_gfn(d, _gfn(frames[i]), NULL, P2M_ALLOC); > if ( !page ) > goto fail; > if ( !get_page_type(page, PGT_seg_desc_page) ) > @@ -209,7 +209,6 @@ int compat_set_gdt(XEN_GUEST_HANDLE_PARAM(uint) > frame_list, > long do_update_descriptor(uint64_t pa, uint64_t desc) > { > struct domain *currd = current->domain; > - unsigned long gmfn = pa >> PAGE_SHIFT; > unsigned long mfn; > unsigned int offset; > struct desc_struct *gdt_pent, d; > @@ -220,7 +219,7 @@ long do_update_descriptor(uint64_t pa, uint64_t desc) > > *(uint64_t *)&d = desc; > > - page = get_page_from_gfn(currd, gmfn, NULL, P2M_ALLOC); > + page = get_page_from_gfn(currd, gaddr_to_gfn(pa), NULL, P2M_ALLOC); > if ( (((unsigned int)pa % sizeof(struct desc_struct)) != 0) || > !page || > !check_descriptor(currd, &d) ) > diff --git a/xen/arch/x86/pv/emul-priv-op.c > b/xen/arch/x86/pv/emul-priv-op.c > index f73ea4a163..a529ebcc3f 100644 > --- a/xen/arch/x86/pv/emul-priv-op.c > +++ b/xen/arch/x86/pv/emul-priv-op.c > @@ -760,12 +760,12 @@ static int write_cr(unsigned int reg, unsigned long > val, > case 3: /* Write CR3 */ > { > struct domain *currd = curr->domain; > - unsigned long gfn; > + gfn_t gfn; > struct page_info *page; > int rc; > > - gfn = !is_pv_32bit_domain(currd) > - ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val); > + gfn = _gfn(!is_pv_32bit_domain(currd) > + ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val)); > page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC); > if ( !page ) > break; > diff --git a/xen/arch/x86/pv/mm.c b/xen/arch/x86/pv/mm.c > index f5ea00ca4e..c9ad1152b4 100644 > --- a/xen/arch/x86/pv/mm.c > +++ b/xen/arch/x86/pv/mm.c > @@ -106,7 +106,7 @@ bool pv_map_ldt_shadow_page(unsigned int offset) > if ( unlikely(!(l1e_get_flags(gl1e) & _PAGE_PRESENT)) ) > return false; > > - page = get_page_from_gfn(currd, l1e_get_pfn(gl1e), NULL, P2M_ALLOC); > + page = get_page_from_gfn(currd, _gfn(l1e_get_pfn(gl1e)), NULL, > P2M_ALLOC); > if ( unlikely(!page) ) > return false; > > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index 9471d89022..d967e49432 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -795,7 +795,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, > uint64_t val) > case 0: /* Write hypercall page */ > { > void *hypercall_page; > - unsigned long gmfn = val >> PAGE_SHIFT; > + gfn_t gfn = gaddr_to_gfn(val); > unsigned int page_index = val & (PAGE_SIZE - 1); > struct page_info *page; > p2m_type_t t; > @@ -808,7 +808,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, > uint64_t val) > return X86EMUL_EXCEPTION; > } > > - page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC); > + page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC); > > if ( !page || !get_page_type(page, PGT_writable_page) ) > { > @@ -817,13 +817,14 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, > uint64_t val) > > if ( p2m_is_paging(t) ) > { > - p2m_mem_paging_populate(d, gmfn); > + p2m_mem_paging_populate(d, gfn_x(gfn)); > return X86EMUL_RETRY; > } > > gdprintk(XENLOG_WARNING, > - "Bad GMFN %lx (MFN %#"PRI_mfn") to MSR %08x\n", > - gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN), > base); > + "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn") to MSR > %08x\n", > + gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : > INVALID_MFN), > + base); > return X86EMUL_EXCEPTION; > } > > diff --git a/xen/common/domain.c b/xen/common/domain.c > index d6650f0656..5e3c05b96c 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -1250,7 +1250,7 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, > unsigned offset) > if ( (v != current) && !(v->pause_flags & VPF_down) ) > return -EINVAL; > > - page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); > + page = get_page_from_gfn(d, _gfn(gfn), NULL, P2M_ALLOC); > if ( !page ) > return -EINVAL; > > diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c > index c49f446754..71a6f673b2 100644 > --- a/xen/common/event_fifo.c > +++ b/xen/common/event_fifo.c > @@ -358,7 +358,7 @@ static const struct evtchn_port_ops > evtchn_port_ops_fifo = > .print_state = evtchn_fifo_print_state, > }; > > -static int map_guest_page(struct domain *d, uint64_t gfn, void **virt) > +static int map_guest_page(struct domain *d, gfn_t gfn, void **virt) > { > struct page_info *p; > > @@ -419,7 +419,7 @@ static int setup_control_block(struct vcpu *v) > return 0; > } > > -static int map_control_block(struct vcpu *v, uint64_t gfn, uint32_t > offset) > +static int map_control_block(struct vcpu *v, gfn_t gfn, uint32_t offset) > { > void *virt; > unsigned int i; > @@ -505,7 +505,7 @@ int evtchn_fifo_init_control(struct > evtchn_init_control *init_control) > { > struct domain *d = current->domain; > uint32_t vcpu_id; > - uint64_t gfn; > + gfn_t gfn; > uint32_t offset; > struct vcpu *v; > int rc; > @@ -513,7 +513,7 @@ int evtchn_fifo_init_control(struct > evtchn_init_control *init_control) > init_control->link_bits = EVTCHN_FIFO_LINK_BITS; > > vcpu_id = init_control->vcpu; > - gfn = init_control->control_gfn; > + gfn = _gfn(init_control->control_gfn); > offset = init_control->offset; > > if ( vcpu_id >= d->max_vcpus || !d->vcpu[vcpu_id] ) > @@ -569,7 +569,7 @@ int evtchn_fifo_init_control(struct > evtchn_init_control *init_control) > return rc; > } > > -static int add_page_to_event_array(struct domain *d, unsigned long gfn) > +static int add_page_to_event_array(struct domain *d, gfn_t gfn) > { > void *virt; > unsigned int slot; > @@ -619,7 +619,7 @@ int evtchn_fifo_expand_array(const struct > evtchn_expand_array *expand_array) > return -EOPNOTSUPP; > > spin_lock(&d->event_lock); > - rc = add_page_to_event_array(d, expand_array->array_gfn); > + rc = add_page_to_event_array(d, _gfn(expand_array->array_gfn)); > spin_unlock(&d->event_lock); > > return rc; > diff --git a/xen/common/memory.c b/xen/common/memory.c > index 987395fbb3..e02733dba0 100644 > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -1365,7 +1365,7 @@ long do_memory_op(unsigned long cmd, > XEN_GUEST_HANDLE_PARAM(void) arg) > return rc; > } > > - page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC); > + page = get_page_from_gfn(d, _gfn(xrfp.gpfn), NULL, P2M_ALLOC); > if ( page ) > { > rc = guest_physmap_remove_page(d, _gfn(xrfp.gpfn), > @@ -1636,7 +1636,7 @@ int check_get_page_from_gfn(struct domain *d, gfn_t > gfn, bool readonly, > p2m_type_t p2mt; > struct page_info *page; > > - page = get_page_from_gfn(d, gfn_x(gfn), &p2mt, q); > + page = get_page_from_gfn(d, gfn, &p2mt, q); > > #ifdef CONFIG_HAS_MEM_PAGING > if ( p2m_is_paging(p2mt) ) > diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c > index bf7b14f79a..72cba7f10c 100644 > --- a/xen/common/tmem_xen.c > +++ b/xen/common/tmem_xen.c > @@ -52,7 +52,7 @@ static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t > *pcli_mfn, > p2m_type_t t; > struct page_info *page; > > - page = get_page_from_gfn(current->domain, cmfn, &t, P2M_ALLOC); > + page = get_page_from_gfn(current->domain, _gfn(cmfn), &t, P2M_ALLOC); > if ( !page || t != p2m_ram_rw ) > { > if ( page ) > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > index 7c67806056..5e598a0b37 100644 > --- a/xen/include/asm-arm/p2m.h > +++ b/xen/include/asm-arm/p2m.h > @@ -278,7 +278,7 @@ struct page_info *p2m_get_page_from_gfn(struct domain > *d, gfn_t gfn, > p2m_type_t *t); > > static inline struct page_info *get_page_from_gfn( > - struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) > + struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q) > { > mfn_t mfn; > p2m_type_t _t; > @@ -289,7 +289,7 @@ static inline struct page_info *get_page_from_gfn( > * not auto-translated. > */ > if ( unlikely(d != dom_xen) ) > - return p2m_get_page_from_gfn(d, _gfn(gfn), t); > + return p2m_get_page_from_gfn(d, gfn, t); > > if ( !t ) > t = &_t; > @@ -300,7 +300,7 @@ static inline struct page_info *get_page_from_gfn( > * DOMID_XEN see 1-1 RAM. The p2m_type is based on the type of the > * page. > */ > - mfn = _mfn(gfn); > + mfn = _mfn(gfn_x(gfn)); > page = mfn_to_page(mfn); > > if ( !mfn_valid(mfn) || !get_page(page, d) ) > diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h > index d08c595887..db1ec37610 100644 > --- a/xen/include/asm-x86/p2m.h > +++ b/xen/include/asm-x86/p2m.h > @@ -489,18 +489,21 @@ struct page_info *p2m_get_page_from_gfn(struct > p2m_domain *p2m, gfn_t gfn, > p2m_query_t q); > > static inline struct page_info *get_page_from_gfn( > - struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q) > + struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q) > { > struct page_info *page; > + mfn_t mfn; > > if ( paging_mode_translate(d) ) > - return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, > NULL, q); > + return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL, q); > > /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */ > if ( t ) > *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct; > - page = mfn_to_page(_mfn(gfn)); > - return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL; > + > + mfn = _mfn(gfn_x(gfn)); > + page = mfn_to_page(mfn); > + return mfn_valid(mfn) && get_page(page, d) ? page : NULL; > } > > /* General conversion function from mfn to gfn */ > -- > 2.11.0 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel [-- Attachment #1.2: Type: text/html, Size: 39928 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-12 16:49 ` Andrii Anisov @ 2018-11-12 16:52 ` Julien Grall 2018-11-12 16:58 ` Andrii Anisov 0 siblings, 1 reply; 44+ messages in thread From: Julien Grall @ 2018-11-12 16:52 UTC (permalink / raw) To: Andrii Anisov Cc: kevin.tian, Stefano Stabellini, Wei Liu, jun.nakajima, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim (Xen.org), julie.grall, Jan Beulich, Paul Durrant, suravee.suthikulpanit, xen-devel, Boris Ostrovsky, brian.woods On 11/12/18 4:49 PM, Andrii Anisov wrote: > Hello Julien, Hi, > > I'm just wondering if this patch really belongs to xentrace series. > It rather looks like a separate cleanup patch. What's wrong with including clean-up patch in a series adding a feature? 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] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-12 16:52 ` Julien Grall @ 2018-11-12 16:58 ` Andrii Anisov 2018-11-12 17:09 ` Julien Grall 0 siblings, 1 reply; 44+ messages in thread From: Andrii Anisov @ 2018-11-12 16:58 UTC (permalink / raw) To: Julien Grall Cc: kevin.tian, Stefano Stabellini, Wei Liu, jun.nakajima, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim (Xen.org), julie.grall, Jan Beulich, Paul Durrant, suravee.suthikulpanit, xen-devel, Boris Ostrovsky, brian.woods > What's wrong with including clean-up patch in a series adding a feature? I do not mean it is wrong. Just assuming that introducing a new feature and cleaning up a code might be different processes with a different review period. Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn 2018-11-12 16:58 ` Andrii Anisov @ 2018-11-12 17:09 ` Julien Grall 0 siblings, 0 replies; 44+ messages in thread From: Julien Grall @ 2018-11-12 17:09 UTC (permalink / raw) To: Andrii Anisov Cc: kevin.tian, Stefano Stabellini, Wei Liu, jun.nakajima, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim (Xen.org), Jan Beulich, Paul Durrant, suravee.suthikulpanit, xen-devel, Boris Ostrovsky, brian.woods Hi, On 11/12/18 4:58 PM, Andrii Anisov wrote: >> What's wrong with including clean-up patch in a series adding a feature? > > I do not mean it is wrong. > Just assuming that introducing a new feature and cleaning up a code > might be different processes with a different review period. We don't have different process nor different review period between clean-up and new feature. I tend to do clean-up when writing new features... See my cacheflush series as well. If the clean-up is small then I will append/prepend to the feature series. This patch modify a function that was called by this patch and therefore depends on the rest of the series. This was written with this series so it makes sense to me to do it together as it dictates the order I would like the patches applied and simplify tracking (I have many series in flight). 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] 44+ messages in thread
end of thread, other threads:[~2018-12-20 11:21 UTC | newest] Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-06 19:14 [PATCH 0/8] xen/arm: Add xentrace support Julien Grall 2018-11-06 19:14 ` [PATCH 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code Julien Grall 2018-11-07 9:28 ` Jan Beulich 2018-11-09 18:06 ` Julien Grall 2018-11-06 19:14 ` [PATCH 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn Julien Grall 2018-11-15 13:31 ` Andrii Anisov 2018-11-15 13:35 ` Andrii Anisov 2018-11-15 15:22 ` Julien Grall 2018-12-20 11:20 ` Andrii Anisov 2018-11-06 19:14 ` [PATCH 3/8] xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw Julien Grall 2018-11-15 11:42 ` Andrii Anisov 2018-11-15 12:07 ` Julien Grall 2018-11-06 19:14 ` [PATCH 4/8] xen/arm: Add support for read-only foreign mappings Julien Grall 2018-11-15 11:33 ` Andrii Anisov 2018-11-15 11:40 ` Julien Grall 2018-11-15 12:02 ` Andrii Anisov 2018-11-15 12:09 ` Julien Grall 2018-11-15 13:19 ` Andrii Anisov 2018-11-15 15:05 ` Julien Grall 2018-11-15 18:44 ` Stefano Stabellini 2018-11-15 19:06 ` Julien Grall 2018-11-15 19:48 ` Stefano Stabellini 2018-11-15 20:20 ` Julien Grall 2018-11-16 9:05 ` Andrii Anisov 2018-11-16 8:37 ` Andrii Anisov 2018-11-20 15:27 ` Andrii Anisov 2018-12-20 11:21 ` Andrii Anisov 2018-11-06 19:14 ` [PATCH 5/8] xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN Julien Grall 2018-11-15 13:45 ` Andrii Anisov 2018-11-06 19:14 ` [PATCH 6/8] xen/arm: Initialize trace buffer Julien Grall 2018-11-15 13:49 ` Andrii Anisov 2018-11-06 19:14 ` [PATCH 7/8] xenalyze: Build for Both ARM and x86 Platforms Julien Grall 2018-11-07 13:04 ` Wei Liu 2018-11-06 19:14 ` [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn Julien Grall 2018-11-06 20:11 ` Andrew Cooper 2018-11-07 8:57 ` Paul Durrant 2018-11-07 13:08 ` Andrew Cooper 2018-11-07 13:08 ` Julien Grall 2018-11-07 9:24 ` Paul Durrant 2018-11-07 13:05 ` Julien Grall 2018-11-12 16:49 ` Andrii Anisov 2018-11-12 16:52 ` Julien Grall 2018-11-12 16:58 ` Andrii Anisov 2018-11-12 17:09 ` Julien Grall
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.