Re: [Qemu-devel] [PATCH v3 01/16] Introduce probe mode for machine type none
On Wed, 4 Mar 2015 16:19:25 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
On Tue, Mar 03, 2015 at 11:55:24AM +0100, Michael Mueller wrote:
On Mon, 02 Mar 2015 17:57:01 +0100
Andreas Färber afaer...@suse.de wrote:
Am 02.03.2015 um 17:43 schrieb Michael Mueller:
On Mon, 02 Mar 2015 14:57:21 +0100
Andreas Färber afaer...@suse.de wrote:
int configure_accelerator(MachineState *ms)
{
-const char *p;
+const char *p, *name;
char buf[10];
int ret;
bool accel_initialised = false;
bool init_failed = false;
AccelClass *acc = NULL;
+ObjectClass *oc;
+bool probe_mode = false;
p = qemu_opt_get(qemu_get_machine_opts(), accel);
if (p == NULL) {
-/* Use the default accelerator, tcg */
-p = tcg;
+oc = (ObjectClass *) MACHINE_GET_CLASS(current_machine);
+name = object_class_get_name(oc);
+probe_mode = !strcmp(name, none TYPE_MACHINE_SUFFIX);
+if (probe_mode) {
+/* Use these accelerators in probe mode, tcg should be
last */
+p = probe_mode_accels;
+} else {
+/* Use the default accelerator, tcg */
+p = tcg;
+}
}
Can't we instead use an explicit ,accel=probe or ,accel=auto?
That would then obsolete the next patch.
How would you express the following with the accel=pseudo-accel
approach?
-probe -machine s390-ccw,accel=kvm
Using machine none as default with tcg as last accelerator
initialized should not break
anything.
-M none
Let me ask differently: What does -machine none or -M none have to do
with probing? It reads as if you are introducing two probe modes. Why do
The machine none? nothing directly, I guess. What are real world use cases
for that
machine type?
you need both? If we have -probe, isn't that independent of which
It is just two different means to switch on the same mode.
machine we specify? Who is going to call either, with which respective
goal?
-probe itself would be sufficient but I currently do not want to enforce
the use of
a new parameter. Best would be not to have that mode at all if possible.
The intended use case is driven by management interfaces that need to draw
decisions
on, in this particular case runnable cpu models, with information
originated by qemu.
Let me walk through Eduardo's suggestion first and crosscheck it with my
requirements
before we enter in a maybe afterwards obsolete discussion.
I have been working on some changes to implement x86 CPU probing code
that creates accel objects on the fly, that may be useful. See:
https://github.com/ehabkost/qemu-hacks/tree/work/user-accel-init
Especially the commit:
kvm: Move /dev/kvm opening/closing to open/close methods
So the idea is to use kvm_open/close() in the query-cpu-definitions callback on
the fly without
to disturb the KVM-side data structures for the machine probe instead of going
through kvm_init()
during accelerator configuration?
The next steps I plan are:
* Create AccelState object on TCG too, and somehow pass it as argument
to cpu_x86_init()
* Change all kvm_enabled() occurrences on target-i386/cpu.c to use
the provided accel object (including
x86_cpu_get_supported_feature_word() and x86_cpu_filter_features())
* Use the new
x86_cpu_get_supported_feature_word()/x86_cpu_filter_features() code
to implement a is_runnable(X86CPUClass*, AccelState*) check
* Use the new is_runnable() check to extend query-cpu-definitions for x86 too
* Add -cpu string and machine-type arguments to the is_runnable() check
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 01/16] Introduce probe mode for machine type none
On Thu, Mar 05, 2015 at 03:56:03PM +0100, Michael Mueller wrote:
On Wed, 4 Mar 2015 16:19:25 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
On Tue, Mar 03, 2015 at 11:55:24AM +0100, Michael Mueller wrote:
On Mon, 02 Mar 2015 17:57:01 +0100
Andreas Färber afaer...@suse.de wrote:
Am 02.03.2015 um 17:43 schrieb Michael Mueller:
On Mon, 02 Mar 2015 14:57:21 +0100
Andreas Färber afaer...@suse.de wrote:
int configure_accelerator(MachineState *ms)
{
-const char *p;
+const char *p, *name;
char buf[10];
int ret;
bool accel_initialised = false;
bool init_failed = false;
AccelClass *acc = NULL;
+ObjectClass *oc;
+bool probe_mode = false;
p = qemu_opt_get(qemu_get_machine_opts(), accel);
if (p == NULL) {
-/* Use the default accelerator, tcg */
-p = tcg;
+oc = (ObjectClass *) MACHINE_GET_CLASS(current_machine);
+name = object_class_get_name(oc);
+probe_mode = !strcmp(name, none TYPE_MACHINE_SUFFIX);
+if (probe_mode) {
+/* Use these accelerators in probe mode, tcg should be
last */
+p = probe_mode_accels;
+} else {
+/* Use the default accelerator, tcg */
+p = tcg;
+}
}
Can't we instead use an explicit ,accel=probe or ,accel=auto?
That would then obsolete the next patch.
How would you express the following with the accel=pseudo-accel
approach?
-probe -machine s390-ccw,accel=kvm
Using machine none as default with tcg as last accelerator
initialized should not break
anything.
-M none
Let me ask differently: What does -machine none or -M none have to do
with probing? It reads as if you are introducing two probe modes. Why do
The machine none? nothing directly, I guess. What are real world use
cases for that
machine type?
you need both? If we have -probe, isn't that independent of which
It is just two different means to switch on the same mode.
machine we specify? Who is going to call either, with which respective
goal?
-probe itself would be sufficient but I currently do not want to enforce
the use of
a new parameter. Best would be not to have that mode at all if possible.
The intended use case is driven by management interfaces that need to
draw decisions
on, in this particular case runnable cpu models, with information
originated by qemu.
Let me walk through Eduardo's suggestion first and crosscheck it with my
requirements
before we enter in a maybe afterwards obsolete discussion.
I have been working on some changes to implement x86 CPU probing code
that creates accel objects on the fly, that may be useful. See:
https://github.com/ehabkost/qemu-hacks/tree/work/user-accel-init
Especially the commit:
kvm: Move /dev/kvm opening/closing to open/close methods
So the idea is to use kvm_open/close() in the query-cpu-definitions callback
on the fly without
to disturb the KVM-side data structures for the machine probe instead of
going through kvm_init()
during accelerator configuration?
If by KVM-side data structures you mean globals like kvm_state, yes. The
idea is to not disturb any global state during probe (including
kvm_state). In the branch above, the open/close methods will affect only
the local AccelState object. Code that will affect MachineState or other
global state will be in the machine_init method.
The next steps I plan are: * Create AccelState object on TCG too,
and somehow pass it as argument to cpu_x86_init() * Change all
kvm_enabled() occurrences on target-i386/cpu.c to use the provided
accel object (including x86_cpu_get_supported_feature_word() and
x86_cpu_filter_features())
* Use the new
x86_cpu_get_supported_feature_word()/x86_cpu_filter_features()
code to implement a is_runnable(X86CPUClass*, AccelState*) check
* Use the new is_runnable() check to extend query-cpu-definitions
for x86 too * Add -cpu string and machine-type arguments to the
is_runnable() check
--
Eduardo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 01/16] Introduce probe mode for machine type none
On Tue, Mar 03, 2015 at 11:55:24AM +0100, Michael Mueller wrote:
On Mon, 02 Mar 2015 17:57:01 +0100
Andreas Färber afaer...@suse.de wrote:
Am 02.03.2015 um 17:43 schrieb Michael Mueller:
On Mon, 02 Mar 2015 14:57:21 +0100
Andreas Färber afaer...@suse.de wrote:
int configure_accelerator(MachineState *ms)
{
-const char *p;
+const char *p, *name;
char buf[10];
int ret;
bool accel_initialised = false;
bool init_failed = false;
AccelClass *acc = NULL;
+ObjectClass *oc;
+bool probe_mode = false;
p = qemu_opt_get(qemu_get_machine_opts(), accel);
if (p == NULL) {
-/* Use the default accelerator, tcg */
-p = tcg;
+oc = (ObjectClass *) MACHINE_GET_CLASS(current_machine);
+name = object_class_get_name(oc);
+probe_mode = !strcmp(name, none TYPE_MACHINE_SUFFIX);
+if (probe_mode) {
+/* Use these accelerators in probe mode, tcg should be
last */
+p = probe_mode_accels;
+} else {
+/* Use the default accelerator, tcg */
+p = tcg;
+}
}
Can't we instead use an explicit ,accel=probe or ,accel=auto?
That would then obsolete the next patch.
How would you express the following with the accel=pseudo-accel
approach?
-probe -machine s390-ccw,accel=kvm
Using machine none as default with tcg as last accelerator initialized
should not break
anything.
-M none
Let me ask differently: What does -machine none or -M none have to do
with probing? It reads as if you are introducing two probe modes. Why do
The machine none? nothing directly, I guess. What are real world use cases
for that
machine type?
you need both? If we have -probe, isn't that independent of which
It is just two different means to switch on the same mode.
machine we specify? Who is going to call either, with which respective goal?
-probe itself would be sufficient but I currently do not want to enforce the
use of
a new parameter. Best would be not to have that mode at all if possible.
The intended use case is driven by management interfaces that need to draw
decisions
on, in this particular case runnable cpu models, with information originated
by qemu.
Let me walk through Eduardo's suggestion first and crosscheck it with my
requirements
before we enter in a maybe afterwards obsolete discussion.
I have been working on some changes to implement x86 CPU probing code
that creates accel objects on the fly, that may be useful. See:
https://github.com/ehabkost/qemu-hacks/tree/work/user-accel-init
Especially the commit:
kvm: Move /dev/kvm opening/closing to open/close methods
The next steps I plan are:
* Create AccelState object on TCG too, and somehow pass it as argument
to cpu_x86_init()
* Change all kvm_enabled() occurrences on target-i386/cpu.c to use
the provided accel object (including
x86_cpu_get_supported_feature_word() and x86_cpu_filter_features())
* Use the new
x86_cpu_get_supported_feature_word()/x86_cpu_filter_features() code
to implement a is_runnable(X86CPUClass*, AccelState*) check
* Use the new is_runnable() check to extend query-cpu-definitions for x86 too
* Add -cpu string and machine-type arguments to the is_runnable() check
--
Eduardo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 01/16] Introduce probe mode for machine type none
On Mon, 2 Mar 2015 16:17:33 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
+if (probe_mode) {
+/* Use these accelerators in probe mode, tcg should be last */
+p = probe_mode_accels;
I don't fully understand the purpose of this patch yet (I will discuss
it in a reply to the cover letter). But if you really want -machine none
to trigger different behavior, why you didn't add a probe_mode field
to MachineClass, so you can set it in the mahine_none class code?
I initially had this machine attribute but, when I remember correctly, but
wasn't able to
communicate it down into the target code. But anyhow, the mode is eventually
obsolete in light
of the temporarily constructed accelerators.. I will further comment on this as
reply to your
comment on the cover letter to keep it in place.
Michael
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 01/16] Introduce probe mode for machine type none
On Mon, 02 Mar 2015 17:57:01 +0100
Andreas Färber afaer...@suse.de wrote:
Am 02.03.2015 um 17:43 schrieb Michael Mueller:
On Mon, 02 Mar 2015 14:57:21 +0100
Andreas Färber afaer...@suse.de wrote:
int configure_accelerator(MachineState *ms)
{
-const char *p;
+const char *p, *name;
char buf[10];
int ret;
bool accel_initialised = false;
bool init_failed = false;
AccelClass *acc = NULL;
+ObjectClass *oc;
+bool probe_mode = false;
p = qemu_opt_get(qemu_get_machine_opts(), accel);
if (p == NULL) {
-/* Use the default accelerator, tcg */
-p = tcg;
+oc = (ObjectClass *) MACHINE_GET_CLASS(current_machine);
+name = object_class_get_name(oc);
+probe_mode = !strcmp(name, none TYPE_MACHINE_SUFFIX);
+if (probe_mode) {
+/* Use these accelerators in probe mode, tcg should be last
*/
+p = probe_mode_accels;
+} else {
+/* Use the default accelerator, tcg */
+p = tcg;
+}
}
Can't we instead use an explicit ,accel=probe or ,accel=auto?
That would then obsolete the next patch.
How would you express the following with the accel=pseudo-accel approach?
-probe -machine s390-ccw,accel=kvm
Using machine none as default with tcg as last accelerator initialized
should not break
anything.
-M none
Let me ask differently: What does -machine none or -M none have to do
with probing? It reads as if you are introducing two probe modes. Why do
The machine none? nothing directly, I guess. What are real world use cases for
that
machine type?
you need both? If we have -probe, isn't that independent of which
It is just two different means to switch on the same mode.
machine we specify? Who is going to call either, with which respective goal?
-probe itself would be sufficient but I currently do not want to enforce the
use of
a new parameter. Best would be not to have that mode at all if possible.
The intended use case is driven by management interfaces that need to draw
decisions
on, in this particular case runnable cpu models, with information originated by
qemu.
Let me walk through Eduardo's suggestion first and crosscheck it with my
requirements
before we enter in a maybe afterwards obsolete discussion.
Thanks,
Michael
I think that changing the semantics of an absent ,accel=foo parameter to
mean something else than its longtime default of tcg is a bad idea.
Have you testing qtest with it? Doesn't -qtest imply accel=qtest or is
that always passed explicitly?
Regards,
Andreas
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 01/16] Introduce probe mode for machine type none
On Mon, 02 Mar 2015 14:57:21 +0100
Andreas Färber afaer...@suse.de wrote:
int configure_accelerator(MachineState *ms)
{
-const char *p;
+const char *p, *name;
char buf[10];
int ret;
bool accel_initialised = false;
bool init_failed = false;
AccelClass *acc = NULL;
+ObjectClass *oc;
+bool probe_mode = false;
p = qemu_opt_get(qemu_get_machine_opts(), accel);
if (p == NULL) {
-/* Use the default accelerator, tcg */
-p = tcg;
+oc = (ObjectClass *) MACHINE_GET_CLASS(current_machine);
+name = object_class_get_name(oc);
+probe_mode = !strcmp(name, none TYPE_MACHINE_SUFFIX);
+if (probe_mode) {
+/* Use these accelerators in probe mode, tcg should be last */
+p = probe_mode_accels;
+} else {
+/* Use the default accelerator, tcg */
+p = tcg;
+}
}
Can't we instead use an explicit ,accel=probe or ,accel=auto?
That would then obsolete the next patch.
How would you express the following with the accel=pseudo-accel approach?
-probe -machine s390-ccw,accel=kvm
Using machine none as default with tcg as last accelerator initialized should
not break
anything.
-M none
The return code of configure_accelerator() is ignored anyway.
Thanks,
Michael
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 01/16] Introduce probe mode for machine type none
Am 02.03.2015 um 17:43 schrieb Michael Mueller:
On Mon, 02 Mar 2015 14:57:21 +0100
Andreas Färber afaer...@suse.de wrote:
int configure_accelerator(MachineState *ms)
{
-const char *p;
+const char *p, *name;
char buf[10];
int ret;
bool accel_initialised = false;
bool init_failed = false;
AccelClass *acc = NULL;
+ObjectClass *oc;
+bool probe_mode = false;
p = qemu_opt_get(qemu_get_machine_opts(), accel);
if (p == NULL) {
-/* Use the default accelerator, tcg */
-p = tcg;
+oc = (ObjectClass *) MACHINE_GET_CLASS(current_machine);
+name = object_class_get_name(oc);
+probe_mode = !strcmp(name, none TYPE_MACHINE_SUFFIX);
+if (probe_mode) {
+/* Use these accelerators in probe mode, tcg should be last */
+p = probe_mode_accels;
+} else {
+/* Use the default accelerator, tcg */
+p = tcg;
+}
}
Can't we instead use an explicit ,accel=probe or ,accel=auto?
That would then obsolete the next patch.
How would you express the following with the accel=pseudo-accel approach?
-probe -machine s390-ccw,accel=kvm
Using machine none as default with tcg as last accelerator initialized
should not break
anything.
-M none
Let me ask differently: What does -machine none or -M none have to do
with probing? It reads as if you are introducing two probe modes. Why do
you need both? If we have -probe, isn't that independent of which
machine we specify? Who is going to call either, with which respective goal?
I think that changing the semantics of an absent ,accel=foo parameter to
mean something else than its longtime default of tcg is a bad idea.
Have you testing qtest with it? Doesn't -qtest imply accel=qtest or is
that always passed explicitly?
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
