From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [V10 PATCH 11/23] PVH xen: support invalid op
 emulation for PVH
Date: Thu, 8 Aug 2013 17:17:12 -0700
Message-ID: <20130808171712.733e637e@mantra.us.oracle.com>
References: <1374631171-15224-1-git-send-email-mukesh.rathor@oracle.com>
	<1374631171-15224-12-git-send-email-mukesh.rathor@oracle.com>
	<CAFLBxZZyTypo6-8hq8mty=6Dv7_Hn=3kAiXY8wHAQtnf_Jv0rA@mail.gmail.com>
	<20130807184912.2115ed40@mantra.us.oracle.com>
	<52035CFE.9020804@eu.citrix.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: <52035CFE.9020804@eu.citrix.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: George Dunlap <george.dunlap@eu.citrix.com>
Cc: "xen-devel@lists.xensource.com" <Xen-devel@lists.xensource.com>, "keir.xen@gmail.com" <keir.xen@gmail.com>
List-Id: xen-devel@lists.xenproject.org

On Thu, 8 Aug 2013 09:55:26 +0100
George Dunlap <george.dunlap@eu.citrix.com> wrote:

> On 08/08/13 02:49, Mukesh Rathor wrote:
> > On Wed, 7 Aug 2013 12:29:13 +0100
> > George Dunlap <George.Dunlap@eu.citrix.com> wrote:
> >
> >> On Wed, Jul 24, 2013 at 2:59 AM, Mukesh Rathor
......... 
> >>> +    if ( (rc = raw_copy_from_guest(sig, (char *)eip,
> >>> sizeof(sig))) != 0 ) {
> >>>           propagate_page_fault(eip + sizeof(sig) - rc, 0);
> >>>           return EXCRET_fault_fixed;
> >>> @@ -931,7 +936,7 @@ static int emulate_forced_invalid_op(struct
> >>> cpu_user_regs *regs) eip += sizeof(sig);
> >>>
> >>>       /* We only emulate CPUID. */
> >>> -    if ( ( rc = copy_from_user(instr, (char *)eip,
> >>> sizeof(instr))) != 0 )
> >>> +    if ( ( rc = raw_copy_from_guest(instr, (char *)eip,
> >>> sizeof(instr))) != 0 ) {
> >>>           propagate_page_fault(eip + sizeof(instr) - rc, 0);
> >>>           return EXCRET_fault_fixed;
> >>> @@ -1076,6 +1081,12 @@ void propagate_page_fault(unsigned long
> >>> addr, u16 error_code) struct vcpu *v = current;
> >>>       struct trap_bounce *tb = &v->arch.pv_vcpu.trap_bounce;
> >>>
> >>> +    if ( is_pvh_vcpu(v) )
> >>> +    {
> >>> +        hvm_inject_page_fault(error_code, addr);
> >>> +        return;
> >>> +    }
> >> Would it make more sense to rename this function
> >> "pv_inject_page_fault", and then make a macro to switch between the
> >> two?
> > I don't think so, propagate_page_fault seems generic enough.
> 
> What I meant was something similar to what I suggested for patch 10
> -- make propagate_page_fault() truly generic, by making it check what
> mode is running and calling either pv_inject_page_fault() or 
> hvm_inject_page_fault() as appropriate.

I guess, what you mean:

propagate_page_fault():
{
   if (pvh)
       hvm_inject_pf

   else if pv
       pv_inject_pf
}

where pv_inject_pf() is all the code after my "if pvh" in the current
patch. Small enough change I can accomodate in the next patch.

Mukesh