Kieran Mansley
2007-May-22 09:24 UTC
[Xen-devel] RFC (take 2): Using grant table to give iomem permission
This is a follow-up to an RFC posted last week with some modifications based on Keir''s suggestions. I''d still regard this as an RFC rather than patch for submission, as it''s not been widely tested (i.e. it seems to work OK for me), but would welcome others'' views on the approach. diff -r 039a10cef2f7 xen/arch/x86/mm.c --- a/xen/arch/x86/mm.c Fri May 18 11:03:05 2007 +0100 +++ b/xen/arch/x86/mm.c Mon May 21 14:14:47 2007 +0100 @@ -581,6 +581,14 @@ get_##level##_linear_pagetable( return 1; \ } + +int iomem_page_test(unsigned long mfn, struct page_info *page) +{ + return unlikely(!mfn_valid(mfn)) || + unlikely(page_get_owner(page) == dom_io); +} + + int get_page_from_l1e( l1_pgentry_t l1e, struct domain *d) @@ -598,8 +606,7 @@ get_page_from_l1e( return 0; } - if ( unlikely(!mfn_valid(mfn)) || - unlikely(page_get_owner(page) == dom_io) ) + if ( iomem_page_test(mfn, page) ) { /* DOMID_IO reverts to caller for privilege checks. */ if ( d == dom_io ) diff -r 039a10cef2f7 xen/common/grant_table.c --- a/xen/common/grant_table.c Fri May 18 11:03:05 2007 +0100 +++ b/xen/common/grant_table.c Tue May 22 10:15:58 2007 +0100 @@ -177,6 +177,7 @@ __gnttab_map_grant_ref( int handle; unsigned long frame = 0; int rc = GNTST_okay; + int is_iomem = 0; struct active_grant_entry *act; struct grant_mapping *mt; grant_entry_t *sha; @@ -305,34 +306,52 @@ __gnttab_map_grant_ref( spin_unlock(&rd->grant_table->lock); - if ( unlikely(!mfn_valid(frame)) || - unlikely(!((op->flags & GNTMAP_readonly) ? - get_page(mfn_to_page(frame), rd) : - get_page_and_type(mfn_to_page(frame), rd, - PGT_writable_page))) ) - { - if ( !rd->is_dying ) - gdprintk(XENLOG_WARNING, "Could not pin grant frame %lx\n", frame); - rc = GNTST_general_error; - goto undo_out; - } - - if ( op->flags & GNTMAP_host_map ) - { - rc = create_grant_host_mapping(op->host_addr, frame, op->flags); - if ( rc != GNTST_okay ) - { - if ( !(op->flags & GNTMAP_readonly) ) - put_page_type(mfn_to_page(frame)); - put_page(mfn_to_page(frame)); + if ( op->flags & GNTMAP_host_map ) + { + /* Could be an iomem page for setting up permission */ + if( iomem_page_test(frame, mfn_to_page(frame)) ) { + is_iomem = 1; + if ( iomem_permit_access(ld, frame, frame) ) { + gdprintk(XENLOG_WARNING, + "Could not permit access to grant frame %lx as iomem\n", + frame); + rc = GNTST_general_error; + goto undo_out; + } + } + } + + if (!is_iomem ) + { + if ( unlikely(!mfn_valid(frame)) || + unlikely(!((op->flags & GNTMAP_readonly) ? + get_page(mfn_to_page(frame), rd) : + get_page_and_type(mfn_to_page(frame), rd, + PGT_writable_page)))) + { + if ( !rd->is_dying ) + gdprintk(XENLOG_WARNING, "Could not pin grant frame %lx\n", frame); + rc = GNTST_general_error; goto undo_out; } - - if ( op->flags & GNTMAP_device_map ) - { - (void)get_page(mfn_to_page(frame), rd); - if ( !(op->flags & GNTMAP_readonly) ) - get_page_type(mfn_to_page(frame), PGT_writable_page); + + if ( op->flags & GNTMAP_host_map ) + { + rc = create_grant_host_mapping(op->host_addr, frame, op->flags); + if ( rc != GNTST_okay ) + { + if ( !(op->flags & GNTMAP_readonly) ) + put_page_type(mfn_to_page(frame)); + put_page(mfn_to_page(frame)); + goto undo_out; + } + + if ( op->flags & GNTMAP_device_map ) + { + (void)get_page(mfn_to_page(frame), rd); + if ( !(op->flags & GNTMAP_readonly) ) + get_page_type(mfn_to_page(frame), PGT_writable_page); + } } } @@ -475,23 +494,31 @@ __gnttab_unmap_grant_ref( } } - if ( (op->host_addr != 0) && (flags & GNTMAP_host_map) ) - { - if ( (rc = destroy_grant_host_mapping(op->host_addr, - frame, flags)) < 0 ) - goto unmap_out; - - ASSERT(act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask)); - map->flags &= ~GNTMAP_host_map; - if ( flags & GNTMAP_readonly ) - { - act->pin -= GNTPIN_hstr_inc; - put_page(mfn_to_page(frame)); - } - else - { - act->pin -= GNTPIN_hstw_inc; - put_page_and_type(mfn_to_page(frame)); + if ( flags & GNTMAP_host_map ) + { + if ( op->host_addr != 0 ) + { + if ( (rc = destroy_grant_host_mapping(op->host_addr, + frame, flags)) < 0 ) + goto unmap_out; + + ASSERT(act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask)); + map->flags &= ~GNTMAP_host_map; + if ( flags & GNTMAP_readonly ) + { + act->pin -= GNTPIN_hstr_inc; + put_page(mfn_to_page(frame)); + } + else + { + act->pin -= GNTPIN_hstw_inc; + put_page_and_type(mfn_to_page(frame)); + } + } else if ( iomem_page_test(frame, mfn_to_page(frame)) && + iomem_access_permitted(ld, frame, frame) ){ + map->flags &= ~GNTMAP_host_map; + + rc = iomem_deny_access(ld, frame, frame); } } @@ -1352,6 +1379,7 @@ gnttab_release_mappings( struct domain *rd; struct active_grant_entry *act; struct grant_entry *sha; + int rc; BUG_ON(!d->is_dying); @@ -1407,9 +1435,15 @@ gnttab_release_mappings( if ( map->flags & GNTMAP_host_map ) { - BUG_ON(!(act->pin & GNTPIN_hstw_mask)); - act->pin -= GNTPIN_hstw_inc; - gnttab_release_put_page_and_type(mfn_to_page(act->frame)); + if ( iomem_page_test(act->frame, mfn_to_page(act->frame)) && + iomem_access_permitted(rd, act->frame, act->frame) ) + rc = iomem_deny_access(rd, act->frame, act->frame); + else + { + BUG_ON(!(act->pin & GNTPIN_hstw_mask)); + act->pin -= GNTPIN_hstw_inc; + gnttab_release_put_page_and_type(mfn_to_page(act->frame)); + } } if ( (act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) == 0 ) diff -r 039a10cef2f7 xen/include/asm-x86/mm.h --- a/xen/include/asm-x86/mm.h Fri May 18 11:03:05 2007 +0100 +++ b/xen/include/asm-x86/mm.h Tue May 22 10:15:58 2007 +0100 @@ -197,6 +197,9 @@ static inline int get_page(struct page_i return 1; } +/* Decide whether this page looks like iomem or real memory */ +int iomem_page_test(unsigned long mfn, struct page_info *page); + void put_page_type(struct page_info *page); int get_page_type(struct page_info *page, unsigned long type); int get_page_from_l1e(l1_pgentry_t l1e, struct domain *d); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel