> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 02 May 2019 14:23 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: Andrew Cooper <andrew.coop...@citrix.com>; George Dunlap > <george.dun...@citrix.com>; Roger Pau > Monne <roger....@citrix.com>; Wei Liu <wei.l...@citrix.com>; xen-devel <xen- > de...@lists.xenproject.org> > Subject: RE: [PATCH 4/9] x86/HVM: move NOFLUSH handling out of hvm_set_cr3() > > >>> On 02.05.19 at 15:07, <paul.durr...@citrix.com> wrote: > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> 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 Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
