From mboxrd@z Thu Jan  1 00:00:00 1970
From: George Dunlap <George.Dunlap@eu.citrix.com>
Subject: Re: [PATCH 1 of 6] The PoD code may split a 1GB superpage
	in a potentially unlocked way
Date: Tue, 15 Nov 2011 15:39:59 +0000
Message-ID: <CAFLBxZYJuXAkdr=8eJM8eHEki5x+x2qz1vga4eQx-msD+VZXxw@mail.gmail.com>
References: <patchbomb.1321307321@xdev.gridcentric.ca>
	<d9a344a5c1e375befd24.1321307322@xdev.gridcentric.ca>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <d9a344a5c1e375befd24.1321307322@xdev.gridcentric.ca>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Cc: olaf@aepfle.de, xen-devel@lists.xensource.com, andres@gridcentric.ca, tim@xen.org, keir.xen@gmail.com, adin@gridcentric.ca
List-Id: xen-devel@lists.xenproject.org

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

On Mon, Nov 14, 2011 at 9:48 PM, Andres Lagar-Cavilla
<andres@lagarcavilla.org> wrote:
> =A0xen/arch/x86/mm/p2m-pod.c | =A01 -
> =A0xen/arch/x86/mm/p2m-pt.c =A0| =A09 ++++++---
> =A02 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
> =A0 =A0 =A0 =A0 set_p2m_entry(p2m, gfn_aligned, _mfn(0), PAGE_ORDER_2M,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 p2m_populate_on_demand, p2m->=
default_access);
> =A0 =A0 =A0 =A0 audit_p2m(p2m, 1);
> - =A0 =A0 =A0 =A0p2m_unlock(p2m);
> =A0 =A0 =A0 =A0 return 0;
> =A0 =A0 }
>
> 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:
> =A0 =A0 =A0 =A0 =A0 =A0 /* The read has succeeded, so we know that mappin=
g exists */
> =A0 =A0 =A0 =A0 =A0 =A0 if ( q !=3D p2m_query )
> =A0 =A0 =A0 =A0 =A0 =A0 {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if ( !p2m_pod_demand_populate(p2m, gfn, =
PAGE_ORDER_1G, q) )
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if ( !p2m_pod_check_and_populate(p2m, gf=
n,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0(l1_pgentry_t *) &l3e, PAGE_ORDER_1G, q) )
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto pod_retry_l3;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 p2mt =3D p2m_invalid;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printk("%s: Allocate 1GB failed!\n", __f=
unc__);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0gdprintk(XENLOG_ERR, "%s: Allocate 1GB f=
ailed!\n", __func__);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out;
> =A0 =A0 =A0 =A0 =A0 =A0 }
> =A0 =A0 =A0 =A0 =A0 =A0 else
> @@ -743,8 +744,10 @@ pod_retry_l3:
> =A0 =A0 =A0 =A0 =A0 =A0 {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ( q !=3D p2m_query )
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if ( !p2m_pod_demand_populate(p2=
m, gfn, PAGE_ORDER_1G, q) )
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if ( !p2m_pod_check_and_populate=
(p2m, gfn,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0(l1_pgentry_t *) l3e, PAGE_ORDER_1G, q) )
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto pod_retry_l3;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0gdprintk(XENLOG_ERR, "%s: Alloca=
te 1GB failed!\n", __func__);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *t =3D p2m_populate_on_demand;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>