From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37740) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fzHPq-0004bT-Lc for qemu-devel@nongnu.org; Mon, 10 Sep 2018 04:19:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fzHPn-0002uC-Dp for qemu-devel@nongnu.org; Mon, 10 Sep 2018 04:19:26 -0400 Date: Mon, 10 Sep 2018 10:19:16 +0200 From: Igor Mammedov Message-ID: <20180910101916.0e49da7d@redhat.com> In-Reply-To: <20180907164402-mutt-send-email-mst@kernel.org> References: <1534931204-112747-1-git-send-email-imammedo@redhat.com> <20180907164402-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] pc: acpi: revert back to 1 SRAT entry for hotpluggable area List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: haozhong.zhang@intel.com, lersek@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com, qemu-stable@nongnu.org On Fri, 7 Sep 2018 16:44:34 -0400 "Michael S. Tsirkin" wrote: > On Wed, Aug 22, 2018 at 11:46:44AM +0200, Igor Mammedov wrote: > > Commit > > 10efd7e108 "pc: acpi: fix memory hotplug regression by reducing stub SRAT entry size" > > attemped to fix hotplug regression introduced by > > 848a1cc1e "hw/acpi-build: build SRAT memory affinity structures for DIMM devices" > > > > fixed issue for Windows/3.0+ linux kernels, however it regressed 2.6 based > > kernels (RHEL6) to the point where guest might crash at boot. > > Reason is that 2.6 kernel discards SRAT table due too small last entry > > which down the road leads to crashes. Hack I've tried in 10efd7e108 is also > > not ACPI spec compliant according to which whole possible RAM should be > > described in SRAT. Revert 10efd7e108 to fix regression for 2.6 based kernels. > > > > With 10efd7e108 reverted, I've also tried splitting SRAT table statically > > in different ways %/node and %/slot but Windows still fails to online > > 2nd pc-dimm hot-plugged into node 0 (as described in 10efd7e108) and > > sometimes even coldplugged pc-dimms where affected with static SRAT > > partitioning. > > The only known so far way where Windows stays happy is when we have 1 > > SRAT entry in the last node covering all hotplug area. > > > > Revert 848a1cc1e until we come up with a way to avoid regression > > on Windows with hotplug area split in several entries. > > Tested this with 2.6/3.0 based kernels (RHEL6/7) and WS20[08/12/12R2/16]). > > > > Signed-off-by: Igor Mammedov > > BTW should this cause any aml test blobs to change? > Does not seem to ... test variants memhp and dimmpxm should be affected by this (I see your pull req has relevant updates to reference SRAT tables) > > > --- > > CC: haozhong.zhang@intel.com > > CC: mst@redhat.com > > CC: qemu-stable@nongnu.org > > CC: ehabkost@redhat.com > > CC: lersek@redhat.com > > --- > > hw/i386/acpi-build.c | 73 +++++++++------------------------------------------- > > 1 file changed, 12 insertions(+), 61 deletions(-) > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index e1ee8ae..1599caa 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -2251,64 +2251,6 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog) > > #define HOLE_640K_START (640 * KiB) > > #define HOLE_640K_END (1 * MiB) > > > > -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; > > - > > - 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; > > - > > - flags = MEM_AFFINITY_ENABLED; > > - if (di->hotpluggable) { > > - flags |= MEM_AFFINITY_HOTPLUGGABLE; > > - } > > - if (is_nvdimm) { > > - flags |= MEM_AFFINITY_NON_VOLATILE; > > - } > > - > > - build_srat_memory(numamem, di->addr, size, di->node, flags); > > - } > > - > > - qapi_free_MemoryDeviceInfoList(info_list); > > -} > > - > > static void > > build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) > > { > > @@ -2414,10 +2356,19 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) > > build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS); > > } > > > > + /* > > + * 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. > > + * Memory devices may override proximity set by this entry, > > + * providing _PXM method if necessary. > > + */ > > if (hotplugabble_address_space_size) { > > - build_srat_hotpluggable_memory(table_data, machine->device_memory->base, > > - hotplugabble_address_space_size, > > - pcms->numa_nodes - 1); > > + numamem = acpi_data_push(table_data, sizeof *numamem); > > + build_srat_memory(numamem, machine->device_memory->base, > > + hotplugabble_address_space_size, pcms->numa_nodes - 1, > > + MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED); > > } > > > > build_header(linker, table_data, > > -- > > 2.7.4 >