> -----Original Message----- > From: Jan Beulich [mailto:[email protected]] > Sent: 02 May 2019 14:23 > To: Paul Durrant <[email protected]> > Cc: Andrew Cooper <[email protected]>; George Dunlap > <[email protected]>; Roger Pau > Monne <[email protected]>; Wei Liu <[email protected]>; xen-devel <xen- > [email protected]> > Subject: RE: [PATCH 4/9] x86/HVM: move NOFLUSH handling out of hvm_set_cr3() > > >>> On 02.05.19 at 15:07, <[email protected]> wrote: > >> From: Jan Beulich [mailto:[email protected]] > >> 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"). >
Ah, missed that... I'd still go for the tighter scope though, but then I don't mind the extra braces. Paul > Jan > _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
