From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Mammedov Subject: Re: [PATCH v2 3/8] nvdimm acpi: introduce _FIT Date: Fri, 30 Sep 2016 15:14:22 +0200 Message-ID: <20160930151422.6327c7d1@nial.brq.redhat.com> References: <1470984850-66891-1-git-send-email-guangrong.xiao@linux.intel.com> <1470984850-66891-4-git-send-email-guangrong.xiao@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: pbonzini@redhat.com, gleb@kernel.org, mtosatti@redhat.com, stefanha@redhat.com, mst@redhat.com, rth@twiddle.net, ehabkost@redhat.com, dan.j.williams@intel.com, kvm@vger.kernel.org, qemu-devel@nongnu.org To: Xiao Guangrong Return-path: Received: from mx1.redhat.com ([209.132.183.28]:47476 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932582AbcI3NO1 (ORCPT ); Fri, 30 Sep 2016 09:14:27 -0400 In-Reply-To: <1470984850-66891-4-git-send-email-guangrong.xiao@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, 12 Aug 2016 14:54:05 +0800 Xiao Guangrong wrote: > _FIT is required for hotplug support, guest will inquire the updated > device info from it if a hotplug event is received > > As FIT buffer is not completely mapped into guest address space, so a > new function, Read FIT whose function index is 0xFFFFFFFF, is reserved > by QEMU to read the piece of FIT buffer. The buffer is concatenated > before _FIT return Only issuer of UUID 2F10E7A4-9E91-11E4-89D3-123B93F75CBA can reserve 0xFFFFFFFF for some purposes. So spec should be amended first or custom generated UUID should be used. > > Refer to docs/specs/acpi-nvdimm.txt for detailed design and amend docs to reflect that. > > Signed-off-by: Xiao Guangrong > --- > hw/acpi/nvdimm.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 82 insertions(+) > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index 0e2b9f0..4bbd1e7 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -886,6 +886,87 @@ static void nvdimm_build_device_dsm(Aml *dev, uint32_t handle) > aml_append(dev, method); > } > > +static void nvdimm_build_fit(Aml *dev) > +{ > + Aml *method, *pkg, *buf, *buf_size, *offset, *call_result; > + Aml *whilectx, *ifcond, *ifctx, *fit; > + > + buf = aml_local(0); > + buf_size = aml_local(1); > + fit = aml_local(2); > + > + /* build helper function, RFIT. */ > + method = aml_method("RFIT", 1, AML_NOTSERIALIZED); since you create named fields (global variable) in method scope, you should make method serialized. Same goes for _FIT method. > + aml_append(method, aml_create_dword_field(aml_buffer(4, NULL), > + aml_int(0), "OFST")); > + > + /* prepare input package. */ > + pkg = aml_package(1); > + aml_append(method, aml_store(aml_arg(0), aml_name("OFST"))); > + aml_append(pkg, aml_name("OFST")); > + > + /* call Read_FIT function. */ > + call_result = aml_call5(NVDIMM_COMMON_DSM, > + aml_touuid("2F10E7A4-9E91-11E4-89D3-123B93F75CBA" > + /* UUID for NVDIMM Root Device */), > + aml_int(1) /* Revision 1 */, > + aml_int(0xFFFFFFFF) /* Read FIT. */, > + pkg, aml_int(0) /* for root device. */); > + aml_append(method, aml_store(call_result, buf)); > + > + /* handle _DSM result. */ > + aml_append(method, aml_create_dword_field(buf, > + aml_int(0) /* offset at byte 0 */, "STAU")); > + > + /* if something is wrong during _DSM. */ > + ifcond = aml_equal(aml_int(0 /* Success */), aml_name("STAU")); > + ifctx = aml_if(aml_lnot(ifcond)); > + aml_append(ifctx, aml_return(aml_buffer(0, NULL))); > + aml_append(method, ifctx); > + aml_append(method, aml_store(aml_sizeof(buf), buf_size)); > + aml_append(method, aml_subtract(buf_size, > + aml_int(4) /* the size of "STAU" */, > + buf_size)); Since you handle error case the same as EOF case you could replace it with EOF case here and on qemu side of interface as well. That should simplify code a bit as you won't need to strip out func_ret_status. > + > + /* if we read the end of fit. */ > + ifctx = aml_if(aml_equal(buf_size, aml_int(0))); > + aml_append(ifctx, aml_return(aml_buffer(0, NULL))); > + aml_append(method, ifctx); > + > + aml_append(method, aml_store(aml_shiftleft(buf_size, aml_int(3)), > + buf_size)); > + aml_append(method, aml_create_field(buf, > + aml_int(4 * BITS_PER_BYTE), /* offset at byte 4.*/ > + buf_size, "BUFF")); > + aml_append(method, aml_return(aml_name("BUFF"))); > + aml_append(dev, method); > + > + /* build _FIT. */ > + method = aml_method("_FIT", 0, AML_NOTSERIALIZED); > + offset = aml_local(3); > + > + aml_append(method, aml_store(aml_buffer(0, NULL), fit)); > + aml_append(method, aml_store(aml_int(0), offset)); > + > + whilectx = aml_while(aml_int(1)); > + aml_append(whilectx, aml_store(aml_call1("RFIT", offset), buf)); > + aml_append(whilectx, aml_store(aml_sizeof(buf), buf_size)); > + > + /* finish fit read if no data is read out. */ > + ifctx = aml_if(aml_equal(buf_size, aml_int(0))); > + aml_append(ifctx, aml_return(fit)); > + aml_append(whilectx, ifctx); > + > + /* update the offset. */ > + aml_append(whilectx, aml_add(offset, buf_size, offset)); > + /* append the data we read out to the fit buffer. */ > + aml_append(whilectx, aml_concatenate(fit, buf, fit)); > + aml_append(method, whilectx); > + > + aml_append(dev, method); > +} > + > static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots) > { > uint32_t slot; > @@ -1001,6 +1082,7 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data, > > /* 0 is reserved for root device. */ > nvdimm_build_device_dsm(dev, 0); > + nvdimm_build_fit(dev); > > nvdimm_build_nvdimm_devices(dev, ram_slots); > From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56582) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpxe4-0007I5-Ja for qemu-devel@nongnu.org; Fri, 30 Sep 2016 09:14:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bpxdz-00015r-Hw for qemu-devel@nongnu.org; Fri, 30 Sep 2016 09:14:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60348) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bpxdz-00015Y-7Y for qemu-devel@nongnu.org; Fri, 30 Sep 2016 09:14:27 -0400 Date: Fri, 30 Sep 2016 15:14:22 +0200 From: Igor Mammedov Message-ID: <20160930151422.6327c7d1@nial.brq.redhat.com> In-Reply-To: <1470984850-66891-4-git-send-email-guangrong.xiao@linux.intel.com> References: <1470984850-66891-1-git-send-email-guangrong.xiao@linux.intel.com> <1470984850-66891-4-git-send-email-guangrong.xiao@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 3/8] nvdimm acpi: introduce _FIT List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xiao Guangrong Cc: pbonzini@redhat.com, gleb@kernel.org, mtosatti@redhat.com, stefanha@redhat.com, mst@redhat.com, rth@twiddle.net, ehabkost@redhat.com, dan.j.williams@intel.com, kvm@vger.kernel.org, qemu-devel@nongnu.org On Fri, 12 Aug 2016 14:54:05 +0800 Xiao Guangrong wrote: > _FIT is required for hotplug support, guest will inquire the updated > device info from it if a hotplug event is received > > As FIT buffer is not completely mapped into guest address space, so a > new function, Read FIT whose function index is 0xFFFFFFFF, is reserved > by QEMU to read the piece of FIT buffer. The buffer is concatenated > before _FIT return Only issuer of UUID 2F10E7A4-9E91-11E4-89D3-123B93F75CBA can reserve 0xFFFFFFFF for some purposes. So spec should be amended first or custom generated UUID should be used. > > Refer to docs/specs/acpi-nvdimm.txt for detailed design and amend docs to reflect that. > > Signed-off-by: Xiao Guangrong > --- > hw/acpi/nvdimm.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 82 insertions(+) > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index 0e2b9f0..4bbd1e7 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -886,6 +886,87 @@ static void nvdimm_build_device_dsm(Aml *dev, uint32_t handle) > aml_append(dev, method); > } > > +static void nvdimm_build_fit(Aml *dev) > +{ > + Aml *method, *pkg, *buf, *buf_size, *offset, *call_result; > + Aml *whilectx, *ifcond, *ifctx, *fit; > + > + buf = aml_local(0); > + buf_size = aml_local(1); > + fit = aml_local(2); > + > + /* build helper function, RFIT. */ > + method = aml_method("RFIT", 1, AML_NOTSERIALIZED); since you create named fields (global variable) in method scope, you should make method serialized. Same goes for _FIT method. > + aml_append(method, aml_create_dword_field(aml_buffer(4, NULL), > + aml_int(0), "OFST")); > + > + /* prepare input package. */ > + pkg = aml_package(1); > + aml_append(method, aml_store(aml_arg(0), aml_name("OFST"))); > + aml_append(pkg, aml_name("OFST")); > + > + /* call Read_FIT function. */ > + call_result = aml_call5(NVDIMM_COMMON_DSM, > + aml_touuid("2F10E7A4-9E91-11E4-89D3-123B93F75CBA" > + /* UUID for NVDIMM Root Device */), > + aml_int(1) /* Revision 1 */, > + aml_int(0xFFFFFFFF) /* Read FIT. */, > + pkg, aml_int(0) /* for root device. */); > + aml_append(method, aml_store(call_result, buf)); > + > + /* handle _DSM result. */ > + aml_append(method, aml_create_dword_field(buf, > + aml_int(0) /* offset at byte 0 */, "STAU")); > + > + /* if something is wrong during _DSM. */ > + ifcond = aml_equal(aml_int(0 /* Success */), aml_name("STAU")); > + ifctx = aml_if(aml_lnot(ifcond)); > + aml_append(ifctx, aml_return(aml_buffer(0, NULL))); > + aml_append(method, ifctx); > + aml_append(method, aml_store(aml_sizeof(buf), buf_size)); > + aml_append(method, aml_subtract(buf_size, > + aml_int(4) /* the size of "STAU" */, > + buf_size)); Since you handle error case the same as EOF case you could replace it with EOF case here and on qemu side of interface as well. That should simplify code a bit as you won't need to strip out func_ret_status. > + > + /* if we read the end of fit. */ > + ifctx = aml_if(aml_equal(buf_size, aml_int(0))); > + aml_append(ifctx, aml_return(aml_buffer(0, NULL))); > + aml_append(method, ifctx); > + > + aml_append(method, aml_store(aml_shiftleft(buf_size, aml_int(3)), > + buf_size)); > + aml_append(method, aml_create_field(buf, > + aml_int(4 * BITS_PER_BYTE), /* offset at byte 4.*/ > + buf_size, "BUFF")); > + aml_append(method, aml_return(aml_name("BUFF"))); > + aml_append(dev, method); > + > + /* build _FIT. */ > + method = aml_method("_FIT", 0, AML_NOTSERIALIZED); > + offset = aml_local(3); > + > + aml_append(method, aml_store(aml_buffer(0, NULL), fit)); > + aml_append(method, aml_store(aml_int(0), offset)); > + > + whilectx = aml_while(aml_int(1)); > + aml_append(whilectx, aml_store(aml_call1("RFIT", offset), buf)); > + aml_append(whilectx, aml_store(aml_sizeof(buf), buf_size)); > + > + /* finish fit read if no data is read out. */ > + ifctx = aml_if(aml_equal(buf_size, aml_int(0))); > + aml_append(ifctx, aml_return(fit)); > + aml_append(whilectx, ifctx); > + > + /* update the offset. */ > + aml_append(whilectx, aml_add(offset, buf_size, offset)); > + /* append the data we read out to the fit buffer. */ > + aml_append(whilectx, aml_concatenate(fit, buf, fit)); > + aml_append(method, whilectx); > + > + aml_append(dev, method); > +} > + > static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots) > { > uint32_t slot; > @@ -1001,6 +1082,7 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data, > > /* 0 is reserved for root device. */ > nvdimm_build_device_dsm(dev, 0); > + nvdimm_build_fit(dev); > > nvdimm_build_nvdimm_devices(dev, ram_slots); >