On Sat, 8 Oct 2016 15:17:14 +0800
Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx> wrote:
> On 09/30/2016 09:14 PM, Igor Mammedov wrote:
> > On Fri, 12 Aug 2016 14:54:05 +0800
> > Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx> 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.
>
> Okay.
>
> I will change the changelog to reflect this fact and move the spec update
> to this patch.
under spec, I've meant ACPI spec where this UUID is declared
>
> >
> >>
> >> Refer to docs/specs/acpi-nvdimm.txt for detailed design
> > and amend docs to reflect that.
>
> Already done in the spec, i will merge the spec changes into
> this patch as you suggested later.
>
> >
> >>
> >> Signed-off-by: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx>
> >> ---
> >> 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.
>
> Indeed, will fix.
>
> >
> >
> >> + 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.
> >
>
> You mean returning NULL buffer if errors happen? However, the buffer is
> generated by NVDIMM_COMMON_DSM function which is also used by _DSM based
> on NVDIMM DSN specification, i.e, the 'func_ret_status' is needed anyway
> no matter is successful or failed.
>
> Or i missed your idea?
ok, leave it as is.
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
