From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH v4 2/3] nvdimm acpi: introduce _FIT Date: Thu, 3 Nov 2016 21:02:22 +0800 Message-ID: References: <1478145090-11987-1-git-send-email-guangrong.xiao@linux.intel.com> <1478145090-11987-3-git-send-email-guangrong.xiao@linux.intel.com> <20161103125808.11857a35@nial.brq.redhat.com> <2f770790-f3e9-ce56-e11b-e9f7e73e7078@linux.intel.com> <20161103140015.5f07b009@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, 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: Igor Mammedov Return-path: Received: from mga11.intel.com ([192.55.52.93]:41422 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756642AbcKCNJm (ORCPT ); Thu, 3 Nov 2016 09:09:42 -0400 In-Reply-To: <20161103140015.5f07b009@nial.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 11/03/2016 09:00 PM, Igor Mammedov wrote: >>> just drop this and describe properly 'len' in spec section >>> i.e. len: length of entire returned data (including the header) >> >> Okay, i will change the spec like this: >> >> QEMU Writes Output Data (based on the offset in the page): >> [0x0 - 0x3]: 4 bytes, length of entire returned data (including the header) >> >> And drop the length field in Read_Fit return buffer, doc >> the fit buffer like this: >> >> +----------+--------+--------+-------------------------------------------+ >> | Field | Length | Offset | Description | >> +----------+--------+--------+-------------------------------------------+ > you need to add length here, otherwise this table is not correct Ah, so i am confused. struct NvdimmFuncReadFITOut definition is based on the layout of Read_FI output. You suggested to drop the length filed in NvdimmFuncReadFITOut but keep it in the layout, it is not consistent. I missed something? > > >> | | | | return status codes | >> | | | | 0x100 - error caused by NFIT update while | >> | status | 4 | 0 | read by _FIT wasn't completed, other | >> | | | | codes follow Chapter 3 in DSM Spec Rev1 | >> +----------+--------+--------+-------------------------------------------+ >> | fit data | Varies | 8 | FIT data, The remaining size in the | >> | | | | returned buffer is used by FIT | >> +----------+--------+--------+-------------------------------------------+ >> >> >> >>>> +} >>>> + >>>> +static void nvdimm_dsm_reserved_root(AcpiNVDIMMState *state, NvdimmDsmIn *in, >>>> + hwaddr dsm_mem_addr) >>> function name doesn't make any sense to me >> >> As i explained above, handle 0x10000 indicates the reserved _DSM method is >> called on the root device... >> >> It makes sense now? :) > function name should reflect what it does, > i.e use verb and I see only nouns here. Got it, will change it to: nvdimm_handle_reserved_root_method().