From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54559) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9ylo-0003um-Pf for qemu-devel@nongnu.org; Thu, 15 Aug 2013 10:43:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V9ylh-0004rc-5l for qemu-devel@nongnu.org; Thu, 15 Aug 2013 10:43:24 -0400 Received: from mail-pb0-f41.google.com ([209.85.160.41]:49654) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9ylg-0004qr-R3 for qemu-devel@nongnu.org; Thu, 15 Aug 2013 10:43:17 -0400 Received: by mail-pb0-f41.google.com with SMTP id rp2so796847pbb.28 for ; Thu, 15 Aug 2013 07:43:16 -0700 (PDT) Message-ID: <520CE8FC.7000807@ozlabs.ru> Date: Fri, 16 Aug 2013 00:43:08 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <7AAF2DA2-1FEF-4ECA-B220-366D94EC70C4@suse.de> <1376552735-2953-1-git-send-email-aik@ozlabs.ru> <520C8C11.6080905@ozlabs.ru> <98FAFA62-3B3A-40E9-8973-705C5F569CEE@suse.de> In-Reply-To: <98FAFA62-3B3A-40E9-8973-705C5F569CEE@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Anthony Liguori , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Paul Mackerras , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= On 08/15/2013 06:45 PM, Alexander Graf wrote: > > 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 >>>> Signed-off-by: Alexey Kardashevskiy >>>> >>>> --- >>>> 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? :) Just at the start of a guest. Then the guest reads PVR and if it is HV-KVM - it will be the actual one OR if it is emulated (PR KVM or TCG) - who cares, the guest will get what QEMU tells it. Who is unhappy? Sorry for my deep ignorance if I miss the point. Thank you. -- Alexey