Re: [Xen-devel] Issues/improvements performing flush of guest TLBs

[Tim Deegan <tim@xxxxxxx>]

Hi,

At 12:16 +0100 on 15 Jan (1579090561), Roger Pau Monné wrote:
>  - Shadow: it's not clear to me exactly which parts of sh_update_cr3
>    are needed in order to perform a guest TLB flush. I think calling:
> 
> #if (SHADOW_OPTIMIZATIONS & SHOPT_VIRTUAL_TLB)
>     /* No longer safe to use cached gva->gfn translations */
>     vtlb_flush(v);
> #endif
> #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
>     /* Need to resync all the shadow entries on a TLB flush. */
>     shadow_resync_current_vcpu(v);
> #endif
> 
>     if ( is_hvm_domain(d) )
>         /*
>          * Linear mappings might be cached in non-root mode when ASID/VPID is
>          * in use and hence they need to be flushed here.
>          */
>         hvm_asid_flush_vcpu(v);
> 
>    Should be enough but I'm not very familiar with the shadow code,
>    and hence would like some feedback from someone more familiar with
>    shadow in order to assert exactly what's required to perform a
>    guest TLB flush.

I would advise keeping the whole thing until you have measurememnts
that show that it's worthwhile being clever here (e.g. that the IPI
costs don't dominate).

But I think for safety we need at least the code you mention and also:
 - the code that reloads the PAE top-level entries; and
 - the shadow_resync_other_vcpus() at the end.

>    Also, AFAICT sh_update_cr3 is not safe to be called on vCPUs
>    currently running on remote pCPUs, albeit there are no assertions
>    to that end. It's also not clear which parts of sh_update_cr3 are
>    safe to be called while the vCPU is running.

Yeah, sh_update_cr3 makes a bunch of state changes and assumes
that the vcpu can't do TLB loads part-way through.  It may be possible
to do some of it remotely but as you say it would take a lot of
thinking, and if the guest is running you're going to need an IPI
anyway to flush the actual TLB.

> FWIW, there also seems to be a lot of unneeded flushes of HVM guests
> TLB, as do_tlb_flush will unconditionally clear all HVM guest TLBs on
> the pCPU by calling hvm_asid_flush_core which I don't think it's
> necessary/intended by quite a lot of the Xen TLB flush callers. I
> guess this would also warrant a different discussion, as there seems
> to be room for improvement in this area.

There may be room for improvement, but do be careful - the Xen MM
safety rules depend on TLB flushes when a page's type or ownership
changes, and that does mean flushing even the guest TLBs.  ISTR
discussing this at the time that vTLBs were introduced and deciding
that it wasn't worth adding all the tracking that would be necessary;
that may have changed now that the p2m infrastructure is better
developed.

Cheers,

Tim.

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