Re: [Xen-devel] [PATCH 4/9] x86/HVM: move NOFLUSH handling out of hvm_set_cr3()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.05.19 at 15:07, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 02 May 2019 13:20
>> 
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -2072,6 +2072,8 @@ static int hvmemul_write_cr(
>>      HVMTRACE_LONG_2D(CR_WRITE, reg, TRC_PAR_LONG(val));
>>      switch ( reg )
>>      {
>> +        bool noflush;
>> +
> 
> Why introduce 'noflush' with this scope when it could be limited to 'case 
> 3:', although...

Because this would entail introducing another set of braces, and
I pretty much dislike these case-block braces: They either don't
properly indent (as we do commonly), or they needlessly increase
indentation of the enclosed block. Hence my general preference
of switch-scope local variables.

>> @@ -2082,7 +2084,10 @@ static int hvmemul_write_cr(
>>          break;
>> 
>>      case 3:
>> -        rc = hvm_set_cr3(val, true);
>> +        noflush = hvm_pcid_enabled(current) && (val & X86_CR3_NOFLUSH);
>> +        if ( noflush )
>> +            val &= ~X86_CR3_NOFLUSH;
> 
> ... can't you just code this as:
> 
> if ( hvm_pcid_enabled(current) )
>     val &= ~X86_CR3_NOFLUSH;
> 
> ?

Because of ...

>> +        rc = hvm_set_cr3(val, noflush, true);

... this further use of "noflush" (alongside the adjusted "val").

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel