|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/6] libxc: introduce xc_domain_get_address_size
On 12/07/13 17:48, Dario Faggioli wrote:
> As a wrapper to XEN_DOMCTL_get_address_size, and use it
> wherever the call was being issued directly via do_domctl(),
> saving quite some line of code.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |