On 08/15/2013 05:55 PM, Alexander Graf wrote:
>
> On 15.08.2013, at 09:45, Alexey Kardashevskiy wrote:
>
>> IBM POWERPC processors encode PVR as a CPU family in higher 16 bits and
>> a CPU version in lower 16 bits. Since there is no significant change
>> in behavior between versions, there is no point to add every single CPU
>> version in QEMU's CPU list. Also, new CPU versions of already supported
>> CPU won't break the existing code.
>>
>> This adds a PVR mask support which means that aliases are replaced with
>> another layer in POWERPC CPU class hierarchy. The patch adds intermediate
>> POWER7, POWER7+ and POWER8 CPU classes and makes use of those in
>> specific versioned POWERPC CPUs.
>>
>> Cc: Andreas Färber <afaer...@suse.de>
>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>>
>> ---
>> Changes:
>> v3:
>> * renamed macros to describe the functionality better
>> * added default PVR value for the powerpc cpu family (what alias used to do)
>>
>> v2:
>> * aliases are replaced with another level in class hierarchy
>> ---
>> target-ppc/cpu-models.c | 54
>> ++++++++++++++++++++++++++++++++-------------
>> target-ppc/cpu-models.h | 7 ++++++
>> target-ppc/cpu-qom.h | 2 ++
>> target-ppc/translate_init.c | 4 ++--
>> 4 files changed, 50 insertions(+), 17 deletions(-)
>>
>> diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
>> index 8dea560..e48004b 100644
>> --- a/target-ppc/cpu-models.c
>> +++ b/target-ppc/cpu-models.c
>> @@ -35,7 +35,8 @@
>> /* PowerPC CPU definitions */
>> #define POWERPC_DEF_PREFIX(pvr, svr, type) \
>> glue(glue(glue(glue(pvr, _), svr), _), type)
>> -#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type)
>> \
>> +#define POWERPC_DEF_SVR_MASK(_name, _desc, _pvr, _pvr_mask, _pvr_default,
>> \
>> + _svr, _type, _parent)
>> \
>> static void \
>> glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_class_init) \
>> (ObjectClass *oc, void *data) \
>> @@ -44,6 +45,8 @@
>> PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); \
>> \
>> pcc->pvr = _pvr; \
>> + pcc->pvr_default = _pvr_default;
>> \
>> + pcc->pvr_mask = _pvr_mask;
>> \
>> pcc->svr = _svr; \
>> dc->desc = _desc; \
>> } \
>> @@ -51,7 +54,7 @@
>> static const TypeInfo \
>> glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_type_info) = { \
>> .name = _name "-" TYPE_POWERPC_CPU, \
>> - .parent = stringify(_type) "-family-" TYPE_POWERPC_CPU,
>> \
>> + .parent = _parent,
>> \
>> .class_init = \
>> glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_class_init), \
>> }; \
>> @@ -66,9 +69,24 @@
>> type_init( \
>> glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_register_types))
>>
>> +#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type)
>> \
>> + POWERPC_DEF_SVR_MASK(_name, _desc, _pvr, CPU_POWERPC_DEFAULT_MASK, 0,
>> \
>> + _svr, _type,
>> \
>> + stringify(_type) "-family-" TYPE_POWERPC_CPU)
>> +
>> #define POWERPC_DEF(_name, _pvr, _type, _desc) \
>> POWERPC_DEF_SVR(_name, _desc, _pvr, POWERPC_SVR_NONE, _type)
>>
>> +#define POWERPC_DEF_FAMILY(_name, _pvr, _pvr_mask, _pvr_default,
>> \
>> + _type, _desc)
>> \
>> + POWERPC_DEF_SVR_MASK(_name, _desc, _pvr, _pvr_mask, _pvr_default,
>> \
>> + POWERPC_SVR_NONE, _type,
>> \
>> + stringify(_type) "-family-" TYPE_POWERPC_CPU)
>> +
>> +#define POWERPC_DEF_FAMILY_MEMBER(_name, _pvr, _type, _desc, _parent)
>> \
>> + POWERPC_DEF_SVR_MASK(_name, _desc, _pvr, CPU_POWERPC_DEFAULT_MASK, 0,
>> \
>> + POWERPC_SVR_NONE, _type, _parent)
>> +
>> /* Embedded PowerPC
>> */
>> /* PowerPC 401 family
>> */
>> POWERPC_DEF("401", CPU_POWERPC_401, 401,
>> @@ -1133,16 +1151,25 @@
>> POWERPC_DEF("POWER6A", CPU_POWERPC_POWER6A, POWER6,
>> "POWER6A")
>> #endif
>> - POWERPC_DEF("POWER7_v2.0", CPU_POWERPC_POWER7_v20, POWER7,
>> - "POWER7 v2.0")
>> - POWERPC_DEF("POWER7_v2.1", CPU_POWERPC_POWER7_v21, POWER7,
>> - "POWER7 v2.1")
>> - POWERPC_DEF("POWER7_v2.3", CPU_POWERPC_POWER7_v23, POWER7,
>> - "POWER7 v2.3")
>> - POWERPC_DEF("POWER7+_v2.1", CPU_POWERPC_POWER7P_v21, POWER7,
>> - "POWER7+ v2.1")
>> - POWERPC_DEF("POWER8_v1.0", CPU_POWERPC_POWER8_v10, POWER8,
>> - "POWER8 v1.0")
>> + POWERPC_DEF_FAMILY("POWER7", CPU_POWERPC_POWER7,
>> CPU_POWERPC_POWER7_MASK,
>> + CPU_POWERPC_POWER7_v23,
>> + POWER7, "POWER7")
>> + POWERPC_DEF_FAMILY_MEMBER("POWER7_v2.0", CPU_POWERPC_POWER7_v20, POWER7,
>> + "POWER7 v2.0", "POWER7-" TYPE_POWERPC_CPU)
>> + POWERPC_DEF_FAMILY_MEMBER("POWER7_v2.1", CPU_POWERPC_POWER7_v21, POWER7,
>> + "POWER7 v2.1", "POWER7-" TYPE_POWERPC_CPU)
>> + POWERPC_DEF_FAMILY_MEMBER("POWER7_v2.3", CPU_POWERPC_POWER7_v23, POWER7,
>> + "POWER7 v2.3", "POWER7-" TYPE_POWERPC_CPU)
>> + POWERPC_DEF_FAMILY("POWER7+", CPU_POWERPC_POWER7P,
>> CPU_POWERPC_POWER7P_MASK,
>> + CPU_POWERPC_POWER7P_v21,
>> + POWER7, "POWER7")
>> + POWERPC_DEF_FAMILY_MEMBER("POWER7+_v2.1", CPU_POWERPC_POWER7P_v21,
>> POWER7,
>> + "POWER7+ v2.1", "POWER7+-" TYPE_POWERPC_CPU)
>> + POWERPC_DEF_FAMILY("POWER8", CPU_POWERPC_POWER8,
>> CPU_POWERPC_POWER8_MASK,
>> + CPU_POWERPC_POWER8_v10,
>> + POWER8, "POWER8")
>> + POWERPC_DEF_FAMILY_MEMBER("POWER8_v1.0", CPU_POWERPC_POWER8_v10, POWER8,
>> + "POWER8 v1.0", "POWER8-" TYPE_POWERPC_CPU)
>> POWERPC_DEF("970", CPU_POWERPC_970, 970,
>> "PowerPC 970")
>> POWERPC_DEF("970fx_v1.0", CPU_POWERPC_970FX_v10, 970FX,
>> @@ -1389,9 +1416,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>> { "POWER3+", "631" },
>> { "POWER5gr", "POWER5" },
>> { "POWER5gs", "POWER5+" },
>> - { "POWER7", "POWER7_v2.3" },
>> - { "POWER7+", "POWER7+_v2.1" },
>> - { "POWER8", "POWER8_v1.0" },
>> { "970fx", "970fx_v3.1" },
>> { "970mp", "970mp_v1.1" },
>> { "Apache", "RS64" },
>> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
>> index d9145d1..2233053 100644
>> --- a/target-ppc/cpu-models.h
>> +++ b/target-ppc/cpu-models.h
>> @@ -39,6 +39,7 @@ extern PowerPCCPUAlias ppc_cpu_aliases[];
>> /*****************************************************************************/
>> /* PVR definitions for most known PowerPC
>> */
>> enum {
>> + CPU_POWERPC_DEFAULT_MASK = 0xFFFFFFFF,
>> /* PowerPC 401 family */
>> /* Generic PowerPC 401 */
>> #define CPU_POWERPC_401 CPU_POWERPC_401G2
>> @@ -557,6 +558,12 @@ enum {
>> CPU_POWERPC_POWER7_v23 = 0x003F0203,
>> CPU_POWERPC_POWER7P_v21 = 0x004A0201,
>> CPU_POWERPC_POWER8_v10 = 0x004B0100,
>> + CPU_POWERPC_POWER7 = 0x003F0000,
>> + CPU_POWERPC_POWER7_MASK = 0xFFFF0000,
>> + CPU_POWERPC_POWER7P = 0x004A0000,
>> + CPU_POWERPC_POWER7P_MASK = 0xFFFF0000,
>> + CPU_POWERPC_POWER8 = 0x004B0000,
>> + CPU_POWERPC_POWER8_MASK = 0xFFFF0000,
>> CPU_POWERPC_970 = 0x00390202,
>> CPU_POWERPC_970FX_v10 = 0x00391100,
>> CPU_POWERPC_970FX_v20 = 0x003C0200,
>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>> index f3c710a..a1a712c 100644
>> --- a/target-ppc/cpu-qom.h
>> +++ b/target-ppc/cpu-qom.h
>> @@ -54,6 +54,8 @@ typedef struct PowerPCCPUClass {
>> void (*parent_reset)(CPUState *cpu);
>>
>> uint32_t pvr;
>> + uint32_t pvr_default;
>> + uint32_t pvr_mask;
>> uint32_t svr;
>> uint64_t insns_flags;
>> uint64_t insns_flags2;
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 13b290c..e73792d 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -7309,7 +7309,7 @@ static void init_ppc_proc(PowerPCCPU *cpu)
>> #endif
>> SPR_NOACCESS,
>> &spr_read_generic, SPR_NOACCESS,
>> - pcc->pvr);
>> + pcc->pvr_default ? pcc->pvr_default : pcc->pvr);
>
> This means that -cpu host on a POWER7_v20 system will still return
> POWER7_v23 and thus expose a different CPU inside the guest than
> expected with PR KVM, no?
./qemu-system-ppc64 \
-cpu \
POWER7_v2.0 \
-S \
-m "1024" \
-machine "pseries,usb=off" \
-nographic \
-vga "none" \
-enable-kvm \
-kernel "host.vmlinux" \
-initrd "1.cpio"
QEMU 1.5.91 monitor - type 'help' for more information
(qemu) info registers
[...]
FPSCR 0000000000000000
SRR0 0000000000000000 SRR1 0000000000000000 PVR 00000000003f0200
[...]
(qemu)
PVR is 003f0200 which is correct.
>
> Is there any case where major=0 minor=0 is valid? If not, we could add a
> check in the class constructor and then default to sane values when we
> hit this case. We'll have to add logic there later anyways if we want to
> allow the user to manually specify major and minor numbers.
As I was told and then noticed in the kernel (and sorry for my ignorance if
I am wrong), the lower 16 bits are major/minor numbers only for IBM POWERPC
CPUs and other CPUs may use different number of bits for this purpose. So I
would not parse those numbers and carry them as major/minor version in QEMU
and rather use some fixed sane value for a whole family for the cases when
the user does not really care about the exact chip version.
>
>
> Alex
>
>> /* Register SVR if it's defined to anything else than POWERPC_SVR_NONE */
>> if (pcc->svr != POWERPC_SVR_NONE) {
>> if (pcc->svr & POWERPC_SVR_E500) {
>> @@ -8161,7 +8161,7 @@ static gint ppc_cpu_compare_class_pvr(gconstpointer a,
>> gconstpointer b)
>> }
>> #endif
>>
>> - return pcc->pvr == pvr ? 0 : -1;
>> + return pcc->pvr == (pvr & pcc->pvr_mask) ? 0 : -1;
>> }
>>
>> PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr)
>> --
>> 1.8.3.2
>>
>
--
Alexey
