From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40087) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1aoH-0008JP-Un for qemu-devel@nongnu.org; Tue, 01 Nov 2016 11:17:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1aoE-00019y-O3 for qemu-devel@nongnu.org; Tue, 01 Nov 2016 11:17:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44168) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1aoE-00019d-HZ for qemu-devel@nongnu.org; Tue, 01 Nov 2016 11:17:06 -0400 Date: Tue, 1 Nov 2016 16:16:59 +0100 From: Igor Mammedov Message-ID: <20161101161659.082585f4@nial.brq.redhat.com> In-Reply-To: <1477672540-27952-2-git-send-email-guangrong.xiao@linux.intel.com> References: <1477672540-27952-1-git-send-email-guangrong.xiao@linux.intel.com> <1477672540-27952-2-git-send-email-guangrong.xiao@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 1/4] nvdimm acpi: prebuild nvdimm devices for available slots List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xiao Guangrong Cc: pbonzini@redhat.com, gleb@kernel.org, mtosatti@redhat.com, stefanha@redhat.com, mst@redhat.com, rth@twiddle.net, ehabkost@redhat.com, dan.j.williams@intel.com, kvm@vger.kernel.org, qemu-devel@nongnu.org On Sat, 29 Oct 2016 00:35:37 +0800 Xiao Guangrong wrote: > For each NVDIMM present or intended to be supported by platform, > platform firmware also exposes an ACPI Namespace Device under > the root device > > So it builds nvdimm devices for all slots to support vNVDIMM hotplug > > Reviewed-by: Stefan Hajnoczi > Signed-off-by: Xiao Guangrong > --- > hw/acpi/nvdimm.c | 41 ++++++++++++++++++++++++----------------- > hw/i386/acpi-build.c | 2 +- > include/hw/mem/nvdimm.h | 3 ++- > 3 files changed, 27 insertions(+), 19 deletions(-) > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index bb896c9..b8a2e62 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -961,12 +961,11 @@ static void nvdimm_build_device_dsm(Aml *dev, uint32_t handle) > aml_append(dev, method); > } > > -static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev) > +static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots) > { > - for (; device_list; device_list = device_list->next) { > - DeviceState *dev = device_list->data; > - int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP, > - NULL); > + uint32_t slot; > + > + for (slot = 0; slot < ram_slots; slot++) { > uint32_t handle = nvdimm_slot_to_handle(slot); > Aml *nvdimm_dev; > > @@ -987,9 +986,9 @@ static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev) > } > } > > -static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, > - GArray *table_data, BIOSLinker *linker, > - GArray *dsm_dma_arrea) > +static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data, > + BIOSLinker *linker, GArray *dsm_dma_arrea, > + uint32_t ram_slots) > { > Aml *ssdt, *sb_scope, *dev; > int mem_addr_offset, nvdimm_ssdt; > @@ -1021,7 +1020,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, > /* 0 is reserved for root device. */ > nvdimm_build_device_dsm(dev, 0); > > - nvdimm_build_nvdimm_devices(device_list, dev); > + nvdimm_build_nvdimm_devices(dev, ram_slots); > > aml_append(sb_scope, dev); > aml_append(ssdt, sb_scope); > @@ -1046,17 +1045,25 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets, > } > > void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, > - BIOSLinker *linker, GArray *dsm_dma_arrea) > + BIOSLinker *linker, GArray *dsm_dma_arrea, > + uint32_t ram_slots) > { > GSList *device_list; > > - /* no NVDIMM device is plugged. */ > device_list = nvdimm_get_plugged_device_list(); > - if (!device_list) { > - return; > + > + /* NVDIMM device is plugged. */ > + if (device_list) { > + nvdimm_build_nfit(device_list, table_offsets, table_data, linker); > + g_slist_free(device_list); > + } > + > + /* > + * NVDIMM device is allowed to be plugged only if there is available > + * slot. > + */ > + if (ram_slots) { put this check before assigning to device_list and bail out early if ram slots 0 as there is no point in proceeding further > + nvdimm_build_ssdt(table_offsets, table_data, linker, dsm_dma_arrea, > + ram_slots); > } after that you'd just keep the hunk below unchanged, which would simplify the patch > - nvdimm_build_nfit(device_list, table_offsets, table_data, linker); > - nvdimm_build_ssdt(device_list, table_offsets, table_data, linker, > - dsm_dma_arrea); > - g_slist_free(device_list); > } > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index e999654..6ae4769 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2767,7 +2767,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine) > } > if (pcms->acpi_nvdimm_state.is_enabled) { > nvdimm_build_acpi(table_offsets, tables_blob, tables->linker, > - pcms->acpi_nvdimm_state.dsm_mem); > + pcms->acpi_nvdimm_state.dsm_mem, machine->ram_slots); > } > > /* Add tables supplied by user (if any) */ > diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h > index 1cfe9e0..63a2b20 100644 > --- a/include/hw/mem/nvdimm.h > +++ b/include/hw/mem/nvdimm.h > @@ -112,5 +112,6 @@ 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, > - BIOSLinker *linker, GArray *dsm_dma_arrea); > + BIOSLinker *linker, GArray *dsm_dma_arrea, > + uint32_t ram_slots); > #endif