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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.01.13 at 03:29, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> Done. No other callers of prop_page_fault for PVH. So are you OK with
> something like this:
> 
> static noinline int vmxit_invalid_op(struct cpu_user_regs *regs)
> {
>     ulong addr=0;
> 
>     if ( guest_kernel_mode(current, regs) ||
>          (addr = emulate_forced_invalid_op(regs)) == 0 )

Actually, on a second thought that depends on whether you want
to be able to build kernels that can run both PV and PVH. If so,
you may need to emulate this even for the guest kernel.

>     {
>         hvm_inject_hw_exception(TRAP_invalid_op,
>     HVM_DELIVER_NO_ERROR_CODE); return 0;
>     }
> 
>     if (addr != EXCRET_fault_fixed)
>         hvm_inject_page_fault(0, addr);
>     return 0;
> }
> 
> 
> unsigned long emulate_forced_invalid_op(struct cpu_user_regs *regs)
> {
>     char sig[5], instr[2];
>     unsigned long eip, rc, addr;
> 
>     eip = regs->eip;
> 
>     /* Check for forced emulation signature: ud2 ; .ascii "xen". */
>     if ( (rc = raw_copy_from_guest(sig, (char *)eip, sizeof(sig))) !=
>     0 ) {
>         addr = eip + sizeof(sig) - rc;
>         if ( is_pvh_vcpu(current) )
>             return addr;
> 
>         propagate_page_fault(addr, 0);
>         return EXCRET_fault_fixed;
>     }
>     if ( memcmp(sig, "\xf\xbxen", sizeof(sig)) )
>         return 0;
>     eip += sizeof(sig);
> 
>     /* We only emulate CPUID. */
>     if ( ( rc = raw_copy_from_guest(instr, (char *)eip,
>     sizeof(instr))) != 0 ) {
>         addr = eip + sizeof(instr) - rc;
>         if ( is_pvh_vcpu(current) )
>             return addr;
> 
>         propagate_page_fault(addr, 0);
>         return EXCRET_fault_fixed;
>     }
>     if ( memcmp(instr, "\xf\xa2", sizeof(instr)) )
>         return 0;
>     eip += sizeof(instr);
> 
>     pv_cpuid(regs);
> 

Looks okay up to here at a first glance.

>     if ( is_pvh_vcpu(current) )
>         regs->eip = eip;
>     else
>         instruction_done(regs, eip, 0);

Why can't you use instruction_done() (or make it fit your needs,
so that other code wanting to use it wouldn't need similar special
casing)?

Jan

>     trace_trap_one_addr(TRC_PV_FORCED_INVALID_OP, regs->eip);
> 
>     return EXCRET_fault_fixed;



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