Re: [PATCH 1 of 6] The PoD code may split a 1GB superpage in a potentially unlocked way

[George Dunlap <George.Dunlap@eu.citrix.com>]

(Including xen-devel in my reply...)

On Mon, Nov 14, 2011 at 9:48 PM, Andres Lagar-Cavilla
<andres@lagarcavilla.org> wrote:
>  xen/arch/x86/mm/p2m-pod.c |  1 -
>  xen/arch/x86/mm/p2m-pt.c  |  9 ++++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
>
> The path p2m-lookup -> p2m-pt->get_entry -> 1GB PoD superpage ->
> pod_demand_populate ends in the pod code performing a p2m_set_entry with
> no locks held (in order to split the 1GB superpage into 512 2MB ones)
>
> Further, it calls p2m_unlock after that, which will break the spinlock.

Yeah, not sure how this got to be the way it did... good catch.

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

>
> This patch attempts to fix that.
>
> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
>
> diff -r 0844b17df7a9 -r d9a344a5c1e3 xen/arch/x86/mm/p2m-pod.c
> --- a/xen/arch/x86/mm/p2m-pod.c
> +++ b/xen/arch/x86/mm/p2m-pod.c
> @@ -987,7 +987,6 @@ p2m_pod_demand_populate(struct p2m_domai
>         set_p2m_entry(p2m, gfn_aligned, _mfn(0), PAGE_ORDER_2M,
>                       p2m_populate_on_demand, p2m->default_access);
>         audit_p2m(p2m, 1);
> -        p2m_unlock(p2m);
>         return 0;
>     }
>
> diff -r 0844b17df7a9 -r d9a344a5c1e3 xen/arch/x86/mm/p2m-pt.c
> --- a/xen/arch/x86/mm/p2m-pt.c
> +++ b/xen/arch/x86/mm/p2m-pt.c
> @@ -542,10 +542,11 @@ pod_retry_l3:
>             /* The read has succeeded, so we know that mapping exists */
>             if ( q != p2m_query )
>             {
> -                if ( !p2m_pod_demand_populate(p2m, gfn, PAGE_ORDER_1G, q) )
> +                if ( !p2m_pod_check_and_populate(p2m, gfn,
> +                                      (l1_pgentry_t *) &l3e, PAGE_ORDER_1G, q) )
>                     goto pod_retry_l3;
>                 p2mt = p2m_invalid;
> -                printk("%s: Allocate 1GB failed!\n", __func__);
> +                gdprintk(XENLOG_ERR, "%s: Allocate 1GB failed!\n", __func__);
>                 goto out;
>             }
>             else
> @@ -743,8 +744,10 @@ pod_retry_l3:
>             {
>                 if ( q != p2m_query )
>                 {
> -                    if ( !p2m_pod_demand_populate(p2m, gfn, PAGE_ORDER_1G, q) )
> +                    if ( !p2m_pod_check_and_populate(p2m, gfn,
> +                                      (l1_pgentry_t *) l3e, PAGE_ORDER_1G, q) )
>                         goto pod_retry_l3;
> +                    gdprintk(XENLOG_ERR, "%s: Allocate 1GB failed!\n", __func__);
>                 }
>                 else
>                     *t = p2m_populate_on_demand;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>