Re: [PATCH 1/2] x86/xen: fix memory setup for PVH dom0

["Roger Pau Monné" <roger.pau@citrix.com>]

On 02/06/14 19:09, David Vrabel wrote:
> Since af06d66ee32b (x86: fix setup of PVH Dom0 memory map) in Xen, PVH
> dom0 need only use the memory memory provided by Xen which has already
> setup all the correct holes.
> 
> xen_memory_setup() then ends up being trivial for a PVH guest so
> introduce a new function (xen_auto_xlated_memory_setup()).
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>


Hello,

Thanks for the patch, as Mukesh pointed out, this is not working as-is.

> ---
>  arch/x86/xen/enlighten.c |    5 ++++-
>  arch/x86/xen/setup.c     |   23 +++++++++++++++++++++++
>  arch/x86/xen/xen-ops.h   |    1 +
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index c34bfc4..7dffa8c 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1536,7 +1536,10 @@ asmlinkage __visible void __init xen_start_kernel(void)
>  	if (!xen_pvh_domain())
>  		pv_cpu_ops = xen_cpu_ops;
>  
> -	x86_init.resources.memory_setup = xen_memory_setup;
> +	if (xen_feature(XENFEAT_auto_translated_physmap))
> +		x86_init.resources.memory_setup = xen_auto_xlated_memory_setup;
> +	else
> +		x86_init.resources.memory_setup = xen_memory_setup;
>  	x86_init.oem.arch_setup = xen_arch_setup;
>  	x86_init.oem.banner = xen_banner;
>  
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index 0982233..e7ef388 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -509,6 +509,29 @@ char * __init xen_memory_setup(void)
>  }
>  
>  /*
> + * Machine specific memory setup for auto-translated guests.
> + */
> +char * __init xen_auto_xlated_memory_setup(void)
> +{
> +	static struct e820entry map[E820MAX] __initdata;
> +
> +	struct xen_memory_map memmap;
> +	int i;
> +	int rc;
> +
> +	memmap.nr_entries = E820MAX;
> +	set_xen_guest_handle(memmap.buffer, map);
> +
> +	rc = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
> +	BUG_ON(rc);

Although not strictly necessary, I think it would be good to add the
following here (it's also done in bare metal Linux):

sanitize_e820_map(map, ARRAY_SIZE(map), &memmap.nr_entries);

> +
> +	for (i = 0; i < memmap.nr_entries; i++)
> +		e820_add_region(map[i].addr, map[i].size, map[i].type);

And I think here we need to reserve the start_info area:

memblock_reserve(__pa(xen_start_info->mfn_list),
	xen_start_info->pt_base - xen_start_info->mfn_list);

With those changes I've been able to boot fine, but I'm still testing.

> +	return "Xen";
> +}
> +
> +/*
>   * Set the bit indicating "nosegneg" library variants should be used.
>   * We only need to bother in pure 32-bit mode; compat 32-bit processes
>   * can have un-truncated segments, so wrapping around is allowed.
> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
> index 1cb6f4c..b371d18 100644
> --- a/arch/x86/xen/xen-ops.h
> +++ b/arch/x86/xen/xen-ops.h
> @@ -34,6 +34,7 @@ extern unsigned long xen_max_p2m_pfn;
>  void xen_set_pat(u64);
>  
>  char * __init xen_memory_setup(void);
> +char * xen_auto_xlated_memory_setup(void);
>  void __init xen_arch_setup(void);
>  void xen_enable_sysenter(void);
>  void xen_enable_syscall(void);
>