On 30/10/16 22:12, David Gibson wrote:
> Current ppc_set_compat() will attempt to set any compatiblity mode
> specified, regardless of whether it's available on the CPU. The caller is
> expected to make sure it is setting a possible mode, which is awkwward
> because most of the information to make that decision is at the CPU level.
>
> This begins to clean this up by introducing a ppc_check_compat() function
> which will determine if a given compatiblity mode is supported on a CPU
> (and also whether it lies within specified minimum and maximum compat
> levels, which will be useful later). It also contains an assertion that
> the CPU has a "virtual hypervisor"[1], that is, that the guest isn't
> permitted to execute hypervisor privilege code. Without that, the guest
> would own the PCR and so could override any mode set here. Only machine
> types which use a virtual hypervisor (i.e. 'pseries') should use
> ppc_check_compat().
>
> ppc_set_compat() is modified to validate the compatibility mode it is given
> and fail if it's not available on this CPU.
>
> [1] Or user-only mode, which also obviously doesn't allow access to the
> hypervisor privileged PCR. We don't use that now, but could in future.
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> target-ppc/compat.c | 41 +++++++++++++++++++++++++++++++++++++++++
> target-ppc/cpu.h | 2 ++
> 2 files changed, 43 insertions(+)
>
> diff --git a/target-ppc/compat.c b/target-ppc/compat.c
> index 66529a6..1059555 100644
> --- a/target-ppc/compat.c
> +++ b/target-ppc/compat.c
> @@ -28,29 +28,37 @@
> typedef struct {
> uint32_t pvr;
> uint64_t pcr;
> + uint64_t pcr_level;
> int max_threads;
> } CompatInfo;
>
> static const CompatInfo compat_table[] = {
> + /*
> + * Ordered from oldest to newest - the code relies on this
> + */
> { /* POWER6, ISA2.05 */
> .pvr = CPU_POWERPC_LOGICAL_2_05,
> .pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
> | PCR_TM_DIS | PCR_VSX_DIS,
> + .pcr_level = PCR_COMPAT_2_05,
> .max_threads = 2,
> },
> { /* POWER7, ISA2.06 */
> .pvr = CPU_POWERPC_LOGICAL_2_06,
> .pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
> + .pcr_level = PCR_COMPAT_2_06,
> .max_threads = 4,
> },
> {
> .pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
> .pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
> + .pcr_level = PCR_COMPAT_2_06,
> .max_threads = 4,
> },
> { /* POWER8, ISA2.07 */
> .pvr = CPU_POWERPC_LOGICAL_2_07,
> .pcr = PCR_COMPAT_2_07,
> + .pcr_level = PCR_COMPAT_2_07,
> .max_threads = 8,
> },
> };
> @@ -67,6 +75,35 @@ static const CompatInfo *compat_by_pvr(uint32_t pvr)
> return NULL;
> }
>
> +bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr,
> + uint32_t min_compat_pvr, uint32_t max_compat_pvr)
> +{
> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> + const CompatInfo *compat = compat_by_pvr(compat_pvr);
> + const CompatInfo *min = compat_by_pvr(min_compat_pvr);
> + const CompatInfo *max = compat_by_pvr(max_compat_pvr);
You keep giving very generic names (as "min" and "max") to local variables ;)
> +
> +#if !defined(CONFIG_USER_ONLY)
> + g_assert(cpu->vhyp);
> +#endif
> + g_assert(!min_compat_pvr || min);
> + g_assert(!max_compat_pvr || max);
> +
> + if (!compat) {
> + /* Not a recognized logical PVR */
> + return false;
> + }
> + if ((min && (compat < min)) || (max && (compat > max))) {
> + /* Outside specified range */
> + return false;
> + }
> + if (!(pcc->pcr_supported & compat->pcr_level)) {
> + /* Not supported by this CPU */
> + return false;
> + }
> + return true;
> +}
> +
> void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp)
> {
> const CompatInfo *compat = compat_by_pvr(compat_pvr);
> @@ -79,6 +116,10 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr,
> Error **errp)
> } else if (!compat) {
> error_setg(errp, "Unknown compatibility PVR 0x%08"PRIx32,
> compat_pvr);
> return;
> + } else if (!ppc_check_compat(cpu, compat_pvr, 0, 0)) {
> + error_setg(errp, "Compatibility PVR 0x%08"PRIx32" not valid for CPU",
> + compat_pvr);
> + return;
> } else {
> pcr = compat->pcr;
> }
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index cfda7b2..91e8be8 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1314,6 +1314,8 @@ static inline int cpu_mmu_index (CPUPPCState *env, bool
> ifetch)
>
> /* Compatibility modes */
> #if defined(TARGET_PPC64)
> +bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr,
> + uint32_t min_compat_pvr, uint32_t max_compat_pvr);
> void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
> int ppc_compat_max_threads(PowerPCCPU *cpu);
> #endif /* defined(TARGET_PPC64) */
>
--
Alexey
