[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support
|
From: |
Alexander Graf |
|
Subject: |
Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support |
|
Date: |
Thu, 15 Aug 2013 10:45:24 +0200 |
On 15.08.2013, at 10:06, Alexey Kardashevskiy wrote:
> 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 <address@hidden>
>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>>
>>> ---
>>> 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 \
This is not -cpu host, is it? :)
> -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.
Yes, I think it makes sense to keep the full PVR around when we want to be
specific. What I'm referring to is class specific logic that can assemble
major/minor numbers from the command line. So
-cpu POWER7,major=2,minor=0
would result in a PVR value that is identical to POWER7_v2.0. The assembly of
this PVR value is class specific, because different classes of CPUs have
different semantics for their major and minor numbers.
That way in the future we won't have to add any new version specific CPU types
but instead the user can assemble those himself, making everyone's life a lot
easier.
My point was that if we have that logic, we could at the same place just say
"if my major/minor is 0, default to something reasonable".
But let's ask Andreas whether he has a better idea here :).
Alex
- Re: [Qemu-devel] [RFC PATCH] powerpc: add PVR mask support, (continued)
Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH] powerpc: add PVR mask support, Alexander Graf, 2013/08/15
- [Qemu-devel] [_R_F_C_ PATCH v2] powerpc: add PVR mask support, Alexey Kardashevskiy, 2013/08/15
- Re: [Qemu-devel] [_R_F_C_ PATCH v2] powerpc: add PVR mask support, Alexander Graf, 2013/08/15
- [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Alexey Kardashevskiy, 2013/08/15
- Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Alexander Graf, 2013/08/15
- Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Alexey Kardashevskiy, 2013/08/15
- Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support,
Alexander Graf <=
- Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Andreas Färber, 2013/08/15
- Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Alexander Graf, 2013/08/15
- Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Andreas Färber, 2013/08/15
- Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Alexander Graf, 2013/08/15
- Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Andreas Färber, 2013/08/19
Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Alexey Kardashevskiy, 2013/08/15
Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Andreas Färber, 2013/08/15
Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Alexander Graf, 2013/08/15
Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Andreas Färber, 2013/08/15
Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support, Alexander Graf, 2013/08/15