Re: [PATCH 10/17] PVH xen: introduce vmx_pvh.c and pvh.c

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

On Thu, 02 May 2013 07:42:16 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 02.05.13 at 03:10, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Wed, 01 May 2013 14:52:27 +0100
> > "Jan Beulich" <jbeulich@suse.com> wrote:
> >> >> > +static int vmxit_invalid_op(struct cpu_user_regs *regs)
> >> >> > +{
> >> >> > +    ulong addr = 0;
> >> >> > +
> >> >> > +    if ( guest_kernel_mode(current, regs) ||
> >> >> > +         emulate_forced_invalid_op(regs, &addr) == 0 )
> >> >> > +    {
> >> >> > +        hvm_inject_hw_exception(TRAP_invalid_op,
> >> >> > HVM_DELIVER_NO_ERROR_CODE);
> >> >> > +        return 0;
> >> >> > +    }
> >> >> > +    if ( addr )
> >> >> > +        hvm_inject_page_fault(0, addr);
> >> >> 
> >> >> This cannot be conditional upon addr being non-zero.
> >> >
> >> >Why not? rc = emulate_forced_invalid_op():
> >> 
> >> Because zero can be a valid address that a fault occurred on.
> > 
> > Hmm... for that to happen, the guest would have to cause vmxit
> > with invalid op at address 000H. I didn't think that was possible.
> 
> Why would it not. You have to cover all guest kernels, and not
> misbehave on malicious ones (i.e. those ought to get an
> exception injected if so needed, no matter what address it
> occurred on).
> 
> > Alternate would be to add a new return code:  EXCRET_inject_pf.
> 
> Something along those lines, yes.

Actually, sigh, I realized I missed emulate_privileged_op() and the 
macro insn_fetch which calls propagate_page_fault for PVH also. So I
am thinking of just giving in and writing up a
pvh_propagate_page_fault() function that propagate_page_fault() can
just call. Then  emulate_forced_invalid_op() can remain as is.

Mukesh