From: Igor Mammedov <imammedo@redhat.com>
To: Xiao Guangrong <guangrong.xiao@linux.intel.com>
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
Subject: Re: [Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT
Date: Thu, 3 Nov 2016 17:13:23 +0100 [thread overview]
Message-ID: <20161103171323.7825e926@Igors-MacBook-Pro.local> (raw)
In-Reply-To: <b26abd2d-7d6a-53cd-818b-9974d01606b8@linux.intel.com>
On Thu, 3 Nov 2016 22:53:43 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>
> On 11/03/2016 10:49 PM, Igor Mammedov wrote:
> > On Thu, 3 Nov 2016 21:02:22 +0800
> > Xiao Guangrong <guangrong.xiao@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.
>
>
> 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.
next prev parent reply other threads:[~2016-11-03 16:13 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-03 3:51 [Qemu-devel] [PATCH v4 0/3] nvdimm: hotplug support Xiao Guangrong
2016-11-03 3:51 ` [Qemu-devel] [PATCH v4 1/3] nvdimm acpi: introduce fit buffer Xiao Guangrong
2016-11-03 10:00 ` Stefan Hajnoczi
2016-11-03 9:58 ` Xiao Guangrong
2016-11-03 11:02 ` Igor Mammedov
2016-11-03 11:09 ` Xiao Guangrong
2016-11-03 12:29 ` Igor Mammedov
2016-11-03 3:51 ` [Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT Xiao Guangrong
2016-11-03 9:53 ` Stefan Hajnoczi
2016-11-03 10:08 ` Xiao Guangrong
2016-11-03 12:30 ` Igor Mammedov
2016-11-03 11:58 ` Igor Mammedov
2016-11-03 12:21 ` Xiao Guangrong
2016-11-03 13:00 ` Igor Mammedov
2016-11-03 13:02 ` Xiao Guangrong
2016-11-03 14:49 ` Igor Mammedov
2016-11-03 14:53 ` Xiao Guangrong
2016-11-03 16:13 ` Igor Mammedov [this message]
2016-11-03 16:17 ` Xiao Guangrong
2016-11-03 16:49 ` Igor Mammedov
2016-11-03 16:53 ` Xiao Guangrong
2016-11-03 17:29 ` Igor Mammedov
2016-11-03 17:39 ` Xiao Guangrong
2016-11-03 17:54 ` Igor Mammedov
2016-11-03 3:51 ` [Qemu-devel] [PATCH v4 3/3] pc: memhp: enable nvdimm device hotplug Xiao Guangrong
2016-11-03 12:51 ` Igor Mammedov
2016-11-03 12:54 ` Xiao Guangrong
2016-11-03 4:14 ` [Qemu-devel] [PATCH v4 0/3] nvdimm: hotplug support Michael S. Tsirkin
2016-11-03 4:25 ` Xiao Guangrong
2016-11-03 4:51 ` Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161103171323.7825e926@Igors-MacBook-Pro.local \
--to=imammedo@redhat.com \
--cc=dan.j.williams@intel.com \
--cc=ehabkost@redhat.com \
--cc=gleb@kernel.org \
--cc=guangrong.xiao@linux.intel.com \
--cc=kvm@vger.kernel.org \
--cc=mst@redhat.com \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).