Hi everyone, I submitted some of the patches in this series before, as a part of the first RFC of the NUMA memory migration feature. While preparing another round of it, I thought there is enough ''meat'' already for a separate series. Basically, there are a few places in libxc where we issue do_domctl()-s directly, instead of using the proper xc_xxx() wrapper. This make the code bigger, redundant, more difficult to understand (e.g., "why xc_xxx() is called here and is not called there?"), less consistent and less grep-able. The first 4 patches of this series address this. The last one introduce a little tool I''m using for debugging. It allows one to see the M2P of the host, the P2M of a domain, some info about the PTEs, and perform basic searches and comparisons on them. I''m finding it very useful and, although I do not claim for my situation to be that common, I figured it could be nice to have it in the tree, so that is patch 6. Patch 5 is the small rework/additions necessary to make 6 possible. Let me know what you think. Thanks and Regards, Dario --- Dario Faggioli (6): libxc: introduce xc_domain_get_address_size libxc: use xc_vcpu_setcontext() instead of calling do_domctl() libxc: use xc_vcpu_getinfo() instead of calling do_domctl() libxc: allow for ctxt to be NULL in xc_vcpu_setcontext libxc: introduce xc_map_domain_meminfo (and xc_unmap_domain_meminfo) tools/misc: introduce xen-mfndump. tools/libxc/xc_core.c | 21 -- tools/libxc/xc_cpuid_x86.c | 8 - tools/libxc/xc_dom_boot.c | 28 +-- tools/libxc/xc_domain.c | 149 +++++++++++++- tools/libxc/xc_domain_restore.c | 13 - tools/libxc/xc_offline_page.c | 192 +++--------------- tools/libxc/xc_pagetab.c | 8 - tools/libxc/xc_private.c | 8 - tools/libxc/xc_resume.c | 23 +- tools/libxc/xenctrl.h | 12 + tools/libxc/xenguest.h | 17 ++ tools/libxc/xg_private.h | 9 + tools/libxc/xg_save_restore.h | 9 - tools/misc/Makefile | 7 - tools/misc/xen-mfndump.c | 425 +++++++++++++++++++++++++++++++++++++++ tools/xentrace/xenctx.c | 9 - 16 files changed, 677 insertions(+), 261 deletions(-) create mode 100644 tools/misc/xen-mfndump.c -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
Dario Faggioli
2013-Jul-12 16:48 UTC
[PATCH 1/6] libxc: introduce xc_domain_get_address_size
As a wrapper to XEN_DOMCTL_get_address_size, and use it wherever the call was being issued directly via do_domctl(), saving quite some line of code. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> --- tools/libxc/xc_core.c | 21 ++------------------- tools/libxc/xc_cpuid_x86.c | 8 +++----- tools/libxc/xc_domain.c | 15 +++++++++++++++ tools/libxc/xc_offline_page.c | 9 ++------- tools/libxc/xc_pagetab.c | 8 +++----- tools/libxc/xc_resume.c | 23 ++++++----------------- tools/libxc/xenctrl.h | 12 ++++++++++++ tools/libxc/xg_save_restore.h | 9 ++------- tools/xentrace/xenctx.c | 9 +++------ 9 files changed, 48 insertions(+), 66 deletions(-) diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c index 4207eed..c8bade5 100644 --- a/tools/libxc/xc_core.c +++ b/tools/libxc/xc_core.c @@ -417,24 +417,6 @@ elfnote_dump_format_version(xc_interface *xch, return dump_rtn(xch, args, (char*)&format_version, sizeof(format_version)); } -static int -get_guest_width(xc_interface *xch, - uint32_t domid, - unsigned int *guest_width) -{ - DECLARE_DOMCTL; - - memset(&domctl, 0, sizeof(domctl)); - domctl.domain = domid; - domctl.cmd = XEN_DOMCTL_get_address_size; - - if ( do_domctl(xch, &domctl) != 0 ) - return 1; - - *guest_width = domctl.u.address_size.size / 8; - return 0; -} - int xc_domain_dumpcore_via_callback(xc_interface *xch, uint32_t domid, @@ -478,11 +460,12 @@ xc_domain_dumpcore_via_callback(xc_interface *xch, struct xc_core_section_headers *sheaders = NULL; Elf64_Shdr *shdr; - if ( get_guest_width(xch, domid, &dinfo->guest_width) != 0 ) + if ( xc_domain_get_address_size(xch, domid, &dinfo->guest_width) != 0 ) { PERROR("Could not get address size for domain"); return sts; } + dinfo->guest_width /= 8; xc_core_arch_context_init(&arch_ctxt); if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL ) diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c index fa47787..99e3997 100644 --- a/tools/libxc/xc_cpuid_x86.c +++ b/tools/libxc/xc_cpuid_x86.c @@ -436,17 +436,15 @@ static void xc_cpuid_pv_policy( const unsigned int *input, unsigned int *regs) { DECLARE_DOMCTL; + unsigned int guest_width; int guest_64bit, xen_64bit = hypervisor_is_64bit(xch); char brand[13]; uint64_t xfeature_mask; xc_cpuid_brand_get(brand); - memset(&domctl, 0, sizeof(domctl)); - domctl.domain = domid; - domctl.cmd = XEN_DOMCTL_get_address_size; - do_domctl(xch, &domctl); - guest_64bit = (domctl.u.address_size.size == 64); + xc_domain_get_address_size(xch, domid, &guest_width); + guest_64bit = (guest_width == 64); /* Detecting Xen''s atitude towards XSAVE */ memset(&domctl, 0, sizeof(domctl)); diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index 3257e2a..d64d0bc 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -270,6 +270,21 @@ out: return ret; } +int xc_domain_get_address_size(xc_interface *xch, uint32_t domid, + unsigned int *addr_size) +{ + DECLARE_DOMCTL; + + memset(&domctl, 0, sizeof(domctl)); + domctl.domain = domid; + domctl.cmd = XEN_DOMCTL_get_address_size; + + if ( do_domctl(xch, &domctl) != 0 ) + return 1; + + *addr_size = domctl.u.address_size.size; + return 0; +} int xc_domain_getinfo(xc_interface *xch, uint32_t first_domid, diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c index 36b9812..c5547a8 100644 --- a/tools/libxc/xc_offline_page.c +++ b/tools/libxc/xc_offline_page.c @@ -193,20 +193,15 @@ static int get_pt_level(xc_interface *xch, uint32_t domid, unsigned int *pt_level, unsigned int *gwidth) { - DECLARE_DOMCTL; xen_capabilities_info_t xen_caps = ""; if (xc_version(xch, XENVER_capabilities, &xen_caps) != 0) return -1; - memset(&domctl, 0, sizeof(domctl)); - domctl.domain = domid; - domctl.cmd = XEN_DOMCTL_get_address_size; - - if ( do_domctl(xch, &domctl) != 0 ) + if (xc_domain_get_address_size(xch, domid, gwidth) != 0) return -1; - *gwidth = domctl.u.address_size.size / 8; + *gwidth /= 8; if (strstr(xen_caps, "xen-3.0-x86_64")) /* Depends on whether it''s a compat 32-on-64 guest */ diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c index 27c4e9f..5937a52 100644 --- a/tools/libxc/xc_pagetab.c +++ b/tools/libxc/xc_pagetab.c @@ -51,15 +51,13 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, pt_levels = (ctx.msr_efer&EFER_LMA) ? 4 : (ctx.cr4&CR4_PAE) ? 3 : 2; paddr = ctx.cr3 & ((pt_levels == 3) ? ~0x1full : ~0xfffull); } else { - DECLARE_DOMCTL; + unsigned int gwidth; vcpu_guest_context_any_t ctx; if (xc_vcpu_getcontext(xch, dom, vcpu, &ctx) != 0) return 0; - domctl.domain = dom; - domctl.cmd = XEN_DOMCTL_get_address_size; - if ( do_domctl(xch, &domctl) != 0 ) + if (xc_domain_get_address_size(xch, dom, &gwidth) != 0) return 0; - if (domctl.u.address_size.size == 64) { + if (gwidth == 64) { pt_levels = 4; paddr = (uint64_t)xen_cr3_to_pfn_x86_64(ctx.x64.ctrlreg[3]) << PAGE_SHIFT; diff --git a/tools/libxc/xc_resume.c b/tools/libxc/xc_resume.c index 1c43ec6..58ea395 100644 --- a/tools/libxc/xc_resume.c +++ b/tools/libxc/xc_resume.c @@ -24,19 +24,6 @@ #include <xen/foreign/x86_64.h> #include <xen/hvm/params.h> -static int pv_guest_width(xc_interface *xch, uint32_t domid) -{ - DECLARE_DOMCTL; - domctl.domain = domid; - domctl.cmd = XEN_DOMCTL_get_address_size; - if ( xc_domctl(xch, &domctl) != 0 ) - { - PERROR("Could not get guest address size"); - return -1; - } - return domctl.u.address_size.size / 8; -} - static int modify_returncode(xc_interface *xch, uint32_t domid) { vcpu_guest_context_any_t ctxt; @@ -71,9 +58,9 @@ static int modify_returncode(xc_interface *xch, uint32_t domid) else { /* Probe PV guest address width. */ - dinfo->guest_width = pv_guest_width(xch, domid); - if ( dinfo->guest_width < 0 ) + if ( xc_domain_get_address_size(xch, domid, &dinfo->guest_width) ) return -1; + dinfo->guest_width /= 8; } if ( (rc = xc_vcpu_getcontext(xch, domid, 0, &ctxt)) != 0 ) @@ -120,7 +107,8 @@ static int xc_domain_resume_any(xc_interface *xch, uint32_t domid) xc_dominfo_t info; int i, rc = -1; #if defined(__i386__) || defined(__x86_64__) - struct domain_info_context _dinfo = { .p2m_size = 0 }; + struct domain_info_context _dinfo = { .guest_width = 0, + .p2m_size = 0 }; struct domain_info_context *dinfo = &_dinfo; unsigned long mfn; vcpu_guest_context_any_t ctxt; @@ -147,7 +135,8 @@ static int xc_domain_resume_any(xc_interface *xch, uint32_t domid) return rc; } - dinfo->guest_width = pv_guest_width(xch, domid); + xc_domain_get_address_size(xch, domid, &dinfo->guest_width); + dinfo->guest_width /= 8; if ( dinfo->guest_width != sizeof(long) ) { ERROR("Cannot resume uncooperative cross-address-size guests"); diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h index 388a9c3..907106e 100644 --- a/tools/libxc/xenctrl.h +++ b/tools/libxc/xenctrl.h @@ -561,6 +561,18 @@ int xc_vcpu_getaffinity(xc_interface *xch, int vcpu, xc_cpumap_t cpumap); + +/** + * This function will return the address size for the specified domain. + * + * @param xch a handle to an open hypervisor interface. + * @param domid the domain id one wants the address size width of. + * @param addr_size the address size. + */ +int xc_domain_get_address_size(xc_interface *xch, uint32_t domid, + unsigned int *addr_size); + + /** * This function will return information about one or more domains. It is * designed to iterate over the list of domains. If a single domain is diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h index 6512003..3c11c44 100644 --- a/tools/libxc/xg_save_restore.h +++ b/tools/libxc/xg_save_restore.h @@ -301,7 +301,6 @@ static inline int get_platform_info(xc_interface *xch, uint32_t dom, { xen_capabilities_info_t xen_caps = ""; xen_platform_parameters_t xen_params; - DECLARE_DOMCTL; if (xc_version(xch, XENVER_platform_parameters, &xen_params) != 0) return 0; @@ -313,14 +312,10 @@ static inline int get_platform_info(xc_interface *xch, uint32_t dom, *hvirt_start = xen_params.virt_start; - memset(&domctl, 0, sizeof(domctl)); - domctl.domain = dom; - domctl.cmd = XEN_DOMCTL_get_address_size; - - if ( do_domctl(xch, &domctl) != 0 ) + if ( xc_domain_get_address_size(xch, dom, guest_width) != 0) return 0; - *guest_width = domctl.u.address_size.size / 8; + *guest_width /= 8; /* 64-bit tools will see the 64-bit hvirt_start, but 32-bit guests * will be using the compat one. */ diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c index 060e480..ae4d6a7 100644 --- a/tools/xentrace/xenctx.c +++ b/tools/xentrace/xenctx.c @@ -771,12 +771,9 @@ static void dump_ctx(int vcpu) } ctxt_word_size = (strstr(xen_caps, "xen-3.0-x86_64")) ? 8 : 4; } else { - struct xen_domctl domctl; - memset(&domctl, 0, sizeof domctl); - domctl.domain = xenctx.domid; - domctl.cmd = XEN_DOMCTL_get_address_size; - if (xc_domctl(xenctx.xc_handle, &domctl) == 0) - ctxt_word_size = guest_word_size = domctl.u.address_size.size / 8; + unsigned int gw; + if ( !xc_domain_get_address_size(xenctx.xc_handle, xenctx.domid, &gw) ) + ctxt_word_size = guest_word_size = gw / 8; } } #endif
Dario Faggioli
2013-Jul-12 16:48 UTC
[PATCH 2/6] libxc: use xc_vcpu_setcontext() instead of calling do_domctl()
The wrapper is there already, so better use it in place of all the stuff required to issue a call to do_domctl() for XEN_DOMCTL_setvcpucontext. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> --- tools/libxc/xc_dom_boot.c | 14 ++++---------- tools/libxc/xc_domain_restore.c | 6 +----- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c index d4d57b4..cf509fa 100644 --- a/tools/libxc/xc_dom_boot.c +++ b/tools/libxc/xc_dom_boot.c @@ -62,19 +62,13 @@ static int setup_hypercall_page(struct xc_dom_image *dom) return rc; } -static int launch_vm(xc_interface *xch, domid_t domid, xc_hypercall_buffer_t *ctxt) +static int launch_vm(xc_interface *xch, domid_t domid, + vcpu_guest_context_any_t *ctxt) { - DECLARE_DOMCTL; - DECLARE_HYPERCALL_BUFFER_ARGUMENT(ctxt); int rc; xc_dom_printf(xch, "%s: called, ctxt=%p", __FUNCTION__, ctxt); - memset(&domctl, 0, sizeof(domctl)); - domctl.cmd = XEN_DOMCTL_setvcpucontext; - domctl.domain = domid; - domctl.u.vcpucontext.vcpu = 0; - set_xen_guest_handle(domctl.u.vcpucontext.ctxt, ctxt); - rc = do_domctl(xch, &domctl); + rc = xc_vcpu_setcontext(xch, domid, 0, ctxt); if ( rc != 0 ) xc_dom_panic(xch, XC_INTERNAL_ERROR, "%s: SETVCPUCONTEXT failed (rc=%d)", __FUNCTION__, rc); @@ -270,7 +264,7 @@ int xc_dom_boot_image(struct xc_dom_image *dom) if ( (rc = dom->arch_hooks->vcpu(dom, ctxt)) != 0 ) return rc; xc_dom_unmap_all(dom); - rc = launch_vm(dom->xch, dom->guest_domid, HYPERCALL_BUFFER(ctxt)); + rc = launch_vm(dom->xch, dom->guest_domid, ctxt); xc_hypercall_buffer_free(dom->xch, ctxt); return rc; diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c index 63d36cd..41a63cb 100644 --- a/tools/libxc/xc_domain_restore.c +++ b/tools/libxc/xc_domain_restore.c @@ -2113,11 +2113,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, } ctxt->x64.ctrlreg[1] = FOLD_CR3(ctx->p2m[pfn]); } - domctl.cmd = XEN_DOMCTL_setvcpucontext; - domctl.domain = (domid_t)dom; - domctl.u.vcpucontext.vcpu = i; - set_xen_guest_handle(domctl.u.vcpucontext.ctxt, ctxt); - frc = xc_domctl(xch, &domctl); + frc = xc_vcpu_setcontext(xch, dom, i, ctxt); if ( frc != 0 ) { PERROR("Couldn''t build vcpu%d", i);
Dario Faggioli
2013-Jul-12 16:48 UTC
[PATCH 3/6] libxc: use xc_vcpu_getinfo() instead of calling do_domctl()
The wrapper is there already, so better use it in place of all the stuff required to issue a call to do_domctl() for XEN_DOMCTL_getdomaininfo. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> --- tools/libxc/xc_dom_boot.c | 14 ++++++-------- tools/libxc/xc_domain_restore.c | 7 +++---- tools/libxc/xc_private.c | 8 +++----- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c index cf509fa..69b5c9c 100644 --- a/tools/libxc/xc_dom_boot.c +++ b/tools/libxc/xc_dom_boot.c @@ -197,8 +197,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, xen_pfn_t pfn, int xc_dom_boot_image(struct xc_dom_image *dom) { - DECLARE_DOMCTL; DECLARE_HYPERCALL_BUFFER(vcpu_guest_context_any_t, ctxt); + xc_dominfo_t info; int rc; ctxt = xc_hypercall_buffer_alloc(dom->xch, ctxt, sizeof(*ctxt)); @@ -212,23 +212,21 @@ int xc_dom_boot_image(struct xc_dom_image *dom) return rc; /* collect some info */ - domctl.cmd = XEN_DOMCTL_getdomaininfo; - domctl.domain = dom->guest_domid; - rc = do_domctl(dom->xch, &domctl); - if ( rc != 0 ) + rc = xc_domain_getinfo(dom->xch, dom->guest_domid, 1, &info); + if ( rc != 1 ) { xc_dom_panic(dom->xch, XC_INTERNAL_ERROR, "%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc); return rc; } - if ( domctl.domain != dom->guest_domid ) + if ( info.domid != dom->guest_domid ) { xc_dom_panic(dom->xch, XC_INTERNAL_ERROR, "%s: Huh? domid mismatch (%d != %d)", __FUNCTION__, - domctl.domain, dom->guest_domid); + info.domid, dom->guest_domid); return -1; } - dom->shared_info_mfn = domctl.u.getdomaininfo.shared_info_frame; + dom->shared_info_mfn = info.shared_info_frame; /* sanity checks */ if ( !xc_dom_compat_check(dom) ) diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c index 41a63cb..b418963 100644 --- a/tools/libxc/xc_domain_restore.c +++ b/tools/libxc/xc_domain_restore.c @@ -1406,6 +1406,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, struct restore_callbacks *callbacks) { DECLARE_DOMCTL; + xc_dominfo_t info; int rc = 1, frc, i, j, n, m, pae_extended_cr3 = 0, ext_vcpucontext = 0; int vcpuextstate = 0; uint32_t vcpuextstate_size = 0; @@ -1562,14 +1563,12 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, ROUNDUP(MAX_BATCH_SIZE * sizeof(xen_pfn_t), PAGE_SHIFT)); /* Get the domain''s shared-info frame. */ - domctl.cmd = XEN_DOMCTL_getdomaininfo; - domctl.domain = (domid_t)dom; - if ( xc_domctl(xch, &domctl) < 0 ) + if ( xc_domain_getinfo(xch, (domid_t)dom, 1, &info) != 1 ) { PERROR("Could not get information on new domain"); goto out; } - shared_info_frame = domctl.u.getdomaininfo.shared_info_frame; + shared_info_frame = info.shared_info_frame; /* Mark all PFNs as invalid; we allocate on demand */ for ( pfn = 0; pfn < dinfo->p2m_size; pfn++ ) diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c index acaf9e0..a260257 100644 --- a/tools/libxc/xc_private.c +++ b/tools/libxc/xc_private.c @@ -609,11 +609,9 @@ int xc_get_pfn_list(xc_interface *xch, long xc_get_tot_pages(xc_interface *xch, uint32_t domid) { - DECLARE_DOMCTL; - domctl.cmd = XEN_DOMCTL_getdomaininfo; - domctl.domain = (domid_t)domid; - return (do_domctl(xch, &domctl) < 0) ? - -1 : domctl.u.getdomaininfo.tot_pages; + xc_dominfo_t info; + return (xc_domain_getinfo(xch, domid, 1, &info) != 1) ? + -1 : info.nr_pages; } int xc_copy_to_domain_page(xc_interface *xch,
Dario Faggioli
2013-Jul-12 16:48 UTC
[PATCH 4/6] libxc: allow for ctxt to be NULL in xc_vcpu_setcontext
Since, as can be seen in xen/common/domctl.c, that is legitimate. Actually, it is the only way to have vcpu_reset() invoked from outside Xen. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Jan Beulich <JBeulich@suse.com> Cc: Keir Fraser <keir@xen.org> --- tools/libxc/xc_domain.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index d64d0bc..a247426 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -1140,12 +1140,6 @@ int xc_vcpu_setcontext(xc_interface *xch, DECLARE_HYPERCALL_BOUNCE(ctxt, sizeof(vcpu_guest_context_any_t), XC_HYPERCALL_BUFFER_BOUNCE_IN); int rc; - if (ctxt == NULL) - { - errno = EINVAL; - return -1; - } - if ( xc_hypercall_bounce_pre(xch, ctxt) ) return -1;
Dario Faggioli
2013-Jul-12 16:48 UTC
[PATCH 5/6] libxc: introduce xc_map_domain_meminfo (and xc_unmap_domain_meminfo)
And use it in xc_exchange_page(). This is basically because the following change need something really similar to the set of steps that are here abstracted in these two functions. Despite of the change in the interface and in the signature of some functions, this is pure code motion. No functional changes involved. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> --- tools/libxc/xc_domain.c | 128 ++++++++++++++++++++++++++++ tools/libxc/xc_offline_page.c | 187 +++++++---------------------------------- tools/libxc/xenguest.h | 17 ++++ tools/libxc/xg_private.h | 9 ++ 4 files changed, 184 insertions(+), 157 deletions(-) diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index a247426..c321e73 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -21,6 +21,8 @@ */ #include "xc_private.h" +#include "xc_core.h" +#include "xg_private.h" #include "xg_save_restore.h" #include <xen/memory.h> #include <xen/hvm/hvm_op.h> @@ -1476,6 +1478,132 @@ int xc_domain_bind_pt_isa_irq( PT_IRQ_TYPE_ISA, 0, 0, 0, machine_irq)); } +int xc_unmap_domain_meminfo(xc_interface *xch, struct xc_domain_meminfo *minfo) +{ + struct domain_info_context _di = { .guest_width = minfo->guest_width }; + struct domain_info_context *dinfo = &_di; + + free(minfo->pfn_type); + if ( minfo->p2m_table ) + munmap(minfo->p2m_table, P2M_FLL_ENTRIES * PAGE_SIZE); + minfo->p2m_table = NULL; + + return 0; +} + +int xc_map_domain_meminfo(xc_interface *xch, int domid, + struct xc_domain_meminfo *minfo) +{ + struct domain_info_context _di; + struct domain_info_context *dinfo = &_di; + + xc_dominfo_t info; + shared_info_any_t *live_shinfo; + xen_capabilities_info_t xen_caps = ""; + int i; + + /* Only be initialized once */ + if ( minfo->pfn_type || minfo->p2m_table ) + { + errno = EINVAL; + return -1; + } + + if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ) + { + PERROR("Could not get domain info"); + return -1; + } + + if ( xc_domain_get_address_size(xch, domid, &minfo->guest_width) ) + { + PERROR("Could not get domain address size"); + return -1; + } + minfo->guest_width /= 8; + _di.guest_width = minfo->guest_width; + + /* Get page table levels (see get_platform_info() in xg_save_restore.h */ + if ( xc_version(xch, XENVER_capabilities, &xen_caps) ) + { + PERROR("Could not get Xen capabilities (for page table levels)"); + return -1; + } + if ( strstr(xen_caps, "xen-3.0-x86_64") ) + /* Depends on whether it''s a compat 32-on-64 guest */ + minfo->pt_levels = ( (minfo->guest_width == 8) ? 4 : 3 ); + else if ( strstr(xen_caps, "xen-3.0-x86_32p") ) + minfo->pt_levels = 3; + else if ( strstr(xen_caps, "xen-3.0-x86_32") ) + minfo->pt_levels = 2; + else + { + errno = EFAULT; + return -1; + } + + /* We need the shared info page for mapping the P2M */ + live_shinfo = xc_map_foreign_range(xch, domid, PAGE_SIZE, PROT_READ, + info.shared_info_frame); + if ( !live_shinfo ) + { + PERROR("Could not map the shared info frame (MFN 0x%lx)", + info.shared_info_frame); + return -1; + } + + if ( xc_core_arch_map_p2m_writable(xch, minfo->guest_width, &info, + live_shinfo, &minfo->p2m_table, + &minfo->p2m_size) ) + { + PERROR("Could not map the P2M table"); + munmap(live_shinfo, PAGE_SIZE); + return -1; + } + munmap(live_shinfo, PAGE_SIZE); + _di.p2m_size = minfo->p2m_size; + + /* Make space and prepare for getting the PFN types */ + minfo->pfn_type = calloc(sizeof(*minfo->pfn_type), minfo->p2m_size); + if ( !minfo->pfn_type ) + { + PERROR("Could not allocate memory for the PFN types"); + goto failed; + } + for ( i = 0; i < minfo->p2m_size; i++ ) + minfo->pfn_type[i] = pfn_to_mfn(i, minfo->p2m_table, + minfo->guest_width); + + /* Retrieve PFN types in batches */ + for ( i = 0; i < minfo->p2m_size ; i+=1024 ) + { + int count = ((minfo->p2m_size - i ) > 1024 ) ? + 1024: (minfo->p2m_size - i); + + if ( xc_get_pfn_type_batch(xch, domid, count, minfo->pfn_type + i) ) + { + PERROR("Could not get %d-eth batch of PFN types", (i+1)/1024); + goto failed; + } + } + + return 0; + +failed: + if ( minfo->pfn_type ) + { + free(minfo->pfn_type); + minfo->pfn_type = NULL; + } + if ( minfo->p2m_table ) + { + munmap(minfo->p2m_table, P2M_FLL_ENTRIES * PAGE_SIZE); + minfo->p2m_table = NULL; + } + + return -1; +} + int xc_domain_memory_mapping( xc_interface *xch, uint32_t domid, diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c index c5547a8..fbb53f5 100644 --- a/tools/libxc/xc_offline_page.c +++ b/tools/libxc/xc_offline_page.c @@ -33,17 +33,6 @@ #include "xg_private.h" #include "xg_save_restore.h" -struct domain_mem_info{ - int domid; - unsigned int pt_level; - unsigned int guest_width; - xen_pfn_t *pfn_type; - xen_pfn_t *p2m_table; - unsigned long p2m_size; - xen_pfn_t *m2p_table; - int max_mfn; -}; - struct pte_backup_entry { xen_pfn_t table_mfn; @@ -180,141 +169,6 @@ static int xc_is_page_granted_v2(xc_interface *xch, xen_pfn_t gpfn, return (i != gnt_num); } -static xen_pfn_t pfn_to_mfn(xen_pfn_t pfn, xen_pfn_t *p2m, int gwidth) -{ - return ((xen_pfn_t) ((gwidth==8)? - (((uint64_t *)p2m)[(pfn)]): - ((((uint32_t *)p2m)[(pfn)]) == 0xffffffffU ? - (-1UL) : - (((uint32_t *)p2m)[(pfn)])))); -} - -static int get_pt_level(xc_interface *xch, uint32_t domid, - unsigned int *pt_level, - unsigned int *gwidth) -{ - xen_capabilities_info_t xen_caps = ""; - - if (xc_version(xch, XENVER_capabilities, &xen_caps) != 0) - return -1; - - if (xc_domain_get_address_size(xch, domid, gwidth) != 0) - return -1; - - *gwidth /= 8; - - if (strstr(xen_caps, "xen-3.0-x86_64")) - /* Depends on whether it''s a compat 32-on-64 guest */ - *pt_level = ( (*gwidth == 8) ? 4 : 3 ); - else if (strstr(xen_caps, "xen-3.0-x86_32p")) - *pt_level = 3; - else if (strstr(xen_caps, "xen-3.0-x86_32")) - *pt_level = 2; - else - return -1; - - return 0; -} - -static int close_mem_info(xc_interface *xch, struct domain_mem_info *minfo) -{ - if (minfo->pfn_type) - free(minfo->pfn_type); - munmap(minfo->m2p_table, M2P_SIZE(minfo->max_mfn)); - munmap(minfo->p2m_table, P2M_FLL_ENTRIES * PAGE_SIZE); - minfo->p2m_table = minfo->m2p_table = NULL; - - return 0; -} - -static int init_mem_info(xc_interface *xch, int domid, - struct domain_mem_info *minfo, - xc_dominfo_t *info) -{ - uint64_aligned_t shared_info_frame; - shared_info_any_t *live_shinfo = NULL; - int i, rc; - - /* Only be initialized once */ - if (minfo->pfn_type || minfo->m2p_table || minfo->p2m_table) - return -EINVAL; - - if ( get_pt_level(xch, domid, &minfo->pt_level, - &minfo->guest_width) ) - { - ERROR("Unable to get PT level info."); - return -EFAULT; - } - dinfo->guest_width = minfo->guest_width; - - shared_info_frame = info->shared_info_frame; - - live_shinfo = xc_map_foreign_range(xch, domid, - PAGE_SIZE, PROT_READ, shared_info_frame); - if ( !live_shinfo ) - { - ERROR("Couldn''t map live_shinfo"); - return -EFAULT; - } - - if ( (rc = xc_core_arch_map_p2m_writable(xch, minfo->guest_width, - info, live_shinfo, &minfo->p2m_table, &minfo->p2m_size)) ) - { - ERROR("Couldn''t map p2m table %x\n", rc); - goto failed; - } - munmap(live_shinfo, PAGE_SIZE); - live_shinfo = NULL; - - dinfo->p2m_size = minfo->p2m_size; - - minfo->max_mfn = xc_maximum_ram_page(xch); - if ( !(minfo->m2p_table - xc_map_m2p(xch, minfo->max_mfn, PROT_READ, NULL)) ) - { - ERROR("Failed to map live M2P table"); - goto failed; - } - - /* Get pfn type */ - minfo->pfn_type = calloc(sizeof(*minfo->pfn_type), minfo->p2m_size); - if (!minfo->pfn_type) - { - ERROR("Failed to malloc pfn_type\n"); - goto failed; - } - - for (i = 0; i < minfo->p2m_size; i++) - minfo->pfn_type[i] = pfn_to_mfn(i, minfo->p2m_table, - minfo->guest_width); - - for (i = 0; i < minfo->p2m_size ; i+=1024) - { - int count = ((dinfo->p2m_size - i ) > 1024 ) ? 1024: (dinfo->p2m_size - i); - if ( ( rc = xc_get_pfn_type_batch(xch, domid, count, - minfo->pfn_type + i)) ) - { - ERROR("Failed to get pfn_type %x\n", rc); - goto failed; - } - } - return 0; - -failed: - if (minfo->pfn_type) - { - free(minfo->pfn_type); - minfo->pfn_type = NULL; - } - if (live_shinfo) - munmap(live_shinfo, PAGE_SIZE); - munmap(minfo->m2p_table, M2P_SIZE(minfo->max_mfn)); - munmap(minfo->p2m_table, P2M_FLL_ENTRIES * PAGE_SIZE); - minfo->p2m_table = minfo->m2p_table = NULL; - - return -1; -} - static int backup_ptes(xen_pfn_t table_mfn, int offset, struct pte_backup *backup) { @@ -404,7 +258,7 @@ static int __update_pte(xc_interface *xch, } static int change_pte(xc_interface *xch, int domid, - struct domain_mem_info *minfo, + struct xc_domain_meminfo *minfo, struct pte_backup *backup, struct xc_mmu *mmu, pte_func func, @@ -414,7 +268,7 @@ static int change_pte(xc_interface *xch, int domid, uint64_t i; void *content = NULL; - pte_num = PAGE_SIZE / ((minfo->pt_level == 2) ? 4 : 8); + pte_num = PAGE_SIZE / ((minfo->pt_levels == 2) ? 4 : 8); for (i = 0; i < minfo->p2m_size; i++) { @@ -437,7 +291,7 @@ static int change_pte(xc_interface *xch, int domid, for (j = 0; j < pte_num; j++) { - if ( minfo->pt_level == 2 ) + if ( minfo->pt_levels == 2 ) pte = ((const uint32_t*)content)[j]; else pte = ((const uint64_t*)content)[j]; @@ -449,7 +303,7 @@ static int change_pte(xc_interface *xch, int domid, case 1: if ( xc_add_mmu_update(xch, mmu, table_mfn << PAGE_SHIFT | - j * ( (minfo->pt_level == 2) ? + j * ( (minfo->pt_levels == 2) ? sizeof(uint32_t): sizeof(uint64_t)) | MMU_PT_UPDATE_PRESERVE_AD, new_pte) ) @@ -482,7 +336,7 @@ failed: } static int update_pte(xc_interface *xch, int domid, - struct domain_mem_info *minfo, + struct xc_domain_meminfo *minfo, struct pte_backup *backup, struct xc_mmu *mmu, unsigned long new_mfn) @@ -492,7 +346,7 @@ static int update_pte(xc_interface *xch, int domid, } static int clear_pte(xc_interface *xch, int domid, - struct domain_mem_info *minfo, + struct xc_domain_meminfo *minfo, struct pte_backup *backup, struct xc_mmu *mmu, xen_pfn_t mfn) @@ -540,7 +394,7 @@ static int is_page_exchangable(xc_interface *xch, int domid, xen_pfn_t mfn, int xc_exchange_page(xc_interface *xch, int domid, xen_pfn_t mfn) { xc_dominfo_t info; - struct domain_mem_info minfo; + struct xc_domain_meminfo minfo; struct xc_mmu *mmu = NULL; struct pte_backup old_ptes = {NULL, 0, 0}; grant_entry_v1_t *gnttab_v1 = NULL; @@ -551,6 +405,8 @@ int xc_exchange_page(xc_interface *xch, int domid, xen_pfn_t mfn) int rc, result = -1; uint32_t status; xen_pfn_t new_mfn, gpfn; + xen_pfn_t *m2p_table; + int max_mfn; if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ) { @@ -570,10 +426,26 @@ int xc_exchange_page(xc_interface *xch, int domid, xen_pfn_t mfn) return -EINVAL; } - /* Get domain''s memory information */ + /* Map M2P and obtain gpfn */ + max_mfn = xc_maximum_ram_page(xch); + if ( !(m2p_table = xc_map_m2p(xch, max_mfn, PROT_READ, NULL)) ) + { + PERROR("Failed to map live M2P table"); + return -EFAULT; + } + gpfn = m2p_table[mfn]; + + /* Map domain''s memory information */ memset(&minfo, 0, sizeof(minfo)); - init_mem_info(xch, domid, &minfo, &info); - gpfn = minfo.m2p_table[mfn]; + if ( xc_map_domain_meminfo(xch, domid, &minfo) ) + { + PERROR("Could not map domain''s memory information\n"); + return -EFAULT; + } + + /* For translation macros */ + dinfo->guest_width = minfo.guest_width; + dinfo->p2m_size = minfo.p2m_size; /* Don''t exchange CR3 for PAE guest in PAE host environment */ if (minfo.guest_width > sizeof(long)) @@ -768,7 +640,8 @@ failed: if (gnttab_v2) munmap(gnttab_v2, gnt_num / (PAGE_SIZE/sizeof(grant_entry_v2_t))); - close_mem_info(xch, &minfo); + xc_unmap_domain_meminfo(xch, &minfo); + munmap(m2p_table, M2P_SIZE(max_mfn)); return result; } diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h index 4714bd2..c12091f 100644 --- a/tools/libxc/xenguest.h +++ b/tools/libxc/xenguest.h @@ -276,6 +276,23 @@ int xc_exchange_page(xc_interface *xch, int domid, xen_pfn_t mfn); /** + * Memory related information, such as PFN types, the P2M table, + * the guest word width and the guest page table levels. + */ +struct xc_domain_meminfo { + unsigned int pt_levels; + unsigned int guest_width; + xen_pfn_t *pfn_type; + xen_pfn_t *p2m_table; + unsigned long p2m_size; +}; + +int xc_map_domain_meminfo(xc_interface *xch, int domid, + struct xc_domain_meminfo *minfo); + +int xc_unmap_domain_meminfo(xc_interface *xch, struct xc_domain_meminfo *mem); + +/** * This function map m2p table * @parm xch a handle to an open hypervisor interface * @parm max_mfn the max pfn diff --git a/tools/libxc/xg_private.h b/tools/libxc/xg_private.h index db02ccf..5ff2124 100644 --- a/tools/libxc/xg_private.h +++ b/tools/libxc/xg_private.h @@ -136,6 +136,15 @@ struct domain_info_context { unsigned long p2m_size; }; +static inline xen_pfn_t pfn_to_mfn(xen_pfn_t pfn, xen_pfn_t *p2m, int gwidth) +{ + return ((xen_pfn_t) ((gwidth==8)? + (((uint64_t *)p2m)[(pfn)]): + ((((uint32_t *)p2m)[(pfn)]) == 0xffffffffU ? + (-1UL) : + (((uint32_t *)p2m)[(pfn)])))); +} + /* Number of xen_pfn_t in a page */ #define FPP (PAGE_SIZE/(dinfo->guest_width))
A little development and debugging tool, useful when looking for information about MFN to PFN mappings, MFN/PFN mappings in a guest''s PTEs, etc. This is what it can do as of now: $ /usr/sbin/xen-mfndump Usage: xen-mfndump <command> [args] Commands: help show this help dump-m2p show M2P dump-p2m <domid> show P2M of <domid> dump-ptes <domid> <mfn> show the PTEs in <mfn> lookup-pte <domid> <mfn> find the PTE mapping <mfn> memcmp-mfns <domid1> <mfn1> <domid2> <mfn2> (str)compare content of <mfn1> & <mfn2> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Tim Deegan <tim@xen.org> Cc: Jan Beulich <JBeulich@suse.com> Cc: Wei Liu <wei.liu2@citrix.com> --- tools/misc/Makefile | 7 + tools/misc/xen-mfndump.c | 425 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 430 insertions(+), 2 deletions(-) create mode 100644 tools/misc/xen-mfndump.c diff --git a/tools/misc/Makefile b/tools/misc/Makefile index 520ef80..59e8d36 100644 --- a/tools/misc/Makefile +++ b/tools/misc/Makefile @@ -10,7 +10,7 @@ CFLAGS += $(CFLAGS_libxenstore) HDRS = $(wildcard *.h) TARGETS-y := xenperf xenpm xen-tmem-list-parse gtraceview gtracestat xenlockprof xenwatchdogd xencov -TARGETS-$(CONFIG_X86) += xen-detect xen-hvmctx xen-hvmcrash xen-lowmemd +TARGETS-$(CONFIG_X86) += xen-detect xen-hvmctx xen-hvmcrash xen-lowmemd xen-mfndump TARGETS-$(CONFIG_MIGRATE) += xen-hptool TARGETS := $(TARGETS-y) @@ -24,7 +24,7 @@ INSTALL_BIN := $(INSTALL_BIN-y) INSTALL_SBIN-y := xm xen-bugtool xen-python-path xend xenperf xsview xenpm xen-tmem-list-parse gtraceview \ gtracestat xenlockprof xenwatchdogd xen-ringwatch xencov -INSTALL_SBIN-$(CONFIG_X86) += xen-hvmctx xen-hvmcrash xen-lowmemd +INSTALL_SBIN-$(CONFIG_X86) += xen-hvmctx xen-hvmcrash xen-lowmemd xen-mfndump INSTALL_SBIN-$(CONFIG_MIGRATE) += xen-hptool INSTALL_SBIN := $(INSTALL_SBIN-y) @@ -77,6 +77,9 @@ xenlockprof: xenlockprof.o xen-hptool: xen-hptool.o $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS) +xen-mfndump: xen-mfndump.o + $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS) + xenwatchdogd: xenwatchdogd.o $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS) diff --git a/tools/misc/xen-mfndump.c b/tools/misc/xen-mfndump.c new file mode 100644 index 0000000..3e8302e --- /dev/null +++ b/tools/misc/xen-mfndump.c @@ -0,0 +1,425 @@ +#include <xenctrl.h> +#include <xc_private.h> +#include <xc_core.h> +#include <errno.h> +#include <unistd.h> + +#include "xg_save_restore.h" + +#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0])) + +static xc_interface *xch; + +int help_func(int argc, char *argv[]) +{ + fprintf(stderr, + "Usage: xen-mfndump <command> [args]\n" + "Commands:\n" + " help show this help\n" + " dump-m2p show M2P\n" + " dump-p2m <domid> show P2M of <domid>\n" + " dump-ptes <domid> <mfn> show the PTEs in <mfn>\n" + " lookup-pte <domid> <mfn> find the PTE mapping <mfn>\n" + " memcmp-mfns <domid1> <mfn1> <domid2> <mfn2>\n" + " compare content of <mfn1> & <mfn2>\n" + ); + + return 0; +} + +int dump_m2p_func(int argc, char *argv[]) +{ + unsigned long i, max_mfn; + xen_pfn_t *m2p_table; + + if ( argc > 0 ) + { + help_func(0, NULL); + return 1; + } + + /* Map M2P and obtain gpfn */ + max_mfn = xc_maximum_ram_page(xch); + if ( !(m2p_table = xc_map_m2p(xch, max_mfn, PROT_READ, NULL)) ) + { + ERROR("Failed to map live M2P table"); + return -1; + } + + printf(" --- Dumping M2P ---\n"); + printf(" Max MFN: %lu\n", max_mfn); + for ( i = 0; i < max_mfn; i++ ) + { + printf(" mfn=0x%lx ==> pfn=0x%lx\n", i, m2p_table[i]); + } + printf(" --- End of M2P ---\n"); + + munmap(m2p_table, M2P_SIZE(max_mfn)); + return 0; +} + +int dump_p2m_func(int argc, char *argv[]) +{ + struct xc_domain_meminfo minfo; + xc_dominfo_t info; + unsigned long i; + int domid; + + if ( argc < 1 ) + { + help_func(0, NULL); + return 1; + } + domid = atoi(argv[0]); + + if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 || + info.domid != domid ) + { + ERROR("Failed to obtain info for domain %d\n", domid); + return -1; + } + + /* Retrieve all the info about the domain''s memory */ + memset(&minfo, 0, sizeof(minfo)); + if ( xc_map_domain_meminfo(xch, domid, &minfo) ) + { + ERROR("Could not map domain %d memory information\n", domid); + return -1; + } + + printf(" --- Dumping P2M for domain %d ---\n", domid); + printf(" Guest Width: %u, PT Levels: %u P2M size: = %lu\n", + minfo.guest_width, minfo.pt_levels, minfo.p2m_size); + for ( i = 0; i < minfo.p2m_size; i++ ) + { + unsigned long pagetype = minfo.pfn_type[i] & + XEN_DOMCTL_PFINFO_LTAB_MASK; + + printf(" pfn=0x%lx ==> mfn=0x%lx (type 0x%lx)", i, minfo.p2m_table[i], + pagetype >> XEN_DOMCTL_PFINFO_LTAB_SHIFT); + + if ( is_mapped(minfo.p2m_table[i]) ) + printf(" [mapped]"); + + if ( pagetype & XEN_DOMCTL_PFINFO_LPINTAB ) + printf (" [pinned]"); + + if ( pagetype == XEN_DOMCTL_PFINFO_XTAB ) + printf(" [xtab]"); + if ( pagetype == XEN_DOMCTL_PFINFO_BROKEN ) + printf(" [broken]"); + if ( pagetype == XEN_DOMCTL_PFINFO_XALLOC ) + printf( " [xalloc]"); + + switch ( pagetype & XEN_DOMCTL_PFINFO_LTABTYPE_MASK ) + { + case XEN_DOMCTL_PFINFO_L1TAB: + printf(" L1 table"); + break; + + case XEN_DOMCTL_PFINFO_L2TAB: + printf(" L2 table"); + break; + + case XEN_DOMCTL_PFINFO_L3TAB: + printf(" L3 table"); + break; + + case XEN_DOMCTL_PFINFO_L4TAB: + printf(" L4 table"); + break; + } + + printf("\n"); + } + printf(" --- End of P2M for domain %d ---\n", domid); + + xc_unmap_domain_meminfo(xch, &minfo); + return 0; +} + +int dump_ptes_func(int argc, char *argv[]) +{ + struct xc_domain_meminfo minfo; + xc_dominfo_t info; + void *page = NULL; + unsigned long i, max_mfn; + int domid, pte_num, rc = 0; + xen_pfn_t pfn, mfn, *m2p_table; + + if ( argc < 2 ) + { + help_func(0, NULL); + return 1; + } + domid = atoi(argv[0]); + mfn = strtoul(argv[1], NULL, 16); + + if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 || + info.domid != domid ) + { + ERROR("Failed to obtain info for domain %d\n", domid); + return -1; + } + + /* Retrieve all the info about the domain''s memory */ + memset(&minfo, 0, sizeof(minfo)); + if ( xc_map_domain_meminfo(xch, domid, &minfo) ) + { + ERROR("Could not map domain %d memory information\n", domid); + return -1; + } + + /* Map M2P and obtain gpfn */ + max_mfn = xc_maximum_ram_page(xch); + if ( (mfn > max_mfn) || + !(m2p_table = xc_map_m2p(xch, max_mfn, PROT_READ, NULL)) ) + { + xc_unmap_domain_meminfo(xch, &minfo); + ERROR("Failed to map live M2P table"); + return -1; + } + + pfn = m2p_table[mfn]; + if ( pfn >= minfo.p2m_size ) + { + ERROR("pfn 0x%lx out of range for domain %d\n", pfn, domid); + rc = -1; + goto out; + } + + if ( !(minfo.pfn_type[pfn] & XEN_DOMCTL_PFINFO_LTABTYPE_MASK) ) + { + ERROR("pfn 0x%lx for domain %d is not a PT\n", pfn, domid); + rc = -1; + goto out; + } + + page = xc_map_foreign_range(xch, domid, PAGE_SIZE, PROT_READ, + minfo.p2m_table[pfn]); + if ( !page ) + { + ERROR("Failed to map 0x%lx\n", minfo.p2m_table[pfn]); + rc = -1; + goto out; + } + + pte_num = PAGE_SIZE / 8; + + printf(" --- Dumping %d PTEs for domain %d ---\n", pte_num, domid); + printf(" Guest Width: %u, PT Levels: %u P2M size: = %lu\n", + minfo.guest_width, minfo.pt_levels, minfo.p2m_size); + printf(" pfn: 0x%lx, mfn: 0x%lx", + pfn, minfo.p2m_table[pfn]); + switch ( minfo.pfn_type[pfn] & XEN_DOMCTL_PFINFO_LTABTYPE_MASK ) + { + case XEN_DOMCTL_PFINFO_L1TAB: + printf(", L1 table"); + break; + case XEN_DOMCTL_PFINFO_L2TAB: + printf(", L2 table"); + break; + case XEN_DOMCTL_PFINFO_L3TAB: + printf(", L3 table"); + break; + case XEN_DOMCTL_PFINFO_L4TAB: + printf(", L4 table"); + break; + } + if ( minfo.pfn_type[pfn] & XEN_DOMCTL_PFINFO_LPINTAB ) + printf (" [pinned]"); + if ( is_mapped(minfo.p2m_table[pfn]) ) + printf(" [mapped]"); + printf("\n"); + + for ( i = 0; i < pte_num; i++ ) + printf(" pte[%lu]: 0x%lx\n", i, ((const uint64_t*)page)[i]); + + printf(" --- End of PTEs for domain %d, pfn=0x%lx (mfn=0x%lx) ---\n", + domid, pfn, minfo.p2m_table[pfn]); + + out: + munmap(page, PAGE_SIZE); + xc_unmap_domain_meminfo(xch, &minfo); + munmap(m2p_table, M2P_SIZE(max_mfn)); + return rc; +} + +int lookup_pte_func(int argc, char *argv[]) +{ + struct xc_domain_meminfo minfo; + xc_dominfo_t info; + void *page = NULL; + unsigned long i, j; + int domid, pte_num; + xen_pfn_t mfn; + + if ( argc < 2 ) + { + help_func(0, NULL); + return 1; + } + domid = atoi(argv[0]); + mfn = strtoul(argv[1], NULL, 16); + + if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 || + info.domid != domid ) + { + ERROR("Failed to obtain info for domain %d\n", domid); + return -1; + } + + /* Retrieve all the info about the domain''s memory */ + memset(&minfo, 0, sizeof(minfo)); + if ( xc_map_domain_meminfo(xch, domid, &minfo) ) + { + ERROR("Could not map domain %d memory information\n", domid); + return -1; + } + + pte_num = PAGE_SIZE / 8; + + printf(" --- Lookig for PTEs mapping mfn 0x%lx for domain %d ---\n", + mfn, domid); + printf(" Guest Width: %u, PT Levels: %u P2M size: = %lu\n", + minfo.guest_width, minfo.pt_levels, minfo.p2m_size); + + for ( i = 0; i < minfo.p2m_size; i++ ) + { + if ( !(minfo.pfn_type[i] & XEN_DOMCTL_PFINFO_LTABTYPE_MASK) ) + continue; + + page = xc_map_foreign_range(xch, domid, PAGE_SIZE, PROT_READ, + minfo.p2m_table[i]); + if ( !page ) + continue; + + for ( j = 0; j < pte_num; j++ ) + { + uint64_t pte = ((const uint64_t*)page)[j]; + +#define __MADDR_BITS_X86 ((minfo.guest_width == 8) ? 52 : 44) +#define __MFN_MASK_X86 ((1ULL << (__MADDR_BITS_X86 - PAGE_SHIFT_X86)) - 1) + if ( ((pte >> PAGE_SHIFT_X86) & __MFN_MASK_X86) == mfn) + printf(" 0x%lx <-- [0x%lx][%lu]: 0x%lx\n", + mfn, minfo.p2m_table[i], j, pte); +#undef __MADDR_BITS_X86 +#undef __MFN_MASK_X8 + } + + munmap(page, PAGE_SIZE); + page = NULL; + } + + xc_unmap_domain_meminfo(xch, &minfo); + + return 1; +} + +int memcmp_mfns_func(int argc, char *argv[]) +{ + xc_dominfo_t info1, info2; + void *page1 = NULL, *page2 = NULL; + int domid1, domid2; + xen_pfn_t mfn1, mfn2; + int rc = 0; + + if ( argc < 4 ) + { + help_func(0, NULL); + return 1; + } + domid1 = atoi(argv[0]); + domid2 = atoi(argv[2]); + mfn1 = strtoul(argv[1], NULL, 16); + mfn2 = strtoul(argv[3], NULL, 16); + + if ( xc_domain_getinfo(xch, domid1, 1, &info1) != 1 || + xc_domain_getinfo(xch, domid2, 1, &info2) != 1 || + info1.domid != domid1 || info2.domid != domid2) + { + ERROR("Failed to obtain info for domains\n"); + return -1; + } + + page1 = xc_map_foreign_range(xch, domid1, PAGE_SIZE, PROT_READ, mfn1); + page2 = xc_map_foreign_range(xch, domid2, PAGE_SIZE, PROT_READ, mfn2); + if ( !page1 || !page2 ) + { + ERROR("Failed to map either 0x%lx[dom %d] or 0x%lx[dom %d]\n", + mfn1, domid1, mfn2, domid2); + rc = -1; + goto out; + } + + printf(" --- Comparing the content of 2 MFNs ---\n"); + printf(" 1: 0x%lx[dom %d], 2: 0x%lx[dom %d]\n", + mfn1, domid1, mfn2, domid2); + printf(" memcpy(1, 2) = %d\n", memcmp(page1, page2, PAGE_SIZE)); + + out: + munmap(page1, PAGE_SIZE); + munmap(page2, PAGE_SIZE); + return rc; +} + + + +struct { + const char *name; + int (*func)(int argc, char *argv[]); +} opts[] = { + { "help", help_func }, + { "dump-m2p", dump_m2p_func }, + { "dump-p2m", dump_p2m_func }, + { "dump-ptes", dump_ptes_func }, + { "lookup-pte", lookup_pte_func }, + { "memcmp-mfns", memcmp_mfns_func}, +}; + +int main(int argc, char *argv[]) +{ + int i, ret; + + if (argc < 2) + { + help_func(0, NULL); + return 1; + } + + xch = xc_interface_open(0, 0, 0); + if ( !xch ) + { + ERROR("Failed to open an xc handler"); + return 1; + } + + for ( i = 0; i < ARRAY_SIZE(opts); i++ ) + { + if ( !strncmp(opts[i].name, argv[1], strlen(argv[1])) ) + break; + } + + if ( i == ARRAY_SIZE(opts) ) + { + fprintf(stderr, "Unknown option ''%s''", argv[1]); + help_func(0, NULL); + return 1; + } + + ret = opts[i].func(argc - 2, argv + 2); + + xc_interface_close(xch); + + return !!ret; +} + +/* + * Local variables: + * mode: C + * c-set-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */
[Cc-ing people for this e-mail too, as I apparently failed to do so with `stg mail''] On ven, 2013-07-12 at 18:47 +0200, Dario Faggioli wrote:> Hi everyone, > > I submitted some of the patches in this series before, as a part of the first > RFC of the NUMA memory migration feature. While preparing another round of it, > I thought there is enough ''meat'' already for a separate series. > > Basically, there are a few places in libxc where we issue do_domctl()-s > directly, instead of using the proper xc_xxx() wrapper. This make the code > bigger, redundant, more difficult to understand (e.g., "why xc_xxx() is called > here and is not called there?"), less consistent and less grep-able. The first > 4 patches of this series address this. > > The last one introduce a little tool I''m using for debugging. It allows one to > see the M2P of the host, the P2M of a domain, some info about the PTEs, and > perform basic searches and comparisons on them. I''m finding it very useful and, > although I do not claim for my situation to be that common, I figured it could > be nice to have it in the tree, so that is patch 6. Patch 5 is the small > rework/additions necessary to make 6 possible. > > Let me know what you think. > > Thanks and Regards, > Dario > > --- > > Dario Faggioli (6): > libxc: introduce xc_domain_get_address_size > libxc: use xc_vcpu_setcontext() instead of calling do_domctl() > libxc: use xc_vcpu_getinfo() instead of calling do_domctl() > libxc: allow for ctxt to be NULL in xc_vcpu_setcontext > libxc: introduce xc_map_domain_meminfo (and xc_unmap_domain_meminfo) > tools/misc: introduce xen-mfndump. > > > tools/libxc/xc_core.c | 21 -- > tools/libxc/xc_cpuid_x86.c | 8 - > tools/libxc/xc_dom_boot.c | 28 +-- > tools/libxc/xc_domain.c | 149 +++++++++++++- > tools/libxc/xc_domain_restore.c | 13 - > tools/libxc/xc_offline_page.c | 192 +++--------------- > tools/libxc/xc_pagetab.c | 8 - > tools/libxc/xc_private.c | 8 - > tools/libxc/xc_resume.c | 23 +- > tools/libxc/xenctrl.h | 12 + > tools/libxc/xenguest.h | 17 ++ > tools/libxc/xg_private.h | 9 + > tools/libxc/xg_save_restore.h | 9 - > tools/misc/Makefile | 7 - > tools/misc/xen-mfndump.c | 425 +++++++++++++++++++++++++++++++++++++++ > tools/xentrace/xenctx.c | 9 - > 16 files changed, 677 insertions(+), 261 deletions(-) > create mode 100644 tools/misc/xen-mfndump.c >-- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Andrew Cooper
2013-Jul-12 17:23 UTC
Re: [PATCH 1/6] libxc: introduce xc_domain_get_address_size
On 12/07/13 17:48, Dario Faggioli wrote:> As a wrapper to XEN_DOMCTL_get_address_size, and use it > wherever the call was being issued directly via do_domctl(), > saving quite some line of code. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>While this is certainly a sensible improvement, there seems to be some confusion in the code.> --- > tools/libxc/xc_core.c | 21 ++------------------- > tools/libxc/xc_cpuid_x86.c | 8 +++----- > tools/libxc/xc_domain.c | 15 +++++++++++++++ > tools/libxc/xc_offline_page.c | 9 ++------- > tools/libxc/xc_pagetab.c | 8 +++----- > tools/libxc/xc_resume.c | 23 ++++++----------------- > tools/libxc/xenctrl.h | 12 ++++++++++++ > tools/libxc/xg_save_restore.h | 9 ++------- > tools/xentrace/xenctx.c | 9 +++------ > 9 files changed, 48 insertions(+), 66 deletions(-) > > diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c > index 4207eed..c8bade5 100644 > --- a/tools/libxc/xc_core.c > +++ b/tools/libxc/xc_core.c > @@ -417,24 +417,6 @@ elfnote_dump_format_version(xc_interface *xch, > return dump_rtn(xch, args, (char*)&format_version, sizeof(format_version)); > } > > -static int > -get_guest_width(xc_interface *xch, > - uint32_t domid, > - unsigned int *guest_width) > -{ > - DECLARE_DOMCTL; > - > - memset(&domctl, 0, sizeof(domctl)); > - domctl.domain = domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - > - if ( do_domctl(xch, &domctl) != 0 ) > - return 1; > - > - *guest_width = domctl.u.address_size.size / 8; > - return 0; > -} > -Here, guest_width is measured in bytes.> int > xc_domain_dumpcore_via_callback(xc_interface *xch, > uint32_t domid, > @@ -478,11 +460,12 @@ xc_domain_dumpcore_via_callback(xc_interface *xch, > struct xc_core_section_headers *sheaders = NULL; > Elf64_Shdr *shdr; > > - if ( get_guest_width(xch, domid, &dinfo->guest_width) != 0 ) > + if ( xc_domain_get_address_size(xch, domid, &dinfo->guest_width) != 0 ) > { > PERROR("Could not get address size for domain"); > return sts; > } > + dinfo->guest_width /= 8;This looks nasty (and frankly looks wrong to a cursory glance), and is because of the functional difference between get_guest_width() and xc_domain_get_address_size()> > xc_core_arch_context_init(&arch_ctxt); > if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL ) > diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c > index fa47787..99e3997 100644 > --- a/tools/libxc/xc_cpuid_x86.c > +++ b/tools/libxc/xc_cpuid_x86.c > @@ -436,17 +436,15 @@ static void xc_cpuid_pv_policy( > const unsigned int *input, unsigned int *regs) > { > DECLARE_DOMCTL; > + unsigned int guest_width; > int guest_64bit, xen_64bit = hypervisor_is_64bit(xch); > char brand[13]; > uint64_t xfeature_mask; > > xc_cpuid_brand_get(brand); > > - memset(&domctl, 0, sizeof(domctl)); > - domctl.domain = domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - do_domctl(xch, &domctl); > - guest_64bit = (domctl.u.address_size.size == 64); > + xc_domain_get_address_size(xch, domid, &guest_width); > + guest_64bit = (guest_width == 64);guest_width here is now measured in bytes. Also, error checking? I know the old code also failed in that regard.> > /* Detecting Xen''s atitude towards XSAVE */ > memset(&domctl, 0, sizeof(domctl)); > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > index 3257e2a..d64d0bc 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -270,6 +270,21 @@ out: > return ret; > } > > +int xc_domain_get_address_size(xc_interface *xch, uint32_t domid, > + unsigned int *addr_size) > +{ > + DECLARE_DOMCTL; > + > + memset(&domctl, 0, sizeof(domctl)); > + domctl.domain = domid; > + domctl.cmd = XEN_DOMCTL_get_address_size; > + > + if ( do_domctl(xch, &domctl) != 0 ) > + return 1; > + > + *addr_size = domctl.u.address_size.size; > + return 0; > +} > > int xc_domain_getinfo(xc_interface *xch, > uint32_t first_domid, > diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c > index 36b9812..c5547a8 100644 > --- a/tools/libxc/xc_offline_page.c > +++ b/tools/libxc/xc_offline_page.c > @@ -193,20 +193,15 @@ static int get_pt_level(xc_interface *xch, uint32_t domid, > unsigned int *pt_level, > unsigned int *gwidth) > { > - DECLARE_DOMCTL; > xen_capabilities_info_t xen_caps = ""; > > if (xc_version(xch, XENVER_capabilities, &xen_caps) != 0) > return -1; > > - memset(&domctl, 0, sizeof(domctl)); > - domctl.domain = domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - > - if ( do_domctl(xch, &domctl) != 0 ) > + if (xc_domain_get_address_size(xch, domid, gwidth) != 0) > return -1; > > - *gwidth = domctl.u.address_size.size / 8; > + *gwidth /= 8;gwidth is now again measured in bytes.> > if (strstr(xen_caps, "xen-3.0-x86_64")) > /* Depends on whether it''s a compat 32-on-64 guest */ > diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c > index 27c4e9f..5937a52 100644 > --- a/tools/libxc/xc_pagetab.c > +++ b/tools/libxc/xc_pagetab.c > @@ -51,15 +51,13 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, > pt_levels = (ctx.msr_efer&EFER_LMA) ? 4 : (ctx.cr4&CR4_PAE) ? 3 : 2; > paddr = ctx.cr3 & ((pt_levels == 3) ? ~0x1full : ~0xfffull); > } else { > - DECLARE_DOMCTL; > + unsigned int gwidth; > vcpu_guest_context_any_t ctx; > if (xc_vcpu_getcontext(xch, dom, vcpu, &ctx) != 0) > return 0; > - domctl.domain = dom; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - if ( do_domctl(xch, &domctl) != 0 ) > + if (xc_domain_get_address_size(xch, dom, &gwidth) != 0) > return 0; > - if (domctl.u.address_size.size == 64) { > + if (gwidth == 64) {but in bits here. ( I shall ignore pointing out the same further down)> pt_levels = 4; > paddr = (uint64_t)xen_cr3_to_pfn_x86_64(ctx.x64.ctrlreg[3]) > << PAGE_SHIFT; > diff --git a/tools/libxc/xc_resume.c b/tools/libxc/xc_resume.c > index 1c43ec6..58ea395 100644 > --- a/tools/libxc/xc_resume.c > +++ b/tools/libxc/xc_resume.c > @@ -24,19 +24,6 @@ > #include <xen/foreign/x86_64.h> > #include <xen/hvm/params.h> > > -static int pv_guest_width(xc_interface *xch, uint32_t domid) > -{ > - DECLARE_DOMCTL; > - domctl.domain = domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - if ( xc_domctl(xch, &domctl) != 0 ) > - { > - PERROR("Could not get guest address size"); > - return -1; > - } > - return domctl.u.address_size.size / 8; > -} > - > static int modify_returncode(xc_interface *xch, uint32_t domid) > { > vcpu_guest_context_any_t ctxt; > @@ -71,9 +58,9 @@ static int modify_returncode(xc_interface *xch, uint32_t domid) > else > { > /* Probe PV guest address width. */ > - dinfo->guest_width = pv_guest_width(xch, domid); > - if ( dinfo->guest_width < 0 ) > + if ( xc_domain_get_address_size(xch, domid, &dinfo->guest_width) ) > return -1; > + dinfo->guest_width /= 8; > } > > if ( (rc = xc_vcpu_getcontext(xch, domid, 0, &ctxt)) != 0 ) > @@ -120,7 +107,8 @@ static int xc_domain_resume_any(xc_interface *xch, uint32_t domid) > xc_dominfo_t info; > int i, rc = -1; > #if defined(__i386__) || defined(__x86_64__) > - struct domain_info_context _dinfo = { .p2m_size = 0 }; > + struct domain_info_context _dinfo = { .guest_width = 0, > + .p2m_size = 0 }; > struct domain_info_context *dinfo = &_dinfo; > unsigned long mfn; > vcpu_guest_context_any_t ctxt; > @@ -147,7 +135,8 @@ static int xc_domain_resume_any(xc_interface *xch, uint32_t domid) > return rc; > } > > - dinfo->guest_width = pv_guest_width(xch, domid); > + xc_domain_get_address_size(xch, domid, &dinfo->guest_width); > + dinfo->guest_width /= 8; > if ( dinfo->guest_width != sizeof(long) ) > { > ERROR("Cannot resume uncooperative cross-address-size guests"); > diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h > index 388a9c3..907106e 100644 > --- a/tools/libxc/xenctrl.h > +++ b/tools/libxc/xenctrl.h > @@ -561,6 +561,18 @@ int xc_vcpu_getaffinity(xc_interface *xch, > int vcpu, > xc_cpumap_t cpumap); > > + > +/** > + * This function will return the address size for the specified domain. > + * > + * @param xch a handle to an open hypervisor interface. > + * @param domid the domain id one wants the address size width of. > + * @param addr_size the address size. > + */ > +int xc_domain_get_address_size(xc_interface *xch, uint32_t domid, > + unsigned int *addr_size); > + > + > /** > * This function will return information about one or more domains. It is > * designed to iterate over the list of domains. If a single domain is > diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h > index 6512003..3c11c44 100644 > --- a/tools/libxc/xg_save_restore.h > +++ b/tools/libxc/xg_save_restore.h > @@ -301,7 +301,6 @@ static inline int get_platform_info(xc_interface *xch, uint32_t dom, > { > xen_capabilities_info_t xen_caps = ""; > xen_platform_parameters_t xen_params; > - DECLARE_DOMCTL; > > if (xc_version(xch, XENVER_platform_parameters, &xen_params) != 0) > return 0; > @@ -313,14 +312,10 @@ static inline int get_platform_info(xc_interface *xch, uint32_t dom, > > *hvirt_start = xen_params.virt_start; > > - memset(&domctl, 0, sizeof(domctl)); > - domctl.domain = dom; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - > - if ( do_domctl(xch, &domctl) != 0 ) > + if ( xc_domain_get_address_size(xch, dom, guest_width) != 0) > return 0; > > - *guest_width = domctl.u.address_size.size / 8; > + *guest_width /= 8; > > /* 64-bit tools will see the 64-bit hvirt_start, but 32-bit guests > * will be using the compat one. */ > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > index 060e480..ae4d6a7 100644 > --- a/tools/xentrace/xenctx.c > +++ b/tools/xentrace/xenctx.c > @@ -771,12 +771,9 @@ static void dump_ctx(int vcpu) > } > ctxt_word_size = (strstr(xen_caps, "xen-3.0-x86_64")) ? 8 : 4; > } else { > - struct xen_domctl domctl; > - memset(&domctl, 0, sizeof domctl); > - domctl.domain = xenctx.domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - if (xc_domctl(xenctx.xc_handle, &domctl) == 0) > - ctxt_word_size = guest_word_size = domctl.u.address_size.size / 8; > + unsigned int gw; > + if ( !xc_domain_get_address_size(xenctx.xc_handle, xenctx.domid, &gw) ) > + ctxt_word_size = guest_word_size = gw / 8; > } > } > #endifAlmost all of the "/= 8"s could be removed by moving it into xc_domain_get_address_size(). I suggest renaming xc_domain_get_address_size() to xc_domain_get_address_width() and changing the one location which checks against 64 (bits) to check against 8 (bytes). ~Andrew> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Andrew Cooper
2013-Jul-12 17:26 UTC
Re: [PATCH 2/6] libxc: use xc_vcpu_setcontext() instead of calling do_domctl()
On 12/07/13 17:48, Dario Faggioli wrote:> The wrapper is there already, so better use it in place of > all the stuff required to issue a call to do_domctl() for > XEN_DOMCTL_setvcpucontext. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>> --- > tools/libxc/xc_dom_boot.c | 14 ++++---------- > tools/libxc/xc_domain_restore.c | 6 +----- > 2 files changed, 5 insertions(+), 15 deletions(-) > > diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c > index d4d57b4..cf509fa 100644 > --- a/tools/libxc/xc_dom_boot.c > +++ b/tools/libxc/xc_dom_boot.c > @@ -62,19 +62,13 @@ static int setup_hypercall_page(struct xc_dom_image *dom) > return rc; > } > > -static int launch_vm(xc_interface *xch, domid_t domid, xc_hypercall_buffer_t *ctxt) > +static int launch_vm(xc_interface *xch, domid_t domid, > + vcpu_guest_context_any_t *ctxt) > { > - DECLARE_DOMCTL; > - DECLARE_HYPERCALL_BUFFER_ARGUMENT(ctxt); > int rc; > > xc_dom_printf(xch, "%s: called, ctxt=%p", __FUNCTION__, ctxt); > - memset(&domctl, 0, sizeof(domctl)); > - domctl.cmd = XEN_DOMCTL_setvcpucontext; > - domctl.domain = domid; > - domctl.u.vcpucontext.vcpu = 0; > - set_xen_guest_handle(domctl.u.vcpucontext.ctxt, ctxt); > - rc = do_domctl(xch, &domctl); > + rc = xc_vcpu_setcontext(xch, domid, 0, ctxt); > if ( rc != 0 ) > xc_dom_panic(xch, XC_INTERNAL_ERROR, > "%s: SETVCPUCONTEXT failed (rc=%d)", __FUNCTION__, rc); > @@ -270,7 +264,7 @@ int xc_dom_boot_image(struct xc_dom_image *dom) > if ( (rc = dom->arch_hooks->vcpu(dom, ctxt)) != 0 ) > return rc; > xc_dom_unmap_all(dom); > - rc = launch_vm(dom->xch, dom->guest_domid, HYPERCALL_BUFFER(ctxt)); > + rc = launch_vm(dom->xch, dom->guest_domid, ctxt); > > xc_hypercall_buffer_free(dom->xch, ctxt); > return rc; > diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c > index 63d36cd..41a63cb 100644 > --- a/tools/libxc/xc_domain_restore.c > +++ b/tools/libxc/xc_domain_restore.c > @@ -2113,11 +2113,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, > } > ctxt->x64.ctrlreg[1] = FOLD_CR3(ctx->p2m[pfn]); > } > - domctl.cmd = XEN_DOMCTL_setvcpucontext; > - domctl.domain = (domid_t)dom; > - domctl.u.vcpucontext.vcpu = i; > - set_xen_guest_handle(domctl.u.vcpucontext.ctxt, ctxt); > - frc = xc_domctl(xch, &domctl); > + frc = xc_vcpu_setcontext(xch, dom, i, ctxt); > if ( frc != 0 ) > { > PERROR("Couldn''t build vcpu%d", i); > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Fri, Jul 12, 2013 at 06:48:00PM +0200, Dario Faggioli wrote:> As a wrapper to XEN_DOMCTL_get_address_size, and use it > wherever the call was being issued directly via do_domctl(), > saving quite some line of code. >So while you''re at it, could you please make use of your function in xc_dom_x86.c:xc_domain_get_native_protocol as well. Wei.> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > --- > tools/libxc/xc_core.c | 21 ++------------------- > tools/libxc/xc_cpuid_x86.c | 8 +++----- > tools/libxc/xc_domain.c | 15 +++++++++++++++ > tools/libxc/xc_offline_page.c | 9 ++------- > tools/libxc/xc_pagetab.c | 8 +++----- > tools/libxc/xc_resume.c | 23 ++++++----------------- > tools/libxc/xenctrl.h | 12 ++++++++++++ > tools/libxc/xg_save_restore.h | 9 ++------- > tools/xentrace/xenctx.c | 9 +++------ > 9 files changed, 48 insertions(+), 66 deletions(-) > > diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c > index 4207eed..c8bade5 100644 > --- a/tools/libxc/xc_core.c > +++ b/tools/libxc/xc_core.c > @@ -417,24 +417,6 @@ elfnote_dump_format_version(xc_interface *xch, > return dump_rtn(xch, args, (char*)&format_version, sizeof(format_version)); > } > > -static int > -get_guest_width(xc_interface *xch, > - uint32_t domid, > - unsigned int *guest_width) > -{ > - DECLARE_DOMCTL; > - > - memset(&domctl, 0, sizeof(domctl)); > - domctl.domain = domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - > - if ( do_domctl(xch, &domctl) != 0 ) > - return 1; > - > - *guest_width = domctl.u.address_size.size / 8; > - return 0; > -} > - > int > xc_domain_dumpcore_via_callback(xc_interface *xch, > uint32_t domid, > @@ -478,11 +460,12 @@ xc_domain_dumpcore_via_callback(xc_interface *xch, > struct xc_core_section_headers *sheaders = NULL; > Elf64_Shdr *shdr; > > - if ( get_guest_width(xch, domid, &dinfo->guest_width) != 0 ) > + if ( xc_domain_get_address_size(xch, domid, &dinfo->guest_width) != 0 ) > { > PERROR("Could not get address size for domain"); > return sts; > } > + dinfo->guest_width /= 8; > > xc_core_arch_context_init(&arch_ctxt); > if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL ) > diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c > index fa47787..99e3997 100644 > --- a/tools/libxc/xc_cpuid_x86.c > +++ b/tools/libxc/xc_cpuid_x86.c > @@ -436,17 +436,15 @@ static void xc_cpuid_pv_policy( > const unsigned int *input, unsigned int *regs) > { > DECLARE_DOMCTL; > + unsigned int guest_width; > int guest_64bit, xen_64bit = hypervisor_is_64bit(xch); > char brand[13]; > uint64_t xfeature_mask; > > xc_cpuid_brand_get(brand); > > - memset(&domctl, 0, sizeof(domctl)); > - domctl.domain = domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - do_domctl(xch, &domctl); > - guest_64bit = (domctl.u.address_size.size == 64); > + xc_domain_get_address_size(xch, domid, &guest_width); > + guest_64bit = (guest_width == 64); > > /* Detecting Xen''s atitude towards XSAVE */ > memset(&domctl, 0, sizeof(domctl)); > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > index 3257e2a..d64d0bc 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -270,6 +270,21 @@ out: > return ret; > } > > +int xc_domain_get_address_size(xc_interface *xch, uint32_t domid, > + unsigned int *addr_size) > +{ > + DECLARE_DOMCTL; > + > + memset(&domctl, 0, sizeof(domctl)); > + domctl.domain = domid; > + domctl.cmd = XEN_DOMCTL_get_address_size; > + > + if ( do_domctl(xch, &domctl) != 0 ) > + return 1; > + > + *addr_size = domctl.u.address_size.size; > + return 0; > +} > > int xc_domain_getinfo(xc_interface *xch, > uint32_t first_domid, > diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c > index 36b9812..c5547a8 100644 > --- a/tools/libxc/xc_offline_page.c > +++ b/tools/libxc/xc_offline_page.c > @@ -193,20 +193,15 @@ static int get_pt_level(xc_interface *xch, uint32_t domid, > unsigned int *pt_level, > unsigned int *gwidth) > { > - DECLARE_DOMCTL; > xen_capabilities_info_t xen_caps = ""; > > if (xc_version(xch, XENVER_capabilities, &xen_caps) != 0) > return -1; > > - memset(&domctl, 0, sizeof(domctl)); > - domctl.domain = domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - > - if ( do_domctl(xch, &domctl) != 0 ) > + if (xc_domain_get_address_size(xch, domid, gwidth) != 0) > return -1; > > - *gwidth = domctl.u.address_size.size / 8; > + *gwidth /= 8; > > if (strstr(xen_caps, "xen-3.0-x86_64")) > /* Depends on whether it''s a compat 32-on-64 guest */ > diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c > index 27c4e9f..5937a52 100644 > --- a/tools/libxc/xc_pagetab.c > +++ b/tools/libxc/xc_pagetab.c > @@ -51,15 +51,13 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom, > pt_levels = (ctx.msr_efer&EFER_LMA) ? 4 : (ctx.cr4&CR4_PAE) ? 3 : 2; > paddr = ctx.cr3 & ((pt_levels == 3) ? ~0x1full : ~0xfffull); > } else { > - DECLARE_DOMCTL; > + unsigned int gwidth; > vcpu_guest_context_any_t ctx; > if (xc_vcpu_getcontext(xch, dom, vcpu, &ctx) != 0) > return 0; > - domctl.domain = dom; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - if ( do_domctl(xch, &domctl) != 0 ) > + if (xc_domain_get_address_size(xch, dom, &gwidth) != 0) > return 0; > - if (domctl.u.address_size.size == 64) { > + if (gwidth == 64) { > pt_levels = 4; > paddr = (uint64_t)xen_cr3_to_pfn_x86_64(ctx.x64.ctrlreg[3]) > << PAGE_SHIFT; > diff --git a/tools/libxc/xc_resume.c b/tools/libxc/xc_resume.c > index 1c43ec6..58ea395 100644 > --- a/tools/libxc/xc_resume.c > +++ b/tools/libxc/xc_resume.c > @@ -24,19 +24,6 @@ > #include <xen/foreign/x86_64.h> > #include <xen/hvm/params.h> > > -static int pv_guest_width(xc_interface *xch, uint32_t domid) > -{ > - DECLARE_DOMCTL; > - domctl.domain = domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - if ( xc_domctl(xch, &domctl) != 0 ) > - { > - PERROR("Could not get guest address size"); > - return -1; > - } > - return domctl.u.address_size.size / 8; > -} > - > static int modify_returncode(xc_interface *xch, uint32_t domid) > { > vcpu_guest_context_any_t ctxt; > @@ -71,9 +58,9 @@ static int modify_returncode(xc_interface *xch, uint32_t domid) > else > { > /* Probe PV guest address width. */ > - dinfo->guest_width = pv_guest_width(xch, domid); > - if ( dinfo->guest_width < 0 ) > + if ( xc_domain_get_address_size(xch, domid, &dinfo->guest_width) ) > return -1; > + dinfo->guest_width /= 8; > } > > if ( (rc = xc_vcpu_getcontext(xch, domid, 0, &ctxt)) != 0 ) > @@ -120,7 +107,8 @@ static int xc_domain_resume_any(xc_interface *xch, uint32_t domid) > xc_dominfo_t info; > int i, rc = -1; > #if defined(__i386__) || defined(__x86_64__) > - struct domain_info_context _dinfo = { .p2m_size = 0 }; > + struct domain_info_context _dinfo = { .guest_width = 0, > + .p2m_size = 0 }; > struct domain_info_context *dinfo = &_dinfo; > unsigned long mfn; > vcpu_guest_context_any_t ctxt; > @@ -147,7 +135,8 @@ static int xc_domain_resume_any(xc_interface *xch, uint32_t domid) > return rc; > } > > - dinfo->guest_width = pv_guest_width(xch, domid); > + xc_domain_get_address_size(xch, domid, &dinfo->guest_width); > + dinfo->guest_width /= 8; > if ( dinfo->guest_width != sizeof(long) ) > { > ERROR("Cannot resume uncooperative cross-address-size guests"); > diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h > index 388a9c3..907106e 100644 > --- a/tools/libxc/xenctrl.h > +++ b/tools/libxc/xenctrl.h > @@ -561,6 +561,18 @@ int xc_vcpu_getaffinity(xc_interface *xch, > int vcpu, > xc_cpumap_t cpumap); > > + > +/** > + * This function will return the address size for the specified domain. > + * > + * @param xch a handle to an open hypervisor interface. > + * @param domid the domain id one wants the address size width of. > + * @param addr_size the address size. > + */ > +int xc_domain_get_address_size(xc_interface *xch, uint32_t domid, > + unsigned int *addr_size); > + > + > /** > * This function will return information about one or more domains. It is > * designed to iterate over the list of domains. If a single domain is > diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h > index 6512003..3c11c44 100644 > --- a/tools/libxc/xg_save_restore.h > +++ b/tools/libxc/xg_save_restore.h > @@ -301,7 +301,6 @@ static inline int get_platform_info(xc_interface *xch, uint32_t dom, > { > xen_capabilities_info_t xen_caps = ""; > xen_platform_parameters_t xen_params; > - DECLARE_DOMCTL; > > if (xc_version(xch, XENVER_platform_parameters, &xen_params) != 0) > return 0; > @@ -313,14 +312,10 @@ static inline int get_platform_info(xc_interface *xch, uint32_t dom, > > *hvirt_start = xen_params.virt_start; > > - memset(&domctl, 0, sizeof(domctl)); > - domctl.domain = dom; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - > - if ( do_domctl(xch, &domctl) != 0 ) > + if ( xc_domain_get_address_size(xch, dom, guest_width) != 0) > return 0; > > - *guest_width = domctl.u.address_size.size / 8; > + *guest_width /= 8; > > /* 64-bit tools will see the 64-bit hvirt_start, but 32-bit guests > * will be using the compat one. */ > diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c > index 060e480..ae4d6a7 100644 > --- a/tools/xentrace/xenctx.c > +++ b/tools/xentrace/xenctx.c > @@ -771,12 +771,9 @@ static void dump_ctx(int vcpu) > } > ctxt_word_size = (strstr(xen_caps, "xen-3.0-x86_64")) ? 8 : 4; > } else { > - struct xen_domctl domctl; > - memset(&domctl, 0, sizeof domctl); > - domctl.domain = xenctx.domid; > - domctl.cmd = XEN_DOMCTL_get_address_size; > - if (xc_domctl(xenctx.xc_handle, &domctl) == 0) > - ctxt_word_size = guest_word_size = domctl.u.address_size.size / 8; > + unsigned int gw; > + if ( !xc_domain_get_address_size(xenctx.xc_handle, xenctx.domid, &gw) ) > + ctxt_word_size = guest_word_size = gw / 8; > } > } > #endif > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Andrew Cooper
2013-Jul-12 17:35 UTC
Re: [PATCH 3/6] libxc: use xc_vcpu_getinfo() instead of calling do_domctl()
On 12/07/13 17:48, Dario Faggioli wrote:> The wrapper is there already, so better use it in place of > all the stuff required to issue a call to do_domctl() for > XEN_DOMCTL_getdomaininfo. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > --- > tools/libxc/xc_dom_boot.c | 14 ++++++-------- > tools/libxc/xc_domain_restore.c | 7 +++---- > tools/libxc/xc_private.c | 8 +++----- > 3 files changed, 12 insertions(+), 17 deletions(-) > > diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c > index cf509fa..69b5c9c 100644 > --- a/tools/libxc/xc_dom_boot.c > +++ b/tools/libxc/xc_dom_boot.c > @@ -197,8 +197,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, xen_pfn_t pfn, > > int xc_dom_boot_image(struct xc_dom_image *dom) > { > - DECLARE_DOMCTL; > DECLARE_HYPERCALL_BUFFER(vcpu_guest_context_any_t, ctxt); > + xc_dominfo_t info; > int rc; > > ctxt = xc_hypercall_buffer_alloc(dom->xch, ctxt, sizeof(*ctxt)); > @@ -212,23 +212,21 @@ int xc_dom_boot_image(struct xc_dom_image *dom) > return rc; > > /* collect some info */ > - domctl.cmd = XEN_DOMCTL_getdomaininfo; > - domctl.domain = dom->guest_domid; > - rc = do_domctl(dom->xch, &domctl); > - if ( rc != 0 ) > + rc = xc_domain_getinfo(dom->xch, dom->guest_domid, 1, &info); > + if ( rc != 1 ) > { > xc_dom_panic(dom->xch, XC_INTERNAL_ERROR, > "%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc); > return rc;With the rc from do_domctl() different to the rc from xc_domain_getinfo(), this error print and return is possibly wrong. An rc of 0 is a now a valid failure case, meaning "I cant find any domains". ~Andrew> } > - if ( domctl.domain != dom->guest_domid ) > + if ( info.domid != dom->guest_domid ) > { > xc_dom_panic(dom->xch, XC_INTERNAL_ERROR, > "%s: Huh? domid mismatch (%d != %d)", __FUNCTION__, > - domctl.domain, dom->guest_domid); > + info.domid, dom->guest_domid); > return -1; > } > - dom->shared_info_mfn = domctl.u.getdomaininfo.shared_info_frame; > + dom->shared_info_mfn = info.shared_info_frame; > > /* sanity checks */ > if ( !xc_dom_compat_check(dom) ) > diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c > index 41a63cb..b418963 100644 > --- a/tools/libxc/xc_domain_restore.c > +++ b/tools/libxc/xc_domain_restore.c > @@ -1406,6 +1406,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, > struct restore_callbacks *callbacks) > { > DECLARE_DOMCTL; > + xc_dominfo_t info; > int rc = 1, frc, i, j, n, m, pae_extended_cr3 = 0, ext_vcpucontext = 0; > int vcpuextstate = 0; > uint32_t vcpuextstate_size = 0; > @@ -1562,14 +1563,12 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, > ROUNDUP(MAX_BATCH_SIZE * sizeof(xen_pfn_t), PAGE_SHIFT)); > > /* Get the domain''s shared-info frame. */ > - domctl.cmd = XEN_DOMCTL_getdomaininfo; > - domctl.domain = (domid_t)dom; > - if ( xc_domctl(xch, &domctl) < 0 ) > + if ( xc_domain_getinfo(xch, (domid_t)dom, 1, &info) != 1 ) > { > PERROR("Could not get information on new domain"); > goto out; > } > - shared_info_frame = domctl.u.getdomaininfo.shared_info_frame; > + shared_info_frame = info.shared_info_frame; > > /* Mark all PFNs as invalid; we allocate on demand */ > for ( pfn = 0; pfn < dinfo->p2m_size; pfn++ ) > diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c > index acaf9e0..a260257 100644 > --- a/tools/libxc/xc_private.c > +++ b/tools/libxc/xc_private.c > @@ -609,11 +609,9 @@ int xc_get_pfn_list(xc_interface *xch, > > long xc_get_tot_pages(xc_interface *xch, uint32_t domid) > { > - DECLARE_DOMCTL; > - domctl.cmd = XEN_DOMCTL_getdomaininfo; > - domctl.domain = (domid_t)domid; > - return (do_domctl(xch, &domctl) < 0) ? > - -1 : domctl.u.getdomaininfo.tot_pages; > + xc_dominfo_t info; > + return (xc_domain_getinfo(xch, domid, 1, &info) != 1) ? > + -1 : info.nr_pages; > } > > int xc_copy_to_domain_page(xc_interface *xch, > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Andrew Cooper
2013-Jul-12 17:38 UTC
Re: [PATCH 4/6] libxc: allow for ctxt to be NULL in xc_vcpu_setcontext
On 12/07/13 17:48, Dario Faggioli wrote:> Since, as can be seen in xen/common/domctl.c, that is legitimate. > Actually, it is the only way to have vcpu_reset() invoked from > outside Xen. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Jan Beulich <JBeulich@suse.com> > Cc: Keir Fraser <keir@xen.org>"it is the only way to have vcpu_reset() invoked from outside Xen." without manually constructing a vcpu_op. Still, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>> --- > tools/libxc/xc_domain.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > index d64d0bc..a247426 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -1140,12 +1140,6 @@ int xc_vcpu_setcontext(xc_interface *xch, > DECLARE_HYPERCALL_BOUNCE(ctxt, sizeof(vcpu_guest_context_any_t), XC_HYPERCALL_BUFFER_BOUNCE_IN); > int rc; > > - if (ctxt == NULL) > - { > - errno = EINVAL; > - return -1; > - } > - > if ( xc_hypercall_bounce_pre(xch, ctxt) ) > return -1; > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel