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 <[email protected]>
---
I don't see /proc/meminfo reporting the hotplugged memory (although
internal data such as max_pfn is updated properly). Need to look at
hotplug code some more. But then I didn't see meminfo changing with
existing code either.
arch/x86/xen/enlighten.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++
arch/x86/xen/setup.c | 6 ++---
drivers/xen/balloon.c | 65 ++++++++++++++++++++++++++++++++++++++-------
3 files changed, 127 insertions(+), 13 deletions(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index d669e9d..19223b9 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -3,6 +3,7 @@
#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 +332,71 @@ 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)
+{
+ struct xen_memory_map memmap;
+ int rc, i, last_guest_ram;
+ unsigned long max_addr = max_pfn << PAGE_SHIFT;
+ struct e820_table *xen_e820_table;
+ struct e820_entry *entry;
+ struct resource *res = NULL;
+
+ if (!xen_initial_domain())
+ return;
+
+ xen_e820_table = kzalloc(sizeof(*xen_e820_table), GFP_KERNEL);
+ if (!xen_e820_table) {
+ pr_warn("%s: Out of memory\n", __func__);
+ return;
+ }
+
+ memmap.nr_entries = ARRAY_SIZE(xen_e820_table->entries);
+ set_xen_guest_handle(memmap.buffer, xen_e820_table->entries);
+ rc = HYPERVISOR_memory_op(XENMEM_machine_memory_map, &memmap);
+ if (rc) {
+ pr_warn("%s: Can't read host e820 (%d)\n", __func__, rc);
+ goto out;
+ }
+
+ last_guest_ram = i = 0;
+ while (xen_e820_table->entries[i].addr < max_addr) {
+ if (xen_e820_table->entries[i].type == E820_TYPE_RAM)
+ last_guest_ram = i;
+ i++;
+ }
+
+ entry = &xen_e820_table->entries[last_guest_ram];
+ if (max_addr >= entry->addr + entry->size)
+ goto out; /* No unallocated host RAM. */
+
+ hostmem_resource->start = max_addr;
+ hostmem_resource->end = entry->addr + entry->size;
+ for (; i < memmap.nr_entries; i++) {
+ entry = &xen_e820_table->entries[i];
+ if (entry->type == E820_TYPE_RAM)
+ continue;
+
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
+ if (!res) {
+ pr_warn("%s: Out of memory\n", __func__);
+ goto out;
+ }
+
+ res->name = "Host memory";
+ res->end = entry->addr + entry->size;
+ res->start = entry->addr;
+ rc = insert_resource(hostmem_resource, res);
+ if (rc) {
+ pr_warn("%s: Can't insert hostmem resource [%llu - %llu] (%d)\n",
+ __func__, res->start, res->end, rc);
+ kfree(res);
+ goto out;
+ }
+ }
+
+ out:
+ kfree(xen_e820_table);
+}
+#endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index c114ca7..6e0d208 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -808,7 +808,6 @@ char * __init xen_memory_setup(void)
addr = xen_e820_table.entries[0].addr;
size = xen_e820_table.entries[0].size;
while (i < xen_e820_table.nr_entries) {
- bool discard = false;
chunk_size = size;
type = xen_e820_table.entries[i].type;
@@ -824,11 +823,10 @@ char * __init xen_memory_setup(void)
xen_add_extra_mem(pfn_s, n_pfns);
xen_max_p2m_pfn = pfn_s + n_pfns;
} else
- discard = true;
+ type = E820_TYPE_UNUSABLE;
}
- if (!discard)
- xen_align_and_add_e820_region(addr, chunk_size, type);
+ xen_align_and_add_e820_region(addr, chunk_size, type);
addr += chunk_size;
size -= chunk_size;
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index f77e499..fb5aa7c 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -257,10 +257,25 @@ static void release_memory_resource(struct resource *resource)
kfree(resource);
}
+/*
+ * Host memory not allocated to dom0. We can use this range for hotplug-based
+ * ballooning.
+ *
+ * It's a type-less resource. Setting IORESOURCE_MEM will make resource
+ * management algorithms (arch_remove_reservations()) look into guest e820,
+ * which we don't want.
+ */
+static struct resource hostmem_resource = {
+ .name = "Host memory",
+};
+
+void __attribute__((weak)) __init arch_xen_balloon_init(struct resource *res)
+{}
+
static struct resource *additional_memory_resource(phys_addr_t size)
{
- struct resource *res;
- int ret;
+ struct resource *res, *res_hostmem;
+ int ret = -ENOMEM;
res = kzalloc(sizeof(*res), GFP_KERNEL);
if (!res)
@@ -269,13 +284,42 @@ static struct resource *additional_memory_resource(phys_addr_t size)
res->name = "System RAM";
res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
- ret = allocate_resource(&iomem_resource, res,
- size, 0, -1,
- PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
- if (ret < 0) {
- pr_err("Cannot allocate new System RAM resource\n");
- kfree(res);
- return NULL;
+ res_hostmem = kzalloc(sizeof(*res), GFP_KERNEL);
+ if (res_hostmem) {
+ /* Try to grab a range from hostmem */
+ res_hostmem->name = "Host memory";
+ ret = allocate_resource(&hostmem_resource, res_hostmem,
+ size, 0, -1,
+ PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
+ }
+
+ if (!ret) {
+ /*
+ * Insert this resource into iomem. Because hostmem_resource
+ * tracks portion of guest e820 marked as UNUSABLE noone else
+ * should try to use it.
+ */
+ res->start = res_hostmem->start;
+ res->end = res_hostmem->end;
+ ret = insert_resource(&iomem_resource, res);
+ if (ret < 0) {
+ pr_err("Can't insert iomem_resource [%llx - %llx]\n",
+ res->start, res->end);
+ release_memory_resource(res_hostmem);
+ res_hostmem = NULL;
+ res->start = res->end = 0;
+ }
+ }
+
+ if (ret) {
+ ret = allocate_resource(&iomem_resource, res,
+ size, 0, -1,
+ PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
+ if (ret < 0) {
+ pr_err("Cannot allocate new System RAM resource\n");
+ kfree(res);
+ return NULL;
+ }
}
#ifdef CONFIG_SPARSEMEM
@@ -287,6 +331,7 @@ static struct resource *additional_memory_resource(phys_addr_t size)
pr_err("New System RAM resource outside addressable RAM (%lu > %lu)\n",
pfn, limit);
release_memory_resource(res);
+ release_memory_resource(res_hostmem);
return NULL;
}
}
@@ -765,6 +810,8 @@ static int __init balloon_init(void)
set_online_page_callback(&xen_online_page);
register_memory_notifier(&xen_memory_nb);
register_sysctl_table(xen_root);
+
+ arch_xen_balloon_init(&hostmem_resource);
#endif
#ifdef CONFIG_XEN_PV
--
2.7.4
On 12/12/17 23:51, 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 <[email protected]>
> ---
> I don't see /proc/meminfo reporting the hotplugged memory (although
> internal data such as max_pfn is updated properly). Need to look at
> hotplug code some more. But then I didn't see meminfo changing with
> existing code either.
>
>
> arch/x86/xen/enlighten.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++
> arch/x86/xen/setup.c | 6 ++---
> drivers/xen/balloon.c | 65 ++++++++++++++++++++++++++++++++++++++-------
> 3 files changed, 127 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index d669e9d..19223b9 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -3,6 +3,7 @@
>
> #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 +332,71 @@ 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)
> +{
> + struct xen_memory_map memmap;
> + int rc, i, last_guest_ram;
i and last_guest_ram should be unsigned int
> + unsigned long max_addr = max_pfn << PAGE_SHIFT;
This might overflow for 32 bit builds
> + struct e820_table *xen_e820_table;
> + struct e820_entry *entry;
> + struct resource *res = NULL;
> +
> + if (!xen_initial_domain())
> + return;
> +
> + xen_e820_table = kzalloc(sizeof(*xen_e820_table), GFP_KERNEL);
> + if (!xen_e820_table) {
> + pr_warn("%s: Out of memory\n", __func__);
> + return;
> + }
> +
> + memmap.nr_entries = ARRAY_SIZE(xen_e820_table->entries);
> + set_xen_guest_handle(memmap.buffer, xen_e820_table->entries);
> + rc = HYPERVISOR_memory_op(XENMEM_machine_memory_map, &memmap);
> + if (rc) {
> + pr_warn("%s: Can't read host e820 (%d)\n", __func__, rc);
> + goto out;
> + }
> +
> + last_guest_ram = i = 0;
> + while (xen_e820_table->entries[i].addr < max_addr) {
> + if (xen_e820_table->entries[i].type == E820_TYPE_RAM)
> + last_guest_ram = i;
> + i++;
Check for i < memmap.nr_entries?
> + }
> +
> + entry = &xen_e820_table->entries[last_guest_ram];
> + if (max_addr >= entry->addr + entry->size)
> + goto out; /* No unallocated host RAM. */
> +
> + hostmem_resource->start = max_addr;
> + hostmem_resource->end = entry->addr + entry->size;
> + for (; i < memmap.nr_entries; i++) {
> + entry = &xen_e820_table->entries[i];
> + if (entry->type == E820_TYPE_RAM)
Shouldn't that be != ?
> + continue;
> +
> + res = kzalloc(sizeof(*res), GFP_KERNEL);
> + if (!res) {
> + pr_warn("%s: Out of memory\n", __func__);
Don't print error message in case of allocation failures.
Juergen
>> +
>> + hostmem_resource->start = max_addr;
>> + hostmem_resource->end = entry->addr + entry->size;
>> + for (; i < memmap.nr_entries; i++) {
>> + entry = &xen_e820_table->entries[i];
>> + if (entry->type == E820_TYPE_RAM)
> Shouldn't that be != ?
No, the idea here is to populate hostmem_resource with ranges already
taken by things other than RAM, leaving memory regions as available for
the balloon hotplug. This will allow us to use allocate_resource(),
which searches for a free range, in the balloon driver.
>
>> + continue;
>> +
>> + res = kzalloc(sizeof(*res), GFP_KERNEL);
>> + if (!res) {
>> + pr_warn("%s: Out of memory\n", __func__);
> Don't print error message in case of allocation failures.
Right, In fact checkpatch also suggested that I drop it. And I forgot.
-boris
On 15/12/17 15:24, Boris Ostrovsky wrote:
>
>>> +
>>> + hostmem_resource->start = max_addr;
>>> + hostmem_resource->end = entry->addr + entry->size;
>>> + for (; i < memmap.nr_entries; i++) {
>>> + entry = &xen_e820_table->entries[i];
>>> + if (entry->type == E820_TYPE_RAM)
>> Shouldn't that be != ?
>
> No, the idea here is to populate hostmem_resource with ranges already
> taken by things other than RAM, leaving memory regions as available for
> the balloon hotplug. This will allow us to use allocate_resource(),
> which searches for a free range, in the balloon driver.
But why says the comment "Host memory not allocated to dom0" then?
And why are you trying to allocate from this resource in case of
hotplugging memory (and fall back to iomem_resource in case of
failure)?
Either the comment is completely wrong and I don't understand the
logic here, or your code is wrong.
Juergen
On 12/15/2017 09:47 AM, Juergen Gross wrote:
> On 15/12/17 15:24, Boris Ostrovsky wrote:
>>>> +
>>>> + hostmem_resource->start = max_addr;
>>>> + hostmem_resource->end = entry->addr + entry->size;
>>>> + for (; i < memmap.nr_entries; i++) {
>>>> + entry = &xen_e820_table->entries[i];
>>>> + if (entry->type == E820_TYPE_RAM)
>>> Shouldn't that be != ?
>> No, the idea here is to populate hostmem_resource with ranges already
>> taken by things other than RAM, leaving memory regions as available for
>> the balloon hotplug. This will allow us to use allocate_resource(),
>> which searches for a free range, in the balloon driver.
> But why says the comment "Host memory not allocated to dom0" then?
hostmem_resource is created starting from the end of dom0 RAM and ending
at the end of host RAM. If these two are the same (or if the former is
larger, which I don't think is possible) then there is nothing to do, as
the hostmem_resource will be empty. That's what the comment is referring
to.
> And why are you trying to allocate from this resource in case of
> hotplugging memory (and fall back to iomem_resource in case of
> failure)?
Because that area (end of dom0 RAM through end of host RAM) is not going
to be used by anyone else and thus is available. That was the idea
behind f5775e0b6116.
-boris
>
> Either the comment is completely wrong and I don't understand the
> logic here, or your code is wrong.
On 15/12/17 15:58, Boris Ostrovsky wrote:
> On 12/15/2017 09:47 AM, Juergen Gross wrote:
>> On 15/12/17 15:24, Boris Ostrovsky wrote:
>>>>> +
>>>>> + hostmem_resource->start = max_addr;
>>>>> + hostmem_resource->end = entry->addr + entry->size;
>>>>> + for (; i < memmap.nr_entries; i++) {
>>>>> + entry = &xen_e820_table->entries[i];
>>>>> + if (entry->type == E820_TYPE_RAM)
>>>> Shouldn't that be != ?
>>> No, the idea here is to populate hostmem_resource with ranges already
>>> taken by things other than RAM, leaving memory regions as available for
>>> the balloon hotplug. This will allow us to use allocate_resource(),
>>> which searches for a free range, in the balloon driver.
>> But why says the comment "Host memory not allocated to dom0" then?
>
> hostmem_resource is created starting from the end of dom0 RAM and ending
> at the end of host RAM. If these two are the same (or if the former is
> larger, which I don't think is possible) then there is nothing to do, as
> the hostmem_resource will be empty. That's what the comment is referring
> to.
>
>> And why are you trying to allocate from this resource in case of
>> hotplugging memory (and fall back to iomem_resource in case of
>> failure)?
>
> Because that area (end of dom0 RAM through end of host RAM) is not going
> to be used by anyone else and thus is available. That was the idea
> behind f5775e0b6116.
Aah, now I've got it.
Could you please add a comment like:
/* Mark non-RAM regions as not available. */
above the test? That would have helped me. :-)
Juergen
Hi Boris,
I love your patch! Yet something to improve:
[auto build test ERROR on xen-tip/linux-next]
[also build test ERROR on v4.15-rc3 next-20171215]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Boris-Ostrovsky/xen-balloon-Mark-unallocated-host-memory-as-UNUSABLE/20171215-231511
base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
config: i386-randconfig-x010-201750 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
arch/x86/xen/enlighten.c: In function 'arch_xen_balloon_init':
>> arch/x86/xen/enlighten.c:341:27: error: 'max_pfn' undeclared (first use in this function); did you mean 'pgd_pfn'?
unsigned long max_addr = max_pfn << PAGE_SHIFT;
^~~~~~~
pgd_pfn
arch/x86/xen/enlighten.c:341:27: note: each undeclared identifier is reported only once for each function it appears in
vim +341 arch/x86/xen/enlighten.c
335
336 #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
337 void __init arch_xen_balloon_init(struct resource *hostmem_resource)
338 {
339 struct xen_memory_map memmap;
340 int rc, i, last_guest_ram;
> 341 unsigned long max_addr = max_pfn << PAGE_SHIFT;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 12/15/2017 10:33 AM, Juergen Gross wrote:
> On 15/12/17 15:58, Boris Ostrovsky wrote:
>> On 12/15/2017 09:47 AM, Juergen Gross wrote:
>>> On 15/12/17 15:24, Boris Ostrovsky wrote:
>>>>>> +
>>>>>> + hostmem_resource->start = max_addr;
>>>>>> + hostmem_resource->end = entry->addr + entry->size;
>>>>>> + for (; i < memmap.nr_entries; i++) {
>>>>>> + entry = &xen_e820_table->entries[i];
>>>>>> + if (entry->type == E820_TYPE_RAM)
>>>>> Shouldn't that be != ?
>>>> No, the idea here is to populate hostmem_resource with ranges already
>>>> taken by things other than RAM, leaving memory regions as available for
>>>> the balloon hotplug. This will allow us to use allocate_resource(),
>>>> which searches for a free range, in the balloon driver.
>>> But why says the comment "Host memory not allocated to dom0" then?
>> hostmem_resource is created starting from the end of dom0 RAM and ending
>> at the end of host RAM. If these two are the same (or if the former is
>> larger, which I don't think is possible) then there is nothing to do, as
>> the hostmem_resource will be empty. That's what the comment is referring
>> to.
>>
>>> And why are you trying to allocate from this resource in case of
>>> hotplugging memory (and fall back to iomem_resource in case of
>>> failure)?
>> Because that area (end of dom0 RAM through end of host RAM) is not going
>> to be used by anyone else and thus is available. That was the idea
>> behind f5775e0b6116.
> Aah, now I've got it.
>
> Could you please add a comment like:
>
> /* Mark non-RAM regions as not available. */
>
> above the test? That would have helped me. :-)
Will do, thanks.
-boris
On 12/15/2017 11:04 AM, kbuild test robot wrote:
> Hi Boris,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on xen-tip/linux-next]
> [also build test ERROR on v4.15-rc3 next-20171215]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Boris-Ostrovsky/xen-balloon-Mark-unallocated-host-memory-as-UNUSABLE/20171215-231511
> base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
> config: i386-randconfig-x010-201750 (attached as .config)
> compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
Oh, right...
-boris
>
> All errors (new ones prefixed by >>):
>
> arch/x86/xen/enlighten.c: In function 'arch_xen_balloon_init':
>>> arch/x86/xen/enlighten.c:341:27: error: 'max_pfn' undeclared (first use in this function); did you mean 'pgd_pfn'?
> unsigned long max_addr = max_pfn << PAGE_SHIFT;
> ^~~~~~~
> pgd_pfn
> arch/x86/xen/enlighten.c:341:27: note: each undeclared identifier is reported only once for each function it appears in
>
> vim +341 arch/x86/xen/enlighten.c
>
> 335
> 336 #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> 337 void __init arch_xen_balloon_init(struct resource *hostmem_resource)
> 338 {
> 339 struct xen_memory_map memmap;
> 340 int rc, i, last_guest_ram;
> > 341 unsigned long max_addr = max_pfn << PAGE_SHIFT;
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation