Re: [Xen-devel] [PATCH v2] arm: Fix asynchronous aborts (SError exceptions) due to bogus PTEs

[Julien Grall <julien.grall@xxxxxxx>]

(CC some ARM folks)

On 21/03/2016 23:18, Shanker Donthineni wrote:
> Hi Julien,
Hello Shanker,

Sorry for the late answer.

> Do you have any other comments to be addressed?
I have a question regarding the implication for what you wrote in the 
commit.
As far as I understand, any speculative table walk might cause an 
imprecise asynchronous abort. So if a guest is using page tables that 
contain garbage, it would be possible to receive an SError. Am I right?
> On 03/16/2016 02:08 PM, Shanker Donthineni wrote:
> > From: Vikram Sethi <vikrams@xxxxxxxxxxxxxx>
> >
> > ARMv8 architecture allows performing prefetch data/instructions
> > from memory locations marked as normal memory. Prefetch does not
> > mean that the data/instruction has to be used/executed in code
> > flow. All PTEs that appear to be valid to MMU must contain valid
> > physical address with proper attributes otherwise MMU table walk
> > might cause imprecise asynchronous aborts.
> >
> > The way current XEN code is preparing page tables for frametable
> > and xenheap memory can create bogus PTEs. This patch fixes the
> > issue by clearing page table memory before populating EL2 L0/L1
> > PTEs. Without this patch XEN crashes on Qualcomm Technologies
> > server chips due to asynchronous aborts.
> >
> > The speculative/prefetch feature explanation is scattered everywhere
> > in ARM specification but below two sections have useful information.
> >
> > E2.8 Memory types and attributes
> > G4.12.6 External abort on a translation table walk
As said on an earlier version of this patch, please mention the version 
of the spec when you quote it.
> > Signed-off-by: Vikram Sethi <vikrams@xxxxxxxxxxxxxx>
> > Signed-off-by: Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>
> > ---
> > Changes since v1:
> >      Replace memset() with clear_page()
> >      Edit commit description
> >
> >   xen/arch/arm/mm.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index 81f9e2e..3fda8f3 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -730,6 +730,8 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
> >           else
> >           {
> >               unsigned long first_mfn = alloc_boot_pages(1, 1);
> > +
> > +            clear_page(mfn_to_virt(first_mfn));
> >               pte = mfn_to_xen_entry(first_mfn, WRITEALLOC);
> >               pte.pt.table = 1;
> >               write_pte(p, pte);
> > @@ -773,6 +775,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t 
> > pe)
> >       second = mfn_to_virt(second_base);
> >       for ( i = 0; i < nr_second; i++ )
> >       {
> > +        clear_page(mfn_to_virt(second_base + i));
> >           pte = mfn_to_xen_entry(second_base + i, WRITEALLOC);
> >           pte.pt.table = 1;
> >           write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i], 
> > pte);
Regards,

--
Julien Grall

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