Re: [Xen-devel] [PATCH V4 15/15] Add ARM EFI boot support

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-09-12 at 18:55 +0100, Stefano Stabellini wrote:
> On Fri, 12 Sep 2014, Roy Franz wrote:
> > On Fri, Sep 12, 2014 at 10:41 AM, Stefano Stabellini
> > <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > >>       > +        /* Turn off Dcache and MMU */
> > >>       > +        mrs   x0, sctlr_el2
> > >>       > +        bic   x0, x0, #1 << 0        /* clear SCTLR.M */
> > >>       > +        bic   x0, x0, #1 << 2        /* clear SCTLR.C */
> > >>
> > >> dsb?
> > >>
> > >> The dsb is done at the end of  __flush_dcache_all
> > >
> > > I think this would be to make sure __flush_dcache_all reads the correct
> > > arguments. We do the same when enabling dcache.
> > 
> > So where exactly are you suggesting the dsb?  Before the call to
> > __flush_dcache_all?
> 
> Sorry my comment was wrong, not had my coffee yet!
> I meant right before:
> 
> msr   sctlr_el2, x0
> 
> to make sure that ordering and arguments are correct. And yes, we do the
> same when enabling dcache see:
> 
>         mrs   x0, SCTLR_EL2
>         orr   x0, x0, #SCTLR_M       /* Enable MMU */
>         orr   x0, x0, #SCTLR_C       /* Enable D-cache */
>         dsb   sy                     /* Flush PTE writes and finish reads */
>         msr   SCTLR_EL2, x0          /* now paging is enabled */
>         isb                          /* Now, flush the icache */

FYI when I asked Will Deacon about this he was of the opinion that the
combination of an msr write to SCTLR_EL2 and an isb was a sufficient
barrier at least in the case I showed him (which was the Linux
equivalent of Roy's code.

The code you quote here is difference because it is enabling the MMU, so
it needs to ensure the previous PTE writes have already occurred, which
is what that barrier does.

Ian.



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