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

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Mon, 24 Sep 2012 12:26:57 +0100
Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:

> 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...

When we are bringup up smp vcpu, we just need to send down gdtaddr and
gdt size for vcpu context. You had suggested doing it during the
code review, remember :). I had originally in xen. Anyways, earlier
I was just using gdt_frames[0] for addr and gdt_ents for size. But Ian
suggested I change it to union.

> 
> 
> >  #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?

No, for HVM and PVH, we want to return -ENODEV.


> 
> >  	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
> > __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

You are right. Testing it out.