Re: [PATCH 2/2] pc-nvdimm acpi: build ACPI tables for pc-nvdimm devices

[Haozhong Zhang <haozhong.zhang@intel.com>]

On 01/06/16 09:50, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 05, 2016 at 10:01:26PM +0800, Haozhong Zhang wrote:
> > On 01/05/16 11:00, Stefano Stabellini wrote:
> > > On Mon, 4 Jan 2016, Konrad Rzeszutek Wilk wrote:
> > > > On Mon, Jan 04, 2016 at 04:01:08PM +0000, Stefano Stabellini wrote:
> > > > > CC'ing the Xen tools maintainers and Anthony.
> > > > > 
> > > > > On Tue, 29 Dec 2015, Haozhong Zhang wrote:
> > > > > > Reuse existing NVDIMM ACPI code to build ACPI tables for pc-nvdimm
> > > > > > devices. The resulting tables are then copied into Xen guest domain so
> > > > > > tha they can be later loaded by Xen hvmloader.
> > > > > > 
> > > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > > > > 
> > > > > How much work would it be to generate the nvdimm acpi tables from the
> > > > > Xen toolstack?
> > > > 
> > > > Why duplicate the code? The QEMU generates the NFIT tables and its sub-tables.
> > > >
> > > >
> > > > > Getting the tables from QEMU doesn't seem like a good idea to me, unless
> > > > > we start getting the whole set of ACPI tables that way.
> > > > 
> > > > There is also the ACPI DSDT code - which requires an memory region
> > > > to be reserved for the AML code to drop the parameters so that QEMU
> > > > can scan the NVDIMM for failures. The region (and size) should be
> > > > determined by QEMU since it works on this data.
> > > 
> > > QEMU can generate the whole set of ACPI tables. Why should we take only
> > > the nvdimm tables and not the others?
> > >
> > NVDIMM tables are the only tables required to support vNVDIMM in this
> > patch series, and they are self-contained and not conflict with other
> > existing tables built by hvmloader. For other tables built by QEMU, I
> > have no idea whether they could work with Xen, so I only take nvdimm
> > tables from QEMU.
> 
> But you also have to deal with the SSDT code right?
NVDIMM's SSDT tables including DSM methods are also copied from QEMU.

> And I was under the
> impression that if you use hvmloader - it loads the ACPI DSDT which
> it had compiled at build-time - only - but not the SSDT?
>

(may be I missed something) Why do we bother with DSDT? It does not
contain information or methods of NVDIMM. 

> In other way - just having the ACPI NFIT won't give you the whole
> picture - unless you also jam in ACPI _DSM (ACPI0012) methods as
> part of the SSDT right?
>

Right, _DSM methods are also in SSDT and copied from QEMU. 

> Or does the ACPI tables generation also include an ACPI SSDT with the
> ACPI _DSM methods as part of it? I only see 'nvdimm_build_acpi' and
> NFIT comments, nothing about SSDT?
>

Not sure if we are looking at the same code. For QEMU commit 38a762f,
nvdimm_build_acpi() calls nvdimm_build_ssdt() to build SSDT of NVDIMM.

> > 
> > > I don't think it is wise to have two components which both think are in
> > > control of generating ACPI tables, hvmloader (soon to be the toolstack
> > > with Anthony's work) and QEMU. From an architectural perspective, it
> > > doesn't look robust to me.
> > >
> > Do you mean whenever nvdimm code in QEMU is changed, we would have to
> > make more efforts to ensure it still works with Xen?
> 
> Not sure I follow that. How is it different from the efforts to ensure
> that the patches here (which only add one ACPI MADT table)
> provide the proper support? Wouldn't you do the same type of checking
> every time you modify the nvdimm_build_acpi code?
>

Yes, I do have to check. I was just not clear what 'robust' meant here
and asked so.

Haozhong

> 
> > 
> > > Could we take this opportunity to switch to QEMU generating the whole
> > > set of ACPI tables?
> > >
> > Not quite sure how much effort would be taken on this change. CCed
> > hvmloader maintainers for their comments.
> > 
> > Thanks,
> > Haozhong
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel