qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: qemu-devel@nongnu.org
Cc: mst@redhat.com, ehabkost@redhat.com, lersek@redhat.com,
	qemu-stable@nongnu.org
Subject: [Qemu-devel] [PATCH] acpi: SRAT: do not create reserved gap entries
Date: Fri, 10 Aug 2018 16:06:57 +0200	[thread overview]
Message-ID: <1533910017-55158-1-git-send-email-imammedo@redhat.com> (raw)

Commit 848a1cc1e8b04 while introducing SRAT entries for DIMM and NVDIMM
also introduced fake entries for gaps between memory devices, assuming
that we need all possible range covered with SRAT entries.
And it did it wrong since gap would overlap with preceeding entry.
Reproduced with following CLI:

 -m 1G,slots=4,maxmem=8 \
 -object memory-backend-ram,size=1G,id=m0 \
 -device pc-dimm,memdev=m0,addr=0x101000000 \
 -object memory-backend-ram,size=1G,id=m1 \
 -device pc-dimm,memdev=m1

However recent development (10efd7e108) showed that gap entries might
be not need. And indeed testing with WS2008DC-WS2016DC guests range
shows that memory hotplug works just fine without gap entries.

So rather than fixing gap entry borders, just drop them altogether
and simplify code around it.

Spotted-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
There is no need to update reference blobs since gaps beween dimms
aren't generated by any exsting test case.

Considering issue is not visible by default lets just merge it into 3.1
and stable 3.0.1

---
 hw/i386/acpi-build.c | 62 +++++++++++++++++++---------------------------------
 1 file changed, 22 insertions(+), 40 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e1ee8ae..d3d690f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2254,48 +2254,16 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)
 static void build_srat_hotpluggable_memory(GArray *table_data, uint64_t base,
                                            uint64_t len, int default_node)
 {
-    MemoryDeviceInfoList *info_list = qmp_memory_device_list();
     MemoryDeviceInfoList *info;
-    MemoryDeviceInfo *mi;
-    PCDIMMDeviceInfo *di;
-    uint64_t end = base + len, cur, size;
-    bool is_nvdimm;
     AcpiSratMemoryAffinity *numamem;
-    MemoryAffinityFlags flags;
+    MemoryDeviceInfoList *info_list = qmp_memory_device_list();;
 
-    for (cur = base, info = info_list;
-         cur < end;
-         cur += size, info = info->next) {
-        numamem = acpi_data_push(table_data, sizeof *numamem);
-
-        if (!info) {
-            /*
-             * Entry is required for Windows to enable memory hotplug in OS
-             * and for Linux to enable SWIOTLB when booted with less than
-             * 4G of RAM. Windows works better if the entry sets proximity
-             * to the highest NUMA node in the machine at the end of the
-             * reserved space.
-             * Memory devices may override proximity set by this entry,
-             * providing _PXM method if necessary.
-             */
-            build_srat_memory(numamem, end - 1, 1, default_node,
-                              MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
-            break;
-        }
-
-        mi = info->value;
-        is_nvdimm = (mi->type == MEMORY_DEVICE_INFO_KIND_NVDIMM);
-        di = !is_nvdimm ? mi->u.dimm.data : mi->u.nvdimm.data;
-
-        if (cur < di->addr) {
-            build_srat_memory(numamem, cur, di->addr - cur, default_node,
-                              MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
-            numamem = acpi_data_push(table_data, sizeof *numamem);
-        }
-
-        size = di->size;
+    for (info = info_list; info != NULL; info = info->next) {
+        MemoryAffinityFlags flags = MEM_AFFINITY_ENABLED;
+        MemoryDeviceInfo *mi = info->value;
+        bool is_nvdimm = (mi->type == MEMORY_DEVICE_INFO_KIND_NVDIMM);
+        PCDIMMDeviceInfo *di = !is_nvdimm ? mi->u.dimm.data : mi->u.nvdimm.data;
 
-        flags = MEM_AFFINITY_ENABLED;
         if (di->hotpluggable) {
             flags |= MEM_AFFINITY_HOTPLUGGABLE;
         }
@@ -2303,10 +2271,24 @@ static void build_srat_hotpluggable_memory(GArray *table_data, uint64_t base,
             flags |= MEM_AFFINITY_NON_VOLATILE;
         }
 
-        build_srat_memory(numamem, di->addr, size, di->node, flags);
+        numamem = acpi_data_push(table_data, sizeof *numamem);
+        build_srat_memory(numamem, di->addr, di->size, di->node, flags);
     }
-
     qapi_free_MemoryDeviceInfoList(info_list);
+
+    /*
+     * Entry is required for Windows to enable memory hotplug in OS
+     * and for Linux to enable SWIOTLB when booted with less than
+     * 4G of RAM. Windows works better if the entry sets proximity
+     * to the highest NUMA node in the machine at the end of the
+     * reserved space.
+     * Memory devices may override proximity set by this entry,
+     * providing _PXM method if necessary.
+     */
+    numamem = acpi_data_push(table_data, sizeof *numamem);
+    build_srat_memory(numamem, base + len - 1, 1, default_node,
+                      MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
+
 }
 
 static void
-- 
2.7.4

             reply	other threads:[~2018-08-10 14:07 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-10 14:06 Igor Mammedov [this message]
2018-08-10 16:01 ` [Qemu-devel] [PATCH] acpi: SRAT: do not create reserved gap entries Igor Mammedov
2018-08-10 18:28   ` Eduardo Habkost
2018-08-13  7:07     ` Igor Mammedov
2018-08-13 12:06       ` Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1533910017-55158-1-git-send-email-imammedo@redhat.com \
    --to=imammedo@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=lersek@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).