* [PATCH] x86/EPT: squash meaningless TLB flush
@ 2021-11-30 16:10 Jan Beulich
2021-11-30 16:44 ` Andrew Cooper
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Jan Beulich @ 2021-11-30 16:10 UTC (permalink / raw)
To: xen-devel; +Cc: Jun Nakajima, Kevin Tian
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>
--- 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)));
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/EPT: squash meaningless TLB flush
2021-11-30 16:10 [PATCH] x86/EPT: squash meaningless TLB flush Jan Beulich
@ 2021-11-30 16:44 ` Andrew Cooper
2021-11-30 16:57 ` Jan Beulich
2021-12-10 9:32 ` Ping: " Jan Beulich
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2021-11-30 16:44 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Jun Nakajima, Kevin Tian
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/EPT: squash meaningless TLB flush
2021-11-30 16:44 ` Andrew Cooper
@ 2021-11-30 16:57 ` Jan Beulich
0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2021-11-30 16:57 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Jun Nakajima, Kevin Tian, xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Ping: [PATCH] x86/EPT: squash meaningless TLB flush
2021-11-30 16:10 [PATCH] x86/EPT: squash meaningless TLB flush Jan Beulich
2021-11-30 16:44 ` Andrew Cooper
@ 2021-12-10 9:32 ` Jan Beulich
2021-12-10 10:30 ` Roger Pau Monné
2021-12-24 7:45 ` Tian, Kevin
3 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2021-12-10 9:32 UTC (permalink / raw)
To: Kevin Tian, Jun Nakajima; +Cc: xen-devel
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)));
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/EPT: squash meaningless TLB flush
2021-11-30 16:10 [PATCH] x86/EPT: squash meaningless TLB flush Jan Beulich
2021-11-30 16:44 ` Andrew Cooper
2021-12-10 9:32 ` Ping: " Jan Beulich
@ 2021-12-10 10:30 ` Roger Pau Monné
2021-12-24 7:45 ` Tian, Kevin
3 siblings, 0 replies; 6+ messages in thread
From: Roger Pau Monné @ 2021-12-10 10:30 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Jun Nakajima, Kevin Tian
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] x86/EPT: squash meaningless TLB flush
2021-11-30 16:10 [PATCH] x86/EPT: squash meaningless TLB flush Jan Beulich
` (2 preceding siblings ...)
2021-12-10 10:30 ` Roger Pau Monné
@ 2021-12-24 7:45 ` Tian, Kevin
3 siblings, 0 replies; 6+ messages in thread
From: Tian, Kevin @ 2021-12-24 7:45 UTC (permalink / raw)
To: Beulich, Jan, xen-devel; +Cc: Nakajima, Jun
> 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)));
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-24 7:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30 16:10 [PATCH] x86/EPT: squash meaningless TLB flush Jan Beulich
2021-11-30 16:44 ` Andrew Cooper
2021-11-30 16:57 ` Jan Beulich
2021-12-10 9:32 ` Ping: " Jan Beulich
2021-12-10 10:30 ` Roger Pau Monné
2021-12-24 7:45 ` Tian, Kevin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.