[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()
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. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |