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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.10.14 at 12:53, <roger.pau@xxxxxxxxxx> 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:

I agree with Andrew that you should audit all code paths for this
problem, and either fix others as well or make clear in the
description that no further similar issues were found.

> --- 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));

Apart from the indentation issue already pointed out, the if() also
violates our coding style (also again on all further instances).

>      paging_lock(d);

I further wonder whether introducing a helper doing the paired
locks/unlocks wouldn't be worthwhile. And of course I wonder
about performance implications - the P2M lock is already a pretty
contended one iirc.

Jan


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