Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.02.17 at 17:34, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 07/02/17 16:22, Jan Beulich wrote:
>>>>> On 07.02.17 at 16:06, <sergey.dyasli@xxxxxxxxxx> wrote:
>>> If I understood correctly, you are suggesting the following change:
>> Mostly.
>>
>>> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
>>> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
>>> @@ -424,8 +424,8 @@ static inline unsigned long vmread_safe(unsigned long 
> field,
>>>      return ret;
>>>  }
>>>  
>>> -static always_inline unsigned long vmwrite_safe(unsigned long field,
>>> -                                                unsigned long value)
>>> +static always_inline enum vmx_insn_errno vmwrite_safe(unsigned long field,
>>> +                                                      unsigned long value)
>>>  {
>>>      unsigned long ret = 0;
>>>      bool fail_invalid, fail_valid;
>>> @@ -440,11 +440,16 @@ static always_inline unsigned long 
> vmwrite_safe(unsigned long field,
>>>                       [value] GAS_VMX_OP("rm", "c") (value));
>>>  
>>>      if ( unlikely(fail_invalid) )
>>> +    {
>>>          ret = VMX_INSN_FAIL_INVALID;
>>> +    }
>> No need to add braces here and ...
>>
>>>      else if ( unlikely(fail_valid) )
>>> +    {
>>>          __vmread(VM_INSTRUCTION_ERROR, &ret);
>>> +        BUG_ON(ret >= ~0U);
>>> +    }
>>>  
>>> -    return ret;
>>> +    return (enum vmx_insn_errno) ret;
>> ... no need for the cast here. (See Andrew's reply for the BUG_ON().)
>>
>>> And I have noticed one inconsistency: vmwrite_safe() is "always_inline"
>>> while vmread_safe() is plain "inline". I believe that plain inline is
>>> enough here, what do you think?
>> I would assume plain inline to be enough, but maybe the VMX
>> maintainers know why always_inline was used.
> 
> The always_inline was my doing IIRC, because the use of unlikely
> sections caused GCC to create a separate identical functions in each
> translation unit, in an attempt to minimise the quantity of out-of-line
> code.

In which case it's not needed in these new flavors.

Jan


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