From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [RFC PATCH 9/16]: PVH xen: create PVH vmcs and
 initialization
Date: Tue, 15 Jan 2013 16:55:12 -0800
Message-ID: <20130115165512.08b21ed2@mantra.us.oracle.com>
References: <20130111175936.7b3baf67@mantra.us.oracle.com>
	<50F4008702000078000B5416@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <50F4008702000078000B5416@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

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