Re: [Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT

[Xiao Guangrong]

On 11/04/2016 12:13 AM, Igor Mammedov wrote:
> On Thu, 3 Nov 2016 22:53:43 +0800
> Xiao Guangrong <address@hidden> wrote:
>
> > On 11/03/2016 10:49 PM, Igor Mammedov wrote:
> > > On Thu, 3 Nov 2016 21:02:22 +0800
> > > Xiao Guangrong <address@hidden> 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.
> >
> >
> > We can not do this.
> >
> > @len is used by QEMU emulation to count the size of the buffer that
> > _DSM should return. It's only used in NVDIMM_COMMON_DSM method which
> > is shared by the DSM method from VM and Read_Fit.
> spec describes buffer layout independently from AML that uses it,
> so it should describe whole data structure.
>
> Then it's upto guest how to read this data, it could be QEMU generated
> AML (as it's here) or some other driver or even BIOS.

However, what we are talking about is Read_FIT method, so this is
the layout of Read_FIT output rather than all memory in the dsm page.

Actually, in the spec we already have documented the common len field:

   QEMU Writes Output Data (based on the offset in the page):
   [0x0 - 0x3]: 4 bytes, the length of result
   [0x4 - 0xFFF]: 4092 bytes, the DSM result filled by QEMU

Also, i really do not hope to use this field to count the buffer size
returned by Read_FIT, we'd try the best to reuse existing DSM method
(NVDIMM_COMMON_DSM), i.e, treat Read_FIT as normal DSM method.