Re: [Xen-devel] [PATCH] xen: arm: zero EL2 pagetable pages before use

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 11/03/2016 20:24, Andrew Cooper wrote:
> On 11/03/16 13:13, Jan Beulich wrote:
> > > > > On 11.03.16 at 13:56, <andrew.cooper3@xxxxxxxxxx> wrote:
> > > On 11/03/16 11:29, Jan Beulich wrote:
> > > > > > > On 10.03.16 at 23:00, <shankerd@xxxxxxxxxxxxxx> wrote:
> > > > > @@ -771,6 +772,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t 
> > > > > pe)
> > > > >       nr_second = frametable_size >> SECOND_SHIFT;
> > > > >       second_base = alloc_boot_pages(nr_second, 1);
> > > > >       second = mfn_to_virt(second_base);
> > > > > +    memset(second, 0, nr_second * PAGE_SIZE);
> > > > >       for ( i = 0; i < nr_second; i++ )
> > > > >       {
> > > > >           pte = mfn_to_xen_entry(second_base + i, WRITEALLOC);
> > > > Along those lines here - use clear_page(), presumably by moving it
> > > > into the loop.
> > > This need only initialise the entries which are not filled by the loop,
> > > which will only be the rounding size up to the next 2M or 32M boundary.
> > >
> > > Most of the content of 'second' is explicitly initialised, so zeroing it
> > > all first is redundant.
> > Well, I certainly don't know all the details of how this works on
> > ARM, but the way I remember the original problem description
> > (sent a few days ago) the problem was with bogus translations
> > to be visible transiently. Of course all depends on whether the
> > page tables that are being modified here are live ones, which
> > I simply don't know.
> Looking at the code here, second is hooked into the live pagetables
> immediately after the loop.  Therefore, bogus translations will only be
> present for the untouched PTEs which make up the alignment space.
The frame table size is always aligned to 2MB/32MB. However, the frame 
table may not use all the entries in a level 2 page table (which cover 
1GB of memory). Those unused entries will be unknown if we don't clear them.
Keeping them unknown is not a problem as long as nobody is trying to 
access the underlying virtual address.
In the case of setup_frametable_mappings, Xen is still running with a 
single processor and the frame_table is not access until after 
create_mappings is called. The function should nuke all the TLBs at the 
end, so it looks like to me that zeroed the entries will hide the real 
problem.
Nonetheless, I would invalidate all the entries in the table to avoid 
polluting the TLBs with bogus entries and get a better crash.
Regards,

--
Julien Grall

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