From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48389) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpfMo-0008Fq-H3 for qemu-devel@nongnu.org; Fri, 23 Oct 2015 12:38:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpfMk-00079z-0y for qemu-devel@nongnu.org; Fri, 23 Oct 2015 12:38:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55078) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpfMj-00079o-PY for qemu-devel@nongnu.org; Fri, 23 Oct 2015 12:38:53 -0400 References: <1445612242-79172-1-git-send-email-imammedo@redhat.com> From: Laszlo Ersek Message-ID: <562A6299.1020007@redhat.com> Date: Fri, 23 Oct 2015 18:38:49 +0200 MIME-Version: 1.0 In-Reply-To: <1445612242-79172-1-git-send-email-imammedo@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 00/19] pc: acpi: move memory hotplug out of DSDT/SSDT into custom table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov , qemu-devel@nongnu.org Cc: Peter Maydell , pbonzini@redhat.com, mst@redhat.com Igor, On 10/23/15 16:57, Igor Mammedov wrote: > As part of moving to dynamic DSDT and dropping > ASL templates althogether this series moves out > ASL part of memory hotplug from DSDT into a custom > ACPI table. Beside of reducing ASL codebase (DSDT) > series tries to generalize and consolidate ACPI > part of memory hotplug code so it could be easier > to reuse it with outher targets (I plan to do it > for ARM later). > > New table uses 64-bit integers for simplification > of code that are available only since ACPI 2.0. > To avoid breaking Windows XP based (rev1) guests > that table is loaded only if guest supports 2.0 > revision Using LoadTable() ASL method. Please consider CC'ing me on future ACPI patches -- it's risky to hope that (or not to care if) I notice them automatically. In particular, it's great that Windows supports LoadTable(), *but* the following requirement of the ACPI spec: Any table referenced by LoadTable must be in memory marked by AddressRangeReserved or AddressRangeNVS. needs extra care if you'd like this to work under OVMF. In fact this topic shares a whole bunch of details with our pending DataTableRegion / VMGenID design: http://thread.gmane.org/gmane.comp.emulators.qemu/357940/focus=361701 and for OVMF's linker/loader (and for edk2's generic EFI_ACPI_TABLE_PROTOCOL) to work with this, you'd have to jump through almost the same hoops. In particular, the new ACPI table should have signature "UEFI" (to make sure EFI_ACPI_TABLE_PROTOCOL allocates it in AcpiNVS). In addition, that signature dictates a data table structure (with a few extra fixed headers), not a definition block, according to the UEFI spec Appendix O. So, I'm not sure this can be made work with OVMF at all, if we'd like to respect all relevant specs (and existing code). ... I wonder when we'll *finally* bite the bullet, and say, "if you want XP to work, then simply don't specify this feature on the QEMU command line, otherwise XP will crash". We could simply create a text file under docs/, and collect all the features / cmdline switches that are *expressly* incompatible with Windows XP. (CC'ing Peter.) For this memory hotplug feature, the direct consequence would be that you could separate out the MHPT stuff into a new *SSDT* instead, and everything would just work automatically with OVMF as well. Of course I can also accept if the consensus is to ignore OVMF... Thanks Laszlo > > As a side effect of code consolidation/simplification > series reduces ACPI tables size on: > - 879 bytes without memory hotplug > - 1216 bytes with 256 hotpluggable memory slots > > More detailed breakdown on max table sizes in default > and hotplug enabled cases: > new+HP new old old+HP > MHPT 50808 - - - > SSDT 2425 2368 2486 53688 > DSDT 2267 2267 3028 3028 > > Tested with following guests: > - RHEL7.2, > - Windows XPsp3, Windows Server 2003EE, > - Windows Server 2008DC, Windows Server 2012R2DC > > Since memory hotpulg doesn't work on XP based guests, > I've just tested that it boots just fine with/without > memory hotplug enabled on QEMU side. > The rest of the guests work as expected. > > git tree for testing: > https://github.com/imammedo/qemu/commits/mhpt_table_v1 > > Igor Mammedov (18): > acpi: aml: add aml_lgreater_equal() and aml_load_table() > acpi: aml: add aml_create_qword_field() > acpi: aml: add aml_decrement() and aml_subtract() > acpi: aml: add aml_call0() helper > pc: acpi: move SSDT part of memhp into a custom table > pc: acpi: memhp: move MHPD._STA method into MHPT table > pc: acpi: memhp: move MHPD.MLCK mutex into NHPT table > pc: acpi: memhp: move MHPD.MSCN method into MHPT table > pc: acpi: make memory device's _UID integer > pc: acpi: memhp: move MHPD.MRST method into MHPT table > pc: acpi: memhp: move MHPD.MPXM method into MHPT table > pc: acpi: memhp: move MHPD.MOST method into MHPT table > pc: acpi: memhp: move MHPD.MEJ0 method into MHPT table > pc: acpi: bump DSDT revision compliance to v2 > pc: acpi: memhp: move MHPD.MCRS method into MHPT table > pc: acpi: memhp: move MHPD Device along with _UID/_HID into MHPT table > pc: acpi: memhp: remove acpi-dsdt-mem-hotplug.dsl and move \_GPE._E03 > into SSDT > pc: acpi: memhp: cleanup MEMORY_HOTPLUG_IO_REGION usage > > Xiao Guangrong (1): > acpi: add aml_mutex(), aml_acquire(), aml_release() > > hw/acpi/Makefile.objs | 2 +- > hw/acpi/aml-build.c | 115 ++++++++++++++- > hw/acpi/memory_hotplug_acpi_table.c | 286 ++++++++++++++++++++++++++++++++++++ > hw/i386/acpi-build.c | 140 ++++-------------- > hw/i386/acpi-dsdt-mem-hotplug.dsl | 171 --------------------- > hw/i386/acpi-dsdt.dsl | 7 +- > hw/i386/q35-acpi-dsdt.dsl | 7 +- > include/hw/acpi/aml-build.h | 9 ++ > include/hw/acpi/memory_hotplug.h | 8 + > include/hw/acpi/pc-hotplug.h | 24 --- > 10 files changed, 443 insertions(+), 326 deletions(-) > create mode 100644 hw/acpi/memory_hotplug_acpi_table.c > delete mode 100644 hw/i386/acpi-dsdt-mem-hotplug.dsl >