On Thu, 3 Nov 2016 21:02:22 +0800
Xiao Guangrong <guangrong.x...@linux.intel.com> wrote:
>
>
> 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?
+struct NvdimmFuncReadFITOut {
+ /* the size of buffer filled by QEMU. */
+ uint32_t len;
+ uint32_t func_ret_status; /* return status code. */
+ uint8_t fit[0]; /* the FIT data. */
+} QEMU_PACKED;
--------------------------------
| field | len | off | desc...
--------------------------------
| length | 4 | 0 | ....
--------------------------------
| status | 4 | 4 | ....
--------------------------------
| fit data | ................
i.e. you were forgetting to add length in spec so offsets were wrong
even for described fields.
>
> >
> >
> >> | | | | 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().
>
