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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/12/16 12:10, Jan Beulich wrote:
>>>> 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.

The first version had

if ( hvm_msr_write_intercept(w->msr, w->value, 0) ==
     X86EMUL_EXCEPTION )

but this looked rather ugly to read.  I prefer the version as submitted,
but am not too fussed if you insist for the latter?

>
>> --- 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.

I will do a cleanup patch and add it to v2.

~Andrew

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