Re: [PATCH v2 2/2] xen/m2p: use GNTTABOP_unmap_and_replace to reinstate the original mapping

[David Vrabel <david.vrabel@citrix.com>]

On 22/07/13 17:28, Stefano Stabellini wrote:
> GNTTABOP_unmap_grant_ref unmaps a grant and replaces it with a 0
> mapping instead of reinstating the original mapping.
> Doing so separately would be racy.
> 
> To unmap a grant and reinstate the original mapping atomically we use
> GNTTABOP_unmap_and_replace.
> GNTTABOP_unmap_and_replace doesn't work with GNTMAP_contains_pte, so
> don't use it for kmaps.  GNTTABOP_unmap_and_replace zeroes the mapping
> passed in new_addr so we have to reinstate it, however that is a
> per-cpu mapping only used for balloon trade pages, so we can be sure that
> it's not going to be accessed while the mapping is not valid.

This solves the problem of userspace accessing a disk image on an NFS
mount but what would blkback talking to an iSCSI LUN?  Will that need
similar fixes to blkback?  This series does not need to fix this now though.

> --- a/arch/x86/xen/p2m.c
> +++ b/arch/x86/xen/p2m.c
> @@ -161,6 +161,7 @@
>  #include <asm/xen/page.h>
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
> +#include <xen/balloon.h>
>  #include <xen/grant_table.h>
>  
>  #include "multicalls.h"
> @@ -967,7 +968,9 @@ int m2p_remove_override(struct page *page,
>  	if (kmap_op != NULL) {
>  		if (!PageHighMem(page)) {
>  			struct multicall_space mcs;
> -			struct gnttab_unmap_grant_ref *unmap_op;
> +			struct gnttab_unmap_and_replace *unmap_op;
> +			unsigned long trade_page_address = (unsigned long)
> +				__va(page_to_pfn(get_balloon_trade_page()) << PAGE_SHIFT);

	struct page *trade_page = get_balloon_trade_page();
	unsigned long trade_page_address = page_address(trade_page);

(And the corresponding put_balloon_trade_page() once you've added it.)

Otherwise,

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David