diff mbox

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

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

Commit Message

Yu Zhang March 21, 2017, 2:52 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>

changes in v4: 
  - According to comments from Jan: remove the redundant "rc = 0" code.
---
 xen/arch/x86/hvm/hvm.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Comments

George Dunlap March 29, 2017, 1:39 p.m. UTC | #1 
On Tue, Mar 21, 2017 at 2:52 AM, Yu Zhang <yu.c.zhang@linux.intel.com> wrote:
> 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>

Who else's ack does this need?  It seems like this is a general
improvement that can go in without the rest of the series.

 -George
Jan Beulich March 29, 2017, 1:50 p.m. UTC | #2 
>>> On 29.03.17 at 15:39, <dunlapg@umich.edu> wrote:
> On Tue, Mar 21, 2017 at 2:52 AM, Yu Zhang <yu.c.zhang@linux.intel.com> wrote:
>> 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>
> 
> Who else's ack does this need?  It seems like this is a general
> improvement that can go in without the rest of the series.

I didn't put it in on its own because it didn't really seem useful to
me without the rest of the series.

Jan
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0282986..bd18d8e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1815,15 +1815,10 @@  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 ( !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 */