[PATCH] xen: don't flush the TLB on grant unmap for auto-translated guests

[PATCH] xen: don't flush the TLB on grant unmap for auto-translated guests

[Roger Pau Monne]

For auto-translated guests the p2m code will do the necessary TLB
flushes, so there's no need to perform any TLB flushes in generic
grant table code.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 xen/common/grant_table.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 107b000..a2a857f 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1099,7 +1099,8 @@ gnttab_unmap_grant_ref(
             guest_handle_add_offset(uop, 1);
         }
 
-        flush_tlb_mask(current->domain->domain_dirty_cpumask);
+        if ( !paging_mode_external(current->domain) )
+            flush_tlb_mask(current->domain->domain_dirty_cpumask);
 
         for ( i = 0; i < partial_done; i++ )
             __gnttab_unmap_common_complete(&(common[i]));
@@ -1114,7 +1115,8 @@ gnttab_unmap_grant_ref(
     return 0;
 
 fault:
-    flush_tlb_mask(current->domain->domain_dirty_cpumask);
+    if ( !paging_mode_external(current->domain) )
+        flush_tlb_mask(current->domain->domain_dirty_cpumask);
 
     for ( i = 0; i < partial_done; i++ )
         __gnttab_unmap_common_complete(&(common[i]));
@@ -1162,7 +1164,8 @@ gnttab_unmap_and_replace(
             guest_handle_add_offset(uop, 1);
         }
         
-        flush_tlb_mask(current->domain->domain_dirty_cpumask);
+        if ( !paging_mode_external(current->domain) )
+            flush_tlb_mask(current->domain->domain_dirty_cpumask);
         
         for ( i = 0; i < partial_done; i++ )
             __gnttab_unmap_common_complete(&(common[i]));
@@ -1177,7 +1180,8 @@ gnttab_unmap_and_replace(
     return 0;
 
 fault:
-    flush_tlb_mask(current->domain->domain_dirty_cpumask);
+    if ( !paging_mode_external(current->domain) )
+        flush_tlb_mask(current->domain->domain_dirty_cpumask);
 
     for ( i = 0; i < partial_done; i++ )
         __gnttab_unmap_common_complete(&(common[i]));
-- 
1.7.7.5 (Apple Git-26)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xen: don't flush the TLB on grant unmap for auto-translated guests

[David Vrabel]

On 06/05/14 17:37, Roger Pau Monne wrote:
> For auto-translated guests the p2m code will do the necessary TLB
> flushes, so there's no need to perform any TLB flushes in generic
> grant table code.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/common/grant_table.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 107b000..a2a857f 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1099,7 +1099,8 @@ gnttab_unmap_grant_ref(
>              guest_handle_add_offset(uop, 1);
>          }
>  
> -        flush_tlb_mask(current->domain->domain_dirty_cpumask);
> +        if ( !paging_mode_external(current->domain) )
> +            flush_tlb_mask(current->domain->domain_dirty_cpumask);

Introduce

void gnttab_flush_tlb(struct domain *d)
{
    if ( !paging_mode_external(d) )
        flush_tlb_mask(d->domain_dirty_cpumask);
}

?

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xen: don't flush the TLB on grant unmap for auto-translated guests

[Jan Beulich]

>>> On 06.05.14 at 19:01, <david.vrabel@citrix.com> wrote:
> On 06/05/14 17:37, Roger Pau Monne wrote:
>> For auto-translated guests the p2m code will do the necessary TLB
>> flushes, so there's no need to perform any TLB flushes in generic
>> grant table code.
>> 
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Tim Deegan <tim@xen.org>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> ---
>>  xen/common/grant_table.c |   12 ++++++++----
>>  1 files changed, 8 insertions(+), 4 deletions(-)
>> 
>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>> index 107b000..a2a857f 100644
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -1099,7 +1099,8 @@ gnttab_unmap_grant_ref(
>>              guest_handle_add_offset(uop, 1);
>>          }
>>  
>> -        flush_tlb_mask(current->domain->domain_dirty_cpumask);
>> +        if ( !paging_mode_external(current->domain) )
>> +            flush_tlb_mask(current->domain->domain_dirty_cpumask);
> 
> Introduce
> 
> void gnttab_flush_tlb(struct domain *d)
> {
>     if ( !paging_mode_external(d) )
>         flush_tlb_mask(d->domain_dirty_cpumask);
> }

Yes, please, but preferably as

static void gnttab_flush_tlb(const struct domain *d)

Also, while you might not be concerned about its performance,
I'd highly appreciate if the one remaining use (in gnttab_transfer())
would also be inspected and - if suitable - replaced (or else a note
added to the commit message clarifying why there the same can't
be done).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xen: don't flush the TLB on grant unmap for auto-translated guests

[Roger Pau Monné]

On 07/05/14 10:15, Jan Beulich wrote:
>>>> On 06.05.14 at 19:01, <david.vrabel@citrix.com> wrote:
>> On 06/05/14 17:37, Roger Pau Monne wrote:
>>> For auto-translated guests the p2m code will do the necessary TLB
>>> flushes, so there's no need to perform any TLB flushes in generic
>>> grant table code.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Cc: Tim Deegan <tim@xen.org>
>>> Cc: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>>  xen/common/grant_table.c |   12 ++++++++----
>>>  1 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>>> index 107b000..a2a857f 100644
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -1099,7 +1099,8 @@ gnttab_unmap_grant_ref(
>>>              guest_handle_add_offset(uop, 1);
>>>          }
>>>  
>>> -        flush_tlb_mask(current->domain->domain_dirty_cpumask);
>>> +        if ( !paging_mode_external(current->domain) )
>>> +            flush_tlb_mask(current->domain->domain_dirty_cpumask);
>>
>> Introduce
>>
>> void gnttab_flush_tlb(struct domain *d)
>> {
>>     if ( !paging_mode_external(d) )
>>         flush_tlb_mask(d->domain_dirty_cpumask);
>> }
> 
> Yes, please, but preferably as
> 
> static void gnttab_flush_tlb(const struct domain *d)

Any reason not to make it static inline void?

> 
> Also, while you might not be concerned about its performance,
> I'd highly appreciate if the one remaining use (in gnttab_transfer())
> would also be inspected and - if suitable - replaced (or else a note
> added to the commit message clarifying why there the same can't
> be done).

Yes, I see the other caller of flush_tlb_mask, and
guest_physmap_remove_page ends up calling p2m_set_entry, which is the
same as the other callers, so AFAICT, the flush is not necessary there
either.

Roger.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xen: don't flush the TLB on grant unmap for auto-translated guests

[Jan Beulich]

>>> On 07.05.14 at 12:00, <roger.pau@citrix.com> wrote:
> On 07/05/14 10:15, Jan Beulich wrote:
>>>>> On 06.05.14 at 19:01, <david.vrabel@citrix.com> wrote:
>>> On 06/05/14 17:37, Roger Pau Monne wrote:
>>>> --- a/xen/common/grant_table.c
>>>> +++ b/xen/common/grant_table.c
>>>> @@ -1099,7 +1099,8 @@ gnttab_unmap_grant_ref(
>>>>              guest_handle_add_offset(uop, 1);
>>>>          }
>>>>  
>>>> -        flush_tlb_mask(current->domain->domain_dirty_cpumask);
>>>> +        if ( !paging_mode_external(current->domain) )
>>>> +            flush_tlb_mask(current->domain->domain_dirty_cpumask);
>>>
>>> Introduce
>>>
>>> void gnttab_flush_tlb(struct domain *d)
>>> {
>>>     if ( !paging_mode_external(d) )
>>>         flush_tlb_mask(d->domain_dirty_cpumask);
>>> }
>> 
>> Yes, please, but preferably as
>> 
>> static void gnttab_flush_tlb(const struct domain *d)
> 
> Any reason not to make it static inline void?

I don't mind the inline, but also don't insist on it.

Jan