[Xen-changelog] [xen master] xen/arm: p2m: Don't use default access permission when shattering a superpage

[patchbot@xxxxxxx]

commit 97323702bf101615cfdcf329913094c4cc82bf99
Author:     Julien Grall <julien.grall@xxxxxxx>
AuthorDate: Fri Jul 29 19:53:19 2016 +0100
Commit:     Stefano Stabellini <sstabellini@xxxxxxxxxx>
CommitDate: Thu Aug 4 11:25:05 2016 -0700

    xen/arm: p2m: Don't use default access permission when shattering a 
superpage
    
    The following message flood the console when memaccess is enabled on
    various platforms:
    
    traps.c:2510:d1v0 HSR=0x9383004f pc=0xffff000008b7d4c4 
gva=0xffff000008eeb8e0 gpa=0x0000004903f8e0
    
    This is because a data abort from a guest was received due to a
    permission fault but memaccess thought there are no permission fault.
    
    On ARM, memaccess permissions are stored in a radix tree because there
    are not enough available bits in the p2m entry to store the access
    restriction. When memaccess is restricting the access (i.e any other
    access than p2m_access_rwx), the access will be added in the radix tree
    using the GFN as a key. This will be done for all 4KB pages.
    
    This means that memaccess has to shatter all the superpages in a given
    region to set the permission on a 4KB granularity. Currently, when a
    superpage is shattered, the new entries are using the value
    p2m->default_access which will restrict permission (because memaccess
    has been enabled). However the radix tree does not yet contain
    an entry for this GFN.
    
    If a guest VCPU is running at the same time and trying to access the
    modified region, it will result to a stage-2 permission fault. As
    the radix tree does not yet contain an entry for the GFN, memaccess will
    deduce that the fault was not valid and a data abort will be injecting
    to the guest (and crash it).
    
    Furthermore, the permission may be restricted outside of the requested
    region if it is only a subset of a 1GB/2MB superpage.
    
    The two issues can be fixed by re-using the permission of the superpage
    entry and override the necessary fields. This is not a problem because
    memaccess cannot work on superpage.
    
    Lastly, document the code which call mfn_to_p2m_entry when creating a
    the p2m entry for a table to explain that create the p2m entry to page table
    to explain that permission are ignored by the hardware (See D4.3.1 in ARM 
DDI
    0487A.j). so the value of the parameter 'access' of mfn_to_p2m_entry does
    not matter.
    
    Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
    Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
---
 xen/arch/arm/p2m.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 40a0b80..d60fbbf 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -434,7 +434,6 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t 
*entry,
     p = __map_domain_page(page);
     if ( splitting )
     {
-        p2m_type_t t = entry->p2m.type;
         mfn_t mfn = _mfn(entry->p2m.base);
         int i;
 
@@ -444,15 +443,20 @@ static int p2m_create_table(struct p2m_domain *p2m, 
lpae_t *entry,
          */
          for ( i=0 ; i < LPAE_ENTRIES; i++ )
          {
-             pte = mfn_to_p2m_entry(mfn_add(mfn, i << (level_shift - 
LPAE_SHIFT)),
-                                    t, p2m->default_access);
+             /*
+              * Use the content of the superpage entry and override
+              * the necessary fields. So the correct permissions are
+              * kept.
+              */
+             pte = *entry;
+             pte.p2m.base = mfn_x(mfn_add(mfn,
+                                          i << (level_shift - LPAE_SHIFT)));
 
              /*
               * First and second level super pages set p2m.table = 0, but
               * third level entries set table = 1.
               */
-             if ( level_shift - LPAE_SHIFT )
-                 pte.p2m.table = 0;
+             pte.p2m.table = !(level_shift - LPAE_SHIFT);
 
              write_pte(&p[i], pte);
          }
@@ -467,6 +471,10 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t 
*entry,
 
     unmap_domain_page(p);
 
+    /*
+     * The access value does not matter because the hardware will ignore
+     * the permission fields for table entry.
+     */
     pte = mfn_to_p2m_entry(_mfn(page_to_mfn(page)), p2m_invalid,
                            p2m->default_access);
 
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog