From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46967) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJ4Ja-0006uH-Ts for qemu-devel@nongnu.org; Tue, 12 Jan 2016 14:09:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aJ4JX-00072u-M7 for qemu-devel@nongnu.org; Tue, 12 Jan 2016 14:09:10 -0500 Received: from mga14.intel.com ([192.55.52.115]:61397) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJ4JX-00072l-Gz for qemu-devel@nongnu.org; Tue, 12 Jan 2016 14:09:07 -0500 References: <1451933528-133684-1-git-send-email-guangrong.xiao@linux.intel.com> <20160107151302.66f752e5@nial.brq.redhat.com> From: Xiao Guangrong Message-ID: <56954DA9.4080502@linux.intel.com> Date: Wed, 13 Jan 2016 03:02:01 +0800 MIME-Version: 1.0 In-Reply-To: <20160107151302.66f752e5@nial.brq.redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/6] NVDIMM ACPI: introduce the framework of QEMU emulated DSM List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: ehabkost@redhat.com, kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, dan.j.williams@intel.com, rth@twiddle.net On 01/07/2016 10:13 PM, Igor Mammedov wrote: > On Tue, 5 Jan 2016 02:52:02 +0800 > Xiao Guangrong wrote: > >> This patchset is against commit 5530427f0ca (acpi: extend aml_and() to >> accept target argument) on pci branch of Michael's git tree >> and can be found at: >> https://github.com/xiaogr/qemu.git nvdimm-acpi-v1 >> >> This is the second part of vNVDIMM implementation which implements the >> BIOS patched dsm memory and introduces the framework that allows QEMU >> to emulate DSM method >> >> Thanks to Michael's idea, we do not reserve any memory for NVDIMM ACPI, >> instead we let BIOS allocate the memory and patch the address to the >> offset we want >> >> IO port is still enabled as it plays as the way to notify QEMU and pass >> the patched dsm memory address, so that IO port region, 0x0a18 - 0xa20, >> is reserved and it is divided into two 32 bits ports and used to pass >> the low 32 bits and high 32 bits of dsm memory address to QEMU >> >> Thanks Igor's idea, this patchset also extends DSDT/SSDT to revision 2 >> to apply 64 bit operations, in order to keeping compatibility, old >> version (<= 2.5) still uses revision 1. Since 64 bit operations breaks >> old guests (such as windows XP), we should keep the 64 bits stuff in >> the private place where common ACPI operation does not touch it >> > > general notes: > 1. could you split out AML API additions/changes into separate patches? > even if series nvdims patches couldn't be accepted on next respin, > AML API patches could be good and we could pick them up just > for API completeness. That also would make them easier to review > and reduces count of patches you'd need to respin. Yes, it is definitely better. Have done it in the v2. > 2. add test case for NVDIMM table blob, see tests/bios-tables-test.c > at the beginning of series. > 3. make V=1 check would show you ASL diff your patches are introducing, > it will save you from booting real guest and dumping/decompiling > tables manually. > 4. at the end of series add NVDIMM table test blob with new table. > you can use tests/acpi-test-data/rebuild-expected-aml.sh to make it > 5. if make check by some miracle passes with these patches, > dump NVDIMM table in guest and try to decompile and then compile it > back with IASL, it will show you what needs to be fixed. Igor, really appreciate the tips you shared to me, that helped me a lot in the development of the new version. BTW, i did not touch the core bios_linker_loader_add_pointer() API as i think it can be changed in a separated patchset. In the new version we zeroed the offset by nvdimm itself and dropped aml_int64(). The new version has been posted out, please review. Thank you!