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

[Andrew Cooper <amc96@xxxxxxxx>]

On 13/01/2022 14:59, Jan Beulich wrote:
> 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 <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.
> Perhaps I was wrong recalling you somewhat disliking such uses of
> "continue" in the past.

Perhaps stockholm syndrome?  More likely, my judgement is subjective
based on what else is in the for loop.

I'll leave it as break to shrink the patch.

~Andrew