| Message ID | c503e06d-9174-1dab-4f3f-1d7c88b499e9@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/EPT: squash meaningless TLB flush | expand |
On 30/11/2021 16:10, Jan Beulich wrote: > ept_free_entry() gets called after a flush - if one is necessary in the > first place - was already issued. That behavior is similar to NPT, which > also doesn't have any further flush in p2m_free_entry(). (Furthermore, > the function being recursive, in case of recursiveness way too many > flushes would have been issued.) > > Signed-off-by: Jan Beulich <jbeulich@suse.com> ept_free_entry() is called either recursively, or after an ept_split_super_page(), and I agree that it does not want to queue multiple flushes. However, I don't see where a suitable flush is in the superpage path. Am I overlooking something? ~Andrew
On 30.11.2021 17:44, Andrew Cooper wrote: > On 30/11/2021 16:10, Jan Beulich wrote: >> ept_free_entry() gets called after a flush - if one is necessary in the >> first place - was already issued. That behavior is similar to NPT, which >> also doesn't have any further flush in p2m_free_entry(). (Furthermore, >> the function being recursive, in case of recursiveness way too many >> flushes would have been issued.) >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > ept_free_entry() is called either recursively, or after an > ept_split_super_page(), and I agree that it does not want to queue > multiple flushes. > > However, I don't see where a suitable flush is in the superpage path. > Am I overlooking something? When it gets called after ept_split_super_page(), that's in case the function failed (and hence no update to live page tables was made). Jan
On 30.11.2021 17:10, Jan Beulich wrote: > ept_free_entry() gets called after a flush - if one is necessary in the > first place - was already issued. That behavior is similar to NPT, which > also doesn't have any further flush in p2m_free_entry(). (Furthermore, > the function being recursive, in case of recursiveness way too many > flushes would have been issued.) > > Signed-off-by: Jan Beulich <jbeulich@suse.com> May I ask for an ack or otherwise here? From all I can tell I have addressed Andrew's comment. Jan > --- a/xen/arch/x86/mm/p2m-ept.c > +++ b/xen/arch/x86/mm/p2m-ept.c > @@ -246,8 +246,7 @@ static void ept_free_entry(struct p2m_do > ept_free_entry(p2m, epte + i, level - 1); > unmap_domain_page(epte); > } > - > - p2m_tlb_flush_sync(p2m); > + > p2m_free_ptp(p2m, mfn_to_page(_mfn(ept_entry->mfn))); > } >
On Tue, Nov 30, 2021 at 05:10:53PM +0100, Jan Beulich wrote: > ept_free_entry() gets called after a flush - if one is necessary in the > first place - was already issued. That behavior is similar to NPT, which > also doesn't have any further flush in p2m_free_entry(). (Furthermore, > the function being recursive, in case of recursiveness way too many > flushes would have been issued.) > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> It's also worth noting that ept_free_entry doesn't modify the p2m itself - it gets handed an EPT entry to free, which should have been unhooked from the p2m previously by the caller, so it should be the responsibility of the caller to perform any flushes. Thanks, Roger.
> From: Jan Beulich <jbeulich@suse.com> > Sent: Wednesday, December 1, 2021 12:11 AM > > ept_free_entry() gets called after a flush - if one is necessary in the > first place - was already issued. That behavior is similar to NPT, which > also doesn't have any further flush in p2m_free_entry(). (Furthermore, > the function being recursive, in case of recursiveness way too many > flushes would have been issued.) > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> > > --- a/xen/arch/x86/mm/p2m-ept.c > +++ b/xen/arch/x86/mm/p2m-ept.c > @@ -246,8 +246,7 @@ static void ept_free_entry(struct p2m_do > ept_free_entry(p2m, epte + i, level - 1); > unmap_domain_page(epte); > } > - > - p2m_tlb_flush_sync(p2m); > + > p2m_free_ptp(p2m, mfn_to_page(_mfn(ept_entry->mfn))); > } >
--- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -246,8 +246,7 @@ static void ept_free_entry(struct p2m_do ept_free_entry(p2m, epte + i, level - 1); unmap_domain_page(epte); } - - p2m_tlb_flush_sync(p2m); + p2m_free_ptp(p2m, mfn_to_page(_mfn(ept_entry->mfn))); }
ept_free_entry() gets called after a flush - if one is necessary in the first place - was already issued. That behavior is similar to NPT, which also doesn't have any further flush in p2m_free_entry(). (Furthermore, the function being recursive, in case of recursiveness way too many flushes would have been issued.) Signed-off-by: Jan Beulich <jbeulich@suse.com>