Re: [Xen-devel] [PATCH 17/17] xen/arm64: Zero BSS after the MMU and D-cache is turned on

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 26 Jun 2019, Julien Grall wrote:
> Hi Stefano,
> 
> On 6/26/19 8:29 PM, Stefano Stabellini wrote:
> > On Mon, 10 Jun 2019, Julien Grall wrote:
> > > At the moment BSS is zeroed before the MMU and D-Cache is turned on.
> > > In other words, the cache will be bypassed when zeroing the BSS section.
> > > 
> > > Per the Image protocol [1], the state of the cache for BSS region is not
> > > known because it is not part of the "loaded kernel image".
> > > 
> > > This means that the cache will need to be invalidated twice for the BSS
> > > region:
> > >      1) Before zeroing to remove any dirty cache line. Otherwise they may
> > >      get evicted while zeroing and therefore overriding the value.
> > >      2) After zeroing to remove any cache line that may have been
> > >      speculated. Otherwise when turning on MMU and D-Cache, the CPU may
> > >      see old values.
> > > 
> > > However, the only reason to have the BSS zeroed early is because the
> > > boot page tables are part of BSS. To avoid the two cache invalidations,
> > > it is possible to move the page tables in the section .data.page_aligned.
> > 
> > I am not following the last part. How is moving the boot page tables to
> > .data.page_aligned solving the problem? Do we need to zero
> > .data.page_aligned early or can we skip it because it is guaranteed to
> > already be zero?
> 
> Global variables are initialized to zero by default regardless the section
> they reside. Usually they are stored in BSS because it saves space in the
> binary.
> 
> With the Image protocol, BSS is not initialized by the bootloader so we have
> to do ourself.
> 
> The section .data.page_aligned is always part of the binary. So the compiler
> will write zero in the binary for any global variables part of that section
> and therefore the corresponding memory will be zeroed when loading the binary.
> 
> If it makes sense, I can try to incorporate that in the commit message.

So basically it is really only BSS the problem. All right, looks good to
me.

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> > > A new macro DEFINE_BOOT_PAGE_TABLE is introduced to create and mark
> > > page-tables used before BSS is zeroed. This includes all boot_* but also
> > > xen_fixmap as zero_bss() will print a message when earlyprintk is
> > > enabled.
> > 
> > On a similar note, and continuing from what I wrote above, do we need to
> > make sure to zero the xen_fixmap before hooking it up setup_fixmap?
> 
> See above.
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel