Re: [PATCH v2 15/16] x86/fsgsbase: Update fs/gs helpers to use wrmsrns()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.08.2025 22:49, Andrew Cooper wrote:
> On 19/08/2025 2:09 pm, Jan Beulich wrote:
>> On 15.08.2025 22:41, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -2733,8 +2733,8 @@ static uint64_t cf_check vmx_get_reg(struct vcpu *v, 
>>> unsigned int reg)
>>>      case MSR_SHADOW_GS_BASE:
>>>          if ( v != curr )
>>>              return v->arch.hvm.vmx.shadow_gs;
>>> -        rdmsrl(MSR_SHADOW_GS_BASE, val);
>>> -        return val;
>>> +        else
>>> +            return rdmsr(MSR_SHADOW_GS_BASE);
>>>      }
>> Here and below, can we please do without the pointless "else"? Strictly
>> speaking in Misra's terms that's "dead code" (things working identically
>> without), and I'm quite happy that I can now use this argument to
>> support my personal antipathy to this style of coding. Or else use the
>> conditional operator in such cases (where applicable).
> 
> No.  I have always, and will always prioritise readability first and
> foremost.

But my preference is precisely because of readability. The excess "else"
gives a wrong impression to the reader, when not looking closely enough.
(And I [now] expect you might say the opposite.)

> I do not agree with your interpretation of MISRA in this case.

Something to discuss in a broader group then, I suppose.

Jan