Re: [Xen-devel] [PATCH] xen: arm: setup sane EL1 state while building domain 0.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-03-17 at 15:37 +0000, Julien Grall wrote:
> 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)

I think setup_state might be more indicative of the operation.

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

I know we do this on context switch now but I think it is unnecessary
and I plan to remove it.

> 
>     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.

Are there any other potential callers?

Ian.


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