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

Re: [Xen-devel] [PATCH v5 4/7] x86/tlb: introduce a flush guests TLB flag

On Mon, Mar 02, 2020 at 11:31:23AM +0100, Jan Beulich wrote:
> On 28.02.2020 17:50, Roger Pau Monné wrote:
> > On Fri, Feb 28, 2020 at 05:14:05PM +0100, Jan Beulich wrote:
> >> On 19.02.2020 18:43, Roger Pau Monne wrote:
> >>> This was previously unconditionally done in each pre_flush call, but
> >>> that's not required: HVM guests not using shadow don't require linear
> >>> TLB flushes as Xen doesn't modify the guest page tables in that case
> >>> (ie: when using HAP).
> >>
> >> This explains the correctness in one direction. What about the
> >> removal of this from the switch_cr3_cr4() path?
> > 
> > AFAICT that's never used by shadow code to modify cr3 or cr4, and
> > hence doesn't require a guest linear TLB flush.
> XSA-294 tells me to be very conservative here. It is not necessarily
> the direct use by shadow code that matters; toggle_guest_*() isn't
> used directly by it, either.

toggle_guest_{mode/pt} seems to be exclusively used by PV guests. I'm
fine with adding extra flushes to be on the safe side, but those
functions are never used against a HVM guest AFAICT. The only reason
to flush a HVM guest 'internal' TLB is when using shadow, and in that
case the shadow code must already take care of issuing such flushes.

> >> I admit the names of the involved functions
> >> (hvm_flush_guest_tlbs(), hvm_asid_flush_core()) are somewhat
> >> misleading, as they don't actually do any flushing, they merely
> >> arrange for what is in the TLB to no longer be able to be used,
> >> so giving this a suitable name is "historically" complicated.
> >> What if we did away with the hvm_flush_guest_tlbs() wrapper,
> >> naming the constant here then after hvm_asid_flush_core(), e.g.
> > 
> > I'm not opposed to renaming. The comment before the definition was
> > also meant to clarify it's usage, and hence the explicit mention of
> ... there's also one more argument for renaming: The present
> name doesn't convey at all that this operation is HVM-only
> (i.e. PV guests wouldn't have their TLBs [as far as one can
> call them "their"] flushed).

Do you think FLUSH_ASID_CORE is clear enough, or would you prefer

Thanks, Roger.

Xen-devel mailing list



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