diff mbox

[v2] xen/arm: p2m: Don't create new table when the mapping is removed

Message ID 1387503680-2850-1-git-send-email-julien.grall@linaro.org
State Accepted
Commit 8672aa8384d18690df62b2cff2627590e93b0359
Headers show

Commit Message

Julien Grall Dec. 20, 2013, 1:41 a.m. UTC 
When  Xen is removing/relinquishing mapping, it will create second/third tables
if they don't exist.

Non-existent table means the address range was never mapped, so Xen can safely
skip them.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---
    Release: This is an improvement for Xen 4.4. It will save time during
    relinquish phase and avoid dummy allocation.
    The downside is the patch is modifying p2m loop which is used everywhere.

    Changes in v2:
        - Handle case where address is not {FIRST,SECOND}_SIZE aligned.
        We can't use ROUND macro because if the address is aligned, it doesn't
        add the size. So it will end up in an infinite loop.
---
 xen/arch/arm/p2m.c |   22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Comments

Ian Campbell Dec. 20, 2013, 9:15 a.m. UTC | #1 
On Fri, 2013-12-20 at 01:41 +0000, Julien Grall wrote:
> When  Xen is removing/relinquishing mapping, it will create second/third tables
> if they don't exist.
> 
> Non-existent table means the address range was never mapped, so Xen can safely
> skip them.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> ---
>     Release: This is an improvement for Xen 4.4. It will save time during
>     relinquish phase and avoid dummy allocation.

(Release: implies it should have been CCd to George)

I would go further than what you say though: This is removing a
potential guest induced long running operation and is therefore a clear
bug fix (and a potential security bug at that). I intend to apply this
(assuming it is OK, I haven't actually read this version yet).

Ian.
Ian Campbell Dec. 20, 2013, 9:57 a.m. UTC | #2 
On Fri, 2013-12-20 at 09:15 +0000, Ian Campbell wrote:
> On Fri, 2013-12-20 at 01:41 +0000, Julien Grall wrote:
> > When  Xen is removing/relinquishing mapping, it will create second/third tables
> > if they don't exist.
> > 
> > Non-existent table means the address range was never mapped, so Xen can safely
> > skip them.
> > 
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > ---
> >     Release: This is an improvement for Xen 4.4. It will save time during
> >     relinquish phase and avoid dummy allocation.
> 
> (Release: implies it should have been CCd to George)
> 
> I would go further than what you say though: This is removing a
> potential guest induced long running operation and is therefore a clear
> bug fix (and a potential security bug at that). I intend to apply this
> (assuming it is OK, I haven't actually read this version yet).

and on that basis now acked + applied.

Ian.
diff mbox

Patch

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d24a6fc..11f4714 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -246,10 +246,12 @@  static int create_p2m_entries(struct domain *d,
                   cur_first_offset = ~0,
                   cur_second_offset = ~0;
     unsigned long count = 0;
+    bool_t populate = (op == INSERT || op == ALLOCATE);
 
     spin_lock(&p2m->lock);
 
-    for(addr = start_gpaddr; addr < end_gpaddr; addr += PAGE_SIZE)
+    addr = start_gpaddr;
+    while ( addr < end_gpaddr )
     {
         if ( cur_first_page != p2m_first_level_index(addr) )
         {
@@ -265,8 +267,15 @@  static int create_p2m_entries(struct domain *d,
 
         if ( !first[first_table_offset(addr)].p2m.valid )
         {
+            if ( !populate )
+            {
+                addr = (addr + FIRST_SIZE) & FIRST_MASK;
+                continue;
+            }
+
             rc = p2m_create_table(d, &first[first_table_offset(addr)]);
-            if ( rc < 0 ) {
+            if ( rc < 0 )
+            {
                 printk("p2m_populate_ram: L1 failed\n");
                 goto out;
             }
@@ -284,6 +293,12 @@  static int create_p2m_entries(struct domain *d,
 
         if ( !second[second_table_offset(addr)].p2m.valid )
         {
+            if ( !populate )
+            {
+                addr = (addr + SECOND_SIZE) & SECOND_MASK;
+                continue;
+            }
+
             rc = p2m_create_table(d, &second[second_table_offset(addr)]);
             if ( rc < 0 ) {
                 printk("p2m_populate_ram: L2 failed\n");
@@ -372,6 +387,9 @@  static int create_p2m_entries(struct domain *d,
             }
             count = 0;
         }
+
+        /* Got the next page */
+        addr += PAGE_SIZE;
     }
 
     if ( op == ALLOCATE || op == INSERT )