Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 11/08/2013 12:36 AM, Igor Mammedov wrote:
On Thu, 7 Nov 2013 20:11:51 +1100
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 11/06/2013 12:53 AM, Andreas Färber wrote: Am 05.11.2013 10:52, schrieb
Paolo Bonzini:
Il 05/11/2013 10:16, Alexander Graf ha scritto:
On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote:
Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
Why is the option under -machine instead of -cpu?
Because it is still the same CPU and the guest will still read the real
PVR from the hardware (which it may not support but this is why we need
compatibility mode).
How do you support migration from a newer to an older CPU then? I think
the guest should never see anything about the hardware CPU model.
POWER can't model that. It always leaks the host CPU information into the
guest. It's the guest kernel's responsibility to not expose that change
to user space.
Yes, it's broken :). I'm not even sure there is any sensible way to do
live migration between different CPU types.
Still in my opinion it should be -cpu, not -machine. Even if it's
just a virtual CPU model.
PowerPC currently does not have -cpu option parsing. If you need to
implement it, I would ask for a generic hook in CPUClass set by
TYPE_POWERPC_CPU, so that the logic does not get hardcoded in cpu_init,
and for the p=v parsing logic to be so generic as to just set property p
to value v on the CPU instance. I.e. please make the compatibility
settings a static property or dynamic property of the CPU.
Maybe the parsing code could even live in generic qom/cpu.c, overridden
by x86/sparc and reused for arm? Somewhere down my to-do list but
patches appreciated...
I spent some time today trying to digest what you said, still having problems
with understanding of what you meant and what Igor meant about global
variables
(I just do not see the point in them).
Below is the result of my excercise. At the moment I would just like to know
if I am going in the right direction or not.
what I've had in mind was a bit simpler and more implicit approach instead of
setting properties on each CPU instance explicitly. It could done using
existing global properties mechanism.
in current code -cpu type,foo1=x,foo2=y... are saved into cpu_model string
which is parsed by target specific cpu_init() effectively parsing cpu_model
each time when creating a CPU.
So to avoid fixing every target I suggest to leave cpu_model be as it is and
add translation hook that will convert type,foo1=x,foo2=y... virtually
into a set of following options:
-global type.foo1=x -global type.foo2=y ...
these options when registered are transparently applied to each new CPU
instance (see device_post_init() for details).
now why do we need translation hook,
* not every target is able to handle type,foo1=x,foo2=y in terms of
properties yet
* legacy feature string might be in non canonical format, like
+foo1,-foo2... so for compatibility reasons with CLI we need target
specific code to convert to canonical format when it becomes available.
* for targets that don't have feature string handling and implementing
new features as properties we can implement generic default hook that
will convert canonical feature string into global properties.
as result we eventually would be able drop cpu_model and use global
properties to store CPU features.
What is wrong doing it in the way the -machine switch does it now?
qemu_get_machine_opts() returns a global list which I can iterate through
via qemu_opt_foreach() and set every property to a CPU, this will check if
a property exists and assigns it = happy Aik :)
see comments below for pseudo code:
And few question while we are here:
1. the proposed common code handles both static and dynamic properties.
What is the current QEMU trend about choosing static vs. dynamic? Can do
both in POWERPC, both have benifits.
I prefer static, since it's usually less boilerplate code.
[...]
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..a17cd73 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -327,6 +327,12 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState
*cpu, vaddr addr)
#endif
/**
+ * cpu_common_set_properties:
+ * @cpu: The CPU whose properties to initialize from the command line.
+ */
+int cpu_common_set_properties(Object *obj);
cpu_translate_features_compat(classname, cpu_model_str)
Here I lost you. I am looking to a generic way of adding any number of
properties to -cpu, not just compat.
diff --git a/qom/cpu.c b/qom/cpu.c
index 818fb26..6516c63 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -24,6 +24,8 @@
#include qemu/notify.h
#include qemu/log.h
#include sysemu/sysemu.h
+#include hw/qdev-properties.h
+#include qapi/qmp/qerror.h
bool cpu_exists(int64_t id)
{
@@ -186,6 +188,28 @@ void cpu_reset(CPUState *cpu)
}
}
+static int
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Am 08.11.2013 09:22, schrieb Alexey Kardashevskiy: On 11/08/2013 12:36 AM, Igor Mammedov wrote: On Thu, 7 Nov 2013 20:11:51 +1100 Alexey Kardashevskiy a...@ozlabs.ru wrote: On 11/06/2013 12:53 AM, Andreas Färber wrote: Am 05.11.2013 10:52, schrieb Paolo Bonzini: Il 05/11/2013 10:16, Alexander Graf ha scritto: On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? I think the guest should never see anything about the hardware CPU model. POWER can't model that. It always leaks the host CPU information into the guest. It's the guest kernel's responsibility to not expose that change to user space. Yes, it's broken :). I'm not even sure there is any sensible way to do live migration between different CPU types. Still in my opinion it should be -cpu, not -machine. Even if it's just a virtual CPU model. PowerPC currently does not have -cpu option parsing. If you need to implement it, I would ask for a generic hook in CPUClass set by TYPE_POWERPC_CPU, so that the logic does not get hardcoded in cpu_init, and for the p=v parsing logic to be so generic as to just set property p to value v on the CPU instance. I.e. please make the compatibility settings a static property or dynamic property of the CPU. Maybe the parsing code could even live in generic qom/cpu.c, overridden by x86/sparc and reused for arm? Somewhere down my to-do list but patches appreciated... I spent some time today trying to digest what you said, still having problems with understanding of what you meant and what Igor meant about global variables (I just do not see the point in them). Below is the result of my excercise. At the moment I would just like to know if I am going in the right direction or not. what I've had in mind was a bit simpler and more implicit approach instead of setting properties on each CPU instance explicitly. It could done using existing global properties mechanism. in current code -cpu type,foo1=x,foo2=y... are saved into cpu_model string which is parsed by target specific cpu_init() effectively parsing cpu_model each time when creating a CPU. So to avoid fixing every target I suggest to leave cpu_model be as it is and add translation hook that will convert type,foo1=x,foo2=y... virtually into a set of following options: -global type.foo1=x -global type.foo2=y ... these options when registered are transparently applied to each new CPU instance (see device_post_init() for details). now why do we need translation hook, * not every target is able to handle type,foo1=x,foo2=y in terms of properties yet * legacy feature string might be in non canonical format, like +foo1,-foo2... so for compatibility reasons with CLI we need target specific code to convert to canonical format when it becomes available. * for targets that don't have feature string handling and implementing new features as properties we can implement generic default hook that will convert canonical feature string into global properties. as result we eventually would be able drop cpu_model and use global properties to store CPU features. What is wrong doing it in the way the -machine switch does it now? qemu_get_machine_opts() returns a global list which I can iterate through via qemu_opt_foreach() and set every property to a CPU, this will check if a property exists and assigns it = happy Aik :) You are happy w/ ppc, but you would make x86 and sparc users unhappy! ;) QemuOpts does not support the +feature,-feature notation, just [key=]value. see comments below for pseudo code: And few question while we are here: 1. the proposed common code handles both static and dynamic properties. What is the current QEMU trend about choosing static vs. dynamic? Can do both in POWERPC, both have benifits. I prefer static, since it's usually less boilerplate code. [...] diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 7739e00..a17cd73 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -327,6 +327,12 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr) #endif /** + * cpu_common_set_properties: + * @cpu: The CPU whose properties to initialize from the command line. + */ +int cpu_common_set_properties(Object *obj); cpu_translate_features_compat(classname, cpu_model_str) Here I lost you. I am looking to a generic way of adding any number of properties to -cpu, not just compat. That's just naming and doesn't rule each other out, important is the string argument that you pass in. diff --git a/qom/cpu.c b/qom/cpu.c index 818fb26..6516c63 100644 ---
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 11/09/2013 12:20 AM, Andreas Färber wrote: When I am finally through with review of Igor's patches then he can implement that for x86 and we/you can copy or adapt it for ppc. No need to do big experiments for a concretely needed ppc feature. Does it mean I better stop wasting your time and relax and you guys do this -cpu subptions thing yourselves? Thanks. -- Alexey
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Am 08.11.2013 15:57, schrieb Alexey Kardashevskiy: On 11/09/2013 12:20 AM, Andreas Färber wrote: When I am finally through with review of Igor's patches then he can implement that for x86 and we/you can copy or adapt it for ppc. No need to do big experiments for a concretely needed ppc feature. Does it mean I better stop wasting your time and relax and you guys do this -cpu subptions thing yourselves? Thanks. No, I meant don't try to mess with translating -cpu to -global and looking up the class name that Igor mentioned. (Depending on when that is being done it may introduce subtle bugs like real -global being overridden.) If you wait for us then you'll wait for some time... Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 11/06/2013 12:53 AM, Andreas Färber wrote: Am 05.11.2013 10:52, schrieb
Paolo Bonzini:
Il 05/11/2013 10:16, Alexander Graf ha scritto:
On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote:
Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
Why is the option under -machine instead of -cpu?
Because it is still the same CPU and the guest will still read the real
PVR from the hardware (which it may not support but this is why we need
compatibility mode).
How do you support migration from a newer to an older CPU then? I think
the guest should never see anything about the hardware CPU model.
POWER can't model that. It always leaks the host CPU information into the
guest. It's the guest kernel's responsibility to not expose that change to
user space.
Yes, it's broken :). I'm not even sure there is any sensible way to do live
migration between different CPU types.
Still in my opinion it should be -cpu, not -machine. Even if it's
just a virtual CPU model.
PowerPC currently does not have -cpu option parsing. If you need to
implement it, I would ask for a generic hook in CPUClass set by
TYPE_POWERPC_CPU, so that the logic does not get hardcoded in cpu_init,
and for the p=v parsing logic to be so generic as to just set property p
to value v on the CPU instance. I.e. please make the compatibility
settings a static property or dynamic property of the CPU.
Maybe the parsing code could even live in generic qom/cpu.c, overridden
by x86/sparc and reused for arm? Somewhere down my to-do list but
patches appreciated...
I spent some time today trying to digest what you said, still having problems
with understanding of what you meant and what Igor meant about global variables
(I just do not see the point in them).
Below is the result of my excercise. At the moment I would just like to know
if I am going in the right direction or not.
And few question while we are here:
1. the proposed common code handles both static and dynamic properties.
What is the current QEMU trend about choosing static vs. dynamic? Can do
both in POWERPC, both have benifits.
2. The static powerpc_properties array only works if defined with POWER7 family
but not POWER family. Both families are abstract so I did not expect any
difference but it is there. Any clue before I continue debugging? :)
Thanks!
---
This adds suboptions support for -cpu and demonstrates the use ot this
by adding a static compat1 and a dynamic compat options to POWERPC
CPUs.
---
include/qom/cpu.h | 6 ++
include/sysemu/sysemu.h | 1 +
qom/cpu.c | 24
target-ppc/translate_init.c | 40
vl.c| 35 ++-
5 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..a17cd73 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -327,6 +327,12 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState
*cpu, vaddr addr)
#endif
/**
+ * cpu_common_set_properties:
+ * @cpu: The CPU whose properties to initialize from the command line.
+ */
+int cpu_common_set_properties(Object *obj);
+
+/**
* cpu_reset:
* @cpu: The CPU whose state is to be reset.
*/
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b27a1df..9007e44 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -192,6 +192,7 @@ DeviceState *get_boot_device_ex(uint32_t position, char
**suffix);
DeviceState *get_boot_device(uint32_t position);
QemuOpts *qemu_get_machine_opts(void);
+QemuOpts *qemu_get_cpu_opts(void);
bool usb_enabled(bool default_usb);
diff --git a/qom/cpu.c b/qom/cpu.c
index 818fb26..6516c63 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -24,6 +24,8 @@
#include qemu/notify.h
#include qemu/log.h
#include sysemu/sysemu.h
+#include hw/qdev-properties.h
+#include qapi/qmp/qerror.h
bool cpu_exists(int64_t id)
{
@@ -186,6 +188,28 @@ void cpu_reset(CPUState *cpu)
}
}
+static int cpu_set_property(const char *name, const char *value, void *opaque)
+{
+DeviceState *dev = opaque;
+Error *err = NULL;
+
+if (strcmp(name, type) == 0)
+return 0;
+
+qdev_prop_parse(dev, name, value, err);
+if (err != NULL) {
+qerror_report_err(err);
+error_free(err);
+return -1;
+}
+return 0;
+}
+
+int cpu_common_set_properties(Object *obj)
+{
+return qemu_opt_foreach(qemu_get_cpu_opts(), cpu_set_property, obj, 1);
+}
+
static void cpu_common_reset(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 93f7cba..d357b1f 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -28,6 +28,8 @@
#include mmu-hash32.h
#include mmu-hash64.h
#include qemu/error-report.h
+#include hw/qdev-properties.h
+#include qapi/visitor.h
//#define PPC_DUMP_CPU
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On Thu, 7 Nov 2013 20:11:51 +1100
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 11/06/2013 12:53 AM, Andreas Färber wrote: Am 05.11.2013 10:52, schrieb
Paolo Bonzini:
Il 05/11/2013 10:16, Alexander Graf ha scritto:
On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote:
Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
Why is the option under -machine instead of -cpu?
Because it is still the same CPU and the guest will still read the real
PVR from the hardware (which it may not support but this is why we need
compatibility mode).
How do you support migration from a newer to an older CPU then? I think
the guest should never see anything about the hardware CPU model.
POWER can't model that. It always leaks the host CPU information into the
guest. It's the guest kernel's responsibility to not expose that change
to user space.
Yes, it's broken :). I'm not even sure there is any sensible way to do
live migration between different CPU types.
Still in my opinion it should be -cpu, not -machine. Even if it's
just a virtual CPU model.
PowerPC currently does not have -cpu option parsing. If you need to
implement it, I would ask for a generic hook in CPUClass set by
TYPE_POWERPC_CPU, so that the logic does not get hardcoded in cpu_init,
and for the p=v parsing logic to be so generic as to just set property p
to value v on the CPU instance. I.e. please make the compatibility
settings a static property or dynamic property of the CPU.
Maybe the parsing code could even live in generic qom/cpu.c, overridden
by x86/sparc and reused for arm? Somewhere down my to-do list but
patches appreciated...
I spent some time today trying to digest what you said, still having problems
with understanding of what you meant and what Igor meant about global
variables
(I just do not see the point in them).
Below is the result of my excercise. At the moment I would just like to know
if I am going in the right direction or not.
what I've had in mind was a bit simpler and more implicit approach instead of
setting properties on each CPU instance explicitly. It could done using
existing global properties mechanism.
in current code -cpu type,foo1=x,foo2=y... are saved into cpu_model string
which is parsed by target specific cpu_init() effectively parsing cpu_model
each time when creating a CPU.
So to avoid fixing every target I suggest to leave cpu_model be as it is and
add translation hook that will convert type,foo1=x,foo2=y... virtually
into a set of following options:
-global type.foo1=x -global type.foo2=y ...
these options when registered are transparently applied to each new CPU
instance (see device_post_init() for details).
now why do we need translation hook,
* not every target is able to handle type,foo1=x,foo2=y in terms of
properties yet
* legacy feature string might be in non canonical format, like
+foo1,-foo2... so for compatibility reasons with CLI we need target
specific code to convert to canonical format when it becomes available.
* for targets that don't have feature string handling and implementing
new features as properties we can implement generic default hook that
will convert canonical feature string into global properties.
as result we eventually would be able drop cpu_model and use global
properties to store CPU features.
see comments below for pseudo code:
And few question while we are here:
1. the proposed common code handles both static and dynamic properties.
What is the current QEMU trend about choosing static vs. dynamic? Can do
both in POWERPC, both have benifits.
I prefer static, since it's usually less boilerplate code.
[...]
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..a17cd73 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -327,6 +327,12 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState
*cpu, vaddr addr)
#endif
/**
+ * cpu_common_set_properties:
+ * @cpu: The CPU whose properties to initialize from the command line.
+ */
+int cpu_common_set_properties(Object *obj);
cpu_translate_features_compat(classname, cpu_model_str)
diff --git a/qom/cpu.c b/qom/cpu.c
index 818fb26..6516c63 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -24,6 +24,8 @@
#include qemu/notify.h
#include qemu/log.h
#include sysemu/sysemu.h
+#include hw/qdev-properties.h
+#include qapi/qmp/qerror.h
bool cpu_exists(int64_t id)
{
@@ -186,6 +188,28 @@ void cpu_reset(CPUState *cpu)
}
}
+static int cpu_set_property(const char *name, const char *value, void
*opaque)
+{
+DeviceState *dev = opaque;
+Error *err = NULL;
+
+if (strcmp(name, type) == 0)
+return 0;
+
+qdev_prop_parse(dev, name, value, err);
+if (err != NULL) {
+qerror_report_err(err);
+error_free(err);
+return -1;
+}
+return 0;
+}
+
+int cpu_common_set_properties(Object *obj)
+{
+return
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Am 07.11.2013 10:11, schrieb Alexey Kardashevskiy:
On 11/06/2013 12:53 AM, Andreas Färber wrote: Am 05.11.2013 10:52, schrieb
Paolo Bonzini:
Il 05/11/2013 10:16, Alexander Graf ha scritto:
On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote:
Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
Why is the option under -machine instead of -cpu?
Because it is still the same CPU and the guest will still read the real
PVR from the hardware (which it may not support but this is why we need
compatibility mode).
How do you support migration from a newer to an older CPU then? I think
the guest should never see anything about the hardware CPU model.
POWER can't model that. It always leaks the host CPU information into the
guest. It's the guest kernel's responsibility to not expose that change to
user space.
Yes, it's broken :). I'm not even sure there is any sensible way to do
live migration between different CPU types.
Still in my opinion it should be -cpu, not -machine. Even if it's
just a virtual CPU model.
PowerPC currently does not have -cpu option parsing. If you need to
implement it, I would ask for a generic hook in CPUClass set by
TYPE_POWERPC_CPU, so that the logic does not get hardcoded in cpu_init,
and for the p=v parsing logic to be so generic as to just set property p
to value v on the CPU instance. I.e. please make the compatibility
settings a static property or dynamic property of the CPU.
Maybe the parsing code could even live in generic qom/cpu.c, overridden
by x86/sparc and reused for arm? Somewhere down my to-do list but
patches appreciated...
I spent some time today trying to digest what you said, still having problems
with understanding of what you meant and what Igor meant about global
variables
(I just do not see the point in them).
Below is the result of my excercise. At the moment I would just like to know
if I am going in the right direction or not.
The overall direction is good ... see below.
And few question while we are here:
1. the proposed common code handles both static and dynamic properties.
What is the current QEMU trend about choosing static vs. dynamic? Can do
both in POWERPC, both have benifits.
Static properties have mostly served to set a field to a value before
the object is realized. You can set a default value there. The setters
are usually no-op (error out) for realized objects.
Dynamic properties allow you (more easily) to implement any logic for
storing/retrieving the value and can serve to inspect or set a value at
runtime.
We were told on a KVM call that discovery of properties should not be a
decision factor towards static properties - management tools need to
inspect an object instance via QMP (and handle a property getting
dropped or renamed).
2. The static powerpc_properties array only works if defined with POWER7
family
but not POWER family. Both families are abstract so I did not expect any
difference but it is there. Any clue before I continue debugging? :)
There is no hierarchy among families. So POWER7 is not a POWER, it's a
powerpc at the bottom of the file. If you want power_properties rather
than powerpc_properties then you need to assign them individually for
POWER, ..., POWER5, ..., POWER7, POWER8 - or tweak the type hierarchy.
Thanks!
---
This adds suboptions support for -cpu and demonstrates the use ot this
by adding a static compat1 and a dynamic compat options to POWERPC
CPUs.
Unfortunately that approach won't work. Both x86 and sparc, as I
mentioned, need special handling, so you can't generalize it. Either we
need #ifdef'fery to rule out the exceptions, or better, what I suggested
was something along the lines of
struct CPUClass {
...
void (*parse_options)(CPUState *cpu, const char *str);
}
with cpu_common_parse_options() as the default implementation assigned
via cc-parse_options = cpu_common_parse_options; rather than called out
of common code.
You could have a trivial (inline?) function to obtain cc and call
cc-parse_options though, for use in cpu_ppc_init().
I also think you can use the object_property_* API rather than
qdev_prop_* for parsing and setting the value, compare Igor's code in
target-i386/cpu.c.
Please do separate these global preparations from the actual new ppc
property. Elsewhere it was discussed whether to use a readable string
value, which might hint at a dynamic property of type string or maybe
towards an enum (/me no experience with those yet and whether that works
better with dynamic or static).
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On Tue, 05 Nov 2013 14:53:14 +0100 Andreas Färber afaer...@suse.de wrote: Am 05.11.2013 10:52, schrieb Paolo Bonzini: Il 05/11/2013 10:16, Alexander Graf ha scritto: On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? I think the guest should never see anything about the hardware CPU model. POWER can't model that. It always leaks the host CPU information into the guest. It's the guest kernel's responsibility to not expose that change to user space. Yes, it's broken :). I'm not even sure there is any sensible way to do live migration between different CPU types. Still in my opinion it should be -cpu, not -machine. Even if it's just a virtual CPU model. PowerPC currently does not have -cpu option parsing. If you need to implement it, I would ask for a generic hook in CPUClass set by TYPE_POWERPC_CPU, so that the logic does not get hardcoded in cpu_init, and for the p=v parsing logic to be so generic as to just set property p to value v on the CPU instance. I.e. please make the compatibility settings a static property or dynamic property of the CPU. Seconded, that's what is on my to-do list for x86 once x86 properties series is applied. moreover, all hook has to do is to translate -cpu to a set of global options, adding [CPUtype.foo, val] pairs to global option list. Then options will be automatically applied to any new instance of CPUtype and there won't be any need for accessing/storing cpu_model string during cpu creation and hotplug. Maybe the parsing code could even live in generic qom/cpu.c, overridden by x86/sparc and reused for arm? Somewhere down my to-do list but patches appreciated... Andreas
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 06.11.2013, at 06:48, Paul Mackerras pau...@samba.org wrote: On Mon, Sep 30, 2013 at 01:25:32PM +0200, Alexander Graf wrote: On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote: To be able to boot on newer hardware that the software support, PowerISA defines a logical PVR, one per every PowerISA specification version from 2.04. [snip] +case 205: +spapr-arch_compat = CPU_POWERPC_LOGICAL_2_05; +break; +case 206: +spapr-arch_compat = CPU_POWERPC_LOGICAL_2_06; Does it make sense to declare compat mode a number or would a string be easier for users? I can imagine that -machine compat=power6 is easier to understand for a user than -machine compat=205. That's probably true. I don't mind either way. Also, we need to handle failure. If the kernel can not set the CPU to 2.05 mode for example (IIRC POWER8 doesn't allow you to) we should bail out here. POWER8 does have 2.05 (POWER6) and 2.06 (POWER7) compatibility modes. +/* Architecture compatibility mode */ +uint32_t arch_compat; Do we really need to carry this in the vcpu struct? Or can we just fire-and-forget about it? If we want to preserve anything, it should be the PCR register. There are two relevant values here; the compatibility mode (if any) that the user has requested via the command line, and the mode that has been negotiated with the ibm,client-architecture-support (CAS) call. CAS should select the latest mode supported by the guest that is not later than the mode requested on the command line, and is supported by QEMU, and is not later than the architecture of the host. Both values should be sent across to the destination VM on migration AFAICS. So how does this work on pHyp? I thought the guest always comes up in full native compat mode and downgrades itself then? On reset/reboot, the compatibility mode should not change. The device tree that is supplied to the new SLOF instance should reflect the current compatibility mode. Ok, this should work if we just don't touch PCR across reset. Alex
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? I think the guest should never see anything about the hardware CPU model. Paolo It is possible to run QEMU with -cpu POWER8 with some old distro which does not know about power8, it will switch to power6-compat mode, the the user does yum update (or analog) and then the new kernel (with power8 support) will do H_CAS again and this time raw power8 should be selected.
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? I think the guest should never see anything about the hardware CPU model. POWER can't model that. It always leaks the host CPU information into the guest. It's the guest kernel's responsibility to not expose that change to user space. Yes, it's broken :). I'm not even sure there is any sensible way to do live migration between different CPU types. Alex
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 05.11.2013, at 03:19, Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 10/01/2013 12:49 AM, Alexander Graf wrote:
On 09/30/2013 03:22 PM, Alexey Kardashevskiy wrote:
On 30.09.2013 21:25, Alexander Graf wrote:
On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote:
I realized it has been a while since I got your response and did not answer
:) Sorry for the delay.
To be able to boot on newer hardware that the software support,
PowerISA defines a logical PVR, one per every PowerISA specification
version from 2.04.
This adds the compat option which takes values 205 or 206 and forces
QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or
CPU_POWERPC_LOGICAL_2_06).
The guest reads the logical PVR value from cpu-version property of
a CPU device node.
Cc: Nikunj A Dadhanianik...@linux.vnet.ibm.com
Cc: Andreas Färberafaer...@suse.de
Signed-off-by: Alexey Kardashevskiya...@ozlabs.ru
---
hw/ppc/spapr.c | 40
include/hw/ppc/spapr.h | 2 ++
target-ppc/cpu-models.h | 10 ++
target-ppc/cpu.h| 3 +++
target-ppc/kvm.c| 2 ++
vl.c| 4
6 files changed, 61 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a09a1d9..737452d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -33,6 +33,7 @@
#include sysemu/kvm.h
#include kvm_ppc.h
#include mmu-hash64.h
+#include cpu-models.h
#include hw/boards.h
#include hw/ppc/ppc.h
@@ -196,6 +197,26 @@ static XICSState *xics_system_init(int nr_servers,
int nr_irqs)
return icp;
}
+static void spapr_compat_mode_init(sPAPREnvironment *spapr)
+{
+QemuOpts *machine_opts = qemu_get_machine_opts();
+uint64_t compat = qemu_opt_get_number(machine_opts, compat, 0);
+
+switch (compat) {
+case 0:
+break;
+case 205:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_05;
+break;
+case 206:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_06;
Does it make sense to declare compat mode a number or would a string
be easier for users? I can imagine that -machine compat=power6 is
easier to understand for a user than -machine compat=205.
I just follow the PowerISA spec. It does not say anywhere (at least I do
not see it) that 2.05==power6. 2.05 was released when power6 was
released and power6 supports 2.05 but these are not synonims. And
compat=power6 would not set cpu-version to any of power6 PVRs, it
still will be a logical PVR. It confuses me too to tell qemu 205
instead of power6 but it is the spec to blame :)
So what is 2_06 plus then? :)
No idea. Why does it matter here?
To me it really sounds like a 1:1 mapping to cores rather than specs - the
ISA defines a lot more capabilities than a single core necessarily
supports, especially with the inclusion of booke into the generic ppc spec.
Sounds - may be. But still not the same. The guest kernel has different
descriptors for power6(raw) and power6(architected) with different flags
and (slightly?) different behavior.
So even the guest kernel calls it power6 then? Why shouldn't we?
+break;
+default:
+perror(Unsupported mode, only are 205, 206 supported\n);
+break;
+}
+}
+
static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
{
int ret = 0, offset;
@@ -206,6 +227,7 @@ static int spapr_fixup_cpu_dt(void *fdt,
sPAPREnvironment *spapr)
CPU_FOREACH(cpu) {
DeviceClass *dc = DEVICE_GET_CLASS(cpu);
+CPUPPCState *env =(POWERPC_CPU(cpu)-env);
uint32_t associativity[] = {cpu_to_be32(0x5),
cpu_to_be32(0x0),
cpu_to_be32(0x0),
@@ -238,6 +260,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
sPAPREnvironment *spapr)
if (ret 0) {
return ret;
}
+
+if (env-arch_compat) {
+ret = fdt_setprop(fdt, offset, cpu-version,
+env-arch_compat, sizeof(env-arch_compat));
+if (ret 0) {
+return ret;
+}
+}
}
return ret;
}
@@ -1145,6 +1175,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs
*args)
spapr = g_malloc0(sizeof(*spapr));
QLIST_INIT(spapr-phbs);
+spapr_compat_mode_init(spapr);
+
cpu_ppc_hypercall = emulate_spapr_hypercall;
/* Allocate RMA if necessary */
@@ -1226,6 +1258,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs
*args)
xics_cpu_setup(spapr-icp, cpu);
+/*
+ * If compat mode is set in the command line, pass it to CPU
so KVM
+ * will be able to set it in the host kernel.
+ */
+if (spapr-arch_compat) {
+env-arch_compat = spapr-arch_compat;
You should set the compat mode in KVM here, rather than doing it in
the put_registers call which gets invoked on every register sync. Or can
the guest change the mode?
I
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Il 05/11/2013 10:16, Alexander Graf ha scritto: On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? I think the guest should never see anything about the hardware CPU model. POWER can't model that. It always leaks the host CPU information into the guest. It's the guest kernel's responsibility to not expose that change to user space. Yes, it's broken :). I'm not even sure there is any sensible way to do live migration between different CPU types. Still in my opinion it should be -cpu, not -machine. Even if it's just a virtual CPU model. Paolo
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 05.11.2013, at 10:52, Paolo Bonzini pbonz...@redhat.com wrote: Il 05/11/2013 10:16, Alexander Graf ha scritto: On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? I think the guest should never see anything about the hardware CPU model. POWER can't model that. It always leaks the host CPU information into the guest. It's the guest kernel's responsibility to not expose that change to user space. Yes, it's broken :). I'm not even sure there is any sensible way to do live migration between different CPU types. Still in my opinion it should be -cpu, not -machine. Even if it's just a virtual CPU model. The only thing that this really changes is an SPR (MSR in x86 speech) on an existing cpu model. It's definitely not a new CPU type. If anything it'd be an option to an existing type. Alex
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Il 05/11/2013 11:27, Alexander Graf ha scritto: On 05.11.2013, at 10:52, Paolo Bonzini pbonz...@redhat.com wrote: Il 05/11/2013 10:16, Alexander Graf ha scritto: On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? I think the guest should never see anything about the hardware CPU model. POWER can't model that. It always leaks the host CPU information into the guest. It's the guest kernel's responsibility to not expose that change to user space. Yes, it's broken :). I'm not even sure there is any sensible way to do live migration between different CPU types. Still in my opinion it should be -cpu, not -machine. Even if it's just a virtual CPU model. The only thing that this really changes is an SPR (MSR in x86 speech) on an existing cpu model. It's definitely not a new CPU type. If anything it'd be an option to an existing type. Agreed. Paolo
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 11/05/2013 08:52 PM, Paolo Bonzini wrote: Il 05/11/2013 10:16, Alexander Graf ha scritto: On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? I think the guest should never see anything about the hardware CPU model. POWER can't model that. It always leaks the host CPU information into the guest. It's the guest kernel's responsibility to not expose that change to user space. Yes, it's broken :). I'm not even sure there is any sensible way to do live migration between different CPU types. Still in my opinion it should be -cpu, not -machine. Even if it's just a virtual CPU model. The compat option itself does not make much sense (yes we could just add yet another CPU class and that's it) but with the ibm,client-architecture-support we will have to implement this compatibility mode anyway. Since the guest can ask for a compatibility mode change, we either have to support compat option or hot-unplug all (all) CPU objects in QEMU and hotplug CPUs of the requested model. Or always reset the guest if it asked for a compatibility mode and recreate CPUs in QEMU during reset. As for me, the compat option seems simpler. -- Alexey
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Il 05/11/2013 11:45, Alexey Kardashevskiy ha scritto: Still in my opinion it should be -cpu, not -machine. Even if it's just a virtual CPU model. The compat option itself does not make much sense (yes we could just add yet another CPU class and that's it) but with the ibm,client-architecture-support we will have to implement this compatibility mode anyway. Since the guest can ask for a compatibility mode change, we either have to support compat option or hot-unplug all (all) CPU objects in QEMU and hotplug CPUs of the requested model. Or always reset the guest if it asked for a compatibility mode and recreate CPUs in QEMU during reset. As for me, the compat option seems simpler. Sure, just make it a suboption of -cpu rather than -machine. Paolo
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Am 05.11.2013 10:52, schrieb Paolo Bonzini: Il 05/11/2013 10:16, Alexander Graf ha scritto: On 05.11.2013, at 10:06, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? I think the guest should never see anything about the hardware CPU model. POWER can't model that. It always leaks the host CPU information into the guest. It's the guest kernel's responsibility to not expose that change to user space. Yes, it's broken :). I'm not even sure there is any sensible way to do live migration between different CPU types. Still in my opinion it should be -cpu, not -machine. Even if it's just a virtual CPU model. PowerPC currently does not have -cpu option parsing. If you need to implement it, I would ask for a generic hook in CPUClass set by TYPE_POWERPC_CPU, so that the logic does not get hardcoded in cpu_init, and for the p=v parsing logic to be so generic as to just set property p to value v on the CPU instance. I.e. please make the compatibility settings a static property or dynamic property of the CPU. Maybe the parsing code could even live in generic qom/cpu.c, overridden by x86/sparc and reused for arm? Somewhere down my to-do list but patches appreciated... Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On Tue, Nov 05, 2013 at 10:06:04AM +0100, Paolo Bonzini wrote: Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). How do you support migration from a newer to an older CPU then? PowerVM has supported this for years, so there are well-established interfaces that existing guest kernels use for this. Basically, the hypervisor can put the CPU into a mode where the processor behaves when in user mode according to an earlier version of the architecture. POWER8 conforms to architecture 2.07, and it has a register with bits that allow it to be put into an architecture 2.05 (POWER6) mode or an architecture 2.06 (POWER7) mode. In 2.05 mode, none of the instructions and registers that are new in POWER7 and POWER8 are available in user mode, and in 2.06 mode, none of the facilities that are new in POWER8 are available in user mode. The other thing the hypervisor does is tell the guest which architecture mode the CPU is running in via a property in the device tree. Linux kernels use that to determine the list of hardware features that they can use. Under PowerVM, if you are running a guest on a POWER8 (say) and you want to be able to be able to migrate to a POWER6 later on, you have to run the guest in architecture 2.05 mode, even if the guest knows about later architecture versions. I think the guest should never see anything about the hardware CPU model. The architecture doesn't let us do that, but our guest kernels are accustomed to largely ignoring the actual PVR and operating on the basis of the architecture level that we tell them via the device tree. Regards, Paul.
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On Mon, Sep 30, 2013 at 01:25:32PM +0200, Alexander Graf wrote: On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote: To be able to boot on newer hardware that the software support, PowerISA defines a logical PVR, one per every PowerISA specification version from 2.04. [snip] +case 205: +spapr-arch_compat = CPU_POWERPC_LOGICAL_2_05; +break; +case 206: +spapr-arch_compat = CPU_POWERPC_LOGICAL_2_06; Does it make sense to declare compat mode a number or would a string be easier for users? I can imagine that -machine compat=power6 is easier to understand for a user than -machine compat=205. That's probably true. I don't mind either way. Also, we need to handle failure. If the kernel can not set the CPU to 2.05 mode for example (IIRC POWER8 doesn't allow you to) we should bail out here. POWER8 does have 2.05 (POWER6) and 2.06 (POWER7) compatibility modes. +/* Architecture compatibility mode */ +uint32_t arch_compat; Do we really need to carry this in the vcpu struct? Or can we just fire-and-forget about it? If we want to preserve anything, it should be the PCR register. There are two relevant values here; the compatibility mode (if any) that the user has requested via the command line, and the mode that has been negotiated with the ibm,client-architecture-support (CAS) call. CAS should select the latest mode supported by the guest that is not later than the mode requested on the command line, and is supported by QEMU, and is not later than the architecture of the host. Both values should be sent across to the destination VM on migration AFAICS. On reset/reboot, the compatibility mode should not change. The device tree that is supplied to the new SLOF instance should reflect the current compatibility mode. Regards, Paul.
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 10/01/2013 12:49 AM, Alexander Graf wrote:
On 09/30/2013 03:22 PM, Alexey Kardashevskiy wrote:
On 30.09.2013 21:25, Alexander Graf wrote:
On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote:
I realized it has been a while since I got your response and did not answer
:) Sorry for the delay.
To be able to boot on newer hardware that the software support,
PowerISA defines a logical PVR, one per every PowerISA specification
version from 2.04.
This adds the compat option which takes values 205 or 206 and forces
QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or
CPU_POWERPC_LOGICAL_2_06).
The guest reads the logical PVR value from cpu-version property of
a CPU device node.
Cc: Nikunj A Dadhanianik...@linux.vnet.ibm.com
Cc: Andreas Färberafaer...@suse.de
Signed-off-by: Alexey Kardashevskiya...@ozlabs.ru
---
hw/ppc/spapr.c | 40
include/hw/ppc/spapr.h | 2 ++
target-ppc/cpu-models.h | 10 ++
target-ppc/cpu.h| 3 +++
target-ppc/kvm.c| 2 ++
vl.c| 4
6 files changed, 61 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a09a1d9..737452d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -33,6 +33,7 @@
#include sysemu/kvm.h
#include kvm_ppc.h
#include mmu-hash64.h
+#include cpu-models.h
#include hw/boards.h
#include hw/ppc/ppc.h
@@ -196,6 +197,26 @@ static XICSState *xics_system_init(int nr_servers,
int nr_irqs)
return icp;
}
+static void spapr_compat_mode_init(sPAPREnvironment *spapr)
+{
+QemuOpts *machine_opts = qemu_get_machine_opts();
+uint64_t compat = qemu_opt_get_number(machine_opts, compat, 0);
+
+switch (compat) {
+case 0:
+break;
+case 205:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_05;
+break;
+case 206:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_06;
Does it make sense to declare compat mode a number or would a string
be easier for users? I can imagine that -machine compat=power6 is
easier to understand for a user than -machine compat=205.
I just follow the PowerISA spec. It does not say anywhere (at least I do
not see it) that 2.05==power6. 2.05 was released when power6 was
released and power6 supports 2.05 but these are not synonims. And
compat=power6 would not set cpu-version to any of power6 PVRs, it
still will be a logical PVR. It confuses me too to tell qemu 205
instead of power6 but it is the spec to blame :)
So what is 2_06 plus then? :)
No idea. Why does it matter here?
To me it really sounds like a 1:1 mapping to cores rather than specs - the
ISA defines a lot more capabilities than a single core necessarily
supports, especially with the inclusion of booke into the generic ppc spec.
Sounds - may be. But still not the same. The guest kernel has different
descriptors for power6(raw) and power6(architected) with different flags
and (slightly?) different behavior.
+break;
+default:
+perror(Unsupported mode, only are 205, 206 supported\n);
+break;
+}
+}
+
static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
{
int ret = 0, offset;
@@ -206,6 +227,7 @@ static int spapr_fixup_cpu_dt(void *fdt,
sPAPREnvironment *spapr)
CPU_FOREACH(cpu) {
DeviceClass *dc = DEVICE_GET_CLASS(cpu);
+CPUPPCState *env =(POWERPC_CPU(cpu)-env);
uint32_t associativity[] = {cpu_to_be32(0x5),
cpu_to_be32(0x0),
cpu_to_be32(0x0),
@@ -238,6 +260,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
sPAPREnvironment *spapr)
if (ret 0) {
return ret;
}
+
+if (env-arch_compat) {
+ret = fdt_setprop(fdt, offset, cpu-version,
+env-arch_compat, sizeof(env-arch_compat));
+if (ret 0) {
+return ret;
+}
+}
}
return ret;
}
@@ -1145,6 +1175,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs
*args)
spapr = g_malloc0(sizeof(*spapr));
QLIST_INIT(spapr-phbs);
+spapr_compat_mode_init(spapr);
+
cpu_ppc_hypercall = emulate_spapr_hypercall;
/* Allocate RMA if necessary */
@@ -1226,6 +1258,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs
*args)
xics_cpu_setup(spapr-icp, cpu);
+/*
+ * If compat mode is set in the command line, pass it to CPU
so KVM
+ * will be able to set it in the host kernel.
+ */
+if (spapr-arch_compat) {
+env-arch_compat = spapr-arch_compat;
You should set the compat mode in KVM here, rather than doing it in
the put_registers call which gets invoked on every register sync. Or can
the guest change the mode?
I will change it here in the next patch (which requires kernel changes
which are not there yet). The guest cannot change it directly but it can
indirectly
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote:
To be able to boot on newer hardware that the software support,
PowerISA defines a logical PVR, one per every PowerISA specification
version from 2.04.
This adds the compat option which takes values 205 or 206 and forces
QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or
CPU_POWERPC_LOGICAL_2_06).
The guest reads the logical PVR value from cpu-version property of
a CPU device node.
Cc: Nikunj A Dadhania nik...@linux.vnet.ibm.com
Cc: Andreas Färber afaer...@suse.de
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/ppc/spapr.c | 40
include/hw/ppc/spapr.h | 2 ++
target-ppc/cpu-models.h | 10 ++
target-ppc/cpu.h| 3 +++
target-ppc/kvm.c| 2 ++
vl.c| 4
6 files changed, 61 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a09a1d9..737452d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -33,6 +33,7 @@
#include sysemu/kvm.h
#include kvm_ppc.h
#include mmu-hash64.h
+#include cpu-models.h
#include hw/boards.h
#include hw/ppc/ppc.h
@@ -196,6 +197,26 @@ static XICSState *xics_system_init(int nr_servers, int
nr_irqs)
return icp;
}
+static void spapr_compat_mode_init(sPAPREnvironment *spapr)
+{
+QemuOpts *machine_opts = qemu_get_machine_opts();
+uint64_t compat = qemu_opt_get_number(machine_opts, compat, 0);
+
+switch (compat) {
+case 0:
+break;
+case 205:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_05;
+break;
+case 206:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_06;
Does it make sense to declare compat mode a number or would a string be easier
for users? I can imagine that -machine compat=power6 is easier to understand
for a user than -machine compat=205.
+break;
+default:
+perror(Unsupported mode, only are 205, 206 supported\n);
+break;
+}
+}
+
static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
{
int ret = 0, offset;
@@ -206,6 +227,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment
*spapr)
CPU_FOREACH(cpu) {
DeviceClass *dc = DEVICE_GET_CLASS(cpu);
+CPUPPCState *env = (POWERPC_CPU(cpu)-env);
uint32_t associativity[] = {cpu_to_be32(0x5),
cpu_to_be32(0x0),
cpu_to_be32(0x0),
@@ -238,6 +260,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
sPAPREnvironment *spapr)
if (ret 0) {
return ret;
}
+
+if (env-arch_compat) {
+ret = fdt_setprop(fdt, offset, cpu-version,
+ env-arch_compat, sizeof(env-arch_compat));
+if (ret 0) {
+return ret;
+}
+}
}
return ret;
}
@@ -1145,6 +1175,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
spapr = g_malloc0(sizeof(*spapr));
QLIST_INIT(spapr-phbs);
+spapr_compat_mode_init(spapr);
+
cpu_ppc_hypercall = emulate_spapr_hypercall;
/* Allocate RMA if necessary */
@@ -1226,6 +1258,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
xics_cpu_setup(spapr-icp, cpu);
+/*
+ * If compat mode is set in the command line, pass it to CPU so KVM
+ * will be able to set it in the host kernel.
+ */
+if (spapr-arch_compat) {
+env-arch_compat = spapr-arch_compat;
You should set the compat mode in KVM here, rather than doing it in the
put_registers call which gets invoked on every register sync. Or can the guest
change the mode?
Also, we need to handle failure. If the kernel can not set the CPU to 2.05 mode
for example (IIRC POWER8 doesn't allow you to) we should bail out here.
And then there's the TCG question. We either have to disable CPU features
similar to how we handle it in KVM (by setting and honoring the respective bits
in PCR) or we need to bail out too and declare compat mode unsupported for TCG.
And then there's the fact that the kernel interface isn't upstream in a way that
+}
+
qemu_register_reset(spapr_cpu_reset, cpu);
}
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index ca175b0..201c578 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -34,6 +34,8 @@ typedef struct sPAPREnvironment {
uint32_t epow_irq;
Notifier epow_notifier;
+uint32_t arch_compat;/* Compatible PVR from the command line */
+
/* Migration state */
int htab_save_index;
bool htab_first_pass;
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index 49ba4a4..d7c033c 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -583,6 +583,16 @@ enum {
CPU_POWERPC_RS64II = 0x0034,
CPU_POWERPC_RS64III
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Il 30/09/2013 13:25, Alexander Graf ha scritto: On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote: To be able to boot on newer hardware that the software support, PowerISA defines a logical PVR, one per every PowerISA specification version from 2.04. This adds the compat option which takes values 205 or 206 and forces QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or CPU_POWERPC_LOGICAL_2_06). The guest reads the logical PVR value from cpu-version property of a CPU device node. Why is the option under -machine instead of -cpu? Paolo
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 30.09.2013 21:52, Paolo Bonzini wrote: Il 30/09/2013 13:25, Alexander Graf ha scritto: On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote: To be able to boot on newer hardware that the software support, PowerISA defines a logical PVR, one per every PowerISA specification version from 2.04. This adds the compat option which takes values 205 or 206 and forces QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or CPU_POWERPC_LOGICAL_2_06). The guest reads the logical PVR value from cpu-version property of a CPU device node. Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). It is possible to run QEMU with -cpu POWER8 with some old distro which does not know about power8, it will switch to power6-compat mode, the the user does yum update (or analog) and then the new kernel (with power8 support) will do H_CAS again and this time raw power8 should be selected. -- With best regards Alexey Kardashevskiy -- icq: 52150396
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 30.09.2013 21:25, Alexander Graf wrote:
On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote:
To be able to boot on newer hardware that the software support,
PowerISA defines a logical PVR, one per every PowerISA specification
version from 2.04.
This adds the compat option which takes values 205 or 206 and forces
QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or
CPU_POWERPC_LOGICAL_2_06).
The guest reads the logical PVR value from cpu-version property of
a CPU device node.
Cc: Nikunj A Dadhania nik...@linux.vnet.ibm.com
Cc: Andreas Färber afaer...@suse.de
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/ppc/spapr.c | 40
include/hw/ppc/spapr.h | 2 ++
target-ppc/cpu-models.h | 10 ++
target-ppc/cpu.h| 3 +++
target-ppc/kvm.c| 2 ++
vl.c| 4
6 files changed, 61 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a09a1d9..737452d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -33,6 +33,7 @@
#include sysemu/kvm.h
#include kvm_ppc.h
#include mmu-hash64.h
+#include cpu-models.h
#include hw/boards.h
#include hw/ppc/ppc.h
@@ -196,6 +197,26 @@ static XICSState *xics_system_init(int nr_servers, int
nr_irqs)
return icp;
}
+static void spapr_compat_mode_init(sPAPREnvironment *spapr)
+{
+QemuOpts *machine_opts = qemu_get_machine_opts();
+uint64_t compat = qemu_opt_get_number(machine_opts, compat, 0);
+
+switch (compat) {
+case 0:
+break;
+case 205:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_05;
+break;
+case 206:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_06;
Does it make sense to declare compat mode a number or would a string
be easier for users? I can imagine that -machine compat=power6 is
easier to understand for a user than -machine compat=205.
I just follow the PowerISA spec. It does not say anywhere (at least I do
not see it) that 2.05==power6. 2.05 was released when power6 was
released and power6 supports 2.05 but these are not synonims. And
compat=power6 would not set cpu-version to any of power6 PVRs, it
still will be a logical PVR. It confuses me too to tell qemu 205
instead of power6 but it is the spec to blame :)
+break;
+default:
+perror(Unsupported mode, only are 205, 206 supported\n);
+break;
+}
+}
+
static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
{
int ret = 0, offset;
@@ -206,6 +227,7 @@ static int spapr_fixup_cpu_dt(void *fdt,
sPAPREnvironment *spapr)
CPU_FOREACH(cpu) {
DeviceClass *dc = DEVICE_GET_CLASS(cpu);
+CPUPPCState *env = (POWERPC_CPU(cpu)-env);
uint32_t associativity[] = {cpu_to_be32(0x5),
cpu_to_be32(0x0),
cpu_to_be32(0x0),
@@ -238,6 +260,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
sPAPREnvironment *spapr)
if (ret 0) {
return ret;
}
+
+if (env-arch_compat) {
+ret = fdt_setprop(fdt, offset, cpu-version,
+ env-arch_compat, sizeof(env-arch_compat));
+if (ret 0) {
+return ret;
+}
+}
}
return ret;
}
@@ -1145,6 +1175,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
spapr = g_malloc0(sizeof(*spapr));
QLIST_INIT(spapr-phbs);
+spapr_compat_mode_init(spapr);
+
cpu_ppc_hypercall = emulate_spapr_hypercall;
/* Allocate RMA if necessary */
@@ -1226,6 +1258,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
xics_cpu_setup(spapr-icp, cpu);
+/*
+ * If compat mode is set in the command line, pass it to CPU so KVM
+ * will be able to set it in the host kernel.
+ */
+if (spapr-arch_compat) {
+env-arch_compat = spapr-arch_compat;
You should set the compat mode in KVM here, rather than doing it in
the put_registers call which gets invoked on every register sync. Or can
the guest change the mode?
I will change it here in the next patch (which requires kernel changes
which are not there yet). The guest cannot change it directly but it can
indirectly via client-architecture-support.
Also, we need to handle failure. If the kernel can not set the CPU
to
2.05 mode for example (IIRC POWER8 doesn't allow you to) we should bail
out here.
Yep, I'll add this easy check :)
And then there's the TCG question. We either have to disable CPU
features similar to how we handle it in KVM (by setting and honoring the
respective bits in PCR) or we need to bail out too and declare compat
mode unsupported for TCG.
At the moment we want to run old distro on new CPUs. This patch would
make more sense with the ibm,client-architecture-support and power8
registers migration patches which I did not post yet.
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 09/30/2013 03:22 PM, Alexey Kardashevskiy wrote:
On 30.09.2013 21:25, Alexander Graf wrote:
On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote:
To be able to boot on newer hardware that the software support,
PowerISA defines a logical PVR, one per every PowerISA specification
version from 2.04.
This adds the compat option which takes values 205 or 206 and forces
QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or
CPU_POWERPC_LOGICAL_2_06).
The guest reads the logical PVR value from cpu-version property of
a CPU device node.
Cc: Nikunj A Dadhanianik...@linux.vnet.ibm.com
Cc: Andreas Färberafaer...@suse.de
Signed-off-by: Alexey Kardashevskiya...@ozlabs.ru
---
hw/ppc/spapr.c | 40
include/hw/ppc/spapr.h | 2 ++
target-ppc/cpu-models.h | 10 ++
target-ppc/cpu.h| 3 +++
target-ppc/kvm.c| 2 ++
vl.c| 4
6 files changed, 61 insertions(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a09a1d9..737452d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -33,6 +33,7 @@
#include sysemu/kvm.h
#include kvm_ppc.h
#include mmu-hash64.h
+#include cpu-models.h
#include hw/boards.h
#include hw/ppc/ppc.h
@@ -196,6 +197,26 @@ static XICSState *xics_system_init(int nr_servers, int
nr_irqs)
return icp;
}
+static void spapr_compat_mode_init(sPAPREnvironment *spapr)
+{
+QemuOpts *machine_opts = qemu_get_machine_opts();
+uint64_t compat = qemu_opt_get_number(machine_opts, compat, 0);
+
+switch (compat) {
+case 0:
+break;
+case 205:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_05;
+break;
+case 206:
+spapr-arch_compat = CPU_POWERPC_LOGICAL_2_06;
Does it make sense to declare compat mode a number or would a string
be easier for users? I can imagine that -machine compat=power6 is
easier to understand for a user than -machine compat=205.
I just follow the PowerISA spec. It does not say anywhere (at least I do
not see it) that 2.05==power6. 2.05 was released when power6 was
released and power6 supports 2.05 but these are not synonims. And
compat=power6 would not set cpu-version to any of power6 PVRs, it
still will be a logical PVR. It confuses me too to tell qemu 205
instead of power6 but it is the spec to blame :)
So what is 2_06 plus then? :)
To me it really sounds like a 1:1 mapping to cores rather than specs -
the ISA defines a lot more capabilities than a single core necessarily
supports, especially with the inclusion of booke into the generic ppc spec.
+break;
+default:
+perror(Unsupported mode, only are 205, 206 supported\n);
+break;
+}
+}
+
static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
{
int ret = 0, offset;
@@ -206,6 +227,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment
*spapr)
CPU_FOREACH(cpu) {
DeviceClass *dc = DEVICE_GET_CLASS(cpu);
+CPUPPCState *env =(POWERPC_CPU(cpu)-env);
uint32_t associativity[] = {cpu_to_be32(0x5),
cpu_to_be32(0x0),
cpu_to_be32(0x0),
@@ -238,6 +260,14 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment
*spapr)
if (ret 0) {
return ret;
}
+
+if (env-arch_compat) {
+ret = fdt_setprop(fdt, offset, cpu-version,
+env-arch_compat, sizeof(env-arch_compat));
+if (ret 0) {
+return ret;
+}
+}
}
return ret;
}
@@ -1145,6 +1175,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
spapr = g_malloc0(sizeof(*spapr));
QLIST_INIT(spapr-phbs);
+spapr_compat_mode_init(spapr);
+
cpu_ppc_hypercall = emulate_spapr_hypercall;
/* Allocate RMA if necessary */
@@ -1226,6 +1258,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
xics_cpu_setup(spapr-icp, cpu);
+/*
+ * If compat mode is set in the command line, pass it to CPU so KVM
+ * will be able to set it in the host kernel.
+ */
+if (spapr-arch_compat) {
+env-arch_compat = spapr-arch_compat;
You should set the compat mode in KVM here, rather than doing it in
the put_registers call which gets invoked on every register sync. Or can
the guest change the mode?
I will change it here in the next patch (which requires kernel changes
which are not there yet). The guest cannot change it directly but it can
indirectly via client-architecture-support.
They probably want a generic callback then. What happens on reset?
Also, we need to handle failure. If the kernel can not set the CPU
to
2.05 mode for example (IIRC POWER8 doesn't allow you to) we should bail
out here.
Yep, I'll add this easy check :)
And then there's the TCG question. We either have to disable CPU
features similar to how we handle it in KVM (by setting and honoring the
respective bits in PCR) or we need
