|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [for-4.11][PATCH v7 13/16] xen/grant: Switch {create, replace}_grant_p2m_mapping to typesafe MFN
On Tue, 3 Apr 2018, Julien Grall wrote:
> The current prototype is slightly confusing because it takes a guest
> physical address and a machine physical frame (not address!). Switching to
> MFN will improve safety and reduce the chance to mistakenly invert the
> 2 parameters.
>
> Signed-off-by: Julien grall <julien.grall@xxxxxxx>
> Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>
> Changes in v5:
> - Add Wei's and Jan's reviewed-by
>
> Changes in v4:
> - Patch added
> ---
> xen/arch/arm/mm.c | 10 +++++-----
> xen/arch/x86/hvm/grant_table.c | 14 +++++++-------
> xen/arch/x86/pv/grant_table.c | 10 +++++-----
> xen/common/grant_table.c | 8 ++++----
> xen/include/asm-arm/grant_table.h | 9 ++++-----
> xen/include/asm-x86/grant_table.h | 4 ++--
> xen/include/asm-x86/hvm/grant_table.h | 8 ++++----
> xen/include/asm-x86/pv/grant_table.h | 8 ++++----
> 8 files changed, 35 insertions(+), 36 deletions(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 7af6baa3d6..49080ca0ac 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1418,7 +1418,7 @@ void gnttab_mark_dirty(struct domain *d, unsigned long
> l)
> }
> }
>
> -int create_grant_host_mapping(unsigned long addr, unsigned long frame,
> +int create_grant_host_mapping(unsigned long addr, mfn_t frame,
> unsigned int flags, unsigned int cache_flags)
> {
> int rc;
> @@ -1431,7 +1431,7 @@ int create_grant_host_mapping(unsigned long addr,
> unsigned long frame,
> t = p2m_grant_map_ro;
>
> rc = guest_physmap_add_entry(current->domain, gaddr_to_gfn(addr),
> - _mfn(frame), 0, t);
> + frame, 0, t);
>
> if ( rc )
> return GNTST_general_error;
> @@ -1439,8 +1439,8 @@ int create_grant_host_mapping(unsigned long addr,
> unsigned long frame,
> return GNTST_okay;
> }
>
> -int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
> - unsigned long new_addr, unsigned int flags)
> +int replace_grant_host_mapping(unsigned long addr, mfn_t mfn,
> + unsigned long new_addr, unsigned int flags)
> {
> gfn_t gfn = gaddr_to_gfn(addr);
> struct domain *d = current->domain;
> @@ -1449,7 +1449,7 @@ int replace_grant_host_mapping(unsigned long addr,
> unsigned long mfn,
> if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
> return GNTST_general_error;
>
> - rc = guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
> + rc = guest_physmap_remove_page(d, gfn, mfn, 0);
>
> return rc ? GNTST_general_error : GNTST_okay;
> }
> diff --git a/xen/arch/x86/hvm/grant_table.c b/xen/arch/x86/hvm/grant_table.c
> index 9ca9fe0425..ecd7d078ab 100644
> --- a/xen/arch/x86/hvm/grant_table.c
> +++ b/xen/arch/x86/hvm/grant_table.c
> @@ -25,7 +25,7 @@
>
> #include <asm/p2m.h>
>
> -int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
> unsigned int flags,
> unsigned int cache_flags)
> {
> @@ -41,14 +41,14 @@ int create_grant_p2m_mapping(uint64_t addr, unsigned long
> frame,
> p2mt = p2m_grant_map_rw;
> rc = guest_physmap_add_entry(current->domain,
> _gfn(addr >> PAGE_SHIFT),
> - _mfn(frame), PAGE_ORDER_4K, p2mt);
> + frame, PAGE_ORDER_4K, p2mt);
> if ( rc )
> return GNTST_general_error;
> else
> return GNTST_okay;
> }
>
> -int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
> uint64_t new_addr, unsigned int flags)
> {
> unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
> @@ -60,15 +60,15 @@ int replace_grant_p2m_mapping(uint64_t addr, unsigned
> long frame,
> return GNTST_general_error;
>
> old_mfn = get_gfn(d, gfn, &type);
> - if ( !p2m_is_grant(type) || mfn_x(old_mfn) != frame )
> + if ( !p2m_is_grant(type) || !mfn_eq(old_mfn, frame) )
> {
> put_gfn(d, gfn);
> gdprintk(XENLOG_WARNING,
> - "old mapping invalid (type %d, mfn %" PRI_mfn ", frame
> %lx)\n",
> - type, mfn_x(old_mfn), frame);
> + "old mapping invalid (type %d, mfn %" PRI_mfn ", frame
> %"PRI_mfn")\n",
> + type, mfn_x(old_mfn), mfn_x(frame));
> return GNTST_general_error;
> }
> - if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K)
> )
> + if ( guest_physmap_remove_page(d, _gfn(gfn), frame, PAGE_ORDER_4K) )
> {
> put_gfn(d, gfn);
> return GNTST_general_error;
> diff --git a/xen/arch/x86/pv/grant_table.c b/xen/arch/x86/pv/grant_table.c
> index 4dbc550366..458085e1b6 100644
> --- a/xen/arch/x86/pv/grant_table.c
> +++ b/xen/arch/x86/pv/grant_table.c
> @@ -50,7 +50,7 @@ static unsigned int grant_to_pte_flags(unsigned int
> grant_flags,
> return pte_flags;
> }
>
> -int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
> unsigned int flags, unsigned int cache_flags)
> {
> struct vcpu *curr = current;
> @@ -60,7 +60,7 @@ int create_grant_pv_mapping(uint64_t addr, unsigned long
> frame,
> mfn_t gl1mfn;
> int rc = GNTST_general_error;
>
> - nl1e = l1e_from_pfn(frame, grant_to_pte_flags(flags, cache_flags));
> + nl1e = l1e_from_mfn(frame, grant_to_pte_flags(flags, cache_flags));
> nl1e = adjust_guest_l1e(nl1e, currd);
>
> /*
> @@ -192,7 +192,7 @@ static bool steal_linear_address(unsigned long linear,
> l1_pgentry_t *out)
> * new_addr has only ever been available via GNTABOP_unmap_and_replace, and
> * only when !(flags & GNTMAP_contains_pte).
> */
> -int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
> uint64_t new_addr, unsigned int flags)
> {
> struct vcpu *curr = current;
> @@ -282,14 +282,14 @@ int replace_grant_pv_mapping(uint64_t addr, unsigned
> long frame,
> * Check that the address supplied is actually mapped to frame (with
> * appropriate permissions).
> */
> - if ( unlikely(l1e_get_pfn(ol1e) != frame) ||
> + if ( unlikely(!mfn_eq(l1e_get_mfn(ol1e), frame)) ||
> unlikely((l1e_get_flags(ol1e) ^ grant_pte_flags) &
> (_PAGE_PRESENT | _PAGE_RW)) )
> {
> gdprintk(XENLOG_ERR,
> "PTE %"PRIpte" for %"PRIx64" doesn't match grant
> (%"PRIpte")\n",
> l1e_get_intpte(ol1e), addr,
> - l1e_get_intpte(l1e_from_pfn(frame, grant_pte_flags)));
> + l1e_get_intpte(l1e_from_mfn(frame, grant_pte_flags)));
> goto out_unlock;
> }
>
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 18201912e4..f9e3d1bb95 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1071,7 +1071,7 @@ map_grant_ref(
>
> if ( op->flags & GNTMAP_host_map )
> {
> - rc = create_grant_host_mapping(op->host_addr, frame, op->flags,
> + rc = create_grant_host_mapping(op->host_addr, _mfn(frame),
> op->flags,
> cache_flags);
> if ( rc != GNTST_okay )
> goto undo_out;
> @@ -1111,7 +1111,7 @@ map_grant_ref(
> typecnt++;
> }
>
> - rc = create_grant_host_mapping(op->host_addr, frame, op->flags,
> 0);
> + rc = create_grant_host_mapping(op->host_addr, _mfn(frame),
> op->flags, 0);
> if ( rc != GNTST_okay )
> goto undo_out;
>
> @@ -1188,7 +1188,7 @@ map_grant_ref(
> undo_out:
> if ( host_map_created )
> {
> - replace_grant_host_mapping(op->host_addr, frame, 0, op->flags);
> + replace_grant_host_mapping(op->host_addr, _mfn(frame), 0, op->flags);
> gnttab_flush_tlb(ld);
> }
>
> @@ -1374,7 +1374,7 @@ unmap_common(
> if ( op->host_addr && (flags & GNTMAP_host_map) )
> {
> if ( (rc = replace_grant_host_mapping(op->host_addr,
> - op->frame, op->new_addr,
> + _mfn(op->frame), op->new_addr,
> flags)) < 0 )
> goto act_release_out;
>
> diff --git a/xen/include/asm-arm/grant_table.h
> b/xen/include/asm-arm/grant_table.h
> index d2027d26b2..24644084a1 100644
> --- a/xen/include/asm-arm/grant_table.h
> +++ b/xen/include/asm-arm/grant_table.h
> @@ -14,12 +14,11 @@ struct grant_table_arch {
> };
>
> void gnttab_clear_flag(unsigned long nr, uint16_t *addr);
> -int create_grant_host_mapping(unsigned long gpaddr,
> - unsigned long mfn, unsigned int flags, unsigned int
> - cache_flags);
> +int create_grant_host_mapping(unsigned long gpaddr, mfn_t mfn,
> + unsigned int flags, unsigned int cache_flags);
> #define gnttab_host_mapping_get_page_type(ro, ld, rd) (0)
> -int replace_grant_host_mapping(unsigned long gpaddr, unsigned long mfn,
> - unsigned long new_gpaddr, unsigned int flags);
> +int replace_grant_host_mapping(unsigned long gpaddr, mfn_t mfn,
> + unsigned long new_gpaddr, unsigned int flags);
> void gnttab_mark_dirty(struct domain *d, unsigned long l);
> #define gnttab_create_status_page(d, t, i) do {} while (0)
> #define gnttab_release_host_mappings(domain) 1
> diff --git a/xen/include/asm-x86/grant_table.h
> b/xen/include/asm-x86/grant_table.h
> index 4ac0b9b4c7..fc07291ff2 100644
> --- a/xen/include/asm-x86/grant_table.h
> +++ b/xen/include/asm-x86/grant_table.h
> @@ -21,7 +21,7 @@ struct grant_table_arch {
> * Caller must own caller's BIGLOCK, is responsible for flushing the TLB, and
> * must hold a reference to the page.
> */
> -static inline int create_grant_host_mapping(uint64_t addr, unsigned long
> frame,
> +static inline int create_grant_host_mapping(uint64_t addr, mfn_t frame,
> unsigned int flags,
> unsigned int cache_flags)
> {
> @@ -30,7 +30,7 @@ static inline int create_grant_host_mapping(uint64_t addr,
> unsigned long frame,
> return create_grant_pv_mapping(addr, frame, flags, cache_flags);
> }
>
> -static inline int replace_grant_host_mapping(uint64_t addr, unsigned long
> frame,
> +static inline int replace_grant_host_mapping(uint64_t addr, mfn_t frame,
> uint64_t new_addr,
> unsigned int flags)
> {
> diff --git a/xen/include/asm-x86/hvm/grant_table.h
> b/xen/include/asm-x86/hvm/grant_table.h
> index 711ce9b560..a5612585b3 100644
> --- a/xen/include/asm-x86/hvm/grant_table.h
> +++ b/xen/include/asm-x86/hvm/grant_table.h
> @@ -23,24 +23,24 @@
>
> #ifdef CONFIG_HVM
>
> -int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
> unsigned int flags,
> unsigned int cache_flags);
> -int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
> uint64_t new_addr, unsigned int flags);
>
> #else
>
> #include <public/grant_table.h>
>
> -static inline int create_grant_p2m_mapping(uint64_t addr, unsigned long
> frame,
> +static inline int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
> unsigned int flags,
> unsigned int cache_flags)
> {
> return GNTST_general_error;
> }
>
> -static inline int replace_grant_p2m_mapping(uint64_t addr, unsigned long
> frame,
> +static inline int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
> uint64_t new_addr, unsigned int
> flags)
> {
> return GNTST_general_error;
> diff --git a/xen/include/asm-x86/pv/grant_table.h
> b/xen/include/asm-x86/pv/grant_table.h
> index 556e68f0eb..85442b6074 100644
> --- a/xen/include/asm-x86/pv/grant_table.h
> +++ b/xen/include/asm-x86/pv/grant_table.h
> @@ -23,23 +23,23 @@
>
> #ifdef CONFIG_PV
>
> -int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
> unsigned int flags, unsigned int cache_flags);
> -int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
> uint64_t new_addr, unsigned int flags);
>
> #else
>
> #include <public/grant_table.h>
>
> -static inline int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +static inline int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
> unsigned int flags,
> unsigned int cache_flags)
> {
> return GNTST_general_error;
> }
>
> -static inline int replace_grant_pv_mapping(uint64_t addr, unsigned long
> frame,
> +static inline int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
> uint64_t new_addr, unsigned int
> flags)
> {
> return GNTST_general_error;
> --
> 2.11.0
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |