On 31/10/16 19:39, David Gibson wrote: > On Mon, Oct 31, 2016 at 04:55:42PM +1100, Alexey Kardashevskiy wrote: >> 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
In last 5+ years, I have never seen pointer compared anyhow but using "==" and "!=". A bit unusual. Reviewed-by: Alexey Kardashevskiy <a...@ozlabs.ru> >>> + */ >>> { /* 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 ;) > > For local variables, brevity is a virtue. -- Alexey
signature.asc
Description: OpenPGP digital signature