| Message ID | 157107825148.27733.10924648339824665145.stgit@lep8c.aus.stglabs.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | ppc: spapr: virtual NVDIMM support | expand |
On Mon, Oct 14, 2019 at 01:37:37PM -0500, Shivaprasad G Bhat wrote: > nvdimm_device_list is required for parsing the list for devices > in subsequent patches. Move it to common utility area. > > Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> LGTM, assuming it's needed in the subsequent patches. An ack from NVDIMM people would be nice if I'm to merge it via my tree. > --- > hw/acpi/nvdimm.c | 28 +--------------------------- > include/qemu/nvdimm-utils.h | 7 +++++++ > util/Makefile.objs | 1 + > util/nvdimm-utils.c | 29 +++++++++++++++++++++++++++++ > 4 files changed, 38 insertions(+), 27 deletions(-) > create mode 100644 include/qemu/nvdimm-utils.h > create mode 100644 util/nvdimm-utils.c > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index 9fdad6dc3f..5219dd0e2e 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -32,33 +32,7 @@ > #include "hw/acpi/bios-linker-loader.h" > #include "hw/nvram/fw_cfg.h" > #include "hw/mem/nvdimm.h" > - > -static int nvdimm_device_list(Object *obj, void *opaque) > -{ > - GSList **list = opaque; > - > - if (object_dynamic_cast(obj, TYPE_NVDIMM)) { > - *list = g_slist_append(*list, DEVICE(obj)); > - } > - > - object_child_foreach(obj, nvdimm_device_list, opaque); > - return 0; > -} > - > -/* > - * inquire NVDIMM devices and link them into the list which is > - * returned to the caller. > - * > - * Note: it is the caller's responsibility to free the list to avoid > - * memory leak. > - */ > -static GSList *nvdimm_get_device_list(void) > -{ > - GSList *list = NULL; > - > - object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list); > - return list; > -} > +#include "qemu/nvdimm-utils.h" > > #define NVDIMM_UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ > { (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \ > diff --git a/include/qemu/nvdimm-utils.h b/include/qemu/nvdimm-utils.h > new file mode 100644 > index 0000000000..4b8b198ba7 > --- /dev/null > +++ b/include/qemu/nvdimm-utils.h > @@ -0,0 +1,7 @@ > +#ifndef NVDIMM_UTILS_H > +#define NVDIMM_UTILS_H > + > +#include "qemu/osdep.h" > + > +GSList *nvdimm_get_device_list(void); > +#endif > diff --git a/util/Makefile.objs b/util/Makefile.objs > index 41bf59d127..a0f40d26e3 100644 > --- a/util/Makefile.objs > +++ b/util/Makefile.objs > @@ -20,6 +20,7 @@ util-obj-y += envlist.o path.o module.o > util-obj-y += host-utils.o > util-obj-y += bitmap.o bitops.o hbitmap.o > util-obj-y += fifo8.o > +util-obj-y += nvdimm-utils.o > util-obj-y += cacheinfo.o > util-obj-y += error.o qemu-error.o > util-obj-y += qemu-print.o > diff --git a/util/nvdimm-utils.c b/util/nvdimm-utils.c > new file mode 100644 > index 0000000000..5cc768ca47 > --- /dev/null > +++ b/util/nvdimm-utils.c > @@ -0,0 +1,29 @@ > +#include "qemu/nvdimm-utils.h" > +#include "hw/mem/nvdimm.h" > + > +static int nvdimm_device_list(Object *obj, void *opaque) > +{ > + GSList **list = opaque; > + > + if (object_dynamic_cast(obj, TYPE_NVDIMM)) { > + *list = g_slist_append(*list, DEVICE(obj)); > + } > + > + object_child_foreach(obj, nvdimm_device_list, opaque); > + return 0; > +} > + > +/* > + * inquire NVDIMM devices and link them into the list which is > + * returned to the caller. > + * > + * Note: it is the caller's responsibility to free the list to avoid > + * memory leak. > + */ > +GSList *nvdimm_get_device_list(void) > +{ > + GSList *list = NULL; > + > + object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list); > + return list; > +} >
On Mon, 14 Oct 2019 13:37:37 -0500 Shivaprasad G Bhat <sbhat@linux.ibm.com> wrote: > nvdimm_device_list is required for parsing the list for devices > in subsequent patches. Move it to common utility area. > > Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> > --- > hw/acpi/nvdimm.c | 28 +--------------------------- > include/qemu/nvdimm-utils.h | 7 +++++++ > util/Makefile.objs | 1 + > util/nvdimm-utils.c | 29 +++++++++++++++++++++++++++++ instead of creating new file, why not to move it to existing hw/mem/nvdimm.c? > 4 files changed, 38 insertions(+), 27 deletions(-) > create mode 100644 include/qemu/nvdimm-utils.h > create mode 100644 util/nvdimm-utils.c > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c > index 9fdad6dc3f..5219dd0e2e 100644 > --- a/hw/acpi/nvdimm.c > +++ b/hw/acpi/nvdimm.c > @@ -32,33 +32,7 @@ > #include "hw/acpi/bios-linker-loader.h" > #include "hw/nvram/fw_cfg.h" > #include "hw/mem/nvdimm.h" > - > -static int nvdimm_device_list(Object *obj, void *opaque) > -{ > - GSList **list = opaque; > - > - if (object_dynamic_cast(obj, TYPE_NVDIMM)) { > - *list = g_slist_append(*list, DEVICE(obj)); > - } > - > - object_child_foreach(obj, nvdimm_device_list, opaque); > - return 0; > -} > - > -/* > - * inquire NVDIMM devices and link them into the list which is > - * returned to the caller. > - * > - * Note: it is the caller's responsibility to free the list to avoid > - * memory leak. > - */ > -static GSList *nvdimm_get_device_list(void) > -{ > - GSList *list = NULL; > - > - object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list); > - return list; > -} > +#include "qemu/nvdimm-utils.h" > > #define NVDIMM_UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ > { (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \ > diff --git a/include/qemu/nvdimm-utils.h b/include/qemu/nvdimm-utils.h > new file mode 100644 > index 0000000000..4b8b198ba7 > --- /dev/null > +++ b/include/qemu/nvdimm-utils.h > @@ -0,0 +1,7 @@ > +#ifndef NVDIMM_UTILS_H > +#define NVDIMM_UTILS_H > + > +#include "qemu/osdep.h" > + > +GSList *nvdimm_get_device_list(void); > +#endif > diff --git a/util/Makefile.objs b/util/Makefile.objs > index 41bf59d127..a0f40d26e3 100644 > --- a/util/Makefile.objs > +++ b/util/Makefile.objs > @@ -20,6 +20,7 @@ util-obj-y += envlist.o path.o module.o > util-obj-y += host-utils.o > util-obj-y += bitmap.o bitops.o hbitmap.o > util-obj-y += fifo8.o > +util-obj-y += nvdimm-utils.o > util-obj-y += cacheinfo.o > util-obj-y += error.o qemu-error.o > util-obj-y += qemu-print.o > diff --git a/util/nvdimm-utils.c b/util/nvdimm-utils.c > new file mode 100644 > index 0000000000..5cc768ca47 > --- /dev/null > +++ b/util/nvdimm-utils.c > @@ -0,0 +1,29 @@ > +#include "qemu/nvdimm-utils.h" > +#include "hw/mem/nvdimm.h" > + > +static int nvdimm_device_list(Object *obj, void *opaque) > +{ > + GSList **list = opaque; > + > + if (object_dynamic_cast(obj, TYPE_NVDIMM)) { > + *list = g_slist_append(*list, DEVICE(obj)); > + } > + > + object_child_foreach(obj, nvdimm_device_list, opaque); > + return 0; > +} > + > +/* > + * inquire NVDIMM devices and link them into the list which is > + * returned to the caller. > + * > + * Note: it is the caller's responsibility to free the list to avoid > + * memory leak. > + */ > +GSList *nvdimm_get_device_list(void) > +{ > + GSList *list = NULL; > + > + object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list); > + return list; > +} > >
Hi Igor, On 11/19/2019 12:43 PM, Igor Mammedov wrote: > On Mon, 14 Oct 2019 13:37:37 -0500 > Shivaprasad G Bhat <sbhat@linux.ibm.com> wrote: > >> nvdimm_device_list is required for parsing the list for devices >> in subsequent patches. Move it to common utility area. >> >> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> >> --- >> hw/acpi/nvdimm.c | 28 +--------------------------- >> include/qemu/nvdimm-utils.h | 7 +++++++ >> util/Makefile.objs | 1 + >> util/nvdimm-utils.c | 29 +++++++++++++++++++++++++++++ > instead of creating new file, why not to move it to existing hw/mem/nvdimm.c? That would break the build for mips-softmmu. The mips has CONFIG_ACPI_NVDIMM=y and not CONFIG_NVDIMM. So, the build would break failing to fetch the definition from hw/mem/nvdimm.c. I have the patch here from v2 of the series, https://github.com/ShivaprasadGBhat/qemu/commit/00512a25e4852f174fe6c07bc5acb5ee7027e3de Thanks, Shivaprasad
On Wed, 20 Nov 2019 13:31:34 +0530 Shivaprasad G Bhat <sbhat@linux.ibm.com> wrote: > Hi Igor, > > > On 11/19/2019 12:43 PM, Igor Mammedov wrote: > > On Mon, 14 Oct 2019 13:37:37 -0500 > > Shivaprasad G Bhat <sbhat@linux.ibm.com> wrote: > > > >> nvdimm_device_list is required for parsing the list for devices > >> in subsequent patches. Move it to common utility area. > >> > >> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> > >> --- > >> hw/acpi/nvdimm.c | 28 +--------------------------- > >> include/qemu/nvdimm-utils.h | 7 +++++++ > >> util/Makefile.objs | 1 + > >> util/nvdimm-utils.c | 29 +++++++++++++++++++++++++++++ > > instead of creating new file, why not to move it to existing hw/mem/nvdimm.c? > > That would break the build for mips-softmmu. The mips has > CONFIG_ACPI_NVDIMM=y > and not CONFIG_NVDIMM. So, the build would break failing to fetch the > definition from > hw/mem/nvdimm.c. Yes, I forgot that mips doesn't really use any acpi stuff, but it still pulls in files as dependency via piix4 and trying to decouple it is not worth effort. So lets go ahead with your variant using util/nvdimm-utils.c Reviewed-by: Igor Mammedov <imammedo@redhat.com> > I have the patch here from v2 of the series, > https://github.com/ShivaprasadGBhat/qemu/commit/00512a25e4852f174fe6c07bc5acb5ee7027e3de > > > Thanks, > Shivaprasad > >
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index 9fdad6dc3f..5219dd0e2e 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -32,33 +32,7 @@ #include "hw/acpi/bios-linker-loader.h" #include "hw/nvram/fw_cfg.h" #include "hw/mem/nvdimm.h" - -static int nvdimm_device_list(Object *obj, void *opaque) -{ - GSList **list = opaque; - - if (object_dynamic_cast(obj, TYPE_NVDIMM)) { - *list = g_slist_append(*list, DEVICE(obj)); - } - - object_child_foreach(obj, nvdimm_device_list, opaque); - return 0; -} - -/* - * inquire NVDIMM devices and link them into the list which is - * returned to the caller. - * - * Note: it is the caller's responsibility to free the list to avoid - * memory leak. - */ -static GSList *nvdimm_get_device_list(void) -{ - GSList *list = NULL; - - object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list); - return list; -} +#include "qemu/nvdimm-utils.h" #define NVDIMM_UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \ { (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \ diff --git a/include/qemu/nvdimm-utils.h b/include/qemu/nvdimm-utils.h new file mode 100644 index 0000000000..4b8b198ba7 --- /dev/null +++ b/include/qemu/nvdimm-utils.h @@ -0,0 +1,7 @@ +#ifndef NVDIMM_UTILS_H +#define NVDIMM_UTILS_H + +#include "qemu/osdep.h" + +GSList *nvdimm_get_device_list(void); +#endif diff --git a/util/Makefile.objs b/util/Makefile.objs index 41bf59d127..a0f40d26e3 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -20,6 +20,7 @@ util-obj-y += envlist.o path.o module.o util-obj-y += host-utils.o util-obj-y += bitmap.o bitops.o hbitmap.o util-obj-y += fifo8.o +util-obj-y += nvdimm-utils.o util-obj-y += cacheinfo.o util-obj-y += error.o qemu-error.o util-obj-y += qemu-print.o diff --git a/util/nvdimm-utils.c b/util/nvdimm-utils.c new file mode 100644 index 0000000000..5cc768ca47 --- /dev/null +++ b/util/nvdimm-utils.c @@ -0,0 +1,29 @@ +#include "qemu/nvdimm-utils.h" +#include "hw/mem/nvdimm.h" + +static int nvdimm_device_list(Object *obj, void *opaque) +{ + GSList **list = opaque; + + if (object_dynamic_cast(obj, TYPE_NVDIMM)) { + *list = g_slist_append(*list, DEVICE(obj)); + } + + object_child_foreach(obj, nvdimm_device_list, opaque); + return 0; +} + +/* + * inquire NVDIMM devices and link them into the list which is + * returned to the caller. + * + * Note: it is the caller's responsibility to free the list to avoid + * memory leak. + */ +GSList *nvdimm_get_device_list(void) +{ + GSList *list = NULL; + + object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list); + return list; +}
nvdimm_device_list is required for parsing the list for devices in subsequent patches. Move it to common utility area. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> --- hw/acpi/nvdimm.c | 28 +--------------------------- include/qemu/nvdimm-utils.h | 7 +++++++ util/Makefile.objs | 1 + util/nvdimm-utils.c | 29 +++++++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 27 deletions(-) create mode 100644 include/qemu/nvdimm-utils.h create mode 100644 util/nvdimm-utils.c