Re: [Xen-devel] [PATCH 1/3] xen: Add instruction length parameter in function hvm_inject_exception

["Hao, Xudong" <xudong.hao@xxxxxxxxx>]

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@xxxxxxxxx] On Behalf Of Keir Fraser
> Sent: Friday, May 25, 2012 4:53 PM
> To: Hao, Xudong; JBeulich@xxxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxx; Dong, Eddie; Aravindh Puthiyaparambil; Ian
> Jackson; Zhang, Xiantao
> Subject: Re: [PATCH 1/3] xen: Add instruction length parameter in function
> hvm_inject_exception
> 
> On 24/05/2012 20:06, "Xudong Hao" <xudong.hao@xxxxxxxxx> wrote:
> 
> > VMX exception: Pass the instruction length field down to exception
> emulation,
> > by changing hypercall parameter and adding a parameter in fucntion
> > hvm_inject_exception(), so that exception emulation can set correct
> > instruction length.
> >
> > Signed-off-by: Xudong Hao <xudong.hao@xxxxxxxxx>
> > Signed-off-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
> 
> I don't like adding yet another parameter for all callers, especially as we
> should also be passing down the event type (sw interrupt, hw exception, ...)
> and that would bloat the parameter list further.
> 
> I attach a cleanup patch which packs everything into a new struct and
> renames hvm_inject_exception to hvm_inject_trap. I then define a couple of
> wrappers around that function for existing callers, so that their parameter
> lists actually *shrink*.
> 
> Let me know what you think. If it looks acceptable I can check it in and we
> can build on top of this restructuring. The aim being to keep
> hvm/vmx_inject_trap dumb, and push down the knowledge from the callers
> into
> it.

Hi, Keir

This patch is fine to me, just one small comments: the note below the 
hvm_inject_hw_exception(TRAP*) should change to hvm_inject_hw_exception from 
hvm_inject_exception.

--- a/xen/arch/x86/hvm/svm/nestedsvm.c  Fri May 25 08:21:25 2012 +0100
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c  Fri May 25 09:45:00 2012 +0100
@@ -735,7 +735,7 @@ nsvm_vcpu_vmrun(struct vcpu *v, struct c
     default:
         gdprintk(XENLOG_ERR,
             "nsvm_vcpu_vmentry failed, injecting #UD\n");
-        hvm_inject_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE, 0);
+        hvm_inject_hw_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE);
         /* Must happen after hvm_inject_exception or it doesn't work right. */

And I'll add instruction length represented in 'struct hvm_trap', and correct 
the _trap.type in new function vmx_inject_trap() based on the restructuring.

> 
>  -- Keir


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