diff mbox

[v8,1/5] x86/ioreq server: Release the p2m lock after mmio is handled.

Message ID 1489834435-6166-2-git-send-email-yu.c.zhang@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yu Zhang March 18, 2017, 10:53 a.m. UTC 
Routine hvmemul_do_io() may need to peek the p2m type of a gfn to
select the ioreq server. For example, operations on gfns with
p2m_ioreq_server type will be delivered to a corresponding ioreq
server, and this requires that the p2m type not be switched back
to p2m_ram_rw during the emulation process. To avoid this race
condition, we delay the release of p2m lock in hvm_hap_nested_page_fault()
until mmio is handled.

Note: previously in hvm_hap_nested_page_fault(), put_gfn() was moved
before the handling of mmio, due to a deadlock risk between the p2m
lock and the event lock(in commit 77b8dfe). Later, a per-event channel
lock was introduced in commit de6acb7, to send events. So we do not
need to worry about the deadlock issue.

Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Paul Durrant <paul.durrant@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/hvm/hvm.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Jan Beulich March 20, 2017, 5:03 p.m. UTC | #1 
>>> On 18.03.17 at 11:53, <yu.c.zhang@linux.intel.com> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1870,18 +1870,14 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
>           (npfec.write_access &&
>            (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
>      {
> -        __put_gfn(p2m, gfn);
> -        if ( ap2m_active )
> -            __put_gfn(hostp2m, gfn);
> -
>          rc = 0;
>          if ( unlikely(is_pvh_domain(currd)) )
> -            goto out;
> +            goto out_put_gfn;

Did you forget to re-base onto staging before you've sent this?
is_pvh_domain() was gone before your submission already. When
re-basing, feel free to also drop the dead "rc = 0;" line.

Jan
Yu Zhang March 21, 2017, 1:18 a.m. UTC | #2 
On 3/21/2017 1:03 AM, Jan Beulich wrote:
>>>> On 18.03.17 at 11:53, <yu.c.zhang@linux.intel.com> wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -1870,18 +1870,14 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
>>            (npfec.write_access &&
>>             (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
>>       {
>> -        __put_gfn(p2m, gfn);
>> -        if ( ap2m_active )
>> -            __put_gfn(hostp2m, gfn);
>> -
>>           rc = 0;
>>           if ( unlikely(is_pvh_domain(currd)) )
>> -            goto out;
>> +            goto out_put_gfn;
> Did you forget to re-base onto staging before you've sent this?
> is_pvh_domain() was gone before your submission already. When
> re-basing, feel free to also drop the dead "rc = 0;" line.

Oops...
I did a rebase days ago, but did not send the patch directly. There were 
some XenGT test performed based on these patch.
Sorry, I should have a check.  Will resend the patch. :)

B.R.
Yu
> Jan
>
>
Yu Zhang March 21, 2017, 2:43 a.m. UTC | #3 
On 3/21/2017 9:18 AM, Yu Zhang wrote:
>
>
> On 3/21/2017 1:03 AM, Jan Beulich wrote:
>>>>> On 18.03.17 at 11:53, <yu.c.zhang@linux.intel.com> wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -1870,18 +1870,14 @@ int hvm_hap_nested_page_fault(paddr_t gpa, 
>>> unsigned long gla,
>>>            (npfec.write_access &&
>>>             (p2m_is_discard_write(p2mt) || (p2mt == 
>>> p2m_ioreq_server))) )
>>>       {
>>> -        __put_gfn(p2m, gfn);
>>> -        if ( ap2m_active )
>>> -            __put_gfn(hostp2m, gfn);
>>> -
>>>           rc = 0;
>>>           if ( unlikely(is_pvh_domain(currd)) )
>>> -            goto out;
>>> +            goto out_put_gfn;
>> Did you forget to re-base onto staging before you've sent this?
>> is_pvh_domain() was gone before your submission already. When
>> re-basing, feel free to also drop the dead "rc = 0;" line.
>
> Oops...
> I did a rebase days ago, but did not send the patch directly. There 
> were some XenGT test performed based on these patch.
> Sorry, I should have a check.  Will resend the patch. :)
>

BTW, since there's another change - "drop the rc = 0", I'd like to send 
the new patchset with a new version nubmer. :-)

Yu
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ccfae4f..a9db7f7 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1870,18 +1870,14 @@  int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
          (npfec.write_access &&
           (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
     {
-        __put_gfn(p2m, gfn);
-        if ( ap2m_active )
-            __put_gfn(hostp2m, gfn);
-
         rc = 0;
         if ( unlikely(is_pvh_domain(currd)) )
-            goto out;
+            goto out_put_gfn;
 
         if ( !handle_mmio_with_translation(gla, gpa >> PAGE_SHIFT, npfec) )
             hvm_inject_hw_exception(TRAP_gp_fault, 0);
         rc = 1;
-        goto out;
+        goto out_put_gfn;
     }
 
     /* Check if the page has been paged out */