[Xen-devel] Ping²: [PATCH 4/4] x86/PV: remove unnecessary toggle_guest_pt() overhead

* [Xen-devel] Ping²: [PATCH 4/4] x86/PV: remove unnecessary toggle_guest_pt() overhead
@ 2019-07-15  8:44 Jan Beulich
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Beulich @ 2019-07-15  8:44 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: George Dunlap, xen-devel, Wei Liu, Roger Pau Monné

>>> On 27.05.19 at 11:25,  wrote:
>>>> On 13.03.19 at 13:39,  wrote:
> > While the mere updating of ->pv_cr3 and ->root_pgt_changed aren't overly
> > expensive (but still needed only for the toggle_guest_mode() path), the
> > effect of the latter on the exit-to-guest path is not insignificant.
> > Move the logic into toggle_guest_mode().
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> I think I did address the one concern you had.
> 
> Jan

https://lists.xenproject.org/archives/html/xen-devel/2019-04/msg00306.html
Ping?

> > --- a/xen/arch/x86/pv/domain.c
> > +++ b/xen/arch/x86/pv/domain.c
> > @@ -349,18 +349,10 @@ bool __init xpti_pcid_enabled(void)
> >  
> >  static void _toggle_guest_pt(struct vcpu *v)
> >  {
> > -    const struct domain *d = v->domain;
> > -    struct cpu_info *cpu_info = get_cpu_info();
> >      unsigned long cr3;
> >  
> >      v->arch.flags ^= TF_kernel_mode;
> >      update_cr3(v);
> > -    if ( d->arch.pv.xpti )
> > -    {
> > -        cpu_info->root_pgt_changed = true;
> > -        cpu_info->pv_cr3 = __pa(this_cpu(root_pgt)) |
> > -                           (d->arch.pv.pcid ? get_pcid_bits(v, true) : 0);
> > -    }
> >  
> >      /*
> >       * Don't flush user global mappings from the TLB. Don't tick TLB clock.
> > @@ -368,15 +360,11 @@ static void _toggle_guest_pt(struct vcpu
> >       * In shadow mode, though, update_cr3() may need to be accompanied by a
> >       * TLB flush (for just the incoming PCID), as the top level page table 
> > may
> >       * have changed behind our backs. To be on the safe side, suppress the
> > -     * no-flush unconditionally in this case. The XPTI CR3 write, if 
> > enabled,
> > -     * will then need to be a flushing one too.
> > +     * no-flush unconditionally in this case.
> >       */
> >      cr3 = v->arch.cr3;
> > -    if ( shadow_mode_enabled(d) )
> > -    {
> > +    if ( shadow_mode_enabled(v->domain) )
> >          cr3 &= ~X86_CR3_NOFLUSH;
> > -        cpu_info->pv_cr3 &= ~X86_CR3_NOFLUSH;
> > -    }
> >      write_cr3(cr3);
> >  
> >      if ( !(v->arch.flags & TF_kernel_mode) )
> > @@ -392,6 +380,8 @@ static void _toggle_guest_pt(struct vcpu
> >  
> >  void toggle_guest_mode(struct vcpu *v)
> >  {
> > +    const struct domain *d = v->domain;
> > +
> >      ASSERT(!is_pv_32bit_vcpu(v));
> >  
> >      /* %fs/%gs bases can only be stale if WR{FS,GS}BASE are usable. */
> > @@ -405,6 +395,21 @@ void toggle_guest_mode(struct vcpu *v)
> >      asm volatile ( "swapgs" );
> >  
> >      _toggle_guest_pt(v);
> > +
> > +    if ( d->arch.pv.xpti )
> > +    {
> > +        struct cpu_info *cpu_info = get_cpu_info();
> > +
> > +        cpu_info->root_pgt_changed = true;
> > +        cpu_info->pv_cr3 = __pa(this_cpu(root_pgt)) |
> > +                           (d->arch.pv.pcid ? get_pcid_bits(v, true) : 0);
> > +        /*
> > +         * As in _toggle_guest_pt() the XPTI CR3 write needs to be a TLB-
> > +         * flushing one too for shadow mode guests.
> > +         */
> > +        if ( shadow_mode_enabled(d) )
> > +            cpu_info->pv_cr3 &= ~X86_CR3_NOFLUSH;
> > +    }
> >  }
> >  
> >  void toggle_guest_pt(struct vcpu *v)
> > 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread