Re: [Xen-devel] [PATCH v4 39/39] arm/xen-access: Add test of xc_altp2m_change_gfn

[Sergej Proskurin]

Hi Razvan,


[...]

>> +
>> +*gfn_new = ++(xenaccess->max_gpfn);
> Unnecessary parentheses.
>

Thanks.

>> +rc = xc_domain_populate_physmap_exact(xenaccess->xc_handle, domain_id, 
>> 1, 0, 0, gfn_new);
>> +if ( rc < 0 )
>> +goto err;
>> +
>> +/* Copy content of the old gfn into the newly allocated gfn */
>> +rc = xenaccess_copy_gfn(xenaccess, domain_id, *gfn_new, gfn_old);
>> +if ( rc < 0 )
>> +goto err;
>> +
>> +rc = xc_altp2m_change_gfn(xenaccess->xc_handle, domain_id, ap2m_idx, 
>> gfn_old, *gfn_new);
>> +if ( rc < 0 )
>> +goto err;
>> +
>> +return 0;
>> +
>> +err:
>> +xc_domain_decrease_reservation_exact(xenaccess->xc_handle, domain_id, 
>> 1, 0, gfn_new);
>> +
>> +(xenaccess->max_gpfn)--;
> Here too.
>
>> +
>> +return -1;
>> +}
>> +
>> +static int xenaccess_reset_gfn(xc_interface *xch,
>> +   domid_t domain_id,
>> +   unsigned int ap2m_idx,
>> +   xen_pfn_t gfn_old,
>> +   xen_pfn_t gfn_new)
>> +{
>> +int rc;
>> +
>> +/* Reset previous state */
>> +xc_altp2m_change_gfn(xch, domain_id, ap2m_idx, gfn_old, INVALID_GFN);
>> +
>> +/* Invalidate the new gfn */
>> +xc_altp2m_change_gfn(xch, domain_id, ap2m_idx, gfn_new, INVALID_GFN);
> Do these two xc_altp2m_change_gfn() calls not require error checking?
>
>> +
>> +rc = xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0, 
>> _new);
>> +if ( rc < 0 )
>> +return -1;
>> +
>> +(xenaccess->max_gpfn)--;
> Again, please remove the parentheses.
>

Thanks again. I will adjust the implementation for v5.

Cheers,
~Sergej

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

Re: [Xen-devel] [PATCH v4 39/39] arm/xen-access: Add test of xc_altp2m_change_gfn

[Razvan Cojocaru]

On 08/30/2017 09:32 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. On success, the guest will continue to run. Subsequent
> altp2m access violations will trap into Xen and be forced by xen-access
> to switch to the default view (altp2m[0]) as before. The introduced test
> can be invoked by providing the argument "altp2m_remap".
> 
> Signed-off-by: Sergej Proskurin 
> ---
> Cc: Razvan Cojocaru 
> Cc: Tamas K Lengyel 
> Cc: Ian Jackson 
> Cc: Wei Liu 
> ---
> v3: Cosmetic fixes in "xenaccess_copy_gfn" and "xenaccess_change_gfn".
> 
> Added munmap in "copy_gfn" in the second error case.
> 
> Added option "altp2m_remap" selecting the altp2m-remap test.
> 
> v4: Dropped the COMPAT API for mapping foreign memory. Instead, we use the
> stable library xenforeignmemory.
> 
> Dropped the use of xc_domain_increase_reservation_exact as we do not
> need to increase the domain's physical memory. Otherwise, remapping
> a page via altp2m would become visible to the guest itself. As long
> as we have additional shadow-memory for the guest domain, we do not
> need to reserve any additional memory.
> ---
>  tools/tests/xen-access/Makefile |   2 +-
>  tools/tests/xen-access/xen-access.c | 182 
> +++-
>  2 files changed, 179 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/tests/xen-access/Makefile b/tools/tests/xen-access/Makefile
> index e11f639ccf..ab195e233f 100644
> --- a/tools/tests/xen-access/Makefile
> +++ b/tools/tests/xen-access/Makefile
> @@ -26,6 +26,6 @@ clean:
>  distclean: clean
>  
>  xen-access: xen-access.o Makefile
> - $(CC) -o $@ $< $(LDFLAGS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) 
> $(LDLIBS_libxenevtchn)
> + $(CC) -o $@ $< $(LDFLAGS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) 
> $(LDLIBS_libxenevtchn) $(LDLIBS_libxenforeignmemory)
>  
>  -include $(DEPS)
> diff --git a/tools/tests/xen-access/xen-access.c 
> b/tools/tests/xen-access/xen-access.c
> index 481337cacd..f9b9fb6bbf 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -41,6 +41,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #if defined(__arm__) || defined(__aarch64__)
>  #include 
> @@ -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))
> @@ -76,12 +79,19 @@ typedef struct vm_event {
>  typedef struct xenaccess {
>  xc_interface *xc_handle;
>  
> +xenforeignmemory_handle *fmem;
> +
>  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)
> @@ -89,6 +99,104 @@ static void close_handler(int sig)
>  interrupted = sig;
>  }
>  
> +static int xenaccess_copy_gfn(xenaccess_t *xenaccess,
> +  domid_t domain_id,
> +  xen_pfn_t dst_gfn,
> +  xen_pfn_t src_gfn)
> +{
> +void *src_vaddr = NULL;
> +void *dst_vaddr = NULL;
> +
> +src_vaddr = xenforeignmemory_map(xenaccess->fmem, domain_id, PROT_READ,
> + 1, _gfn, NULL);
> +if ( src_vaddr == NULL )
> +return -1;
> +
> +dst_vaddr = xenforeignmemory_map(xenaccess->fmem, domain_id, PROT_WRITE,
> + 1, _gfn, NULL> +if ( dst_vaddr 
> == NULL )
> +{
> +munmap(src_vaddr, XC_PAGE_SIZE);
> +return -1;
> +}
> +
> +memcpy(dst_vaddr, src_vaddr, XC_PAGE_SIZE);
> +
> +xenforeignmemory_unmap(xenaccess->fmem, src_vaddr, 1);
> +xenforeignmemory_unmap(xenaccess->fmem, dst_vaddr, 1);
> +
> +return 0;
> +}
> +
> +/*
> + * This function allocates and populates a page in the guest's physmap that 
> is
> + * subsequently filled with contents of the trapping address. Finally, 
> through
> + * the invocation of xc_altp2m_change_gfn, the altp2m subsystem changes the 
> gfn
> + * to mfn mapping of the target altp2m view.
> + */
> +static int xenaccess_change_gfn(xenaccess_t *xenaccess,
> +domid_t domain_id,
> +unsigned int ap2m_idx,
> +xen_pfn_t gfn_old,
> +xen_pfn_t *gfn_new)
> +{
> +int rc;
> +
> +/*
> + 

[Xen-devel] [PATCH v4 39/39] arm/xen-access: Add test of xc_altp2m_change_gfn

[Sergej Proskurin]

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. On success, the guest will continue to run. Subsequent
altp2m access violations will trap into Xen and be forced by xen-access
to switch to the default view (altp2m[0]) as before. The introduced test
can be invoked by providing the argument "altp2m_remap".

Signed-off-by: Sergej Proskurin 
---
Cc: Razvan Cojocaru 
Cc: Tamas K Lengyel 
Cc: Ian Jackson 
Cc: Wei Liu 
---
v3: Cosmetic fixes in "xenaccess_copy_gfn" and "xenaccess_change_gfn".

Added munmap in "copy_gfn" in the second error case.

Added option "altp2m_remap" selecting the altp2m-remap test.

v4: Dropped the COMPAT API for mapping foreign memory. Instead, we use the
stable library xenforeignmemory.

Dropped the use of xc_domain_increase_reservation_exact as we do not
need to increase the domain's physical memory. Otherwise, remapping
a page via altp2m would become visible to the guest itself. As long
as we have additional shadow-memory for the guest domain, we do not
need to reserve any additional memory.
---
 tools/tests/xen-access/Makefile |   2 +-
 tools/tests/xen-access/xen-access.c | 182 +++-
 2 files changed, 179 insertions(+), 5 deletions(-)

diff --git a/tools/tests/xen-access/Makefile b/tools/tests/xen-access/Makefile
index e11f639ccf..ab195e233f 100644
--- a/tools/tests/xen-access/Makefile
+++ b/tools/tests/xen-access/Makefile
@@ -26,6 +26,6 @@ clean:
 distclean: clean
 
 xen-access: xen-access.o Makefile
-   $(CC) -o $@ $< $(LDFLAGS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) 
$(LDLIBS_libxenevtchn)
+   $(CC) -o $@ $< $(LDFLAGS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) 
$(LDLIBS_libxenevtchn) $(LDLIBS_libxenforeignmemory)
 
 -include $(DEPS)
diff --git a/tools/tests/xen-access/xen-access.c 
b/tools/tests/xen-access/xen-access.c
index 481337cacd..f9b9fb6bbf 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #if defined(__arm__) || defined(__aarch64__)
 #include 
@@ -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))
@@ -76,12 +79,19 @@ typedef struct vm_event {
 typedef struct xenaccess {
 xc_interface *xc_handle;
 
+xenforeignmemory_handle *fmem;
+
 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)
@@ -89,6 +99,104 @@ static void close_handler(int sig)
 interrupted = sig;
 }
 
+static int xenaccess_copy_gfn(xenaccess_t *xenaccess,
+  domid_t domain_id,
+  xen_pfn_t dst_gfn,
+  xen_pfn_t src_gfn)
+{
+void *src_vaddr = NULL;
+void *dst_vaddr = NULL;
+
+src_vaddr = xenforeignmemory_map(xenaccess->fmem, domain_id, PROT_READ,
+ 1, _gfn, NULL);
+if ( src_vaddr == NULL )
+return -1;
+
+dst_vaddr = xenforeignmemory_map(xenaccess->fmem, domain_id, PROT_WRITE,
+ 1, _gfn, NULL);
+if ( dst_vaddr == NULL )
+{
+munmap(src_vaddr, XC_PAGE_SIZE);
+return -1;
+}
+
+memcpy(dst_vaddr, src_vaddr, XC_PAGE_SIZE);
+
+xenforeignmemory_unmap(xenaccess->fmem, src_vaddr, 1);
+xenforeignmemory_unmap(xenaccess->fmem, dst_vaddr, 1);
+
+return 0;
+}
+
+/*
+ * This function allocates and populates a page in the guest's physmap that is
+ * subsequently filled with contents of the trapping address. Finally, through
+ * the invocation of xc_altp2m_change_gfn, the altp2m subsystem changes the gfn
+ * to mfn mapping of the target altp2m view.
+ */
+static int xenaccess_change_gfn(xenaccess_t *xenaccess,
+domid_t domain_id,
+unsigned int ap2m_idx,
+xen_pfn_t gfn_old,
+xen_pfn_t *gfn_new)
+{
+int rc;
+
+/*
+ * We perform this function only once as it is intended to be used for
+ * testing and demonstration purposes. Thus, we signalize that further
+ * altp2m-related traps will not change trapping gfn's.
+ */
+gfn_changed = 1;
+
+*gfn_new = ++(xenaccess->max_gpfn);
+
+rc =