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

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

[Roger Pau Monné]

Hello,

For the last days I've been trying to figure out how to properly
perform flushes of guest TLBs from the hypervisor. We currently
provide a hypercall to HVM guests (HVMOP_flush_tlbs). The hypercall
however pauses all vCPUs that require a flush and then call
paging_update_cr3, which is highly inefficient. The performance of
such implementation on a non-overloaded environment seems to be on par
with the guest issuing IPIs and performing cr3/cr4 writes in order to
flush, which makes the point of providing such hypercall moot.

I would like to provide hooks (in the paging_mode struct) for the HAP
and Shadow paging modes in order to perform guest TLB flushes:

 - HAP: depends on whether ASID/VPID is in use. If not in use the
   TLBs will be flushed on each vmexit/vmenter. If in use changing the
   VPID/ASID or flushing the specific VPID/ASID should be enough. This
   requires calling hvm_asid_flush_vcpu for each vCPU to be flushed
   and issuing an IPI to the currently running vCPUs in order to
   trigger a vmexit that will sync the VPID/ASID with the value in the
   vmcs/vmcb. Ie:

    for_each_vcpu( v, d )
        hvm_asid_flush_vcpu(v);
    on_selected_cpus(....);

 - 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.

   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.

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.

Thanks, Roger.

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

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

[Tim Deegan]

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel