[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Thu, Sep 12, 2019 at 01:52:12PM +0200, Jan Beulich wrote:
> On 12.09.2019 13:35, Roger Pau Monné  wrote:
> > On Wed, Sep 11, 2019 at 05:23:20PM +0200, Jan Beulich wrote:
> >> The bit is meaningful only for MOV-to-CR3 insns, not anywhere else, in
> >> particular not when loading nested guest state.
> > 
> > Can't you use the current vcpu to check if the guest is in nested
> > mode, and avoid having to explicitly pass the noflush parameter?
> 
> Even if this implication held today (it doesn't according to
> the uses in hvmemul_write_cr() and hvm_mov_to_cr()), I don't
> think introducing such a dependency would be a good idea.

Oh, I see. Even when running a nested guest hvm_mov_to_cr could still
cause a no flush load.

> >> @@ -2282,12 +2287,11 @@ int hvm_set_cr0(unsigned long value, boo
> >>      return X86EMUL_OKAY;
> >>  }
> >>  
> >> -int hvm_set_cr3(unsigned long value, bool may_defer)
> >> +int hvm_set_cr3(unsigned long value, bool noflush, bool may_defer)
> > 
> > I would rather introduce a flush parameter instead, I think negated
> > booleans are harder to parse mentally, and easier to get wrong.
> 
> I did actually consider this, but decided against for the
> reason of this "no flush" behavior being a later addition to
> the effects CR3 writes have, i.e. I'd intentionally like it
> to be in line with X86_CR3_NOFLUSH.

IMO the hardware addition is a noflush flag in order to keep the
previous behaviour of a cr3 write (ie: no flush has to be an
opt-in). Here it's a software interface that already requires you to
change the call sites anyway, and hence I would have preferred a flush
parameter. I see however there's already quite some functions using
such a noflush parameter, so I'm not going to oppose.

On a different question, AFAICT hvm_set_cr3 should never be called
with X86_CR3_NOFLUSH set? If so, do you think it would make sense to
add an assert to that regard?

Thanks, Roger.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.