From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60094) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn1Rs-0003E1-05 for qemu-devel@nongnu.org; Wed, 21 May 2014 04:00:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wn1Rk-00073h-4O for qemu-devel@nongnu.org; Wed, 21 May 2014 04:00:27 -0400 Message-ID: <537C5D12.3060301@suse.de> Date: Wed, 21 May 2014 10:00:18 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1400153291-20759-1-git-send-email-aik@ozlabs.ru> <1400153291-20759-5-git-send-email-aik@ozlabs.ru> <53761B17.1040808@suse.de> <53762C0E.9080103@ozlabs.ru> <53767951.1040707@suse.de> <537C4E4E.8090803@ozlabs.ru> <537C55E9.5030905@suse.de> <537C5CC7.4050901@ozlabs.ru> In-Reply-To: <537C5CC7.4050901@ozlabs.ru> Content-Type: text/plain; charset=KOI8-R; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , "qemu-devel@nongnu.org" , "qemu-ppc@nongnu.org" On 21.05.14 09:59, Alexey Kardashevskiy wrote: > On 05/21/2014 05:29 PM, Alexander Graf wrote: >> On 21.05.14 08:57, Alexey Kardashevskiy wrote: >>> On 05/17/2014 06:47 AM, Alexander Graf wrote: >>>> On 16.05.14 17:17, Alexey Kardashevskiy wrote: >>>>> On 05/17/2014 12:05 AM, Alexander Graf wrote: >>>>>> On 15.05.14 13:28, Alexey Kardashevskiy wrote: >>>>>>> This adds basic support for the "compat" CPU option. By specifying >>>>>>> the compat property, the user can manually switch guest CPU mode from >>>>>>> "raw" to "architected". >>>>>>> >>>>>>> Since the actual compatibility mode is not implemented yet, this does >>>>>>> not change the existing behavior. >>>>>>> >>>>>>> Signed-off-by: Alexey Kardashevskiy >>>>>>> --- >>>>>>> hw/ppc/spapr.c | 15 ++++++++++++++- >>>>>>> target-ppc/cpu-models.h | 6 ++++++ >>>>>>> target-ppc/cpu-qom.h | 2 ++ >>>>>>> target-ppc/cpu.h | 3 +++ >>>>>>> target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++++++ >>>>>>> 5 files changed, 60 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>>>>>> index 0f8bd95..61d0675 100644 >>>>>>> --- a/hw/ppc/spapr.c >>>>>>> +++ b/hw/ppc/spapr.c >>>>>>> @@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt, >>>>>>> sPAPREnvironment *spapr) >>>>>>> CPU_FOREACH(cpu) { >>>>>>> DeviceClass *dc = DEVICE_GET_CLASS(cpu); >>>>>>> - int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu)); >>>>>>> + PowerPCCPU *pcpu = POWERPC_CPU(cpu); >>>>>>> + int index = ppc_get_vcpu_dt_id(pcpu); >>>>>>> uint32_t associativity[] = {cpu_to_be32(0x5), >>>>>>> cpu_to_be32(0x0), >>>>>>> cpu_to_be32(0x0), >>>>>>> @@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt, >>>>>>> sPAPREnvironment *spapr) >>>>>>> return ret; >>>>>>> } >>>>>>> + if (pcpu->cpu_version) { >>>>>>> + ret = fdt_setprop(fdt, offset, "cpu-version", >>>>>>> + &pcpu->cpu_version, >>>>>>> sizeof(pcpu->cpu_version)); >>>>>>> + if (ret < 0) { >>>>>>> + return ret; >>>>>>> + } >>>>>>> + } >>>>>>> + >>>>>>> /* Build interrupt servers and gservers properties */ >>>>>>> for (i = 0; i < smp_threads; i++) { >>>>>>> servers_prop[i] = cpu_to_be32(index + i); >>>>>>> @@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs >>>>>>> *args) >>>>>>> kvmppc_set_papr(cpu); >>>>>>> } >>>>>>> + if (cpu->max_compat) { >>>>>>> + ppc_set_compat(cpu, cpu->max_compat); >>>>>>> + } >>>>>>> + >>>>>>> xics_cpu_setup(spapr->icp, cpu); >>>>>>> qemu_register_reset(spapr_cpu_reset, cpu); >>>>>>> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h >>>>>>> index 33d2c51..45c0028 100644 >>>>>>> --- a/target-ppc/cpu-models.h >>>>>>> +++ b/target-ppc/cpu-models.h >>>>>>> @@ -753,4 +753,10 @@ enum { >>>>>>> POWERPC_SVR_8641D = 0x80900121, >>>>>>> }; >>>>>>> +/* Processor Compatibility mask (PCR) */ >>>>>>> +enum { >>>>>>> + POWERPC_ISA_COMPAT_2_05 = 0x02, >>>>>>> + POWERPC_ISA_COMPAT_2_06 = 0x04, >>>>>>> +}; >>>>>>> + >>>>>>> #endif >>>>>>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h >>>>>>> index 2a8515b..dfd1419 100644 >>>>>>> --- a/target-ppc/cpu-qom.h >>>>>>> +++ b/target-ppc/cpu-qom.h >>>>>>> @@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass { >>>>>>> * @env: #CPUPPCState >>>>>>> * @cpu_dt_id: CPU index used in the device tree. KVM uses this >>>>>>> index too >>>>>>> * @max_compat: Maximal supported logical PVR from the command line >>>>>>> + * @cpu_version: Current logical PVR, zero if in "raw" mode >>>>>>> * >>>>>>> * A PowerPC CPU. >>>>>>> */ >>>>>>> @@ -94,6 +95,7 @@ struct PowerPCCPU { >>>>>>> CPUPPCState env; >>>>>>> int cpu_dt_id; >>>>>>> uint32_t max_compat; >>>>>>> + uint32_t cpu_version; >>>>>>> }; >>>>>>> static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env) >>>>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h >>>>>>> index d498340..f61675a 100644 >>>>>>> --- a/target-ppc/cpu.h >>>>>>> +++ b/target-ppc/cpu.h >>>>>>> @@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env, target_ulong >>>>>>> value); >>>>>>> void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf); >>>>>>> +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version); >>>>>>> + >>>>>>> /* Time-base and decrementer management */ >>>>>>> #ifndef NO_CPU_IO_DEFS >>>>>>> uint64_t cpu_ppc_load_tbl (CPUPPCState *env); >>>>>>> @@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState *env) >>>>>>> #define SPR_LPCR (0x13E) >>>>>>> #define SPR_BOOKE_DVC2 (0x13F) >>>>>>> #define SPR_BOOKE_TSR (0x150) >>>>>>> +#define SPR_PCR (0x152) >>>>>>> #define SPR_BOOKE_TCR (0x154) >>>>>>> #define SPR_BOOKE_TLB0PS (0x158) >>>>>>> #define SPR_BOOKE_TLB1PS (0x159) >>>>>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c >>>>>>> index 6f61b34..c4bd5de 100644 >>>>>>> --- a/target-ppc/translate_init.c >>>>>>> +++ b/target-ppc/translate_init.c >>>>>>> @@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env) >>>>>>> /* Can't find information on what this should be on reset. This >>>>>>> * value is the one used by 74xx processors. */ >>>>>>> vscr_init(env, 0x00010000); >>>>>>> + >>>>>>> + /* >>>>>>> + * Register PCR to report POWERPC_EXCP_PRIV_REG instead of >>>>>>> + * POWERPC_EXCP_INVAL_SPR. >>>>>>> + */ >>>>>>> + spr_register(env, SPR_PCR, "PCR", >>>>>>> + SPR_NOACCESS, SPR_NOACCESS, >>>>>>> + SPR_NOACCESS, SPR_NOACCESS, >>>>>>> + 0x00000000); >>>>>>> } >>>>>>> POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) >>>>>>> @@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, >>>>>>> Error **errp) >>>>>>> } >>>>>>> } >>>>>>> +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version) >>>>>>> +{ >>>>>>> + CPUPPCState *env = &cpu->env; >>>>>>> + >>>>>>> + cpu->cpu_version = cpu_version; >>>>>>> + >>>>>>> + /* >>>>>>> + * Calculate PCR value from virtual PVR. >>>>>>> + * TODO: support actual compatibility in TCG. >>>>>>> + */ >>>>>>> + switch (cpu_version) { >>>>>>> + case CPU_POWERPC_LOGICAL_2_05: >>>>>>> + env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05; >>>>>>> + break; >>>>>>> + case CPU_POWERPC_LOGICAL_2_06: >>>>>>> + env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06; >>>>>>> + break; >>>>>>> + case CPU_POWERPC_LOGICAL_2_06_PLUS: >>>>>>> + env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06; >>>>>>> + break; >>>>>> Don't we have to disable all those fancy p8 features too? >>>>>> >>>>>> #define PCR_VEC_DIS (1ul << (63-0)) /* Vec. disable (bit NA since >>>>>> POWER8) */ >>>>>> #define PCR_VSX_DIS (1ul << (63-1)) /* VSX disable (bit NA since >>>>>> POWER8) */ >>>>>> #define PCR_TM_DIS (1ul << (63-2)) /* Trans. memory disable >>>>>> (POWER8) */ >>>>> Yep, makes sense to set "TM" for POWER < 8 and VSX+VEC for POWER < 7. >>>>> >>>>>> In fact, we probably want those bits always set when compat < power8. >>>>> Why? VSX is present on power7. VEC is defined in 2.06 too. >>>> True. So how does pre-p8 treat those p8 and above bits? >>> PowerISA 2.07 says if 2.06 mode is selected, the TM bit does not matter >>> anymore - TM will be disabled because 2.06 does not define it at all. The >>> same is true for VSX and 2.05 mode. So just setting a mode must be ok. >> And for POWER8-on-POWER9 (or whatever it will be called) we can just set >> PCR to the respective feature disable bits. I think that should work. > I lost you here. I suspect it will also have "2.07" compatibility mode and > we will use that instead of disabling feature one by one. Considering that the architects added by-feature bits, I would assume they won't introduce a 2.07 bit but rather make every new feature disableable one by one. But maybe I'm wrong - the point is that either way will work and is future proof ;). Alex