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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.
> >> FLUSH_ASID_CORE?
> > 
> > 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
> > ASID/VPID.
> 
> ... 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
FLUSH_HVM_ASID_CORE?

Thanks, Roger.

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