From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49261) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGqHn-00011T-AZ for qemu-devel@nongnu.org; Wed, 06 Jan 2016 10:46:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGqHj-0006IE-8K for qemu-devel@nongnu.org; Wed, 06 Jan 2016 10:46:07 -0500 Received: from mga03.intel.com ([134.134.136.65]:36873) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGqHi-0006G9-Tt for qemu-devel@nongnu.org; Wed, 06 Jan 2016 10:46:03 -0500 References: <1451933528-133684-1-git-send-email-guangrong.xiao@linux.intel.com> <1451933528-133684-4-git-send-email-guangrong.xiao@linux.intel.com> <20160106162301.735e6517@nial.brq.redhat.com> From: Xiao Guangrong Message-ID: <568D3518.2000402@linux.intel.com> Date: Wed, 6 Jan 2016 23:39:04 +0800 MIME-Version: 1.0 In-Reply-To: <20160106162301.735e6517@nial.brq.redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/6] nvdimm acpi: introduce patched dsm memory 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/06/2016 11:23 PM, Igor Mammedov wrote: > On Tue, 5 Jan 2016 02:52:05 +0800 > Xiao Guangrong wrote: > >> The dsm memory is used to save the input parameters and store >> the dsm result which is filled by QEMU. >> >> The address of dsm memory is decided by bios and patched into >> int64 object returned by "MEMA" method >> >> Signed-off-by: Xiao Guangrong >> --- >> hw/acpi/aml-build.c | 12 ++++++++++++ >> hw/acpi/nvdimm.c | 24 ++++++++++++++++++++++-- >> include/hw/acpi/aml-build.h | 1 + >> 3 files changed, 35 insertions(+), 2 deletions(-) >> >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c >> index 78e1290..83eadb3 100644 >> --- a/hw/acpi/aml-build.c >> +++ b/hw/acpi/aml-build.c >> @@ -394,6 +394,18 @@ Aml *aml_int(const uint64_t val) >> } >> >> /* >> + * ACPI 1.0b: 16.2.3 Data Objects Encoding: >> + * encode: QWordConst >> + */ >> +Aml *aml_int64(const uint64_t val) >> +{ >> + Aml *var = aml_alloc(); >> + build_append_byte(var->buf, 0x0E); /* QWordPrefix */ >> + build_append_int_noprefix(var->buf, val, 8); >> + return var; >> +} >> + >> +/* >> * helper to construct NameString, which returns Aml object >> * for using with aml_append or other aml_* terms >> */ >> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c >> index bc7cd8f..a72104c 100644 >> --- a/hw/acpi/nvdimm.c >> +++ b/hw/acpi/nvdimm.c >> @@ -28,6 +28,7 @@ >> >> #include "hw/acpi/acpi.h" >> #include "hw/acpi/aml-build.h" >> +#include "hw/acpi/bios-linker-loader.h" >> #include "hw/nvram/fw_cfg.h" >> #include "hw/mem/nvdimm.h" >> >> @@ -402,7 +403,8 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io, >> state->dsm_mem->len); >> } >> >> -#define NVDIMM_COMMON_DSM "NCAL" >> +#define NVDIMM_GET_DSM_MEM "MEMA" >> +#define NVDIMM_COMMON_DSM "NCAL" >> >> static void nvdimm_build_common_dsm(Aml *dev) >> { >> @@ -468,7 +470,8 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, >> GArray *table_data, GArray *linker, >> uint8_t revision) >> { >> - Aml *ssdt, *sb_scope, *dev; >> + Aml *ssdt, *sb_scope, *dev, *method; >> + int offset; >> >> acpi_add_table(table_offsets, table_data); >> >> @@ -499,9 +502,26 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, >> >> aml_append(sb_scope, dev); >> >> + /* >> + * leave it at the end of ssdt so that we can conveniently get the >> + * offset of int64 object returned by the function which will be >> + * patched with the real address of the dsm memory by BIOS. >> + */ >> + method = aml_method(NVDIMM_GET_DSM_MEM, 0, AML_NOTSERIALIZED); >> + aml_append(method, aml_return(aml_int64(0x0))); > there is no need in dedicated aml_int64(), you can use aml_int(0x6400000000) trick We can not do this due to the trick in bios_linker_loader_add_pointer() which will issue a COMMAND_ADD_POINTER to BIOS, however, this request does: /* * COMMAND_ADD_POINTER - patch the table (originating from * @dest_file) at @pointer.offset, by adding a pointer to the table * originating from @src_file. 1,2,4 or 8 byte unsigned * addition is used depending on @pointer.size. */ that means the new-offset = old-offset + the address of the new table allocated by BIOS. So we expect 0 offset here. > >> + aml_append(sb_scope, method); >> aml_append(ssdt, sb_scope); >> /* copy AML table into ACPI tables blob and patch header there */ >> g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len); >> + >> + offset = table_data->len - 8; >> + >> + bios_linker_loader_alloc(linker, NVDIMM_DSM_MEM_FILE, TARGET_PAGE_SIZE, >> + false /* high memory */); >> + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, >> + NVDIMM_DSM_MEM_FILE, table_data, >> + table_data->data + offset, >> + sizeof(uint64_t)); > this offset magic will break badly as soon as someone add something > to the end of SSDT. > Yes, it is, so don't do that, :) and this is why we made the comment here: + /* + * leave it at the end of ssdt so that we can conveniently get the + * offset of int64 object returned by the function which will be + * patched with the real address of the dsm memory by BIOS. + */