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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 17 September 2019 14:03
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Suravee Suthikulpanit 
> <suravee.suthikulpanit@xxxxxxx>; Andrew
> Cooper <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; 
> Roger Pau Monne
> <roger.pau@xxxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Kevin Tian 
> <kevin.tian@xxxxxxxxx>;
> BorisOstrovsky <boris.ostrovsky@xxxxxxxxxx>; Paul Durrant <paul@xxxxxxx>; Wei 
> Liu <wl@xxxxxxx>
> Subject: Re: [Xen-devel] [PATCH v2 4/9] x86/HVM: move NOFLUSH handling out of 
> hvm_set_cr3()
> 
> On 17.09.2019 14:45, Paul Durrant wrote:
> >> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan 
> >> Beulich
> >> Sent: 17 September 2019 07:15
> >>
> >> --- a/xen/arch/x86/hvm/emulate.c
> >> +++ b/xen/arch/x86/hvm/emulate.c
> >> @@ -2080,6 +2080,8 @@ static int hvmemul_write_cr(
> >>      HVMTRACE_LONG_2D(CR_WRITE, reg, TRC_PAR_LONG(val));
> >>      switch ( reg )
> >>      {
> >> +        bool noflush;
> >> +
> >
> > I said this before... I think the tighter case-scope is better.
> 
> I recall you saying so, but I don't recall you actually making this a
> requirement to get your ack.
> 
> > Cosmetically it may look a little odd, but surely it gives the
> > compiler a better chance to optimize?
> 
> I don't think so - they're pretty good to limit the life range of
> variables (at least in not overly hairy functions) these days. The
> more narrow scopes we often ask for are more for the reader to
> easily know what the intended usage range of a variable is.

Ok. I'm not going to insist, but I would still prefer case-scope here.

Reviewed-by: Paul Durrant <paul@xxxxxxx>

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