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

[Julien Grall <julien.grall@arm.com>]

(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@codeaurora.org>
>>
>> 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@codeaurora.org>
>> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
>> ---
>> 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@lists.xen.org
http://lists.xen.org/xen-devel