Re: [Qemu-devel] [PATCH v5 1/5] nvdimm acpi: initialize the resource used by NVDIMM ACPI

[Xiao Guangrong]

On 03/02/2016 07:58 PM, Michael S. Tsirkin wrote:
> On Wed, Mar 02, 2016 at 07:50:37PM +0800, Xiao Guangrong wrote:
> > 32 bits IO port starting from 0x0a18 in guest is reserved for NVDIMM
> > ACPI emulation. The table, NVDIMM_DSM_MEM_FILE, will be patched into
> > NVDIMM ACPI binary code
> >
> > OSPM uses this port to tell QEMU the final address of the DSM memory
> > and notify QEMU to emulate the DSM method
> >
> > Signed-off-by: Xiao Guangrong <address@hidden>
>
>
> two minor comments: don't respin just for these, can
> be addressed later if necessary.

Okay.

> > ---
> >   hw/acpi/Makefile.objs   |  2 +-
> >   hw/acpi/nvdimm.c        | 35 +++++++++++++++++++++++++++++++++++
> >   hw/i386/acpi-build.c    | 10 +---------
> >   hw/i386/pc.c            |  6 +++---
> >   hw/i386/pc_piix.c       |  5 +++++
> >   hw/i386/pc_q35.c        |  8 +++++++-
> >   include/hw/i386/pc.h    |  4 +++-
> >   include/hw/mem/nvdimm.h | 28 +++++++++++++++++++++++++++-
> >   8 files changed, 82 insertions(+), 16 deletions(-)
>
> How about a spec document to document the interface?

Sure, good to me. Actually, there was a spec file in the old versions (e.g,
https://github.com/xiaogr/qemu/commit/3e30799182ec53fd56af1e753a24ccf9f6a8f047),
 I will
add it in the last part which will implement the real DSM functions.

> > +/*
> > + * AcpiNVDIMMState:
> > + * @is_enabled: detect if NVDIMM support is enabled.
> > + *
> > + * @dsm_mem: the data of the fw_cfg file NVDIMM_DSM_MEM_FILE.
> > + * @io_mr: the IO region used by OSPM to transfer control to QEMU.
> > + */
>
> this is not the way we document structures normally.
> comments belong near fields.

Okay. keep that in my mind. ;)