From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@linaro.org>
Subject: Re: [PATCH] xen: arm: setup sane EL1 state while
 building domain 0.
Date: Mon, 17 Mar 2014 15:37:20 +0000
Message-ID: <532716B0.6030508@linaro.org>
References: <1395070269-32356-1-git-send-email-ian.campbell@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1395070269-32356-1-git-send-email-ian.campbell@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Campbell <ian.campbell@citrix.com>
Cc: stefano.stabellini@eu.citrix.com, tim@xen.org, Fu Wei <fu.wei@linaro.org>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

Hi Ian,

On 03/17/2014 03:31 PM, Ian Campbell wrote:
> The address translation functions used while building dom0 rely on certain EL1
> state being configured. In particular they are subject to the behaviour of
> SCTLR_EL1.M (stage 1 MMU enabled).
> 
> The Xen (and Linux) boot protocol require that the kernel be entered with the
> MMU disabled but they don't say anything explicitly about exception levels
> other than the one which is active when entering the kernels. Arguably the
> protocol could be said to apply to all exception levels but in any case we
> should cope with this and setup the EL1 state as necessary.
> 
> Fu Wei discovered this when booting Xen from grub.efi over UEFI, it's not
> clear whether grub or UEFI is responsible for leaving stage 1 MMU enabled.

I was about to send a similar patch :).

>      /* The following loads use the domain's p2m */
>      p2m_load_VTTBR(d);
> +    /* Various EL2 operations, such as guest address translations used
> +     * part of the domain build, rely on EL1 state (i.e. whether the
> +     * guest has paging enabled). Since the bootloader may have left
> +     * this state in an arbitrary configuration set it to something
> +     * safe here.
> +     */
> +    WRITE_SYSREG32(SCTLR_GUEST_INIT, SCTLR_EL1);

I think it would make more sense to create a new function call
p2m_restore_state which contains:

void p2m_restore_state(struct vcpu *n)
{
    register_t hcr;

    hcr = READ_SYSREG(HCR_EL2);
    WRITE_SYSREG(hcr & ~HCR_VM, HCR_EL2);
    isb();

    p2m_load_VTTBR(n->domain);
    isb();

    if ( is_pv32_domain(n->domain) )
        hcr &= ~HCR_RW;
    else
        hcr |= HCR_RW;

    WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
    isb();

    WRITE_SYSREG(hcr, HCR_EL2);
    isb();
}

IHMO, it's more clear than continuing "hardcoding" setup in dom0 code.

Regards,

-- 
Julien Grall