Re: [Xen-devel] [PATCH v1 1/8]: PVH basic and hader file changes

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 21 Sep 2012, Mukesh Rathor wrote:
> ---
>  arch/x86/include/asm/xen/interface.h |    8 +++++++-
>  arch/x86/include/asm/xen/page.h      |    3 +++
>  arch/x86/xen/irq.c                   |    5 ++++-
>  arch/x86/xen/p2m.c                   |    2 +-
>  drivers/xen/cpu_hotplug.c            |    4 +++-
>  drivers/xen/events.c                 |   12 ++++++++----
>  drivers/xen/xenbus/xenbus_client.c   |    2 +-
>  drivers/xen/xenbus/xenbus_probe.c    |    6 +++++-
>  include/xen/interface/memory.h       |   27 +++++++++++++++++++++++++++
>  include/xen/interface/physdev.h      |   10 ++++++++++
>  10 files changed, 69 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/include/asm/xen/interface.h 
> b/arch/x86/include/asm/xen/interface.h
> index cbf0c9d..1d22131 100644
> --- a/arch/x86/include/asm/xen/interface.h
> +++ b/arch/x86/include/asm/xen/interface.h
> @@ -136,7 +136,13 @@ struct vcpu_guest_context {
>      struct cpu_user_regs user_regs;         /* User-level CPU registers     
> */
>      struct trap_info trap_ctxt[256];        /* Virtual IDT                  
> */
>      unsigned long ldt_base, ldt_ents;       /* LDT (linear address, # ents) 
> */
> -    unsigned long gdt_frames[16], gdt_ents; /* GDT (machine frames, # ents) 
> */
> +    union {
> +     struct {
> +             /* PV: GDT (machine frames, # ents).*/
> +             unsigned long gdt_frames[16], gdt_ents;
> +     };
> +     unsigned long gdtaddr, gdtsz;       /* PVH: GDTR addr and size */
> +    };
>      unsigned long kernel_ss, kernel_sp;     /* Virtual TSS (only SS1/SP1)   
> */
>      /* NB. User pagetable on x86/64 is placed in ctrlreg[1]. */
>      unsigned long ctrlreg[8];               /* CR0-CR7 (control registers)  
> */

I think I'll be fully able to understand what these are for only after I
read the Xen side patches...


> diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
> index 93971e8..d1cfb96 100644
> --- a/arch/x86/include/asm/xen/page.h
> +++ b/arch/x86/include/asm/xen/page.h
> @@ -158,6 +158,9 @@ static inline xpaddr_t machine_to_phys(xmaddr_t machine)
>  static inline unsigned long mfn_to_local_pfn(unsigned long mfn)
>  {
>       unsigned long pfn = mfn_to_pfn(mfn);
> +
> +     if (xen_feature(XENFEAT_auto_translated_physmap))
> +             return mfn;
>       if (get_phys_to_machine(pfn) != mfn)
>               return -1; /* force !pfn_valid() */
>       return pfn;
> diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> index 1573376..31959a7 100644
> --- a/arch/x86/xen/irq.c
> +++ b/arch/x86/xen/irq.c
> @@ -5,6 +5,7 @@
>  #include <xen/interface/xen.h>
>  #include <xen/interface/sched.h>
>  #include <xen/interface/vcpu.h>
> +#include <xen/features.h>
>  
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
> @@ -128,6 +129,8 @@ static const struct pv_irq_ops xen_irq_ops __initconst = {
>  
>  void __init xen_init_irq_ops(void)
>  {
> -     pv_irq_ops = xen_irq_ops;
> +     /* For PVH we use default pv_irq_ops settings */
> +     if (!xen_feature(XENFEAT_hvm_callback_vector))
> +             pv_irq_ops = xen_irq_ops;
>       x86_init.irqs.intr_init = xen_init_IRQ;
>  }
> diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
> index 64effdc..a954f83 100644
> --- a/arch/x86/xen/p2m.c
> +++ b/arch/x86/xen/p2m.c
> @@ -626,7 +626,7 @@ bool __set_phys_to_machine(unsigned long pfn, unsigned 
> long mfn)
>  {
>       unsigned topidx, mididx, idx;
>  
> -     if (unlikely(xen_feature(XENFEAT_auto_translated_physmap))) {
> +     if (xen_feature(XENFEAT_auto_translated_physmap)) {
>               BUG_ON(pfn != mfn && mfn != INVALID_P2M_ENTRY);
>               return true;
>       }
> diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
> index 4dcfced..de6bcf9 100644
> --- a/drivers/xen/cpu_hotplug.c
> +++ b/drivers/xen/cpu_hotplug.c
> @@ -2,6 +2,7 @@
>  
>  #include <xen/xen.h>
>  #include <xen/xenbus.h>
> +#include <xen/features.h>
>  
>  #include <asm/xen/hypervisor.h>
>  #include <asm/cpu.h>
> @@ -100,7 +101,8 @@ static int __init setup_vcpu_hotplug_event(void)
>       static struct notifier_block xsn_cpu = {
>               .notifier_call = setup_cpu_watcher };
>  
> -     if (!xen_pv_domain())
> +     /* PVH TBD/FIXME: future work */
> +     if (!xen_pv_domain() || xen_feature(XENFEAT_auto_translated_physmap))
>               return -ENODEV;

Didn't we say that a PVH domain is actually a pv guest?
In that case shouldn't the test

if (!xen_pv_domain())

be sufficient?


>       register_xenstore_notifier(&xsn_cpu);
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 7595581..f656791 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -1755,7 +1755,7 @@ int xen_set_callback_via(uint64_t via)
>  }
>  EXPORT_SYMBOL_GPL(xen_set_callback_via);
>  
> -#ifdef CONFIG_XEN_PVHVM
> +
>  /* Vector callbacks are better than PCI interrupts to receive event
>   * channel notifications because we can receive vector callbacks on any
>   * vcpu and we don't need PCI support or APIC interactions. */
> @@ -1779,9 +1779,6 @@ void xen_callback_vector(void)
>                       alloc_intr_gate(XEN_HVM_EVTCHN_CALLBACK, 
> xen_hvm_callback_vector);
>       }
>  }
> -#else
> -void xen_callback_vector(void) {}
> -#endif
>  
>  void __init xen_init_IRQ(void)
>  {
> @@ -1814,6 +1811,13 @@ void __init xen_init_IRQ(void)
>               if (xen_initial_domain())
>                       pci_xen_initial_domain();
>  
> +             if (xen_feature(XENFEAT_hvm_callback_vector)) {
> +                     xen_callback_vector();
> +                     return;
> +             }
> +
> +             /* PVH: TBD/FIXME: debug and fix eio map to work with pvh */
> +
>               pirq_eoi_map = (void *)__get_free_page(GFP_KERNEL|__GFP_ZERO);
>               eoi_gmfn.gmfn = virt_to_mfn(pirq_eoi_map);
>               rc = HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_gmfn_v2, 
> &eoi_gmfn);
> diff --git a/drivers/xen/xenbus/xenbus_client.c 
> b/drivers/xen/xenbus/xenbus_client.c
> index b3e146e..a81e66b 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -743,7 +743,7 @@ static const struct xenbus_ring_ops ring_ops_hvm = {
>  
>  void __init xenbus_ring_ops_init(void)
>  {
> -     if (xen_pv_domain())
> +     if (xen_pv_domain() && !xen_feature(XENFEAT_auto_translated_physmap))
>               ring_ops = &ring_ops_pv;
>       else
>               ring_ops = &ring_ops_hvm;
> diff --git a/drivers/xen/xenbus/xenbus_probe.c 
> b/drivers/xen/xenbus/xenbus_probe.c
> index b793723..481dc72 100644
> --- a/drivers/xen/xenbus/xenbus_probe.c
> +++ b/drivers/xen/xenbus/xenbus_probe.c
> @@ -749,7 +749,11 @@ static int __init xenbus_init(void)
>                       if (err)
>                               goto out_error;
>               }
> -             xen_store_interface = mfn_to_virt(xen_store_mfn);
> +                if (xen_feature(XENFEAT_auto_translated_physmap))

small code style issue here

> +                     /* mfn is actually a pfn */
> +                     xen_store_interface = __va(xen_store_mfn<<PAGE_SHIFT);
> +             else
> +                     xen_store_interface = mfn_to_virt(xen_store_mfn);
>       }

I think that mfn_to_virt should work for PVH too: mfn_to_virt is:

(__va(mfn_to_pfn(m) << PAGE_SHIFT))

and mfn_to_pfn just return mfn when XENFEAT_auto_translated_physmap is
set

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