[PATCH 04/29] arm64: KVM: system register definitions for 64bit guests

[cov@codeaurora.org (Christopher Covington)]

Hi Marc,

Here are a few minor questions and suggestions.

On 03/04/2013 10:47 PM, Marc Zyngier wrote:
> Define the saved/restored registers for 64bit guests.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/include/asm/kvm_asm.h | 68 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 arch/arm64/include/asm/kvm_asm.h

[...]

> +/*
> + * 0 is reserved as an invalid value.
> + * Order *must* be kept in sync with the hyp switch code.
> + */
> +#define	MPIDR_EL1	1	/* MultiProcessor Affinity Register */
> +#define	CSSELR_EL1	2	/* Cache Size Selection Register */
> +#define	SCTLR_EL1	3	/* System Control Register */
> +#define	ACTLR_EL1	4	/* Auxilliary Control Register */
> +#define	CPACR_EL1	5	/* Coprocessor Access Control */
> +#define	TTBR0_EL1	6	/* Translation Table Base Register 0 */
> +#define	TTBR1_EL1	7	/* Translation Table Base Register 1 */
> +#define	TCR_EL1		8	/* Translation Control Register */
> +#define	ESR_EL1		9	/* Exception Syndrome Register */
> +#define	AFSR0_EL1	10	/* Auxilary Fault Status Register 0 */
> +#define	AFSR1_EL1	11	/* Auxilary Fault Status Register 1 */
> +#define	FAR_EL1		12	/* Fault Address Register */
> +#define	MAIR_EL1	13	/* Memory Attribute Indirection Register */
> +#define	VBAR_EL1	14	/* Vector Base Address Register */
> +#define	CONTEXTIDR_EL1	15	/* Context ID Register */
> +#define	TPIDR_EL0	16	/* Thread ID, User R/W */
> +#define	TPIDRRO_EL0	17	/* Thread ID, User R/O */
> +#define	TPIDR_EL1	18	/* Thread ID, Privileged */
> +#define	AMAIR_EL1	19	/* Aux Memory Attribute Indirection Register */
> +#define	CNTKCTL_EL1	20	/* Timer Control Register (EL1) */

Any particular reason to have CNTKCTL_EL1 enumerated here and handled by
(dump|load)_sysregs, but all the other timer registers handled by
(save|restore)_timer_state in hyp.S?

> +#define	NR_SYS_REGS	21

[...]

> +/* Hyp Syndrome Register (HSR) bits */
> +#define ESR_EL2_EC_SHIFT	(26)
> +#define ESR_EL2_EC		(0x3fU << ESR_EL2_EC_SHIFT)
> +#define ESR_EL2_IL		(1U << 25)
> +#define ESR_EL2_ISS		(ESR_EL2_IL - 1)
> +#define ESR_EL2_ISV_SHIFT	(24)
> +#define ESR_EL2_ISV		(1U << ESR_EL2_ISV_SHIFT)
> +#define ESR_EL2_SAS_SHIFT	(22)
> +#define ESR_EL2_SAS		(3U << ESR_EL2_SAS_SHIFT)
> +#define ESR_EL2_SSE		(1 << 21)
> +#define ESR_EL2_SRT_SHIFT	(16)
> +#define ESR_EL2_SRT_MASK	(0x1f << ESR_EL2_SRT_SHIFT)
> +#define ESR_EL2_SF 		(1 << 15)
> +#define ESR_EL2_AR 		(1 << 14)
> +#define ESR_EL2_EA 		(1 << 9)
> +#define ESR_EL2_CM 		(1 << 8)
> +#define ESR_EL2_S1PTW 		(1 << 7)
> +#define ESR_EL2_WNR		(1 << 6)
> +#define ESR_EL2_FSC		(0x3f)
> +#define ESR_EL2_FSC_TYPE	(0x3c)
> +
> +#define ESR_EL2_CV_SHIFT	(24)
> +#define ESR_EL2_CV		(1U << ESR_EL2_CV_SHIFT)
> +#define ESR_EL2_COND_SHIFT	(20)
> +#define ESR_EL2_COND		(0xfU << ESR_EL2_COND_SHIFT)

[...]

> +#define ESR_EL2_EC_UNKNOWN	(0x00)
> +#define ESR_EL2_EC_WFI		(0x01)
> +#define ESR_EL2_EC_CP15_32	(0x03)
> +#define ESR_EL2_EC_CP15_64	(0x04)
> +#define ESR_EL2_EC_CP14_MR	(0x05)
> +#define ESR_EL2_EC_CP14_LS	(0x06)
> +#define ESR_EL2_EC_SIMD_FP	(0x07)
> +#define ESR_EL2_EC_CP10_ID	(0x08)
> +#define ESR_EL2_EC_CP14_64	(0x0C)
> +#define ESR_EL2_EC_ILL_ISS	(0x0E)
> +#define ESR_EL2_EC_SVC32	(0x11)
> +#define ESR_EL2_EC_HVC32	(0x12)
> +#define ESR_EL2_EC_SMC32	(0x13)
> +#define ESR_EL2_EC_SVC64	(0x14)
> +#define ESR_EL2_EC_HVC64	(0x16)
> +#define ESR_EL2_EC_SMC64	(0x17)
> +#define ESR_EL2_EC_SYS64	(0x18)
> +#define ESR_EL2_EC_IABT		(0x20)
> +#define ESR_EL2_EC_IABT_HYP	(0x21)
> +#define ESR_EL2_EC_PC_ALIGN	(0x22)
> +#define ESR_EL2_EC_DABT		(0x24)
> +#define ESR_EL2_EC_DABT_HYP	(0x25)
> +#define ESR_EL2_EC_SP_ALIGN	(0x26)
> +#define ESR_EL2_EC_FP32		(0x28)
> +#define ESR_EL2_EC_FP64		(0x2C)
> +#define ESR_EL2_EC_SERRROR	(0x2F)
> +#define ESR_EL2_EC_BREAKPT	(0x30)
> +#define ESR_EL2_EC_BREAKPT_HYP	(0x31)
> +#define ESR_EL2_EC_SOFTSTP	(0x32)
> +#define ESR_EL2_EC_SOFTSTP_HYP	(0x33)
> +#define ESR_EL2_EC_WATCHPT	(0x34)
> +#define ESR_EL2_EC_WATCHPT_HYP	(0x35)
> +#define ESR_EL2_EC_BKPT32	(0x38)
> +#define ESR_EL2_EC_VECTOR32	(0x3A)
> +#define ESR_EL2_EC_BKPT64	(0x3C)

Is there any functional difference between these fields and bits at EL2 and
these fields at other exception levels? If not, you could consider omitting
"_EL2".

[...]

Regards,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
the Linux Foundation