[PATCH v2 0/4] Scrubbing updates

[PATCH v2 0/4] Scrubbing updates

[Boris Ostrovsky]

First patch fixes a long-standing bug where a low memory monitor is
not initialized if boottime scrubbing is turned off.

The other threee patches are performace and readability optimizations.

I will send the last patch from previous posting (the one that broke the
tree) later. I have a couple of variants and want to test both of them longer 

Boris Ostrovsky (4):
  mm: Initialize lowmem virq when boot-time scrubbing is disabled
  mm: Change boot_scrub_done definition
  mm: Don't poison a page if scrub_debug is off
  mm: Don't request scrubbing until dom0 is running

 xen/arch/arm/setup.c    |  3 +--
 xen/arch/x86/setup.c    |  3 +--
 xen/common/page_alloc.c | 42 +++++++++++++++++++++---------------------
 xen/include/xen/mm.h    |  2 +-
 4 files changed, 24 insertions(+), 26 deletions(-)

-- 
1.8.3.1


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

[PATCH v2 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled

[Boris Ostrovsky]

scrub_heap_pages() does early return if boot-time scrubbing is
disabled, neglecting to initialize lowmem VIRQ.

Because setup_low_mem_virq() doesn't logically belong in
scrub_heap_pages() we put them both into the newly added
heap_init_late().

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
CC: Julien Grall <julien.grall@arm.com>
---
Changes in v2:
* Dropped unnecessary opt_bootscrub test in scrub_heap_pages()
* Restored comment for setup_low_mem_virq().

 xen/arch/arm/setup.c    |  3 +--
 xen/arch/x86/setup.c    |  3 +--
 xen/common/page_alloc.c | 18 +++++++++++-------
 xen/include/xen/mm.h    |  2 +-
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 3b34855..92f173b 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -861,8 +861,7 @@ void __init start_xen(unsigned long boot_phys_offset,
     if ( construct_dom0(dom0) != 0)
             panic("Could not set up DOM0 guest OS");
 
-    /* Scrub RAM that is still free and so may go to an unprivileged domain. */
-    scrub_heap_pages();
+    heap_init_late();
 
     init_constructors();
 
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ec96287..bc466e8 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1662,8 +1662,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
         cr4_pv32_mask |= X86_CR4_SMAP;
     }
 
-    /* Scrub RAM that is still free and so may go to an unprivileged domain. */
-    scrub_heap_pages();
+    heap_init_late();
 
     init_trace_bufs();
 
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 9fa62d2..43f5a38 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1839,7 +1839,7 @@ static int __init find_non_smt(unsigned int node, cpumask_t *dest)
  * Scrub all unallocated pages in all heap zones. This function uses all
  * online cpu's to scrub the memory in parallel.
  */
-void __init scrub_heap_pages(void)
+static void __init scrub_heap_pages(void)
 {
     cpumask_t node_cpus, all_worker_cpus;
     unsigned int i, j;
@@ -1849,9 +1849,6 @@ void __init scrub_heap_pages(void)
     int last_distance, best_node;
     int cpus;
 
-    if ( !opt_bootscrub )
-        return;
-
     cpumask_clear(&all_worker_cpus);
     /* Scrub block size. */
     chunk_size = opt_bootscrub_chunk >> PAGE_SHIFT;
@@ -1970,12 +1967,19 @@ void __init scrub_heap_pages(void)
 #ifdef CONFIG_SCRUB_DEBUG
     boot_scrub_done = true;
 #endif
+}
 
-    /* Now that the heap is initialized, run checks and set bounds
-     * for the low mem virq algorithm. */
+void __init heap_init_late(void)
+{
+    /*
+     * Now that the heap is initialized set bounds
+     * for the low mem virq algorithm.
+     */
     setup_low_mem_virq();
-}
 
+    if ( opt_bootscrub )
+        scrub_heap_pages();
+}
 
 
 /*************************
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index ddc3fb3..c2f5a08 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -199,7 +199,7 @@ int offline_page(unsigned long mfn, int broken, uint32_t *status);
 int query_page_offline(unsigned long mfn, uint32_t *status);
 unsigned long total_free_pages(void);
 
-void scrub_heap_pages(void);
+void heap_init_late(void);
 
 int assign_pages(
     struct domain *d,
-- 
1.8.3.1


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

[PATCH v2 2/4] mm: Change boot_scrub_done definition

[Boris Ostrovsky]

Rename it to the more appropriate scrub_debug and define as a macro
for !CONFIG_SCRUB_DEBUG. This will allow us to get rid of some
ifdefs (here and in the subsequent patch).

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Suggested-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/page_alloc.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 43f5a38..2c7675b 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -171,7 +171,9 @@ static unsigned long __initdata opt_bootscrub_chunk = MB(128);
 size_param("bootscrub_chunk", opt_bootscrub_chunk);
 
 #ifdef CONFIG_SCRUB_DEBUG
-static bool __read_mostly boot_scrub_done;
+static bool __read_mostly scrub_debug;
+#else
+#define scrub_debug    false
 #endif
 
 /*
@@ -725,7 +727,7 @@ static void check_one_page(struct page_info *pg)
     const uint64_t *ptr;
     unsigned int i;
 
-    if ( !boot_scrub_done )
+    if ( !scrub_debug )
         return;
 
     ptr = map_domain_page(mfn);
@@ -1696,12 +1698,7 @@ static void init_heap_pages(
             nr_pages -= n;
         }
 
-#ifndef CONFIG_SCRUB_DEBUG
-        free_heap_pages(pg + i, 0, false);
-#else
-        free_heap_pages(pg + i, 0, boot_scrub_done);
-#endif
-	
+        free_heap_pages(pg + i, 0, scrub_debug);
     }
 }
 
@@ -1965,7 +1962,7 @@ static void __init scrub_heap_pages(void)
     printk("done.\n");
 
 #ifdef CONFIG_SCRUB_DEBUG
-    boot_scrub_done = true;
+    scrub_debug = true;
 #endif
 }
 
-- 
1.8.3.1


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

[PATCH v2 3/4] mm: Don't poison a page if scrub_debug is off

[Boris Ostrovsky]

If scrub_debug is off we don't check pages in check_one_page().
Thus there is no reason to ever poison them.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/page_alloc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 2c7675b..2b8bb95 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -714,6 +714,9 @@ static void poison_one_page(struct page_info *pg)
     mfn_t mfn = _mfn(page_to_mfn(pg));
     uint64_t *ptr;
 
+    if ( !scrub_debug )
+        return;
+
     ptr = map_domain_page(mfn);
     *ptr = ~SCRUB_PATTERN;
     unmap_domain_page(ptr);
-- 
1.8.3.1


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

[PATCH v2 4/4] mm: Don't request scrubbing until dom0 is running

[Boris Ostrovsky]

There is no need to scrub pages freed during dom0 construction since
once dom0 is ready the heap will be scrubbed by scrub_heap_pages() anyway,
setting scrub_debug at the end.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v2:
* Use '||' instead of '|'. Drop '!!'
* Clarified commit message.

 xen/common/page_alloc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 2b8bb95..dbad1e1 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2248,16 +2248,12 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
 
             spin_unlock_recursive(&d->page_alloc_lock);
 
-#ifndef CONFIG_SCRUB_DEBUG
             /*
              * Normally we expect a domain to clear pages before freeing them,
              * if it cares about the secrecy of their contents. However, after
              * a domain has died we assume responsibility for erasure.
              */
-            scrub = !!d->is_dying;
-#else
-            scrub = true;
-#endif
+            scrub = d->is_dying || scrub_debug;
         }
         else
         {
-- 
1.8.3.1


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

Re: [PATCH v2 4/4] mm: Don't request scrubbing until dom0 is running

[Wei Liu]

On Thu, Aug 31, 2017 at 09:16:14AM -0400, Boris Ostrovsky wrote:
> There is no need to scrub pages freed during dom0 construction since
> once dom0 is ready the heap will be scrubbed by scrub_heap_pages() anyway,
> setting scrub_debug at the end.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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