The functions it guards are dead code. Worse, while intended to exist in
debug builds only, as of commit bacbf0cb7349 ("build: convert debug to
Kconfig") they also get compiled in release builds.

The remaining uses in show_stack_overflow() aren't really related to any
memory guarding anymore - with CET-SS support the stacks now get set up
the same in debug and release builds. Use CONFIG_DEBUG there instead.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -6109,36 +6109,6 @@ void free_perdomain_mappings(struct doma
     d->arch.perdomain_l3_pg = NULL;
 }
 
-#ifdef MEMORY_GUARD
-
-static void __memguard_change_range(void *p, unsigned long l, int guard)
-{
-    unsigned long _p = (unsigned long)p;
-    unsigned long _l = (unsigned long)l;
-    unsigned int flags = __PAGE_HYPERVISOR_RW | MAP_SMALL_PAGES;
-
-    /* Ensure we are dealing with a page-aligned whole number of pages. */
-    ASSERT(IS_ALIGNED(_p, PAGE_SIZE));
-    ASSERT(IS_ALIGNED(_l, PAGE_SIZE));
-
-    if ( guard )
-        flags &= ~_PAGE_PRESENT;
-
-    map_pages_to_xen(_p, virt_to_mfn(p), PFN_DOWN(_l), flags);
-}
-
-void memguard_guard_range(void *p, unsigned long l)
-{
-    __memguard_change_range(p, l, 1);
-}
-
-void memguard_unguard_range(void *p, unsigned long l)
-{
-    __memguard_change_range(p, l, 0);
-}
-
-#endif
-
 static void write_sss_token(unsigned long *ptr)
 {
     /*
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -642,7 +642,7 @@ void show_stack_overflow(unsigned int cp
 {
     unsigned long esp = regs->rsp;
     unsigned long curr_stack_base = esp & ~(STACK_SIZE - 1);
-#ifdef MEMORY_GUARD
+#ifdef CONFIG_DEBUG
     unsigned long esp_top, esp_bottom;
 #endif
 
@@ -650,7 +650,7 @@ void show_stack_overflow(unsigned int cp
         printk("Current stack base %p differs from expected %p\n",
                _p(curr_stack_base), stack_base[cpu]);
 
-#ifdef MEMORY_GUARD
+#ifdef CONFIG_DEBUG
     esp_bottom = (esp | (STACK_SIZE - 1)) + 1;
     esp_top    = esp_bottom - PRIMARY_STACK_SIZE;
 
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -57,10 +57,6 @@
 
 #define NR_hypercalls 64
 
-#ifndef NDEBUG
-#define MEMORY_GUARD
-#endif
-
 #define STACK_ORDER 3
 #define STACK_SIZE  (PAGE_SIZE << STACK_ORDER)
 
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -530,14 +530,6 @@ extern struct rangeset *mmio_ro_ranges;
 #define compat_pfn_to_cr3(pfn) (((unsigned)(pfn) << 12) | ((unsigned)(pfn) >> 20))
 #define compat_cr3_to_pfn(cr3) (((unsigned)(cr3) >> 12) | ((unsigned)(cr3) << 20))
 
-#ifdef MEMORY_GUARD
-void memguard_guard_range(void *p, unsigned long l);
-void memguard_unguard_range(void *p, unsigned long l);
-#else
-#define memguard_guard_range(_p,_l)    ((void)0)
-#define memguard_unguard_range(_p,_l)  ((void)0)
-#endif
-
 void memguard_guard_stack(void *p);
 void memguard_unguard_stack(void *p);
 

On 14/12/2021 15:13, Jan Beulich wrote:
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -642,7 +642,7 @@ void show_stack_overflow(unsigned int cp
>  {
>      unsigned long esp = regs->rsp;
>      unsigned long curr_stack_base = esp & ~(STACK_SIZE - 1);
> -#ifdef MEMORY_GUARD
> +#ifdef CONFIG_DEBUG
>      unsigned long esp_top, esp_bottom;
>  #endif
>  
> @@ -650,7 +650,7 @@ void show_stack_overflow(unsigned int cp
>          printk("Current stack base %p differs from expected %p\n",
>                 _p(curr_stack_base), stack_base[cpu]);
>  
> -#ifdef MEMORY_GUARD
> +#ifdef CONFIG_DEBUG

Looking at these, I think we'd be better dropping the ifdef and
compiling it in unconditionally.

This is only used in the #DF path, which is a fatal error path. 
Throwing away information we trivially have to hand is actively
unhelpful to whomever is analysing the logs.

Preferably with the ifdefary dropped, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>