Re: [Xen-devel] [PATCH v2 25/25] arm/altp2m: Add test of xc_altp2m_change_gfn.

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 08/01/2016 08:10 PM, Sergej Proskurin wrote:
> This commit extends xen-access by a simple test of the functionality
> provided by "xc_altp2m_change_gfn". The idea is to dynamically remap a
> trapping gfn to another mfn, which holds the same content as the
> original mfn.
> 
> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
> ---
> Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> Cc: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  tools/tests/xen-access/xen-access.c | 135 
> +++++++++++++++++++++++++++++++++++-
>  1 file changed, 132 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/tests/xen-access/xen-access.c 
> b/tools/tests/xen-access/xen-access.c
> index eafd7d6..39b7ddf 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -38,6 +38,7 @@
>  #include <sys/mman.h>
>  #include <sys/poll.h>
>  
> +#define XC_WANT_COMPAT_MAP_FOREIGN_API
>  #include <xenctrl.h>
>  #include <xenevtchn.h>
>  #include <xen/vm_event.h>
> @@ -49,6 +50,8 @@
>  #define START_PFN 0ULL
>  #endif
>  
> +#define INVALID_GFN ~(0UL)
> +
>  #define DPRINTF(a, b...) fprintf(stderr, a, ## b)
>  #define ERROR(a, b...) fprintf(stderr, a "\n", ## b)
>  #define PERROR(a, b...) fprintf(stderr, a ": %s\n", ## b, strerror(errno))
> @@ -72,9 +75,14 @@ typedef struct xenaccess {
>      xen_pfn_t max_gpfn;
>  
>      vm_event_t vm_event;
> +
> +    unsigned int ap2m_idx;
> +    xen_pfn_t gfn_old;
> +    xen_pfn_t gfn_new;
>  } xenaccess_t;
>  
>  static int interrupted;
> +static int gfn_changed = 0;
>  bool evtchn_bind = 0, evtchn_open = 0, mem_access_enable = 0;
>  
>  static void close_handler(int sig)
> @@ -82,6 +90,94 @@ static void close_handler(int sig)
>      interrupted = sig;
>  }
>  
> +static int copy_gfn(xc_interface *xch, domid_t domain_id,
> +                    xen_pfn_t dst_gfn, xen_pfn_t src_gfn)
> +{
> +    void *src_vaddr = NULL;
> +    void *dst_vaddr = NULL;
> +
> +    src_vaddr = xc_map_foreign_range(xch, domain_id, XC_PAGE_SIZE,
> +                                     PROT_READ, src_gfn);
> +    if ( src_vaddr == MAP_FAILED || src_vaddr == NULL)
> +    {
> +        return -1;
> +    }

Please don't use scopes for single return statements (i.e. no { ... }
around them).

> +
> +    dst_vaddr = xc_map_foreign_range(xch, domain_id, XC_PAGE_SIZE,
> +                                     PROT_WRITE, dst_gfn);
> +    if ( dst_vaddr == MAP_FAILED || dst_vaddr == NULL)
> +    {
> +        return -1;
> +    }

But if this failed and the first one succeeds, shouldn't we munmap() the
first one before the return?


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel