[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v5 14/16] xen/grant: Switch common/grant_table.c to use typesafe MFN
From: Julien Grall <julien.grall@xxxxxxx> At the same time replace _mfn(0) by INVALID_MFN or drop the initializer when it is not necessary. This will make clearer that the MFN initialized is not valid. Other than _mfn(0) -> INVALID_MFN, no functional change intended. Signed-off-by: Julien Grall <julien.grall@xxxxxxx> --- 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: - Remove _mfn(0) when not needed or replace by INVALID_MFN. Changes in v4: - Patch added --- xen/arch/arm/mm.c | 2 +- xen/common/grant_table.c | 147 ++++++++++++++++++++------------------ xen/include/asm-arm/grant_table.h | 2 +- xen/include/asm-x86/grant_table.h | 2 +- 4 files changed, 82 insertions(+), 71 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 4268dd5c2d..db74466a16 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1413,7 +1413,7 @@ void gnttab_clear_flag(unsigned long nr, uint16_t *addr) } while (cmpxchg(addr, old, old & mask) != old); } -void gnttab_mark_dirty(struct domain *d, unsigned long l) +void gnttab_mark_dirty(struct domain *d, mfn_t mfn) { /* XXX: mark dirty */ static int warning; diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index f9e3d1bb95..4bedf5984a 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -40,6 +40,12 @@ #include <xsm/xsm.h> #include <asm/flushtlb.h> +/* Override macros from asm/page.h to make them work with mfn_t */ +#undef page_to_mfn +#define page_to_mfn(pg) _mfn(__page_to_mfn(pg)) +#undef mfn_to_page +#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn)) + /* Per-domain grant information. */ struct grant_table { /* @@ -167,7 +173,7 @@ struct gnttab_unmap_common { /* Shared state beteen *_unmap and *_unmap_complete */ uint16_t done; - unsigned long frame; + mfn_t frame; struct domain *rd; grant_ref_t ref; }; @@ -266,7 +272,7 @@ struct active_grant_entry { grant. */ grant_ref_t trans_gref; struct domain *trans_domain; - unsigned long frame; /* Frame being granted. */ + mfn_t frame; /* Frame being granted. */ #ifndef NDEBUG gfn_t gfn; /* Guest's idea of the frame being granted. */ #endif @@ -371,14 +377,14 @@ static inline unsigned int grant_to_status_frames(unsigned int grant_frames) If rc == GNTST_okay, *page contains the page struct with a ref taken. Caller must do put_page(*page). If any error, *page = NULL, *frame = INVALID_MFN, no ref taken. */ -static int get_paged_frame(unsigned long gfn, unsigned long *frame, +static int get_paged_frame(unsigned long gfn, mfn_t *frame, struct page_info **page, bool readonly, struct domain *rd) { int rc = GNTST_okay; p2m_type_t p2mt; - *frame = mfn_x(INVALID_MFN); + *frame = INVALID_MFN; *page = get_page_from_gfn(rd, gfn, &p2mt, readonly ? P2M_ALLOC : P2M_UNSHARE); if ( !*page ) @@ -823,7 +829,7 @@ static int _set_status(unsigned gt_version, static struct active_grant_entry *grant_map_exists(const struct domain *ld, struct grant_table *rgt, - unsigned long mfn, + mfn_t mfn, grant_ref_t *cur_ref) { grant_ref_t ref, max_iter; @@ -842,7 +848,8 @@ static struct active_grant_entry *grant_map_exists(const struct domain *ld, { struct active_grant_entry *act = active_entry_acquire(rgt, ref); - if ( act->pin && act->domid == ld->domain_id && act->frame == mfn ) + if ( act->pin && act->domid == ld->domain_id && + mfn_eq(act->frame, mfn) ) return act; active_entry_release(act); } @@ -859,7 +866,7 @@ static struct active_grant_entry *grant_map_exists(const struct domain *ld, #define MAPKIND_READ 1 #define MAPKIND_WRITE 2 static unsigned int mapkind( - struct grant_table *lgt, const struct domain *rd, unsigned long mfn) + struct grant_table *lgt, const struct domain *rd, mfn_t mfn) { struct grant_mapping *map; grant_handle_t handle, limit = lgt->maptrack_limit; @@ -884,7 +891,7 @@ static unsigned int mapkind( if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) || map->domid != rd->domain_id ) continue; - if ( _active_entry(rd->grant_table, map->ref).frame == mfn ) + if ( mfn_eq(_active_entry(rd->grant_table, map->ref).frame, mfn) ) kind |= map->flags & GNTMAP_readonly ? MAPKIND_READ : MAPKIND_WRITE; } @@ -907,7 +914,7 @@ map_grant_ref( struct grant_table *lgt, *rgt; struct vcpu *led; grant_handle_t handle; - unsigned long frame = 0; + mfn_t frame; struct page_info *pg = NULL; int rc = GNTST_okay; u32 old_pin; @@ -1034,7 +1041,7 @@ map_grant_ref( /* pg may be set, with a refcount included, from get_paged_frame(). */ if ( !pg ) { - pg = mfn_valid(_mfn(frame)) ? mfn_to_page(frame) : NULL; + pg = mfn_valid(frame) ? mfn_to_page(frame) : NULL; if ( pg ) owner = page_get_owner_and_reference(pg); } @@ -1060,18 +1067,18 @@ map_grant_ref( goto undo_out; } - if ( !iomem_access_permitted(rd, frame, frame) ) + if ( !iomem_access_permitted(rd, mfn_x(frame), mfn_x(frame)) ) { gdprintk(XENLOG_WARNING, - "Iomem mapping not permitted %lx (domain %d)\n", - frame, rd->domain_id); + "Iomem mapping not permitted %#"PRI_mfn" (domain %d)\n", + mfn_x(frame), rd->domain_id); rc = GNTST_general_error; goto undo_out; } if ( op->flags & GNTMAP_host_map ) { - rc = create_grant_host_mapping(op->host_addr, _mfn(frame), op->flags, + rc = create_grant_host_mapping(op->host_addr, frame, op->flags, cache_flags); if ( rc != GNTST_okay ) goto undo_out; @@ -1111,7 +1118,7 @@ map_grant_ref( typecnt++; } - rc = create_grant_host_mapping(op->host_addr, _mfn(frame), op->flags, 0); + rc = create_grant_host_mapping(op->host_addr, frame, op->flags, 0); if ( rc != GNTST_okay ) goto undo_out; @@ -1122,8 +1129,8 @@ map_grant_ref( { could_not_pin: if ( !rd->is_dying ) - gdprintk(XENLOG_WARNING, "Could not pin grant frame %lx\n", - frame); + gdprintk(XENLOG_WARNING, "Could not pin grant frame %#"PRI_mfn"\n", + mfn_x(frame)); rc = GNTST_general_error; goto undo_out; } @@ -1143,13 +1150,14 @@ map_grant_ref( !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) ) { if ( !(kind & MAPKIND_WRITE) ) - err = iommu_map_page(ld, frame, frame, + err = iommu_map_page(ld, mfn_x(frame), mfn_x(frame), IOMMUF_readable|IOMMUF_writable); } else if ( act_pin && !old_pin ) { if ( !kind ) - err = iommu_map_page(ld, frame, frame, IOMMUF_readable); + err = iommu_map_page(ld, mfn_x(frame), mfn_x(frame), + IOMMUF_readable); } if ( err ) { @@ -1178,7 +1186,7 @@ map_grant_ref( if ( need_iommu ) double_gt_unlock(lgt, rgt); - op->dev_bus_addr = (u64)frame << PAGE_SHIFT; + op->dev_bus_addr = mfn_to_maddr(frame); op->handle = handle; op->status = GNTST_okay; @@ -1188,7 +1196,7 @@ map_grant_ref( undo_out: if ( host_map_created ) { - replace_grant_host_mapping(op->host_addr, _mfn(frame), 0, op->flags); + replace_grant_host_mapping(op->host_addr, frame, 0, op->flags); gnttab_flush_tlb(ld); } @@ -1366,15 +1374,15 @@ unmap_common( op->frame = act->frame; if ( op->dev_bus_addr && - unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) ) + unlikely(op->dev_bus_addr != mfn_to_maddr(act->frame)) ) PIN_FAIL(act_release_out, GNTST_general_error, "Bus address doesn't match gntref (%"PRIx64" != %"PRIpaddr")\n", - op->dev_bus_addr, pfn_to_paddr(act->frame)); + op->dev_bus_addr, mfn_to_maddr(act->frame)); if ( op->host_addr && (flags & GNTMAP_host_map) ) { if ( (rc = replace_grant_host_mapping(op->host_addr, - _mfn(op->frame), op->new_addr, + op->frame, op->new_addr, flags)) < 0 ) goto act_release_out; @@ -1411,9 +1419,10 @@ unmap_common( kind = mapkind(lgt, rd, op->frame); if ( !kind ) - err = iommu_unmap_page(ld, op->frame); + err = iommu_unmap_page(ld, mfn_x(op->frame)); else if ( !(kind & MAPKIND_WRITE) ) - err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); + err = iommu_map_page(ld, mfn_x(op->frame), + mfn_x(op->frame), IOMMUF_readable); double_gt_unlock(lgt, rgt); @@ -1464,7 +1473,7 @@ unmap_common_complete(struct gnttab_unmap_common *op) if ( op->done & GNTMAP_device_map ) { - if ( !is_iomem_page(_mfn(act->frame)) ) + if ( !is_iomem_page(act->frame) ) { if ( op->done & GNTMAP_readonly ) put_page(pg); @@ -1481,7 +1490,7 @@ unmap_common_complete(struct gnttab_unmap_common *op) if ( op->done & GNTMAP_host_map ) { - if ( !is_iomem_page(_mfn(op->frame)) ) + if ( !is_iomem_page(op->frame) ) { if ( gnttab_host_mapping_get_page_type(op->done & GNTMAP_readonly, ld, rd) ) @@ -1522,7 +1531,7 @@ unmap_grant_ref( common->done = 0; common->new_addr = 0; common->rd = NULL; - common->frame = 0; + common->frame = INVALID_MFN; unmap_common(common); op->status = common->status; @@ -1588,7 +1597,7 @@ unmap_and_replace( common->done = 0; common->dev_bus_addr = 0; common->rd = NULL; - common->frame = 0; + common->frame = INVALID_MFN; unmap_common(common); op->status = common->status; @@ -1692,7 +1701,7 @@ gnttab_unpopulate_status_frames(struct domain *d, struct grant_table *gt) int rc = gfn_eq(gfn, INVALID_GFN) ? 0 : guest_physmap_remove_page(d, gfn, - _mfn(page_to_mfn(pg)), 0); + page_to_mfn(pg), 0); if ( rc ) { @@ -2097,7 +2106,7 @@ gnttab_transfer( struct page_info *page; int i; struct gnttab_transfer gop; - unsigned long mfn; + mfn_t mfn; unsigned int max_bitsize; struct active_grant_entry *act; @@ -2121,16 +2130,16 @@ gnttab_transfer( { p2m_type_t p2mt; - mfn = mfn_x(get_gfn_unshare(d, gop.mfn, &p2mt)); + mfn = get_gfn_unshare(d, gop.mfn, &p2mt); if ( p2m_is_shared(p2mt) || !p2m_is_valid(p2mt) ) - mfn = mfn_x(INVALID_MFN); + mfn = INVALID_MFN; } #else - mfn = mfn_x(gfn_to_mfn(d, _gfn(gop.mfn))); + mfn = gfn_to_mfn(d, _gfn(gop.mfn)); #endif /* Check the passed page frame for basic validity. */ - if ( unlikely(!mfn_valid(_mfn(mfn))) ) + if ( unlikely(!mfn_valid(mfn)) ) { put_gfn(d, gop.mfn); gdprintk(XENLOG_INFO, "out-of-range %lx\n", (unsigned long)gop.mfn); @@ -2146,12 +2155,13 @@ gnttab_transfer( goto copyback; } - rc = guest_physmap_remove_page(d, _gfn(gop.mfn), _mfn(mfn), 0); + rc = guest_physmap_remove_page(d, _gfn(gop.mfn), mfn, 0); gnttab_flush_tlb(d); if ( rc ) { - gdprintk(XENLOG_INFO, "can't remove GFN %"PRI_xen_pfn" (MFN %lx)\n", - gop.mfn, mfn); + gdprintk(XENLOG_INFO, + "can't remove GFN %"PRI_xen_pfn" (MFN %#"PRI_mfn")\n", + gop.mfn, mfn_x(mfn)); gop.status = GNTST_general_error; goto put_gfn_and_copyback; } @@ -2180,7 +2190,7 @@ gnttab_transfer( e, e->grant_table->gt_version > 1 || paging_mode_translate(e) ? BITS_PER_LONG + PAGE_SHIFT : 32 + PAGE_SHIFT); if ( max_bitsize < BITS_PER_LONG + PAGE_SHIFT && - (mfn >> (max_bitsize - PAGE_SHIFT)) ) + (mfn_x(mfn) >> (max_bitsize - PAGE_SHIFT)) ) { struct page_info *new_page; @@ -2192,7 +2202,7 @@ gnttab_transfer( goto unlock_and_copyback; } - copy_domain_page(_mfn(page_to_mfn(new_page)), _mfn(mfn)); + copy_domain_page(page_to_mfn(new_page), mfn); page->count_info &= ~(PGC_count_mask|PGC_allocated); free_domheap_page(page); @@ -2269,18 +2279,17 @@ gnttab_transfer( { grant_entry_v1_t *sha = &shared_entry_v1(e->grant_table, gop.ref); - guest_physmap_add_page(e, _gfn(sha->frame), _mfn(mfn), 0); + guest_physmap_add_page(e, _gfn(sha->frame), mfn, 0); if ( !paging_mode_translate(e) ) - sha->frame = mfn; + sha->frame = mfn_x(mfn); } else { grant_entry_v2_t *sha = &shared_entry_v2(e->grant_table, gop.ref); - guest_physmap_add_page(e, _gfn(sha->full_page.frame), - _mfn(mfn), 0); + guest_physmap_add_page(e, _gfn(sha->full_page.frame), mfn, 0); if ( !paging_mode_translate(e) ) - sha->full_page.frame = mfn; + sha->full_page.frame = mfn_x(mfn); } smp_wmb(); shared_entry_header(e->grant_table, gop.ref)->flags |= @@ -2316,7 +2325,7 @@ release_grant_for_copy( struct grant_table *rgt = rd->grant_table; grant_entry_header_t *sha; struct active_grant_entry *act; - unsigned long r_frame; + mfn_t r_frame; uint16_t *status; grant_ref_t trans_gref; struct domain *td; @@ -2393,7 +2402,7 @@ static void fixup_status_for_copy_pin(const struct active_grant_entry *act, static int acquire_grant_for_copy( struct domain *rd, grant_ref_t gref, domid_t ldom, bool readonly, - unsigned long *frame, struct page_info **page, + mfn_t *frame, struct page_info **page, uint16_t *page_off, uint16_t *length, bool allow_transitive) { struct grant_table *rgt = rd->grant_table; @@ -2405,7 +2414,7 @@ acquire_grant_for_copy( domid_t trans_domid; grant_ref_t trans_gref; struct domain *td; - unsigned long grant_frame; + mfn_t grant_frame; uint16_t trans_page_off; uint16_t trans_length; bool is_sub_page; @@ -2506,7 +2515,8 @@ acquire_grant_for_copy( */ if ( rgt->gt_version != 2 || act->pin != old_pin || - (old_pin && (act->domid != ldom || act->frame != grant_frame || + (old_pin && (act->domid != ldom || + !mfn_eq(act->frame, grant_frame) || act->start != trans_page_off || act->length != trans_length || act->trans_domain != td || @@ -2598,7 +2608,7 @@ acquire_grant_for_copy( } else { - ASSERT(mfn_valid(_mfn(act->frame))); + ASSERT(mfn_valid(act->frame)); *page = mfn_to_page(act->frame); td = page_get_owner_and_reference(*page); /* @@ -2653,7 +2663,7 @@ struct gnttab_copy_buf { /* Mapped etc. */ struct domain *domain; - unsigned long frame; + mfn_t frame; struct page_info *page; void *virt; bool_t read_only; @@ -2785,15 +2795,16 @@ static int gnttab_copy_claim_buf(const struct gnttab_copy *op, if ( !get_page_type(buf->page, PGT_writable_page) ) { if ( !buf->domain->is_dying ) - gdprintk(XENLOG_WARNING, "Could not get writable frame %lx\n", - buf->frame); + gdprintk(XENLOG_WARNING, + "Could not get writable frame %#"PRI_mfn"\n", + mfn_x(buf->frame)); rc = GNTST_general_error; goto out; } buf->have_type = 1; } - buf->virt = map_domain_page(_mfn(buf->frame)); + buf->virt = map_domain_page(buf->frame); rc = GNTST_okay; out: @@ -3296,7 +3307,7 @@ static int cache_flush(const gnttab_cache_flush_t *cflush, grant_ref_t *cur_ref) { struct domain *d, *owner; struct page_info *page; - unsigned long mfn; + mfn_t mfn; struct active_grant_entry *act = NULL; void *v; int ret; @@ -3315,9 +3326,9 @@ static int cache_flush(const gnttab_cache_flush_t *cflush, grant_ref_t *cur_ref) return -EOPNOTSUPP; d = rcu_lock_current_domain(); - mfn = cflush->a.dev_bus_addr >> PAGE_SHIFT; + mfn = maddr_to_mfn(cflush->a.dev_bus_addr); - if ( !mfn_valid(_mfn(mfn)) ) + if ( !mfn_valid(mfn) ) { rcu_unlock_domain(d); return -EINVAL; @@ -3345,7 +3356,7 @@ static int cache_flush(const gnttab_cache_flush_t *cflush, grant_ref_t *cur_ref) } } - v = map_domain_page(_mfn(mfn)); + v = map_domain_page(mfn); v += cflush->offset; if ( (cflush->op & GNTTAB_CACHE_INVAL) && (cflush->op & GNTTAB_CACHE_CLEAN) ) @@ -3663,7 +3674,7 @@ gnttab_release_mappings( { BUG_ON(!(act->pin & GNTPIN_devr_mask)); act->pin -= GNTPIN_devr_inc; - if ( !is_iomem_page(_mfn(act->frame)) ) + if ( !is_iomem_page(act->frame) ) put_page(pg); } @@ -3672,7 +3683,7 @@ gnttab_release_mappings( BUG_ON(!(act->pin & GNTPIN_hstr_mask)); act->pin -= GNTPIN_hstr_inc; if ( gnttab_release_host_mappings(d) && - !is_iomem_page(_mfn(act->frame)) ) + !is_iomem_page(act->frame) ) put_page(pg); } } @@ -3682,7 +3693,7 @@ gnttab_release_mappings( { BUG_ON(!(act->pin & GNTPIN_devw_mask)); act->pin -= GNTPIN_devw_inc; - if ( !is_iomem_page(_mfn(act->frame)) ) + if ( !is_iomem_page(act->frame) ) put_page_and_type(pg); } @@ -3691,7 +3702,7 @@ gnttab_release_mappings( BUG_ON(!(act->pin & GNTPIN_hstw_mask)); act->pin -= GNTPIN_hstw_inc; if ( gnttab_release_host_mappings(d) && - !is_iomem_page(_mfn(act->frame)) ) + !is_iomem_page(act->frame) ) { if ( gnttab_host_mapping_get_page_type((map->flags & GNTMAP_readonly), @@ -3743,12 +3754,12 @@ void grant_table_warn_active_grants(struct domain *d) #ifndef NDEBUG "GFN %lx, " #endif - "MFN: %lx)\n", + "MFN: %#"PRI_mfn")\n", d->domain_id, ref, #ifndef NDEBUG gfn_x(act->gfn), #endif - act->frame); + mfn_x(act->frame)); active_entry_release(act); } @@ -3955,9 +3966,9 @@ static void gnttab_usage_print(struct domain *rd) first = 0; - /* [0xXXX] ddddd 0xXXXXXX 0xXXXXXXXX ddddd 0xXXXXXX 0xXX */ - printk("[0x%03x] %5d 0x%06lx 0x%08x %5d 0x%06"PRIx64" 0x%02x\n", - ref, act->domid, act->frame, act->pin, + /* [0xXXX] ddddd 0xXXXXX 0xXXXXXXXX ddddd 0xXXXXXX 0xXX */ + printk("[0x%03x] %5d 0x%"PRI_mfn" 0x%08x %5d 0x%06"PRIx64" 0x%02x\n", + ref, act->domid, mfn_x(act->frame), act->pin, sha->domid, frame, status); active_entry_release(act); } diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h index b05e5ca22d..9b365498b1 100644 --- a/xen/include/asm-arm/grant_table.h +++ b/xen/include/asm-arm/grant_table.h @@ -19,7 +19,7 @@ int create_grant_host_mapping(unsigned long gpaddr, mfn_t mfn, #define gnttab_host_mapping_get_page_type(ro, ld, rd) (0) 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); +void gnttab_mark_dirty(struct domain *d, mfn_t mfn); #define gnttab_create_status_page(d, t, i) do {} while (0) #define gnttab_release_host_mappings(domain) 1 static inline int replace_grant_supported(void) diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h index 05a457b7eb..a4375297ef 100644 --- a/xen/include/asm-x86/grant_table.h +++ b/xen/include/asm-x86/grant_table.h @@ -82,7 +82,7 @@ static inline unsigned int gnttab_dom0_max(void) #define gnttab_status_gmfn(d, t, i) \ (mfn_to_gmfn(d, gnttab_status_mfn(t, i))) -#define gnttab_mark_dirty(d, f) paging_mark_dirty((d), _mfn(f)) +#define gnttab_mark_dirty(d, f) paging_mark_dirty((d), f) static inline void gnttab_clear_flag(unsigned int nr, uint16_t *st) { -- 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 |