Re: OvmfPkg (PlatformPei?): supporting Qemu's nvdimm device

[Laszlo Ersek <lersek@redhat.com>]

On 08/09/17 22:47, Rebecca Cran wrote:
> I've been investigating adding support for Qemu's nvdimm devices to
> OVMF.  I was thinking such support would go into PlatformPei, but it
> looks like I can only read the ACPI NFIT in the DXE phase. So, should
> Qemu be changed to add non-volatile memory to the e820 table, or
> should such memory be added during the DXE phase instead?

I don't have the slightest idea, sorry.

One thing that does look bad is that both edk2's

  MdeModulePkg/Universal/Disk/RamDiskDxe

and QEMU's

  hw/acpi/nvdimm.c

think they own the NVDIMM root device with, with _HID=ACPI0012.

I don't recall being consulted when QEMU's NVDIMM stuff was designed and
implemented, despite the fact that it uses the ACPI linker/loader
(apparently).

In general, using the ACPI linker/loader is a good recipe for making an
ACPI-defined feature "just work" under both SeaBIOS and OVMF, without
having to resort to specialized information channels such as new fw_cfg
files, or additional paravirt hardware. However, there are enough quirks
in edk2 / UEFI with regard to ACPI that, without special attention to
OVMF / edk2 at design time, things will likely break due to infighting
over ACPI ownership.

In RamDiskPublishNfit(), there is

    //
    // Assumption is made that if no NFIT is in the ACPI table, there is no
    // NVDIMM root device in the \SB scope.
    // Therefore, a NVDIMM root device will be reported via Secondary System
    // Description Table (SSDT).
    //
    Status = RamDiskPublishSsdt ();

That assumption might or might not be good enough, when QEMU pushes down
its NVDIMM / NFIT stuff. I suggest reading "docs/specs/acpi_nvdimm.txt"
in the QEMU tree, and figuring out if that can work with OVMF's ACPI
linker/loader client, and the rest of the edk2 modules. The assumption
could also be affected by whether OvmfPkg/AcpiTableDxe installs QEMU's
NVDIM stuff before or after RamDiskDxe does its thing. I have no clue.

I apologize but I don't think I can help (or even find the bandwidth to
research or discuss this -- it's 00:13 local time and I'm reviewing
patches...)

I'm CC'ing Michael and Igor, maybe they can provide you with pointers.
(Also adding qemu-devel.)

Thanks
Laszlo