Re: [PATCH 11/16] x86: add a boot option to enable and disable the direct map

From: Julien Grall <julien@xen.org>
To: Hongyan Xia <hx242@xen.org>, xen-devel@lists.xenproject.org
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH 11/16] x86: add a boot option to enable and disable the direct map
Date: Fri, 1 May 2020 09:43:37 +0100	[thread overview]
Message-ID: <91d65dd4-ef38-9d42-c4ac-275831acdb61@xen.org> (raw)
In-Reply-To: <7360b59e8fd39796fee56430a437b20c948d08c2.1588278317.git.hongyxia@amazon.com>

Hi Hongyan,

On 30/04/2020 21:44, Hongyan Xia wrote:
> From: Hongyan Xia <hongyxia@amazon.com>
> 
> Also add a helper function to retrieve it. Change arch_mfn_in_direct_map
> to check this option before returning.
> 
> This is added as a boot command line option, not a Kconfig. We do not
> produce different builds for EC2 so this is not introduced as a
> compile-time configuration.
> 
> Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
> ---
>   docs/misc/xen-command-line.pandoc | 12 ++++++++++++
>   xen/arch/x86/mm.c                 |  3 +++
>   xen/arch/x86/setup.c              |  2 ++
>   xen/include/asm-arm/mm.h          |  5 +++++
>   xen/include/asm-x86/mm.h          | 17 ++++++++++++++++-
>   5 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> index ee12b0f53f..7027e3a15c 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -652,6 +652,18 @@ Specify the size of the console debug trace buffer. By specifying `cpu:`
>   additionally a trace buffer of the specified size is allocated per cpu.
>   The debug trace feature is only enabled in debugging builds of Xen.
>   
> +### directmap (x86)
> +> `= <boolean>`
> +
> +> Default: `true`
> +
> +Enable or disable the direct map region in Xen.
> +
> +By default, Xen creates the direct map region which maps physical memory
> +in that region. Setting this to no will remove the direct map, blocking
> +exploits that leak secrets via speculative memory access in the direct
> +map.
> +
>   ### dma_bits
>   > `= <integer>`
>   
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index b3530d2763..64da997764 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -162,6 +162,9 @@ l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
>   l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
>       l1_fixmap_x[L1_PAGETABLE_ENTRIES];
>   
> +bool __read_mostly opt_directmap = true;
> +boolean_param("directmap", opt_directmap);
> +
>   paddr_t __read_mostly mem_hotplug;
>   
>   /* Frame table size in pages. */
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index faca8c9758..60fc4038be 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1282,6 +1282,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>       if ( highmem_start )
>           xenheap_max_mfn(PFN_DOWN(highmem_start - 1));
>   
> +    printk("Booting with directmap %s\n", arch_has_directmap() ? "on" : "off");
> +
>       /*
>        * Walk every RAM region and map it in its entirety (on x86/64, at least)
>        * and notify it to the boot allocator.
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index 7df91280bc..e6fd934113 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -366,6 +366,11 @@ int arch_acquire_resource(struct domain *d, unsigned int type, unsigned int id,
>       return -EOPNOTSUPP;
>   }
>   
> +static inline bool arch_has_directmap(void)
> +{
> +    return true;

arm32 doesn't have a directmap, so this needs to be false for arm32 and 
true for arm64.

I would also like the implementation of the helper close to 
arch_mfn_in_directmap() in asm-arm/arm*/mm.h.

Cheers,

-- 
Julien Grall