Re: [Xen-devel] [PATCH v2] xen: handle paged gfn in wrmsr_hypervisor_regs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.05.13 at 16:26, Keir Fraser <keir.xen@xxxxxxxxx> wrote:
> On 03/05/2013 14:58, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
>>>>> On 03.05.13 at 14:57, Olaf Hering <olaf@xxxxxxxxx> wrote:
>>> @@ -1682,14 +1682,25 @@ static int svm_msr_write_intercept(unsig
>>>          if ( wrmsr_viridian_regs(msr, msr_content) )
>>>              break;
>>>  
>>> -        wrmsr_hypervisor_regs(msr, msr_content);
>>> +        ret = wrmsr_hypervisor_regs(msr, msr_content);
>>> +        switch ( ret )
>>> +        {
>>> +            case -EAGAIN:
>>> +                result = X86EMUL_RETRY;
>>> +                break;
>>> +            case 0:
>>> +                result = X86EMUL_UNHANDLEABLE;
>>> +                break;
>>> +            default:
>>> +                break;
>> 
>> As you had already noticed the hard way - case 0 and default of
>> course need to be switched (0 -> okay, anything else ->
>> unhandleable).
> 
> No!
> 
> Actually anything other than -EAGAIN should be handled here as 'okay'. In
> fact the return codes from wrmsr_hypervisor_regs are going to be a bit of a
> mess if we're not careful.
> 
> I suggest the following return codes:
>  0: not handled
>  1: handled
>  -EINVAL: error during handling
>  -EAGAIN: retry
> 
> The HVM callers should then handle as follows:
>  -EAGAIN: rc = X86EMUL_RETRY
>  -EINVAL: goto gp_fault
>  0: try other msr handlers (if any)
>  1: we're done, return X86EMUL_OKAY
> 
> Does that make sense?

Sure - you may have seen my later reply where I also notide
this mistake in my first response to Olaf.

The only thing I'd like to ensure is to not constrain the code to
specific error codes - any negative value other than -EAGAIN
should result in #GP (or whatever a suitable action is) imo.

Jan


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