Re: [Xen-devel] [PATCH 5/8] x86/hvm: Don't raise #GP behind the emulators back for MSR accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.12.16 at 11:09, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -509,7 +509,11 @@ void hvm_do_resume(struct vcpu *v)
>  
>          if ( w->do_write.msr )
>          {
> -            hvm_msr_write_intercept(w->msr, w->value, 0);
> +            int rc = hvm_msr_write_intercept(w->msr, w->value, 0);
> +
> +            if ( rc == X86EMUL_EXCEPTION )
> +                hvm_inject_hw_exception(TRAP_gp_fault, 0);

The use of a local variable looks kind of pointless here.

> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1788,7 +1788,6 @@ static int svm_msr_read_intercept(unsigned int msr, 
> uint64_t *msr_content)
>      return X86EMUL_OKAY;
>  
>   gpf:
> -    hvm_inject_hw_exception(TRAP_gp_fault, 0);
>      return X86EMUL_EXCEPTION;
>  }
>  
> @@ -1945,7 +1944,6 @@ static int svm_msr_write_intercept(unsigned int msr, 
> uint64_t msr_content)
>      return result;
>  
>   gpf:
> -    hvm_inject_hw_exception(TRAP_gp_fault, 0);
>      return X86EMUL_EXCEPTION;
>  }

In cases like these it would certainly be nice to get rid of the now
rather pointless goto-s, but of course we can equally well do this
in a later patch.

> @@ -1976,6 +1974,8 @@ static void svm_do_msr_access(struct cpu_user_regs 
> *regs)
>  
>      if ( rc == X86EMUL_OKAY )
>          __update_guest_eip(regs, inst_len);
> +    else if ( rc == X86EMUL_EXCEPTION )
> +        hvm_inject_hw_exception(TRAP_gp_fault, 0);

    else
        ASSERT_UNREACHABLE();

? (And then similarly for VMX.)

> +/*
> + * May return X86EMUL_EXCEPTION, at which point the caller is responsible for
> + * injecting a #GP fault.  Used to support speculative reads.
> + */
> +int hvm_msr_read_intercept(unsigned int msr, uint64_t *msr_content);
> +int hvm_msr_write_intercept(
> +    unsigned int msr, uint64_t msr_content, bool_t may_defer);

Please add __must_check to both. With at least this one taken care of
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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