Re: [PATCH] x86/hvm: Improve hvm_set_guest_pat() code generation

[Andrew Cooper <amc96@xxxxxxxx>]

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 <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> 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.

~Andrew

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.