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

[Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>]

On 03/22/2016 05:21 PM, Julien Grall wrote:
> (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?
>

Yes, you are right (applies to EL1 TTBR0/TTBR1, EL2 TTBR0/TTBR1 and EL3 TTBR0 
tables).

>>
>> 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.
>

Sure, should I post V3 patch mentioning ARM spec version?
>>>
>>> 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,
>

-- 
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project


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