Re: [Xen-devel] [PATCH v1 3/8]: PVH startup changes (enlighten.c)

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 21 Sep 2012, Mukesh Rathor wrote:
> ---
>  arch/x86/xen/enlighten.c |   99 ++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 79 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index bf4bda6..98f1798 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -45,6 +45,7 @@
>  #include <xen/hvm.h>
>  #include <xen/hvc-console.h>
>  #include <xen/acpi.h>
> +#include <xen/features.h>
>  
>  #include <asm/paravirt.h>
>  #include <asm/apic.h>
> @@ -105,6 +106,9 @@ RESERVE_BRK(shared_info_page_brk, PAGE_SIZE);
>  __read_mostly int xen_have_vector_callback;
>  EXPORT_SYMBOL_GPL(xen_have_vector_callback);
>  
> +#define xen_pvh_domain() (xen_pv_domain() && \
> +                       xen_feature(XENFEAT_auto_translated_physmap) && \
> +                       xen_have_vector_callback)
>  /*
>   * Point at some empty memory to start with. We map the real shared_info
>   * page as soon as fixmap is up and running.
> @@ -139,6 +143,8 @@ struct tls_descs {
>   */
>  static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>  
> +static void __init xen_hvm_init_shared_info(void);
> +
>  static void clamp_max_cpus(void)
>  {
>  #ifdef CONFIG_SMP
> @@ -217,8 +223,9 @@ static void __init xen_banner(void)
>       struct xen_extraversion extra;
>       HYPERVISOR_xen_version(XENVER_extraversion, &extra);
>  
> -     printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
> -            pv_info.name);
> +     printk(KERN_INFO "Booting paravirtualized kernel %son %s\n",
> +             xen_feature(XENFEAT_auto_translated_physmap) ?
> +                     "with PVH extensions " : "", pv_info.name);
>       printk(KERN_INFO "Xen version: %d.%d%s%s\n",
>              version >> 16, version & 0xffff, extra.extraversion,
>              xen_feature(XENFEAT_mmu_pt_update_preserve_ad) ? " 
> (preserve-AD)" : "");
> @@ -271,12 +278,15 @@ static void xen_cpuid(unsigned int *ax, unsigned int 
> *bx,
>               break;
>       }
>  
> -     asm(XEN_EMULATE_PREFIX "cpuid"
> -             : "=a" (*ax),
> -               "=b" (*bx),
> -               "=c" (*cx),
> -               "=d" (*dx)
> -             : "0" (*ax), "2" (*cx));
> +     if (xen_pvh_domain())
> +             native_cpuid(ax, bx, cx, dx);
> +     else
> +             asm(XEN_EMULATE_PREFIX "cpuid"
> +                     : "=a" (*ax),
> +                     "=b" (*bx),
> +                     "=c" (*cx),
> +                     "=d" (*dx)
> +                     : "0" (*ax), "2" (*cx));

can't we just set the cpuid pvop to native_cpuid?


>       *bx &= maskebx;
>       *cx &= maskecx;
> @@ -1034,6 +1044,10 @@ static int xen_write_msr_safe(unsigned int msr, 
> unsigned low, unsigned high)
>  
>  void xen_setup_shared_info(void)
>  {
> +     /* do later in xen_pvh_guest_init() when extend_brk is properly setup*/

Which one is the function that is going to get called later to do the
initialization? It cannot be this one because it would still return
immediately.


> +     if (xen_pvh_domain() && xen_initial_domain())
> +             return;
> +
>       if (!xen_feature(XENFEAT_auto_translated_physmap)) {
>               set_fixmap(FIX_PARAVIRT_BOOTMAP,
>                          xen_start_info->shared_info);
>
> @@ -1044,6 +1058,10 @@ void xen_setup_shared_info(void)
>               HYPERVISOR_shared_info =
>                       (struct shared_info *)__va(xen_start_info->shared_info);
>  
> +     /* PVH TBD/FIXME: vcpu info placement in phase 2 */
> +     if (xen_pvh_domain())
> +             return;

It seems that if xen_initial_domain we always skip the initialization
while if !xen_initial_domain we only initialize HYPERVISOR_shared_info.
I don't understand why we have this difference.

Also it might be worth refactoring xen_setup_shared_info moving out the
call to xen_setup_vcpu_info_placement and xen_setup_mfn_list_list in
order to avoid all these

if ([!]xen_pvh_domain())
    return;

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