On 13.01.2022 15:45, Andrew Cooper wrote:
> On 13/01/2022 14:38, Jan Beulich wrote:
>> On 13.01.2022 14:50, Andrew Cooper wrote:
>>> This is a fastpath on virtual vmentry/exit, and forcing guest_pat to be
>>> spilled to the stack is bad.  Performing the shift in a register is far more
>>> efficient.
>>>
>>> Drop the (IMO useless) log message.  MSR_PAT only gets altered on boot, and 
>>> a
>>> bad value will be entirely evident in the ensuing #GP backtrace.
>>>
>>> Signed-off-by: Andrew Cooper <[email protected]>
>> Reviewed-by: Jan Beulich <[email protected]>
>>
>> I'm curious though why ...
>>
>>> @@ -313,10 +313,9 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
>>>          case PAT_TYPE_WRCOMB:
>>>          case PAT_TYPE_WRPROT:
>>>          case PAT_TYPE_WRTHROUGH:
>>> -            break;
>>> +            continue;
>> ... you're going from "break" to "continue" here.
> 
> I went through a couple of iterations, including one not having a switch
> statement at all.
> 
> Personally, I think continue is clearer to follow in constructs such as
> this, because it is clearly bound to the loop, while the break logic
> only works due to the switch being the final (only) clause.

Perhaps I was wrong recalling you somewhat disliking such uses of
"continue" in the past.

> P.S. if you want to see a hilarious Clang (mis)feature, check out
> https://godbolt.org/z/7z6PnKP31 - scroll to the bottom of the -O2 output.

"Nice".

Jan


Reply via email to