From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47907) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X2gNT-0004UQ-7t for qemu-devel@nongnu.org; Thu, 03 Jul 2014 08:44:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X2gNM-0008Ib-4R for qemu-devel@nongnu.org; Thu, 03 Jul 2014 08:44:39 -0400 Message-ID: <53B5502D.2090706@suse.de> Date: Thu, 03 Jul 2014 14:44:29 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1404138618-1981-1-git-send-email-aik@ozlabs.ru> <1404138618-1981-2-git-send-email-aik@ozlabs.ru> In-Reply-To: <1404138618-1981-2-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] target-ppc: Add pvr_match() callback List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org On 30.06.14 16:30, Alexey Kardashevskiy wrote: > So far it was enough to have a base PVR value and mask per CPU > family such as POWER7 or POWER8. However there CPUs which are > completely architecturally compatible but have different PVRs such > as POWER7/POWER7+ and POWER8/POWER8E. For these CPUs, top 16 bits > are CPU family and low 16 bits are the version. The families have > PVR base values different enough so defining a mask which > would cover both (or potentially more) CPUs within the family is > not possible. > > This adds a pvr_match() callback to PowerPCCPUClass. The default > handler simply compares PVR defined in the class. > > This implements ppc_pvr_match_power7/ppc_pvr_match_power8 callbacks > for POWER7/8 families. These check for POWER7/POWER7+ and POWER8/POWER8E. > > This changes ppc_cpu_compare_class_pvr_mask() not to check masks but > use the pvr_match() callback. > > Since all server CPUs use the same mask, this defines one mask > value - CPU_POWERPC_POWER_SERVER_MASK - which is used everywhere now. > This removes other mask definitions. > > This removes pvr_mask from PowerPCCPUClass as it is not used anymore. > This removes pvr initialization for POWER7/8 families as it is not used > to find the class, the pvr_match() callback is used instead. > > Signed-off-by: Alexey Kardashevskiy > --- > target-ppc/cpu-models.c | 1 - > target-ppc/cpu-models.h | 5 +---- > target-ppc/cpu-qom.h | 2 +- > target-ppc/translate_init.c | 49 ++++++++++++++++++++++++++++++++------------- > 4 files changed, 37 insertions(+), 20 deletions(-) > > diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c > index 9a91af9..c9112e9 100644 > --- a/target-ppc/cpu-models.c > +++ b/target-ppc/cpu-models.c > @@ -44,7 +44,6 @@ > PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); \ > \ > pcc->pvr = _pvr; \ > - pcc->pvr_mask = CPU_POWERPC_DEFAULT_MASK; \ We don't need the DEFAULT_MASK anymore now, no? > pcc->svr = _svr; \ > dc->desc = _desc; \ > } \ > diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h > index c39d03a..9d2ee91 100644 > --- a/target-ppc/cpu-models.h > +++ b/target-ppc/cpu-models.h > @@ -553,17 +553,14 @@ enum { > CPU_POWERPC_POWER6 = 0x003E0000, > CPU_POWERPC_POWER6_5 = 0x0F000001, /* POWER6 in POWER5 mode */ > CPU_POWERPC_POWER6A = 0x0F000002, > + CPU_POWERPC_POWER_SERVER_MASK = 0xFFFF0000, > CPU_POWERPC_POWER7_BASE = 0x003F0000, > - CPU_POWERPC_POWER7_MASK = 0xFFFF0000, > CPU_POWERPC_POWER7_v23 = 0x003F0203, > CPU_POWERPC_POWER7P_BASE = 0x004A0000, > - CPU_POWERPC_POWER7P_MASK = 0xFFFF0000, > CPU_POWERPC_POWER7P_v21 = 0x004A0201, > CPU_POWERPC_POWER8E_BASE = 0x004B0000, > - CPU_POWERPC_POWER8E_MASK = 0xFFFF0000, > CPU_POWERPC_POWER8E_v10 = 0x004B0100, > CPU_POWERPC_POWER8_BASE = 0x004D0000, > - CPU_POWERPC_POWER8_MASK = 0xFFFF0000, > CPU_POWERPC_POWER8_v10 = 0x004D0100, > CPU_POWERPC_970 = 0x00390202, > CPU_POWERPC_970FX_v10 = 0x00391100, > diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h > index f1f0a52..0fee36f 100644 > --- a/target-ppc/cpu-qom.h > +++ b/target-ppc/cpu-qom.h > @@ -56,7 +56,7 @@ typedef struct PowerPCCPUClass { > void (*parent_reset)(CPUState *cpu); > > uint32_t pvr; > - uint32_t pvr_mask; > + bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr); > uint64_t pcr_mask; > uint32_t svr; > uint64_t insns_flags; > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index 2ab2810..5d4fee7 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -8062,6 +8062,17 @@ static void init_proc_POWER7 (CPUPPCState *env) > init_proc_book3s_64(env, BOOK3S_CPU_POWER7); > } > > +static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr) > +{ > + if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7P_BASE) { > + return true; > + } > + if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7_BASE) { > + return true; > + } > + return false; > +} > + > POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > @@ -8070,8 +8081,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) > dc->fw_name = "PowerPC,POWER7"; > dc->desc = "POWER7"; > dc->props = powerpc_servercpu_properties; > - pcc->pvr = CPU_POWERPC_POWER7_BASE; > - pcc->pvr_mask = CPU_POWERPC_POWER7_MASK; > + pcc->pvr_match = ppc_pvr_match_power7; > pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06; > pcc->init_proc = init_proc_POWER7; > pcc->check_pow = check_pow_nocheck; > @@ -8131,8 +8141,7 @@ POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data) > dc->fw_name = "PowerPC,POWER7+"; > dc->desc = "POWER7+"; > dc->props = powerpc_servercpu_properties; > - pcc->pvr = CPU_POWERPC_POWER7P_BASE; > - pcc->pvr_mask = CPU_POWERPC_POWER7P_MASK; > + pcc->pvr_match = ppc_pvr_match_power7; > pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06; > pcc->init_proc = init_proc_POWER7; > pcc->check_pow = check_pow_nocheck; > @@ -8189,6 +8198,17 @@ static void init_proc_POWER8(CPUPPCState *env) > init_proc_book3s_64(env, BOOK3S_CPU_POWER8); > } > > +static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr) > +{ > + if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8E_BASE) { > + return true; > + } > + if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8_BASE) { > + return true; > + } > + return false; > +} > + > POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > @@ -8197,8 +8217,7 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data) > dc->fw_name = "PowerPC,POWER8"; > dc->desc = "POWER8E"; > dc->props = powerpc_servercpu_properties; > - pcc->pvr = CPU_POWERPC_POWER8E_BASE; > - pcc->pvr_mask = CPU_POWERPC_POWER8E_MASK; > + pcc->pvr_match = ppc_pvr_match_power8; > pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06; > pcc->init_proc = init_proc_POWER8; > pcc->check_pow = check_pow_nocheck; > @@ -8256,13 +8275,10 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data) > POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > - PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); > > ppc_POWER8E_cpu_family_class_init(oc, data); > > dc->desc = "POWER8"; > - pcc->pvr = CPU_POWERPC_POWER8_BASE; > - pcc->pvr_mask = CPU_POWERPC_POWER8_MASK; > } > #endif /* defined (TARGET_PPC64) */ > > @@ -9245,7 +9261,6 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b) > ObjectClass *oc = (ObjectClass *)a; > uint32_t pvr = *(uint32_t *)b; > PowerPCCPUClass *pcc = (PowerPCCPUClass *)a; > - gint ret; > > /* -cpu host does a PVR lookup during construction */ > if (unlikely(strcmp(object_class_get_name(oc), > @@ -9257,9 +9272,11 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b) > return -1; > } > > - ret = (((pcc->pvr & pcc->pvr_mask) == (pvr & pcc->pvr_mask)) ? 0 : -1); > + if (pcc->pvr_match && pcc->pvr_match(pcc, pvr)) { We can safely assume that pvr_match() always exists. Alex