Re: [RFC PATCH 9/16]: PVH xen: create PVH vmcs and initialization

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

On Mon, 14 Jan 2013 11:56:39 +0000
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 12.01.13 at 02:59, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > --- a/xen/arch/x86/hvm/hvm.c	Fri Jan 11 16:31:33 2013 -0800
> > +++ b/xen/arch/x86/hvm/hvm.c	Fri Jan 11 16:32:36 2013 -0800
> > @@ -331,6 +331,9 @@ void hvm_do_resume(struct vcpu *v)
> >  {
> >      ioreq_t *p;
> >  
> > +    if ( is_pvh_vcpu(v) )
> > +        return;
> 
> Better avoid the call at the call site?

Done.

> > @@ -697,6 +697,246 @@ void vmx_vmcs_switch(struct vmcs_struct 
> >      spin_unlock(&vmx->vmcs_lock);
> >  }
> >  
> > +static noinline int pvh_construct_vmcs(struct vcpu *v)
> > +{
> 
> This looks a lot like the "normal" HVM function - can't those be
> folded reasonably, to reduce redundancy?
> 
> > @@ -1156,6 +1404,9 @@ void vmx_do_resume(struct vcpu *v)
> >          hvm_asid_flush_vcpu(v);
> >      }
> >  
> > +    if ( is_pvh_vcpu(v) )
> > +        goto skip_inteLdbgr;   /* PVH supports gdbsx and gdb
> > inside PVH */
> 
> What do "inte", "L", and "dbgr" stand for here? Also, to me the
> comment doesn't really explain anything.

I understand the code is for some external intel debugger. I meant
intel_dbgr. PVH, btw, is supported at the moment by gdbsx and gdb
running inside the PVH guest. Anyways, got rid of the label:

    if ( is_pvh_vcpu(v)
            reset_stack_and_jump(vmx_asm_do_vmentry);

    debug_state = v->domain->debugger_attached
......


thanks,
Mukesh