Re: [Xen-devel] [PATCH 5/6] x86/hvm: Forced Emulation Prefix for debug builds of Xen

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.09.14 at 17:03, <andrew.cooper3@xxxxxxxxxx> wrote:

[re-ordering patch contents for a better response context]

> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -514,6 +514,11 @@ bool_t nhvm_vmcx_hap_enabled(struct vcpu *v);
>  /* interrupt */
>  enum hvm_intblk nhvm_interrupt_blocked(struct vcpu *v);
>  
> +#ifndef NDEBUG
> +/* Permit use of the Forced Emulation Prefix in HVM guests */
> +extern bool_t opt_hvm_fep;

#else
#define opt_hvm_fep 0

> +#endif

allowing this to be the single place of dependency on NDEBUG (in
case someone wants to change the condition to some thing else)
by ...

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -86,6 +86,11 @@ unsigned long __attribute__ ((__section__ 
> (".bss.page_aligned")))
>  static bool_t __initdata opt_hap_enabled = 1;
>  boolean_param("hap", opt_hap_enabled);
>  
> +#ifndef NDEBUG
> +bool_t opt_hvm_fep;
> +boolean_param("hvm_fep", opt_hvm_fep);
> +#endif

... making this

#ifndef opt_hvm_fep

and ...

> +#ifndef NDEBUG
> +    if ( opt_hvm_fep )
> +    {
> +        XEN_GUEST_HANDLE(char) guest_rip = { (char*)regs->eip };
> +        char sig[5]; /* ud2; .ascii "xen" */
> +
> +        if ( (copy_from_guest(sig, guest_rip, sizeof(sig)) == 0) &&
> +             (memcmp(sig, "\xf\xbxen", sizeof(sig)) == 0) )
> +        {
> +            regs->eip += sizeof(sig);
> +            regs->eflags &= ~X86_EFLAGS_RF;
> +        }
> +    }
> +#endif

... dropping the preprocessor conditionals both here and in the
similar VMX code chunk.

Apart from that I'd really like you to consider using
copy_from_user_hvm() or hvm_fetch_from_guest_virt_nofault()
- there's no visible benefit from using the PV-aware
copy_from_guest() here.

Jan


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