[PATCH v2] x86/hap: Defer NPT P2M TLB flushes

[PATCH v2] x86/hap: Defer NPT P2M TLB flushes

[Ross Lagerwall]

Like the EPT code, defer TLB flushes for NPT to reduce the number of
flushes and avoid holding the P2M lock while flushing. This can
substantially improve performance in some scenarios.

The cases where the TLB needs to be flushed without deferring are
already handled by the call to p2m_tlb_flush_sync() in p2m_free_ptp().

Suggested-by: Roger Pau Monne <roger.pau@citrix.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
In v2:
* Tweak commit message.
* Call guest_flush_tlb_mask() if the assertion fails.

 xen/arch/x86/mm/hap/hap.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index a337752bf488..67137611d9db 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -814,15 +814,33 @@ static void cf_check hap_update_paging_modes(struct vcpu *v)
 static void cf_check
 hap_write_p2m_entry_post(struct p2m_domain *p2m, unsigned int oflags)
 {
-    struct domain *d = p2m->domain;
+    if ( !(oflags & _PAGE_PRESENT) )
+        return;
+
+    if ( unlikely(!p2m->defer_flush) )
+    {
+        struct domain *d = p2m->domain;
 
-    if ( oflags & _PAGE_PRESENT )
+        ASSERT_UNREACHABLE();
         guest_flush_tlb_mask(d, d->dirty_cpumask);
+        return;
+    }
+
+    p2m->need_flush = true;
+}
+
+static void cf_check
+hap_p2m_tlb_flush(struct p2m_domain *p2m)
+{
+    struct domain *d = p2m->domain;
+
+    guest_flush_tlb_mask(d, d->dirty_cpumask);
 }
 
 void hap_p2m_init(struct p2m_domain *p2m)
 {
     p2m->write_p2m_entry_post = hap_write_p2m_entry_post;
+    p2m->tlb_flush = hap_p2m_tlb_flush;
 }
 
 static unsigned long cf_check hap_gva_to_gfn_real_mode(
-- 
2.53.0

Re: [PATCH v2] x86/hap: Defer NPT P2M TLB flushes

[Roger Pau Monné]

On Thu, Mar 19, 2026 at 11:40:41AM +0000, Ross Lagerwall wrote:
> Like the EPT code, defer TLB flushes for NPT to reduce the number of
> flushes and avoid holding the P2M lock while flushing. This can
> substantially improve performance in some scenarios.
> 
> The cases where the TLB needs to be flushed without deferring are
> already handled by the call to p2m_tlb_flush_sync() in p2m_free_ptp().
> 
> Suggested-by: Roger Pau Monne <roger.pau@citrix.com>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
> In v2:
> * Tweak commit message.
> * Call guest_flush_tlb_mask() if the assertion fails.
> 
>  xen/arch/x86/mm/hap/hap.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> index a337752bf488..67137611d9db 100644
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -814,15 +814,33 @@ static void cf_check hap_update_paging_modes(struct vcpu *v)
>  static void cf_check
>  hap_write_p2m_entry_post(struct p2m_domain *p2m, unsigned int oflags)
>  {
> -    struct domain *d = p2m->domain;
> +    if ( !(oflags & _PAGE_PRESENT) )
> +        return;
> +
> +    if ( unlikely(!p2m->defer_flush) )
> +    {
> +        struct domain *d = p2m->domain;

As you are moving this around, and seeing that guest_flush_tlb_mask()
takes a const domain parameter, I think you could make this local
variable const.  Possibly the same below with the other d local
variable.

With that:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH v2] x86/hap: Defer NPT P2M TLB flushes

[Ross Lagerwall]

On 3/19/26 2:06 PM, Roger Pau Monné wrote:
> On Thu, Mar 19, 2026 at 11:40:41AM +0000, Ross Lagerwall wrote:
>> Like the EPT code, defer TLB flushes for NPT to reduce the number of
>> flushes and avoid holding the P2M lock while flushing. This can
>> substantially improve performance in some scenarios.
>>
>> The cases where the TLB needs to be flushed without deferring are
>> already handled by the call to p2m_tlb_flush_sync() in p2m_free_ptp().
>>
>> Suggested-by: Roger Pau Monne <roger.pau@citrix.com>
>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>> ---
>> In v2:
>> * Tweak commit message.
>> * Call guest_flush_tlb_mask() if the assertion fails.
>>
>>   xen/arch/x86/mm/hap/hap.c | 22 ++++++++++++++++++++--
>>   1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
>> index a337752bf488..67137611d9db 100644
>> --- a/xen/arch/x86/mm/hap/hap.c
>> +++ b/xen/arch/x86/mm/hap/hap.c
>> @@ -814,15 +814,33 @@ static void cf_check hap_update_paging_modes(struct vcpu *v)
>>   static void cf_check
>>   hap_write_p2m_entry_post(struct p2m_domain *p2m, unsigned int oflags)
>>   {
>> -    struct domain *d = p2m->domain;
>> +    if ( !(oflags & _PAGE_PRESENT) )
>> +        return;
>> +
>> +    if ( unlikely(!p2m->defer_flush) )
>> +    {
>> +        struct domain *d = p2m->domain;
> 
> As you are moving this around, and seeing that guest_flush_tlb_mask()
> takes a const domain parameter, I think you could make this local
> variable const.  Possibly the same below with the other d local
> variable.
> 

Yes, that makes sense. Can the adjustment be done on commit if there is
no other feedback?

Thanks,
Ross

Re: [PATCH v2] x86/hap: Defer NPT P2M TLB flushes

[Roger Pau Monné]

On Fri, Mar 20, 2026 at 09:44:39AM +0000, Ross Lagerwall wrote:
> On 3/19/26 2:06 PM, Roger Pau Monné wrote:
> > On Thu, Mar 19, 2026 at 11:40:41AM +0000, Ross Lagerwall wrote:
> > > Like the EPT code, defer TLB flushes for NPT to reduce the number of
> > > flushes and avoid holding the P2M lock while flushing. This can
> > > substantially improve performance in some scenarios.
> > > 
> > > The cases where the TLB needs to be flushed without deferring are
> > > already handled by the call to p2m_tlb_flush_sync() in p2m_free_ptp().
> > > 
> > > Suggested-by: Roger Pau Monne <roger.pau@citrix.com>
> > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> > > ---
> > > In v2:
> > > * Tweak commit message.
> > > * Call guest_flush_tlb_mask() if the assertion fails.
> > > 
> > >   xen/arch/x86/mm/hap/hap.c | 22 ++++++++++++++++++++--
> > >   1 file changed, 20 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> > > index a337752bf488..67137611d9db 100644
> > > --- a/xen/arch/x86/mm/hap/hap.c
> > > +++ b/xen/arch/x86/mm/hap/hap.c
> > > @@ -814,15 +814,33 @@ static void cf_check hap_update_paging_modes(struct vcpu *v)
> > >   static void cf_check
> > >   hap_write_p2m_entry_post(struct p2m_domain *p2m, unsigned int oflags)
> > >   {
> > > -    struct domain *d = p2m->domain;
> > > +    if ( !(oflags & _PAGE_PRESENT) )
> > > +        return;
> > > +
> > > +    if ( unlikely(!p2m->defer_flush) )
> > > +    {
> > > +        struct domain *d = p2m->domain;
> > 
> > As you are moving this around, and seeing that guest_flush_tlb_mask()
> > takes a const domain parameter, I think you could make this local
> > variable const.  Possibly the same below with the other d local
> > variable.
> > 
> 
> Yes, that makes sense. Can the adjustment be done on commit if there is
> no other feedback?

Sure, I can take care of that at commit time if there are no further
comments.

Thanks, Roger.