| Message ID | 20160801171028.11615-26-proskurin@sec.in.tum.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@sec.in.tum.de> > --- > Cc: Razvan Cojocaru <rcojocaru@bitdefender.com> > Cc: Tamas K Lengyel <tamas@tklengyel.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > --- > 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
Hi Razvan, On 08/02/2016 11:14 AM, Razvan Cojocaru wrote: > 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@sec.in.tum.de> >> --- >> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com> >> Cc: Tamas K Lengyel <tamas@tklengyel.com> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >> Cc: Wei Liu <wei.liu2@citrix.com> >> --- >> 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). > I will adapt the scopes to the coding style, thank you. These are leftovers from my test outputs. >> + >> + 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? > You are absolutely right. I will adapt the implementation accordingly. Thank you very much. Best regards, ~Sergej
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; + } + + 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; + } + + memcpy(dst_vaddr, src_vaddr, XC_PAGE_SIZE); + + munmap(src_vaddr, XC_PAGE_SIZE); + munmap(dst_vaddr, XC_PAGE_SIZE); + + 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(xc_interface *xch, + 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; + + rc = xc_domain_increase_reservation_exact(xch, domain_id, 1, 0, 0, gfn_new); + if ( rc < 0 ) + return -1; + + rc = xc_domain_populate_physmap_exact(xch, domain_id, 1, 0, 0, gfn_new); + if ( rc < 0 ) + return -1; + + /* Copy content of the old gfn into the newly allocated gfn */ + rc = copy_gfn(xch, domain_id, *gfn_new, gfn_old); + if ( rc < 0 ) + return -1; + + xc_altp2m_change_gfn(xch, domain_id, ap2m_idx, gfn_old, *gfn_new); + + return 0; +} + +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); + + rc = xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0, &gfn_new); + if ( rc < 0 ) + return -1; + + return 0; +} + int xc_wait_for_event_or_timeout(xc_interface *xch, xenevtchn_handle *xce, unsigned long ms) { struct pollfd fd = { .fd = xenevtchn_fd(xce), .events = POLLIN | POLLERR }; @@ -227,6 +323,10 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r, domid_t domain_id) } mem_access_enable = 1; + xenaccess->ap2m_idx = ~(0); + xenaccess->gfn_old = INVALID_GFN; + xenaccess->gfn_new = INVALID_GFN; + /* Open event channel */ xenaccess->vm_event.xce_handle = xenevtchn_open(NULL, 0); if ( xenaccess->vm_event.xce_handle == NULL ) @@ -662,10 +762,27 @@ int main(int argc, char *argv[]) if ( altp2m && req.flags & VM_EVENT_FLAG_ALTERNATE_P2M) { - DPRINTF("\tSwitching back to default view!\n"); - rsp.flags |= (VM_EVENT_FLAG_ALTERNATE_P2M | VM_EVENT_FLAG_TOGGLE_SINGLESTEP); - rsp.altp2m_idx = 0; + + if ( !gfn_changed ) + { + /* Store trapping gfn and ap2m index for cleanup. */ + xenaccess->gfn_old = req.u.mem_access.gfn; + xenaccess->ap2m_idx = req.altp2m_idx; + + /* Note that this function is called only once. */ + xenaccess_change_gfn(xenaccess->xc_handle, domain_id, req.altp2m_idx, + xenaccess->gfn_old, &xenaccess->gfn_new); + + /* Do not change the currently active altp2m view, yet. */ + rsp.altp2m_idx = req.altp2m_idx; + } + else + { + DPRINTF("\tSwitching back to default view!\n"); + + rsp.altp2m_idx = 0; + } } else if ( default_access != after_first_access ) { @@ -783,6 +900,18 @@ exit: for ( vcpu_id = 0; vcpu_id<XEN_LEGACY_MAX_VCPUS; vcpu_id++) rc = control_singlestep(xch, domain_id, vcpu_id, 0); #endif + + /* Reset changed gfn. */ + if ( xenaccess->gfn_new != INVALID_GFN ) + { + rc = xenaccess_reset_gfn(xenaccess->xc_handle, xenaccess->vm_event.domain_id, + xenaccess->ap2m_idx, xenaccess->gfn_old, xenaccess->gfn_new); + if ( rc != 0 ) + { + ERROR("Error resetting the remapped gfn"); + return rc; + } + } } /* Tear down domain xenaccess */
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@sec.in.tum.de> --- Cc: Razvan Cojocaru <rcojocaru@bitdefender.com> Cc: Tamas K Lengyel <tamas@tklengyel.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> --- tools/tests/xen-access/xen-access.c | 135 +++++++++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 3 deletions(-)