* [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
@ 2019-08-07 7:14 ` Balamuruhan S
2019-08-07 10:20 ` Philippe Mathieu-Daudé
` (2 more replies)
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 2/6] hw/ppc/pnv_xscom: extend xscom to use python interface Balamuruhan S
` (9 subsequent siblings)
10 siblings, 3 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 7:14 UTC (permalink / raw)
To: qemu-devel; +Cc: maddy, Balamuruhan S, anju, clg, hari, pbonzini, david
Adds scripting interface with python library to call functions in
python modules from Qemu that can be used to feed input externally
and without recompiling Qemu that can be used for early development,
testing and can be extended to abstract some of Qemu code out to a
python script to ease maintenance.
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
configure | 10 +++++
include/sysemu/python_api.h | 30 +++++++++++++
util/Makefile.objs | 1 +
util/python_api.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 141 insertions(+)
create mode 100644 include/sysemu/python_api.h
create mode 100644 util/python_api.c
diff --git a/configure b/configure
index 714e7fb6a1..fddddcc879 100755
--- a/configure
+++ b/configure
@@ -1866,6 +1866,11 @@ fi
# Preserve python version since some functionality is dependent on it
python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
+# Python config to be used for CFLAGS and LDFLAGS
+if ! [ -z "$python" ]; then
+ python_config="$python-config"
+fi
+
# Suppress writing compiled files
python="$python -B"
@@ -6304,6 +6309,11 @@ echo_version() {
fi
}
+if ! [ -z "$python_config" ]; then
+ QEMU_CFLAGS="$QEMU_CFLAGS $($python_config --includes)"
+ QEMU_LDFLAGS="$QEMU_LDFLAGS $($python_config --ldflags)"
+fi
+
# prepend pixman and ftd flags after all config tests are done
QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS"
QEMU_LDFLAGS="$fdt_ldflags $QEMU_LDFLAGS"
diff --git a/include/sysemu/python_api.h b/include/sysemu/python_api.h
new file mode 100644
index 0000000000..ff02d58377
--- /dev/null
+++ b/include/sysemu/python_api.h
@@ -0,0 +1,30 @@
+#ifndef _PPC_PNV_PYTHON_H
+#define _PPC_PNV_PYTHON_H
+
+#include <stdbool.h>
+#include <Python.h>
+
+extern PyObject *python_callback(const char *abs_module_path, const char *mod,
+ const char *func, char *args[],
+ const int nargs);
+
+extern uint64_t python_callback_int(const char *abs_module_path,
+ const char *mod,
+ const char *func, char *args[],
+ const int nargs);
+
+extern char *python_callback_str(const char *abs_module_path, const char *mod,
+ const char *func, char *args[],
+ const int nargs);
+
+extern bool python_callback_bool(const char *abs_module_path, const char *mod,
+ const char *func, char *args[],
+ const int nargs);
+
+extern void python_args_init_cast_int(char *args[], int arg, int pos);
+
+extern void python_args_init_cast_long(char *args[], uint64_t arg, int pos);
+
+extern void python_args_clean(char *args[], int nargs);
+
+#endif
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 41bf59d127..05851c94a7 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -50,6 +50,7 @@ util-obj-y += range.o
util-obj-y += stats64.o
util-obj-y += systemd.o
util-obj-y += iova-tree.o
+util-obj-y += python_api.o
util-obj-$(CONFIG_INOTIFY1) += filemonitor-inotify.o
util-obj-$(CONFIG_LINUX) += vfio-helpers.o
util-obj-$(CONFIG_POSIX) += drm.o
diff --git a/util/python_api.c b/util/python_api.c
new file mode 100644
index 0000000000..854187e00f
--- /dev/null
+++ b/util/python_api.c
@@ -0,0 +1,100 @@
+#include "sysemu/python_api.h"
+#include "qemu/osdep.h"
+
+PyObject *python_callback(const char *abs_module_path, const char *mod,
+ const char *func, char *args[], const int nargs)
+{
+ PyObject *mod_name, *module, *mod_ref, *function, *arguments;
+ PyObject *result = 0;
+ PyObject *value = NULL;
+
+ /* Set PYTHONPATH to absolute module path directory */
+ if (!abs_module_path)
+ abs_module_path = ".";
+ setenv("PYTHONPATH", abs_module_path, 1);
+
+ /* Initialize the Python Interpreter */
+ Py_Initialize();
+ mod_name = PyUnicode_FromString(mod);
+ /* Import module object */
+ module = PyImport_Import(mod_name);
+ if (!module) {
+ PyErr_Print();
+ fprintf(stderr, "Failed to load \"%s\"\n", mod);
+ exit(EXIT_FAILURE);
+ }
+ mod_ref = PyModule_GetDict(module);
+ function = PyDict_GetItemString(mod_ref, func);
+ if (function && PyCallable_Check(function)) {
+ arguments = PyTuple_New(nargs);
+ for (int i = 0; i < nargs; i++) {
+ value = PyUnicode_FromString(args[i]);
+ if (!value) {
+ Py_DECREF(arguments);
+ Py_DECREF(module);
+ fprintf(stderr, "Cannot convert argument\n");
+ exit(EXIT_FAILURE);
+ }
+ PyTuple_SetItem(arguments, i, value);
+ }
+ PyErr_Print();
+ result = PyObject_CallObject(function, arguments);
+ PyErr_Print();
+ }
+ else {
+ if (PyErr_Occurred())
+ PyErr_Print();
+ fprintf(stderr, "Cannot find function \"%s\"\n", func);
+ exit(EXIT_FAILURE);
+ }
+ /* Clean up */
+ Py_DECREF(value);
+ Py_DECREF(module);
+ Py_DECREF(mod_name);
+ /* Finish the Python Interpreter */
+ Py_Finalize();
+ return result;
+}
+
+uint64_t python_callback_int(const char *abs_module_path, const char *mod,
+ const char *func, char *args[], const int nargs)
+{
+ PyObject *result;
+ result = python_callback(abs_module_path, mod, func, args, nargs);
+ return PyLong_AsLong(result);
+}
+
+char *python_callback_str(const char *abs_module_path, const char *mod,
+ const char *func, char *args[], const int nargs)
+{
+ PyObject *result;
+ result = python_callback(abs_module_path, mod, func, args, nargs);
+ return PyUnicode_AsUTF8(result);
+}
+
+bool python_callback_bool(const char *abs_module_path, const char *mod,
+ const char *func, char *args[], const int nargs)
+{
+ PyObject *result;
+ result = python_callback(abs_module_path, mod, func, args, nargs);
+ return (result == Py_True);
+}
+
+void python_args_init_cast_int(char *args[], int arg, int pos)
+{
+ args[pos]= malloc(sizeof(int));
+ sprintf(args[pos], "%d", arg);
+}
+
+void python_args_init_cast_long(char *args[], uint64_t arg, int pos)
+{
+ args[pos]= g_malloc(sizeof(uint64_t) * 2);
+ sprintf(args[pos], "%lx", arg);
+}
+
+void python_args_clean(char *args[], int nargs)
+{
+ for (int i = 0; i < nargs; i++) {
+ g_free(args[i]);
+ }
+}
--
2.14.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib Balamuruhan S
@ 2019-08-07 10:20 ` Philippe Mathieu-Daudé
2019-08-08 10:10 ` Stefan Hajnoczi
2019-08-08 10:09 ` Stefan Hajnoczi
2019-08-08 10:49 ` Daniel P. Berrangé
2 siblings, 1 reply; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-08-07 10:20 UTC (permalink / raw)
To: Balamuruhan S, qemu-devel
Cc: Peter Maydell, maddy, anju, hari, clg, Stefan Hajnoczi, pbonzini, david
Hi,
On 8/7/19 9:14 AM, Balamuruhan S wrote:
> Adds scripting interface with python library to call functions in
> python modules from Qemu that can be used to feed input externally
> and without recompiling Qemu that can be used for early development,
> testing and can be extended to abstract some of Qemu code out to a
> python script to ease maintenance.
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> configure | 10 +++++
> include/sysemu/python_api.h | 30 +++++++++++++
> util/Makefile.objs | 1 +
> util/python_api.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 141 insertions(+)
> create mode 100644 include/sysemu/python_api.h
> create mode 100644 util/python_api.c
>
> diff --git a/configure b/configure
> index 714e7fb6a1..fddddcc879 100755
> --- a/configure
> +++ b/configure
> @@ -1866,6 +1866,11 @@ fi
> # Preserve python version since some functionality is dependent on it
> python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
>
> +# Python config to be used for CFLAGS and LDFLAGS
> +if ! [ -z "$python" ]; then
> + python_config="$python-config"
> +fi
> +
> # Suppress writing compiled files
> python="$python -B"
>
> @@ -6304,6 +6309,11 @@ echo_version() {
> fi
> }
>
> +if ! [ -z "$python_config" ]; then
> + QEMU_CFLAGS="$QEMU_CFLAGS $($python_config --includes)"
> + QEMU_LDFLAGS="$QEMU_LDFLAGS $($python_config --ldflags)"
> +fi
> +
> # prepend pixman and ftd flags after all config tests are done
> QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS"
> QEMU_LDFLAGS="$fdt_ldflags $QEMU_LDFLAGS"
> diff --git a/include/sysemu/python_api.h b/include/sysemu/python_api.h
> new file mode 100644
> index 0000000000..ff02d58377
> --- /dev/null
> +++ b/include/sysemu/python_api.h
> @@ -0,0 +1,30 @@
> +#ifndef _PPC_PNV_PYTHON_H
> +#define _PPC_PNV_PYTHON_H
> +
> +#include <stdbool.h>
> +#include <Python.h>
> +
> +extern PyObject *python_callback(const char *abs_module_path, const char *mod,
> + const char *func, char *args[],
> + const int nargs);
> +
> +extern uint64_t python_callback_int(const char *abs_module_path,
> + const char *mod,
> + const char *func, char *args[],
> + const int nargs);
> +
> +extern char *python_callback_str(const char *abs_module_path, const char *mod,
> + const char *func, char *args[],
> + const int nargs);
> +
> +extern bool python_callback_bool(const char *abs_module_path, const char *mod,
> + const char *func, char *args[],
> + const int nargs);
> +
> +extern void python_args_init_cast_int(char *args[], int arg, int pos);
> +
> +extern void python_args_init_cast_long(char *args[], uint64_t arg, int pos);
> +
> +extern void python_args_clean(char *args[], int nargs);
> +
> +#endif
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 41bf59d127..05851c94a7 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -50,6 +50,7 @@ util-obj-y += range.o
> util-obj-y += stats64.o
> util-obj-y += systemd.o
> util-obj-y += iova-tree.o
> +util-obj-y += python_api.o
This is probably conditional to having python-dev (or whatever distribs
call the package) installed.
> util-obj-$(CONFIG_INOTIFY1) += filemonitor-inotify.o
> util-obj-$(CONFIG_LINUX) += vfio-helpers.o
> util-obj-$(CONFIG_POSIX) += drm.o
> diff --git a/util/python_api.c b/util/python_api.c
> new file mode 100644
> index 0000000000..854187e00f
> --- /dev/null
> +++ b/util/python_api.c
> @@ -0,0 +1,100 @@
> +#include "sysemu/python_api.h"
> +#include "qemu/osdep.h"
> +
> +PyObject *python_callback(const char *abs_module_path, const char *mod,
> + const char *func, char *args[], const int nargs)
> +{
> + PyObject *mod_name, *module, *mod_ref, *function, *arguments;
> + PyObject *result = 0;
> + PyObject *value = NULL;
> +
> + /* Set PYTHONPATH to absolute module path directory */
> + if (!abs_module_path)
> + abs_module_path = ".";
> + setenv("PYTHONPATH", abs_module_path, 1);
> +
> + /* Initialize the Python Interpreter */
> + Py_Initialize();
> + mod_name = PyUnicode_FromString(mod);
> + /* Import module object */
> + module = PyImport_Import(mod_name);
> + if (!module) {
> + PyErr_Print();
> + fprintf(stderr, "Failed to load \"%s\"\n", mod);
> + exit(EXIT_FAILURE);
> + }
> + mod_ref = PyModule_GetDict(module);
> + function = PyDict_GetItemString(mod_ref, func);
> + if (function && PyCallable_Check(function)) {
> + arguments = PyTuple_New(nargs);
> + for (int i = 0; i < nargs; i++) {
> + value = PyUnicode_FromString(args[i]);
> + if (!value) {
> + Py_DECREF(arguments);
> + Py_DECREF(module);
> + fprintf(stderr, "Cannot convert argument\n");
> + exit(EXIT_FAILURE);
> + }
> + PyTuple_SetItem(arguments, i, value);
> + }
> + PyErr_Print();
> + result = PyObject_CallObject(function, arguments);
> + PyErr_Print();
> + }
> + else {
> + if (PyErr_Occurred())
> + PyErr_Print();
> + fprintf(stderr, "Cannot find function \"%s\"\n", func);
> + exit(EXIT_FAILURE);
> + }
> + /* Clean up */
> + Py_DECREF(value);
> + Py_DECREF(module);
> + Py_DECREF(mod_name);
> + /* Finish the Python Interpreter */
> + Py_Finalize();
> + return result;
> +}
> +
> +uint64_t python_callback_int(const char *abs_module_path, const char *mod,
> + const char *func, char *args[], const int nargs)
> +{
> + PyObject *result;
> + result = python_callback(abs_module_path, mod, func, args, nargs);
> + return PyLong_AsLong(result);
> +}
> +
> +char *python_callback_str(const char *abs_module_path, const char *mod,
> + const char *func, char *args[], const int nargs)
> +{
> + PyObject *result;
> + result = python_callback(abs_module_path, mod, func, args, nargs);
> + return PyUnicode_AsUTF8(result);
> +}
> +
> +bool python_callback_bool(const char *abs_module_path, const char *mod,
> + const char *func, char *args[], const int nargs)
> +{
> + PyObject *result;
> + result = python_callback(abs_module_path, mod, func, args, nargs);
> + return (result == Py_True);
> +}
> +
> +void python_args_init_cast_int(char *args[], int arg, int pos)
> +{
> + args[pos]= malloc(sizeof(int));
> + sprintf(args[pos], "%d", arg);
> +}
> +
> +void python_args_init_cast_long(char *args[], uint64_t arg, int pos)
> +{
> + args[pos]= g_malloc(sizeof(uint64_t) * 2);
> + sprintf(args[pos], "%lx", arg);
> +}
> +
> +void python_args_clean(char *args[], int nargs)
> +{
> + for (int i = 0; i < nargs; i++) {
> + g_free(args[i]);
> + }
> +}
>
Wondering about security, is this feature safe to enable in production
environment? It seems to bypass all the hard effort to harden QEMU security.
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-07 10:20 ` Philippe Mathieu-Daudé
@ 2019-08-08 10:10 ` Stefan Hajnoczi
2019-08-08 10:33 ` Philippe Mathieu-Daudé
2019-08-08 10:53 ` Daniel P. Berrangé
0 siblings, 2 replies; 45+ messages in thread
From: Stefan Hajnoczi @ 2019-08-08 10:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, maddy, qemu-devel, Balamuruhan S, anju, clg,
Stefan Hajnoczi, hari, pbonzini, david
[-- Attachment #1: Type: text/plain, Size: 520 bytes --]
On Wed, Aug 07, 2019 at 12:20:47PM +0200, Philippe Mathieu-Daudé wrote:
> > +void python_args_clean(char *args[], int nargs)
> > +{
> > + for (int i = 0; i < nargs; i++) {
> > + g_free(args[i]);
> > + }
> > +}
> >
>
> Wondering about security, is this feature safe to enable in production
> environment? It seems to bypass all the hard effort to harden QEMU security.
This seems like a feature that distros would not enable. Only users
building QEMU from source could enable it.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-08 10:10 ` Stefan Hajnoczi
@ 2019-08-08 10:33 ` Philippe Mathieu-Daudé
2019-08-08 10:53 ` Daniel P. Berrangé
1 sibling, 0 replies; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-08-08 10:33 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Peter Maydell, maddy, qemu-devel, Balamuruhan S, anju, clg,
Stefan Hajnoczi, hari, pbonzini, david
[-- Attachment #1.1: Type: text/plain, Size: 679 bytes --]
On 8/8/19 12:10 PM, Stefan Hajnoczi wrote:
> On Wed, Aug 07, 2019 at 12:20:47PM +0200, Philippe Mathieu-Daudé wrote:
>>> +void python_args_clean(char *args[], int nargs)
>>> +{
>>> + for (int i = 0; i < nargs; i++) {
>>> + g_free(args[i]);
>>> + }
>>> +}
>>>
>>
>> Wondering about security, is this feature safe to enable in production
>> environment? It seems to bypass all the hard effort to harden QEMU security.
>
> This seems like a feature that distros would not enable. Only users
> building QEMU from source could enable it.
Good. What about throwing big ./configure warning like the unsupported
cpu/os ones? Better safe than sorry :)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-08 10:10 ` Stefan Hajnoczi
2019-08-08 10:33 ` Philippe Mathieu-Daudé
@ 2019-08-08 10:53 ` Daniel P. Berrangé
2019-08-09 8:46 ` Stefan Hajnoczi
1 sibling, 1 reply; 45+ messages in thread
From: Daniel P. Berrangé @ 2019-08-08 10:53 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Peter Maydell, maddy, qemu-devel, Balamuruhan S, anju, clg,
Stefan Hajnoczi, hari, pbonzini, Philippe Mathieu-Daudé,
david
On Thu, Aug 08, 2019 at 11:10:13AM +0100, Stefan Hajnoczi wrote:
> On Wed, Aug 07, 2019 at 12:20:47PM +0200, Philippe Mathieu-Daudé wrote:
> > > +void python_args_clean(char *args[], int nargs)
> > > +{
> > > + for (int i = 0; i < nargs; i++) {
> > > + g_free(args[i]);
> > > + }
> > > +}
> > >
> >
> > Wondering about security, is this feature safe to enable in production
> > environment? It seems to bypass all the hard effort to harden QEMU security.
>
> This seems like a feature that distros would not enable. Only users
> building QEMU from source could enable it.
Well that's true when this scripting is only used from one obscure ppc
device. Once merged though, its inevitable that people will want to
extend scripting to more & more parts of QEMU code. This is a big can
of worms...
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-08 10:53 ` Daniel P. Berrangé
@ 2019-08-09 8:46 ` Stefan Hajnoczi
2019-08-12 4:53 ` Balamuruhan S
0 siblings, 1 reply; 45+ messages in thread
From: Stefan Hajnoczi @ 2019-08-09 8:46 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Peter Maydell, maddy, Stefan Hajnoczi, qemu-devel, Balamuruhan S,
anju, clg, hari, pbonzini, Philippe Mathieu-Daudé,
david
[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]
On Thu, Aug 08, 2019 at 11:53:07AM +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 08, 2019 at 11:10:13AM +0100, Stefan Hajnoczi wrote:
> > On Wed, Aug 07, 2019 at 12:20:47PM +0200, Philippe Mathieu-Daudé wrote:
> > > > +void python_args_clean(char *args[], int nargs)
> > > > +{
> > > > + for (int i = 0; i < nargs; i++) {
> > > > + g_free(args[i]);
> > > > + }
> > > > +}
> > > >
> > >
> > > Wondering about security, is this feature safe to enable in production
> > > environment? It seems to bypass all the hard effort to harden QEMU security.
> >
> > This seems like a feature that distros would not enable. Only users
> > building QEMU from source could enable it.
>
> Well that's true when this scripting is only used from one obscure ppc
> device. Once merged though, its inevitable that people will want to
> extend scripting to more & more parts of QEMU code. This is a big can
> of worms...
When it gets used in new contexts it will be necessary to address
problems or accept that it is unsuitable for those use cases. Starting
simple and dealing with challenges as and when necessary seems okay to
me.
I think we should give features a chance in QEMU if there is a
maintainer to support them. I don't want to use this feature myself and
I see lots of issues with it for my use cases, but if it is compiled out
and doesn't place many requirements on code that does not use it, let's
give it a chance.
My main concern is licensing. I think the QEMU Python API should be GPL
licensed because these scripts are executing as part of the QEMU
process.
Beyond that, let's see if people find this feature useful. Maybe it
will die and be removed, maybe it will become popular and we'll have to
change our perspective :).
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-09 8:46 ` Stefan Hajnoczi
@ 2019-08-12 4:53 ` Balamuruhan S
0 siblings, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-12 4:53 UTC (permalink / raw)
To: Stefan Hajnoczi, Daniel P. Berrangé
Cc: Peter Maydell, maddy, Stefan Hajnoczi, qemu-devel, anju, clg,
hari, pbonzini, Philippe Mathieu-Daudé,
david
On 8/9/19 2:16 PM, Stefan Hajnoczi wrote:
> On Thu, Aug 08, 2019 at 11:53:07AM +0100, Daniel P. Berrangé wrote:
>> On Thu, Aug 08, 2019 at 11:10:13AM +0100, Stefan Hajnoczi wrote:
>>> On Wed, Aug 07, 2019 at 12:20:47PM +0200, Philippe Mathieu-Daudé wrote:
>>>>> +void python_args_clean(char *args[], int nargs)
>>>>> +{
>>>>> + for (int i = 0; i < nargs; i++) {
>>>>> + g_free(args[i]);
>>>>> + }
>>>>> +}
>>>>>
>>>> Wondering about security, is this feature safe to enable in production
>>>> environment? It seems to bypass all the hard effort to harden QEMU security.
>>> This seems like a feature that distros would not enable. Only users
>>> building QEMU from source could enable it.
>> Well that's true when this scripting is only used from one obscure ppc
>> device. Once merged though, its inevitable that people will want to
>> extend scripting to more & more parts of QEMU code. This is a big can
>> of worms...
> When it gets used in new contexts it will be necessary to address
> problems or accept that it is unsuitable for those use cases. Starting
> simple and dealing with challenges as and when necessary seems okay to
> me.
>
> I think we should give features a chance in QEMU if there is a
> maintainer to support them. I don't want to use this feature myself and
> I see lots of issues with it for my use cases, but if it is compiled out
> and doesn't place many requirements on code that does not use it, let's
> give it a chance.
>
> My main concern is licensing. I think the QEMU Python API should be GPL
> licensed because these scripts are executing as part of the QEMU
> process.
>
> Beyond that, let's see if people find this feature useful. Maybe it
> will die and be removed, maybe it will become popular and we'll have to
> change our perspective :).
Thank you all for review and sharing the thoughts :)
>
> Stefan
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib Balamuruhan S
2019-08-07 10:20 ` Philippe Mathieu-Daudé
@ 2019-08-08 10:09 ` Stefan Hajnoczi
2019-08-11 6:39 ` Balamuruhan S
2019-08-08 10:49 ` Daniel P. Berrangé
2 siblings, 1 reply; 45+ messages in thread
From: Stefan Hajnoczi @ 2019-08-08 10:09 UTC (permalink / raw)
To: Balamuruhan S; +Cc: maddy, anju, qemu-devel, hari, clg, pbonzini, david
[-- Attachment #1: Type: text/plain, Size: 848 bytes --]
On Wed, Aug 07, 2019 at 12:44:40PM +0530, Balamuruhan S wrote:
> +void python_args_init_cast_int(char *args[], int arg, int pos)
> +{
> + args[pos]= malloc(sizeof(int));
> + sprintf(args[pos], "%d", arg);
> +}
This is broken. args[pos] is a (possibly NULL) pointer to 4 bytes.
sprintf() will buffer overflow if arg has more than 3 digits.
A correct way to do this is:
args[pos] = g_strdup_printf("%d", arg);
> +void python_args_init_cast_long(char *args[], uint64_t arg, int pos)
> +{
> + args[pos]= g_malloc(sizeof(uint64_t) * 2);
> + sprintf(args[pos], "%lx", arg);
> +}
Same issue.
> +void python_args_clean(char *args[], int nargs)
> +{
> + for (int i = 0; i < nargs; i++) {
> + g_free(args[i]);
> + }
> +}
Mixing malloc() and g_free() is unsafe. If you switch to
g_strdup_printf() then g_free() is correct.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-08 10:09 ` Stefan Hajnoczi
@ 2019-08-11 6:39 ` Balamuruhan S
0 siblings, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-11 6:39 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: maddy, anju, qemu-devel, hari, clg, pbonzini, david
On 8/8/19 3:39 PM, Stefan Hajnoczi wrote:
> On Wed, Aug 07, 2019 at 12:44:40PM +0530, Balamuruhan S wrote:
>> +void python_args_init_cast_int(char *args[], int arg, int pos)
>> +{
>> + args[pos]= malloc(sizeof(int));
>> + sprintf(args[pos], "%d", arg);
>> +}
> This is broken. args[pos] is a (possibly NULL) pointer to 4 bytes.
> sprintf() will buffer overflow if arg has more than 3 digits.
>
> A correct way to do this is:
>
> args[pos] = g_strdup_printf("%d", arg);
Thanks for correcting it.
>
>> +void python_args_init_cast_long(char *args[], uint64_t arg, int pos)
>> +{
>> + args[pos]= g_malloc(sizeof(uint64_t) * 2);
>> + sprintf(args[pos], "%lx", arg);
>> +}
> Same issue.
>
>> +void python_args_clean(char *args[], int nargs)
>> +{
>> + for (int i = 0; i < nargs; i++) {
>> + g_free(args[i]);
>> + }
>> +}
> Mixing malloc() and g_free() is unsafe. If you switch to
> g_strdup_printf() then g_free() is correct.
sure, I will fix it.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib Balamuruhan S
2019-08-07 10:20 ` Philippe Mathieu-Daudé
2019-08-08 10:09 ` Stefan Hajnoczi
@ 2019-08-08 10:49 ` Daniel P. Berrangé
2019-08-08 12:45 ` Philippe Mathieu-Daudé
2 siblings, 1 reply; 45+ messages in thread
From: Daniel P. Berrangé @ 2019-08-08 10:49 UTC (permalink / raw)
To: Balamuruhan S; +Cc: maddy, anju, qemu-devel, hari, clg, pbonzini, david
On Wed, Aug 07, 2019 at 12:44:40PM +0530, Balamuruhan S wrote:
> Adds scripting interface with python library to call functions in
> python modules from Qemu that can be used to feed input externally
> and without recompiling Qemu that can be used for early development,
> testing and can be extended to abstract some of Qemu code out to a
> python script to ease maintenance.
I admit the use case is interesting, but this is opening a can of
worms...
Historically the project has held the view that we do not wish
to have an mechanism to support loading out of tree code into the
QEMU process. Much previously talk was around dlopen'd C plugins,
but dynanically loaded Python plugins are doing the same thing
at a conceptual level.
We didn't wish to expose internals of QEMU in a plugin API to
avoid having any kind of API promise across releases.
There was also the question of licensing with plugins opening
the door for people to extend QEMU with non-free/closed source
functionality.
While this series only uses the plugin for one fairly obscure
device, once a python plugin feature is intergrated in QEMU
there will inevitably be requests to use it in further areas
of QEMU.
IOW, acceptance of this patch is a significant question for
the project, and a broader discussion point, than just this
PPC feature patch series.
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> configure | 10 +++++
> include/sysemu/python_api.h | 30 +++++++++++++
> util/Makefile.objs | 1 +
> util/python_api.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 141 insertions(+)
> create mode 100644 include/sysemu/python_api.h
> create mode 100644 util/python_api.c
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-08 10:49 ` Daniel P. Berrangé
@ 2019-08-08 12:45 ` Philippe Mathieu-Daudé
2019-08-09 4:39 ` David Gibson
2019-08-12 4:45 ` Balamuruhan S
0 siblings, 2 replies; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-08-08 12:45 UTC (permalink / raw)
To: Daniel P. Berrangé, Balamuruhan S
Cc: Damien Hedde, maddy, anju, qemu-devel, hari, clg, pbonzini, david
On 8/8/19 12:49 PM, Daniel P. Berrangé wrote:
> On Wed, Aug 07, 2019 at 12:44:40PM +0530, Balamuruhan S wrote:
>> Adds scripting interface with python library to call functions in
>> python modules from Qemu that can be used to feed input externally
>> and without recompiling Qemu that can be used for early development,
>> testing and can be extended to abstract some of Qemu code out to a
>> python script to ease maintenance.
>
> I admit the use case is interesting, but this is opening a can of
> worms...
>
> Historically the project has held the view that we do not wish
> to have an mechanism to support loading out of tree code into the
> QEMU process. Much previously talk was around dlopen'd C plugins,
> but dynanically loaded Python plugins are doing the same thing
> at a conceptual level.
>
> We didn't wish to expose internals of QEMU in a plugin API to
> avoid having any kind of API promise across releases.
>
> There was also the question of licensing with plugins opening
> the door for people to extend QEMU with non-free/closed source
> functionality.
>
> While this series only uses the plugin for one fairly obscure
> device, once a python plugin feature is intergrated in QEMU
> there will inevitably be requests to use it in further areas
> of QEMU.
>
> IOW, acceptance of this patch is a significant question for
> the project, and a broader discussion point, than just this
> PPC feature patch series.
Since performance is not an issue, we can use a QMP-PyMMIO bridge.
Most of the functions required are already exposed, Damien completed the
missing ones in his 'FAULT INJECTION FRAMEWORK' series:
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06230.html
Maybe we simply need a clearer (better documented) QMP 'MMIO' API?
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-08 12:45 ` Philippe Mathieu-Daudé
@ 2019-08-09 4:39 ` David Gibson
2019-08-12 4:45 ` Balamuruhan S
1 sibling, 0 replies; 45+ messages in thread
From: David Gibson @ 2019-08-09 4:39 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Damien Hedde, maddy, Daniel P. Berrangé,
qemu-devel, Balamuruhan S, anju, clg, hari, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2566 bytes --]
On Thu, Aug 08, 2019 at 02:45:02PM +0200, Philippe Mathieu-Daudé wrote:
> On 8/8/19 12:49 PM, Daniel P. Berrangé wrote:
> > On Wed, Aug 07, 2019 at 12:44:40PM +0530, Balamuruhan S wrote:
> >> Adds scripting interface with python library to call functions in
> >> python modules from Qemu that can be used to feed input externally
> >> and without recompiling Qemu that can be used for early development,
> >> testing and can be extended to abstract some of Qemu code out to a
> >> python script to ease maintenance.
> >
> > I admit the use case is interesting, but this is opening a can of
> > worms...
> >
> > Historically the project has held the view that we do not wish
> > to have an mechanism to support loading out of tree code into the
> > QEMU process. Much previously talk was around dlopen'd C plugins,
> > but dynanically loaded Python plugins are doing the same thing
> > at a conceptual level.
> >
> > We didn't wish to expose internals of QEMU in a plugin API to
> > avoid having any kind of API promise across releases.
> >
> > There was also the question of licensing with plugins opening
> > the door for people to extend QEMU with non-free/closed source
> > functionality.
> >
> > While this series only uses the plugin for one fairly obscure
> > device, once a python plugin feature is intergrated in QEMU
> > there will inevitably be requests to use it in further areas
> > of QEMU.
> >
> > IOW, acceptance of this patch is a significant question for
> > the project, and a broader discussion point, than just this
> > PPC feature patch series.
>
> Since performance is not an issue, we can use a QMP-PyMMIO bridge.
> Most of the functions required are already exposed, Damien completed the
> missing ones in his 'FAULT INJECTION FRAMEWORK' series:
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06230.html
>
> Maybe we simply need a clearer (better documented) QMP 'MMIO' API?
I tend to agree. If performance is not a consideration to the point
that we can use an embedded Python interpreter, it should also not be
an issue to the point that we can move the processing to an entirely
different process with a vetted protocol in between them (QMP or an
extension thereof being the obvious choice). That seems safer than
embeddeding arbitrary scripting right into the MMIO paths.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib
2019-08-08 12:45 ` Philippe Mathieu-Daudé
2019-08-09 4:39 ` David Gibson
@ 2019-08-12 4:45 ` Balamuruhan S
1 sibling, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-12 4:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Daniel P. Berrangé
Cc: Damien Hedde, maddy, anju, qemu-devel, hari, clg, pbonzini, david
On 8/8/19 6:15 PM, Philippe Mathieu-Daudé wrote:
> On 8/8/19 12:49 PM, Daniel P. Berrangé wrote:
>> On Wed, Aug 07, 2019 at 12:44:40PM +0530, Balamuruhan S wrote:
>>> Adds scripting interface with python library to call functions in
>>> python modules from Qemu that can be used to feed input externally
>>> and without recompiling Qemu that can be used for early development,
>>> testing and can be extended to abstract some of Qemu code out to a
>>> python script to ease maintenance.
>> I admit the use case is interesting, but this is opening a can of
>> worms...
>>
>> Historically the project has held the view that we do not wish
>> to have an mechanism to support loading out of tree code into the
>> QEMU process. Much previously talk was around dlopen'd C plugins,
>> but dynanically loaded Python plugins are doing the same thing
>> at a conceptual level.
>>
>> We didn't wish to expose internals of QEMU in a plugin API to
>> avoid having any kind of API promise across releases.
>>
>> There was also the question of licensing with plugins opening
>> the door for people to extend QEMU with non-free/closed source
>> functionality.
>>
>> While this series only uses the plugin for one fairly obscure
>> device, once a python plugin feature is intergrated in QEMU
>> there will inevitably be requests to use it in further areas
>> of QEMU.
>>
>> IOW, acceptance of this patch is a significant question for
>> the project, and a broader discussion point, than just this
>> PPC feature patch series.
> Since performance is not an issue, we can use a QMP-PyMMIO bridge.
> Most of the functions required are already exposed, Damien completed the
> missing ones in his 'FAULT INJECTION FRAMEWORK' series:
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06230.html
will look at this approach and try using QMP-PyMMIO bridge.
Thank you all for review and suggestions.
>
> Maybe we simply need a clearer (better documented) QMP 'MMIO' API?
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [RFC PATCH 2/6] hw/ppc/pnv_xscom: extend xscom to use python interface
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib Balamuruhan S
@ 2019-08-07 7:14 ` Balamuruhan S
2019-08-08 9:04 ` Cédric Le Goater
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV Balamuruhan S
` (8 subsequent siblings)
10 siblings, 1 reply; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 7:14 UTC (permalink / raw)
To: qemu-devel; +Cc: maddy, Balamuruhan S, anju, clg, hari, pbonzini, david
Existing xscom access emulation for read/write can be
extended with the python interface to support feeding
data externally.
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
hw/ppc/pnv_xscom.c | 31 ++++++++++++++++++++++++++++---
include/sysemu/sysemu.h | 4 ++++
qemu-options.hx | 14 ++++++++++++++
vl.c | 42 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 88 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index 2b81c75f56..5d5b5e9884 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -17,11 +17,13 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "sysemu/python_api.h"
#include "qemu/osdep.h"
#include "hw/hw.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "sysemu/hw_accel.h"
+#include "sysemu/sysemu.h"
#include "target/ppc/cpu.h"
#include "hw/sysbus.h"
@@ -157,8 +159,20 @@ static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
uint64_t val = 0;
MemTxResult result;
- /* Handle some SCOMs here before dispatch */
- val = xscom_read_default(chip, pcba);
+ if (xscom_module && xscom_readp) {
+ char **args = g_malloc(2 * sizeof(uint64_t));
+ PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
+ python_args_init_cast_long(args, pcba, 0);
+ python_args_init_cast_int(args, pcc->chip_type, 1);
+ val = python_callback_int(module_path, xscom_module, xscom_readp,
+ args, 2);
+ python_args_clean(args, 2);
+ g_free(args);
+ }
+ else {
+ /* Handle some SCOMs here before dispatch */
+ val = xscom_read_default(chip, pcba);
+ }
if (val != -1) {
goto complete;
}
@@ -184,8 +198,19 @@ static void xscom_write(void *opaque, hwaddr addr, uint64_t val,
uint32_t pcba = pnv_xscom_pcba(chip, addr);
MemTxResult result;
+ if (xscom_module && xscom_writep) {
+ char **args = g_malloc(sizeof(uint64_t));
+ bool xscom_success;
+ python_args_init_cast_long(args, pcba, 0);
+ xscom_success = python_callback_bool(module_path, xscom_module,
+ xscom_writep, args, 1);
+ python_args_clean(args, 1);
+ g_free(args);
+ if (xscom_success)
+ goto complete;
+ }
/* Handle some SCOMs here before dispatch */
- if (xscom_write_default(chip, pcba, val)) {
+ else if (xscom_write_default(chip, pcba, val)) {
goto complete;
}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 984c439ac9..9b8dc346d6 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -117,6 +117,10 @@ extern bool enable_mlock;
extern bool enable_cpu_pm;
extern QEMUClockType rtc_clock;
extern const char *mem_path;
+extern const char *module_path;
+extern const char *xscom_module;
+extern const char *xscom_readp;
+extern const char *xscom_writep;
extern int mem_prealloc;
#define MAX_NODES 128
diff --git a/qemu-options.hx b/qemu-options.hx
index 9621e934c0..06c9f34d99 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -385,6 +385,20 @@ STEXI
Allocate guest RAM from a temporarily created file in @var{path}.
ETEXI
+DEF("module-path", HAS_ARG, QEMU_OPTION_modulepath,
+ "-module-path FILE provide absolute path where python modules"
+ "resides\n", QEMU_ARCH_ALL)
+STEXI
+@item -module-path [path=]@var{absolute path}[,homer_module=homer,homer_func=func1]
+@findex -module-path
+Provides information about where the python modules exist and the callback
+functions defined.
+
+@example
+qemu-system-ppc64 -module-path /home/modules/,homer_module=homer,homer_func=homer_read
+@end example
+ETEXI
+
DEF("mem-prealloc", 0, QEMU_OPTION_mem_prealloc,
"-mem-prealloc preallocate guest memory (use with -mem-path)\n",
QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index b426b32134..28f0dc1c1b 100644
--- a/vl.c
+++ b/vl.c
@@ -140,6 +140,10 @@ int display_opengl;
const char* keyboard_layout = NULL;
ram_addr_t ram_size;
const char *mem_path = NULL;
+const char *module_path = NULL;
+const char *xscom_module = NULL;
+const char *xscom_readp = NULL;
+const char *xscom_writep = NULL;
int mem_prealloc = 0; /* force preallocation of physical target memory */
bool enable_mlock = false;
bool enable_cpu_pm = false;
@@ -469,6 +473,32 @@ static QemuOptsList qemu_mem_opts = {
},
};
+static QemuOptsList qemu_module_opts = {
+ .name = "module_path",
+ .implied_opt_name = "module_path",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_module_opts.head),
+ .merge_lists = true,
+ .desc = {
+ {
+ .name = "module_path",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "xscom_module",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "xscom_read",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "xscom_write",
+ .type = QEMU_OPT_STRING,
+ },
+ { /* end of list */ }
+ },
+};
+
static QemuOptsList qemu_icount_opts = {
.name = "icount",
.implied_opt_name = "shift",
@@ -2923,6 +2953,7 @@ int main(int argc, char **argv, char **envp)
qemu_add_opts(&qemu_machine_opts);
qemu_add_opts(&qemu_accel_opts);
qemu_add_opts(&qemu_mem_opts);
+ qemu_add_opts(&qemu_module_opts);
qemu_add_opts(&qemu_smp_opts);
qemu_add_opts(&qemu_boot_opts);
qemu_add_opts(&qemu_add_fd_opts);
@@ -3190,6 +3221,17 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_mempath:
mem_path = optarg;
break;
+ case QEMU_OPTION_modulepath:
+ opts = qemu_opts_parse_noisily(qemu_find_opts("module_path"),
+ optarg, true);
+ if (!opts) {
+ exit(EXIT_FAILURE);
+ }
+ module_path = qemu_opt_get(opts, "module_path");
+ xscom_module = qemu_opt_get(opts, "xscom_module");
+ xscom_readp = qemu_opt_get(opts, "xscom_read");
+ xscom_writep = qemu_opt_get(opts, "xscom_write");
+ break;
case QEMU_OPTION_mem_prealloc:
mem_prealloc = 1;
break;
--
2.14.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 2/6] hw/ppc/pnv_xscom: extend xscom to use python interface
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 2/6] hw/ppc/pnv_xscom: extend xscom to use python interface Balamuruhan S
@ 2019-08-08 9:04 ` Cédric Le Goater
0 siblings, 0 replies; 45+ messages in thread
From: Cédric Le Goater @ 2019-08-08 9:04 UTC (permalink / raw)
To: Balamuruhan S, qemu-devel
Cc: Peter Maydell, maddy, anju, Joel Stanley, hari, pbonzini, david
On 07/08/2019 09:14, Balamuruhan S wrote:
> Existing xscom access emulation for read/write can be
> extended with the python interface to support feeding
> data externally.
You should take a look at Rashmica's patch :
hw/gpio: Add basic Aspeed GPIO model for AST2400 and AST2500
https://patchwork.ozlabs.org/patch/1138787
The patch generates model properties in the instance_init handler
for all Aspeed GPIO pins (a lot) that can then be un/set from the
QEMU monitor to tweak the values exposed to the guest. It integrates
well in QEMU and its unit test framework. This is a complex example,
they are simpler ones in QEMU.
I think you could do the same for the OCC SRAM which exposes in host
memory a large set of data to the PowerNV machine, sensors, pstates,
etc. You would need to define the properties on top of the structures
which are shared with the host.
You could follow the same pattern for other well known XSCOM registers.
From there, it is relatively simple to write automated unit tests or
a tui/gui to tweak at runtime the OCC data of the machine through the
model properties.
Anyhow, first, we need to extend and correct the PowerNV models that
you are using.
Thanks,
C.
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> hw/ppc/pnv_xscom.c | 31 ++++++++++++++++++++++++++++---
> include/sysemu/sysemu.h | 4 ++++
> qemu-options.hx | 14 ++++++++++++++
> vl.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 88 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index 2b81c75f56..5d5b5e9884 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -17,11 +17,13 @@
> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include "sysemu/python_api.h"
> #include "qemu/osdep.h"
> #include "hw/hw.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> #include "sysemu/hw_accel.h"
> +#include "sysemu/sysemu.h"
> #include "target/ppc/cpu.h"
> #include "hw/sysbus.h"
>
> @@ -157,8 +159,20 @@ static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
> uint64_t val = 0;
> MemTxResult result;
>
> - /* Handle some SCOMs here before dispatch */
> - val = xscom_read_default(chip, pcba);
> + if (xscom_module && xscom_readp) {
> + char **args = g_malloc(2 * sizeof(uint64_t));
> + PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> + python_args_init_cast_long(args, pcba, 0);
> + python_args_init_cast_int(args, pcc->chip_type, 1);
> + val = python_callback_int(module_path, xscom_module, xscom_readp,
> + args, 2);
> + python_args_clean(args, 2);
> + g_free(args);
> + }
> + else {
> + /* Handle some SCOMs here before dispatch */
> + val = xscom_read_default(chip, pcba);
> + }
> if (val != -1) {
> goto complete;
> }
> @@ -184,8 +198,19 @@ static void xscom_write(void *opaque, hwaddr addr, uint64_t val,
> uint32_t pcba = pnv_xscom_pcba(chip, addr);
> MemTxResult result;
>
> + if (xscom_module && xscom_writep) {
> + char **args = g_malloc(sizeof(uint64_t));
> + bool xscom_success;
> + python_args_init_cast_long(args, pcba, 0);
> + xscom_success = python_callback_bool(module_path, xscom_module,
> + xscom_writep, args, 1);
> + python_args_clean(args, 1);
> + g_free(args);
> + if (xscom_success)
> + goto complete;
> + }
> /* Handle some SCOMs here before dispatch */
> - if (xscom_write_default(chip, pcba, val)) {
> + else if (xscom_write_default(chip, pcba, val)) {
> goto complete;
> }
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 984c439ac9..9b8dc346d6 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -117,6 +117,10 @@ extern bool enable_mlock;
> extern bool enable_cpu_pm;
> extern QEMUClockType rtc_clock;
> extern const char *mem_path;
> +extern const char *module_path;
> +extern const char *xscom_module;
> +extern const char *xscom_readp;
> +extern const char *xscom_writep;
> extern int mem_prealloc;
>
> #define MAX_NODES 128
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9621e934c0..06c9f34d99 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -385,6 +385,20 @@ STEXI
> Allocate guest RAM from a temporarily created file in @var{path}.
> ETEXI
>
> +DEF("module-path", HAS_ARG, QEMU_OPTION_modulepath,
> + "-module-path FILE provide absolute path where python modules"
> + "resides\n", QEMU_ARCH_ALL)
> +STEXI
> +@item -module-path [path=]@var{absolute path}[,homer_module=homer,homer_func=func1]
> +@findex -module-path
> +Provides information about where the python modules exist and the callback
> +functions defined.
> +
> +@example
> +qemu-system-ppc64 -module-path /home/modules/,homer_module=homer,homer_func=homer_read
> +@end example
> +ETEXI
> +
> DEF("mem-prealloc", 0, QEMU_OPTION_mem_prealloc,
> "-mem-prealloc preallocate guest memory (use with -mem-path)\n",
> QEMU_ARCH_ALL)
> diff --git a/vl.c b/vl.c
> index b426b32134..28f0dc1c1b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -140,6 +140,10 @@ int display_opengl;
> const char* keyboard_layout = NULL;
> ram_addr_t ram_size;
> const char *mem_path = NULL;
> +const char *module_path = NULL;
> +const char *xscom_module = NULL;
> +const char *xscom_readp = NULL;
> +const char *xscom_writep = NULL;
> int mem_prealloc = 0; /* force preallocation of physical target memory */
> bool enable_mlock = false;
> bool enable_cpu_pm = false;
> @@ -469,6 +473,32 @@ static QemuOptsList qemu_mem_opts = {
> },
> };
>
> +static QemuOptsList qemu_module_opts = {
> + .name = "module_path",
> + .implied_opt_name = "module_path",
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_module_opts.head),
> + .merge_lists = true,
> + .desc = {
> + {
> + .name = "module_path",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "xscom_module",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "xscom_read",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "xscom_write",
> + .type = QEMU_OPT_STRING,
> + },
> + { /* end of list */ }
> + },
> +};
> +
> static QemuOptsList qemu_icount_opts = {
> .name = "icount",
> .implied_opt_name = "shift",
> @@ -2923,6 +2953,7 @@ int main(int argc, char **argv, char **envp)
> qemu_add_opts(&qemu_machine_opts);
> qemu_add_opts(&qemu_accel_opts);
> qemu_add_opts(&qemu_mem_opts);
> + qemu_add_opts(&qemu_module_opts);
> qemu_add_opts(&qemu_smp_opts);
> qemu_add_opts(&qemu_boot_opts);
> qemu_add_opts(&qemu_add_fd_opts);
> @@ -3190,6 +3221,17 @@ int main(int argc, char **argv, char **envp)
> case QEMU_OPTION_mempath:
> mem_path = optarg;
> break;
> + case QEMU_OPTION_modulepath:
> + opts = qemu_opts_parse_noisily(qemu_find_opts("module_path"),
> + optarg, true);
> + if (!opts) {
> + exit(EXIT_FAILURE);
> + }
> + module_path = qemu_opt_get(opts, "module_path");
> + xscom_module = qemu_opt_get(opts, "xscom_module");
> + xscom_readp = qemu_opt_get(opts, "xscom_read");
> + xscom_writep = qemu_opt_get(opts, "xscom_write");
> + break;
> case QEMU_OPTION_mem_prealloc:
> mem_prealloc = 1;
> break;
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib Balamuruhan S
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 2/6] hw/ppc/pnv_xscom: extend xscom to use python interface Balamuruhan S
@ 2019-08-07 7:14 ` Balamuruhan S
2019-08-07 7:54 ` Cédric Le Goater
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area Balamuruhan S
` (7 subsequent siblings)
10 siblings, 1 reply; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 7:14 UTC (permalink / raw)
To: qemu-devel; +Cc: maddy, Balamuruhan S, anju, clg, hari, pbonzini, david
Add mmio callback functions to enable homer/occ common area
to emulate pstate table, occ-sensors, slw, occ static and
dynamic values for Power8 and Power9 chips. It also works for
multiple chips as offset remains the same whereas the base
address are handled appropriately while initializing device
tree.
currently skiboot disables the homer/occ code path with
`QUIRK_NO_PBA`, this quirk have to be removed in skiboot
for it to use this infrastructure.
Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com>
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
hw/ppc/Makefile.objs | 2 +-
hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++
include/hw/ppc/pnv.h | 14 ++++
include/hw/ppc/pnv_homer.h | 41 ++++++++++
4 files changed, 241 insertions(+), 1 deletion(-)
create mode 100644 hw/ppc/pnv_homer.c
create mode 100644 include/hw/ppc/pnv_homer.h
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 9da93af905..7260b4a96c 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o
# IBM PowerNV
-obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
+obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o
ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
endif
diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
new file mode 100644
index 0000000000..73a94856d0
--- /dev/null
+++ b/hw/ppc/pnv_homer.c
@@ -0,0 +1,185 @@
+/*
+ * QEMU PowerPC PowerNV Homer and OCC common area region
+ *
+ * Copyright (c) 2019, IBM Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "qemu/osdep.h"
+#include "sysemu/hw_accel.h"
+#include "sysemu/cpus.h"
+#include "hw/ppc/pnv.h"
+
+static bool core_max_array(hwaddr addr)
+{
+ char *cpu_type;
+ hwaddr core_max_base = 0xe2819;
+ MachineState *ms = MACHINE(qdev_get_machine());
+ cpu_type = strstr(ms->cpu_type, "power8");
+ if (cpu_type)
+ core_max_base = 0x1f8810;
+ for (int i = 0; i <= ms->smp.cores; i++)
+ if (addr == (core_max_base + i))
+ return true;
+ return false;
+}
+
+static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
+{
+ switch (addr) {
+ case 0xe2006: /* max pstate ultra turbo */
+ case 0xe2018: /* pstate id for 0 */
+ case 0x1f8001: /* P8 occ pstate version */
+ case 0x1f8003: /* P8 pstate min */
+ case 0x1f8010: /* P8 pstate id for 0 */
+ return 0;
+ case 0xe2000: /* occ data area */
+ case 0xe2002: /* occ_role master/slave*/
+ case 0xe2004: /* pstate nom */
+ case 0xe2005: /* pstate turbo */
+ case 0xe2020: /* pstate id for 1 */
+ case 0xe2818: /* pstate ultra turbo */
+ case 0xe2b85: /* opal dynamic data (runtime) */
+ case 0x1f8000: /* P8 occ pstate valid */
+ case 0x1f8002: /* P8 throttle */
+ case 0x1f8004: /* P8 pstate nom */
+ case 0x1f8005: /* P8 pstate turbo */
+ case 0x1f8012: /* vdd voltage identifier */
+ case 0x1f8013: /* vcs voltage identifier */
+ case 0x1f8018: /* P8 pstate id for 1 */
+ return 1;
+ case 0xe2003: /* pstate min (2 as pstate min) */
+ case 0xe2028: /* pstate id for 2 */
+ case 0x1f8006: /* P8 pstate ultra turbo */
+ case 0x1f8020: /* P8 pstate id for 2 */
+ return 2;
+ case 0xe2001: /* major version */
+ return 0x90;
+ /* 3000 khz frequency for 0, 1, and 2 pstates */
+ case 0xe201c:
+ case 0xe2024:
+ case 0xe202c:
+ /* P8 frequency for 0, 1, and 2 pstates */
+ case 0x1f8014:
+ case 0x1f801c:
+ case 0x1f8024:
+ return 3000;
+ case 0x0: /* homer base */
+ case 0xe2008: /* occ data area + 8 */
+ case 0x1f8008: /* P8 occ data area + 8 */
+ case 0x200008: /* homer base access to get homer image pointer*/
+ return 0x1000000000000000;
+ }
+ /* pstate table core max array */
+ if (core_max_array(addr))
+ return 1;
+ return 0;
+}
+
+static void homer_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned width)
+{
+ /* callback function defined to homer write */
+ return;
+}
+
+const MemoryRegionOps pnv_homer_ops = {
+ .read = homer_read,
+ .write = homer_write,
+ .valid.min_access_size = 1,
+ .valid.max_access_size = 8,
+ .impl.min_access_size = 1,
+ .impl.max_access_size = 8,
+ .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
+{
+ switch (addr) {
+ /*
+ * occ-sensor sanity check that asserts the sensor
+ * header block
+ */
+ case 0x580000: /* occ sensor data block */
+ case 0x580001: /* valid */
+ case 0x580002: /* version */
+ case 0x580004: /* reading_version */
+ case 0x580008: /* nr_sensors */
+ case 0x580010: /* names_offset */
+ case 0x580014: /* reading_ping_offset */
+ case 0x58000c: /* reading_pong_offset */
+ case 0x580023: /* structure_type */
+ return 1;
+ case 0x58000d: /* name length */
+ return 0x30;
+ case 0x580022: /* occ sensor loc core */
+ return 0x0040;
+ case 0x580003: /* occ sensor type power */
+ return 0x0080;
+ case 0x580005: /* sensor name */
+ return 0x1000;
+ case 0x58001e: /* HWMON_SENSORS_MASK */
+ case 0x580020:
+ return 0x8e00;
+ case 0x0: /* P8 slw base access for slw image size */
+ return 0x1000000000000000;
+ }
+ return 0;
+}
+
+static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned width)
+{
+ /* callback function defined to occ common area write */
+ return;
+}
+
+const MemoryRegionOps pnv_occ_common_area_ops = {
+ .read = occ_common_area_read,
+ .write = occ_common_area_write,
+ .valid.min_access_size = 1,
+ .valid.max_access_size = 8,
+ .impl.min_access_size = 1,
+ .impl.max_access_size = 8,
+ .endianness = DEVICE_BIG_ENDIAN,
+};
+
+void pnv_occ_common_area_realize(PnvChip *chip, Error **errp)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
+ sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS;
+ char *occ_common_area;
+
+ /* occ common area */
+ occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id);
+ memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip),
+ &pnv_occ_common_area_ops, chip, occ_common_area,
+ PNV_OCC_COMMON_AREA_SIZE);
+ sysbus_init_mmio(sbd, &chip->occ_common_area_mmio);
+ g_free(occ_common_area);
+}
+
+void pnv_homer_realize(PnvChip *chip, Error **errp)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
+ sbd->num_mmio = PNV_HOMER_SYSBUS;
+ char *homer;
+
+ /* homer region */
+ homer = g_strdup_printf("homer-%x", chip->chip_id);
+ memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops,
+ chip, homer, PNV_HOMER_SIZE);
+ sysbus_init_mmio(sbd, &chip->homer_mmio);
+ g_free(homer);
+}
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index fb123edc4e..6464e32892 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -28,6 +28,7 @@
#include "hw/ppc/pnv_occ.h"
#include "hw/ppc/pnv_xive.h"
#include "hw/ppc/pnv_core.h"
+#include "hw/ppc/pnv_homer.h"
#define TYPE_PNV_CHIP "pnv-chip"
#define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP)
@@ -36,6 +37,13 @@
#define PNV_CHIP_GET_CLASS(obj) \
OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP)
+enum SysBusNum {
+ PNV_XSCOM_SYSBUS,
+ PNV_ICP_SYSBUS,
+ PNV_HOMER_SYSBUS,
+ PNV_OCC_COMMON_AREA_SYSBUS,
+};
+
typedef enum PnvChipType {
PNV_CHIP_POWER8E, /* AKA Murano (default) */
PNV_CHIP_POWER8, /* AKA Venice */
@@ -56,6 +64,8 @@ typedef struct PnvChip {
uint64_t cores_mask;
void *cores;
+ MemoryRegion homer_mmio;
+ MemoryRegion occ_common_area_mmio;
MemoryRegion xscom_mmio;
MemoryRegion xscom;
AddressSpace xscom_as;
@@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv)
void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt);
void pnv_bmc_powerdown(IPMIBmc *bmc);
+extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp);
+extern void pnv_homer_realize(PnvChip *chip, Error **errp);
+
+
/*
* POWER8 MMIO base addresses
*/
diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
new file mode 100644
index 0000000000..0fe6469abe
--- /dev/null
+++ b/include/hw/ppc/pnv_homer.h
@@ -0,0 +1,41 @@
+/*
+ * QEMU PowerPC PowerNV Homer and occ common area definitions
+ *
+ * Copyright (c) 2019, IBM Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _PPC_PNV_HOMER_H
+#define _PPC_PNV_HOMER_H
+
+#include "qom/object.h"
+
+/*
+ * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip)
+ * so chip_num can be used to offset between HOMER region from its base address
+ */
+#define PNV_HOMER_SIZE 0x300000
+#define PNV_OCC_COMMON_AREA_SIZE 0x700000
+
+#define PNV_HOMER_BASE(chip) \
+ (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
+#define PNV_OCC_COMMON_AREA(chip) \
+ (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
+
+#define PNV9_HOMER_BASE(chip) \
+ (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
+#define PNV9_OCC_COMMON_AREA(chip) \
+ (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
+
+#endif /* _PPC_PNV_HOMER_H */
--
2.14.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV Balamuruhan S
@ 2019-08-07 7:54 ` Cédric Le Goater
2019-08-07 10:07 ` Balamuruhan S
2019-08-09 4:44 ` David Gibson
0 siblings, 2 replies; 45+ messages in thread
From: Cédric Le Goater @ 2019-08-07 7:54 UTC (permalink / raw)
To: Balamuruhan S, qemu-devel; +Cc: pbonzini, maddy, anju, david, hari
On 07/08/2019 09:14, Balamuruhan S wrote:
> Add mmio callback functions to enable homer/occ common area
> to emulate pstate table, occ-sensors, slw, occ static and
> dynamic values for Power8 and Power9 chips. It also works for
> multiple chips as offset remains the same whereas the base
> address are handled appropriately while initializing device
> tree.
>
> currently skiboot disables the homer/occ code path with
> `QUIRK_NO_PBA`, this quirk have to be removed in skiboot
> for it to use this infrastructure.
I think this patch can come before the others as it is adding
support without the python extra facilities.
Some comments below,
> Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> hw/ppc/Makefile.objs | 2 +-
> hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++
> include/hw/ppc/pnv.h | 14 ++++
> include/hw/ppc/pnv_homer.h | 41 ++++++++++
> 4 files changed, 241 insertions(+), 1 deletion(-)
> create mode 100644 hw/ppc/pnv_homer.c
> create mode 100644 include/hw/ppc/pnv_homer.h
>
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 9da93af905..7260b4a96c 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
> obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
> obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o
> # IBM PowerNV
> -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o
add an extra line.
> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
> endif
> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> new file mode 100644
> index 0000000000..73a94856d0
> --- /dev/null
> +++ b/hw/ppc/pnv_homer.c
> @@ -0,0 +1,185 @@
> +/*
> + * QEMU PowerPC PowerNV Homer and OCC common area region
> + *
> + * Copyright (c) 2019, IBM Corporation.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +#include "sysemu/hw_accel.h"
> +#include "sysemu/cpus.h"
> +#include "hw/ppc/pnv.h"
> +
> +static bool core_max_array(hwaddr addr)
> +{
> + char *cpu_type;
> + hwaddr core_max_base = 0xe2819;
What is this representing ?
> + MachineState *ms = MACHINE(qdev_get_machine());
> + cpu_type = strstr(ms->cpu_type, "power8");
you need to get this information some other way. The PnvChip should have it.
> + if (cpu_type)
> + core_max_base = 0x1f8810;
It could be a PnvChipClass value.
> + for (int i = 0; i <= ms->smp.cores; i++)
> + if (addr == (core_max_base + i))
> + return true;
> + return false;
> +}
> +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
> +{
> + switch (addr) {
We should be using defines for the case statements below.
Are we accessing one or more structures which are mapped at specific
addresses ? If so I would define them in this file and change the
memory ops to use well known offsets.
Are these structures the same on P9 and P8 ?
Are there default values ? May be we could use a reset handler
in this case.
> + case 0xe2006: /* max pstate ultra turbo */
> + case 0xe2018: /* pstate id for 0 */
> + case 0x1f8001: /* P8 occ pstate version */
> + case 0x1f8003: /* P8 pstate min */
> + case 0x1f8010: /* P8 pstate id for 0 */
> + return 0;
> + case 0xe2000: /* occ data area */
> + case 0xe2002: /* occ_role master/slave*/
> + case 0xe2004: /* pstate nom */
> + case 0xe2005: /* pstate turbo */
> + case 0xe2020: /* pstate id for 1 */
> + case 0xe2818: /* pstate ultra turbo */
> + case 0xe2b85: /* opal dynamic data (runtime) */
> + case 0x1f8000: /* P8 occ pstate valid */
> + case 0x1f8002: /* P8 throttle */
> + case 0x1f8004: /* P8 pstate nom */
> + case 0x1f8005: /* P8 pstate turbo */
> + case 0x1f8012: /* vdd voltage identifier */
> + case 0x1f8013: /* vcs voltage identifier */
> + case 0x1f8018: /* P8 pstate id for 1 */
> + return 1;
> + case 0xe2003: /* pstate min (2 as pstate min) */
> + case 0xe2028: /* pstate id for 2 */
> + case 0x1f8006: /* P8 pstate ultra turbo */
> + case 0x1f8020: /* P8 pstate id for 2 */
> + return 2;
> + case 0xe2001: /* major version */
> + return 0x90;
> + /* 3000 khz frequency for 0, 1, and 2 pstates */
> + case 0xe201c:
> + case 0xe2024:
> + case 0xe202c:
> + /* P8 frequency for 0, 1, and 2 pstates */
> + case 0x1f8014:
> + case 0x1f801c:
> + case 0x1f8024:
> + return 3000;
> + case 0x0: /* homer base */
> + case 0xe2008: /* occ data area + 8 */
> + case 0x1f8008: /* P8 occ data area + 8 */
> + case 0x200008: /* homer base access to get homer image pointer*/
> + return 0x1000000000000000;
> + }
> + /* pstate table core max array */
> + if (core_max_array(addr))
> + return 1;
I don't understand what the core_max_array is returning
> + return 0;
> +}
> +
> +static void homer_write(void *opaque, hwaddr addr, uint64_t val,
> + unsigned width)
> +{
> + /* callback function defined to homer write */
> + return;
> +}
> +
> +const MemoryRegionOps pnv_homer_ops = {
> + .read = homer_read,
> + .write = homer_write,
> + .valid.min_access_size = 1,
> + .valid.max_access_size = 8,
> + .impl.min_access_size = 1,
> + .impl.max_access_size = 8,
> + .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
> +{
> + switch (addr) {
> + /*
> + * occ-sensor sanity check that asserts the sensor
> + * header block
> + */
Same comments as above.
> + case 0x580000: /* occ sensor data block */
> + case 0x580001: /* valid */
> + case 0x580002: /* version */
> + case 0x580004: /* reading_version */
> + case 0x580008: /* nr_sensors */
> + case 0x580010: /* names_offset */
> + case 0x580014: /* reading_ping_offset */
> + case 0x58000c: /* reading_pong_offset */
> + case 0x580023: /* structure_type */
> + return 1;
> + case 0x58000d: /* name length */
> + return 0x30;
> + case 0x580022: /* occ sensor loc core */
> + return 0x0040;
> + case 0x580003: /* occ sensor type power */
> + return 0x0080;
> + case 0x580005: /* sensor name */
> + return 0x1000;
> + case 0x58001e: /* HWMON_SENSORS_MASK */
> + case 0x580020:
> + return 0x8e00;
> + case 0x0: /* P8 slw base access for slw image size */
> + return 0x1000000000000000;
> + }
> + return 0;
> +}
> +
> +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val,
> + unsigned width)
> +{
> + /* callback function defined to occ common area write */
> + return;
> +}
> +
> +const MemoryRegionOps pnv_occ_common_area_ops = {
> + .read = occ_common_area_read,
> + .write = occ_common_area_write,
> + .valid.min_access_size = 1,
> + .valid.max_access_size = 8,
> + .impl.min_access_size = 1,
> + .impl.max_access_size = 8,
> + .endianness = DEVICE_BIG_ENDIAN,
> +};
Why aren't you using the PnvOCC model ?
> +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS;
> + char *occ_common_area;
> +
> + /* occ common area */
> + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id);
> + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip),
> + &pnv_occ_common_area_ops, chip, occ_common_area,
> + PNV_OCC_COMMON_AREA_SIZE);
> + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio);
> + g_free(occ_common_area);
> +}
May be this "device" deserves a PnvHomer model, one for P8 and one for P9.
> +void pnv_homer_realize(PnvChip *chip, Error **errp)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> + sbd->num_mmio = PNV_HOMER_SYSBUS;
> + char *homer;
> +
> + /* homer region */
> + homer = g_strdup_printf("homer-%x", chip->chip_id);
> + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops,
> + chip, homer, PNV_HOMER_SIZE);
> + sysbus_init_mmio(sbd, &chip->homer_mmio);
> + g_free(homer);
> +}
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index fb123edc4e..6464e32892 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -28,6 +28,7 @@
> #include "hw/ppc/pnv_occ.h"
> #include "hw/ppc/pnv_xive.h"
> #include "hw/ppc/pnv_core.h"
> +#include "hw/ppc/pnv_homer.h"
>
> #define TYPE_PNV_CHIP "pnv-chip"
> #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP)
> @@ -36,6 +37,13 @@
> #define PNV_CHIP_GET_CLASS(obj) \
> OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP)
>
> +enum SysBusNum {
> + PNV_XSCOM_SYSBUS,
> + PNV_ICP_SYSBUS,
> + PNV_HOMER_SYSBUS,
> + PNV_OCC_COMMON_AREA_SYSBUS,
> +};
What is this ?
> typedef enum PnvChipType {
> PNV_CHIP_POWER8E, /* AKA Murano (default) */
> PNV_CHIP_POWER8, /* AKA Venice */
> @@ -56,6 +64,8 @@ typedef struct PnvChip {
> uint64_t cores_mask;
> void *cores;
>
> + MemoryRegion homer_mmio;
> + MemoryRegion occ_common_area_mmio;
> MemoryRegion xscom_mmio;
> MemoryRegion xscom;
> AddressSpace xscom_as;
> @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv)
> void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt);
> void pnv_bmc_powerdown(IPMIBmc *bmc);
>
> +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp);
> +extern void pnv_homer_realize(PnvChip *chip, Error **errp);
> +
> +
> /*
> * POWER8 MMIO base addresses
> */
> diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
> new file mode 100644
> index 0000000000..0fe6469abe
> --- /dev/null
> +++ b/include/hw/ppc/pnv_homer.h
> @@ -0,0 +1,41 @@
> +/*
> + * QEMU PowerPC PowerNV Homer and occ common area definitions
> + *
> + * Copyright (c) 2019, IBM Corporation.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef _PPC_PNV_HOMER_H
> +#define _PPC_PNV_HOMER_H
> +
> +#include "qom/object.h"
> +
> +/*
> + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip)
> + * so chip_num can be used to offset between HOMER region from its base address
> + */
> +#define PNV_HOMER_SIZE 0x300000
> +#define PNV_OCC_COMMON_AREA_SIZE 0x700000
> +
> +#define PNV_HOMER_BASE(chip) \
> + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
> +#define PNV_OCC_COMMON_AREA(chip) \
> + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
> +
> +#define PNV9_HOMER_BASE(chip) \
> + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
> +#define PNV9_OCC_COMMON_AREA(chip) \
> + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
> +
> +#endif /* _PPC_PNV_HOMER_H */
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV
2019-08-07 7:54 ` Cédric Le Goater
@ 2019-08-07 10:07 ` Balamuruhan S
2019-08-08 8:32 ` Cédric Le Goater
2019-08-09 4:44 ` David Gibson
1 sibling, 1 reply; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 10:07 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: hari, anju, qemu-devel, maddy, pbonzini, david
On Wed, Aug 07, 2019 at 09:54:55AM +0200, Cédric Le Goater wrote:
> On 07/08/2019 09:14, Balamuruhan S wrote:
> > Add mmio callback functions to enable homer/occ common area
> > to emulate pstate table, occ-sensors, slw, occ static and
> > dynamic values for Power8 and Power9 chips. It also works for
> > multiple chips as offset remains the same whereas the base
> > address are handled appropriately while initializing device
> > tree.
> >
> > currently skiboot disables the homer/occ code path with
> > `QUIRK_NO_PBA`, this quirk have to be removed in skiboot
> > for it to use this infrastructure.
>
>
> I think this patch can come before the others as it is adding
> support without the python extra facilities.
Okay sure.
>
> Some comments below,
>
> > Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com>
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> > hw/ppc/Makefile.objs | 2 +-
> > hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/ppc/pnv.h | 14 ++++
> > include/hw/ppc/pnv_homer.h | 41 ++++++++++
> > 4 files changed, 241 insertions(+), 1 deletion(-)
> > create mode 100644 hw/ppc/pnv_homer.c
> > create mode 100644 include/hw/ppc/pnv_homer.h
> >
> > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > index 9da93af905..7260b4a96c 100644
> > --- a/hw/ppc/Makefile.objs
> > +++ b/hw/ppc/Makefile.objs
> > @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
> > obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
> > obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o
> > # IBM PowerNV
> > -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
> > +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o
>
> add an extra line.
I will do that.
>
> > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> > obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
> > endif
> > diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> > new file mode 100644
> > index 0000000000..73a94856d0
> > --- /dev/null
> > +++ b/hw/ppc/pnv_homer.c
> > @@ -0,0 +1,185 @@
> > +/*
> > + * QEMU PowerPC PowerNV Homer and OCC common area region
> > + *
> > + * Copyright (c) 2019, IBM Corporation.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#include "qemu/osdep.h"
> > +#include "sysemu/hw_accel.h"
> > +#include "sysemu/cpus.h"
> > +#include "hw/ppc/pnv.h"
> > +
> > +static bool core_max_array(hwaddr addr)
> > +{
> > + char *cpu_type;
> > + hwaddr core_max_base = 0xe2819;
>
> What is this representing ?
>
> > + MachineState *ms = MACHINE(qdev_get_machine());
> > + cpu_type = strstr(ms->cpu_type, "power8");
>
> you need to get this information some other way. The PnvChip should have it.
okay I will use it, I think you mean PnvChipType chip_type.
>
> > + if (cpu_type)
> > + core_max_base = 0x1f8810;
>
> It could be a PnvChipClass value.
or should we also consider it also as macro ?
>
> > + for (int i = 0; i <= ms->smp.cores; i++)
> > + if (addr == (core_max_base + i))
> > + return true;
> > + return false;
> > +}
>
>
> > +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
> > +{
> > + switch (addr) {
>
> We should be using defines for the case statements below.
>
> Are we accessing one or more structures which are mapped at specific
> addresses ? If so I would define them in this file and change the
> memory ops to use well known offsets.
okay, but lot of defines have to be done, will that be fine ?
>
> Are these structures the same on P9 and P8 ?
no, stuctures are different on P9 and P8, occ pstate table version 0x90 is for
P9 and 0x01, 0x02 is for P8.
I have mentioned P8 specific offsets in comments and rest are P9.
>
> Are there default values ? May be we could use a reset handler
> in this case.
yes, I tried to return default expected values for skiboot. I am
not aware of reset handler, where can I check it ?
>
> > + case 0xe2006: /* max pstate ultra turbo */
> > + case 0xe2018: /* pstate id for 0 */
> > + case 0x1f8001: /* P8 occ pstate version */
> > + case 0x1f8003: /* P8 pstate min */
> > + case 0x1f8010: /* P8 pstate id for 0 */
> > + return 0;
> > + case 0xe2000: /* occ data area */
> > + case 0xe2002: /* occ_role master/slave*/
> > + case 0xe2004: /* pstate nom */
> > + case 0xe2005: /* pstate turbo */
> > + case 0xe2020: /* pstate id for 1 */
> > + case 0xe2818: /* pstate ultra turbo */
> > + case 0xe2b85: /* opal dynamic data (runtime) */
> > + case 0x1f8000: /* P8 occ pstate valid */
> > + case 0x1f8002: /* P8 throttle */
> > + case 0x1f8004: /* P8 pstate nom */
> > + case 0x1f8005: /* P8 pstate turbo */
> > + case 0x1f8012: /* vdd voltage identifier */
> > + case 0x1f8013: /* vcs voltage identifier */
> > + case 0x1f8018: /* P8 pstate id for 1 */
> > + return 1;
> > + case 0xe2003: /* pstate min (2 as pstate min) */
> > + case 0xe2028: /* pstate id for 2 */
> > + case 0x1f8006: /* P8 pstate ultra turbo */
> > + case 0x1f8020: /* P8 pstate id for 2 */
> > + return 2;
> > + case 0xe2001: /* major version */
> > + return 0x90;
> > + /* 3000 khz frequency for 0, 1, and 2 pstates */
> > + case 0xe201c:
> > + case 0xe2024:
> > + case 0xe202c:
> > + /* P8 frequency for 0, 1, and 2 pstates */
> > + case 0x1f8014:
> > + case 0x1f801c:
> > + case 0x1f8024:
> > + return 3000;
> > + case 0x0: /* homer base */
> > + case 0xe2008: /* occ data area + 8 */
> > + case 0x1f8008: /* P8 occ data area + 8 */
> > + case 0x200008: /* homer base access to get homer image pointer*/
> > + return 0x1000000000000000;
> > + }
> > + /* pstate table core max array */
> > + if (core_max_array(addr))
> > + return 1;
>
> I don't understand what the core_max_array is returning
core_max_array defaults to nominal pstate 1 as maximum sustainable
pstate. please advise if you think it is not right.
>
> > + return 0;
> > +}
> > +
> > +static void homer_write(void *opaque, hwaddr addr, uint64_t val,
> > + unsigned width)
> > +{
> > + /* callback function defined to homer write */
> > + return;
> > +}
> > +
> > +const MemoryRegionOps pnv_homer_ops = {
> > + .read = homer_read,
> > + .write = homer_write,
> > + .valid.min_access_size = 1,
> > + .valid.max_access_size = 8,
> > + .impl.min_access_size = 1,
> > + .impl.max_access_size = 8,
> > + .endianness = DEVICE_BIG_ENDIAN,
> > +};
> > +
> > +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
> > +{
> > + switch (addr) {
> > + /*
> > + * occ-sensor sanity check that asserts the sensor
> > + * header block
> > + */
>
> Same comments as above.
okay, will do.
>
> > + case 0x580000: /* occ sensor data block */
> > + case 0x580001: /* valid */
> > + case 0x580002: /* version */
> > + case 0x580004: /* reading_version */
> > + case 0x580008: /* nr_sensors */
> > + case 0x580010: /* names_offset */
> > + case 0x580014: /* reading_ping_offset */
> > + case 0x58000c: /* reading_pong_offset */
> > + case 0x580023: /* structure_type */
> > + return 1;
> > + case 0x58000d: /* name length */
> > + return 0x30;
> > + case 0x580022: /* occ sensor loc core */
> > + return 0x0040;
> > + case 0x580003: /* occ sensor type power */
> > + return 0x0080;
> > + case 0x580005: /* sensor name */
> > + return 0x1000;
> > + case 0x58001e: /* HWMON_SENSORS_MASK */
> > + case 0x580020:
> > + return 0x8e00;
> > + case 0x0: /* P8 slw base access for slw image size */
> > + return 0x1000000000000000;
> > + }
> > + return 0;
> > +}
> > +
> > +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val,
> > + unsigned width)
> > +{
> > + /* callback function defined to occ common area write */
> > + return;
> > +}
> > +
> > +const MemoryRegionOps pnv_occ_common_area_ops = {
> > + .read = occ_common_area_read,
> > + .write = occ_common_area_write,
> > + .valid.min_access_size = 1,
> > + .valid.max_access_size = 8,
> > + .impl.min_access_size = 1,
> > + .impl.max_access_size = 8,
> > + .endianness = DEVICE_BIG_ENDIAN,
> > +};
>
>
> Why aren't you using the PnvOCC model ?
I was not aware of how to use it here, I will check it to make use
of it.
>
> > +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp)
> > +{
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> > + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS;
> > + char *occ_common_area;
> > +
> > + /* occ common area */
> > + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id);
> > + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip),
> > + &pnv_occ_common_area_ops, chip, occ_common_area,
> > + PNV_OCC_COMMON_AREA_SIZE);
> > + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio);
> > + g_free(occ_common_area);
> > +}
>
>
> May be this "device" deserves a PnvHomer model, one for P8 and one for P9.
:+1:
>
> > +void pnv_homer_realize(PnvChip *chip, Error **errp)
> > +{
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> > + sbd->num_mmio = PNV_HOMER_SYSBUS;
> > + char *homer;
> > +
> > + /* homer region */
> > + homer = g_strdup_printf("homer-%x", chip->chip_id);
> > + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops,
> > + chip, homer, PNV_HOMER_SIZE);
> > + sysbus_init_mmio(sbd, &chip->homer_mmio);
> > + g_free(homer);
> > +}
> > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > index fb123edc4e..6464e32892 100644
> > --- a/include/hw/ppc/pnv.h
> > +++ b/include/hw/ppc/pnv.h
> > @@ -28,6 +28,7 @@
> > #include "hw/ppc/pnv_occ.h"
> > #include "hw/ppc/pnv_xive.h"
> > #include "hw/ppc/pnv_core.h"
> > +#include "hw/ppc/pnv_homer.h"
> >
> > #define TYPE_PNV_CHIP "pnv-chip"
> > #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP)
> > @@ -36,6 +37,13 @@
> > #define PNV_CHIP_GET_CLASS(obj) \
> > OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP)
> >
> > +enum SysBusNum {
> > + PNV_XSCOM_SYSBUS,
> > + PNV_ICP_SYSBUS,
> > + PNV_HOMER_SYSBUS,
> > + PNV_OCC_COMMON_AREA_SYSBUS,
> > +};
>
> What is this ?
enumeration for SysBusDevice device init, it is needed for mapping to
work, please correct me if it is not the right way.
>
>
> > typedef enum PnvChipType {
> > PNV_CHIP_POWER8E, /* AKA Murano (default) */
> > PNV_CHIP_POWER8, /* AKA Venice */
> > @@ -56,6 +64,8 @@ typedef struct PnvChip {
> > uint64_t cores_mask;
> > void *cores;
> >
> > + MemoryRegion homer_mmio;
> > + MemoryRegion occ_common_area_mmio;
> > MemoryRegion xscom_mmio;
> > MemoryRegion xscom;
> > AddressSpace xscom_as;
> > @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv)
> > void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt);
> > void pnv_bmc_powerdown(IPMIBmc *bmc);
> >
> > +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp);
> > +extern void pnv_homer_realize(PnvChip *chip, Error **errp);
> > +
> > +
> > /*
> > * POWER8 MMIO base addresses
> > */
> > diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
> > new file mode 100644
> > index 0000000000..0fe6469abe
> > --- /dev/null
> > +++ b/include/hw/ppc/pnv_homer.h
> > @@ -0,0 +1,41 @@
> > +/*
> > + * QEMU PowerPC PowerNV Homer and occ common area definitions
> > + *
> > + * Copyright (c) 2019, IBM Corporation.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#ifndef _PPC_PNV_HOMER_H
> > +#define _PPC_PNV_HOMER_H
> > +
> > +#include "qom/object.h"
> > +
> > +/*
> > + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip)
> > + * so chip_num can be used to offset between HOMER region from its base address
> > + */
> > +#define PNV_HOMER_SIZE 0x300000
> > +#define PNV_OCC_COMMON_AREA_SIZE 0x700000
> > +
> > +#define PNV_HOMER_BASE(chip) \
> > + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
> > +#define PNV_OCC_COMMON_AREA(chip) \
> > + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
> > +
> > +#define PNV9_HOMER_BASE(chip) \
> > + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
> > +#define PNV9_OCC_COMMON_AREA(chip) \
> > + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
> > +
> > +#endif /* _PPC_PNV_HOMER_H */
> >
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV
2019-08-07 10:07 ` Balamuruhan S
@ 2019-08-08 8:32 ` Cédric Le Goater
0 siblings, 0 replies; 45+ messages in thread
From: Cédric Le Goater @ 2019-08-08 8:32 UTC (permalink / raw)
To: Balamuruhan S; +Cc: hari, anju, qemu-devel, maddy, pbonzini, david
On 07/08/2019 12:07, Balamuruhan S wrote:
> On Wed, Aug 07, 2019 at 09:54:55AM +0200, Cédric Le Goater wrote:
>> On 07/08/2019 09:14, Balamuruhan S wrote:
>>> Add mmio callback functions to enable homer/occ common area
>>> to emulate pstate table, occ-sensors, slw, occ static and
>>> dynamic values for Power8 and Power9 chips. It also works for
>>> multiple chips as offset remains the same whereas the base
>>> address are handled appropriately while initializing device
>>> tree.
>>>
>>> currently skiboot disables the homer/occ code path with
>>> `QUIRK_NO_PBA`, this quirk have to be removed in skiboot
>>> for it to use this infrastructure.
>>
>>
>> I think this patch can come before the others as it is adding
>> support without the python extra facilities.
>
> Okay sure.
>
>>
>> Some comments below,
>>
>>> Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com>
>>> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
>>> ---
>>> hw/ppc/Makefile.objs | 2 +-
>>> hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++
>>> include/hw/ppc/pnv.h | 14 ++++
>>> include/hw/ppc/pnv_homer.h | 41 ++++++++++
>>> 4 files changed, 241 insertions(+), 1 deletion(-)
>>> create mode 100644 hw/ppc/pnv_homer.c
>>> create mode 100644 include/hw/ppc/pnv_homer.h
>>>
>>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
>>> index 9da93af905..7260b4a96c 100644
>>> --- a/hw/ppc/Makefile.objs
>>> +++ b/hw/ppc/Makefile.objs
>>> @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
>>> obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
>>> obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o
>>> # IBM PowerNV
>>> -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
>>> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o
>>
>> add an extra line.
>
> I will do that.
>
>>
>>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>>> obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
>>> endif
>>> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
>>> new file mode 100644
>>> index 0000000000..73a94856d0
>>> --- /dev/null
>>> +++ b/hw/ppc/pnv_homer.c
>>> @@ -0,0 +1,185 @@
>>> +/*
>>> + * QEMU PowerPC PowerNV Homer and OCC common area region
>>> + *
>>> + * Copyright (c) 2019, IBM Corporation.
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +#include "qemu/osdep.h"
>>> +#include "sysemu/hw_accel.h"
>>> +#include "sysemu/cpus.h"
>>> +#include "hw/ppc/pnv.h"
>>> +
>>> +static bool core_max_array(hwaddr addr)
>>> +{
>>> + char *cpu_type;
>>> + hwaddr core_max_base = 0xe2819;
>>
>> What is this representing ?
>>
>>> + MachineState *ms = MACHINE(qdev_get_machine());
>>> + cpu_type = strstr(ms->cpu_type, "power8");
>>
>> you need to get this information some other way. The PnvChip should have it.
>
> okay I will use it, I think you mean PnvChipType chip_type.
you will need to fetch this information from some model instance or
class, PnvChip or a new one.
>>
>>> + if (cpu_type)
>>> + core_max_base = 0x1f8810;
>>
>> It could be a PnvChipClass value.
>
> or should we also consider it also as macro ?
it's a chip constant ? see the pnv_chip_*_class_init routine.
>>
>>> + for (int i = 0; i <= ms->smp.cores; i++)
>>> + if (addr == (core_max_base + i))
>>> + return true;
>>> + return false;
>>> +}
>>
>>
>>> +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
>>> +{
>>> + switch (addr) {
>>
>> We should be using defines for the case statements below.
>>
>> Are we accessing one or more structures which are mapped at specific
>> addresses ? If so I would define them in this file and change the
>> memory ops to use well known offsets.
>
> okay, but lot of defines have to be done, will that be fine ?
Not a problem. You can copy the definitions and the structures from
skiboot and adapt them to QEMU coding style. Look at the other Pnv
models PSI, XIVE, LPC.
>
>>
>> Are these structures the same on P9 and P8 ?
>
> no, stuctures are different on P9 and P8, occ pstate table version 0x90 is for
> P9 and 0x01, 0x02 is for P8.
These are class constants. most probably a new PnvXScom/PnvHomer or
PnvOCC
> I have mentioned P8 specific offsets in comments and rest are P9.
>
>>
>> Are there default values ? May be we could use a reset handler
>> in this case.
>
> yes, I tried to return default expected values for skiboot. I am
> not aware of reset handler, where can I check it ?
You need to introduce a new model and add a reset handler for it, or
use an existing one. I am not sure we need a PnvHomer model but we might
need to add PnvXscom and add the HOMER stuff into it.
>>> + case 0xe2006: /* max pstate ultra turbo */
>>> + case 0xe2018: /* pstate id for 0 */
>>> + case 0x1f8001: /* P8 occ pstate version */
>>> + case 0x1f8003: /* P8 pstate min */
>>> + case 0x1f8010: /* P8 pstate id for 0 */
>>> + return 0;
>>> + case 0xe2000: /* occ data area */
>>> + case 0xe2002: /* occ_role master/slave*/
>>> + case 0xe2004: /* pstate nom */
>>> + case 0xe2005: /* pstate turbo */
>>> + case 0xe2020: /* pstate id for 1 */
>>> + case 0xe2818: /* pstate ultra turbo */
>>> + case 0xe2b85: /* opal dynamic data (runtime) */
>>> + case 0x1f8000: /* P8 occ pstate valid */
>>> + case 0x1f8002: /* P8 throttle */
>>> + case 0x1f8004: /* P8 pstate nom */
>>> + case 0x1f8005: /* P8 pstate turbo */
>>> + case 0x1f8012: /* vdd voltage identifier */
>>> + case 0x1f8013: /* vcs voltage identifier */
>>> + case 0x1f8018: /* P8 pstate id for 1 */
>>> + return 1;
>>> + case 0xe2003: /* pstate min (2 as pstate min) */
>>> + case 0xe2028: /* pstate id for 2 */
>>> + case 0x1f8006: /* P8 pstate ultra turbo */
>>> + case 0x1f8020: /* P8 pstate id for 2 */
>>> + return 2;
>>> + case 0xe2001: /* major version */
>>> + return 0x90;
>>> + /* 3000 khz frequency for 0, 1, and 2 pstates */
>>> + case 0xe201c:
>>> + case 0xe2024:
>>> + case 0xe202c:
>>> + /* P8 frequency for 0, 1, and 2 pstates */
>>> + case 0x1f8014:
>>> + case 0x1f801c:
>>> + case 0x1f8024:
>>> + return 3000;
>>> + case 0x0: /* homer base */
>>> + case 0xe2008: /* occ data area + 8 */
>>> + case 0x1f8008: /* P8 occ data area + 8 */
>>> + case 0x200008: /* homer base access to get homer image pointer*/
>>> + return 0x1000000000000000;
>>> + }
>>> + /* pstate table core max array */
>>> + if (core_max_array(addr))
>>> + return 1;
>>
>> I don't understand what the core_max_array is returning
>
> core_max_array defaults to nominal pstate 1 as maximum sustainable
> pstate. please advise if you think it is not right.
I still do not understand what this is about :/
>
>>
>>> + return 0;
>>> +}
>>> +
>>> +static void homer_write(void *opaque, hwaddr addr, uint64_t val,
>>> + unsigned width)
>>> +{
>>> + /* callback function defined to homer write */
>>> + return;
>>> +}
>>> +
>>> +const MemoryRegionOps pnv_homer_ops = {
>>> + .read = homer_read,
>>> + .write = homer_write,
>>> + .valid.min_access_size = 1,
>>> + .valid.max_access_size = 8,
>>> + .impl.min_access_size = 1,
>>> + .impl.max_access_size = 8,
>>> + .endianness = DEVICE_BIG_ENDIAN,
>>> +};
>>> +
>>> +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
>>> +{
>>> + switch (addr) {
>>> + /*
>>> + * occ-sensor sanity check that asserts the sensor
>>> + * header block
>>> + */
>>
>> Same comments as above.
>
> okay, will do.
>>
>>> + case 0x580000: /* occ sensor data block */
>>> + case 0x580001: /* valid */
>>> + case 0x580002: /* version */
>>> + case 0x580004: /* reading_version */
>>> + case 0x580008: /* nr_sensors */
>>> + case 0x580010: /* names_offset */
>>> + case 0x580014: /* reading_ping_offset */
>>> + case 0x58000c: /* reading_pong_offset */
>>> + case 0x580023: /* structure_type */
>>> + return 1;
>>> + case 0x58000d: /* name length */
>>> + return 0x30;
>>> + case 0x580022: /* occ sensor loc core */
>>> + return 0x0040;
>>> + case 0x580003: /* occ sensor type power */
>>> + return 0x0080;
>>> + case 0x580005: /* sensor name */
>>> + return 0x1000;
>>> + case 0x58001e: /* HWMON_SENSORS_MASK */
>>> + case 0x580020:
>>> + return 0x8e00;
>>> + case 0x0: /* P8 slw base access for slw image size */
>>> + return 0x1000000000000000;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val,
>>> + unsigned width)
>>> +{
>>> + /* callback function defined to occ common area write */
>>> + return;
>>> +}
>>> +
>>> +const MemoryRegionOps pnv_occ_common_area_ops = {
>>> + .read = occ_common_area_read,
>>> + .write = occ_common_area_write,
>>> + .valid.min_access_size = 1,
>>> + .valid.max_access_size = 8,
>>> + .impl.min_access_size = 1,
>>> + .impl.max_access_size = 8,
>>> + .endianness = DEVICE_BIG_ENDIAN,
>>> +};
>>
>>
>> Why aren't you using the PnvOCC model ?
>
> I was not aware of how to use it here, I will check it to make use
> of it.
Yes. Check how the machine and the chips are defined in pnv.c and then
the other logic units, LPC, XIVE, OCC, etc. The XSCOM bus does not have
a model of its own because it is limited to an address space. We might
need to introduce a PnvXscom model and put the HOMER info there.
>
>>
>>> +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp)
>>> +{
>>> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
>>> + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS;
>>> + char *occ_common_area;
>>> +
>>> + /* occ common area */
>>> + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id);
>>> + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip),
>>> + &pnv_occ_common_area_ops, chip, occ_common_area,
>>> + PNV_OCC_COMMON_AREA_SIZE);
>>> + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio);
>>> + g_free(occ_common_area);
>>> +}
>>
>>
>> May be this "device" deserves a PnvHomer model, one for P8 and one for P9.
>
> :+1:
For OCC stuff, please try to extend the OCC model with a new region mapping
the structures of the SRAM that the chip uses for sensors, pstate, etc.
>>
>>> +void pnv_homer_realize(PnvChip *chip, Error **errp)
>>> +{
>>> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
>>> + sbd->num_mmio = PNV_HOMER_SYSBUS;
>>> + char *homer;
>>> +
>>> + /* homer region */
>>> + homer = g_strdup_printf("homer-%x", chip->chip_id);
>>> + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops,
>>> + chip, homer, PNV_HOMER_SIZE);
>>> + sysbus_init_mmio(sbd, &chip->homer_mmio);
>>> + g_free(homer);
>>> +}
>>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
>>> index fb123edc4e..6464e32892 100644
>>> --- a/include/hw/ppc/pnv.h
>>> +++ b/include/hw/ppc/pnv.h
>>> @@ -28,6 +28,7 @@
>>> #include "hw/ppc/pnv_occ.h"
>>> #include "hw/ppc/pnv_xive.h"
>>> #include "hw/ppc/pnv_core.h"
>>> +#include "hw/ppc/pnv_homer.h"
>>>
>>> #define TYPE_PNV_CHIP "pnv-chip"
>>> #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP)
>>> @@ -36,6 +37,13 @@
>>> #define PNV_CHIP_GET_CLASS(obj) \
>>> OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP)
>>>
>>> +enum SysBusNum {
>>> + PNV_XSCOM_SYSBUS,
>>> + PNV_ICP_SYSBUS,
>>> + PNV_HOMER_SYSBUS,
>>> + PNV_OCC_COMMON_AREA_SYSBUS,
>>> +};
>>
>> What is this ?
>
> enumeration for SysBusDevice device init, it is needed for mapping to
> work, please correct me if it is not the right way.
I think you can add the HOMER memory region in a new PnvXscom model.
the SRAM memory region can go in the OCC model.
We need to check how all these models interacts.
Thanks,
C.
>
>>
>>
>>> typedef enum PnvChipType {
>>> PNV_CHIP_POWER8E, /* AKA Murano (default) */
>>> PNV_CHIP_POWER8, /* AKA Venice */
>>> @@ -56,6 +64,8 @@ typedef struct PnvChip {
>>> uint64_t cores_mask;
>>> void *cores;
>>>
>>> + MemoryRegion homer_mmio;
>>> + MemoryRegion occ_common_area_mmio;
>>> MemoryRegion xscom_mmio;
>>> MemoryRegion xscom;
>>> AddressSpace xscom_as;
>>> @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv)
>>> void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt);
>>> void pnv_bmc_powerdown(IPMIBmc *bmc);
>>>
>>> +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp);
>>> +extern void pnv_homer_realize(PnvChip *chip, Error **errp);
>>> +
>>> +
>>> /*
>>> * POWER8 MMIO base addresses
>>> */
>>> diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
>>> new file mode 100644
>>> index 0000000000..0fe6469abe
>>> --- /dev/null
>>> +++ b/include/hw/ppc/pnv_homer.h
>>> @@ -0,0 +1,41 @@
>>> +/*
>>> + * QEMU PowerPC PowerNV Homer and occ common area definitions
>>> + *
>>> + * Copyright (c) 2019, IBM Corporation.
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +#ifndef _PPC_PNV_HOMER_H
>>> +#define _PPC_PNV_HOMER_H
>>> +
>>> +#include "qom/object.h"
>>> +
>>> +/*
>>> + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip)
>>> + * so chip_num can be used to offset between HOMER region from its base address
>>> + */
>>> +#define PNV_HOMER_SIZE 0x300000
>>> +#define PNV_OCC_COMMON_AREA_SIZE 0x700000
>>> +
>>> +#define PNV_HOMER_BASE(chip) \
>>> + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
>>> +#define PNV_OCC_COMMON_AREA(chip) \
>>> + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
>>> +
>>> +#define PNV9_HOMER_BASE(chip) \
>>> + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
>>> +#define PNV9_OCC_COMMON_AREA(chip) \
>>> + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
>>> +
>>> +#endif /* _PPC_PNV_HOMER_H */
>>>
>>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV
2019-08-07 7:54 ` Cédric Le Goater
2019-08-07 10:07 ` Balamuruhan S
@ 2019-08-09 4:44 ` David Gibson
2019-08-11 6:34 ` Balamuruhan S
1 sibling, 1 reply; 45+ messages in thread
From: David Gibson @ 2019-08-09 4:44 UTC (permalink / raw)
To: Cédric Le Goater
Cc: maddy, qemu-devel, Balamuruhan S, anju, hari, pbonzini
[-- Attachment #1: Type: text/plain, Size: 14686 bytes --]
On Wed, Aug 07, 2019 at 09:54:55AM +0200, Cédric Le Goater wrote:
> On 07/08/2019 09:14, Balamuruhan S wrote:
> > Add mmio callback functions to enable homer/occ common area
> > to emulate pstate table, occ-sensors, slw, occ static and
> > dynamic values for Power8 and Power9 chips. It also works for
> > multiple chips as offset remains the same whereas the base
> > address are handled appropriately while initializing device
> > tree.
> >
> > currently skiboot disables the homer/occ code path with
> > `QUIRK_NO_PBA`, this quirk have to be removed in skiboot
> > for it to use this infrastructure.
>
>
> I think this patch can come before the others as it is adding
> support without the python extra facilities.
Right. In fact it seems to me having it as an entirely separate
series would be preferable. I don't think we want to tie review of a
basic OCC extension to to the frankly not all that palatable idea of
adding arbitrary scripting into the MMIO path.
>
> Some comments below,
>
> > Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com>
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> > hw/ppc/Makefile.objs | 2 +-
> > hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/ppc/pnv.h | 14 ++++
> > include/hw/ppc/pnv_homer.h | 41 ++++++++++
> > 4 files changed, 241 insertions(+), 1 deletion(-)
> > create mode 100644 hw/ppc/pnv_homer.c
> > create mode 100644 include/hw/ppc/pnv_homer.h
> >
> > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > index 9da93af905..7260b4a96c 100644
> > --- a/hw/ppc/Makefile.objs
> > +++ b/hw/ppc/Makefile.objs
> > @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
> > obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
> > obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o
> > # IBM PowerNV
> > -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
> > +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o
>
> add an extra line.
>
> > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> > obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
> > endif
> > diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> > new file mode 100644
> > index 0000000000..73a94856d0
> > --- /dev/null
> > +++ b/hw/ppc/pnv_homer.c
> > @@ -0,0 +1,185 @@
> > +/*
> > + * QEMU PowerPC PowerNV Homer and OCC common area region
> > + *
> > + * Copyright (c) 2019, IBM Corporation.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#include "qemu/osdep.h"
> > +#include "sysemu/hw_accel.h"
> > +#include "sysemu/cpus.h"
> > +#include "hw/ppc/pnv.h"
> > +
> > +static bool core_max_array(hwaddr addr)
> > +{
> > + char *cpu_type;
> > + hwaddr core_max_base = 0xe2819;
>
> What is this representing ?
>
> > + MachineState *ms = MACHINE(qdev_get_machine());
> > + cpu_type = strstr(ms->cpu_type, "power8");
>
> you need to get this information some other way. The PnvChip should have it.
>
> > + if (cpu_type)
> > + core_max_base = 0x1f8810;
>
> It could be a PnvChipClass value.
>
> > + for (int i = 0; i <= ms->smp.cores; i++)
> > + if (addr == (core_max_base + i))
> > + return true;
> > + return false;
> > +}
>
>
> > +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
> > +{
> > + switch (addr) {
>
> We should be using defines for the case statements below.
>
> Are we accessing one or more structures which are mapped at specific
> addresses ? If so I would define them in this file and change the
> memory ops to use well known offsets.
>
> Are these structures the same on P9 and P8 ?
>
> Are there default values ? May be we could use a reset handler
> in this case.
>
> > + case 0xe2006: /* max pstate ultra turbo */
> > + case 0xe2018: /* pstate id for 0 */
> > + case 0x1f8001: /* P8 occ pstate version */
> > + case 0x1f8003: /* P8 pstate min */
> > + case 0x1f8010: /* P8 pstate id for 0 */
> > + return 0;
> > + case 0xe2000: /* occ data area */
> > + case 0xe2002: /* occ_role master/slave*/
> > + case 0xe2004: /* pstate nom */
> > + case 0xe2005: /* pstate turbo */
> > + case 0xe2020: /* pstate id for 1 */
> > + case 0xe2818: /* pstate ultra turbo */
> > + case 0xe2b85: /* opal dynamic data (runtime) */
> > + case 0x1f8000: /* P8 occ pstate valid */
> > + case 0x1f8002: /* P8 throttle */
> > + case 0x1f8004: /* P8 pstate nom */
> > + case 0x1f8005: /* P8 pstate turbo */
> > + case 0x1f8012: /* vdd voltage identifier */
> > + case 0x1f8013: /* vcs voltage identifier */
> > + case 0x1f8018: /* P8 pstate id for 1 */
> > + return 1;
> > + case 0xe2003: /* pstate min (2 as pstate min) */
> > + case 0xe2028: /* pstate id for 2 */
> > + case 0x1f8006: /* P8 pstate ultra turbo */
> > + case 0x1f8020: /* P8 pstate id for 2 */
> > + return 2;
> > + case 0xe2001: /* major version */
> > + return 0x90;
> > + /* 3000 khz frequency for 0, 1, and 2 pstates */
> > + case 0xe201c:
> > + case 0xe2024:
> > + case 0xe202c:
> > + /* P8 frequency for 0, 1, and 2 pstates */
> > + case 0x1f8014:
> > + case 0x1f801c:
> > + case 0x1f8024:
> > + return 3000;
> > + case 0x0: /* homer base */
> > + case 0xe2008: /* occ data area + 8 */
> > + case 0x1f8008: /* P8 occ data area + 8 */
> > + case 0x200008: /* homer base access to get homer image pointer*/
> > + return 0x1000000000000000;
> > + }
> > + /* pstate table core max array */
> > + if (core_max_array(addr))
> > + return 1;
>
> I don't understand what the core_max_array is returning
>
> > + return 0;
> > +}
> > +
> > +static void homer_write(void *opaque, hwaddr addr, uint64_t val,
> > + unsigned width)
> > +{
> > + /* callback function defined to homer write */
> > + return;
> > +}
> > +
> > +const MemoryRegionOps pnv_homer_ops = {
> > + .read = homer_read,
> > + .write = homer_write,
> > + .valid.min_access_size = 1,
> > + .valid.max_access_size = 8,
> > + .impl.min_access_size = 1,
> > + .impl.max_access_size = 8,
> > + .endianness = DEVICE_BIG_ENDIAN,
> > +};
> > +
> > +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
> > +{
> > + switch (addr) {
> > + /*
> > + * occ-sensor sanity check that asserts the sensor
> > + * header block
> > + */
>
> Same comments as above.
>
> > + case 0x580000: /* occ sensor data block */
> > + case 0x580001: /* valid */
> > + case 0x580002: /* version */
> > + case 0x580004: /* reading_version */
> > + case 0x580008: /* nr_sensors */
> > + case 0x580010: /* names_offset */
> > + case 0x580014: /* reading_ping_offset */
> > + case 0x58000c: /* reading_pong_offset */
> > + case 0x580023: /* structure_type */
> > + return 1;
> > + case 0x58000d: /* name length */
> > + return 0x30;
> > + case 0x580022: /* occ sensor loc core */
> > + return 0x0040;
> > + case 0x580003: /* occ sensor type power */
> > + return 0x0080;
> > + case 0x580005: /* sensor name */
> > + return 0x1000;
> > + case 0x58001e: /* HWMON_SENSORS_MASK */
> > + case 0x580020:
> > + return 0x8e00;
> > + case 0x0: /* P8 slw base access for slw image size */
> > + return 0x1000000000000000;
> > + }
> > + return 0;
> > +}
> > +
> > +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val,
> > + unsigned width)
> > +{
> > + /* callback function defined to occ common area write */
> > + return;
> > +}
> > +
> > +const MemoryRegionOps pnv_occ_common_area_ops = {
> > + .read = occ_common_area_read,
> > + .write = occ_common_area_write,
> > + .valid.min_access_size = 1,
> > + .valid.max_access_size = 8,
> > + .impl.min_access_size = 1,
> > + .impl.max_access_size = 8,
> > + .endianness = DEVICE_BIG_ENDIAN,
> > +};
>
>
> Why aren't you using the PnvOCC model ?
>
> > +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp)
> > +{
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> > + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS;
> > + char *occ_common_area;
> > +
> > + /* occ common area */
> > + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id);
> > + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip),
> > + &pnv_occ_common_area_ops, chip, occ_common_area,
> > + PNV_OCC_COMMON_AREA_SIZE);
> > + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio);
> > + g_free(occ_common_area);
> > +}
>
>
> May be this "device" deserves a PnvHomer model, one for P8 and one for P9.
>
> > +void pnv_homer_realize(PnvChip *chip, Error **errp)
> > +{
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> > + sbd->num_mmio = PNV_HOMER_SYSBUS;
> > + char *homer;
> > +
> > + /* homer region */
> > + homer = g_strdup_printf("homer-%x", chip->chip_id);
> > + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops,
> > + chip, homer, PNV_HOMER_SIZE);
> > + sysbus_init_mmio(sbd, &chip->homer_mmio);
> > + g_free(homer);
> > +}
> > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > index fb123edc4e..6464e32892 100644
> > --- a/include/hw/ppc/pnv.h
> > +++ b/include/hw/ppc/pnv.h
> > @@ -28,6 +28,7 @@
> > #include "hw/ppc/pnv_occ.h"
> > #include "hw/ppc/pnv_xive.h"
> > #include "hw/ppc/pnv_core.h"
> > +#include "hw/ppc/pnv_homer.h"
> >
> > #define TYPE_PNV_CHIP "pnv-chip"
> > #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP)
> > @@ -36,6 +37,13 @@
> > #define PNV_CHIP_GET_CLASS(obj) \
> > OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP)
> >
> > +enum SysBusNum {
> > + PNV_XSCOM_SYSBUS,
> > + PNV_ICP_SYSBUS,
> > + PNV_HOMER_SYSBUS,
> > + PNV_OCC_COMMON_AREA_SYSBUS,
> > +};
>
> What is this ?
>
>
> > typedef enum PnvChipType {
> > PNV_CHIP_POWER8E, /* AKA Murano (default) */
> > PNV_CHIP_POWER8, /* AKA Venice */
> > @@ -56,6 +64,8 @@ typedef struct PnvChip {
> > uint64_t cores_mask;
> > void *cores;
> >
> > + MemoryRegion homer_mmio;
> > + MemoryRegion occ_common_area_mmio;
> > MemoryRegion xscom_mmio;
> > MemoryRegion xscom;
> > AddressSpace xscom_as;
> > @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv)
> > void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt);
> > void pnv_bmc_powerdown(IPMIBmc *bmc);
> >
> > +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp);
> > +extern void pnv_homer_realize(PnvChip *chip, Error **errp);
> > +
> > +
> > /*
> > * POWER8 MMIO base addresses
> > */
> > diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
> > new file mode 100644
> > index 0000000000..0fe6469abe
> > --- /dev/null
> > +++ b/include/hw/ppc/pnv_homer.h
> > @@ -0,0 +1,41 @@
> > +/*
> > + * QEMU PowerPC PowerNV Homer and occ common area definitions
> > + *
> > + * Copyright (c) 2019, IBM Corporation.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#ifndef _PPC_PNV_HOMER_H
> > +#define _PPC_PNV_HOMER_H
> > +
> > +#include "qom/object.h"
> > +
> > +/*
> > + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip)
> > + * so chip_num can be used to offset between HOMER region from its base address
> > + */
> > +#define PNV_HOMER_SIZE 0x300000
> > +#define PNV_OCC_COMMON_AREA_SIZE 0x700000
> > +
> > +#define PNV_HOMER_BASE(chip) \
> > + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
> > +#define PNV_OCC_COMMON_AREA(chip) \
> > + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
> > +
> > +#define PNV9_HOMER_BASE(chip) \
> > + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
> > +#define PNV9_OCC_COMMON_AREA(chip) \
> > + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
> > +
> > +#endif /* _PPC_PNV_HOMER_H */
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV
2019-08-09 4:44 ` David Gibson
@ 2019-08-11 6:34 ` Balamuruhan S
0 siblings, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-11 6:34 UTC (permalink / raw)
To: David Gibson, Cédric Le Goater
Cc: pbonzini, maddy, hari, qemu-devel, anju
On 8/9/19 10:14 AM, David Gibson wrote:
> On Wed, Aug 07, 2019 at 09:54:55AM +0200, Cédric Le Goater wrote:
>> On 07/08/2019 09:14, Balamuruhan S wrote:
>>> Add mmio callback functions to enable homer/occ common area
>>> to emulate pstate table, occ-sensors, slw, occ static and
>>> dynamic values for Power8 and Power9 chips. It also works for
>>> multiple chips as offset remains the same whereas the base
>>> address are handled appropriately while initializing device
>>> tree.
>>>
>>> currently skiboot disables the homer/occ code path with
>>> `QUIRK_NO_PBA`, this quirk have to be removed in skiboot
>>> for it to use this infrastructure.
>>
>> I think this patch can come before the others as it is adding
>> support without the python extra facilities.
> Right. In fact it seems to me having it as an entirely separate
> series would be preferable. I don't think we want to tie review of a
> basic OCC extension to to the frankly not all that palatable idea of
> adding arbitrary scripting into the MMIO path.
sure, I will send them as separate series. But the idea is to demonstrate
how the scripting interface can be added and to leverage it and I can
get feedback/suggestions to correct/improve it.
>
>> Some comments below,
>>
>>> Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com>
>>> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
>>> ---
>>> hw/ppc/Makefile.objs | 2 +-
>>> hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++
>>> include/hw/ppc/pnv.h | 14 ++++
>>> include/hw/ppc/pnv_homer.h | 41 ++++++++++
>>> 4 files changed, 241 insertions(+), 1 deletion(-)
>>> create mode 100644 hw/ppc/pnv_homer.c
>>> create mode 100644 include/hw/ppc/pnv_homer.h
>>>
>>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
>>> index 9da93af905..7260b4a96c 100644
>>> --- a/hw/ppc/Makefile.objs
>>> +++ b/hw/ppc/Makefile.objs
>>> @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
>>> obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
>>> obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o
>>> # IBM PowerNV
>>> -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
>>> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o
>> add an extra line.
>>
>>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>>> obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
>>> endif
>>> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
>>> new file mode 100644
>>> index 0000000000..73a94856d0
>>> --- /dev/null
>>> +++ b/hw/ppc/pnv_homer.c
>>> @@ -0,0 +1,185 @@
>>> +/*
>>> + * QEMU PowerPC PowerNV Homer and OCC common area region
>>> + *
>>> + * Copyright (c) 2019, IBM Corporation.
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +#include "qemu/osdep.h"
>>> +#include "sysemu/hw_accel.h"
>>> +#include "sysemu/cpus.h"
>>> +#include "hw/ppc/pnv.h"
>>> +
>>> +static bool core_max_array(hwaddr addr)
>>> +{
>>> + char *cpu_type;
>>> + hwaddr core_max_base = 0xe2819;
>> What is this representing ?
>>
>>> + MachineState *ms = MACHINE(qdev_get_machine());
>>> + cpu_type = strstr(ms->cpu_type, "power8");
>> you need to get this information some other way. The PnvChip should have it.
>>
>>> + if (cpu_type)
>>> + core_max_base = 0x1f8810;
>> It could be a PnvChipClass value.
>>
>>> + for (int i = 0; i <= ms->smp.cores; i++)
>>> + if (addr == (core_max_base + i))
>>> + return true;
>>> + return false;
>>> +}
>>
>>> +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
>>> +{
>>> + switch (addr) {
>> We should be using defines for the case statements below.
>>
>> Are we accessing one or more structures which are mapped at specific
>> addresses ? If so I would define them in this file and change the
>> memory ops to use well known offsets.
>>
>> Are these structures the same on P9 and P8 ?
>>
>> Are there default values ? May be we could use a reset handler
>> in this case.
>>
>>> + case 0xe2006: /* max pstate ultra turbo */
>>> + case 0xe2018: /* pstate id for 0 */
>>> + case 0x1f8001: /* P8 occ pstate version */
>>> + case 0x1f8003: /* P8 pstate min */
>>> + case 0x1f8010: /* P8 pstate id for 0 */
>>> + return 0;
>>> + case 0xe2000: /* occ data area */
>>> + case 0xe2002: /* occ_role master/slave*/
>>> + case 0xe2004: /* pstate nom */
>>> + case 0xe2005: /* pstate turbo */
>>> + case 0xe2020: /* pstate id for 1 */
>>> + case 0xe2818: /* pstate ultra turbo */
>>> + case 0xe2b85: /* opal dynamic data (runtime) */
>>> + case 0x1f8000: /* P8 occ pstate valid */
>>> + case 0x1f8002: /* P8 throttle */
>>> + case 0x1f8004: /* P8 pstate nom */
>>> + case 0x1f8005: /* P8 pstate turbo */
>>> + case 0x1f8012: /* vdd voltage identifier */
>>> + case 0x1f8013: /* vcs voltage identifier */
>>> + case 0x1f8018: /* P8 pstate id for 1 */
>>> + return 1;
>>> + case 0xe2003: /* pstate min (2 as pstate min) */
>>> + case 0xe2028: /* pstate id for 2 */
>>> + case 0x1f8006: /* P8 pstate ultra turbo */
>>> + case 0x1f8020: /* P8 pstate id for 2 */
>>> + return 2;
>>> + case 0xe2001: /* major version */
>>> + return 0x90;
>>> + /* 3000 khz frequency for 0, 1, and 2 pstates */
>>> + case 0xe201c:
>>> + case 0xe2024:
>>> + case 0xe202c:
>>> + /* P8 frequency for 0, 1, and 2 pstates */
>>> + case 0x1f8014:
>>> + case 0x1f801c:
>>> + case 0x1f8024:
>>> + return 3000;
>>> + case 0x0: /* homer base */
>>> + case 0xe2008: /* occ data area + 8 */
>>> + case 0x1f8008: /* P8 occ data area + 8 */
>>> + case 0x200008: /* homer base access to get homer image pointer*/
>>> + return 0x1000000000000000;
>>> + }
>>> + /* pstate table core max array */
>>> + if (core_max_array(addr))
>>> + return 1;
>> I don't understand what the core_max_array is returning
>>
>>> + return 0;
>>> +}
>>> +
>>> +static void homer_write(void *opaque, hwaddr addr, uint64_t val,
>>> + unsigned width)
>>> +{
>>> + /* callback function defined to homer write */
>>> + return;
>>> +}
>>> +
>>> +const MemoryRegionOps pnv_homer_ops = {
>>> + .read = homer_read,
>>> + .write = homer_write,
>>> + .valid.min_access_size = 1,
>>> + .valid.max_access_size = 8,
>>> + .impl.min_access_size = 1,
>>> + .impl.max_access_size = 8,
>>> + .endianness = DEVICE_BIG_ENDIAN,
>>> +};
>>> +
>>> +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
>>> +{
>>> + switch (addr) {
>>> + /*
>>> + * occ-sensor sanity check that asserts the sensor
>>> + * header block
>>> + */
>> Same comments as above.
>>
>>> + case 0x580000: /* occ sensor data block */
>>> + case 0x580001: /* valid */
>>> + case 0x580002: /* version */
>>> + case 0x580004: /* reading_version */
>>> + case 0x580008: /* nr_sensors */
>>> + case 0x580010: /* names_offset */
>>> + case 0x580014: /* reading_ping_offset */
>>> + case 0x58000c: /* reading_pong_offset */
>>> + case 0x580023: /* structure_type */
>>> + return 1;
>>> + case 0x58000d: /* name length */
>>> + return 0x30;
>>> + case 0x580022: /* occ sensor loc core */
>>> + return 0x0040;
>>> + case 0x580003: /* occ sensor type power */
>>> + return 0x0080;
>>> + case 0x580005: /* sensor name */
>>> + return 0x1000;
>>> + case 0x58001e: /* HWMON_SENSORS_MASK */
>>> + case 0x580020:
>>> + return 0x8e00;
>>> + case 0x0: /* P8 slw base access for slw image size */
>>> + return 0x1000000000000000;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val,
>>> + unsigned width)
>>> +{
>>> + /* callback function defined to occ common area write */
>>> + return;
>>> +}
>>> +
>>> +const MemoryRegionOps pnv_occ_common_area_ops = {
>>> + .read = occ_common_area_read,
>>> + .write = occ_common_area_write,
>>> + .valid.min_access_size = 1,
>>> + .valid.max_access_size = 8,
>>> + .impl.min_access_size = 1,
>>> + .impl.max_access_size = 8,
>>> + .endianness = DEVICE_BIG_ENDIAN,
>>> +};
>>
>> Why aren't you using the PnvOCC model ?
>>
>>> +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp)
>>> +{
>>> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
>>> + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS;
>>> + char *occ_common_area;
>>> +
>>> + /* occ common area */
>>> + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id);
>>> + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip),
>>> + &pnv_occ_common_area_ops, chip, occ_common_area,
>>> + PNV_OCC_COMMON_AREA_SIZE);
>>> + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio);
>>> + g_free(occ_common_area);
>>> +}
>>
>> May be this "device" deserves a PnvHomer model, one for P8 and one for P9.
>>
>>> +void pnv_homer_realize(PnvChip *chip, Error **errp)
>>> +{
>>> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
>>> + sbd->num_mmio = PNV_HOMER_SYSBUS;
>>> + char *homer;
>>> +
>>> + /* homer region */
>>> + homer = g_strdup_printf("homer-%x", chip->chip_id);
>>> + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops,
>>> + chip, homer, PNV_HOMER_SIZE);
>>> + sysbus_init_mmio(sbd, &chip->homer_mmio);
>>> + g_free(homer);
>>> +}
>>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
>>> index fb123edc4e..6464e32892 100644
>>> --- a/include/hw/ppc/pnv.h
>>> +++ b/include/hw/ppc/pnv.h
>>> @@ -28,6 +28,7 @@
>>> #include "hw/ppc/pnv_occ.h"
>>> #include "hw/ppc/pnv_xive.h"
>>> #include "hw/ppc/pnv_core.h"
>>> +#include "hw/ppc/pnv_homer.h"
>>>
>>> #define TYPE_PNV_CHIP "pnv-chip"
>>> #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP)
>>> @@ -36,6 +37,13 @@
>>> #define PNV_CHIP_GET_CLASS(obj) \
>>> OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP)
>>>
>>> +enum SysBusNum {
>>> + PNV_XSCOM_SYSBUS,
>>> + PNV_ICP_SYSBUS,
>>> + PNV_HOMER_SYSBUS,
>>> + PNV_OCC_COMMON_AREA_SYSBUS,
>>> +};
>> What is this ?
>>
>>
>>> typedef enum PnvChipType {
>>> PNV_CHIP_POWER8E, /* AKA Murano (default) */
>>> PNV_CHIP_POWER8, /* AKA Venice */
>>> @@ -56,6 +64,8 @@ typedef struct PnvChip {
>>> uint64_t cores_mask;
>>> void *cores;
>>>
>>> + MemoryRegion homer_mmio;
>>> + MemoryRegion occ_common_area_mmio;
>>> MemoryRegion xscom_mmio;
>>> MemoryRegion xscom;
>>> AddressSpace xscom_as;
>>> @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv)
>>> void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt);
>>> void pnv_bmc_powerdown(IPMIBmc *bmc);
>>>
>>> +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp);
>>> +extern void pnv_homer_realize(PnvChip *chip, Error **errp);
>>> +
>>> +
>>> /*
>>> * POWER8 MMIO base addresses
>>> */
>>> diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
>>> new file mode 100644
>>> index 0000000000..0fe6469abe
>>> --- /dev/null
>>> +++ b/include/hw/ppc/pnv_homer.h
>>> @@ -0,0 +1,41 @@
>>> +/*
>>> + * QEMU PowerPC PowerNV Homer and occ common area definitions
>>> + *
>>> + * Copyright (c) 2019, IBM Corporation.
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +#ifndef _PPC_PNV_HOMER_H
>>> +#define _PPC_PNV_HOMER_H
>>> +
>>> +#include "qom/object.h"
>>> +
>>> +/*
>>> + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip)
>>> + * so chip_num can be used to offset between HOMER region from its base address
>>> + */
>>> +#define PNV_HOMER_SIZE 0x300000
>>> +#define PNV_OCC_COMMON_AREA_SIZE 0x700000
>>> +
>>> +#define PNV_HOMER_BASE(chip) \
>>> + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
>>> +#define PNV_OCC_COMMON_AREA(chip) \
>>> + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
>>> +
>>> +#define PNV9_HOMER_BASE(chip) \
>>> + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE)
>>> +#define PNV9_OCC_COMMON_AREA(chip) \
>>> + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
>>> +
>>> +#endif /* _PPC_PNV_HOMER_H */
>>>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
` (2 preceding siblings ...)
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV Balamuruhan S
@ 2019-08-07 7:14 ` Balamuruhan S
2019-08-07 7:59 ` Cédric Le Goater
2019-08-09 4:45 ` David Gibson
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 5/6] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs Balamuruhan S
` (6 subsequent siblings)
10 siblings, 2 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 7:14 UTC (permalink / raw)
To: qemu-devel; +Cc: maddy, Balamuruhan S, anju, clg, hari, pbonzini, david
homer and occ common area region base address are initialized
to create device tree and realized to map the address with
mmio callbacks during `pnv_chip_realize()`.
`SysBusNum` enum is introduced to set sysbus for XSCOM, ICP,
HOMER and OCC appropriately and chip_num to initialize and
retrieve base address + size contiguously on a PowerNV with
multichip boot.
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
hw/ppc/pnv.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
include/hw/ppc/pnv.h | 1 +
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index bd4531c822..f6e56e915d 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -675,6 +675,7 @@ static void pnv_init(MachineState *machine)
Object *chip = object_new(chip_typename);
pnv->chips[i] = PNV_CHIP(chip);
+ PNV_CHIP(chip)->chip_num = i;
/* TODO: put all the memory in one node on chip 0 until we find a
* way to specify different ranges for each chip
@@ -824,18 +825,20 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
{
PnvChip *chip = PNV_CHIP(chip8);
PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
const char *typename = pnv_chip_core_typename(chip);
size_t typesize = object_type_get_instance_size(typename);
int i, j;
char *name;
XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
+ sbd->num_mmio = PNV_ICP_SYSBUS;
name = g_strdup_printf("icp-%x", chip->chip_id);
memory_region_init(&chip8->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
- sysbus_init_mmio(SYS_BUS_DEVICE(chip), &chip8->icp_mmio);
+ sysbus_init_mmio(sbd, &chip8->icp_mmio);
g_free(name);
- sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
+ sysbus_mmio_map(sbd, PNV_ICP_SYSBUS, PNV_ICP_BASE(chip));
/* Map the ICP registers for each thread */
for (i = 0; i < chip->nr_cores; i++) {
@@ -866,7 +869,26 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
error_propagate(errp, local_err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV_XSCOM_BASE(chip));
+ sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
+ PNV_XSCOM_BASE(chip));
+
+ /* homer */
+ pnv_homer_realize(chip, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
+ PNV_HOMER_BASE(chip));
+
+ /* occ common area */
+ pnv_occ_common_area_realize(chip, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
+ PNV_OCC_COMMON_AREA(chip));
pcc->parent_realize(dev, &local_err);
if (local_err) {
@@ -1035,7 +1057,26 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
error_propagate(errp, local_err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV9_XSCOM_BASE(chip));
+ sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
+ PNV9_XSCOM_BASE(chip));
+
+ /* homer */
+ pnv_homer_realize(chip, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
+ PNV9_HOMER_BASE(chip));
+
+ /* occ common area */
+ pnv_occ_common_area_realize(chip, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
+ PNV9_OCC_COMMON_AREA(chip));
pcc->parent_realize(dev, &local_err);
if (local_err) {
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 6464e32892..dea6772988 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -57,6 +57,7 @@ typedef struct PnvChip {
/*< public >*/
uint32_t chip_id;
+ uint32_t chip_num;
uint64_t ram_start;
uint64_t ram_size;
--
2.14.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area Balamuruhan S
@ 2019-08-07 7:59 ` Cédric Le Goater
2019-08-07 10:12 ` Balamuruhan S
2019-08-09 4:45 ` David Gibson
1 sibling, 1 reply; 45+ messages in thread
From: Cédric Le Goater @ 2019-08-07 7:59 UTC (permalink / raw)
To: Balamuruhan S, qemu-devel; +Cc: pbonzini, maddy, anju, david, hari
On 07/08/2019 09:14, Balamuruhan S wrote:
> homer and occ common area region base address are initialized
> to create device tree and realized to map the address with
> mmio callbacks during `pnv_chip_realize()`.
>
> `SysBusNum` enum is introduced to set sysbus for XSCOM, ICP,
> HOMER and OCC appropriately and chip_num to initialize and
> retrieve base address + size contiguously on a PowerNV with
> multichip boot.
Can't you use the chip_id ?
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> hw/ppc/pnv.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
> include/hw/ppc/pnv.h | 1 +
> 2 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index bd4531c822..f6e56e915d 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -675,6 +675,7 @@ static void pnv_init(MachineState *machine)
> Object *chip = object_new(chip_typename);
>
> pnv->chips[i] = PNV_CHIP(chip);
> + PNV_CHIP(chip)->chip_num = i;
>
> /* TODO: put all the memory in one node on chip 0 until we find a
> * way to specify different ranges for each chip
> @@ -824,18 +825,20 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
> {
> PnvChip *chip = PNV_CHIP(chip8);
> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> const char *typename = pnv_chip_core_typename(chip);
> size_t typesize = object_type_get_instance_size(typename);
> int i, j;
> char *name;
> XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
>
> + sbd->num_mmio = PNV_ICP_SYSBUS;
OK. I think I know why you want this but it probably means that you need
a new PnvHomer model and that you should use PnvOCC instead.
> name = g_strdup_printf("icp-%x", chip->chip_id);
> memory_region_init(&chip8->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
> - sysbus_init_mmio(SYS_BUS_DEVICE(chip), &chip8->icp_mmio);
> + sysbus_init_mmio(sbd, &chip8->icp_mmio);
> g_free(name);
>
> - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
> + sysbus_mmio_map(sbd, PNV_ICP_SYSBUS, PNV_ICP_BASE(chip));
>
> /* Map the ICP registers for each thread */
> for (i = 0; i < chip->nr_cores; i++) {
> @@ -866,7 +869,26 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, local_err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV_XSCOM_BASE(chip));
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
> + PNV_XSCOM_BASE(chip));
> +
> + /* homer */
> + pnv_homer_realize(chip, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
> + PNV_HOMER_BASE(chip));
> + /* occ common area */
> + pnv_occ_common_area_realize(chip, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
> + PNV_OCC_COMMON_AREA(chip));
>
> pcc->parent_realize(dev, &local_err);
> if (local_err) {
> @@ -1035,7 +1057,26 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, local_err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV9_XSCOM_BASE(chip));
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
> + PNV9_XSCOM_BASE(chip));
> +
> + /* homer */
> + pnv_homer_realize(chip, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
> + PNV9_HOMER_BASE(chip));
> +
> + /* occ common area */
> + pnv_occ_common_area_realize(chip, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
> + PNV9_OCC_COMMON_AREA(chip));
>
> pcc->parent_realize(dev, &local_err);
> if (local_err) {
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 6464e32892..dea6772988 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -57,6 +57,7 @@ typedef struct PnvChip {
>
> /*< public >*/
> uint32_t chip_id;
> + uint32_t chip_num;
> uint64_t ram_start;
> uint64_t ram_size;
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area
2019-08-07 7:59 ` Cédric Le Goater
@ 2019-08-07 10:12 ` Balamuruhan S
2019-08-08 8:46 ` Cédric Le Goater
0 siblings, 1 reply; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 10:12 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: hari, anju, qemu-devel, maddy, pbonzini, david
On Wed, Aug 07, 2019 at 09:59:26AM +0200, Cédric Le Goater wrote:
> On 07/08/2019 09:14, Balamuruhan S wrote:
> > homer and occ common area region base address are initialized
> > to create device tree and realized to map the address with
> > mmio callbacks during `pnv_chip_realize()`.
> >
> > `SysBusNum` enum is introduced to set sysbus for XSCOM, ICP,
> > HOMER and OCC appropriately and chip_num to initialize and
> > retrieve base address + size contiguously on a PowerNV with
> > multichip boot.
>
> Can't you use the chip_id ?
if the chip_id is contiguous always we can use it. I was not
sure about it.
>
> >
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> > hw/ppc/pnv.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
> > include/hw/ppc/pnv.h | 1 +
> > 2 files changed, 46 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index bd4531c822..f6e56e915d 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -675,6 +675,7 @@ static void pnv_init(MachineState *machine)
> > Object *chip = object_new(chip_typename);
> >
> > pnv->chips[i] = PNV_CHIP(chip);
> > + PNV_CHIP(chip)->chip_num = i;
> >
> > /* TODO: put all the memory in one node on chip 0 until we find a
> > * way to specify different ranges for each chip
> > @@ -824,18 +825,20 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
> > {
> > PnvChip *chip = PNV_CHIP(chip8);
> > PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> > const char *typename = pnv_chip_core_typename(chip);
> > size_t typesize = object_type_get_instance_size(typename);
> > int i, j;
> > char *name;
> > XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
> >
> > + sbd->num_mmio = PNV_ICP_SYSBUS;
>
> OK. I think I know why you want this but it probably means that you need
> a new PnvHomer model and that you should use PnvOCC instead.
okay, I will have to work on it.
>
> > name = g_strdup_printf("icp-%x", chip->chip_id);
> > memory_region_init(&chip8->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
> > - sysbus_init_mmio(SYS_BUS_DEVICE(chip), &chip8->icp_mmio);
> > + sysbus_init_mmio(sbd, &chip8->icp_mmio);
> > g_free(name);
> >
> > - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
> > + sysbus_mmio_map(sbd, PNV_ICP_SYSBUS, PNV_ICP_BASE(chip));
> >
> > /* Map the ICP registers for each thread */
> > for (i = 0; i < chip->nr_cores; i++) {
> > @@ -866,7 +869,26 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
> > error_propagate(errp, local_err);
> > return;
> > }
> > - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV_XSCOM_BASE(chip));
> > + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
> > + PNV_XSCOM_BASE(chip));
> > +
> > + /* homer */
> > + pnv_homer_realize(chip, &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
> > + PNV_HOMER_BASE(chip));
> > + /* occ common area */
> > + pnv_occ_common_area_realize(chip, &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
> > + PNV_OCC_COMMON_AREA(chip));
> >
> > pcc->parent_realize(dev, &local_err);
> > if (local_err) {
> > @@ -1035,7 +1057,26 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> > error_propagate(errp, local_err);
> > return;
> > }
> > - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV9_XSCOM_BASE(chip));
> > + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
> > + PNV9_XSCOM_BASE(chip));
> > +
> > + /* homer */
> > + pnv_homer_realize(chip, &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
> > + PNV9_HOMER_BASE(chip));
> > +
> > + /* occ common area */
> > + pnv_occ_common_area_realize(chip, &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
> > + PNV9_OCC_COMMON_AREA(chip));
> >
> > pcc->parent_realize(dev, &local_err);
> > if (local_err) {
> > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > index 6464e32892..dea6772988 100644
> > --- a/include/hw/ppc/pnv.h
> > +++ b/include/hw/ppc/pnv.h
> > @@ -57,6 +57,7 @@ typedef struct PnvChip {
> >
> > /*< public >*/
> > uint32_t chip_id;
> > + uint32_t chip_num;
> > uint64_t ram_start;
> > uint64_t ram_size;
> >
> >
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area
2019-08-07 10:12 ` Balamuruhan S
@ 2019-08-08 8:46 ` Cédric Le Goater
0 siblings, 0 replies; 45+ messages in thread
From: Cédric Le Goater @ 2019-08-08 8:46 UTC (permalink / raw)
To: Balamuruhan S; +Cc: hari, anju, qemu-devel, maddy, pbonzini, david
On 07/08/2019 12:12, Balamuruhan S wrote:
> On Wed, Aug 07, 2019 at 09:59:26AM +0200, Cédric Le Goater wrote:
>> On 07/08/2019 09:14, Balamuruhan S wrote:
>>> homer and occ common area region base address are initialized
>>> to create device tree and realized to map the address with
>>> mmio callbacks during `pnv_chip_realize()`.
>>>
>>> `SysBusNum` enum is introduced to set sysbus for XSCOM, ICP,
>>> HOMER and OCC appropriately and chip_num to initialize and
>>> retrieve base address + size contiguously on a PowerNV with
>>> multichip boot.
>>
>> Can't you use the chip_id ?
>
> if the chip_id is contiguous always we can use it. I was not
> sure about it.
When I introduced PNV_CHIP_HWID(), I made it follow the 2S tuletas
numbers, which are DCMs. So you get 0x0, 0x1, 0x10, 0x11.
PNV_CHIP_INDEX() is to be used for MMIO addresses. We can fix that
if there is a problem.
C.
>>
>>>
>>> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
>>> ---
>>> hw/ppc/pnv.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
>>> include/hw/ppc/pnv.h | 1 +
>>> 2 files changed, 46 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>>> index bd4531c822..f6e56e915d 100644
>>> --- a/hw/ppc/pnv.c
>>> +++ b/hw/ppc/pnv.c
>>> @@ -675,6 +675,7 @@ static void pnv_init(MachineState *machine)
>>> Object *chip = object_new(chip_typename);
>>>
>>> pnv->chips[i] = PNV_CHIP(chip);
>>> + PNV_CHIP(chip)->chip_num = i;
>>>
>>> /* TODO: put all the memory in one node on chip 0 until we find a
>>> * way to specify different ranges for each chip
>>> @@ -824,18 +825,20 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
>>> {
>>> PnvChip *chip = PNV_CHIP(chip8);
>>> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>>> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
>>> const char *typename = pnv_chip_core_typename(chip);
>>> size_t typesize = object_type_get_instance_size(typename);
>>> int i, j;
>>> char *name;
>>> XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
>>>
>>> + sbd->num_mmio = PNV_ICP_SYSBUS;
>>
>> OK. I think I know why you want this but it probably means that you need
>> a new PnvHomer model and that you should use PnvOCC instead.
>
> okay, I will have to work on it.
>
>>
>>> name = g_strdup_printf("icp-%x", chip->chip_id);
>>> memory_region_init(&chip8->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
>>> - sysbus_init_mmio(SYS_BUS_DEVICE(chip), &chip8->icp_mmio);
>>> + sysbus_init_mmio(sbd, &chip8->icp_mmio);
>>> g_free(name);
>>>
>>> - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
>>> + sysbus_mmio_map(sbd, PNV_ICP_SYSBUS, PNV_ICP_BASE(chip));
>>>
>>> /* Map the ICP registers for each thread */
>>> for (i = 0; i < chip->nr_cores; i++) {
>>> @@ -866,7 +869,26 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>>> error_propagate(errp, local_err);
>>> return;
>>> }
>>> - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV_XSCOM_BASE(chip));
>>> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
>>> + PNV_XSCOM_BASE(chip));
>>> +
>>> + /* homer */
>>> + pnv_homer_realize(chip, &local_err);
>>> + if (local_err) {
>>> + error_propagate(errp, local_err);
>>> + return;
>>> + }
>>> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
>>> + PNV_HOMER_BASE(chip));
>>> + /* occ common area */
>>> + pnv_occ_common_area_realize(chip, &local_err);
>>> + if (local_err) {
>>> + error_propagate(errp, local_err);
>>> + return;
>>> + }
>>> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
>>> + PNV_OCC_COMMON_AREA(chip));
>>>
>>> pcc->parent_realize(dev, &local_err);
>>> if (local_err) {
>>> @@ -1035,7 +1057,26 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>>> error_propagate(errp, local_err);
>>> return;
>>> }
>>> - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV9_XSCOM_BASE(chip));
>>> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
>>> + PNV9_XSCOM_BASE(chip));
>>> +
>>> + /* homer */
>>> + pnv_homer_realize(chip, &local_err);
>>> + if (local_err) {
>>> + error_propagate(errp, local_err);
>>> + return;
>>> + }
>>> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
>>> + PNV9_HOMER_BASE(chip));
>>> +
>>> + /* occ common area */
>>> + pnv_occ_common_area_realize(chip, &local_err);
>>> + if (local_err) {
>>> + error_propagate(errp, local_err);
>>> + return;
>>> + }
>>> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
>>> + PNV9_OCC_COMMON_AREA(chip));
>>>
>>> pcc->parent_realize(dev, &local_err);
>>> if (local_err) {
>>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
>>> index 6464e32892..dea6772988 100644
>>> --- a/include/hw/ppc/pnv.h
>>> +++ b/include/hw/ppc/pnv.h
>>> @@ -57,6 +57,7 @@ typedef struct PnvChip {
>>>
>>> /*< public >*/
>>> uint32_t chip_id;
>>> + uint32_t chip_num;
>>> uint64_t ram_start;
>>> uint64_t ram_size;
>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area Balamuruhan S
2019-08-07 7:59 ` Cédric Le Goater
@ 2019-08-09 4:45 ` David Gibson
1 sibling, 0 replies; 45+ messages in thread
From: David Gibson @ 2019-08-09 4:45 UTC (permalink / raw)
To: Balamuruhan S; +Cc: maddy, anju, qemu-devel, hari, clg, pbonzini
[-- Attachment #1: Type: text/plain, Size: 4953 bytes --]
On Wed, Aug 07, 2019 at 12:44:43PM +0530, Balamuruhan S wrote:
> homer and occ common area region base address are initialized
> to create device tree and realized to map the address with
> mmio callbacks during `pnv_chip_realize()`.
>
> `SysBusNum` enum is introduced to set sysbus for XSCOM, ICP,
> HOMER and OCC appropriately and chip_num to initialize and
> retrieve base address + size contiguously on a PowerNV with
> multichip boot.
Same comments here as on the previous patch - I don't think this
belongs in the series with the scripting extensions.
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> hw/ppc/pnv.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
> include/hw/ppc/pnv.h | 1 +
> 2 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index bd4531c822..f6e56e915d 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -675,6 +675,7 @@ static void pnv_init(MachineState *machine)
> Object *chip = object_new(chip_typename);
>
> pnv->chips[i] = PNV_CHIP(chip);
> + PNV_CHIP(chip)->chip_num = i;
>
> /* TODO: put all the memory in one node on chip 0 until we find a
> * way to specify different ranges for each chip
> @@ -824,18 +825,20 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
> {
> PnvChip *chip = PNV_CHIP(chip8);
> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> const char *typename = pnv_chip_core_typename(chip);
> size_t typesize = object_type_get_instance_size(typename);
> int i, j;
> char *name;
> XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
>
> + sbd->num_mmio = PNV_ICP_SYSBUS;
> name = g_strdup_printf("icp-%x", chip->chip_id);
> memory_region_init(&chip8->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
> - sysbus_init_mmio(SYS_BUS_DEVICE(chip), &chip8->icp_mmio);
> + sysbus_init_mmio(sbd, &chip8->icp_mmio);
> g_free(name);
>
> - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
> + sysbus_mmio_map(sbd, PNV_ICP_SYSBUS, PNV_ICP_BASE(chip));
>
> /* Map the ICP registers for each thread */
> for (i = 0; i < chip->nr_cores; i++) {
> @@ -866,7 +869,26 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, local_err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV_XSCOM_BASE(chip));
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
> + PNV_XSCOM_BASE(chip));
> +
> + /* homer */
> + pnv_homer_realize(chip, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
> + PNV_HOMER_BASE(chip));
> +
> + /* occ common area */
> + pnv_occ_common_area_realize(chip, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
> + PNV_OCC_COMMON_AREA(chip));
>
> pcc->parent_realize(dev, &local_err);
> if (local_err) {
> @@ -1035,7 +1057,26 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, local_err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV9_XSCOM_BASE(chip));
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
> + PNV9_XSCOM_BASE(chip));
> +
> + /* homer */
> + pnv_homer_realize(chip, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
> + PNV9_HOMER_BASE(chip));
> +
> + /* occ common area */
> + pnv_occ_common_area_realize(chip, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
> + PNV9_OCC_COMMON_AREA(chip));
>
> pcc->parent_realize(dev, &local_err);
> if (local_err) {
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 6464e32892..dea6772988 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -57,6 +57,7 @@ typedef struct PnvChip {
>
> /*< public >*/
> uint32_t chip_id;
> + uint32_t chip_num;
> uint64_t ram_start;
> uint64_t ram_size;
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [RFC PATCH 5/6] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
` (3 preceding siblings ...)
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area Balamuruhan S
@ 2019-08-07 7:14 ` Balamuruhan S
2019-08-07 8:01 ` Cédric Le Goater
2019-08-09 4:45 ` David Gibson
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area Balamuruhan S
` (5 subsequent siblings)
10 siblings, 2 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 7:14 UTC (permalink / raw)
To: qemu-devel; +Cc: maddy, Balamuruhan S, anju, clg, hari, pbonzini, david
During PowerNV boot skiboot populates the device tree by retrieving
base address of homer/occ common area from PBA BARs and prd ipoll
mask by accessing xscom read/write accesses.
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
hw/ppc/pnv_xscom.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index 5d5b5e9884..18a780bcdf 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -77,6 +77,29 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
case 0x18002: /* ECID2 */
return 0;
+ /* PBA BAR0 */
+ case 0x5012b00: /* P9 homer base address */
+ return PNV9_HOMER_BASE(chip);
+ case 0x2013f00: /* P8 homer base address */
+ return PNV_HOMER_BASE(chip);
+
+ /* PBA BARMASK0 */
+ case 0x5012b04: /* P9 homer region size */
+ case 0x2013f04: /* P8 homer region size */
+ return PNV_HOMER_SIZE;
+
+ /* PBA BAR2 */
+ case 0x5012b02: /* P9 occ common area */
+ return PNV9_OCC_COMMON_AREA(chip);
+ case 0x2013f02: /* P8 occ common area */
+ return PNV_OCC_COMMON_AREA(chip);
+
+ /* PBA BARMASK2 */
+ case 0x5012b06: /* P9 occ common area size */
+ case 0x2013f06: /* P8 occ common area size */
+ return PNV_OCC_COMMON_AREA_SIZE;
+
+
case 0x1010c00: /* PIBAM FIR */
case 0x1010c03: /* PIBAM FIR MASK */
@@ -96,13 +119,9 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
case 0x2020009: /* ADU stuff, error register */
case 0x202000f: /* ADU stuff, receive status register*/
return 0;
- case 0x2013f00: /* PBA stuff */
case 0x2013f01: /* PBA stuff */
- case 0x2013f02: /* PBA stuff */
case 0x2013f03: /* PBA stuff */
- case 0x2013f04: /* PBA stuff */
case 0x2013f05: /* PBA stuff */
- case 0x2013f06: /* PBA stuff */
case 0x2013f07: /* PBA stuff */
return 0;
case 0x2013028: /* CAPP stuff */
--
2.14.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 5/6] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 5/6] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs Balamuruhan S
@ 2019-08-07 8:01 ` Cédric Le Goater
2019-08-07 10:22 ` Balamuruhan S
2019-08-09 4:45 ` David Gibson
1 sibling, 1 reply; 45+ messages in thread
From: Cédric Le Goater @ 2019-08-07 8:01 UTC (permalink / raw)
To: Balamuruhan S, qemu-devel; +Cc: pbonzini, maddy, anju, david, hari
On 07/08/2019 09:14, Balamuruhan S wrote:
> During PowerNV boot skiboot populates the device tree by retrieving
> base address of homer/occ common area from PBA BARs and prd ipoll
> mask by accessing xscom read/write accesses.
This looks good. If you could add defines it would be better.
Our common XSCOM ops is starting to be a bit messy. May we should think
about introducing one for P9 and one for P8.
Thanks,
C.
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> hw/ppc/pnv_xscom.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index 5d5b5e9884..18a780bcdf 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -77,6 +77,29 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> case 0x18002: /* ECID2 */
> return 0;
>
> + /* PBA BAR0 */
> + case 0x5012b00: /* P9 homer base address */
> + return PNV9_HOMER_BASE(chip);
> + case 0x2013f00: /* P8 homer base address */
> + return PNV_HOMER_BASE(chip);
> +
> + /* PBA BARMASK0 */
> + case 0x5012b04: /* P9 homer region size */
> + case 0x2013f04: /* P8 homer region size */
> + return PNV_HOMER_SIZE;
> +
> + /* PBA BAR2 */
> + case 0x5012b02: /* P9 occ common area */
> + return PNV9_OCC_COMMON_AREA(chip);
> + case 0x2013f02: /* P8 occ common area */
> + return PNV_OCC_COMMON_AREA(chip);
> +
> + /* PBA BARMASK2 */
> + case 0x5012b06: /* P9 occ common area size */
> + case 0x2013f06: /* P8 occ common area size */
> + return PNV_OCC_COMMON_AREA_SIZE;
> +
> +
> case 0x1010c00: /* PIBAM FIR */
> case 0x1010c03: /* PIBAM FIR MASK */
>
> @@ -96,13 +119,9 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> case 0x2020009: /* ADU stuff, error register */
> case 0x202000f: /* ADU stuff, receive status register*/
> return 0;
> - case 0x2013f00: /* PBA stuff */
> case 0x2013f01: /* PBA stuff */
> - case 0x2013f02: /* PBA stuff */
> case 0x2013f03: /* PBA stuff */
> - case 0x2013f04: /* PBA stuff */
> case 0x2013f05: /* PBA stuff */
> - case 0x2013f06: /* PBA stuff */
> case 0x2013f07: /* PBA stuff */
> return 0;
> case 0x2013028: /* CAPP stuff */
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 5/6] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
2019-08-07 8:01 ` Cédric Le Goater
@ 2019-08-07 10:22 ` Balamuruhan S
0 siblings, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 10:22 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: hari, anju, qemu-devel, maddy, pbonzini, david
On Wed, Aug 07, 2019 at 10:01:58AM +0200, Cédric Le Goater wrote:
> On 07/08/2019 09:14, Balamuruhan S wrote:
> > During PowerNV boot skiboot populates the device tree by retrieving
> > base address of homer/occ common area from PBA BARs and prd ipoll
> > mask by accessing xscom read/write accesses.
>
> This looks good. If you could add defines it would be better.
sure.
>
> Our common XSCOM ops is starting to be a bit messy. May we should think
> about introducing one for P9 and one for P8.
yes, point taken.
Thanks Cedric.
>
> Thanks,
>
> C.
>
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> > hw/ppc/pnv_xscom.c | 27 +++++++++++++++++++++++----
> > 1 file changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> > index 5d5b5e9884..18a780bcdf 100644
> > --- a/hw/ppc/pnv_xscom.c
> > +++ b/hw/ppc/pnv_xscom.c
> > @@ -77,6 +77,29 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> > case 0x18002: /* ECID2 */
> > return 0;
> >
> > + /* PBA BAR0 */
> > + case 0x5012b00: /* P9 homer base address */
> > + return PNV9_HOMER_BASE(chip);
> > + case 0x2013f00: /* P8 homer base address */
> > + return PNV_HOMER_BASE(chip);
> > +
> > + /* PBA BARMASK0 */
> > + case 0x5012b04: /* P9 homer region size */
> > + case 0x2013f04: /* P8 homer region size */
> > + return PNV_HOMER_SIZE;
> > +
> > + /* PBA BAR2 */
> > + case 0x5012b02: /* P9 occ common area */
> > + return PNV9_OCC_COMMON_AREA(chip);
> > + case 0x2013f02: /* P8 occ common area */
> > + return PNV_OCC_COMMON_AREA(chip);
> > +
> > + /* PBA BARMASK2 */
> > + case 0x5012b06: /* P9 occ common area size */
> > + case 0x2013f06: /* P8 occ common area size */
> > + return PNV_OCC_COMMON_AREA_SIZE;
> > +
> > +
> > case 0x1010c00: /* PIBAM FIR */
> > case 0x1010c03: /* PIBAM FIR MASK */
> >
> > @@ -96,13 +119,9 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> > case 0x2020009: /* ADU stuff, error register */
> > case 0x202000f: /* ADU stuff, receive status register*/
> > return 0;
> > - case 0x2013f00: /* PBA stuff */
> > case 0x2013f01: /* PBA stuff */
> > - case 0x2013f02: /* PBA stuff */
> > case 0x2013f03: /* PBA stuff */
> > - case 0x2013f04: /* PBA stuff */
> > case 0x2013f05: /* PBA stuff */
> > - case 0x2013f06: /* PBA stuff */
> > case 0x2013f07: /* PBA stuff */
> > return 0;
> > case 0x2013028: /* CAPP stuff */
> >
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 5/6] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 5/6] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs Balamuruhan S
2019-08-07 8:01 ` Cédric Le Goater
@ 2019-08-09 4:45 ` David Gibson
1 sibling, 0 replies; 45+ messages in thread
From: David Gibson @ 2019-08-09 4:45 UTC (permalink / raw)
To: Balamuruhan S; +Cc: maddy, anju, qemu-devel, hari, clg, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2524 bytes --]
On Wed, Aug 07, 2019 at 12:44:44PM +0530, Balamuruhan S wrote:
> During PowerNV boot skiboot populates the device tree by retrieving
> base address of homer/occ common area from PBA BARs and prd ipoll
> mask by accessing xscom read/write accesses.
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Again seems unrelatedto the scripting.
> ---
> hw/ppc/pnv_xscom.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index 5d5b5e9884..18a780bcdf 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -77,6 +77,29 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> case 0x18002: /* ECID2 */
> return 0;
>
> + /* PBA BAR0 */
> + case 0x5012b00: /* P9 homer base address */
> + return PNV9_HOMER_BASE(chip);
> + case 0x2013f00: /* P8 homer base address */
> + return PNV_HOMER_BASE(chip);
> +
> + /* PBA BARMASK0 */
> + case 0x5012b04: /* P9 homer region size */
> + case 0x2013f04: /* P8 homer region size */
> + return PNV_HOMER_SIZE;
> +
> + /* PBA BAR2 */
> + case 0x5012b02: /* P9 occ common area */
> + return PNV9_OCC_COMMON_AREA(chip);
> + case 0x2013f02: /* P8 occ common area */
> + return PNV_OCC_COMMON_AREA(chip);
> +
> + /* PBA BARMASK2 */
> + case 0x5012b06: /* P9 occ common area size */
> + case 0x2013f06: /* P8 occ common area size */
> + return PNV_OCC_COMMON_AREA_SIZE;
> +
> +
> case 0x1010c00: /* PIBAM FIR */
> case 0x1010c03: /* PIBAM FIR MASK */
>
> @@ -96,13 +119,9 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> case 0x2020009: /* ADU stuff, error register */
> case 0x202000f: /* ADU stuff, receive status register*/
> return 0;
> - case 0x2013f00: /* PBA stuff */
> case 0x2013f01: /* PBA stuff */
> - case 0x2013f02: /* PBA stuff */
> case 0x2013f03: /* PBA stuff */
> - case 0x2013f04: /* PBA stuff */
> case 0x2013f05: /* PBA stuff */
> - case 0x2013f06: /* PBA stuff */
> case 0x2013f07: /* PBA stuff */
> return 0;
> case 0x2013028: /* CAPP stuff */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [RFC PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
` (4 preceding siblings ...)
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 5/6] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs Balamuruhan S
@ 2019-08-07 7:14 ` Balamuruhan S
2019-08-07 10:27 ` Philippe Mathieu-Daudé
2019-08-09 4:46 ` David Gibson
2019-08-07 7:33 ` [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface no-reply
` (4 subsequent siblings)
10 siblings, 2 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 7:14 UTC (permalink / raw)
To: qemu-devel; +Cc: maddy, Balamuruhan S, anju, clg, hari, pbonzini, david
use python interface APIs in homer/occ common area emulation to
interact with scripts if provided else fallback to normal flow,
it shows how simple to use the interface to call python methods
with any number of arguments in any script placed in common
-module-path provided in qemu commandline.
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
hw/ppc/pnv_homer.c | 20 ++++++++++++++++++++
hw/ppc/pnv_xscom.c | 9 +++++----
include/sysemu/sysemu.h | 4 ++++
vl.c | 24 ++++++++++++++++++++++++
4 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
index 73a94856d0..6ae5e74f19 100644
--- a/hw/ppc/pnv_homer.c
+++ b/hw/ppc/pnv_homer.c
@@ -16,7 +16,9 @@
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "sysemu/python_api.h"
#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
#include "sysemu/hw_accel.h"
#include "sysemu/cpus.h"
#include "hw/ppc/pnv.h"
@@ -37,6 +39,15 @@ static bool core_max_array(hwaddr addr)
static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
{
+ if (homer_module && homer) {
+ uint64_t homer_ret;
+ char **address = g_malloc(sizeof(uint64_t));
+ python_args_init_cast_long(address, addr, 0);
+ homer_ret = python_callback_int(module_path, homer_module, homer, address, 1);
+ python_args_clean(address, 1);
+ g_free(address);
+ return homer_ret;
+ }
switch (addr) {
case 0xe2006: /* max pstate ultra turbo */
case 0xe2018: /* pstate id for 0 */
@@ -106,6 +117,15 @@ const MemoryRegionOps pnv_homer_ops = {
static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
{
+ if (occ_module && occ) {
+ uint64_t occ_ret;
+ char **address = g_malloc(sizeof(uint64_t));
+ python_args_init_cast_long(address, addr, 0);
+ occ_ret = python_callback_int(module_path, occ_module, occ, address, 1);
+ python_args_clean(address, 1);
+ g_free(address);
+ return occ_ret;
+ }
switch (addr) {
/*
* occ-sensor sanity check that asserts the sensor
diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index 18a780bcdf..5e41b7c953 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -179,13 +179,14 @@ static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
MemTxResult result;
if (xscom_module && xscom_readp) {
- char **args = g_malloc(2 * sizeof(uint64_t));
+ char **args = g_malloc(3 * sizeof(uint64_t));
PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
python_args_init_cast_long(args, pcba, 0);
- python_args_init_cast_int(args, pcc->chip_type, 1);
+ python_args_init_cast_int(args, chip->chip_num, 1);
+ python_args_init_cast_int(args, pcc->chip_type, 2);
val = python_callback_int(module_path, xscom_module, xscom_readp,
- args, 2);
- python_args_clean(args, 2);
+ args, 3);
+ python_args_clean(args, 3);
g_free(args);
}
else {
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 9b8dc346d6..3c8119e040 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -121,6 +121,10 @@ extern const char *module_path;
extern const char *xscom_module;
extern const char *xscom_readp;
extern const char *xscom_writep;
+extern const char *homer_module;
+extern const char *homer;
+extern const char *occ_module;
+extern const char *occ;
extern int mem_prealloc;
#define MAX_NODES 128
diff --git a/vl.c b/vl.c
index 28f0dc1c1b..c96d35d907 100644
--- a/vl.c
+++ b/vl.c
@@ -144,6 +144,10 @@ const char *module_path = NULL;
const char *xscom_module = NULL;
const char *xscom_readp = NULL;
const char *xscom_writep = NULL;
+const char *homer_module = NULL;
+const char *homer = NULL;
+const char *occ_module = NULL;
+const char *occ = NULL;
int mem_prealloc = 0; /* force preallocation of physical target memory */
bool enable_mlock = false;
bool enable_cpu_pm = false;
@@ -495,6 +499,22 @@ static QemuOptsList qemu_module_opts = {
.name = "xscom_write",
.type = QEMU_OPT_STRING,
},
+ {
+ .name = "homer_module",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "homer",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "occ_module",
+ .type = QEMU_OPT_STRING,
+ },
+ {
+ .name = "occ",
+ .type = QEMU_OPT_STRING,
+ },
{ /* end of list */ }
},
};
@@ -3231,6 +3251,10 @@ int main(int argc, char **argv, char **envp)
xscom_module = qemu_opt_get(opts, "xscom_module");
xscom_readp = qemu_opt_get(opts, "xscom_read");
xscom_writep = qemu_opt_get(opts, "xscom_write");
+ homer_module = qemu_opt_get(opts, "homer_module");
+ homer = qemu_opt_get(opts, "homer");
+ occ_module = qemu_opt_get(opts, "occ_module");
+ occ = qemu_opt_get(opts, "occ");
break;
case QEMU_OPTION_mem_prealloc:
mem_prealloc = 1;
--
2.14.5
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area Balamuruhan S
@ 2019-08-07 10:27 ` Philippe Mathieu-Daudé
2019-08-11 6:05 ` Balamuruhan S
2019-08-09 4:46 ` David Gibson
1 sibling, 1 reply; 45+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-08-07 10:27 UTC (permalink / raw)
To: Balamuruhan S, qemu-devel; +Cc: maddy, anju, hari, clg, pbonzini, david
On 8/7/19 9:14 AM, Balamuruhan S wrote:
> use python interface APIs in homer/occ common area emulation to
> interact with scripts if provided else fallback to normal flow,
> it shows how simple to use the interface to call python methods
> with any number of arguments in any script placed in common
> -module-path provided in qemu commandline.
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> hw/ppc/pnv_homer.c | 20 ++++++++++++++++++++
> hw/ppc/pnv_xscom.c | 9 +++++----
> include/sysemu/sysemu.h | 4 ++++
> vl.c | 24 ++++++++++++++++++++++++
> 4 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> index 73a94856d0..6ae5e74f19 100644
> --- a/hw/ppc/pnv_homer.c
> +++ b/hw/ppc/pnv_homer.c
> @@ -16,7 +16,9 @@
> * You should have received a copy of the GNU Lesser General Public
> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> */
> +#include "sysemu/python_api.h"
> #include "qemu/osdep.h"
> +#include "sysemu/sysemu.h"
> #include "sysemu/hw_accel.h"
> #include "sysemu/cpus.h"
> #include "hw/ppc/pnv.h"
> @@ -37,6 +39,15 @@ static bool core_max_array(hwaddr addr)
>
> static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
> {
> + if (homer_module && homer) {
> + uint64_t homer_ret;
> + char **address = g_malloc(sizeof(uint64_t));
> + python_args_init_cast_long(address, addr, 0);
> + homer_ret = python_callback_int(module_path, homer_module, homer, address, 1);
> + python_args_clean(address, 1);
> + g_free(address);
Maybe the heap overhead can be simplified alloc'ing in the PnvChip
structure.
> + return homer_ret;
> + }
> switch (addr) {
> case 0xe2006: /* max pstate ultra turbo */
> case 0xe2018: /* pstate id for 0 */
> @@ -106,6 +117,15 @@ const MemoryRegionOps pnv_homer_ops = {
>
> static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
> {
> + if (occ_module && occ) {
> + uint64_t occ_ret;
> + char **address = g_malloc(sizeof(uint64_t));
> + python_args_init_cast_long(address, addr, 0);
> + occ_ret = python_callback_int(module_path, occ_module, occ, address, 1);
> + python_args_clean(address, 1);
> + g_free(address);
> + return occ_ret;
> + }
> switch (addr) {
> /*
> * occ-sensor sanity check that asserts the sensor
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index 18a780bcdf..5e41b7c953 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -179,13 +179,14 @@ static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
> MemTxResult result;
>
> if (xscom_module && xscom_readp) {
> - char **args = g_malloc(2 * sizeof(uint64_t));
> + char **args = g_malloc(3 * sizeof(uint64_t));
> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> python_args_init_cast_long(args, pcba, 0);
> - python_args_init_cast_int(args, pcc->chip_type, 1);
> + python_args_init_cast_int(args, chip->chip_num, 1);
> + python_args_init_cast_int(args, pcc->chip_type, 2);
> val = python_callback_int(module_path, xscom_module, xscom_readp,
> - args, 2);
> - python_args_clean(args, 2);
> + args, 3);
> + python_args_clean(args, 3);
> g_free(args);
> }
> else {
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 9b8dc346d6..3c8119e040 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -121,6 +121,10 @@ extern const char *module_path;
> extern const char *xscom_module;
> extern const char *xscom_readp;
> extern const char *xscom_writep;
> +extern const char *homer_module;
> +extern const char *homer;
> +extern const char *occ_module;
> +extern const char *occ;
> extern int mem_prealloc;
>
> #define MAX_NODES 128
> diff --git a/vl.c b/vl.c
> index 28f0dc1c1b..c96d35d907 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -144,6 +144,10 @@ const char *module_path = NULL;
> const char *xscom_module = NULL;
> const char *xscom_readp = NULL;
> const char *xscom_writep = NULL;
> +const char *homer_module = NULL;
> +const char *homer = NULL;
> +const char *occ_module = NULL;
> +const char *occ = NULL;
> int mem_prealloc = 0; /* force preallocation of physical target memory */
> bool enable_mlock = false;
> bool enable_cpu_pm = false;
> @@ -495,6 +499,22 @@ static QemuOptsList qemu_module_opts = {
> .name = "xscom_write",
> .type = QEMU_OPT_STRING,
> },
> + {
> + .name = "homer_module",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "homer",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "occ_module",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "occ",
> + .type = QEMU_OPT_STRING,
> + },
> { /* end of list */ }
> },
> };
> @@ -3231,6 +3251,10 @@ int main(int argc, char **argv, char **envp)
> xscom_module = qemu_opt_get(opts, "xscom_module");
> xscom_readp = qemu_opt_get(opts, "xscom_read");
> xscom_writep = qemu_opt_get(opts, "xscom_write");
> + homer_module = qemu_opt_get(opts, "homer_module");
> + homer = qemu_opt_get(opts, "homer");
> + occ_module = qemu_opt_get(opts, "occ_module");
> + occ = qemu_opt_get(opts, "occ");
> break;
> case QEMU_OPTION_mem_prealloc:
> mem_prealloc = 1;
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area
2019-08-07 10:27 ` Philippe Mathieu-Daudé
@ 2019-08-11 6:05 ` Balamuruhan S
0 siblings, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-11 6:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: maddy, anju, hari, clg, pbonzini, david
On 8/7/19 3:57 PM, Philippe Mathieu-Daudé wrote:
> On 8/7/19 9:14 AM, Balamuruhan S wrote:
>> use python interface APIs in homer/occ common area emulation to
>> interact with scripts if provided else fallback to normal flow,
>> it shows how simple to use the interface to call python methods
>> with any number of arguments in any script placed in common
>> -module-path provided in qemu commandline.
>>
>> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
>> ---
>> hw/ppc/pnv_homer.c | 20 ++++++++++++++++++++
>> hw/ppc/pnv_xscom.c | 9 +++++----
>> include/sysemu/sysemu.h | 4 ++++
>> vl.c | 24 ++++++++++++++++++++++++
>> 4 files changed, 53 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
>> index 73a94856d0..6ae5e74f19 100644
>> --- a/hw/ppc/pnv_homer.c
>> +++ b/hw/ppc/pnv_homer.c
>> @@ -16,7 +16,9 @@
>> * You should have received a copy of the GNU Lesser General Public
>> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> */
>> +#include "sysemu/python_api.h"
>> #include "qemu/osdep.h"
>> +#include "sysemu/sysemu.h"
>> #include "sysemu/hw_accel.h"
>> #include "sysemu/cpus.h"
>> #include "hw/ppc/pnv.h"
>> @@ -37,6 +39,15 @@ static bool core_max_array(hwaddr addr)
>>
>> static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
>> {
>> + if (homer_module && homer) {
>> + uint64_t homer_ret;
>> + char **address = g_malloc(sizeof(uint64_t));
>> + python_args_init_cast_long(address, addr, 0);
>> + homer_ret = python_callback_int(module_path, homer_module, homer, address, 1);
>> + python_args_clean(address, 1);
>> + g_free(address);
> Maybe the heap overhead can be simplified alloc'ing in the PnvChip
> structure.
But it also depends on with how many arguments that we need to call python
functions associated with read/write ops. sure, I will check the way to
adopt this suggestion.
Thanks Philippe.
>
>> + return homer_ret;
>> + }
>> switch (addr) {
>> case 0xe2006: /* max pstate ultra turbo */
>> case 0xe2018: /* pstate id for 0 */
>> @@ -106,6 +117,15 @@ const MemoryRegionOps pnv_homer_ops = {
>>
>> static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
>> {
>> + if (occ_module && occ) {
>> + uint64_t occ_ret;
>> + char **address = g_malloc(sizeof(uint64_t));
>> + python_args_init_cast_long(address, addr, 0);
>> + occ_ret = python_callback_int(module_path, occ_module, occ, address, 1);
>> + python_args_clean(address, 1);
>> + g_free(address);
>> + return occ_ret;
>> + }
>> switch (addr) {
>> /*
>> * occ-sensor sanity check that asserts the sensor
>> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
>> index 18a780bcdf..5e41b7c953 100644
>> --- a/hw/ppc/pnv_xscom.c
>> +++ b/hw/ppc/pnv_xscom.c
>> @@ -179,13 +179,14 @@ static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
>> MemTxResult result;
>>
>> if (xscom_module && xscom_readp) {
>> - char **args = g_malloc(2 * sizeof(uint64_t));
>> + char **args = g_malloc(3 * sizeof(uint64_t));
>> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>> python_args_init_cast_long(args, pcba, 0);
>> - python_args_init_cast_int(args, pcc->chip_type, 1);
>> + python_args_init_cast_int(args, chip->chip_num, 1);
>> + python_args_init_cast_int(args, pcc->chip_type, 2);
>> val = python_callback_int(module_path, xscom_module, xscom_readp,
>> - args, 2);
>> - python_args_clean(args, 2);
>> + args, 3);
>> + python_args_clean(args, 3);
>> g_free(args);
>> }
>> else {
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 9b8dc346d6..3c8119e040 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -121,6 +121,10 @@ extern const char *module_path;
>> extern const char *xscom_module;
>> extern const char *xscom_readp;
>> extern const char *xscom_writep;
>> +extern const char *homer_module;
>> +extern const char *homer;
>> +extern const char *occ_module;
>> +extern const char *occ;
>> extern int mem_prealloc;
>>
>> #define MAX_NODES 128
>> diff --git a/vl.c b/vl.c
>> index 28f0dc1c1b..c96d35d907 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -144,6 +144,10 @@ const char *module_path = NULL;
>> const char *xscom_module = NULL;
>> const char *xscom_readp = NULL;
>> const char *xscom_writep = NULL;
>> +const char *homer_module = NULL;
>> +const char *homer = NULL;
>> +const char *occ_module = NULL;
>> +const char *occ = NULL;
>> int mem_prealloc = 0; /* force preallocation of physical target memory */
>> bool enable_mlock = false;
>> bool enable_cpu_pm = false;
>> @@ -495,6 +499,22 @@ static QemuOptsList qemu_module_opts = {
>> .name = "xscom_write",
>> .type = QEMU_OPT_STRING,
>> },
>> + {
>> + .name = "homer_module",
>> + .type = QEMU_OPT_STRING,
>> + },
>> + {
>> + .name = "homer",
>> + .type = QEMU_OPT_STRING,
>> + },
>> + {
>> + .name = "occ_module",
>> + .type = QEMU_OPT_STRING,
>> + },
>> + {
>> + .name = "occ",
>> + .type = QEMU_OPT_STRING,
>> + },
>> { /* end of list */ }
>> },
>> };
>> @@ -3231,6 +3251,10 @@ int main(int argc, char **argv, char **envp)
>> xscom_module = qemu_opt_get(opts, "xscom_module");
>> xscom_readp = qemu_opt_get(opts, "xscom_read");
>> xscom_writep = qemu_opt_get(opts, "xscom_write");
>> + homer_module = qemu_opt_get(opts, "homer_module");
>> + homer = qemu_opt_get(opts, "homer");
>> + occ_module = qemu_opt_get(opts, "occ_module");
>> + occ = qemu_opt_get(opts, "occ");
>> break;
>> case QEMU_OPTION_mem_prealloc:
>> mem_prealloc = 1;
>>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area Balamuruhan S
2019-08-07 10:27 ` Philippe Mathieu-Daudé
@ 2019-08-09 4:46 ` David Gibson
2019-08-11 6:19 ` Balamuruhan S
1 sibling, 1 reply; 45+ messages in thread
From: David Gibson @ 2019-08-09 4:46 UTC (permalink / raw)
To: Balamuruhan S; +Cc: maddy, anju, qemu-devel, hari, clg, pbonzini
[-- Attachment #1: Type: text/plain, Size: 6120 bytes --]
On Wed, Aug 07, 2019 at 12:44:45PM +0530, Balamuruhan S wrote:
> use python interface APIs in homer/occ common area emulation to
> interact with scripts if provided else fallback to normal flow,
> it shows how simple to use the interface to call python methods
> with any number of arguments in any script placed in common
> -module-path provided in qemu commandline.
What's the use case for this?
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> hw/ppc/pnv_homer.c | 20 ++++++++++++++++++++
> hw/ppc/pnv_xscom.c | 9 +++++----
> include/sysemu/sysemu.h | 4 ++++
> vl.c | 24 ++++++++++++++++++++++++
> 4 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> index 73a94856d0..6ae5e74f19 100644
> --- a/hw/ppc/pnv_homer.c
> +++ b/hw/ppc/pnv_homer.c
> @@ -16,7 +16,9 @@
> * You should have received a copy of the GNU Lesser General Public
> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> */
> +#include "sysemu/python_api.h"
> #include "qemu/osdep.h"
> +#include "sysemu/sysemu.h"
> #include "sysemu/hw_accel.h"
> #include "sysemu/cpus.h"
> #include "hw/ppc/pnv.h"
> @@ -37,6 +39,15 @@ static bool core_max_array(hwaddr addr)
>
> static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
> {
> + if (homer_module && homer) {
> + uint64_t homer_ret;
> + char **address = g_malloc(sizeof(uint64_t));
> + python_args_init_cast_long(address, addr, 0);
> + homer_ret = python_callback_int(module_path, homer_module, homer, address, 1);
> + python_args_clean(address, 1);
> + g_free(address);
> + return homer_ret;
> + }
> switch (addr) {
> case 0xe2006: /* max pstate ultra turbo */
> case 0xe2018: /* pstate id for 0 */
> @@ -106,6 +117,15 @@ const MemoryRegionOps pnv_homer_ops = {
>
> static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
> {
> + if (occ_module && occ) {
> + uint64_t occ_ret;
> + char **address = g_malloc(sizeof(uint64_t));
> + python_args_init_cast_long(address, addr, 0);
> + occ_ret = python_callback_int(module_path, occ_module, occ, address, 1);
> + python_args_clean(address, 1);
> + g_free(address);
> + return occ_ret;
> + }
> switch (addr) {
> /*
> * occ-sensor sanity check that asserts the sensor
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index 18a780bcdf..5e41b7c953 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -179,13 +179,14 @@ static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
> MemTxResult result;
>
> if (xscom_module && xscom_readp) {
> - char **args = g_malloc(2 * sizeof(uint64_t));
> + char **args = g_malloc(3 * sizeof(uint64_t));
> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> python_args_init_cast_long(args, pcba, 0);
> - python_args_init_cast_int(args, pcc->chip_type, 1);
> + python_args_init_cast_int(args, chip->chip_num, 1);
> + python_args_init_cast_int(args, pcc->chip_type, 2);
> val = python_callback_int(module_path, xscom_module, xscom_readp,
> - args, 2);
> - python_args_clean(args, 2);
> + args, 3);
> + python_args_clean(args, 3);
> g_free(args);
> }
> else {
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 9b8dc346d6..3c8119e040 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -121,6 +121,10 @@ extern const char *module_path;
> extern const char *xscom_module;
> extern const char *xscom_readp;
> extern const char *xscom_writep;
> +extern const char *homer_module;
> +extern const char *homer;
> +extern const char *occ_module;
> +extern const char *occ;
> extern int mem_prealloc;
>
> #define MAX_NODES 128
> diff --git a/vl.c b/vl.c
> index 28f0dc1c1b..c96d35d907 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -144,6 +144,10 @@ const char *module_path = NULL;
> const char *xscom_module = NULL;
> const char *xscom_readp = NULL;
> const char *xscom_writep = NULL;
> +const char *homer_module = NULL;
> +const char *homer = NULL;
> +const char *occ_module = NULL;
> +const char *occ = NULL;
> int mem_prealloc = 0; /* force preallocation of physical target memory */
> bool enable_mlock = false;
> bool enable_cpu_pm = false;
> @@ -495,6 +499,22 @@ static QemuOptsList qemu_module_opts = {
> .name = "xscom_write",
> .type = QEMU_OPT_STRING,
> },
> + {
> + .name = "homer_module",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "homer",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "occ_module",
> + .type = QEMU_OPT_STRING,
> + },
> + {
> + .name = "occ",
> + .type = QEMU_OPT_STRING,
> + },
> { /* end of list */ }
> },
> };
> @@ -3231,6 +3251,10 @@ int main(int argc, char **argv, char **envp)
> xscom_module = qemu_opt_get(opts, "xscom_module");
> xscom_readp = qemu_opt_get(opts, "xscom_read");
> xscom_writep = qemu_opt_get(opts, "xscom_write");
> + homer_module = qemu_opt_get(opts, "homer_module");
> + homer = qemu_opt_get(opts, "homer");
> + occ_module = qemu_opt_get(opts, "occ_module");
> + occ = qemu_opt_get(opts, "occ");
> break;
> case QEMU_OPTION_mem_prealloc:
> mem_prealloc = 1;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area
2019-08-09 4:46 ` David Gibson
@ 2019-08-11 6:19 ` Balamuruhan S
0 siblings, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-11 6:19 UTC (permalink / raw)
To: David Gibson; +Cc: maddy, anju, qemu-devel, hari, clg, pbonzini
On 8/9/19 10:16 AM, David Gibson wrote:
> On Wed, Aug 07, 2019 at 12:44:45PM +0530, Balamuruhan S wrote:
>> use python interface APIs in homer/occ common area emulation to
>> interact with scripts if provided else fallback to normal flow,
>> it shows how simple to use the interface to call python methods
>> with any number of arguments in any script placed in common
>> -module-path provided in qemu commandline.
> What's the use case for this?
The usecase can be performing multiple boot test of the vm or during runtime
with different values expected/unexpected by firmware/kernel and check
how it is behaving or how it should behave.
It can be used as a framework for CI or regression.
>
>> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
>> ---
>> hw/ppc/pnv_homer.c | 20 ++++++++++++++++++++
>> hw/ppc/pnv_xscom.c | 9 +++++----
>> include/sysemu/sysemu.h | 4 ++++
>> vl.c | 24 ++++++++++++++++++++++++
>> 4 files changed, 53 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
>> index 73a94856d0..6ae5e74f19 100644
>> --- a/hw/ppc/pnv_homer.c
>> +++ b/hw/ppc/pnv_homer.c
>> @@ -16,7 +16,9 @@
>> * You should have received a copy of the GNU Lesser General Public
>> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> */
>> +#include "sysemu/python_api.h"
>> #include "qemu/osdep.h"
>> +#include "sysemu/sysemu.h"
>> #include "sysemu/hw_accel.h"
>> #include "sysemu/cpus.h"
>> #include "hw/ppc/pnv.h"
>> @@ -37,6 +39,15 @@ static bool core_max_array(hwaddr addr)
>>
>> static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
>> {
>> + if (homer_module && homer) {
>> + uint64_t homer_ret;
>> + char **address = g_malloc(sizeof(uint64_t));
>> + python_args_init_cast_long(address, addr, 0);
>> + homer_ret = python_callback_int(module_path, homer_module, homer, address, 1);
>> + python_args_clean(address, 1);
>> + g_free(address);
>> + return homer_ret;
>> + }
>> switch (addr) {
>> case 0xe2006: /* max pstate ultra turbo */
>> case 0xe2018: /* pstate id for 0 */
>> @@ -106,6 +117,15 @@ const MemoryRegionOps pnv_homer_ops = {
>>
>> static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width)
>> {
>> + if (occ_module && occ) {
>> + uint64_t occ_ret;
>> + char **address = g_malloc(sizeof(uint64_t));
>> + python_args_init_cast_long(address, addr, 0);
>> + occ_ret = python_callback_int(module_path, occ_module, occ, address, 1);
>> + python_args_clean(address, 1);
>> + g_free(address);
>> + return occ_ret;
>> + }
>> switch (addr) {
>> /*
>> * occ-sensor sanity check that asserts the sensor
>> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
>> index 18a780bcdf..5e41b7c953 100644
>> --- a/hw/ppc/pnv_xscom.c
>> +++ b/hw/ppc/pnv_xscom.c
>> @@ -179,13 +179,14 @@ static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
>> MemTxResult result;
>>
>> if (xscom_module && xscom_readp) {
>> - char **args = g_malloc(2 * sizeof(uint64_t));
>> + char **args = g_malloc(3 * sizeof(uint64_t));
>> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>> python_args_init_cast_long(args, pcba, 0);
>> - python_args_init_cast_int(args, pcc->chip_type, 1);
>> + python_args_init_cast_int(args, chip->chip_num, 1);
>> + python_args_init_cast_int(args, pcc->chip_type, 2);
>> val = python_callback_int(module_path, xscom_module, xscom_readp,
>> - args, 2);
>> - python_args_clean(args, 2);
>> + args, 3);
>> + python_args_clean(args, 3);
>> g_free(args);
>> }
>> else {
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 9b8dc346d6..3c8119e040 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -121,6 +121,10 @@ extern const char *module_path;
>> extern const char *xscom_module;
>> extern const char *xscom_readp;
>> extern const char *xscom_writep;
>> +extern const char *homer_module;
>> +extern const char *homer;
>> +extern const char *occ_module;
>> +extern const char *occ;
>> extern int mem_prealloc;
>>
>> #define MAX_NODES 128
>> diff --git a/vl.c b/vl.c
>> index 28f0dc1c1b..c96d35d907 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -144,6 +144,10 @@ const char *module_path = NULL;
>> const char *xscom_module = NULL;
>> const char *xscom_readp = NULL;
>> const char *xscom_writep = NULL;
>> +const char *homer_module = NULL;
>> +const char *homer = NULL;
>> +const char *occ_module = NULL;
>> +const char *occ = NULL;
>> int mem_prealloc = 0; /* force preallocation of physical target memory */
>> bool enable_mlock = false;
>> bool enable_cpu_pm = false;
>> @@ -495,6 +499,22 @@ static QemuOptsList qemu_module_opts = {
>> .name = "xscom_write",
>> .type = QEMU_OPT_STRING,
>> },
>> + {
>> + .name = "homer_module",
>> + .type = QEMU_OPT_STRING,
>> + },
>> + {
>> + .name = "homer",
>> + .type = QEMU_OPT_STRING,
>> + },
>> + {
>> + .name = "occ_module",
>> + .type = QEMU_OPT_STRING,
>> + },
>> + {
>> + .name = "occ",
>> + .type = QEMU_OPT_STRING,
>> + },
>> { /* end of list */ }
>> },
>> };
>> @@ -3231,6 +3251,10 @@ int main(int argc, char **argv, char **envp)
>> xscom_module = qemu_opt_get(opts, "xscom_module");
>> xscom_readp = qemu_opt_get(opts, "xscom_read");
>> xscom_writep = qemu_opt_get(opts, "xscom_write");
>> + homer_module = qemu_opt_get(opts, "homer_module");
>> + homer = qemu_opt_get(opts, "homer");
>> + occ_module = qemu_opt_get(opts, "occ_module");
>> + occ = qemu_opt_get(opts, "occ");
>> break;
>> case QEMU_OPTION_mem_prealloc:
>> mem_prealloc = 1;
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
` (5 preceding siblings ...)
2019-08-07 7:14 ` [Qemu-devel] [RFC PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area Balamuruhan S
@ 2019-08-07 7:33 ` no-reply
2019-08-07 8:15 ` Cédric Le Goater
` (3 subsequent siblings)
10 siblings, 0 replies; 45+ messages in thread
From: no-reply @ 2019-08-07 7:33 UTC (permalink / raw)
To: bala24; +Cc: maddy, qemu-devel, bala24, anju, clg, hari, pbonzini, david
Patchew URL: https://patchew.org/QEMU/20190807071445.4109-1-bala24@linux.ibm.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
Message-id: 20190807071445.4109-1-bala24@linux.ibm.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20190807071445.4109-1-bala24@linux.ibm.com -> patchew/20190807071445.4109-1-bala24@linux.ibm.com
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF'
Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 'roms/edk2'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware'
Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for path 'roms/opensbi'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 'slirp'
Submodule 'tests/fp/berkeley-softfloat-3' (https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' (https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 'ba1ab360eebe6338bb8d7d83a9220ccf7e213af3'
Cloning into 'roms/edk2'...
Submodule path 'roms/edk2': checked out '20d2e5a125e34fc8501026613a71549b2a1a3e54'
Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'
Submodule 'CryptoPkg/Library/OpensslLib/openssl' (https://github.com/openssl/openssl) registered for path 'CryptoPkg/Library/OpensslLib/openssl'
Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'...
Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'CryptoPkg/Library/OpensslLib/openssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out '50eaac9f3337667259de725451f201e784599687'
Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered for path 'boringssl'
Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5'
Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) registered for path 'pyca-cryptography'
Cloning into 'boringssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f'
Cloning into 'krb5'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked out 'b9ad6c49505c96a088326b62a52568e3484f2168'
Cloning into 'pyca-cryptography'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/pyca-cryptography': checked out '09403100de2f6f1cdd0d484dcb8e620f1c335c8f'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out 'c79e0ecb84f4f1ee3f73f521622e264edd1bf174'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/opensbi'...
Submodule path 'roms/opensbi': checked out 'ce228ee0919deb9957192d723eecc8aaae2697c6'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out 'bf0e13698872450164fa7040da36a95d2d4b326f'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 'a5cab58e9a3fb6e168aba919c5669bea406573b4'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out '0f4fe84658165e96ce35870fd19fc634e182e77b'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out '261ca8e779e5138869a45f174caa49be6a274501'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 'd3689267f92c5956e09cc7d1baa4700141662bff'
Cloning into 'roms/u-boot-sam460ex'...
Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588'
Cloning into 'slirp'...
Submodule path 'slirp': checked out '126c04acbabd7ad32c2b018fe10dfac2a3bc1210'
Cloning into 'tests/fp/berkeley-softfloat-3'...
Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'tests/fp/berkeley-testfloat-3'...
Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'
0dabfa2 hw/ppc/pnv_homer: add python interface support for homer/occ common area
aa6063d hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
43e9902 hw/ppc/pnv: initialize and realize homer/occ common area
a4755fe hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV
4cd625a hw/ppc/pnv_xscom: extend xscom to use python interface
9ad8c48 utils/python_api: add scripting interface for Qemu with python lib
=== OUTPUT BEGIN ===
1/6 Checking commit 9ad8c482b9ae (utils/python_api: add scripting interface for Qemu with python lib)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#45:
new file mode 100644
ERROR: braces {} are necessary for all arms of this statement
#109: FILE: util/python_api.c:12:
+ if (!abs_module_path)
[...]
ERROR: else should follow close brace '}'
#141: FILE: util/python_api.c:44:
+ }
+ else {
ERROR: braces {} are necessary for all arms of this statement
#142: FILE: util/python_api.c:45:
+ if (PyErr_Occurred())
[...]
ERROR: spaces required around that '=' (ctx:VxW)
#182: FILE: util/python_api.c:85:
+ args[pos]= malloc(sizeof(int));
^
ERROR: spaces required around that '=' (ctx:VxW)
#188: FILE: util/python_api.c:91:
+ args[pos]= g_malloc(sizeof(uint64_t) * 2);
^
total: 5 errors, 1 warnings, 159 lines checked
Patch 1/6 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/6 Checking commit 4cd625a90b2a (hw/ppc/pnv_xscom: extend xscom to use python interface)
ERROR: else should follow close brace '}'
#48: FILE: hw/ppc/pnv_xscom.c:172:
+ }
+ else {
ERROR: braces {} are necessary for all arms of this statement
#67: FILE: hw/ppc/pnv_xscom.c:209:
+ if (xscom_success)
[...]
ERROR: do not initialise globals to 0 or NULL
#124: FILE: vl.c:143:
+const char *module_path = NULL;
ERROR: do not initialise globals to 0 or NULL
#125: FILE: vl.c:144:
+const char *xscom_module = NULL;
ERROR: do not initialise globals to 0 or NULL
#126: FILE: vl.c:145:
+const char *xscom_readp = NULL;
ERROR: do not initialise globals to 0 or NULL
#127: FILE: vl.c:146:
+const char *xscom_writep = NULL;
WARNING: Block comments use a leading /* on a separate line
#157: FILE: vl.c:498:
+ { /* end of list */ }
total: 6 errors, 1 warnings, 151 lines checked
Patch 2/6 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/6 Checking commit a4755feccbee (hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36:
new file mode 100644
ERROR: braces {} are necessary for all arms of this statement
#70: FILE: hw/ppc/pnv_homer.c:30:
+ if (cpu_type)
[...]
ERROR: suspect code indent for conditional statements (4, 7)
#72: FILE: hw/ppc/pnv_homer.c:32:
+ for (int i = 0; i <= ms->smp.cores; i++)
+ if (addr == (core_max_base + i))
ERROR: suspect code indent for conditional statements (7, 11)
#73: FILE: hw/ppc/pnv_homer.c:33:
+ if (addr == (core_max_base + i))
+ return true;
ERROR: braces {} are necessary for all arms of this statement
#73: FILE: hw/ppc/pnv_homer.c:33:
+ if (addr == (core_max_base + i))
[...]
ERROR: switch and case should be at the same indent
#80: FILE: hw/ppc/pnv_homer.c:40:
+ switch (addr) {
+ case 0xe2006: /* max pstate ultra turbo */
+ case 0xe2018: /* pstate id for 0 */
+ case 0x1f8001: /* P8 occ pstate version */
+ case 0x1f8003: /* P8 pstate min */
+ case 0x1f8010: /* P8 pstate id for 0 */
[...]
+ case 0xe2000: /* occ data area */
+ case 0xe2002: /* occ_role master/slave*/
+ case 0xe2004: /* pstate nom */
+ case 0xe2005: /* pstate turbo */
+ case 0xe2020: /* pstate id for 1 */
+ case 0xe2818: /* pstate ultra turbo */
+ case 0xe2b85: /* opal dynamic data (runtime) */
+ case 0x1f8000: /* P8 occ pstate valid */
+ case 0x1f8002: /* P8 throttle */
+ case 0x1f8004: /* P8 pstate nom */
+ case 0x1f8005: /* P8 pstate turbo */
+ case 0x1f8012: /* vdd voltage identifier */
+ case 0x1f8013: /* vcs voltage identifier */
+ case 0x1f8018: /* P8 pstate id for 1 */
[...]
+ case 0xe2003: /* pstate min (2 as pstate min) */
+ case 0xe2028: /* pstate id for 2 */
+ case 0x1f8006: /* P8 pstate ultra turbo */
+ case 0x1f8020: /* P8 pstate id for 2 */
[...]
+ case 0xe2001: /* major version */
[...]
+ case 0xe201c:
+ case 0xe2024:
+ case 0xe202c:
[...]
+ case 0x1f8014:
+ case 0x1f801c:
+ case 0x1f8024:
[...]
+ case 0x0: /* homer base */
+ case 0xe2008: /* occ data area + 8 */
+ case 0x1f8008: /* P8 occ data area + 8 */
+ case 0x200008: /* homer base access to get homer image pointer*/
ERROR: braces {} are necessary for all arms of this statement
#125: FILE: hw/ppc/pnv_homer.c:85:
+ if (core_max_array(addr))
[...]
ERROR: switch and case should be at the same indent
#149: FILE: hw/ppc/pnv_homer.c:109:
+ switch (addr) {
[...]
+ case 0x580000: /* occ sensor data block */
+ case 0x580001: /* valid */
+ case 0x580002: /* version */
+ case 0x580004: /* reading_version */
+ case 0x580008: /* nr_sensors */
+ case 0x580010: /* names_offset */
+ case 0x580014: /* reading_ping_offset */
+ case 0x58000c: /* reading_pong_offset */
+ case 0x580023: /* structure_type */
[...]
+ case 0x58000d: /* name length */
[...]
+ case 0x580022: /* occ sensor loc core */
[...]
+ case 0x580003: /* occ sensor type power */
[...]
+ case 0x580005: /* sensor name */
[...]
+ case 0x58001e: /* HWMON_SENSORS_MASK */
+ case 0x580020:
[...]
+ case 0x0: /* P8 slw base access for slw image size */
WARNING: line over 80 characters
#316: FILE: include/hw/ppc/pnv_homer.h:39:
+ (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE)
total: 7 errors, 2 warnings, 272 lines checked
Patch 3/6 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/6 Checking commit 43e990229339 (hw/ppc/pnv: initialize and realize homer/occ common area)
5/6 Checking commit aa6063d8ca2f (hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs)
6/6 Checking commit 0dabfa28124e (hw/ppc/pnv_homer: add python interface support for homer/occ common area)
WARNING: line over 80 characters
#38: FILE: hw/ppc/pnv_homer.c:46:
+ homer_ret = python_callback_int(module_path, homer_module, homer, address, 1);
ERROR: do not initialise globals to 0 or NULL
#108: FILE: vl.c:147:
+const char *homer_module = NULL;
ERROR: do not initialise globals to 0 or NULL
#109: FILE: vl.c:148:
+const char *homer = NULL;
ERROR: do not initialise globals to 0 or NULL
#110: FILE: vl.c:149:
+const char *occ_module = NULL;
ERROR: do not initialise globals to 0 or NULL
#111: FILE: vl.c:150:
+const char *occ = NULL;
total: 4 errors, 1 warnings, 109 lines checked
Patch 6/6 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190807071445.4109-1-bala24@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
` (6 preceding siblings ...)
2019-08-07 7:33 ` [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface no-reply
@ 2019-08-07 8:15 ` Cédric Le Goater
2019-08-07 10:16 ` Balamuruhan S
2019-08-09 4:49 ` David Gibson
2019-08-07 8:51 ` no-reply
` (2 subsequent siblings)
10 siblings, 2 replies; 45+ messages in thread
From: Cédric Le Goater @ 2019-08-07 8:15 UTC (permalink / raw)
To: Balamuruhan S, qemu-devel
Cc: Peter Maydell, maddy, anju, hari, pbonzini, david
On 07/08/2019 09:14, Balamuruhan S wrote:
> Hi All,
>
> This is a proposal to extend mmio callbacks in Qemu with scripting interface
> that is prototyped with python in this implementation. It gives ability to
> feed runtime data through callbacks without recompiling Qemu in generic way.
> This patchset adds library that provides APIs for Qemu to talk with python
> scripts placed in path -module-path and how existing xscom can be extended
> with python interface infrastructure.
>
> We have also added an hacky emulation for memory region (OCC common area and HOMER)
> which is shared between core and un-core engine (ideally this should be via
> sram device) to showcase the effectiveness of having the scripting interface
> (uncore engine taken for discussion here is powerpc specificed called OCC).
We should try to merge this part first. It is useful as it is after some
cleanups.
> Having scripting interface helps to emulate/test different uncore-core
> interactions including uncore engine failure or hang. It also helps in feeding
> randomized data at byte level access. This patchset is primarily to extend mmio
> callbacks with scripting interface and to demonstrate effectiveness it.
It is already possible to feed device models with external data using QMP or
external agents using a chardev backend transport. What are the benefits
of using the embedded python approach ?
> Some changes are required in PowerPC skiboot tree to test these changes since
> the memory region is disabled currently for Qemu emulated PowerNV host,
> https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
You should send that patch.
Thanks,
C.
> Qemu commandline used to test,
>
> ```
> # qemu/ppc64-softmmu/qemu-system-ppc64 \
> -M powernv \
> -cpu POWER9 \
> -m 16G \
> -kernel vmlinux \
> -initrd debug_homer.cpio \
> -nographic -bios skiboot/skiboot.lid \
> -module-path /home/bala/homer/python-modules/,xscom_module=homer,xscom_read=xscom_read,xscom_write=xscom_write,homer_module=homer,homer=homer_read,occ_module=homer,occ=occ_read
> ```
>
> Script used to feed data can be something like,
> https://github.com/balamuruhans/python-modules/blob/master/script.py
>
> It could uncover couple of firmware bugs,
> https://github.com/balamuruhans/skiboot/commit/fd3d93d92ec66a7494346d6d24ced7b48264c9a0
> https://github.com/balamuruhans/skiboot/commit/165b3829a93bc177c18133945a8cca3a2d701173
>
> Code changes:
> Patch 1: adds library to provide python interface APIs
> Patch 2: extend existing xscom to adopt this python interface
> Patch 3 - 6: emulate uncore/core shared memory region with mmio callbacks and
> add support with this infrastructure.
>
> I request for comments, suggestions, ideas on getting a scripting interface
> like python added in qemu.
>
> Balamuruhan S (6):
> utils/python_api: add scripting interface for Qemu with python lib
> hw/ppc/pnv_xscom: extend xscom to use python interface
> hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV
> hw/ppc/pnv: initialize and realize homer/occ common area
> hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
> hw/ppc/pnv_homer: add python interface support for homer/occ common
> area
>
> configure | 10 +++
> hw/ppc/Makefile.objs | 2 +-
> hw/ppc/pnv.c | 49 ++++++++++-
> hw/ppc/pnv_homer.c | 205 ++++++++++++++++++++++++++++++++++++++++++++
> hw/ppc/pnv_xscom.c | 59 +++++++++++--
> include/hw/ppc/pnv.h | 15 ++++
> include/hw/ppc/pnv_homer.h | 41 +++++++++
> include/sysemu/python_api.h | 30 +++++++
> include/sysemu/sysemu.h | 8 ++
> qemu-options.hx | 14 +++
> util/Makefile.objs | 1 +
> util/python_api.c | 100 +++++++++++++++++++++
> vl.c | 66 ++++++++++++++
> 13 files changed, 588 insertions(+), 12 deletions(-)
> create mode 100644 hw/ppc/pnv_homer.c
> create mode 100644 include/hw/ppc/pnv_homer.h
> create mode 100644 include/sysemu/python_api.h
> create mode 100644 util/python_api.c
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
2019-08-07 8:15 ` Cédric Le Goater
@ 2019-08-07 10:16 ` Balamuruhan S
2019-08-09 4:49 ` David Gibson
1 sibling, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-07 10:16 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: hari, anju, qemu-devel, maddy, pbonzini, david
On Wed, Aug 07, 2019 at 10:15:48AM +0200, Cédric Le Goater wrote:
> On 07/08/2019 09:14, Balamuruhan S wrote:
> > Hi All,
> >
> > This is a proposal to extend mmio callbacks in Qemu with scripting interface
> > that is prototyped with python in this implementation. It gives ability to
> > feed runtime data through callbacks without recompiling Qemu in generic way.
> > This patchset adds library that provides APIs for Qemu to talk with python
> > scripts placed in path -module-path and how existing xscom can be extended
> > with python interface infrastructure.
> >
> > We have also added an hacky emulation for memory region (OCC common area and HOMER)
> > which is shared between core and un-core engine (ideally this should be via
> > sram device) to showcase the effectiveness of having the scripting interface
> > (uncore engine taken for discussion here is powerpc specificed called OCC).
>
> We should try to merge this part first. It is useful as it is after some
> cleanups.
okay :+1:
>
> > Having scripting interface helps to emulate/test different uncore-core
> > interactions including uncore engine failure or hang. It also helps in feeding
> > randomized data at byte level access. This patchset is primarily to extend mmio
> > callbacks with scripting interface and to demonstrate effectiveness it.
>
> It is already possible to feed device models with external data using QMP or
> external agents using a chardev backend transport. What are the benefits
> of using the embedded python approach ?
>
> > Some changes are required in PowerPC skiboot tree to test these changes since
> > the memory region is disabled currently for Qemu emulated PowerNV host,
> > https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
>
> You should send that patch.
okay, I will send it.
Thank you Cedric for your review and suggestions.
-- Bala
>
> Thanks,
>
> C.
>
> > Qemu commandline used to test,
> >
> > ```
> > # qemu/ppc64-softmmu/qemu-system-ppc64 \
> > -M powernv \
> > -cpu POWER9 \
> > -m 16G \
> > -kernel vmlinux \
> > -initrd debug_homer.cpio \
> > -nographic -bios skiboot/skiboot.lid \
> > -module-path /home/bala/homer/python-modules/,xscom_module=homer,xscom_read=xscom_read,xscom_write=xscom_write,homer_module=homer,homer=homer_read,occ_module=homer,occ=occ_read
> > ```
> >
> > Script used to feed data can be something like,
> > https://github.com/balamuruhans/python-modules/blob/master/script.py
> >
> > It could uncover couple of firmware bugs,
> > https://github.com/balamuruhans/skiboot/commit/fd3d93d92ec66a7494346d6d24ced7b48264c9a0
> > https://github.com/balamuruhans/skiboot/commit/165b3829a93bc177c18133945a8cca3a2d701173
> >
> > Code changes:
> > Patch 1: adds library to provide python interface APIs
> > Patch 2: extend existing xscom to adopt this python interface
> > Patch 3 - 6: emulate uncore/core shared memory region with mmio callbacks and
> > add support with this infrastructure.
> >
> > I request for comments, suggestions, ideas on getting a scripting interface
> > like python added in qemu.
> >
> > Balamuruhan S (6):
> > utils/python_api: add scripting interface for Qemu with python lib
> > hw/ppc/pnv_xscom: extend xscom to use python interface
> > hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV
> > hw/ppc/pnv: initialize and realize homer/occ common area
> > hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
> > hw/ppc/pnv_homer: add python interface support for homer/occ common
> > area
> >
> > configure | 10 +++
> > hw/ppc/Makefile.objs | 2 +-
> > hw/ppc/pnv.c | 49 ++++++++++-
> > hw/ppc/pnv_homer.c | 205 ++++++++++++++++++++++++++++++++++++++++++++
> > hw/ppc/pnv_xscom.c | 59 +++++++++++--
> > include/hw/ppc/pnv.h | 15 ++++
> > include/hw/ppc/pnv_homer.h | 41 +++++++++
> > include/sysemu/python_api.h | 30 +++++++
> > include/sysemu/sysemu.h | 8 ++
> > qemu-options.hx | 14 +++
> > util/Makefile.objs | 1 +
> > util/python_api.c | 100 +++++++++++++++++++++
> > vl.c | 66 ++++++++++++++
> > 13 files changed, 588 insertions(+), 12 deletions(-)
> > create mode 100644 hw/ppc/pnv_homer.c
> > create mode 100644 include/hw/ppc/pnv_homer.h
> > create mode 100644 include/sysemu/python_api.h
> > create mode 100644 util/python_api.c
> >
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
2019-08-07 8:15 ` Cédric Le Goater
2019-08-07 10:16 ` Balamuruhan S
@ 2019-08-09 4:49 ` David Gibson
2019-08-12 5:07 ` Balamuruhan S
1 sibling, 1 reply; 45+ messages in thread
From: David Gibson @ 2019-08-09 4:49 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Maydell, maddy, qemu-devel, Balamuruhan S, anju, hari, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]
On Wed, Aug 07, 2019 at 10:15:48AM +0200, Cédric Le Goater wrote:
> On 07/08/2019 09:14, Balamuruhan S wrote:
> > Hi All,
> >
> > This is a proposal to extend mmio callbacks in Qemu with scripting interface
> > that is prototyped with python in this implementation. It gives ability to
> > feed runtime data through callbacks without recompiling Qemu in generic way.
> > This patchset adds library that provides APIs for Qemu to talk with python
> > scripts placed in path -module-path and how existing xscom can be extended
> > with python interface infrastructure.
> >
> > We have also added an hacky emulation for memory region (OCC common area and HOMER)
> > which is shared between core and un-core engine (ideally this should be via
> > sram device) to showcase the effectiveness of having the scripting interface
> > (uncore engine taken for discussion here is powerpc specificed called OCC).
>
> We should try to merge this part first. It is useful as it is after some
> cleanups.
>
> > Having scripting interface helps to emulate/test different uncore-core
> > interactions including uncore engine failure or hang. It also helps in feeding
> > randomized data at byte level access. This patchset is primarily to extend mmio
> > callbacks with scripting interface and to demonstrate effectiveness it.
>
> It is already possible to feed device models with external data using QMP or
> external agents using a chardev backend transport. What are the benefits
> of using the embedded python approach ?
Yeah, I also think this needs better justification.
In particular what's the case that Python makes this significantly
easier than hacking up experimental interactions with C. I mean you
already have to understand POWER9 internals to work with this, right,
so I wouldn't expect Python's greater accessibility to be a big
concern here.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
2019-08-09 4:49 ` David Gibson
@ 2019-08-12 5:07 ` Balamuruhan S
0 siblings, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-12 5:07 UTC (permalink / raw)
To: David Gibson, Cédric Le Goater
Cc: Peter Maydell, maddy, anju, qemu-devel, hari, pbonzini
On 8/9/19 10:19 AM, David Gibson wrote:
> On Wed, Aug 07, 2019 at 10:15:48AM +0200, Cédric Le Goater wrote:
>> On 07/08/2019 09:14, Balamuruhan S wrote:
>>> Hi All,
>>>
>>> This is a proposal to extend mmio callbacks in Qemu with scripting interface
>>> that is prototyped with python in this implementation. It gives ability to
>>> feed runtime data through callbacks without recompiling Qemu in generic way.
>>> This patchset adds library that provides APIs for Qemu to talk with python
>>> scripts placed in path -module-path and how existing xscom can be extended
>>> with python interface infrastructure.
>>>
>>> We have also added an hacky emulation for memory region (OCC common area and HOMER)
>>> which is shared between core and un-core engine (ideally this should be via
>>> sram device) to showcase the effectiveness of having the scripting interface
>>> (uncore engine taken for discussion here is powerpc specificed called OCC).
>> We should try to merge this part first. It is useful as it is after some
>> cleanups.
>>
>>> Having scripting interface helps to emulate/test different uncore-core
>>> interactions including uncore engine failure or hang. It also helps in feeding
>>> randomized data at byte level access. This patchset is primarily to extend mmio
>>> callbacks with scripting interface and to demonstrate effectiveness it.
>> It is already possible to feed device models with external data using QMP or
>> external agents using a chardev backend transport. What are the benefits
>> of using the embedded python approach ?
> Yeah, I also think this needs better justification.
>
> In particular what's the case that Python makes this significantly
> easier than hacking up experimental interactions with C. I mean you
> already have to understand POWER9 internals to work with this, right,
> so I wouldn't expect Python's greater accessibility to be a big
> concern here.
right, with python interface what I could think of is,
1. we don't have to patch up every experimental interactions and recompile.
2. we can easily feed in invalid data type to see the behavior for negative/error
scenarios.
3. Similar to qtest and acceptance test we can use this to cover many scenarios.
4. Ease the CI and maintenance to have test/code separately.
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
` (7 preceding siblings ...)
2019-08-07 8:15 ` Cédric Le Goater
@ 2019-08-07 8:51 ` no-reply
2019-08-07 9:18 ` no-reply
2019-08-08 10:25 ` Stefan Hajnoczi
10 siblings, 0 replies; 45+ messages in thread
From: no-reply @ 2019-08-07 8:51 UTC (permalink / raw)
To: bala24; +Cc: maddy, qemu-devel, bala24, anju, clg, hari, pbonzini, david
Patchew URL: https://patchew.org/QEMU/20190807071445.4109-1-bala24@linux.ibm.com/
Hi,
This series failed build test on s390x host. Please find the details below.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo
echo "=== ENV ==="
env
echo
echo "=== PACKAGES ==="
rpm -qa
echo
echo "=== UNAME ==="
uname -a
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===
CC util/filemonitor-inotify.o
CC util/vfio-helpers.o
/var/tmp/patchew-tester-tmp-j_q3aucf/src/util/python_api.c: In function ‘python_callback_str’:
/var/tmp/patchew-tester-tmp-j_q3aucf/src/util/python_api.c:72:12: error: return discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
72 | return PyUnicode_AsUTF8(result);
| ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
The full log is available at
http://patchew.org/logs/20190807071445.4109-1-bala24@linux.ibm.com/testing.s390x/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
` (8 preceding siblings ...)
2019-08-07 8:51 ` no-reply
@ 2019-08-07 9:18 ` no-reply
2019-08-08 10:25 ` Stefan Hajnoczi
10 siblings, 0 replies; 45+ messages in thread
From: no-reply @ 2019-08-07 9:18 UTC (permalink / raw)
To: bala24; +Cc: maddy, qemu-devel, bala24, anju, clg, hari, pbonzini, david
Patchew URL: https://patchew.org/QEMU/20190807071445.4109-1-bala24@linux.ibm.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC accel/tcg/trace.o
CC crypto/trace.o
In file included from /tmp/qemu-test/src/util/python_api.c:1:
/tmp/qemu-test/src/include/sysemu/python_api.h:5:10: fatal error: 'Python.h' file not found
#include <Python.h>
^~~~~~~~~~
CC monitor/trace.o
The full log is available at
http://patchew.org/logs/20190807071445.4109-1-bala24@linux.ibm.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
2019-08-07 7:14 [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface Balamuruhan S
` (9 preceding siblings ...)
2019-08-07 9:18 ` no-reply
@ 2019-08-08 10:25 ` Stefan Hajnoczi
2019-08-12 6:03 ` Balamuruhan S
10 siblings, 1 reply; 45+ messages in thread
From: Stefan Hajnoczi @ 2019-08-08 10:25 UTC (permalink / raw)
To: Balamuruhan S; +Cc: maddy, anju, qemu-devel, hari, clg, pbonzini, david
[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]
On Wed, Aug 07, 2019 at 12:44:39PM +0530, Balamuruhan S wrote:
> This is a proposal to extend mmio callbacks in Qemu with scripting interface
> that is prototyped with python in this implementation. It gives ability to
> feed runtime data through callbacks without recompiling Qemu in generic way.
> This patchset adds library that provides APIs for Qemu to talk with python
> scripts placed in path -module-path and how existing xscom can be extended
> with python interface infrastructure.
>
> We have also added an hacky emulation for memory region (OCC common area and HOMER)
> which is shared between core and un-core engine (ideally this should be via
> sram device) to showcase the effectiveness of having the scripting interface
> (uncore engine taken for discussion here is powerpc specificed called OCC).
> Having scripting interface helps to emulate/test different uncore-core
> interactions including uncore engine failure or hang. It also helps in feeding
> randomized data at byte level access. This patchset is primarily to extend mmio
> callbacks with scripting interface and to demonstrate effectiveness it.
>
> Some changes are required in PowerPC skiboot tree to test these changes since
> the memory region is disabled currently for Qemu emulated PowerNV host,
> https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
Although writing Python is quick and easy, carefully wiring up the
Python C API for it is not. In practice you lose much of the benefit of
Python if you need to study the Python C API every time you wish to do
some quick scripting :(.
It must be possible to compile out the Python integration code. If the
Python integration code remains in the device model then the QEMU binary
has a dependency on libpython, which is undesirable when this feature is
not in use.
Assuming this feature can be compiled out, I think it should have a
chance to prove its usefulness and gain users. Documentation and an
active maintainer are essential.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface
2019-08-08 10:25 ` Stefan Hajnoczi
@ 2019-08-12 6:03 ` Balamuruhan S
0 siblings, 0 replies; 45+ messages in thread
From: Balamuruhan S @ 2019-08-12 6:03 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: maddy, anju, qemu-devel, hari, clg, pbonzini, david
On 8/8/19 3:55 PM, Stefan Hajnoczi wrote:
> On Wed, Aug 07, 2019 at 12:44:39PM +0530, Balamuruhan S wrote:
>> This is a proposal to extend mmio callbacks in Qemu with scripting interface
>> that is prototyped with python in this implementation. It gives ability to
>> feed runtime data through callbacks without recompiling Qemu in generic way.
>> This patchset adds library that provides APIs for Qemu to talk with python
>> scripts placed in path -module-path and how existing xscom can be extended
>> with python interface infrastructure.
>>
>> We have also added an hacky emulation for memory region (OCC common area and HOMER)
>> which is shared between core and un-core engine (ideally this should be via
>> sram device) to showcase the effectiveness of having the scripting interface
>> (uncore engine taken for discussion here is powerpc specificed called OCC).
>> Having scripting interface helps to emulate/test different uncore-core
>> interactions including uncore engine failure or hang. It also helps in feeding
>> randomized data at byte level access. This patchset is primarily to extend mmio
>> callbacks with scripting interface and to demonstrate effectiveness it.
>>
>> Some changes are required in PowerPC skiboot tree to test these changes since
>> the memory region is disabled currently for Qemu emulated PowerNV host,
>> https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
> Although writing Python is quick and easy, carefully wiring up the
> Python C API for it is not. In practice you lose much of the benefit of
> Python if you need to study the Python C API every time you wish to do
> some quick scripting :(.
Initially as you said to wire Python C API it needs study and care, but
once the framework reach to a stability we can use it for easy scripting
using it seamlessly.
>
> It must be possible to compile out the Python integration code. If the
> Python integration code remains in the device model then the QEMU binary
> has a dependency on libpython, which is undesirable when this feature is
> not in use.
if we can keep it conditional during configure, so that by default qemu
binary will not depend on libpython. If user needs to use this feature
then python 3 based libpython and python-config are necessary.
>
> Assuming this feature can be compiled out, I think it should have a
> chance to prove its usefulness and gain users. Documentation and an
> active maintainer are essential.
yes, hope that community will find its usefulness as it helped us to
test, study behavior of firmware/kernel and find bugs that is hard
to find otherwise.
Agreed that it would need proper documentation and active maintainer.
>
> Stefan
^ permalink raw reply [flat|nested] 45+ messages in thread