Subject: Re: [PATCH v2] xen/balloon: Mark unallocated host memory as UNUSABLE
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>, xen-devel@lists.xen.org,
        linux-kernel@vger.kernel.org
Cc: helgaas@kernel.org, christian.koenig@amd.com
References: <1513635771-11779-1-git-send-email-boris.ostrovsky@oracle.com>
From: Juergen Gross <jgross@suse.com>
Message-ID: <05afe0fb-f33f-6b6f-d47e-617e9dc9b9a6@suse.com>
Date: Tue, 19 Dec 2017 09:22:13 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.0
MIME-Version: 1.0
In-Reply-To: <1513635771-11779-1-git-send-email-boris.ostrovsky@oracle.com>
Content-Type: text/plain; charset=utf-8
Content-Language: de-DE
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2481
Lines: 71

On 18/12/17 23:22, Boris Ostrovsky wrote:
> Commit f5775e0b6116 ("x86/xen: discard RAM regions above the maximum
> reservation") left host memory not assigned to dom0 as available for
> memory hotplug.
> 
> Unfortunately this also meant that those regions could be used by
> others. Specifically, commit fa564ad96366 ("x86/PCI: Enable a 64bit BAR
> on AMD Family 15h (Models 00-1f, 30-3f, 60-7f)") may try to map those
> addresses as MMIO.
> 
> To prevent this mark unallocated host memory as E820_TYPE_UNUSABLE (thus
> effectively reverting f5775e0b6116) and keep track of that region as
> a hostmem resource that can be used for the hotplug.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> Changes in v2:
> 
> In enlighten.c:
> - Fix 32-bit build problem (include bootmem.h), make variables 32-bit safe
> - Add a test to avoid inserting a resource into hostmem which is beyond
>   hostmem's end
> - Replace 'while' loop with 'for' to simplify array boundary check.
> - Drop out of memory warnings
> - Add a comment clarifying use of hostmem_resource.
> 
>  arch/x86/xen/enlighten.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++
>  arch/x86/xen/setup.c     |  6 ++--
>  drivers/xen/balloon.c    | 65 ++++++++++++++++++++++++++++++++++------
>  3 files changed, 135 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index d669e9d..ac96142 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1,8 +1,12 @@
> +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> +#include <linux/bootmem.h>
> +#endif
>  #include <linux/cpu.h>
>  #include <linux/kexec.h>
>  
>  #include <xen/features.h>
>  #include <xen/page.h>
> +#include <xen/interface/memory.h>
>  
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
> @@ -331,3 +335,76 @@ void xen_arch_unregister_cpu(int num)
>  }
>  EXPORT_SYMBOL(xen_arch_unregister_cpu);
>  #endif
> +
> +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> +void __init arch_xen_balloon_init(struct resource *hostmem_resource)

Sorry for noticing only now, but shouldn't you add a prototype in some
header for this function?

> +{
> +	struct xen_memory_map memmap;
> +	int rc;
> +	unsigned int i, last_guest_ram;
> +	phys_addr_t max_addr = max_pfn << PAGE_SHIFT;

Using PFN_PHYS() would have avoided the still present overflow on 32-bit
kernel: the shift is done with the 32-bit quantity and only then the
result is promoted to 64-bit.


Juergen