Re: [PATCH for 4.7] xen: Replace alloc_vcpu_guest_context() with vmalloc()

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Aug 21, 2015 at 06:51:46PM +0100, Andrew Cooper wrote:
> This essentially reverts c/s 2037f2adb "x86: introduce
> alloc_vcpu_guest_context()", including the newer arm bits, but achieves
> the same end goal by using the newer vmalloc() infrastructure.

Could you explain what this fixes? Or perhaps with an explanation
of why this will make Xen [ ]better; [ ] faster [ ] magical.

:-)


Thanks.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Tim Deegan <tim@xen.org>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> 
> Build tested on all architectures, functionally tested on x86.
> ---
>  xen/arch/arm/domain.c        |   11 ----------
>  xen/arch/arm/vpsci.c         |    5 +++--
>  xen/arch/x86/domain.c        |   46 ------------------------------------------
>  xen/common/domain.c          |    7 ++++---
>  xen/common/domctl.c          |    5 +++--
>  xen/include/asm-x86/fixmap.h |    3 ---
>  xen/include/xen/domain.h     |    6 ------
>  7 files changed, 10 insertions(+), 73 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index b2bfc7d..e0ef0d8 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -466,17 +466,6 @@ void free_vcpu_struct(struct vcpu *v)
>      free_xenheap_page(v);
>  }
>  
> -struct vcpu_guest_context *alloc_vcpu_guest_context(void)
> -{
> -    return xmalloc(struct vcpu_guest_context);
> -
> -}
> -
> -void free_vcpu_guest_context(struct vcpu_guest_context *vgc)
> -{
> -    xfree(vgc);
> -}
> -
>  int vcpu_initialise(struct vcpu *v)
>  {
>      int rc = 0;
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index aebe1e2..ba3414d 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -13,6 +13,7 @@
>  #include <xen/errno.h>
>  #include <xen/sched.h>
>  #include <xen/types.h>
> +#include <xen/vmap.h>
>  
>  #include <asm/current.h>
>  #include <asm/gic.h>
> @@ -45,7 +46,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>              ( !test_bit(_VPF_down, &v->pause_flags) ) )
>          return PSCI_ALREADY_ON;
>  
> -    if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
> +    if ( (ctxt = vmalloc(sizeof(struct vcpu_guest_context))) == NULL )
>          return PSCI_DENIED;
>  
>      vgic_clear_pending_irqs(v);
> @@ -78,7 +79,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>  
>      domain_lock(d);
>      rc = arch_set_info_guest(v, ctxt);
> -    free_vcpu_guest_context(ctxt);
> +    vfree(ctxt);
>  
>      if ( rc < 0 )
>      {
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 045f6ff..12ccdb8 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -48,7 +48,6 @@
>  #include <asm/cpuidle.h>
>  #include <asm/mpspec.h>
>  #include <asm/ldt.h>
> -#include <asm/fixmap.h>
>  #include <asm/hvm/hvm.h>
>  #include <asm/hvm/support.h>
>  #include <asm/hvm/viridian.h>
> @@ -272,51 +271,6 @@ void free_vcpu_struct(struct vcpu *v)
>      free_xenheap_page(v);
>  }
>  
> -static DEFINE_PER_CPU(struct page_info *[
> -    PFN_UP(sizeof(struct vcpu_guest_context))], vgc_pages);
> -
> -struct vcpu_guest_context *alloc_vcpu_guest_context(void)
> -{
> -    unsigned int i, cpu = smp_processor_id();
> -    enum fixed_addresses idx = FIX_VGC_BEGIN -
> -        cpu * PFN_UP(sizeof(struct vcpu_guest_context));
> -
> -    BUG_ON(per_cpu(vgc_pages[0], cpu) != NULL);
> -
> -    for ( i = 0; i < PFN_UP(sizeof(struct vcpu_guest_context)); ++i )
> -    {
> -        struct page_info *pg = alloc_domheap_page(current->domain,
> -                                                  MEMF_no_owner);
> -
> -        if ( unlikely(pg == NULL) )
> -        {
> -            free_vcpu_guest_context(NULL);
> -            return NULL;
> -        }
> -        __set_fixmap(idx - i, page_to_mfn(pg), __PAGE_HYPERVISOR_RW);
> -        per_cpu(vgc_pages[i], cpu) = pg;
> -    }
> -    return (void *)fix_to_virt(idx);
> -}
> -
> -void free_vcpu_guest_context(struct vcpu_guest_context *vgc)
> -{
> -    unsigned int i, cpu = smp_processor_id();
> -    enum fixed_addresses idx = FIX_VGC_BEGIN -
> -        cpu * PFN_UP(sizeof(struct vcpu_guest_context));
> -
> -    BUG_ON(vgc && vgc != (void *)fix_to_virt(idx));
> -
> -    for ( i = 0; i < PFN_UP(sizeof(struct vcpu_guest_context)); ++i )
> -    {
> -        if ( !per_cpu(vgc_pages[i], cpu) )
> -            continue;
> -        clear_fixmap(idx - i);
> -        free_domheap_page(per_cpu(vgc_pages[i], cpu));
> -        per_cpu(vgc_pages[i], cpu) = NULL;
> -    }
> -}
> -
>  static int setup_compat_l4(struct vcpu *v)
>  {
>      struct page_info *pg;
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 1b9fcfc..31d9e56 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -43,6 +43,7 @@
>  #include <xen/trace.h>
>  #include <xen/tmem.h>
>  #include <asm/setup.h>
> +#include <xen/vmap.h>
>  
>  /* Linux config option: propageted to domain0 */
>  /* xen_processor_pmbits: xen control Cx, Px, ... */
> @@ -1185,12 +1186,12 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
>          if ( v->vcpu_info == &dummy_vcpu_info )
>              return -EINVAL;
>  
> -        if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
> +        if ( (ctxt = vmalloc(sizeof(struct vcpu_guest_context))) == NULL )
>              return -ENOMEM;
>  
>          if ( copy_from_guest(ctxt, arg, 1) )
>          {
> -            free_vcpu_guest_context(ctxt);
> +            vfree(ctxt);
>              return -EFAULT;
>          }
>  
> @@ -1198,7 +1199,7 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
>          rc = v->is_initialised ? -EEXIST : arch_set_info_guest(v, ctxt);
>          domain_unlock(d);
>  
> -        free_vcpu_guest_context(ctxt);
> +        vfree(ctxt);
>  
>          if ( rc == -ERESTART )
>              rc = hypercall_create_continuation(__HYPERVISOR_vcpu_op, "iuh",
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 7f959f3..90cc13c 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -32,6 +32,7 @@
>  #include <asm/monitor.h>
>  #include <public/domctl.h>
>  #include <xsm/xsm.h>
> +#include <xen/vmap.h>
>  
>  static DEFINE_SPINLOCK(domctl_lock);
>  DEFINE_SPINLOCK(vcpu_alloc_lock);
> @@ -492,7 +493,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>                       < sizeof(struct compat_vcpu_guest_context));
>  #endif
>          ret = -ENOMEM;
> -        if ( (c.nat = alloc_vcpu_guest_context()) == NULL )
> +        if ( (c.nat = vmalloc(sizeof(struct vcpu_guest_context))) == NULL )
>              break;
>  
>  #ifdef CONFIG_COMPAT
> @@ -518,7 +519,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>                            __HYPERVISOR_domctl, "h", u_domctl);
>          }
>  
> -        free_vcpu_guest_context(c.nat);
> +        vfree(c.nat);
>          break;
>      }
>  
> diff --git a/xen/include/asm-x86/fixmap.h b/xen/include/asm-x86/fixmap.h
> index 70eadff..1e24b11 100644
> --- a/xen/include/asm-x86/fixmap.h
> +++ b/xen/include/asm-x86/fixmap.h
> @@ -47,9 +47,6 @@ enum fixed_addresses {
>      FIX_COM_END,
>      FIX_EHCI_DBGP,
>      /* Everything else should go further down. */
> -    FIX_VGC_END,
> -    FIX_VGC_BEGIN = FIX_VGC_END
> -      + PFN_UP(sizeof(struct vcpu_guest_context)) * NR_CPUS - 1,
>      FIX_APIC_BASE,
>      FIX_IO_APIC_BASE_0,
>      FIX_IO_APIC_BASE_END = FIX_IO_APIC_BASE_0 + MAX_IO_APICS-1,
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index 848db8a..db191d3 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -31,12 +31,6 @@ struct vcpu *alloc_vcpu(
>  struct vcpu *alloc_vcpu_struct(void);
>  void free_vcpu_struct(struct vcpu *v);
>  
> -/* Allocate/free a vcpu_guest_context structure. */
> -#ifndef alloc_vcpu_guest_context
> -struct vcpu_guest_context *alloc_vcpu_guest_context(void);
> -void free_vcpu_guest_context(struct vcpu_guest_context *);
> -#endif
> -
>  /* Allocate/free a PIRQ structure. */
>  #ifndef alloc_pirq_struct
>  struct pirq *alloc_pirq_struct(struct domain *);
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel