Re: [Qemu-devel] [RFC PATCH 4/4] nvdimm acpi: build flush hint address structure if required

[Stefan Hajnoczi]

On Fri, Mar 31, 2017 at 04:41:47PM +0800, Haozhong Zhang wrote:
> +/*
> + * ACPI 6.0: 5.2.25.7 Flush Hint Address Structure
> + */
> +static void nvdimm_build_structure_flush_hint(GArray *structures,
> +                                              DeviceState *dev,
> +                                              unsigned int cache_line_size,
> +                                              Error **errp)
> +{
> +    NvdimmNfitFlushHintAddress *flush_hint;
> +    int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP, NULL);
> +    PCDIMMDevice *dimm = PC_DIMM(dev);
> +    NVDIMMDevice *nvdimm = NVDIMM(dev);
> +    uint64_t addr;
> +    unsigned int i;
> +    MemoryRegion *mr;
> +    Error *local_err = NULL;
> +
> +    if (!nvdimm->flush_hint_enabled) {
> +        return;
> +    }
> +
> +    if (cache_line_size * NR_FLUSH_HINT_ADDR > dimm->reserved_size) {
> +        error_setg(&local_err,
> +                   "insufficient reserved space for flush hint buffers");
> +        goto out;
> +    }
> +
> +    addr = object_property_get_int(OBJECT(dev), PC_DIMM_ADDR_PROP, NULL);
> +    addr += object_property_get_int(OBJECT(dev), PC_DIMM_SIZE_PROP, NULL);
> +
> +    flush_hint = acpi_data_push(structures, sizeof(*flush_hint));
> +    flush_hint->type = cpu_to_le16(6 /* Flush Hint Address Structure */);
> +    flush_hint->length = cpu_to_le16(sizeof(*flush_hint));
> +    flush_hint->nfit_handle = cpu_to_le32(nvdimm_slot_to_handle(slot));
> +    flush_hint->nr_flush_hint_addr = cpu_to_le16(NR_FLUSH_HINT_ADDR);
> +
> +    for (i = 0; i < NR_FLUSH_HINT_ADDR; i++, addr += cache_line_size) {
> +        flush_hint->flush_hint_addr[i] = cpu_to_le64(addr);
> +
> +        mr = g_new0(MemoryRegion, 1);
> +        memory_region_init_io(mr, OBJECT(dev), &nvdimm_flush_hint_ops, 
> nvdimm,
> +                              "nvdimm-flush-hint", cache_line_size);
> +        memory_region_add_subregion(get_system_memory(), addr, mr);

It would be cleaner to create the memory region in the nvdimm device
code instead of the acpi code.

The acpi code just needs to get the number of flush hints and the MMIO
page address from the nvdimm device.  It doesn't need to create the
memory region object or provide the read/write functions.

Stefan

signature.asc
Description: PGP signature