From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [Qemu-devel] [PATCH 0/6] NVDIMM ACPI: introduce the framework of QEMU emulated DSM Date: Wed, 13 Jan 2016 03:02:01 +0800 Message-ID: <56954DA9.4080502@linux.intel.com> References: <1451933528-133684-1-git-send-email-guangrong.xiao@linux.intel.com> <20160107151302.66f752e5@nial.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: pbonzini@redhat.com, ehabkost@redhat.com, kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, dan.j.williams@intel.com, rth@twiddle.net To: Igor Mammedov Return-path: Received: from mga03.intel.com ([134.134.136.65]:49905 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753165AbcALTJQ (ORCPT ); Tue, 12 Jan 2016 14:09:16 -0500 In-Reply-To: <20160107151302.66f752e5@nial.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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!