Re: [Xen-devel] [RFC PATCH 12/16]: PVH xen: return PVH features during creation, etc...

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.01.13 at 03:07, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> @@ -910,6 +915,10 @@ int emulate_forced_invalid_op(struct cpu
>      /* Check for forced emulation signature: ud2 ; .ascii "xen". */
>      if ( (rc = copy_from_user(sig, (char *)eip, sizeof(sig))) != 0 )
>      {
> +        /* PVH: fixme: hmm... what do we do for PVH? */
> +        if ( is_pvh_vcpu(current) )

The fixme and check ought to sit earlier - the copy_from_user()
above isn't valid there. And I don't see how you would validly
get here anyway - you don't need to intercept GP faults to
emulate guest CPUID invocations.

> @@ -1566,6 +1586,10 @@ static int guest_io_okay(
>      int user_mode = !(v->arch.flags & TF_kernel_mode);
>  #define TOGGLE_MODE() if ( user_mode ) toggle_guest_mode(v)
>  
> +    /* for PVH we check this in vmexit for EXIT_REASON_IO_INSTRUCTION */
> +    if (is_pvh_vcpu(v))

The why would it get here at all?

> @@ -1811,8 +1835,9 @@ static inline uint64_t guest_misc_enable
>          _ptr = (unsigned int)_ptr;                                          \
>      if ( (limit) < sizeof(_x) - 1 || (eip) > (limit) - (sizeof(_x) - 1) )   \
>          goto fail;                                                          \
> -    if ( (_rc = copy_from_user(&_x, (type *)_ptr, sizeof(_x))) != 0 )       \
> +    if ( (_rc = raw_copy_from_guest(&_x, (type *)_ptr, sizeof(_x))) != 0 )  \

So here you realized the need to change the call.

>      {                                                                       \
> +        /* PVH: fixme: probably return -EFAULT ??? */                       \
>          propagate_page_fault(_ptr + sizeof(_x) - _rc, 0);                   \

I don't think so - propagate_page_fault() should do the right thing
in that case, if you can validly get here for a PVH guest.

> @@ -2132,7 +2157,8 @@ int emulate_privileged_op(struct cpu_use
>  
>      case 0xfa: /* CLI */
>      case 0xfb: /* STI */
> -        if ( v->arch.pv_vcpu.iopl < (guest_kernel_mode(v, regs) ? 1 : 3) )
> +        if ( !is_pvh_vcpu(v)  &&

This ought to be impossible.

> @@ -444,6 +444,8 @@ static long register_guest_callback(stru
>      long ret = 0;
>      struct vcpu *v = current;
>  
> +    NO_PVH_ASSERT_VCPU(v);

Either the code is unreachable for a PVH guest (in which case the
assert is likely superfluous, or you need to return an error here
rather than asserting.

Jan


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