Re: [Qemu-devel] [PATCH v7 25/35] nvdimm acpi: init the resource used by NVDIMM ACPI

[Igor Mammedov]

On Mon,  2 Nov 2015 17:13:27 +0800
Xiao Guangrong <address@hidden> wrote:

> A page staring from 0xFF00000 and IO port 0x0a18 - 0xa1b in guest are
                               ^^ missing one 0???

> reserved for NVDIMM ACPI emulation, refer to docs/specs/acpi_nvdimm.txt
> for detailed design
> 
> A parameter, 'nvdimm-support', is introduced for PIIX4_PM and ICH9-LPC
> that controls if nvdimm support is enabled, it is true on default and
> it is false on 2.4 and its earlier version to keep compatibility
> 
> Signed-off-by: Xiao Guangrong <address@hidden>
[...]

> @@ -33,6 +33,15 @@
>   */
>  #define MIN_NAMESPACE_LABEL_SIZE      (128UL << 10)
>  
> +/*
> + * A page staring from 0xFF00000 and IO port 0x0a18 - 0xa1b in guest are
                                 ^^^ missing 0 or value in define below has an 
extra 0

> + * reserved for NVDIMM ACPI emulation, refer to docs/specs/acpi_nvdimm.txt
> + * for detailed design.
> + */
> +#define NVDIMM_ACPI_MEM_BASE          0xFF000000ULL
it still maps RAM at arbitrary place,
that's the reason why VMGenID patches hasn't been merged for
more than several months.
I'm not against of using (more exactly I'm for it) direct mapping
but we should reach consensus when and how to use it first.

I'd wouldn't use addresses below 4G as it may be used firmware or
legacy hardware and I won't bet that 0xFF000000ULL won't conflict
with anything.
An alternative place to allocate reserve from could be high memory.
For pc we have "reserved-memory-end" which currently makes sure
that hotpluggable memory range isn't used by firmware.

How about making API that allows to map additional memory
ranges before reserved-memory-end and pushes it up as mappings are
added.

Michael, Paolo what do you think about it?


> +#define NVDIMM_ACPI_IO_BASE           0x0a18
> +#define NVDIMM_ACPI_IO_LEN            4
> +
>  #define TYPE_NVDIMM      "nvdimm"
>  #define NVDIMM(obj)      OBJECT_CHECK(NVDIMMDevice, (obj), TYPE_NVDIMM)
>  #define NVDIMM_CLASS(oc) OBJECT_CLASS_CHECK(NVDIMMClass, (oc), TYPE_NVDIMM)
> @@ -80,4 +89,29 @@ struct NVDIMMClass {
>  };
>  typedef struct NVDIMMClass NVDIMMClass;
>  
> +/*
> + * AcpiNVDIMMState:
> + * @is_enabled: detect if NVDIMM support is enabled.
> + *
> + * @fit: fit buffer which will be accessed via ACPI _FIT method. It is
> + *       dynamically built based on current NVDIMM devices so that it does
> + *       not require to keep consistent during live migration.
> + *
> + * @ram_mr: RAM-based memory region which is mapped into guest address
> + *          space and used to transfer data between OSPM and QEMU.
> + * @io_mr: the IO region used by OSPM to transfer control to QEMU.
> + */
> +struct AcpiNVDIMMState {
> +    bool is_enabled;
> +
> +    GArray *fit;
> +
> +    MemoryRegion ram_mr;
> +    MemoryRegion io_mr;
> +};
> +typedef struct AcpiNVDIMMState AcpiNVDIMMState;
> +
> +/* Initialize the memory and IO region needed by NVDIMM ACPI emulation.*/
> +void nvdimm_init_acpi_state(MemoryRegion *memory, MemoryRegion *io,
> +                            Object *owner, AcpiNVDIMMState *state);
>  #endif