Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Wed, 6 May 2015 08:41:50 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
On Wed, May 06, 2015 at 11:59:38AM +0200, Michael Mueller wrote:
On Tue, 5 May 2015 10:26:02 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
The patch defines ids per accelerator and adds the accel_id and
the model_name to the CPUState. The accel_id is initialized by
common code, the model name needs to be initialized by target
specific code.
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
Acked-by: Christian Borntraeger borntrae...@de.ibm.com
---
include/qom/cpu.h | 5 +
qapi-schema.json | 9 +
qom/cpu.c | 14 ++
3 files changed, 28 insertions(+)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 9dafb48..4ffc050 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -236,6 +236,8 @@ struct kvm_run;
* @mem_io_pc: Host Program Counter at which the memory was accessed.
* @mem_io_vaddr: Target virtual address at which the memory was
accessed.
* @kvm_fd: vCPU file descriptor for KVM.
+ * @accel_id: accelerator id of this CPU.
+ * @model_name: model name of this CPU
*
* State of one CPU core or thread.
*/
@@ -313,6 +315,9 @@ struct CPUState {
(absolute value) offset as small as possible. This reduces code
size, especially for hosts without large memory offsets. */
volatile sig_atomic_t tcg_exit_req;
+
+AccelId accel_id;
This can be a AccelState pointer, set on initialization, because we have
another user case for having a AccelState pointer: query-cpu-definition
implementations may create temporary CPU objects with a different accel
object to be able to probe for accel-specific data.
(The pointer may become a link QOM property later.)
+char *model_name;
};
QTAILQ_HEAD(CPUTailQ, CPUState);
diff --git a/qapi-schema.json b/qapi-schema.json
index ac9594d..540e520 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2515,6 +2515,15 @@
##
{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+# @AccelId
+#
+# Defines accelerator ids
+#
+# Since: 2.4
+##
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
+
Not sure if it is better to have an enum or simply a string here.
##
# @CpuDefinitionInfo:
#
diff --git a/qom/cpu.c b/qom/cpu.c
index 108bfa2..457afc7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename,
const char *cpu_model)
goto out;
}
+if (tcg_enabled()) {
+cpu-accel_id = ACCEL_ID_TCG;
+} else if (kvm_enabled()) {
+cpu-accel_id = ACCEL_ID_KVM;
+}
+#ifdef CONFIG_XEN
+else if (xen_enabled()) {
+cpu-accel_id = ACCEL_ID_XEN;
+}
+#endif
+else {
+cpu-accel_id = ACCEL_ID_QTEST;
+}
You can simply use ACCEL_GET_CLASS(current_machine-accelerator)-name
here. If we really want an enum, we can add an AccelId field to
AccelClass, and initialize it properly on the accel class_init
functions.
The AccelClass (ac = ACCEL_GET_CLASS(current_machine-accelerator) would be
ok though.
That will allow to access the ac-accel_id (not yet there) at places where
required.
I'm just not sure how to access current_machine here.
It is a global variable declared in hw/boards.h. But it makes sense only
if !CONFIG_USER.
But I am starting to wonder if we shouldn't simply expose accel info as
a /mmachine property, instead of per-CPU information. It may become
per-CPU in the future, but right now we really don't support having
multiple accelerators so why are we trying to pretend we do?
That brings me back into the direction I was before with the proposed
query-cpu-model
QMP call... but that's OK. In the end that holds true also for the cpu model
name as
well...
CONFIG_USER may require some special code when returning the accelerator
ID as tcg because IIRC it doesn't use the QOM accel classes (yet).
+
object_property_set_bool(OBJECT(cpu), true, realized, err);
out:
--
1.8.3.1
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Thu, May 07, 2015 at 09:55:45AM +0200, Michael Mueller wrote:
On Wed, 6 May 2015 08:41:50 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
[...]
diff --git a/qom/cpu.c b/qom/cpu.c
index 108bfa2..457afc7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename,
const char *cpu_model)
goto out;
}
+if (tcg_enabled()) {
+cpu-accel_id = ACCEL_ID_TCG;
+} else if (kvm_enabled()) {
+cpu-accel_id = ACCEL_ID_KVM;
+}
+#ifdef CONFIG_XEN
+else if (xen_enabled()) {
+cpu-accel_id = ACCEL_ID_XEN;
+}
+#endif
+else {
+cpu-accel_id = ACCEL_ID_QTEST;
+}
You can simply use ACCEL_GET_CLASS(current_machine-accelerator)-name
here. If we really want an enum, we can add an AccelId field to
AccelClass, and initialize it properly on the accel class_init
functions.
The AccelClass (ac = ACCEL_GET_CLASS(current_machine-accelerator) would
be ok though.
That will allow to access the ac-accel_id (not yet there) at places
where required.
I'm just not sure how to access current_machine here.
It is a global variable declared in hw/boards.h. But it makes sense only
if !CONFIG_USER.
But I am starting to wonder if we shouldn't simply expose accel info as
a /mmachine property, instead of per-CPU information. It may become
per-CPU in the future, but right now we really don't support having
multiple accelerators so why are we trying to pretend we do?
That brings me back into the direction I was before with the proposed
query-cpu-model
QMP call... but that's OK. In the end that holds true also for the cpu model
name as
well...
That's probably true with the current command-line options and current
machine implementations, but CPU model information is already inside the
CPU objects. Accel information, on the other hand, is still in global
variables such as kvm_allowed and current_machine.
I still believe per-CPU accel links are the right way to go in the
future, but it's not how the accelerator code works today.
That means I am OK with both approaches: exposing CPU::accel info, or
MachineState::accel info. The difference is that /machine/accel is
simpler to implement today.
--
Eduardo
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Tue, 5 May 2015 10:26:02 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
The patch defines ids per accelerator and adds the accel_id and
the model_name to the CPUState. The accel_id is initialized by
common code, the model name needs to be initialized by target
specific code.
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
Acked-by: Christian Borntraeger borntrae...@de.ibm.com
---
include/qom/cpu.h | 5 +
qapi-schema.json | 9 +
qom/cpu.c | 14 ++
3 files changed, 28 insertions(+)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 9dafb48..4ffc050 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -236,6 +236,8 @@ struct kvm_run;
* @mem_io_pc: Host Program Counter at which the memory was accessed.
* @mem_io_vaddr: Target virtual address at which the memory was accessed.
* @kvm_fd: vCPU file descriptor for KVM.
+ * @accel_id: accelerator id of this CPU.
+ * @model_name: model name of this CPU
*
* State of one CPU core or thread.
*/
@@ -313,6 +315,9 @@ struct CPUState {
(absolute value) offset as small as possible. This reduces code
size, especially for hosts without large memory offsets. */
volatile sig_atomic_t tcg_exit_req;
+
+AccelId accel_id;
This can be a AccelState pointer, set on initialization, because we have
another user case for having a AccelState pointer: query-cpu-definition
implementations may create temporary CPU objects with a different accel
object to be able to probe for accel-specific data.
(The pointer may become a link QOM property later.)
+char *model_name;
};
QTAILQ_HEAD(CPUTailQ, CPUState);
diff --git a/qapi-schema.json b/qapi-schema.json
index ac9594d..540e520 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2515,6 +2515,15 @@
##
{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+# @AccelId
+#
+# Defines accelerator ids
+#
+# Since: 2.4
+##
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
+
Not sure if it is better to have an enum or simply a string here.
##
# @CpuDefinitionInfo:
#
diff --git a/qom/cpu.c b/qom/cpu.c
index 108bfa2..457afc7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const
char *cpu_model)
goto out;
}
+if (tcg_enabled()) {
+cpu-accel_id = ACCEL_ID_TCG;
+} else if (kvm_enabled()) {
+cpu-accel_id = ACCEL_ID_KVM;
+}
+#ifdef CONFIG_XEN
+else if (xen_enabled()) {
+cpu-accel_id = ACCEL_ID_XEN;
+}
+#endif
+else {
+cpu-accel_id = ACCEL_ID_QTEST;
+}
You can simply use ACCEL_GET_CLASS(current_machine-accelerator)-name
here. If we really want an enum, we can add an AccelId field to
AccelClass, and initialize it properly on the accel class_init
functions.
The AccelClass (ac = ACCEL_GET_CLASS(current_machine-accelerator) would be ok
though.
That will allow to access the ac-accel_id (not yet there) at places where
required.
I'm just not sure how to access current_machine here.
CONFIG_USER may require some special code when returning the accelerator
ID as tcg because IIRC it doesn't use the QOM accel classes (yet).
+
object_property_set_bool(OBJECT(cpu), true, realized, err);
out:
--
1.8.3.1
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Tue, 5 May 2015 11:46:04 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
On Tue, May 05, 2015 at 08:36:45AM -0600, Eric Blake wrote:
On 05/05/2015 07:26 AM, Eduardo Habkost wrote:
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
+
Not sure if it is better to have an enum or simply a string here.
enum is almost always better when there is a finite set of possible
strings - it's better documented, and when introspection is in place,
will make it easier to determine when the set has grown.
True, and there are other cases where we could use an enum internally in
QEMU (e.g. arrays for accelerator-specific data inside CPU classes).
Actually the accelerator name currently represented as string (ac-name =
KVM;)
can be initialized from the respective lookup value (AccelId_lookup[]) instead.
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Wed, May 06, 2015 at 11:59:38AM +0200, Michael Mueller wrote:
On Tue, 5 May 2015 10:26:02 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
The patch defines ids per accelerator and adds the accel_id and
the model_name to the CPUState. The accel_id is initialized by
common code, the model name needs to be initialized by target
specific code.
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
Acked-by: Christian Borntraeger borntrae...@de.ibm.com
---
include/qom/cpu.h | 5 +
qapi-schema.json | 9 +
qom/cpu.c | 14 ++
3 files changed, 28 insertions(+)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 9dafb48..4ffc050 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -236,6 +236,8 @@ struct kvm_run;
* @mem_io_pc: Host Program Counter at which the memory was accessed.
* @mem_io_vaddr: Target virtual address at which the memory was
accessed.
* @kvm_fd: vCPU file descriptor for KVM.
+ * @accel_id: accelerator id of this CPU.
+ * @model_name: model name of this CPU
*
* State of one CPU core or thread.
*/
@@ -313,6 +315,9 @@ struct CPUState {
(absolute value) offset as small as possible. This reduces code
size, especially for hosts without large memory offsets. */
volatile sig_atomic_t tcg_exit_req;
+
+AccelId accel_id;
This can be a AccelState pointer, set on initialization, because we have
another user case for having a AccelState pointer: query-cpu-definition
implementations may create temporary CPU objects with a different accel
object to be able to probe for accel-specific data.
(The pointer may become a link QOM property later.)
+char *model_name;
};
QTAILQ_HEAD(CPUTailQ, CPUState);
diff --git a/qapi-schema.json b/qapi-schema.json
index ac9594d..540e520 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2515,6 +2515,15 @@
##
{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+# @AccelId
+#
+# Defines accelerator ids
+#
+# Since: 2.4
+##
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
+
Not sure if it is better to have an enum or simply a string here.
##
# @CpuDefinitionInfo:
#
diff --git a/qom/cpu.c b/qom/cpu.c
index 108bfa2..457afc7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const
char *cpu_model)
goto out;
}
+if (tcg_enabled()) {
+cpu-accel_id = ACCEL_ID_TCG;
+} else if (kvm_enabled()) {
+cpu-accel_id = ACCEL_ID_KVM;
+}
+#ifdef CONFIG_XEN
+else if (xen_enabled()) {
+cpu-accel_id = ACCEL_ID_XEN;
+}
+#endif
+else {
+cpu-accel_id = ACCEL_ID_QTEST;
+}
You can simply use ACCEL_GET_CLASS(current_machine-accelerator)-name
here. If we really want an enum, we can add an AccelId field to
AccelClass, and initialize it properly on the accel class_init
functions.
The AccelClass (ac = ACCEL_GET_CLASS(current_machine-accelerator) would be
ok though.
That will allow to access the ac-accel_id (not yet there) at places where
required.
I'm just not sure how to access current_machine here.
It is a global variable declared in hw/boards.h. But it makes sense only
if !CONFIG_USER.
But I am starting to wonder if we shouldn't simply expose accel info as
a /mmachine property, instead of per-CPU information. It may become
per-CPU in the future, but right now we really don't support having
multiple accelerators so why are we trying to pretend we do?
CONFIG_USER may require some special code when returning the accelerator
ID as tcg because IIRC it doesn't use the QOM accel classes (yet).
+
object_property_set_bool(OBJECT(cpu), true, realized, err);
out:
--
1.8.3.1
--
Eduardo
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
The patch defines ids per accelerator and adds the accel_id and
the model_name to the CPUState. The accel_id is initialized by
common code, the model name needs to be initialized by target
specific code.
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
Acked-by: Christian Borntraeger borntrae...@de.ibm.com
---
include/qom/cpu.h | 5 +
qapi-schema.json | 9 +
qom/cpu.c | 14 ++
3 files changed, 28 insertions(+)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 9dafb48..4ffc050 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -236,6 +236,8 @@ struct kvm_run;
* @mem_io_pc: Host Program Counter at which the memory was accessed.
* @mem_io_vaddr: Target virtual address at which the memory was accessed.
* @kvm_fd: vCPU file descriptor for KVM.
+ * @accel_id: accelerator id of this CPU.
+ * @model_name: model name of this CPU
*
* State of one CPU core or thread.
*/
@@ -313,6 +315,9 @@ struct CPUState {
(absolute value) offset as small as possible. This reduces code
size, especially for hosts without large memory offsets. */
volatile sig_atomic_t tcg_exit_req;
+
+AccelId accel_id;
This can be a AccelState pointer, set on initialization, because we have
another user case for having a AccelState pointer: query-cpu-definition
implementations may create temporary CPU objects with a different accel
object to be able to probe for accel-specific data.
(The pointer may become a link QOM property later.)
+char *model_name;
};
QTAILQ_HEAD(CPUTailQ, CPUState);
diff --git a/qapi-schema.json b/qapi-schema.json
index ac9594d..540e520 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2515,6 +2515,15 @@
##
{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+# @AccelId
+#
+# Defines accelerator ids
+#
+# Since: 2.4
+##
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
+
Not sure if it is better to have an enum or simply a string here.
##
# @CpuDefinitionInfo:
#
diff --git a/qom/cpu.c b/qom/cpu.c
index 108bfa2..457afc7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const
char *cpu_model)
goto out;
}
+if (tcg_enabled()) {
+cpu-accel_id = ACCEL_ID_TCG;
+} else if (kvm_enabled()) {
+cpu-accel_id = ACCEL_ID_KVM;
+}
+#ifdef CONFIG_XEN
+else if (xen_enabled()) {
+cpu-accel_id = ACCEL_ID_XEN;
+}
+#endif
+else {
+cpu-accel_id = ACCEL_ID_QTEST;
+}
You can simply use ACCEL_GET_CLASS(current_machine-accelerator)-name
here. If we really want an enum, we can add an AccelId field to
AccelClass, and initialize it properly on the accel class_init
functions.
CONFIG_USER may require some special code when returning the accelerator
ID as tcg because IIRC it doesn't use the QOM accel classes (yet).
+
object_property_set_bool(OBJECT(cpu), true, realized, err);
out:
--
1.8.3.1
--
Eduardo
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Tue, May 05, 2015 at 08:36:45AM -0600, Eric Blake wrote:
On 05/05/2015 07:26 AM, Eduardo Habkost wrote:
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
+
Not sure if it is better to have an enum or simply a string here.
enum is almost always better when there is a finite set of possible
strings - it's better documented, and when introspection is in place,
will make it easier to determine when the set has grown.
True, and there are other cases where we could use an enum internally in
QEMU (e.g. arrays for accelerator-specific data inside CPU classes).
--
Eduardo
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On 05/05/2015 07:26 AM, Eduardo Habkost wrote:
On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
The patch defines ids per accelerator and adds the accel_id and
the model_name to the CPUState. The accel_id is initialized by
common code, the model name needs to be initialized by target
specific code.
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
Acked-by: Christian Borntraeger borntrae...@de.ibm.com
---
include/qom/cpu.h | 5 +
qapi-schema.json | 9 +
qom/cpu.c | 14 ++
3 files changed, 28 insertions(+)
+# @AccelId
+#
+# Defines accelerator ids
+#
+# Since: 2.4
Might be nice to document the possible values, but not a show-stopper if
you don't.
+##
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
+
Not sure if it is better to have an enum or simply a string here.
enum is almost always better when there is a finite set of possible
strings - it's better documented, and when introspection is in place,
will make it easier to determine when the set has grown.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
The patch defines ids per accelerator and adds the accel_id and
the model_name to the CPUState. The accel_id is initialized by
common code, the model name needs to be initialized by target
specific code.
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
Acked-by: Christian Borntraeger borntrae...@de.ibm.com
---
include/qom/cpu.h | 5 +
qapi-schema.json | 9 +
qom/cpu.c | 14 ++
3 files changed, 28 insertions(+)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 9dafb48..4ffc050 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -236,6 +236,8 @@ struct kvm_run;
* @mem_io_pc: Host Program Counter at which the memory was accessed.
* @mem_io_vaddr: Target virtual address at which the memory was accessed.
* @kvm_fd: vCPU file descriptor for KVM.
+ * @accel_id: accelerator id of this CPU.
+ * @model_name: model name of this CPU
*
* State of one CPU core or thread.
*/
@@ -313,6 +315,9 @@ struct CPUState {
(absolute value) offset as small as possible. This reduces code
size, especially for hosts without large memory offsets. */
volatile sig_atomic_t tcg_exit_req;
+
+AccelId accel_id;
+char *model_name;
};
QTAILQ_HEAD(CPUTailQ, CPUState);
diff --git a/qapi-schema.json b/qapi-schema.json
index ac9594d..540e520 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2515,6 +2515,15 @@
##
{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+# @AccelId
+#
+# Defines accelerator ids
+#
+# Since: 2.4
+##
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
+
##
# @CpuDefinitionInfo:
#
diff --git a/qom/cpu.c b/qom/cpu.c
index 108bfa2..457afc7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char
*cpu_model)
goto out;
}
+if (tcg_enabled()) {
+cpu-accel_id = ACCEL_ID_TCG;
+} else if (kvm_enabled()) {
+cpu-accel_id = ACCEL_ID_KVM;
+}
+#ifdef CONFIG_XEN
+else if (xen_enabled()) {
+cpu-accel_id = ACCEL_ID_XEN;
+}
+#endif
+else {
+cpu-accel_id = ACCEL_ID_QTEST;
+}
+
object_property_set_bool(OBJECT(cpu), true, realized, err);
out:
--
1.8.3.1
