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 <guangrong.x...@linux.intel.com>
two minor comments: don't respin just for these, can
be addressed later if necessary.
> ---
> 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?
>
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index f3ade9a..faee86c 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -2,7 +2,7 @@ common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
> common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> cpu_hotplug_acpi_table.o
> common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> memory_hotplug_acpi_table.o
> -common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> +obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
> common-obj-$(CONFIG_ACPI) += acpi_interface.o
> common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
> common-obj-$(CONFIG_ACPI) += aml-build.o
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 49ee68e..8568b20 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -29,6 +29,7 @@
> #include "qemu/osdep.h"
> #include "hw/acpi/acpi.h"
> #include "hw/acpi/aml-build.h"
> +#include "hw/nvram/fw_cfg.h"
> #include "hw/mem/nvdimm.h"
>
> static int nvdimm_plugged_device_list(Object *obj, void *opaque)
> @@ -370,6 +371,40 @@ static void nvdimm_build_nfit(GSList *device_list,
> GArray *table_offsets,
> g_array_free(structures, true);
> }
>
> +static uint64_t
> +nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + return 0;
> +}
> +
> +static void
> +nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps nvdimm_dsm_ops = {
> + .read = nvdimm_dsm_read,
> + .write = nvdimm_dsm_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid = {
> + .min_access_size = 4,
> + .max_access_size = 4,
> + },
> +};
> +
> +void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
> + FWCfgState *fw_cfg, Object *owner)
> +{
> + memory_region_init_io(&state->io_mr, owner, &nvdimm_dsm_ops, state,
> + "nvdimm-acpi-io", NVDIMM_ACPI_IO_LEN);
> + memory_region_add_subregion(io, NVDIMM_ACPI_IO_BASE, &state->io_mr);
> +
> + state->dsm_mem = g_array_new(false, true /* clear */, 1);
> + acpi_data_push(state->dsm_mem, TARGET_PAGE_SIZE);
> + fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE, state->dsm_mem->data,
> + state->dsm_mem->len);
> +}
> +
> #define NVDIMM_COMMON_DSM "NCAL"
>
> static void nvdimm_build_common_dsm(Aml *dev)
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 6d8d23b..f8ff89d 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -38,7 +38,6 @@
> #include "hw/loader.h"
> #include "hw/isa/isa.h"
> #include "hw/acpi/memory_hotplug.h"
> -#include "hw/mem/nvdimm.h"
> #include "sysemu/tpm.h"
> #include "hw/acpi/tpm.h"
> #include "sysemu/tpm_backend.h"
> @@ -2582,13 +2581,6 @@ static bool acpi_has_iommu(void)
> return intel_iommu && !ambiguous;
> }
>
> -static bool acpi_has_nvdimm(void)
> -{
> - PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> -
> - return pcms->nvdimm;
> -}
> -
> static
> void acpi_build(AcpiBuildTables *tables)
> {
> @@ -2673,7 +2665,7 @@ void acpi_build(AcpiBuildTables *tables)
> build_dmar_q35(tables_blob, tables->linker);
> }
>
> - if (acpi_has_nvdimm()) {
> + if (pcms->acpi_nvdimm_state.is_enabled) {
> nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
> }
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0aeefd2..5194acd 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1854,14 +1854,14 @@ static bool pc_machine_get_nvdimm(Object *obj, Error
> **errp)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
>
> - return pcms->nvdimm;
> + return pcms->acpi_nvdimm_state.is_enabled;
> }
>
> static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
>
> - pcms->nvdimm = value;
> + pcms->acpi_nvdimm_state.is_enabled = value;
> }
>
> static void pc_machine_initfn(Object *obj)
> @@ -1900,7 +1900,7 @@ static void pc_machine_initfn(Object *obj)
> &error_abort);
>
> /* nvdimm is disabled on default. */
> - pcms->nvdimm = false;
> + pcms->acpi_nvdimm_state.is_enabled = false;
> object_property_add_bool(obj, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm,
> pc_machine_set_nvdimm, &error_abort);
> }
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 6f8c2cd..6a69b23 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -274,6 +274,11 @@ static void pc_init1(MachineState *machine,
> if (pcmc->pci_enabled) {
> pc_pci_device_init(pci_bus);
> }
> +
> + if (pcms->acpi_nvdimm_state.is_enabled) {
> + nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
> + pcms->fw_cfg, OBJECT(pcms));
> + }
> }
>
> /* Looking for a pc_compat_2_4() function? It doesn't exist.
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 46522c9..17915b0 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -61,6 +61,7 @@ static void pc_q35_init(MachineState *machine)
> PCIDevice *lpc;
> BusState *idebus[MAX_SATA_PORTS];
> ISADevice *rtc_state;
> + MemoryRegion *system_io = get_system_io();
> MemoryRegion *pci_memory;
> MemoryRegion *rom_memory;
> MemoryRegion *ram_memory;
> @@ -160,7 +161,7 @@ static void pc_q35_init(MachineState *machine)
> q35_host->mch.ram_memory = ram_memory;
> q35_host->mch.pci_address_space = pci_memory;
> q35_host->mch.system_memory = get_system_memory();
> - q35_host->mch.address_space_io = get_system_io();
> + q35_host->mch.address_space_io = system_io;
> q35_host->mch.below_4g_mem_size = pcms->below_4g_mem_size;
> q35_host->mch.above_4g_mem_size = pcms->above_4g_mem_size;
> /* pci */
> @@ -251,6 +252,11 @@ static void pc_q35_init(MachineState *machine)
> if (pcmc->pci_enabled) {
> pc_pci_device_init(host_bus);
> }
> +
> + if (pcms->acpi_nvdimm_state.is_enabled) {
> + nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
> + pcms->fw_cfg, OBJECT(pcms));
> + }
> }
>
> #define DEFINE_Q35_MACHINE(suffix, name, compatfn, optionfn) \
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8b3546e..3937f54 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -17,6 +17,7 @@
> #include "hw/boards.h"
> #include "hw/compat.h"
> #include "hw/mem/pc-dimm.h"
> +#include "hw/mem/nvdimm.h"
>
> #define HPET_INTCAP "hpet-intcap"
>
> @@ -57,7 +58,8 @@ struct PCMachineState {
> uint64_t max_ram_below_4g;
> OnOffAuto vmport;
> OnOffAuto smm;
> - bool nvdimm;
> +
> + AcpiNVDIMMState acpi_nvdimm_state;
>
> /* RAM information (sizes, addresses, configuration): */
> ram_addr_t below_4g_mem_size, above_4g_mem_size;
> diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> index 49183c1..634c60b 100644
> --- a/include/hw/mem/nvdimm.h
> +++ b/include/hw/mem/nvdimm.h
> @@ -25,8 +25,34 @@
>
> #include "hw/mem/pc-dimm.h"
>
> -#define TYPE_NVDIMM "nvdimm"
> +#define TYPE_NVDIMM "nvdimm"
>
> +#define NVDIMM_DSM_MEM_FILE "etc/acpi/nvdimm-mem"
> +
> +/*
> + * 32 bits IO port starting from 0x0a18 in guest is reserved for
> + * NVDIMM ACPI emulation.
> + */
> +#define NVDIMM_ACPI_IO_BASE 0x0a18
> +#define NVDIMM_ACPI_IO_LEN 4
> +
> +/*
> + * 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.
> +struct AcpiNVDIMMState {
> + bool is_enabled;
> +
> + GArray *dsm_mem;
> + MemoryRegion io_mr;
> +};
> +typedef struct AcpiNVDIMMState AcpiNVDIMMState;
> +
> +void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
> + FWCfgState *fw_cfg, Object *owner);
> void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
> GArray *linker);
> #endif
> --
> 1.8.3.1
