Re: [Xen-devel] [PATCH RFC 1/2] xen/pvh: take the p2m lock when doing logdirty ops from HVM domains

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/10/14 11:53, Roger Pau Monne wrote:
> Due to locking order, the p2m lock must be taken before the paging lock, or
> else the following panic occurs when trying to use logdirty ops from a PVH
> Dom0:
>
> (XEN) mm locking order violation: 292 > 222
> (XEN) Xen BUG at mm-locks.h:140
> (XEN) ----[ Xen-4.5-unstable  x86_64  debug=y  Not tainted ]----
> (XEN) CPU:    1
> (XEN) RIP:    e008:[<ffff82d0801e9ea5>] get_page_from_gfn_p2m+0xb0/0x286
> (XEN) RFLAGS: 0000000000010282   CONTEXT: hypervisor
> (XEN) rax: 0000000000000000   rbx: ffff83019a1f7884   rcx: 0000000000000000
> (XEN) rdx: ffff83019a1f0000   rsi: 000000000000000a   rdi: ffff82d0802926c0
> (XEN) rbp: ffff83019a1f77f8   rsp: ffff83019a1f7798   r8:  ffff83019e830000
> (XEN) r9:  0000000000000003   r10: 00000000000000de   r11: 0000000000000003
> (XEN) r12: ffff83019a1f77c4   r13: ffff83019a138820   r14: ffff83019a1f7974
> (XEN) r15: 0000000000057431   cr0: 0000000080050033   cr4: 00000000000026f0
> (XEN) cr3: 000000019ea86000   cr2: 000000080205d000
> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: 0000   cs: e008
> [...]
> (XEN) Xen call trace:
> (XEN)    [<ffff82d0801e9ea5>] get_page_from_gfn_p2m+0xb0/0x286
> (XEN)    [<ffff82d0802223e9>] hap_p2m_ga_to_gfn_4_levels+0x59/0x2b7
> (XEN)    [<ffff82d080222663>] hap_gva_to_gfn_4_levels+0x1c/0x29
> (XEN)    [<ffff82d0801edf67>] paging_gva_to_gfn+0xb8/0xce
> (XEN)    [<ffff82d0801b9bc0>] clear_user_hvm+0xd7/0x324
> (XEN)    [<ffff82d0801e8776>] paging_log_dirty_op+0x358/0x552
> (XEN)    [<ffff82d0801e8d47>] paging_domctl+0x140/0x177
> (XEN)    [<ffff82d08015ccca>] arch_do_domctl+0x212/0x269e
> (XEN)    [<ffff82d08010487c>] do_domctl+0x195d/0x1cd1
> (XEN)    [<ffff82d0801bafba>] hvm_do_hypercall+0x1b8/0x31c
> (XEN)    [<ffff82d0801e0d3f>] vmx_vmexit_handler+0xf91/0x1a5f
> (XEN)    [<ffff82d0801e7a51>] vmx_asm_vmexit_handler+0x41/0xc0
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 1:
> (XEN) Xen BUG at mm-locks.h:140
> (XEN) ****************************************
> (XEN)
> (XEN) Reboot in five seconds...
>
> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>

Hmm.  I suspect there might be more of these issues scattered around
with the other DOMCTL/SYSCTL hypercalls, none of which have ever been
used from an HVM guest before.

> ---
>  xen/arch/x86/mm/paging.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
> index 6b788f7..5af6309 100644
> --- a/xen/arch/x86/mm/paging.c
> +++ b/xen/arch/x86/mm/paging.c
> @@ -412,6 +412,8 @@ static int paging_log_dirty_op(struct domain *d,
>  
>      if ( !resuming )
>          domain_pause(d);
> +     if (has_hvm_container_vcpu(current))
> +         p2m_lock(p2m_get_hostp2m(current->domain));

It appears as if there is some indentation issue here.

Also, can you pull current into a struct vcpu *curr on the stack?

~Andrew

>      paging_lock(d);
>  
>      if ( !d->arch.paging.preempt.dom )
> @@ -421,6 +423,8 @@ static int paging_log_dirty_op(struct domain *d,
>                d->arch.paging.preempt.op != sc->op )
>      {
>          paging_unlock(d);
> +        if (has_hvm_container_vcpu(current))
> +            p2m_unlock(p2m_get_hostp2m(current->domain));
>          ASSERT(!resuming);
>          domain_unpause(d);
>          return -EBUSY;
> @@ -533,6 +537,8 @@ static int paging_log_dirty_op(struct domain *d,
>      }
>  
>      paging_unlock(d);
> +    if (has_hvm_container_vcpu(current))
> +        p2m_unlock(p2m_get_hostp2m(current->domain));
>  
>      if ( rv )
>      {
> @@ -555,6 +561,8 @@ static int paging_log_dirty_op(struct domain *d,
>   out:
>      d->arch.paging.preempt.dom = NULL;
>      paging_unlock(d);
> +    if (has_hvm_container_vcpu(current))
> +        p2m_unlock(p2m_get_hostp2m(current->domain));
>      domain_unpause(d);
>  
>      if ( l1 )


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