xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: osstest service user <osstest@xenbits.xen.org>,
	xen-devel@lists.xensource.com
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	ian.jackson@eu.citrix.com, Jan Beulich <JBeulich@suse.com>,
	Tim Deegan <tim@xen.org>
Subject: Re: [xen-unstable test] 56456: regressions - FAIL
Date: Sat, 16 May 2015 13:45:32 +0200	[thread overview]
Message-ID: <55572DDC.8050306@citrix.com> (raw)
In-Reply-To: <osstest-56456-mainreport@xen.org>

El 16/05/15 a les 10.51, osstest service user ha escrit:
> flight 56456 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/56456/
> 
> Regressions :-(

This is my fault, paging_gva_to_gfn cannot be used to translate a PV 
guest VA to a GFN. The patch above restores the previous path for PV 
callers.

---
commit 4cfaf46cbb116ce8dd0124bbbea319489db4b068
Author: Roger Pau Monne <roger.pau@citrix.com>
Date:   Sat May 16 13:42:39 2015 +0200

    x86: restore PV path on paging_log_dirty_op
    
    Restore previous path for PV domains calling paging_log_dirty_op. This fixes
    the fallout from a809eeea06d20b115d78f12e473502bcb6209844.
    
    Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index 5eee88c..2f51126 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -416,6 +416,7 @@ static inline void *map_dirty_bitmap(XEN_GUEST_HANDLE_64(uint8) dirty_bitmap,
     unsigned long gfn;
     p2m_type_t p2mt;
 
+    ASSERT(paging_mode_enabled(current->domain));
     gfn = paging_gva_to_gfn(current,
                             (unsigned long)(dirty_bitmap.p + (pages >> 3)),
                             &pfec);
@@ -446,6 +447,7 @@ static inline void *map_dirty_bitmap(XEN_GUEST_HANDLE_64(uint8) dirty_bitmap,
 
 static inline void unmap_dirty_bitmap(void *addr, struct page_info *page)
 {
+    ASSERT(paging_mode_enabled(current->domain));
     if ( addr != NULL )
     {
         unmap_domain_page(addr);
@@ -465,9 +467,9 @@ static int paging_log_dirty_op(struct domain *d,
     mfn_t *l4 = NULL, *l3 = NULL, *l2 = NULL;
     unsigned long *l1 = NULL;
     int i4, i3, i2;
-    uint8_t *dirty_bitmap;
-    struct page_info *page;
-    unsigned long index_mapped;
+    uint8_t *dirty_bitmap = NULL;
+    struct page_info *page = NULL;
+    unsigned long index_mapped = 0;
 
  again:
     if ( !resuming )
@@ -482,12 +484,15 @@ static int paging_log_dirty_op(struct domain *d,
         p2m_flush_hardware_cached_dirty(d);
     }
 
-    index_mapped = resuming ? d->arch.paging.preempt.log_dirty.done : 0;
-    dirty_bitmap = map_dirty_bitmap(sc->dirty_bitmap, index_mapped, &page);
-    if ( dirty_bitmap == NULL )
+    if ( paging_mode_enabled(current->domain) )
     {
-        domain_unpause(d);
-        return -EFAULT;
+        index_mapped = resuming ? d->arch.paging.preempt.log_dirty.done : 0;
+        dirty_bitmap = map_dirty_bitmap(sc->dirty_bitmap, index_mapped, &page);
+        if ( dirty_bitmap == NULL )
+        {
+            domain_unpause(d);
+            return -EFAULT;
+        }
     }
 
     paging_lock(d);
@@ -549,7 +554,8 @@ static int paging_log_dirty_op(struct domain *d,
                     bytes = (unsigned int)((sc->pages - pages + 7) >> 3);
                 if ( likely(peek) )
                 {
-                    if ( pages >> (3 + PAGE_SHIFT) !=
+                    if ( paging_mode_enabled(current->domain) &&
+                         pages >> (3 + PAGE_SHIFT) !=
                          index_mapped >> (3 + PAGE_SHIFT) )
                     {
                         /* We need to map next page */
@@ -564,13 +570,30 @@ static int paging_log_dirty_op(struct domain *d,
                         unmap_dirty_bitmap(dirty_bitmap, page);
                         goto again;
                     }
-                    ASSERT(((pages >> 3) % PAGE_SIZE) + bytes <= PAGE_SIZE);
-                    if ( l1 )
-                        memcpy(dirty_bitmap + ((pages >> 3) % PAGE_SIZE), l1,
-                               bytes);
+
+                    if ( paging_mode_enabled(current->domain) )
+                    {
+                        ASSERT(((pages >> 3) % PAGE_SIZE) + bytes <= PAGE_SIZE);
+                        if ( l1 )
+                            memcpy(dirty_bitmap + ((pages >> 3) % PAGE_SIZE),
+                                   l1, bytes);
+                        else
+                            memset(dirty_bitmap + ((pages >> 3) % PAGE_SIZE),
+                                   0, bytes);
+                    }
                     else
-                        memset(dirty_bitmap + ((pages >> 3) % PAGE_SIZE), 0,
-                               bytes);
+                    {
+                        if ( (l1 ? copy_to_guest_offset(sc->dirty_bitmap,
+                                                        pages >> 3,
+                                                        (uint8_t *)l1,
+                                                        bytes)
+                                 : clear_guest_offset(sc->dirty_bitmap,
+                                                      pages >> 3, bytes)) != 0 )
+                        {
+                            rv = -EFAULT;
+                            goto out;
+                        }
+                    }
                 }
                 pages += bytes << 3;
                 if ( l1 )
@@ -630,7 +653,8 @@ static int paging_log_dirty_op(struct domain *d,
     if ( rv )
     {
         /* Never leave the domain paused on real errors. */
-        unmap_dirty_bitmap(dirty_bitmap, page);
+        if ( paging_mode_enabled(current->domain) )
+            unmap_dirty_bitmap(dirty_bitmap, page);
         ASSERT(rv == -ERESTART);
         return rv;
     }
@@ -643,14 +667,16 @@ static int paging_log_dirty_op(struct domain *d,
          * paging modes (shadow or hap).  Safe because the domain is paused. */
         d->arch.paging.log_dirty.clean_dirty_bitmap(d);
     }
-    unmap_dirty_bitmap(dirty_bitmap, page);
+    if ( paging_mode_enabled(current->domain) )
+        unmap_dirty_bitmap(dirty_bitmap, page);
     domain_unpause(d);
     return rv;
 
  out:
     d->arch.paging.preempt.dom = NULL;
     paging_unlock(d);
-    unmap_dirty_bitmap(dirty_bitmap, page);
+    if ( paging_mode_enabled(current->domain) )
+        unmap_dirty_bitmap(dirty_bitmap, page);
     domain_unpause(d);
 
     if ( l1 )

  reply	other threads:[~2015-05-16 11:45 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-16  8:51 [xen-unstable test] 56456: regressions - FAIL osstest service user
2015-05-16 11:45 ` Roger Pau Monné [this message]
2015-05-18  8:34   ` Jan Beulich
2015-05-18 10:17     ` Tim Deegan
2015-05-18 10:36       ` Jan Beulich
2015-05-18 10:50       ` Roger Pau Monné
2015-05-18 11:00         ` Tim Deegan
2015-05-18 11:19         ` Jan Beulich
2015-05-19 10:20           ` Tim Deegan
2015-05-19 10:29             ` Jan Beulich
2015-05-19 15:07               ` Roger Pau Monné
2015-05-20  8:58                 ` Roger Pau Monné
2015-05-20  9:12                   ` Jan Beulich
2015-05-20  9:43                     ` Tim Deegan
2015-05-20  9:55                       ` Roger Pau Monné

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55572DDC.8050306@citrix.com \
    --to=roger.pau@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=osstest@xenbits.xen.org \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).