All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] xen-mfndump and some libxc cleanups
@ 2013-07-12 16:47 Dario Faggioli
  2013-07-12 16:48 ` [PATCH 1/6] libxc: introduce xc_domain_get_address_size Dario Faggioli
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Dario Faggioli @ 2013-07-12 16:47 UTC (permalink / raw)
  To: xen-devel

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)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/6] libxc: introduce xc_domain_get_address_size
  2013-07-12 16:47 [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
@ 2013-07-12 16:48 ` Dario Faggioli
  2013-07-12 17:23   ` Andrew Cooper
  2013-07-12 17:30   ` Wei Liu
  2013-07-12 16:48 ` [PATCH 2/6] libxc: use xc_vcpu_setcontext() instead of calling do_domctl() Dario Faggioli
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 13+ messages in thread
From: Dario Faggioli @ 2013-07-12 16:48 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Ian Jackson, Ian Campbell

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/6] libxc: use xc_vcpu_setcontext() instead of calling do_domctl()
  2013-07-12 16:47 [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
  2013-07-12 16:48 ` [PATCH 1/6] libxc: introduce xc_domain_get_address_size Dario Faggioli
@ 2013-07-12 16:48 ` Dario Faggioli
  2013-07-12 17:26   ` Andrew Cooper
  2013-07-12 16:48 ` [PATCH 3/6] libxc: use xc_vcpu_getinfo() " Dario Faggioli
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Dario Faggioli @ 2013-07-12 16:48 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Ian Jackson, Ian Campbell

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);

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/6] libxc: use xc_vcpu_getinfo() instead of calling do_domctl()
  2013-07-12 16:47 [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
  2013-07-12 16:48 ` [PATCH 1/6] libxc: introduce xc_domain_get_address_size Dario Faggioli
  2013-07-12 16:48 ` [PATCH 2/6] libxc: use xc_vcpu_setcontext() instead of calling do_domctl() Dario Faggioli
@ 2013-07-12 16:48 ` Dario Faggioli
  2013-07-12 17:35   ` Andrew Cooper
  2013-07-12 16:48 ` [PATCH 4/6] libxc: allow for ctxt to be NULL in xc_vcpu_setcontext Dario Faggioli
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Dario Faggioli @ 2013-07-12 16:48 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Ian Jackson, Ian Campbell

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,

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/6] libxc: allow for ctxt to be NULL in xc_vcpu_setcontext
  2013-07-12 16:47 [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
                   ` (2 preceding siblings ...)
  2013-07-12 16:48 ` [PATCH 3/6] libxc: use xc_vcpu_getinfo() " Dario Faggioli
@ 2013-07-12 16:48 ` Dario Faggioli
  2013-07-12 17:38   ` Andrew Cooper
  2013-07-12 16:48 ` [PATCH 5/6] libxc: introduce xc_map_domain_meminfo (and xc_unmap_domain_meminfo) Dario Faggioli
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Dario Faggioli @ 2013-07-12 16:48 UTC (permalink / raw)
  To: xen-devel
  Cc: George Dunlap, Ian Jackson, Keir Fraser, Ian Campbell, Jan Beulich

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;

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/6] libxc: introduce xc_map_domain_meminfo (and xc_unmap_domain_meminfo)
  2013-07-12 16:47 [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
                   ` (3 preceding siblings ...)
  2013-07-12 16:48 ` [PATCH 4/6] libxc: allow for ctxt to be NULL in xc_vcpu_setcontext Dario Faggioli
@ 2013-07-12 16:48 ` Dario Faggioli
  2013-07-12 16:48 ` [PATCH 6/6] tools/misc: introduce xen-mfndump Dario Faggioli
  2013-07-12 16:57 ` [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
  6 siblings, 0 replies; 13+ messages in thread
From: Dario Faggioli @ 2013-07-12 16:48 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Ian Jackson, Ian Campbell

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))

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/6] tools/misc: introduce xen-mfndump.
  2013-07-12 16:47 [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
                   ` (4 preceding siblings ...)
  2013-07-12 16:48 ` [PATCH 5/6] libxc: introduce xc_map_domain_meminfo (and xc_unmap_domain_meminfo) Dario Faggioli
@ 2013-07-12 16:48 ` Dario Faggioli
  2013-07-12 16:57 ` [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
  6 siblings, 0 replies; 13+ messages in thread
From: Dario Faggioli @ 2013-07-12 16:48 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Ian Campbell, George Dunlap, Tim Deegan, Ian Jackson,
	Jan Beulich

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:
+ */

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/6] xen-mfndump and some libxc cleanups
  2013-07-12 16:47 [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
                   ` (5 preceding siblings ...)
  2013-07-12 16:48 ` [PATCH 6/6] tools/misc: introduce xen-mfndump Dario Faggioli
@ 2013-07-12 16:57 ` Dario Faggioli
  6 siblings, 0 replies; 13+ messages in thread
From: Dario Faggioli @ 2013-07-12 16:57 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Ian Jackson, Ian Campbell


[-- Attachment #1.1: Type: text/plain, Size: 2880 bytes --]

[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)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] libxc: introduce xc_domain_get_address_size
  2013-07-12 16:48 ` [PATCH 1/6] libxc: introduce xc_domain_get_address_size Dario Faggioli
@ 2013-07-12 17:23   ` Andrew Cooper
  2013-07-12 17:30   ` Wei Liu
  1 sibling, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2013-07-12 17:23 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: George Dunlap, Ian Jackson, Ian Campbell, xen-devel

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;
>          }
>      }
>  #endif

Almost 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/6] libxc: use xc_vcpu_setcontext() instead of calling do_domctl()
  2013-07-12 16:48 ` [PATCH 2/6] libxc: use xc_vcpu_setcontext() instead of calling do_domctl() Dario Faggioli
@ 2013-07-12 17:26   ` Andrew Cooper
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2013-07-12 17:26 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: George Dunlap, Ian Jackson, Ian Campbell, xen-devel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] libxc: introduce xc_domain_get_address_size
  2013-07-12 16:48 ` [PATCH 1/6] libxc: introduce xc_domain_get_address_size Dario Faggioli
  2013-07-12 17:23   ` Andrew Cooper
@ 2013-07-12 17:30   ` Wei Liu
  1 sibling, 0 replies; 13+ messages in thread
From: Wei Liu @ 2013-07-12 17:30 UTC (permalink / raw)
  To: Dario Faggioli
  Cc: George Dunlap, wei.liu2, Ian Jackson, Ian Campbell, 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/6] libxc: use xc_vcpu_getinfo() instead of calling do_domctl()
  2013-07-12 16:48 ` [PATCH 3/6] libxc: use xc_vcpu_getinfo() " Dario Faggioli
@ 2013-07-12 17:35   ` Andrew Cooper
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2013-07-12 17:35 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: George Dunlap, Ian Jackson, Ian Campbell, xen-devel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/6] libxc: allow for ctxt to be NULL in xc_vcpu_setcontext
  2013-07-12 16:48 ` [PATCH 4/6] libxc: allow for ctxt to be NULL in xc_vcpu_setcontext Dario Faggioli
@ 2013-07-12 17:38   ` Andrew Cooper
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2013-07-12 17:38 UTC (permalink / raw)
  To: Dario Faggioli
  Cc: Keir Fraser, Ian Campbell, George Dunlap, Ian Jackson, xen-devel,
	Jan Beulich

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-07-12 17:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-12 16:47 [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli
2013-07-12 16:48 ` [PATCH 1/6] libxc: introduce xc_domain_get_address_size Dario Faggioli
2013-07-12 17:23   ` Andrew Cooper
2013-07-12 17:30   ` Wei Liu
2013-07-12 16:48 ` [PATCH 2/6] libxc: use xc_vcpu_setcontext() instead of calling do_domctl() Dario Faggioli
2013-07-12 17:26   ` Andrew Cooper
2013-07-12 16:48 ` [PATCH 3/6] libxc: use xc_vcpu_getinfo() " Dario Faggioli
2013-07-12 17:35   ` Andrew Cooper
2013-07-12 16:48 ` [PATCH 4/6] libxc: allow for ctxt to be NULL in xc_vcpu_setcontext Dario Faggioli
2013-07-12 17:38   ` Andrew Cooper
2013-07-12 16:48 ` [PATCH 5/6] libxc: introduce xc_map_domain_meminfo (and xc_unmap_domain_meminfo) Dario Faggioli
2013-07-12 16:48 ` [PATCH 6/6] tools/misc: introduce xen-mfndump Dario Faggioli
2013-07-12 16:57 ` [PATCH 0/6] xen-mfndump and some libxc cleanups Dario Faggioli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.