WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] allow xc_core arch code top optimization. And so

To: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] allow xc_core arch code top optimization. And some code clean up.
From: Keir Fraser <keir@xxxxxxxxxxxxx>
Date: Tue, 28 Aug 2007 15:56:22 +0100
Cc: xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Tue, 28 Aug 2007 07:57:06 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20070828062149.GC12712%yamahata@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acfpg5j8130JDVV2EdyEfAAX8io7RQ==
Thread-topic: [Xen-devel] [PATCH] allow xc_core arch code top optimization. And some code clean up.
User-agent: Microsoft-Entourage/11.3.6.070618
There's a fair bit of churn here -- please separate cleanups from actual
code changes (e.g., feature additions or optimisations).

I think xc_core_arch_gpfn_present() is not needed. The stupid thing in the
current code is that we map each guest page *twice* -- once to detect its
presence and one to copy it. Unless you are attempting to map a lot of pages
that actually aren't present, you will get the same time saving by
generating the pfn_table as you do the copy, and then put .xen_pfn after
.xen_pages, rather than before (since it can't be put before, as you're
creating as you're generating .xen_pages). Make sense?

 -- Keir

On 28/8/07 07:21, "Isaku Yamahata" <yamahata@xxxxxxxxxxxxx> wrote:

> # HG changeset patch
> # User yamahata@xxxxxxxxxxxxx
> # Date 1188281121 -32400
> # Node ID 81be02b7e4cc2c0239e050b6f2dcd0ef69ffdcf9
> # Parent  ec6538af0ce5c0cee25d7a063c1d33df0f80a8d9
> allow xc_core arch code top optimization. And some code clean up.
> Currently xc core code tries to map page in order to deterine a given gmfn
> page is allocated or not when auto transated mode. It takes long time.
> Introduce xc_core_arch_gpfn_present() arch hook to allow arch code
> arch optimized way. and implement default function which tries to
> map a page as before.
> PATCHNAME: allow_xc_core_arch_optimization
> 
> Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
> 
> diff -r ec6538af0ce5 -r 81be02b7e4cc tools/libxc/xc_core.c
> --- a/tools/libxc/xc_core.c Tue Aug 28 14:53:42 2007 +0900
> +++ b/tools/libxc/xc_core.c Tue Aug 28 15:05:21 2007 +0900
> @@ -231,6 +231,35 @@ xc_core_shdr_set(Elf64_Shdr *shdr,
>      return 0;
>  }
>  
> +static void
> +xc_core_ehdr_init(Elf64_Ehdr *ehdr)
> +{
> +    memset(ehdr, 0, sizeof(*ehdr));
> +    ehdr->e_ident[EI_MAG0] = ELFMAG0;
> +    ehdr->e_ident[EI_MAG1] = ELFMAG1;
> +    ehdr->e_ident[EI_MAG2] = ELFMAG2;
> +    ehdr->e_ident[EI_MAG3] = ELFMAG3;
> +    ehdr->e_ident[EI_CLASS] = ELFCLASS64;
> +    ehdr->e_ident[EI_DATA] = ELF_ARCH_DATA;
> +    ehdr->e_ident[EI_VERSION] = EV_CURRENT;
> +    ehdr->e_ident[EI_OSABI] = ELFOSABI_SYSV;
> +    ehdr->e_ident[EI_ABIVERSION] = EV_CURRENT;
> +
> +    ehdr->e_type = ET_CORE;
> +    ehdr->e_machine = ELF_ARCH_MACHINE;
> +    ehdr->e_version = EV_CURRENT;
> +    ehdr->e_entry = 0;
> +    ehdr->e_phoff = 0;
> +    ehdr->e_shoff = sizeof(*ehdr);
> +    ehdr->e_flags = ELF_CORE_EFLAGS;
> +    ehdr->e_ehsize = sizeof(*ehdr);
> +    ehdr->e_phentsize = sizeof(Elf64_Phdr);
> +    ehdr->e_phnum = 0;
> +    ehdr->e_shentsize = sizeof(Elf64_Shdr);
> +    /* ehdr->e_shnum and ehdr->e_shstrndx aren't known here yet.
> +     * fill it later */
> +}
> +
>  static int
>  elfnote_fill_xen_version(int xc_handle,
>                           struct xen_dumpcore_elfnote_xen_version_desc
> @@ -277,13 +306,116 @@ elfnote_fill_xen_version(int xc_handle,
>      return 0;
>  }
>  
> -static int
> +static void
>  elfnote_fill_format_version(struct xen_dumpcore_elfnote_format_version_desc
>                              *format_version)
>  {
>      format_version->version = XEN_DUMPCORE_FORMAT_VERSION_CURRENT;
> -    return 0;
> -}
> +}
> +
> +static void
> +elfnote_init(struct elfnote *elfnote)
> +{
> +    /* elf note section */
> +    memset(elfnote, 0, sizeof(*elfnote));
> +    elfnote->namesz = strlen(XEN_DUMPCORE_ELFNOTE_NAME) + 1;
> +    strncpy(elfnote->name, XEN_DUMPCORE_ELFNOTE_NAME, sizeof(elfnote->name));
> +}
> +
> +static int
> +elfnote_dump_none(void *args, dumpcore_rtn_t dump_rtn)
> +{
> +    int sts;
> +    struct elfnote elfnote;
> +    struct xen_dumpcore_elfnote_none_desc none;
> +
> +    elfnote_init(&elfnote);
> +    memset(&none, 0, sizeof(none));
> +
> +    elfnote.descsz = sizeof(none);
> +    elfnote.type = XEN_ELFNOTE_DUMPCORE_NONE;
> +    sts = dump_rtn(args, (char*)&elfnote, sizeof(elfnote));
> +    if ( sts != 0 )
> +        return sts;
> +    return dump_rtn(args, (char*)&none, sizeof(none));
> +}
> +
> +static int
> +elfnote_dump_core_header(
> +    void *args, dumpcore_rtn_t dump_rtn, const xc_dominfo_t *info,
> +    int nr_vcpus, unsigned long nr_pages)
> +{
> +    int sts;
> +    struct elfnote elfnote;
> +    struct xen_dumpcore_elfnote_header_desc header;
> +    
> +    elfnote_init(&elfnote);
> +    memset(&header, 0, sizeof(header));
> +    
> +    elfnote.descsz = sizeof(header);
> +    elfnote.type = XEN_ELFNOTE_DUMPCORE_HEADER;
> +    header.xch_magic = info->hvm ? XC_CORE_MAGIC_HVM : XC_CORE_MAGIC;
> +    header.xch_nr_vcpus = nr_vcpus;
> +    header.xch_nr_pages = nr_pages;
> +    header.xch_page_size = PAGE_SIZE;
> +    sts = dump_rtn(args, (char*)&elfnote, sizeof(elfnote));
> +    if ( sts != 0 )
> +        return sts;
> +    return dump_rtn(args, (char*)&header, sizeof(header));
> +}
> +
> +static int
> +elfnote_dump_xen_version(void *args, dumpcore_rtn_t dump_rtn, int xc_handle)
> +{
> +    int sts;
> +    struct elfnote elfnote;
> +    struct xen_dumpcore_elfnote_xen_version_desc xen_version;
> +
> +    elfnote_init(&elfnote);
> +    memset(&xen_version, 0, sizeof(xen_version));
> +
> +    elfnote.descsz = sizeof(xen_version);
> +    elfnote.type = XEN_ELFNOTE_DUMPCORE_XEN_VERSION;
> +    elfnote_fill_xen_version(xc_handle, &xen_version);
> +    sts = dump_rtn(args, (char*)&elfnote, sizeof(elfnote));
> +    if ( sts != 0 )
> +        return sts;
> +    return dump_rtn(args, (char*)&xen_version, sizeof(xen_version));
> +}
> +
> +static int
> +elfnote_dump_format_version(void *args, dumpcore_rtn_t dump_rtn)
> +{
> +    int sts;
> +    struct elfnote elfnote;
> +    struct xen_dumpcore_elfnote_format_version_desc format_version;
> +
> +    elfnote_init(&elfnote);
> +    memset(&format_version, 0, sizeof(format_version));
> +    
> +    elfnote.descsz = sizeof(format_version);
> +    elfnote.type = XEN_ELFNOTE_DUMPCORE_FORMAT_VERSION;
> +    elfnote_fill_format_version(&format_version);
> +    sts = dump_rtn(args, (char*)&elfnote, sizeof(elfnote));
> +    if ( sts != 0 )
> +        return sts;
> +    return dump_rtn(args, (char*)&format_version, sizeof(format_version));
> +}
> +
> +int
> +xc_core_arch_gpfn_present_default(int xc_handle,
> +                                  struct xc_core_arch_context *unused,
> +                                  uint32_t domid, unsigned long pfn)
> +{
> +    /* try to map page to determin wheter it has underlying page */
> +    void *vaddr = xc_map_foreign_range(xc_handle, domid,
> +                                       PAGE_SIZE, PROT_READ, pfn);
> +    munmap(vaddr, PAGE_SIZE);
> +    return vaddr != NULL;
> +}
> +#ifndef xc_core_arch_gpfn_present
> +# define xc_core_arch_gpfn_present xc_core_arch_gpfn_present_default
> +#endif
>  
>  int
>  xc_domain_dumpcore_via_callback(int xc_handle,
> @@ -327,13 +459,6 @@ xc_domain_dumpcore_via_callback(int xc_h
>      struct xc_core_section_headers *sheaders = NULL;
>      Elf64_Shdr *shdr;
>  
> -    /* elf notes */
> -    struct elfnote elfnote;
> -    struct xen_dumpcore_elfnote_none_desc none;
> -    struct xen_dumpcore_elfnote_header_desc header;
> -    struct xen_dumpcore_elfnote_xen_version_desc xen_version;
> -    struct xen_dumpcore_elfnote_format_version_desc format_version;
> -
>      xc_core_arch_context_init(&arch_ctxt);
>      if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
>      {
> @@ -379,8 +504,9 @@ xc_domain_dumpcore_via_callback(int xc_h
>      }
>  
>      /* obtain memory map */
> -    sts = xc_core_arch_memory_map_get(xc_handle, &info, live_shinfo,
> -                                      &memory_map, &nr_memory_map);
> +    sts = xc_core_arch_memory_map_get(xc_handle, &arch_ctxt, &info,
> +                                      live_shinfo, &memory_map,
> +                                      &nr_memory_map);
>      if ( sts != 0 )
>          goto out;
>  
> @@ -430,12 +556,9 @@ xc_domain_dumpcore_via_callback(int xc_h
>              }
>              else
>              {
> -                /* try to map page to determin wheter it has underlying page
> */
> -                void *vaddr = xc_map_foreign_range(xc_handle, domid,
> -                                                   PAGE_SIZE, PROT_READ, i);
> -                if ( vaddr == NULL )
> +                if (!xc_core_arch_gpfn_present(xc_handle, &arch_ctxt,
> +                                               domid, i))
>                      continue;
> -                munmap(vaddr, PAGE_SIZE);
>                  pfn_array[j] = i;
>              }
>  
> @@ -451,29 +574,8 @@ xc_domain_dumpcore_via_callback(int xc_h
>          nr_pages = j;
>      }
>  
> -    memset(&ehdr, 0, sizeof(ehdr));
> -    ehdr.e_ident[EI_MAG0] = ELFMAG0;
> -    ehdr.e_ident[EI_MAG1] = ELFMAG1;
> -    ehdr.e_ident[EI_MAG2] = ELFMAG2;
> -    ehdr.e_ident[EI_MAG3] = ELFMAG3;
> -    ehdr.e_ident[EI_CLASS] = ELFCLASS64;
> -    ehdr.e_ident[EI_DATA] = ELF_ARCH_DATA;
> -    ehdr.e_ident[EI_VERSION] = EV_CURRENT;
> -    ehdr.e_ident[EI_OSABI] = ELFOSABI_SYSV;
> -    ehdr.e_ident[EI_ABIVERSION] = EV_CURRENT;
> -
> -    ehdr.e_type = ET_CORE;
> -    ehdr.e_machine = ELF_ARCH_MACHINE;
> -    ehdr.e_version = EV_CURRENT;
> -    ehdr.e_entry = 0;
> -    ehdr.e_phoff = 0;
> -    ehdr.e_shoff = sizeof(ehdr);
> -    ehdr.e_flags = ELF_CORE_EFLAGS;
> -    ehdr.e_ehsize = sizeof(ehdr);
> -    ehdr.e_phentsize = sizeof(Elf64_Phdr);
> -    ehdr.e_phnum = 0;
> -    ehdr.e_shentsize = sizeof(Elf64_Shdr);
>      /* ehdr.e_shnum and ehdr.e_shstrndx aren't known here yet. fill it
> later*/
> +    xc_core_ehdr_init(&ehdr);
>  
>      /* create section header */
>      strtab = xc_core_strtab_init();
> @@ -549,7 +651,7 @@ xc_domain_dumpcore_via_callback(int xc_h
>      /* arch context */
>      sts = xc_core_arch_context_get_shdr(&arch_ctxt, sheaders, strtab,
>                                          &filesz, offset);
> -    if ( sts != 0)
> +    if ( sts != 0 )
>          goto out;
>      offset += filesz;
>  
> @@ -645,54 +747,23 @@ xc_domain_dumpcore_via_callback(int xc_h
>      if ( sts != 0 )
>          goto out;
>  
> -    /* elf note section */
> -    memset(&elfnote, 0, sizeof(elfnote));
> -    elfnote.namesz = strlen(XEN_DUMPCORE_ELFNOTE_NAME) + 1;
> -    strncpy(elfnote.name, XEN_DUMPCORE_ELFNOTE_NAME, sizeof(elfnote.name));
> -
> -    /* elf note section:xen core header */
> -    elfnote.descsz = sizeof(none);
> -    elfnote.type = XEN_ELFNOTE_DUMPCORE_NONE;
> -    sts = dump_rtn(args, (char*)&elfnote, sizeof(elfnote));
> -    if ( sts != 0 )
> -        goto out;
> -    sts = dump_rtn(args, (char*)&none, sizeof(none));
> -    if ( sts != 0 )
> -        goto out;
> -
> -    /* elf note section:xen core header */
> -    elfnote.descsz = sizeof(header);
> -    elfnote.type = XEN_ELFNOTE_DUMPCORE_HEADER;
> -    header.xch_magic = info.hvm ? XC_CORE_MAGIC_HVM : XC_CORE_MAGIC;
> -    header.xch_nr_vcpus = nr_vcpus;
> -    header.xch_nr_pages = nr_pages;
> -    header.xch_page_size = PAGE_SIZE;
> -    sts = dump_rtn(args, (char*)&elfnote, sizeof(elfnote));
> -    if ( sts != 0 )
> -        goto out;
> -    sts = dump_rtn(args, (char*)&header, sizeof(header));
> +    /* elf note section: xen core header */
> +    sts = elfnote_dump_none(args, dump_rtn);
> +    if ( sts != 0)
> +        goto out;
> +
> +    /* elf note section: xen core header */
> +    sts = elfnote_dump_core_header(args, dump_rtn, &info, nr_vcpus,
> nr_pages);
>      if ( sts != 0 )
>          goto out;
>  
>      /* elf note section: xen version */
> -    elfnote.descsz = sizeof(xen_version);
> -    elfnote.type = XEN_ELFNOTE_DUMPCORE_XEN_VERSION;
> -    elfnote_fill_xen_version(xc_handle, &xen_version);
> -    sts = dump_rtn(args, (char*)&elfnote, sizeof(elfnote));
> -    if ( sts != 0 )
> -        goto out;
> -    sts = dump_rtn(args, (char*)&xen_version, sizeof(xen_version));
> +    sts = elfnote_dump_xen_version(args, dump_rtn, xc_handle);
>      if ( sts != 0 )
>          goto out;
>  
>      /* elf note section: format version */
> -    elfnote.descsz = sizeof(format_version);
> -    elfnote.type = XEN_ELFNOTE_DUMPCORE_FORMAT_VERSION;
> -    elfnote_fill_format_version(&format_version);
> -    sts = dump_rtn(args, (char*)&elfnote, sizeof(elfnote));
> -    if ( sts != 0 )
> -        goto out;
> -    sts = dump_rtn(args, (char*)&format_version, sizeof(format_version));
> +    sts = elfnote_dump_format_version(args, dump_rtn);
>      if ( sts != 0 )
>          goto out;
>  
> diff -r ec6538af0ce5 -r 81be02b7e4cc tools/libxc/xc_core.h
> --- a/tools/libxc/xc_core.h Tue Aug 28 14:53:42 2007 +0900
> +++ b/tools/libxc/xc_core.h Tue Aug 28 15:05:21 2007 +0900
> @@ -131,14 +131,18 @@ struct xc_core_memory_map {
>  };
>  typedef struct xc_core_memory_map xc_core_memory_map_t;
>  int xc_core_arch_auto_translated_physmap(const xc_dominfo_t *info);
> -int xc_core_arch_memory_map_get(int xc_handle, xc_dominfo_t *info,
> -                                shared_info_t *live_shinfo,
> +struct xc_core_arch_context;
> +int xc_core_arch_memory_map_get(int xc_handle,
> +                                struct xc_core_arch_context *arch_ctxt,
> +                                xc_dominfo_t *info, shared_info_t
> *live_shinfo,
>                                  xc_core_memory_map_t **mapp,
>                                  unsigned int *nr_entries);
>  int xc_core_arch_map_p2m(int xc_handle, xc_dominfo_t *info,
>                           shared_info_t *live_shinfo, xen_pfn_t **live_p2m,
>                           unsigned long *pfnp);
> -
> +int xc_core_arch_gpfn_present_default(int xc_handle,
> +                                      struct xc_core_arch_context *unused,
> +                                      uint32_t domid, unsigned long pfn);
>  
>  #if defined (__i386__) || defined (__x86_64__)
>  # include "xc_core_x86.h"
> diff -r ec6538af0ce5 -r 81be02b7e4cc tools/libxc/xc_core_powerpc.c
> --- a/tools/libxc/xc_core_powerpc.c Tue Aug 28 14:53:42 2007 +0900
> +++ b/tools/libxc/xc_core_powerpc.c Tue Aug 28 15:05:21 2007 +0900
> @@ -43,8 +43,8 @@ xc_core_arch_map_p2m(int xc_handle, xc_d
>  }
>  
>  int
> -xc_core_arch_memory_map_get(int xc_handle, xc_dominfo_t *info,
> -                            shared_info_t *live_shinfo,
> +xc_core_arch_memory_map_get(int xc_handle, struct xc_core_arch_context
> *unused,
> +                            xc_dominfo_t *info, shared_info_t *live_shinfo,
>                              xc_core_memory_map_t **mapp,
>                              unsigned int *nr_entries)
>  {
> diff -r ec6538af0ce5 -r 81be02b7e4cc tools/libxc/xc_core_x86.c
> --- a/tools/libxc/xc_core_x86.c Tue Aug 28 14:53:42 2007 +0900
> +++ b/tools/libxc/xc_core_x86.c Tue Aug 28 15:05:21 2007 +0900
> @@ -33,8 +33,8 @@ xc_core_arch_auto_translated_physmap(con
>  }
>  
>  int
> -xc_core_arch_memory_map_get(int xc_handle, xc_dominfo_t *info,
> -                            shared_info_t *live_shinfo,
> +xc_core_arch_memory_map_get(int xc_handle, struct xc_core_arch_context
> *unused,
> +                            xc_dominfo_t *info, shared_info_t *live_shinfo,
>                              xc_core_memory_map_t **mapp,
>                              unsigned int *nr_entries)
>  {
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>