* [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo
2020-11-30 14:21 [PATCH v2 0/7] xen/arm: Emulate ID registers Bertrand Marquis
@ 2020-11-30 14:21 ` Bertrand Marquis
2020-11-30 19:55 ` Volodymyr Babchuk
2020-12-04 23:52 ` Stefano Stabellini
2020-11-30 14:21 ` [PATCH v2 2/7] xen/arm: Add arm64 ID registers definitions Bertrand Marquis
` (5 subsequent siblings)
6 siblings, 2 replies; 31+ messages in thread
From: Bertrand Marquis @ 2020-11-30 14:21 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk
Add definition and entries in cpuinfo for ID registers introduced in
newer Arm Architecture reference manual:
- ID_PFR2: processor feature register 2
- ID_DFR1: debug feature register 1
- ID_MMFR4 and ID_MMFR5: Memory model feature registers 4 and 5
- ID_ISA6: ISA Feature register 6
Add more bitfield definitions in PFR fields of cpuinfo.
Add MVFR2 register definition for aarch32.
Add mvfr values in cpuinfo.
Add some registers definition for arm64 in sysregs as some are not
always know by compilers.
Initialize the new values added in cpuinfo in identify_cpu during init.
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2:
Fix dbg32 table size and add proper initialisation of the second entry
of the table by reading ID_DFR1 register.
---
xen/arch/arm/cpufeature.c | 17 ++++++++
xen/include/asm-arm/arm64/sysregs.h | 25 ++++++++++++
xen/include/asm-arm/cpregs.h | 11 +++++
xen/include/asm-arm/cpufeature.h | 63 ++++++++++++++++++++++++-----
4 files changed, 107 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 44126dbf07..204be9b084 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -114,15 +114,20 @@ void identify_cpu(struct cpuinfo_arm *c)
c->mm64.bits[0] = READ_SYSREG64(ID_AA64MMFR0_EL1);
c->mm64.bits[1] = READ_SYSREG64(ID_AA64MMFR1_EL1);
+ c->mm64.bits[2] = READ_SYSREG64(ID_AA64MMFR2_EL1);
c->isa64.bits[0] = READ_SYSREG64(ID_AA64ISAR0_EL1);
c->isa64.bits[1] = READ_SYSREG64(ID_AA64ISAR1_EL1);
+
+ c->zfr64.bits[0] = READ_SYSREG64(ID_AA64ZFR0_EL1);
#endif
c->pfr32.bits[0] = READ_SYSREG32(ID_PFR0_EL1);
c->pfr32.bits[1] = READ_SYSREG32(ID_PFR1_EL1);
+ c->pfr32.bits[2] = READ_SYSREG32(ID_PFR2_EL1);
c->dbg32.bits[0] = READ_SYSREG32(ID_DFR0_EL1);
+ c->dbg32.bits[1] = READ_SYSREG32(ID_DFR1_EL1);
c->aux32.bits[0] = READ_SYSREG32(ID_AFR0_EL1);
@@ -130,6 +135,8 @@ void identify_cpu(struct cpuinfo_arm *c)
c->mm32.bits[1] = READ_SYSREG32(ID_MMFR1_EL1);
c->mm32.bits[2] = READ_SYSREG32(ID_MMFR2_EL1);
c->mm32.bits[3] = READ_SYSREG32(ID_MMFR3_EL1);
+ c->mm32.bits[4] = READ_SYSREG32(ID_MMFR4_EL1);
+ c->mm32.bits[5] = READ_SYSREG32(ID_MMFR5_EL1);
c->isa32.bits[0] = READ_SYSREG32(ID_ISAR0_EL1);
c->isa32.bits[1] = READ_SYSREG32(ID_ISAR1_EL1);
@@ -137,6 +144,16 @@ void identify_cpu(struct cpuinfo_arm *c)
c->isa32.bits[3] = READ_SYSREG32(ID_ISAR3_EL1);
c->isa32.bits[4] = READ_SYSREG32(ID_ISAR4_EL1);
c->isa32.bits[5] = READ_SYSREG32(ID_ISAR5_EL1);
+ c->isa32.bits[6] = READ_SYSREG32(ID_ISAR6_EL1);
+
+#ifdef CONFIG_ARM_64
+ c->mvfr.bits[0] = READ_SYSREG64(MVFR0_EL1);
+ c->mvfr.bits[1] = READ_SYSREG64(MVFR1_EL1);
+ c->mvfr.bits[2] = READ_SYSREG64(MVFR2_EL1);
+#else
+ c->mvfr.bits[0] = READ_CP32(MVFR0);
+ c->mvfr.bits[1] = READ_CP32(MVFR1);
+#endif
}
/*
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index c60029d38f..5abbeda3fd 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -57,6 +57,31 @@
#define ICH_AP1R2_EL2 __AP1Rx_EL2(2)
#define ICH_AP1R3_EL2 __AP1Rx_EL2(3)
+/*
+ * Define ID coprocessor registers if they are not
+ * already defined by the compiler.
+ *
+ * Values picked from linux kernel
+ */
+#ifndef ID_AA64MMFR2_EL1
+#define ID_AA64MMFR2_EL1 S3_0_C0_C7_2
+#endif
+#ifndef ID_PFR2_EL1
+#define ID_PFR2_EL1 S3_0_C0_C3_4
+#endif
+#ifndef ID_MMFR5_EL1
+#define ID_MMFR5_EL1 S3_0_C0_C3_6
+#endif
+#ifndef ID_ISAR6_EL1
+#define ID_ISAR6_EL1 S3_0_C0_C2_7
+#endif
+#ifndef ID_AA64ZFR0_EL1
+#define ID_AA64ZFR0_EL1 S3_0_C0_C4_4
+#endif
+#ifndef ID_DFR1_EL1
+#define ID_DFR1_EL1 S3_0_C0_C3_5
+#endif
+
/* Access to system registers */
#define READ_SYSREG32(name) ((uint32_t)READ_SYSREG64(name))
diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
index 8fd344146e..58be898891 100644
--- a/xen/include/asm-arm/cpregs.h
+++ b/xen/include/asm-arm/cpregs.h
@@ -63,6 +63,7 @@
#define FPSID p10,7,c0,c0,0 /* Floating-Point System ID Register */
#define FPSCR p10,7,c1,c0,0 /* Floating-Point Status and Control Register */
#define MVFR0 p10,7,c7,c0,0 /* Media and VFP Feature Register 0 */
+#define MVFR1 p10,7,c6,c0,0 /* Media and VFP Feature Register 1 */
#define FPEXC p10,7,c8,c0,0 /* Floating-Point Exception Control Register */
#define FPINST p10,7,c9,c0,0 /* Floating-Point Instruction Register */
#define FPINST2 p10,7,c10,c0,0 /* Floating-point Instruction Register 2 */
@@ -108,18 +109,23 @@
#define MPIDR p15,0,c0,c0,5 /* Multiprocessor Affinity Register */
#define ID_PFR0 p15,0,c0,c1,0 /* Processor Feature Register 0 */
#define ID_PFR1 p15,0,c0,c1,1 /* Processor Feature Register 1 */
+#define ID_PFR2 p15,0,c0,c3,4 /* Processor Feature Register 2 */
#define ID_DFR0 p15,0,c0,c1,2 /* Debug Feature Register 0 */
+#define ID_DFR1 p15,0,c0,c3,5 /* Debug Feature Register 1 */
#define ID_AFR0 p15,0,c0,c1,3 /* Auxiliary Feature Register 0 */
#define ID_MMFR0 p15,0,c0,c1,4 /* Memory Model Feature Register 0 */
#define ID_MMFR1 p15,0,c0,c1,5 /* Memory Model Feature Register 1 */
#define ID_MMFR2 p15,0,c0,c1,6 /* Memory Model Feature Register 2 */
#define ID_MMFR3 p15,0,c0,c1,7 /* Memory Model Feature Register 3 */
+#define ID_MMFR4 p15,0,c0,c2,6 /* Memory Model Feature Register 4 */
+#define ID_MMFR5 p15,0,c0,c3,6 /* Memory Model Feature Register 5 */
#define ID_ISAR0 p15,0,c0,c2,0 /* ISA Feature Register 0 */
#define ID_ISAR1 p15,0,c0,c2,1 /* ISA Feature Register 1 */
#define ID_ISAR2 p15,0,c0,c2,2 /* ISA Feature Register 2 */
#define ID_ISAR3 p15,0,c0,c2,3 /* ISA Feature Register 3 */
#define ID_ISAR4 p15,0,c0,c2,4 /* ISA Feature Register 4 */
#define ID_ISAR5 p15,0,c0,c2,5 /* ISA Feature Register 5 */
+#define ID_ISAR6 p15,0,c0,c2,7 /* ISA Feature Register 6 */
#define CCSIDR p15,1,c0,c0,0 /* Cache Size ID Registers */
#define CLIDR p15,1,c0,c0,1 /* Cache Level ID Register */
#define CSSELR p15,2,c0,c0,0 /* Cache Size Selection Register */
@@ -312,18 +318,23 @@
#define HSTR_EL2 HSTR
#define ID_AFR0_EL1 ID_AFR0
#define ID_DFR0_EL1 ID_DFR0
+#define ID_DFR1_EL1 ID_DFR1
#define ID_ISAR0_EL1 ID_ISAR0
#define ID_ISAR1_EL1 ID_ISAR1
#define ID_ISAR2_EL1 ID_ISAR2
#define ID_ISAR3_EL1 ID_ISAR3
#define ID_ISAR4_EL1 ID_ISAR4
#define ID_ISAR5_EL1 ID_ISAR5
+#define ID_ISAR6_EL1 ID_ISAR6
#define ID_MMFR0_EL1 ID_MMFR0
#define ID_MMFR1_EL1 ID_MMFR1
#define ID_MMFR2_EL1 ID_MMFR2
#define ID_MMFR3_EL1 ID_MMFR3
+#define ID_MMFR4_EL1 ID_MMFR4
+#define ID_MMFR5_EL1 ID_MMFR5
#define ID_PFR0_EL1 ID_PFR0
#define ID_PFR1_EL1 ID_PFR1
+#define ID_PFR2_EL1 ID_PFR2
#define IFSR32_EL2 IFSR
#define MDCR_EL2 HDCR
#define MIDR_EL1 MIDR
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index c7b5052992..64354c3f19 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -148,6 +148,7 @@ struct cpuinfo_arm {
union {
uint64_t bits[2];
struct {
+ /* PFR0 */
unsigned long el0:4;
unsigned long el1:4;
unsigned long el2:4;
@@ -155,9 +156,23 @@ struct cpuinfo_arm {
unsigned long fp:4; /* Floating Point */
unsigned long simd:4; /* Advanced SIMD */
unsigned long gic:4; /* GIC support */
- unsigned long __res0:28;
+ unsigned long ras:4;
+ unsigned long sve:4;
+ unsigned long sel2:4;
+ unsigned long mpam:4;
+ unsigned long amu:4;
+ unsigned long dit:4;
+ unsigned long __res0:4;
unsigned long csv2:4;
- unsigned long __res1:4;
+ unsigned long cvs3:4;
+
+ /* PFR1 */
+ unsigned long bt:4;
+ unsigned long ssbs:4;
+ unsigned long mte:4;
+ unsigned long ras_frac:4;
+ unsigned long mpam_frac:4;
+ unsigned long __res1:44;
};
} pfr64;
@@ -170,7 +185,7 @@ struct cpuinfo_arm {
} aux64;
union {
- uint64_t bits[2];
+ uint64_t bits[3];
struct {
unsigned long pa_range:4;
unsigned long asid_bits:4;
@@ -190,6 +205,8 @@ struct cpuinfo_arm {
unsigned long pan:4;
unsigned long __res1:8;
unsigned long __res2:32;
+
+ unsigned long __res3:64;
};
} mm64;
@@ -197,6 +214,10 @@ struct cpuinfo_arm {
uint64_t bits[2];
} isa64;
+ struct {
+ uint64_t bits[1];
+ } zfr64;
+
#endif
/*
@@ -204,25 +225,38 @@ struct cpuinfo_arm {
* when running in 32-bit mode.
*/
union {
- uint32_t bits[2];
+ uint32_t bits[3];
struct {
+ /* PFR0 */
unsigned long arm:4;
unsigned long thumb:4;
unsigned long jazelle:4;
unsigned long thumbee:4;
- unsigned long __res0:16;
+ unsigned long csv2:4;
+ unsigned long amu:4;
+ unsigned long dit:4;
+ unsigned long ras:4;
+ /* PFR1 */
unsigned long progmodel:4;
unsigned long security:4;
unsigned long mprofile:4;
unsigned long virt:4;
unsigned long gentimer:4;
- unsigned long __res1:12;
+ unsigned long sec_frac:4;
+ unsigned long virt_frac:4;
+ unsigned long gic:4;
+
+ /* PFR2 */
+ unsigned long csv3:4;
+ unsigned long ssbs:4;
+ unsigned long ras_frac:4;
+ unsigned long __res2:20;
};
} pfr32;
struct {
- uint32_t bits[1];
+ uint32_t bits[2];
} dbg32;
struct {
@@ -230,12 +264,23 @@ struct cpuinfo_arm {
} aux32;
struct {
- uint32_t bits[4];
+ uint32_t bits[6];
} mm32;
struct {
- uint32_t bits[6];
+ uint32_t bits[7];
} isa32;
+
+#ifdef CONFIG_ARM_64
+ struct {
+ uint64_t bits[3];
+ } mvfr;
+#else
+ /* Only MVFR0 and MVFR1 exist on armv7 */
+ struct {
+ uint32_t bits[2];
+ } mvfr;
+#endif
};
extern struct cpuinfo_arm boot_cpu_data;
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo
2020-11-30 14:21 ` [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo Bertrand Marquis
@ 2020-11-30 19:55 ` Volodymyr Babchuk
2020-12-04 23:52 ` Stefano Stabellini
1 sibling, 0 replies; 31+ messages in thread
From: Volodymyr Babchuk @ 2020-11-30 19:55 UTC (permalink / raw)
To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hello Bertrand,
Bertrand Marquis writes:
> Add definition and entries in cpuinfo for ID registers introduced in
> newer Arm Architecture reference manual:
> - ID_PFR2: processor feature register 2
> - ID_DFR1: debug feature register 1
> - ID_MMFR4 and ID_MMFR5: Memory model feature registers 4 and 5
> - ID_ISA6: ISA Feature register 6
> Add more bitfield definitions in PFR fields of cpuinfo.
> Add MVFR2 register definition for aarch32.
> Add mvfr values in cpuinfo.
> Add some registers definition for arm64 in sysregs as some are not
> always know by compilers.
> Initialize the new values added in cpuinfo in identify_cpu during init.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>
> ---
> Changes in V2:
> Fix dbg32 table size and add proper initialisation of the second entry
> of the table by reading ID_DFR1 register.
> ---
> xen/arch/arm/cpufeature.c | 17 ++++++++
> xen/include/asm-arm/arm64/sysregs.h | 25 ++++++++++++
> xen/include/asm-arm/cpregs.h | 11 +++++
> xen/include/asm-arm/cpufeature.h | 63 ++++++++++++++++++++++++-----
> 4 files changed, 107 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 44126dbf07..204be9b084 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -114,15 +114,20 @@ void identify_cpu(struct cpuinfo_arm *c)
>
> c->mm64.bits[0] = READ_SYSREG64(ID_AA64MMFR0_EL1);
> c->mm64.bits[1] = READ_SYSREG64(ID_AA64MMFR1_EL1);
> + c->mm64.bits[2] = READ_SYSREG64(ID_AA64MMFR2_EL1);
>
> c->isa64.bits[0] = READ_SYSREG64(ID_AA64ISAR0_EL1);
> c->isa64.bits[1] = READ_SYSREG64(ID_AA64ISAR1_EL1);
> +
> + c->zfr64.bits[0] = READ_SYSREG64(ID_AA64ZFR0_EL1);
> #endif
>
> c->pfr32.bits[0] = READ_SYSREG32(ID_PFR0_EL1);
> c->pfr32.bits[1] = READ_SYSREG32(ID_PFR1_EL1);
> + c->pfr32.bits[2] = READ_SYSREG32(ID_PFR2_EL1);
>
> c->dbg32.bits[0] = READ_SYSREG32(ID_DFR0_EL1);
> + c->dbg32.bits[1] = READ_SYSREG32(ID_DFR1_EL1);
>
> c->aux32.bits[0] = READ_SYSREG32(ID_AFR0_EL1);
>
> @@ -130,6 +135,8 @@ void identify_cpu(struct cpuinfo_arm *c)
> c->mm32.bits[1] = READ_SYSREG32(ID_MMFR1_EL1);
> c->mm32.bits[2] = READ_SYSREG32(ID_MMFR2_EL1);
> c->mm32.bits[3] = READ_SYSREG32(ID_MMFR3_EL1);
> + c->mm32.bits[4] = READ_SYSREG32(ID_MMFR4_EL1);
> + c->mm32.bits[5] = READ_SYSREG32(ID_MMFR5_EL1);
>
> c->isa32.bits[0] = READ_SYSREG32(ID_ISAR0_EL1);
> c->isa32.bits[1] = READ_SYSREG32(ID_ISAR1_EL1);
> @@ -137,6 +144,16 @@ void identify_cpu(struct cpuinfo_arm *c)
> c->isa32.bits[3] = READ_SYSREG32(ID_ISAR3_EL1);
> c->isa32.bits[4] = READ_SYSREG32(ID_ISAR4_EL1);
> c->isa32.bits[5] = READ_SYSREG32(ID_ISAR5_EL1);
> + c->isa32.bits[6] = READ_SYSREG32(ID_ISAR6_EL1);
> +
> +#ifdef CONFIG_ARM_64
> + c->mvfr.bits[0] = READ_SYSREG64(MVFR0_EL1);
> + c->mvfr.bits[1] = READ_SYSREG64(MVFR1_EL1);
> + c->mvfr.bits[2] = READ_SYSREG64(MVFR2_EL1);
> +#else
> + c->mvfr.bits[0] = READ_CP32(MVFR0);
> + c->mvfr.bits[1] = READ_CP32(MVFR1);
> +#endif
> }
>
> /*
> diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
> index c60029d38f..5abbeda3fd 100644
> --- a/xen/include/asm-arm/arm64/sysregs.h
> +++ b/xen/include/asm-arm/arm64/sysregs.h
> @@ -57,6 +57,31 @@
> #define ICH_AP1R2_EL2 __AP1Rx_EL2(2)
> #define ICH_AP1R3_EL2 __AP1Rx_EL2(3)
>
> +/*
> + * Define ID coprocessor registers if they are not
> + * already defined by the compiler.
> + *
> + * Values picked from linux kernel
> + */
> +#ifndef ID_AA64MMFR2_EL1
> +#define ID_AA64MMFR2_EL1 S3_0_C0_C7_2
> +#endif
> +#ifndef ID_PFR2_EL1
> +#define ID_PFR2_EL1 S3_0_C0_C3_4
> +#endif
> +#ifndef ID_MMFR5_EL1
> +#define ID_MMFR5_EL1 S3_0_C0_C3_6
> +#endif
> +#ifndef ID_ISAR6_EL1
> +#define ID_ISAR6_EL1 S3_0_C0_C2_7
> +#endif
> +#ifndef ID_AA64ZFR0_EL1
> +#define ID_AA64ZFR0_EL1 S3_0_C0_C4_4
> +#endif
> +#ifndef ID_DFR1_EL1
> +#define ID_DFR1_EL1 S3_0_C0_C3_5
> +#endif
> +
> /* Access to system registers */
>
> #define READ_SYSREG32(name) ((uint32_t)READ_SYSREG64(name))
> diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
> index 8fd344146e..58be898891 100644
> --- a/xen/include/asm-arm/cpregs.h
> +++ b/xen/include/asm-arm/cpregs.h
> @@ -63,6 +63,7 @@
> #define FPSID p10,7,c0,c0,0 /* Floating-Point System ID Register */
> #define FPSCR p10,7,c1,c0,0 /* Floating-Point Status and Control Register */
> #define MVFR0 p10,7,c7,c0,0 /* Media and VFP Feature Register 0 */
> +#define MVFR1 p10,7,c6,c0,0 /* Media and VFP Feature Register 1 */
> #define FPEXC p10,7,c8,c0,0 /* Floating-Point Exception Control Register */
> #define FPINST p10,7,c9,c0,0 /* Floating-Point Instruction Register */
> #define FPINST2 p10,7,c10,c0,0 /* Floating-point Instruction Register 2 */
> @@ -108,18 +109,23 @@
> #define MPIDR p15,0,c0,c0,5 /* Multiprocessor Affinity Register */
> #define ID_PFR0 p15,0,c0,c1,0 /* Processor Feature Register 0 */
> #define ID_PFR1 p15,0,c0,c1,1 /* Processor Feature Register 1 */
> +#define ID_PFR2 p15,0,c0,c3,4 /* Processor Feature Register 2 */
> #define ID_DFR0 p15,0,c0,c1,2 /* Debug Feature Register 0 */
> +#define ID_DFR1 p15,0,c0,c3,5 /* Debug Feature Register 1 */
> #define ID_AFR0 p15,0,c0,c1,3 /* Auxiliary Feature Register 0 */
> #define ID_MMFR0 p15,0,c0,c1,4 /* Memory Model Feature Register 0 */
> #define ID_MMFR1 p15,0,c0,c1,5 /* Memory Model Feature Register 1 */
> #define ID_MMFR2 p15,0,c0,c1,6 /* Memory Model Feature Register 2 */
> #define ID_MMFR3 p15,0,c0,c1,7 /* Memory Model Feature Register 3 */
> +#define ID_MMFR4 p15,0,c0,c2,6 /* Memory Model Feature Register 4 */
> +#define ID_MMFR5 p15,0,c0,c3,6 /* Memory Model Feature Register 5 */
> #define ID_ISAR0 p15,0,c0,c2,0 /* ISA Feature Register 0 */
> #define ID_ISAR1 p15,0,c0,c2,1 /* ISA Feature Register 1 */
> #define ID_ISAR2 p15,0,c0,c2,2 /* ISA Feature Register 2 */
> #define ID_ISAR3 p15,0,c0,c2,3 /* ISA Feature Register 3 */
> #define ID_ISAR4 p15,0,c0,c2,4 /* ISA Feature Register 4 */
> #define ID_ISAR5 p15,0,c0,c2,5 /* ISA Feature Register 5 */
> +#define ID_ISAR6 p15,0,c0,c2,7 /* ISA Feature Register 6 */
> #define CCSIDR p15,1,c0,c0,0 /* Cache Size ID Registers */
> #define CLIDR p15,1,c0,c0,1 /* Cache Level ID Register */
> #define CSSELR p15,2,c0,c0,0 /* Cache Size Selection Register */
> @@ -312,18 +318,23 @@
> #define HSTR_EL2 HSTR
> #define ID_AFR0_EL1 ID_AFR0
> #define ID_DFR0_EL1 ID_DFR0
> +#define ID_DFR1_EL1 ID_DFR1
> #define ID_ISAR0_EL1 ID_ISAR0
> #define ID_ISAR1_EL1 ID_ISAR1
> #define ID_ISAR2_EL1 ID_ISAR2
> #define ID_ISAR3_EL1 ID_ISAR3
> #define ID_ISAR4_EL1 ID_ISAR4
> #define ID_ISAR5_EL1 ID_ISAR5
> +#define ID_ISAR6_EL1 ID_ISAR6
> #define ID_MMFR0_EL1 ID_MMFR0
> #define ID_MMFR1_EL1 ID_MMFR1
> #define ID_MMFR2_EL1 ID_MMFR2
> #define ID_MMFR3_EL1 ID_MMFR3
> +#define ID_MMFR4_EL1 ID_MMFR4
> +#define ID_MMFR5_EL1 ID_MMFR5
> #define ID_PFR0_EL1 ID_PFR0
> #define ID_PFR1_EL1 ID_PFR1
> +#define ID_PFR2_EL1 ID_PFR2
> #define IFSR32_EL2 IFSR
> #define MDCR_EL2 HDCR
> #define MIDR_EL1 MIDR
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index c7b5052992..64354c3f19 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -148,6 +148,7 @@ struct cpuinfo_arm {
> union {
> uint64_t bits[2];
> struct {
> + /* PFR0 */
> unsigned long el0:4;
> unsigned long el1:4;
> unsigned long el2:4;
> @@ -155,9 +156,23 @@ struct cpuinfo_arm {
> unsigned long fp:4; /* Floating Point */
> unsigned long simd:4; /* Advanced SIMD */
> unsigned long gic:4; /* GIC support */
> - unsigned long __res0:28;
> + unsigned long ras:4;
> + unsigned long sve:4;
> + unsigned long sel2:4;
> + unsigned long mpam:4;
> + unsigned long amu:4;
> + unsigned long dit:4;
> + unsigned long __res0:4;
> unsigned long csv2:4;
> - unsigned long __res1:4;
> + unsigned long cvs3:4;
> +
> + /* PFR1 */
> + unsigned long bt:4;
> + unsigned long ssbs:4;
> + unsigned long mte:4;
> + unsigned long ras_frac:4;
> + unsigned long mpam_frac:4;
> + unsigned long __res1:44;
> };
> } pfr64;
>
> @@ -170,7 +185,7 @@ struct cpuinfo_arm {
> } aux64;
>
> union {
> - uint64_t bits[2];
> + uint64_t bits[3];
> struct {
> unsigned long pa_range:4;
> unsigned long asid_bits:4;
> @@ -190,6 +205,8 @@ struct cpuinfo_arm {
> unsigned long pan:4;
> unsigned long __res1:8;
> unsigned long __res2:32;
> +
> + unsigned long __res3:64;
> };
> } mm64;
>
> @@ -197,6 +214,10 @@ struct cpuinfo_arm {
> uint64_t bits[2];
> } isa64;
>
> + struct {
> + uint64_t bits[1];
> + } zfr64;
> +
> #endif
>
> /*
> @@ -204,25 +225,38 @@ struct cpuinfo_arm {
> * when running in 32-bit mode.
> */
> union {
> - uint32_t bits[2];
> + uint32_t bits[3];
> struct {
> + /* PFR0 */
> unsigned long arm:4;
> unsigned long thumb:4;
> unsigned long jazelle:4;
> unsigned long thumbee:4;
> - unsigned long __res0:16;
> + unsigned long csv2:4;
> + unsigned long amu:4;
> + unsigned long dit:4;
> + unsigned long ras:4;
>
> + /* PFR1 */
> unsigned long progmodel:4;
> unsigned long security:4;
> unsigned long mprofile:4;
> unsigned long virt:4;
> unsigned long gentimer:4;
> - unsigned long __res1:12;
> + unsigned long sec_frac:4;
> + unsigned long virt_frac:4;
> + unsigned long gic:4;
> +
> + /* PFR2 */
> + unsigned long csv3:4;
> + unsigned long ssbs:4;
> + unsigned long ras_frac:4;
> + unsigned long __res2:20;
> };
> } pfr32;
>
> struct {
> - uint32_t bits[1];
> + uint32_t bits[2];
> } dbg32;
>
> struct {
> @@ -230,12 +264,23 @@ struct cpuinfo_arm {
> } aux32;
>
> struct {
> - uint32_t bits[4];
> + uint32_t bits[6];
> } mm32;
>
> struct {
> - uint32_t bits[6];
> + uint32_t bits[7];
> } isa32;
> +
> +#ifdef CONFIG_ARM_64
> + struct {
> + uint64_t bits[3];
> + } mvfr;
> +#else
> + /* Only MVFR0 and MVFR1 exist on armv7 */
> + struct {
> + uint32_t bits[2];
> + } mvfr;
> +#endif
> };
>
> extern struct cpuinfo_arm boot_cpu_data;
--
Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo
2020-11-30 14:21 ` [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo Bertrand Marquis
2020-11-30 19:55 ` Volodymyr Babchuk
@ 2020-12-04 23:52 ` Stefano Stabellini
2020-12-07 17:35 ` Bertrand Marquis
1 sibling, 1 reply; 31+ messages in thread
From: Stefano Stabellini @ 2020-12-04 23:52 UTC (permalink / raw)
To: Bertrand Marquis
Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk
There is a typo in the subject line
On Mon, 30 Nov 2020, Bertrand Marquis wrote:
> Add definition and entries in cpuinfo for ID registers introduced in
> newer Arm Architecture reference manual:
> - ID_PFR2: processor feature register 2
> - ID_DFR1: debug feature register 1
> - ID_MMFR4 and ID_MMFR5: Memory model feature registers 4 and 5
> - ID_ISA6: ISA Feature register 6
> Add more bitfield definitions in PFR fields of cpuinfo.
> Add MVFR2 register definition for aarch32.
> Add mvfr values in cpuinfo.
> Add some registers definition for arm64 in sysregs as some are not
> always know by compilers.
> Initialize the new values added in cpuinfo in identify_cpu during init.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
I realize I am using an old compiler but I am getting a build error:
/local/repos/gcc-linaro-5.3.1-2016.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -MMD -MP -MF ./.cpufeature.o.d -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include /local/repos/xen-upstream/xen/include/xen/config.h -Wa,--strip-local-absolute -g -mcpu=generic -mgeneral-regs-only -I/local/repos/xen-upstream/xen/include -fno-stack-protector -fno-exceptions -fno-asynchronous-unwind-tables -Wnested-externs '-D__OBJECT_FILE__="cpufeature.o"' -c cpufeature.c -o cpufeature.o
{standard input}: Assembler messages:
{standard input}:634: Error: unknown or missing system register name at operand 2 -- `mrs x1,ID_MMFR4_EL1'
If I remove the line:
c->mm32.bits[4] = READ_SYSREG32(ID_MMFR4_EL1);
it builds just fine
> ---
> Changes in V2:
> Fix dbg32 table size and add proper initialisation of the second entry
> of the table by reading ID_DFR1 register.
> ---
> xen/arch/arm/cpufeature.c | 17 ++++++++
> xen/include/asm-arm/arm64/sysregs.h | 25 ++++++++++++
> xen/include/asm-arm/cpregs.h | 11 +++++
> xen/include/asm-arm/cpufeature.h | 63 ++++++++++++++++++++++++-----
> 4 files changed, 107 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 44126dbf07..204be9b084 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -114,15 +114,20 @@ void identify_cpu(struct cpuinfo_arm *c)
>
> c->mm64.bits[0] = READ_SYSREG64(ID_AA64MMFR0_EL1);
> c->mm64.bits[1] = READ_SYSREG64(ID_AA64MMFR1_EL1);
> + c->mm64.bits[2] = READ_SYSREG64(ID_AA64MMFR2_EL1);
>
> c->isa64.bits[0] = READ_SYSREG64(ID_AA64ISAR0_EL1);
> c->isa64.bits[1] = READ_SYSREG64(ID_AA64ISAR1_EL1);
> +
> + c->zfr64.bits[0] = READ_SYSREG64(ID_AA64ZFR0_EL1);
> #endif
>
> c->pfr32.bits[0] = READ_SYSREG32(ID_PFR0_EL1);
> c->pfr32.bits[1] = READ_SYSREG32(ID_PFR1_EL1);
> + c->pfr32.bits[2] = READ_SYSREG32(ID_PFR2_EL1);
>
> c->dbg32.bits[0] = READ_SYSREG32(ID_DFR0_EL1);
> + c->dbg32.bits[1] = READ_SYSREG32(ID_DFR1_EL1);
>
> c->aux32.bits[0] = READ_SYSREG32(ID_AFR0_EL1);
>
> @@ -130,6 +135,8 @@ void identify_cpu(struct cpuinfo_arm *c)
> c->mm32.bits[1] = READ_SYSREG32(ID_MMFR1_EL1);
> c->mm32.bits[2] = READ_SYSREG32(ID_MMFR2_EL1);
> c->mm32.bits[3] = READ_SYSREG32(ID_MMFR3_EL1);
> + c->mm32.bits[4] = READ_SYSREG32(ID_MMFR4_EL1);
> + c->mm32.bits[5] = READ_SYSREG32(ID_MMFR5_EL1);
>
> c->isa32.bits[0] = READ_SYSREG32(ID_ISAR0_EL1);
> c->isa32.bits[1] = READ_SYSREG32(ID_ISAR1_EL1);
> @@ -137,6 +144,16 @@ void identify_cpu(struct cpuinfo_arm *c)
> c->isa32.bits[3] = READ_SYSREG32(ID_ISAR3_EL1);
> c->isa32.bits[4] = READ_SYSREG32(ID_ISAR4_EL1);
> c->isa32.bits[5] = READ_SYSREG32(ID_ISAR5_EL1);
> + c->isa32.bits[6] = READ_SYSREG32(ID_ISAR6_EL1);
> +
> +#ifdef CONFIG_ARM_64
> + c->mvfr.bits[0] = READ_SYSREG64(MVFR0_EL1);
> + c->mvfr.bits[1] = READ_SYSREG64(MVFR1_EL1);
> + c->mvfr.bits[2] = READ_SYSREG64(MVFR2_EL1);
> +#else
> + c->mvfr.bits[0] = READ_CP32(MVFR0);
> + c->mvfr.bits[1] = READ_CP32(MVFR1);
> +#endif
> }
>
> /*
> diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
> index c60029d38f..5abbeda3fd 100644
> --- a/xen/include/asm-arm/arm64/sysregs.h
> +++ b/xen/include/asm-arm/arm64/sysregs.h
> @@ -57,6 +57,31 @@
> #define ICH_AP1R2_EL2 __AP1Rx_EL2(2)
> #define ICH_AP1R3_EL2 __AP1Rx_EL2(3)
>
> +/*
> + * Define ID coprocessor registers if they are not
> + * already defined by the compiler.
> + *
> + * Values picked from linux kernel
> + */
> +#ifndef ID_AA64MMFR2_EL1
> +#define ID_AA64MMFR2_EL1 S3_0_C0_C7_2
> +#endif
> +#ifndef ID_PFR2_EL1
> +#define ID_PFR2_EL1 S3_0_C0_C3_4
> +#endif
> +#ifndef ID_MMFR5_EL1
> +#define ID_MMFR5_EL1 S3_0_C0_C3_6
> +#endif
> +#ifndef ID_ISAR6_EL1
> +#define ID_ISAR6_EL1 S3_0_C0_C2_7
> +#endif
> +#ifndef ID_AA64ZFR0_EL1
> +#define ID_AA64ZFR0_EL1 S3_0_C0_C4_4
> +#endif
> +#ifndef ID_DFR1_EL1
> +#define ID_DFR1_EL1 S3_0_C0_C3_5
> +#endif
> +
> /* Access to system registers */
>
> #define READ_SYSREG32(name) ((uint32_t)READ_SYSREG64(name))
> diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
> index 8fd344146e..58be898891 100644
> --- a/xen/include/asm-arm/cpregs.h
> +++ b/xen/include/asm-arm/cpregs.h
> @@ -63,6 +63,7 @@
> #define FPSID p10,7,c0,c0,0 /* Floating-Point System ID Register */
> #define FPSCR p10,7,c1,c0,0 /* Floating-Point Status and Control Register */
> #define MVFR0 p10,7,c7,c0,0 /* Media and VFP Feature Register 0 */
> +#define MVFR1 p10,7,c6,c0,0 /* Media and VFP Feature Register 1 */
> #define FPEXC p10,7,c8,c0,0 /* Floating-Point Exception Control Register */
> #define FPINST p10,7,c9,c0,0 /* Floating-Point Instruction Register */
> #define FPINST2 p10,7,c10,c0,0 /* Floating-point Instruction Register 2 */
> @@ -108,18 +109,23 @@
> #define MPIDR p15,0,c0,c0,5 /* Multiprocessor Affinity Register */
> #define ID_PFR0 p15,0,c0,c1,0 /* Processor Feature Register 0 */
> #define ID_PFR1 p15,0,c0,c1,1 /* Processor Feature Register 1 */
> +#define ID_PFR2 p15,0,c0,c3,4 /* Processor Feature Register 2 */
> #define ID_DFR0 p15,0,c0,c1,2 /* Debug Feature Register 0 */
> +#define ID_DFR1 p15,0,c0,c3,5 /* Debug Feature Register 1 */
> #define ID_AFR0 p15,0,c0,c1,3 /* Auxiliary Feature Register 0 */
> #define ID_MMFR0 p15,0,c0,c1,4 /* Memory Model Feature Register 0 */
> #define ID_MMFR1 p15,0,c0,c1,5 /* Memory Model Feature Register 1 */
> #define ID_MMFR2 p15,0,c0,c1,6 /* Memory Model Feature Register 2 */
> #define ID_MMFR3 p15,0,c0,c1,7 /* Memory Model Feature Register 3 */
> +#define ID_MMFR4 p15,0,c0,c2,6 /* Memory Model Feature Register 4 */
> +#define ID_MMFR5 p15,0,c0,c3,6 /* Memory Model Feature Register 5 */
> #define ID_ISAR0 p15,0,c0,c2,0 /* ISA Feature Register 0 */
> #define ID_ISAR1 p15,0,c0,c2,1 /* ISA Feature Register 1 */
> #define ID_ISAR2 p15,0,c0,c2,2 /* ISA Feature Register 2 */
> #define ID_ISAR3 p15,0,c0,c2,3 /* ISA Feature Register 3 */
> #define ID_ISAR4 p15,0,c0,c2,4 /* ISA Feature Register 4 */
> #define ID_ISAR5 p15,0,c0,c2,5 /* ISA Feature Register 5 */
> +#define ID_ISAR6 p15,0,c0,c2,7 /* ISA Feature Register 6 */
> #define CCSIDR p15,1,c0,c0,0 /* Cache Size ID Registers */
> #define CLIDR p15,1,c0,c0,1 /* Cache Level ID Register */
> #define CSSELR p15,2,c0,c0,0 /* Cache Size Selection Register */
> @@ -312,18 +318,23 @@
> #define HSTR_EL2 HSTR
> #define ID_AFR0_EL1 ID_AFR0
> #define ID_DFR0_EL1 ID_DFR0
> +#define ID_DFR1_EL1 ID_DFR1
> #define ID_ISAR0_EL1 ID_ISAR0
> #define ID_ISAR1_EL1 ID_ISAR1
> #define ID_ISAR2_EL1 ID_ISAR2
> #define ID_ISAR3_EL1 ID_ISAR3
> #define ID_ISAR4_EL1 ID_ISAR4
> #define ID_ISAR5_EL1 ID_ISAR5
> +#define ID_ISAR6_EL1 ID_ISAR6
> #define ID_MMFR0_EL1 ID_MMFR0
> #define ID_MMFR1_EL1 ID_MMFR1
> #define ID_MMFR2_EL1 ID_MMFR2
> #define ID_MMFR3_EL1 ID_MMFR3
> +#define ID_MMFR4_EL1 ID_MMFR4
> +#define ID_MMFR5_EL1 ID_MMFR5
> #define ID_PFR0_EL1 ID_PFR0
> #define ID_PFR1_EL1 ID_PFR1
> +#define ID_PFR2_EL1 ID_PFR2
> #define IFSR32_EL2 IFSR
> #define MDCR_EL2 HDCR
> #define MIDR_EL1 MIDR
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index c7b5052992..64354c3f19 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -148,6 +148,7 @@ struct cpuinfo_arm {
> union {
> uint64_t bits[2];
> struct {
> + /* PFR0 */
> unsigned long el0:4;
> unsigned long el1:4;
> unsigned long el2:4;
> @@ -155,9 +156,23 @@ struct cpuinfo_arm {
> unsigned long fp:4; /* Floating Point */
> unsigned long simd:4; /* Advanced SIMD */
> unsigned long gic:4; /* GIC support */
> - unsigned long __res0:28;
> + unsigned long ras:4;
> + unsigned long sve:4;
> + unsigned long sel2:4;
> + unsigned long mpam:4;
> + unsigned long amu:4;
> + unsigned long dit:4;
> + unsigned long __res0:4;
> unsigned long csv2:4;
> - unsigned long __res1:4;
> + unsigned long cvs3:4;
> +
> + /* PFR1 */
> + unsigned long bt:4;
> + unsigned long ssbs:4;
> + unsigned long mte:4;
> + unsigned long ras_frac:4;
> + unsigned long mpam_frac:4;
> + unsigned long __res1:44;
> };
> } pfr64;
>
> @@ -170,7 +185,7 @@ struct cpuinfo_arm {
> } aux64;
>
> union {
> - uint64_t bits[2];
> + uint64_t bits[3];
> struct {
> unsigned long pa_range:4;
> unsigned long asid_bits:4;
> @@ -190,6 +205,8 @@ struct cpuinfo_arm {
> unsigned long pan:4;
> unsigned long __res1:8;
> unsigned long __res2:32;
> +
> + unsigned long __res3:64;
> };
> } mm64;
>
> @@ -197,6 +214,10 @@ struct cpuinfo_arm {
> uint64_t bits[2];
> } isa64;
>
> + struct {
> + uint64_t bits[1];
> + } zfr64;
> +
> #endif
>
> /*
> @@ -204,25 +225,38 @@ struct cpuinfo_arm {
> * when running in 32-bit mode.
> */
> union {
> - uint32_t bits[2];
> + uint32_t bits[3];
> struct {
> + /* PFR0 */
> unsigned long arm:4;
> unsigned long thumb:4;
> unsigned long jazelle:4;
> unsigned long thumbee:4;
> - unsigned long __res0:16;
> + unsigned long csv2:4;
> + unsigned long amu:4;
> + unsigned long dit:4;
> + unsigned long ras:4;
>
> + /* PFR1 */
> unsigned long progmodel:4;
> unsigned long security:4;
> unsigned long mprofile:4;
> unsigned long virt:4;
> unsigned long gentimer:4;
> - unsigned long __res1:12;
> + unsigned long sec_frac:4;
> + unsigned long virt_frac:4;
> + unsigned long gic:4;
> +
> + /* PFR2 */
> + unsigned long csv3:4;
> + unsigned long ssbs:4;
> + unsigned long ras_frac:4;
> + unsigned long __res2:20;
> };
> } pfr32;
>
> struct {
> - uint32_t bits[1];
> + uint32_t bits[2];
> } dbg32;
>
> struct {
> @@ -230,12 +264,23 @@ struct cpuinfo_arm {
> } aux32;
>
> struct {
> - uint32_t bits[4];
> + uint32_t bits[6];
> } mm32;
>
> struct {
> - uint32_t bits[6];
> + uint32_t bits[7];
> } isa32;
> +
> +#ifdef CONFIG_ARM_64
> + struct {
> + uint64_t bits[3];
> + } mvfr;
> +#else
> + /* Only MVFR0 and MVFR1 exist on armv7 */
> + struct {
> + uint32_t bits[2];
> + } mvfr;
> +#endif
> };
>
> extern struct cpuinfo_arm boot_cpu_data;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo
2020-12-04 23:52 ` Stefano Stabellini
@ 2020-12-07 17:35 ` Bertrand Marquis
0 siblings, 0 replies; 31+ messages in thread
From: Bertrand Marquis @ 2020-12-07 17:35 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Xen-devel, Julien Grall, Volodymyr Babchuk
Hi Stefano,
> On 4 Dec 2020, at 23:52, Stefano Stabellini <sstabellini@kernel.org> wrote:
>
> There is a typo in the subject line
I will fix that in the v3.
>
>
> On Mon, 30 Nov 2020, Bertrand Marquis wrote:
>> Add definition and entries in cpuinfo for ID registers introduced in
>> newer Arm Architecture reference manual:
>> - ID_PFR2: processor feature register 2
>> - ID_DFR1: debug feature register 1
>> - ID_MMFR4 and ID_MMFR5: Memory model feature registers 4 and 5
>> - ID_ISA6: ISA Feature register 6
>> Add more bitfield definitions in PFR fields of cpuinfo.
>> Add MVFR2 register definition for aarch32.
>> Add mvfr values in cpuinfo.
>> Add some registers definition for arm64 in sysregs as some are not
>> always know by compilers.
>> Initialize the new values added in cpuinfo in identify_cpu during init.
>>
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>
> I realize I am using an old compiler but I am getting a build error:
>
> /local/repos/gcc-linaro-5.3.1-2016.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -MMD -MP -MF ./.cpufeature.o.d -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include /local/repos/xen-upstream/xen/include/xen/config.h -Wa,--strip-local-absolute -g -mcpu=generic -mgeneral-regs-only -I/local/repos/xen-upstream/xen/include -fno-stack-protector -fno-exceptions -fno-asynchronous-unwind-tables -Wnested-externs '-D__OBJECT_FILE__="cpufeature.o"' -c cpufeature.c -o cpufeature.o
> {standard input}: Assembler messages:
> {standard input}:634: Error: unknown or missing system register name at operand 2 -- `mrs x1,ID_MMFR4_EL1'
>
> If I remove the line:
>
> c->mm32.bits[4] = READ_SYSREG32(ID_MMFR4_EL1);
I will add MMFR4 definition if it is not defined by the compiler in v3.
Cheers
Bertrand
>
>
> it builds just fine
>
>
>
>> ---
>> Changes in V2:
>> Fix dbg32 table size and add proper initialisation of the second entry
>> of the table by reading ID_DFR1 register.
>> ---
>> xen/arch/arm/cpufeature.c | 17 ++++++++
>> xen/include/asm-arm/arm64/sysregs.h | 25 ++++++++++++
>> xen/include/asm-arm/cpregs.h | 11 +++++
>> xen/include/asm-arm/cpufeature.h | 63 ++++++++++++++++++++++++-----
>> 4 files changed, 107 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
>> index 44126dbf07..204be9b084 100644
>> --- a/xen/arch/arm/cpufeature.c
>> +++ b/xen/arch/arm/cpufeature.c
>> @@ -114,15 +114,20 @@ void identify_cpu(struct cpuinfo_arm *c)
>>
>> c->mm64.bits[0] = READ_SYSREG64(ID_AA64MMFR0_EL1);
>> c->mm64.bits[1] = READ_SYSREG64(ID_AA64MMFR1_EL1);
>> + c->mm64.bits[2] = READ_SYSREG64(ID_AA64MMFR2_EL1);
>>
>> c->isa64.bits[0] = READ_SYSREG64(ID_AA64ISAR0_EL1);
>> c->isa64.bits[1] = READ_SYSREG64(ID_AA64ISAR1_EL1);
>> +
>> + c->zfr64.bits[0] = READ_SYSREG64(ID_AA64ZFR0_EL1);
>> #endif
>>
>> c->pfr32.bits[0] = READ_SYSREG32(ID_PFR0_EL1);
>> c->pfr32.bits[1] = READ_SYSREG32(ID_PFR1_EL1);
>> + c->pfr32.bits[2] = READ_SYSREG32(ID_PFR2_EL1);
>>
>> c->dbg32.bits[0] = READ_SYSREG32(ID_DFR0_EL1);
>> + c->dbg32.bits[1] = READ_SYSREG32(ID_DFR1_EL1);
>>
>> c->aux32.bits[0] = READ_SYSREG32(ID_AFR0_EL1);
>>
>> @@ -130,6 +135,8 @@ void identify_cpu(struct cpuinfo_arm *c)
>> c->mm32.bits[1] = READ_SYSREG32(ID_MMFR1_EL1);
>> c->mm32.bits[2] = READ_SYSREG32(ID_MMFR2_EL1);
>> c->mm32.bits[3] = READ_SYSREG32(ID_MMFR3_EL1);
>> + c->mm32.bits[4] = READ_SYSREG32(ID_MMFR4_EL1);
>> + c->mm32.bits[5] = READ_SYSREG32(ID_MMFR5_EL1);
>>
>> c->isa32.bits[0] = READ_SYSREG32(ID_ISAR0_EL1);
>> c->isa32.bits[1] = READ_SYSREG32(ID_ISAR1_EL1);
>> @@ -137,6 +144,16 @@ void identify_cpu(struct cpuinfo_arm *c)
>> c->isa32.bits[3] = READ_SYSREG32(ID_ISAR3_EL1);
>> c->isa32.bits[4] = READ_SYSREG32(ID_ISAR4_EL1);
>> c->isa32.bits[5] = READ_SYSREG32(ID_ISAR5_EL1);
>> + c->isa32.bits[6] = READ_SYSREG32(ID_ISAR6_EL1);
>> +
>> +#ifdef CONFIG_ARM_64
>> + c->mvfr.bits[0] = READ_SYSREG64(MVFR0_EL1);
>> + c->mvfr.bits[1] = READ_SYSREG64(MVFR1_EL1);
>> + c->mvfr.bits[2] = READ_SYSREG64(MVFR2_EL1);
>> +#else
>> + c->mvfr.bits[0] = READ_CP32(MVFR0);
>> + c->mvfr.bits[1] = READ_CP32(MVFR1);
>> +#endif
>> }
>>
>> /*
>> diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
>> index c60029d38f..5abbeda3fd 100644
>> --- a/xen/include/asm-arm/arm64/sysregs.h
>> +++ b/xen/include/asm-arm/arm64/sysregs.h
>> @@ -57,6 +57,31 @@
>> #define ICH_AP1R2_EL2 __AP1Rx_EL2(2)
>> #define ICH_AP1R3_EL2 __AP1Rx_EL2(3)
>>
>> +/*
>> + * Define ID coprocessor registers if they are not
>> + * already defined by the compiler.
>> + *
>> + * Values picked from linux kernel
>> + */
>> +#ifndef ID_AA64MMFR2_EL1
>> +#define ID_AA64MMFR2_EL1 S3_0_C0_C7_2
>> +#endif
>> +#ifndef ID_PFR2_EL1
>> +#define ID_PFR2_EL1 S3_0_C0_C3_4
>> +#endif
>> +#ifndef ID_MMFR5_EL1
>> +#define ID_MMFR5_EL1 S3_0_C0_C3_6
>> +#endif
>> +#ifndef ID_ISAR6_EL1
>> +#define ID_ISAR6_EL1 S3_0_C0_C2_7
>> +#endif
>> +#ifndef ID_AA64ZFR0_EL1
>> +#define ID_AA64ZFR0_EL1 S3_0_C0_C4_4
>> +#endif
>> +#ifndef ID_DFR1_EL1
>> +#define ID_DFR1_EL1 S3_0_C0_C3_5
>> +#endif
>> +
>> /* Access to system registers */
>>
>> #define READ_SYSREG32(name) ((uint32_t)READ_SYSREG64(name))
>> diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
>> index 8fd344146e..58be898891 100644
>> --- a/xen/include/asm-arm/cpregs.h
>> +++ b/xen/include/asm-arm/cpregs.h
>> @@ -63,6 +63,7 @@
>> #define FPSID p10,7,c0,c0,0 /* Floating-Point System ID Register */
>> #define FPSCR p10,7,c1,c0,0 /* Floating-Point Status and Control Register */
>> #define MVFR0 p10,7,c7,c0,0 /* Media and VFP Feature Register 0 */
>> +#define MVFR1 p10,7,c6,c0,0 /* Media and VFP Feature Register 1 */
>> #define FPEXC p10,7,c8,c0,0 /* Floating-Point Exception Control Register */
>> #define FPINST p10,7,c9,c0,0 /* Floating-Point Instruction Register */
>> #define FPINST2 p10,7,c10,c0,0 /* Floating-point Instruction Register 2 */
>> @@ -108,18 +109,23 @@
>> #define MPIDR p15,0,c0,c0,5 /* Multiprocessor Affinity Register */
>> #define ID_PFR0 p15,0,c0,c1,0 /* Processor Feature Register 0 */
>> #define ID_PFR1 p15,0,c0,c1,1 /* Processor Feature Register 1 */
>> +#define ID_PFR2 p15,0,c0,c3,4 /* Processor Feature Register 2 */
>> #define ID_DFR0 p15,0,c0,c1,2 /* Debug Feature Register 0 */
>> +#define ID_DFR1 p15,0,c0,c3,5 /* Debug Feature Register 1 */
>> #define ID_AFR0 p15,0,c0,c1,3 /* Auxiliary Feature Register 0 */
>> #define ID_MMFR0 p15,0,c0,c1,4 /* Memory Model Feature Register 0 */
>> #define ID_MMFR1 p15,0,c0,c1,5 /* Memory Model Feature Register 1 */
>> #define ID_MMFR2 p15,0,c0,c1,6 /* Memory Model Feature Register 2 */
>> #define ID_MMFR3 p15,0,c0,c1,7 /* Memory Model Feature Register 3 */
>> +#define ID_MMFR4 p15,0,c0,c2,6 /* Memory Model Feature Register 4 */
>> +#define ID_MMFR5 p15,0,c0,c3,6 /* Memory Model Feature Register 5 */
>> #define ID_ISAR0 p15,0,c0,c2,0 /* ISA Feature Register 0 */
>> #define ID_ISAR1 p15,0,c0,c2,1 /* ISA Feature Register 1 */
>> #define ID_ISAR2 p15,0,c0,c2,2 /* ISA Feature Register 2 */
>> #define ID_ISAR3 p15,0,c0,c2,3 /* ISA Feature Register 3 */
>> #define ID_ISAR4 p15,0,c0,c2,4 /* ISA Feature Register 4 */
>> #define ID_ISAR5 p15,0,c0,c2,5 /* ISA Feature Register 5 */
>> +#define ID_ISAR6 p15,0,c0,c2,7 /* ISA Feature Register 6 */
>> #define CCSIDR p15,1,c0,c0,0 /* Cache Size ID Registers */
>> #define CLIDR p15,1,c0,c0,1 /* Cache Level ID Register */
>> #define CSSELR p15,2,c0,c0,0 /* Cache Size Selection Register */
>> @@ -312,18 +318,23 @@
>> #define HSTR_EL2 HSTR
>> #define ID_AFR0_EL1 ID_AFR0
>> #define ID_DFR0_EL1 ID_DFR0
>> +#define ID_DFR1_EL1 ID_DFR1
>> #define ID_ISAR0_EL1 ID_ISAR0
>> #define ID_ISAR1_EL1 ID_ISAR1
>> #define ID_ISAR2_EL1 ID_ISAR2
>> #define ID_ISAR3_EL1 ID_ISAR3
>> #define ID_ISAR4_EL1 ID_ISAR4
>> #define ID_ISAR5_EL1 ID_ISAR5
>> +#define ID_ISAR6_EL1 ID_ISAR6
>> #define ID_MMFR0_EL1 ID_MMFR0
>> #define ID_MMFR1_EL1 ID_MMFR1
>> #define ID_MMFR2_EL1 ID_MMFR2
>> #define ID_MMFR3_EL1 ID_MMFR3
>> +#define ID_MMFR4_EL1 ID_MMFR4
>> +#define ID_MMFR5_EL1 ID_MMFR5
>> #define ID_PFR0_EL1 ID_PFR0
>> #define ID_PFR1_EL1 ID_PFR1
>> +#define ID_PFR2_EL1 ID_PFR2
>> #define IFSR32_EL2 IFSR
>> #define MDCR_EL2 HDCR
>> #define MIDR_EL1 MIDR
>> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
>> index c7b5052992..64354c3f19 100644
>> --- a/xen/include/asm-arm/cpufeature.h
>> +++ b/xen/include/asm-arm/cpufeature.h
>> @@ -148,6 +148,7 @@ struct cpuinfo_arm {
>> union {
>> uint64_t bits[2];
>> struct {
>> + /* PFR0 */
>> unsigned long el0:4;
>> unsigned long el1:4;
>> unsigned long el2:4;
>> @@ -155,9 +156,23 @@ struct cpuinfo_arm {
>> unsigned long fp:4; /* Floating Point */
>> unsigned long simd:4; /* Advanced SIMD */
>> unsigned long gic:4; /* GIC support */
>> - unsigned long __res0:28;
>> + unsigned long ras:4;
>> + unsigned long sve:4;
>> + unsigned long sel2:4;
>> + unsigned long mpam:4;
>> + unsigned long amu:4;
>> + unsigned long dit:4;
>> + unsigned long __res0:4;
>> unsigned long csv2:4;
>> - unsigned long __res1:4;
>> + unsigned long cvs3:4;
>> +
>> + /* PFR1 */
>> + unsigned long bt:4;
>> + unsigned long ssbs:4;
>> + unsigned long mte:4;
>> + unsigned long ras_frac:4;
>> + unsigned long mpam_frac:4;
>> + unsigned long __res1:44;
>> };
>> } pfr64;
>>
>> @@ -170,7 +185,7 @@ struct cpuinfo_arm {
>> } aux64;
>>
>> union {
>> - uint64_t bits[2];
>> + uint64_t bits[3];
>> struct {
>> unsigned long pa_range:4;
>> unsigned long asid_bits:4;
>> @@ -190,6 +205,8 @@ struct cpuinfo_arm {
>> unsigned long pan:4;
>> unsigned long __res1:8;
>> unsigned long __res2:32;
>> +
>> + unsigned long __res3:64;
>> };
>> } mm64;
>>
>> @@ -197,6 +214,10 @@ struct cpuinfo_arm {
>> uint64_t bits[2];
>> } isa64;
>>
>> + struct {
>> + uint64_t bits[1];
>> + } zfr64;
>> +
>> #endif
>>
>> /*
>> @@ -204,25 +225,38 @@ struct cpuinfo_arm {
>> * when running in 32-bit mode.
>> */
>> union {
>> - uint32_t bits[2];
>> + uint32_t bits[3];
>> struct {
>> + /* PFR0 */
>> unsigned long arm:4;
>> unsigned long thumb:4;
>> unsigned long jazelle:4;
>> unsigned long thumbee:4;
>> - unsigned long __res0:16;
>> + unsigned long csv2:4;
>> + unsigned long amu:4;
>> + unsigned long dit:4;
>> + unsigned long ras:4;
>>
>> + /* PFR1 */
>> unsigned long progmodel:4;
>> unsigned long security:4;
>> unsigned long mprofile:4;
>> unsigned long virt:4;
>> unsigned long gentimer:4;
>> - unsigned long __res1:12;
>> + unsigned long sec_frac:4;
>> + unsigned long virt_frac:4;
>> + unsigned long gic:4;
>> +
>> + /* PFR2 */
>> + unsigned long csv3:4;
>> + unsigned long ssbs:4;
>> + unsigned long ras_frac:4;
>> + unsigned long __res2:20;
>> };
>> } pfr32;
>>
>> struct {
>> - uint32_t bits[1];
>> + uint32_t bits[2];
>> } dbg32;
>>
>> struct {
>> @@ -230,12 +264,23 @@ struct cpuinfo_arm {
>> } aux32;
>>
>> struct {
>> - uint32_t bits[4];
>> + uint32_t bits[6];
>> } mm32;
>>
>> struct {
>> - uint32_t bits[6];
>> + uint32_t bits[7];
>> } isa32;
>> +
>> +#ifdef CONFIG_ARM_64
>> + struct {
>> + uint64_t bits[3];
>> + } mvfr;
>> +#else
>> + /* Only MVFR0 and MVFR1 exist on armv7 */
>> + struct {
>> + uint32_t bits[2];
>> + } mvfr;
>> +#endif
>> };
>>
>> extern struct cpuinfo_arm boot_cpu_data;
>> --
>> 2.17.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 2/7] xen/arm: Add arm64 ID registers definitions
2020-11-30 14:21 [PATCH v2 0/7] xen/arm: Emulate ID registers Bertrand Marquis
2020-11-30 14:21 ` [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo Bertrand Marquis
@ 2020-11-30 14:21 ` Bertrand Marquis
2020-11-30 20:08 ` Volodymyr Babchuk
2020-11-30 14:21 ` [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest Bertrand Marquis
` (4 subsequent siblings)
6 siblings, 1 reply; 31+ messages in thread
From: Bertrand Marquis @ 2020-11-30 14:21 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk
Add coprocessor registers definitions for all ID registers trapped
through the TID3 bit of HSR.
Those are the one that will be emulated in Xen to only publish to guests
the features that are supported by Xen and that are accessible to
guests.
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: rebase
---
xen/include/asm-arm/arm64/hsr.h | 37 +++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/xen/include/asm-arm/arm64/hsr.h b/xen/include/asm-arm/arm64/hsr.h
index ca931dd2fe..e691d41c17 100644
--- a/xen/include/asm-arm/arm64/hsr.h
+++ b/xen/include/asm-arm/arm64/hsr.h
@@ -110,6 +110,43 @@
#define HSR_SYSREG_CNTP_CTL_EL0 HSR_SYSREG(3,3,c14,c2,1)
#define HSR_SYSREG_CNTP_CVAL_EL0 HSR_SYSREG(3,3,c14,c2,2)
+/* Those registers are used when HCR_EL2.TID3 is set */
+#define HSR_SYSREG_ID_PFR0_EL1 HSR_SYSREG(3,0,c0,c1,0)
+#define HSR_SYSREG_ID_PFR1_EL1 HSR_SYSREG(3,0,c0,c1,1)
+#define HSR_SYSREG_ID_PFR2_EL1 HSR_SYSREG(3,0,c0,c3,4)
+#define HSR_SYSREG_ID_DFR0_EL1 HSR_SYSREG(3,0,c0,c1,2)
+#define HSR_SYSREG_ID_DFR1_EL1 HSR_SYSREG(3,0,c0,c3,5)
+#define HSR_SYSREG_ID_AFR0_EL1 HSR_SYSREG(3,0,c0,c1,3)
+#define HSR_SYSREG_ID_MMFR0_EL1 HSR_SYSREG(3,0,c0,c1,4)
+#define HSR_SYSREG_ID_MMFR1_EL1 HSR_SYSREG(3,0,c0,c1,5)
+#define HSR_SYSREG_ID_MMFR2_EL1 HSR_SYSREG(3,0,c0,c1,6)
+#define HSR_SYSREG_ID_MMFR3_EL1 HSR_SYSREG(3,0,c0,c1,7)
+#define HSR_SYSREG_ID_MMFR4_EL1 HSR_SYSREG(3,0,c0,c2,6)
+#define HSR_SYSREG_ID_MMFR5_EL1 HSR_SYSREG(3,0,c0,c3,6)
+#define HSR_SYSREG_ID_ISAR0_EL1 HSR_SYSREG(3,0,c0,c2,0)
+#define HSR_SYSREG_ID_ISAR1_EL1 HSR_SYSREG(3,0,c0,c2,1)
+#define HSR_SYSREG_ID_ISAR2_EL1 HSR_SYSREG(3,0,c0,c2,2)
+#define HSR_SYSREG_ID_ISAR3_EL1 HSR_SYSREG(3,0,c0,c2,3)
+#define HSR_SYSREG_ID_ISAR4_EL1 HSR_SYSREG(3,0,c0,c2,4)
+#define HSR_SYSREG_ID_ISAR5_EL1 HSR_SYSREG(3,0,c0,c2,5)
+#define HSR_SYSREG_ID_ISAR6_EL1 HSR_SYSREG(3,0,c0,c2,7)
+#define HSR_SYSREG_MVFR0_EL1 HSR_SYSREG(3,0,c0,c3,0)
+#define HSR_SYSREG_MVFR1_EL1 HSR_SYSREG(3,0,c0,c3,1)
+#define HSR_SYSREG_MVFR2_EL1 HSR_SYSREG(3,0,c0,c3,2)
+
+#define HSR_SYSREG_ID_AA64PFR0_EL1 HSR_SYSREG(3,0,c0,c4,0)
+#define HSR_SYSREG_ID_AA64PFR1_EL1 HSR_SYSREG(3,0,c0,c4,1)
+#define HSR_SYSREG_ID_AA64DFR0_EL1 HSR_SYSREG(3,0,c0,c5,0)
+#define HSR_SYSREG_ID_AA64DFR1_EL1 HSR_SYSREG(3,0,c0,c5,1)
+#define HSR_SYSREG_ID_AA64ISAR0_EL1 HSR_SYSREG(3,0,c0,c6,0)
+#define HSR_SYSREG_ID_AA64ISAR1_EL1 HSR_SYSREG(3,0,c0,c6,1)
+#define HSR_SYSREG_ID_AA64MMFR0_EL1 HSR_SYSREG(3,0,c0,c7,0)
+#define HSR_SYSREG_ID_AA64MMFR1_EL1 HSR_SYSREG(3,0,c0,c7,1)
+#define HSR_SYSREG_ID_AA64MMFR2_EL1 HSR_SYSREG(3,0,c0,c7,2)
+#define HSR_SYSREG_ID_AA64AFR0_EL1 HSR_SYSREG(3,0,c0,c5,4)
+#define HSR_SYSREG_ID_AA64AFR1_EL1 HSR_SYSREG(3,0,c0,c5,5)
+#define HSR_SYSREG_ID_AA64ZFR0_EL1 HSR_SYSREG(3,0,c0,c4,4)
+
#endif /* __ASM_ARM_ARM64_HSR_H */
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 2/7] xen/arm: Add arm64 ID registers definitions
2020-11-30 14:21 ` [PATCH v2 2/7] xen/arm: Add arm64 ID registers definitions Bertrand Marquis
@ 2020-11-30 20:08 ` Volodymyr Babchuk
2020-12-04 23:54 ` Stefano Stabellini
0 siblings, 1 reply; 31+ messages in thread
From: Volodymyr Babchuk @ 2020-11-30 20:08 UTC (permalink / raw)
To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Bertrand Marquis writes:
> Add coprocessor registers definitions for all ID registers trapped
> through the TID3 bit of HSR.
> Those are the one that will be emulated in Xen to only publish to guests
> the features that are supported by Xen and that are accessible to
> guests.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> ---
> Changes in V2: rebase
> ---
> xen/include/asm-arm/arm64/hsr.h | 37 +++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/xen/include/asm-arm/arm64/hsr.h b/xen/include/asm-arm/arm64/hsr.h
> index ca931dd2fe..e691d41c17 100644
> --- a/xen/include/asm-arm/arm64/hsr.h
> +++ b/xen/include/asm-arm/arm64/hsr.h
> @@ -110,6 +110,43 @@
> #define HSR_SYSREG_CNTP_CTL_EL0 HSR_SYSREG(3,3,c14,c2,1)
> #define HSR_SYSREG_CNTP_CVAL_EL0 HSR_SYSREG(3,3,c14,c2,2)
>
> +/* Those registers are used when HCR_EL2.TID3 is set */
> +#define HSR_SYSREG_ID_PFR0_EL1 HSR_SYSREG(3,0,c0,c1,0)
> +#define HSR_SYSREG_ID_PFR1_EL1 HSR_SYSREG(3,0,c0,c1,1)
> +#define HSR_SYSREG_ID_PFR2_EL1 HSR_SYSREG(3,0,c0,c3,4)
> +#define HSR_SYSREG_ID_DFR0_EL1 HSR_SYSREG(3,0,c0,c1,2)
> +#define HSR_SYSREG_ID_DFR1_EL1 HSR_SYSREG(3,0,c0,c3,5)
> +#define HSR_SYSREG_ID_AFR0_EL1 HSR_SYSREG(3,0,c0,c1,3)
> +#define HSR_SYSREG_ID_MMFR0_EL1 HSR_SYSREG(3,0,c0,c1,4)
> +#define HSR_SYSREG_ID_MMFR1_EL1 HSR_SYSREG(3,0,c0,c1,5)
> +#define HSR_SYSREG_ID_MMFR2_EL1 HSR_SYSREG(3,0,c0,c1,6)
> +#define HSR_SYSREG_ID_MMFR3_EL1 HSR_SYSREG(3,0,c0,c1,7)
> +#define HSR_SYSREG_ID_MMFR4_EL1 HSR_SYSREG(3,0,c0,c2,6)
> +#define HSR_SYSREG_ID_MMFR5_EL1 HSR_SYSREG(3,0,c0,c3,6)
> +#define HSR_SYSREG_ID_ISAR0_EL1 HSR_SYSREG(3,0,c0,c2,0)
> +#define HSR_SYSREG_ID_ISAR1_EL1 HSR_SYSREG(3,0,c0,c2,1)
> +#define HSR_SYSREG_ID_ISAR2_EL1 HSR_SYSREG(3,0,c0,c2,2)
> +#define HSR_SYSREG_ID_ISAR3_EL1 HSR_SYSREG(3,0,c0,c2,3)
> +#define HSR_SYSREG_ID_ISAR4_EL1 HSR_SYSREG(3,0,c0,c2,4)
> +#define HSR_SYSREG_ID_ISAR5_EL1 HSR_SYSREG(3,0,c0,c2,5)
> +#define HSR_SYSREG_ID_ISAR6_EL1 HSR_SYSREG(3,0,c0,c2,7)
> +#define HSR_SYSREG_MVFR0_EL1 HSR_SYSREG(3,0,c0,c3,0)
> +#define HSR_SYSREG_MVFR1_EL1 HSR_SYSREG(3,0,c0,c3,1)
> +#define HSR_SYSREG_MVFR2_EL1 HSR_SYSREG(3,0,c0,c3,2)
> +
> +#define HSR_SYSREG_ID_AA64PFR0_EL1 HSR_SYSREG(3,0,c0,c4,0)
> +#define HSR_SYSREG_ID_AA64PFR1_EL1 HSR_SYSREG(3,0,c0,c4,1)
> +#define HSR_SYSREG_ID_AA64DFR0_EL1 HSR_SYSREG(3,0,c0,c5,0)
> +#define HSR_SYSREG_ID_AA64DFR1_EL1 HSR_SYSREG(3,0,c0,c5,1)
> +#define HSR_SYSREG_ID_AA64ISAR0_EL1 HSR_SYSREG(3,0,c0,c6,0)
> +#define HSR_SYSREG_ID_AA64ISAR1_EL1 HSR_SYSREG(3,0,c0,c6,1)
> +#define HSR_SYSREG_ID_AA64MMFR0_EL1 HSR_SYSREG(3,0,c0,c7,0)
> +#define HSR_SYSREG_ID_AA64MMFR1_EL1 HSR_SYSREG(3,0,c0,c7,1)
> +#define HSR_SYSREG_ID_AA64MMFR2_EL1 HSR_SYSREG(3,0,c0,c7,2)
> +#define HSR_SYSREG_ID_AA64AFR0_EL1 HSR_SYSREG(3,0,c0,c5,4)
> +#define HSR_SYSREG_ID_AA64AFR1_EL1 HSR_SYSREG(3,0,c0,c5,5)
> +#define HSR_SYSREG_ID_AA64ZFR0_EL1 HSR_SYSREG(3,0,c0,c4,4)
> +
> #endif /* __ASM_ARM_ARM64_HSR_H */
>
> /*
--
Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 2/7] xen/arm: Add arm64 ID registers definitions
2020-11-30 20:08 ` Volodymyr Babchuk
@ 2020-12-04 23:54 ` Stefano Stabellini
0 siblings, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2020-12-04 23:54 UTC (permalink / raw)
To: Volodymyr Babchuk
Cc: Bertrand Marquis, xen-devel, Stefano Stabellini, Julien Grall
On Mon, 30 Nov 2020, Volodymyr Babchuk wrote:
> Bertrand Marquis writes:
>
> > Add coprocessor registers definitions for all ID registers trapped
> > through the TID3 bit of HSR.
> > Those are the one that will be emulated in Xen to only publish to guests
> > the features that are supported by Xen and that are accessible to
> > guests.
> >
> > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> > Changes in V2: rebase
> > ---
> > xen/include/asm-arm/arm64/hsr.h | 37 +++++++++++++++++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> >
> > diff --git a/xen/include/asm-arm/arm64/hsr.h b/xen/include/asm-arm/arm64/hsr.h
> > index ca931dd2fe..e691d41c17 100644
> > --- a/xen/include/asm-arm/arm64/hsr.h
> > +++ b/xen/include/asm-arm/arm64/hsr.h
> > @@ -110,6 +110,43 @@
> > #define HSR_SYSREG_CNTP_CTL_EL0 HSR_SYSREG(3,3,c14,c2,1)
> > #define HSR_SYSREG_CNTP_CVAL_EL0 HSR_SYSREG(3,3,c14,c2,2)
> >
> > +/* Those registers are used when HCR_EL2.TID3 is set */
> > +#define HSR_SYSREG_ID_PFR0_EL1 HSR_SYSREG(3,0,c0,c1,0)
> > +#define HSR_SYSREG_ID_PFR1_EL1 HSR_SYSREG(3,0,c0,c1,1)
> > +#define HSR_SYSREG_ID_PFR2_EL1 HSR_SYSREG(3,0,c0,c3,4)
> > +#define HSR_SYSREG_ID_DFR0_EL1 HSR_SYSREG(3,0,c0,c1,2)
> > +#define HSR_SYSREG_ID_DFR1_EL1 HSR_SYSREG(3,0,c0,c3,5)
> > +#define HSR_SYSREG_ID_AFR0_EL1 HSR_SYSREG(3,0,c0,c1,3)
> > +#define HSR_SYSREG_ID_MMFR0_EL1 HSR_SYSREG(3,0,c0,c1,4)
> > +#define HSR_SYSREG_ID_MMFR1_EL1 HSR_SYSREG(3,0,c0,c1,5)
> > +#define HSR_SYSREG_ID_MMFR2_EL1 HSR_SYSREG(3,0,c0,c1,6)
> > +#define HSR_SYSREG_ID_MMFR3_EL1 HSR_SYSREG(3,0,c0,c1,7)
> > +#define HSR_SYSREG_ID_MMFR4_EL1 HSR_SYSREG(3,0,c0,c2,6)
> > +#define HSR_SYSREG_ID_MMFR5_EL1 HSR_SYSREG(3,0,c0,c3,6)
> > +#define HSR_SYSREG_ID_ISAR0_EL1 HSR_SYSREG(3,0,c0,c2,0)
> > +#define HSR_SYSREG_ID_ISAR1_EL1 HSR_SYSREG(3,0,c0,c2,1)
> > +#define HSR_SYSREG_ID_ISAR2_EL1 HSR_SYSREG(3,0,c0,c2,2)
> > +#define HSR_SYSREG_ID_ISAR3_EL1 HSR_SYSREG(3,0,c0,c2,3)
> > +#define HSR_SYSREG_ID_ISAR4_EL1 HSR_SYSREG(3,0,c0,c2,4)
> > +#define HSR_SYSREG_ID_ISAR5_EL1 HSR_SYSREG(3,0,c0,c2,5)
> > +#define HSR_SYSREG_ID_ISAR6_EL1 HSR_SYSREG(3,0,c0,c2,7)
> > +#define HSR_SYSREG_MVFR0_EL1 HSR_SYSREG(3,0,c0,c3,0)
> > +#define HSR_SYSREG_MVFR1_EL1 HSR_SYSREG(3,0,c0,c3,1)
> > +#define HSR_SYSREG_MVFR2_EL1 HSR_SYSREG(3,0,c0,c3,2)
> > +
> > +#define HSR_SYSREG_ID_AA64PFR0_EL1 HSR_SYSREG(3,0,c0,c4,0)
> > +#define HSR_SYSREG_ID_AA64PFR1_EL1 HSR_SYSREG(3,0,c0,c4,1)
> > +#define HSR_SYSREG_ID_AA64DFR0_EL1 HSR_SYSREG(3,0,c0,c5,0)
> > +#define HSR_SYSREG_ID_AA64DFR1_EL1 HSR_SYSREG(3,0,c0,c5,1)
> > +#define HSR_SYSREG_ID_AA64ISAR0_EL1 HSR_SYSREG(3,0,c0,c6,0)
> > +#define HSR_SYSREG_ID_AA64ISAR1_EL1 HSR_SYSREG(3,0,c0,c6,1)
> > +#define HSR_SYSREG_ID_AA64MMFR0_EL1 HSR_SYSREG(3,0,c0,c7,0)
> > +#define HSR_SYSREG_ID_AA64MMFR1_EL1 HSR_SYSREG(3,0,c0,c7,1)
> > +#define HSR_SYSREG_ID_AA64MMFR2_EL1 HSR_SYSREG(3,0,c0,c7,2)
> > +#define HSR_SYSREG_ID_AA64AFR0_EL1 HSR_SYSREG(3,0,c0,c5,4)
> > +#define HSR_SYSREG_ID_AA64AFR1_EL1 HSR_SYSREG(3,0,c0,c5,5)
> > +#define HSR_SYSREG_ID_AA64ZFR0_EL1 HSR_SYSREG(3,0,c0,c4,4)
> > +
> > #endif /* __ASM_ARM_ARM64_HSR_H */
> >
> > /*
>
>
> --
> Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest
2020-11-30 14:21 [PATCH v2 0/7] xen/arm: Emulate ID registers Bertrand Marquis
2020-11-30 14:21 ` [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo Bertrand Marquis
2020-11-30 14:21 ` [PATCH v2 2/7] xen/arm: Add arm64 ID registers definitions Bertrand Marquis
@ 2020-11-30 14:21 ` Bertrand Marquis
2020-11-30 20:15 ` Volodymyr Babchuk
2020-12-04 23:57 ` Stefano Stabellini
2020-11-30 14:21 ` [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64 Bertrand Marquis
` (3 subsequent siblings)
6 siblings, 2 replies; 31+ messages in thread
From: Bertrand Marquis @ 2020-11-30 14:21 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk
Create a cpuinfo structure for guest and mask into it the features that
we do not support in Xen or that we do not want to publish to guests.
Modify some values in the cpuinfo structure for guests to mask some
features which we do not want to allow to guests (like AMU) or we do not
support (like SVE).
The code is trying to group together registers modifications for the
same feature to be able in the long term to easily enable/disable a
feature depending on user parameters or add other registers modification
in the same place (like enabling/disabling HCR bits).
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: rebase
---
xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++
xen/include/asm-arm/cpufeature.h | 2 ++
2 files changed, 53 insertions(+)
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 204be9b084..309941ff37 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -24,6 +24,8 @@
DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
+struct cpuinfo_arm __read_mostly guest_cpuinfo;
+
void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
const char *info)
{
@@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c)
#endif
}
+/*
+ * This function is creating a cpuinfo structure with values modified to mask
+ * all cpu features that should not be published to guest.
+ * The created structure is then used to provide ID registers values to guests.
+ */
+static int __init create_guest_cpuinfo(void)
+{
+ /*
+ * TODO: The code is currently using only the features detected on the boot
+ * core. In the long term we should try to compute values containing only
+ * features supported by all cores.
+ */
+ identify_cpu(&guest_cpuinfo);
+
+#ifdef CONFIG_ARM_64
+ /* Disable MPAM as xen does not support it */
+ guest_cpuinfo.pfr64.mpam = 0;
+ guest_cpuinfo.pfr64.mpam_frac = 0;
+
+ /* Disable SVE as Xen does not support it */
+ guest_cpuinfo.pfr64.sve = 0;
+ guest_cpuinfo.zfr64.bits[0] = 0;
+
+ /* Disable MTE as Xen does not support it */
+ guest_cpuinfo.pfr64.mte = 0;
+#endif
+
+ /* Disable AMU */
+#ifdef CONFIG_ARM_64
+ guest_cpuinfo.pfr64.amu = 0;
+#endif
+ guest_cpuinfo.pfr32.amu = 0;
+
+ /* Disable RAS as Xen does not support it */
+#ifdef CONFIG_ARM_64
+ guest_cpuinfo.pfr64.ras = 0;
+ guest_cpuinfo.pfr64.ras_frac = 0;
+#endif
+ guest_cpuinfo.pfr32.ras = 0;
+ guest_cpuinfo.pfr32.ras_frac = 0;
+
+ return 0;
+}
+/*
+ * This function needs to be run after all smp are started to have
+ * cpuinfo structures for all cores.
+ */
+__initcall(create_guest_cpuinfo);
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 64354c3f19..0ab6dd42a0 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *);
extern struct cpuinfo_arm cpu_data[];
#define current_cpu_data cpu_data[smp_processor_id()]
+extern struct cpuinfo_arm guest_cpuinfo;
+
#endif /* __ASSEMBLY__ */
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest
2020-11-30 14:21 ` [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest Bertrand Marquis
@ 2020-11-30 20:15 ` Volodymyr Babchuk
2020-12-01 11:41 ` Bertrand Marquis
2020-12-04 23:57 ` Stefano Stabellini
1 sibling, 1 reply; 31+ messages in thread
From: Volodymyr Babchuk @ 2020-11-30 20:15 UTC (permalink / raw)
To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Bertrand Marquis writes:
> Create a cpuinfo structure for guest and mask into it the features that
> we do not support in Xen or that we do not want to publish to guests.
>
> Modify some values in the cpuinfo structure for guests to mask some
> features which we do not want to allow to guests (like AMU) or we do not
> support (like SVE).
>
> The code is trying to group together registers modifications for the
> same feature to be able in the long term to easily enable/disable a
> feature depending on user parameters or add other registers modification
> in the same place (like enabling/disabling HCR bits).
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in V2: rebase
> ---
> xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++
> xen/include/asm-arm/cpufeature.h | 2 ++
> 2 files changed, 53 insertions(+)
>
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 204be9b084..309941ff37 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -24,6 +24,8 @@
>
> DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>
> +struct cpuinfo_arm __read_mostly guest_cpuinfo;
> +
> void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
> const char *info)
> {
> @@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c)
> #endif
> }
>
> +/*
> + * This function is creating a cpuinfo structure with values modified to mask
> + * all cpu features that should not be published to guest.
> + * The created structure is then used to provide ID registers values to guests.
> + */
> +static int __init create_guest_cpuinfo(void)
> +{
> + /*
> + * TODO: The code is currently using only the features detected on the boot
> + * core. In the long term we should try to compute values containing only
> + * features supported by all cores.
> + */
> + identify_cpu(&guest_cpuinfo);
> +
> +#ifdef CONFIG_ARM_64
> + /* Disable MPAM as xen does not support it */
> + guest_cpuinfo.pfr64.mpam = 0;
> + guest_cpuinfo.pfr64.mpam_frac = 0;
> +
> + /* Disable SVE as Xen does not support it */
> + guest_cpuinfo.pfr64.sve = 0;
> + guest_cpuinfo.zfr64.bits[0] = 0;
> +
> + /* Disable MTE as Xen does not support it */
> + guest_cpuinfo.pfr64.mte = 0;
> +#endif
> +
> + /* Disable AMU */
> +#ifdef CONFIG_ARM_64
> + guest_cpuinfo.pfr64.amu = 0;
> +#endif
> + guest_cpuinfo.pfr32.amu = 0;
> +
> + /* Disable RAS as Xen does not support it */
> +#ifdef CONFIG_ARM_64
> + guest_cpuinfo.pfr64.ras = 0;
> + guest_cpuinfo.pfr64.ras_frac = 0;
> +#endif
> + guest_cpuinfo.pfr32.ras = 0;
> + guest_cpuinfo.pfr32.ras_frac = 0;
> +
> + return 0;
> +}
> +/*
> + * This function needs to be run after all smp are started to have
> + * cpuinfo structures for all cores.
> + */
This comment contradicts with TODO at the beginning of
create_guest_cpuinfo().
> +__initcall(create_guest_cpuinfo);
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 64354c3f19..0ab6dd42a0 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *);
> extern struct cpuinfo_arm cpu_data[];
> #define current_cpu_data cpu_data[smp_processor_id()]
>
> +extern struct cpuinfo_arm guest_cpuinfo;
> +
> #endif /* __ASSEMBLY__ */
>
> #endif
--
Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest
2020-11-30 20:15 ` Volodymyr Babchuk
@ 2020-12-01 11:41 ` Bertrand Marquis
0 siblings, 0 replies; 31+ messages in thread
From: Bertrand Marquis @ 2020-12-01 11:41 UTC (permalink / raw)
To: Volodymyr Babchuk; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hi Volodymyr,
> On 30 Nov 2020, at 20:15, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>
>
> Bertrand Marquis writes:
>
>> Create a cpuinfo structure for guest and mask into it the features that
>> we do not support in Xen or that we do not want to publish to guests.
>>
>> Modify some values in the cpuinfo structure for guests to mask some
>> features which we do not want to allow to guests (like AMU) or we do not
>> support (like SVE).
>>
>> The code is trying to group together registers modifications for the
>> same feature to be able in the long term to easily enable/disable a
>> feature depending on user parameters or add other registers modification
>> in the same place (like enabling/disabling HCR bits).
>>
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> Changes in V2: rebase
>> ---
>> xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++
>> xen/include/asm-arm/cpufeature.h | 2 ++
>> 2 files changed, 53 insertions(+)
>>
>> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
>> index 204be9b084..309941ff37 100644
>> --- a/xen/arch/arm/cpufeature.c
>> +++ b/xen/arch/arm/cpufeature.c
>> @@ -24,6 +24,8 @@
>>
>> DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>>
>> +struct cpuinfo_arm __read_mostly guest_cpuinfo;
>> +
>> void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>> const char *info)
>> {
>> @@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c)
>> #endif
>> }
>>
>> +/*
>> + * This function is creating a cpuinfo structure with values modified to mask
>> + * all cpu features that should not be published to guest.
>> + * The created structure is then used to provide ID registers values to guests.
>> + */
>> +static int __init create_guest_cpuinfo(void)
>> +{
>> + /*
>> + * TODO: The code is currently using only the features detected on the boot
>> + * core. In the long term we should try to compute values containing only
>> + * features supported by all cores.
>> + */
>> + identify_cpu(&guest_cpuinfo);
>> +
>> +#ifdef CONFIG_ARM_64
>> + /* Disable MPAM as xen does not support it */
>> + guest_cpuinfo.pfr64.mpam = 0;
>> + guest_cpuinfo.pfr64.mpam_frac = 0;
>> +
>> + /* Disable SVE as Xen does not support it */
>> + guest_cpuinfo.pfr64.sve = 0;
>> + guest_cpuinfo.zfr64.bits[0] = 0;
>> +
>> + /* Disable MTE as Xen does not support it */
>> + guest_cpuinfo.pfr64.mte = 0;
>> +#endif
>> +
>> + /* Disable AMU */
>> +#ifdef CONFIG_ARM_64
>> + guest_cpuinfo.pfr64.amu = 0;
>> +#endif
>> + guest_cpuinfo.pfr32.amu = 0;
>> +
>> + /* Disable RAS as Xen does not support it */
>> +#ifdef CONFIG_ARM_64
>> + guest_cpuinfo.pfr64.ras = 0;
>> + guest_cpuinfo.pfr64.ras_frac = 0;
>> +#endif
>> + guest_cpuinfo.pfr32.ras = 0;
>> + guest_cpuinfo.pfr32.ras_frac = 0;
>> +
>> + return 0;
>> +}
>> +/*
>> + * This function needs to be run after all smp are started to have
>> + * cpuinfo structures for all cores.
>> + */
>
> This comment contradicts with TODO at the beginning of
> create_guest_cpuinfo().
>
I think the comment is coherent as it is a prerequisite to solve the TODO.
I made it this way so that nothing would need to be modified there to handle the TODO.
So I do not really see a contradiction there, what would you suggest to say instead ?
Regards
Bertrand
>> +__initcall(create_guest_cpuinfo);
>> +
>> /*
>> * Local variables:
>> * mode: C
>> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
>> index 64354c3f19..0ab6dd42a0 100644
>> --- a/xen/include/asm-arm/cpufeature.h
>> +++ b/xen/include/asm-arm/cpufeature.h
>> @@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *);
>> extern struct cpuinfo_arm cpu_data[];
>> #define current_cpu_data cpu_data[smp_processor_id()]
>>
>> +extern struct cpuinfo_arm guest_cpuinfo;
>> +
>> #endif /* __ASSEMBLY__ */
>>
>> #endif
>
>
> --
> Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest
2020-11-30 14:21 ` [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest Bertrand Marquis
2020-11-30 20:15 ` Volodymyr Babchuk
@ 2020-12-04 23:57 ` Stefano Stabellini
2020-12-07 17:24 ` Bertrand Marquis
1 sibling, 1 reply; 31+ messages in thread
From: Stefano Stabellini @ 2020-12-04 23:57 UTC (permalink / raw)
To: Bertrand Marquis
Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk
On Mon, 30 Nov 2020, Bertrand Marquis wrote:
> Create a cpuinfo structure for guest and mask into it the features that
> we do not support in Xen or that we do not want to publish to guests.
>
> Modify some values in the cpuinfo structure for guests to mask some
> features which we do not want to allow to guests (like AMU) or we do not
> support (like SVE).
The first two sentences seem to say the same thing in two different
ways.
> The code is trying to group together registers modifications for the
> same feature to be able in the long term to easily enable/disable a
> feature depending on user parameters or add other registers modification
> in the same place (like enabling/disabling HCR bits).
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in V2: rebase
> ---
> xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++
> xen/include/asm-arm/cpufeature.h | 2 ++
> 2 files changed, 53 insertions(+)
>
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 204be9b084..309941ff37 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -24,6 +24,8 @@
>
> DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>
> +struct cpuinfo_arm __read_mostly guest_cpuinfo;
> +
> void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
> const char *info)
> {
> @@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c)
> #endif
> }
>
> +/*
> + * This function is creating a cpuinfo structure with values modified to mask
> + * all cpu features that should not be published to guest.
> + * The created structure is then used to provide ID registers values to guests.
> + */
> +static int __init create_guest_cpuinfo(void)
> +{
> + /*
> + * TODO: The code is currently using only the features detected on the boot
> + * core. In the long term we should try to compute values containing only
> + * features supported by all cores.
> + */
> + identify_cpu(&guest_cpuinfo);
Given that we already have boot_cpu_data and current_cpu_data, which
should be already initialized at this point, we could simply:
guest_cpuinfo = current_cpu_data;
or
guest_cpuinfo = boot_cpu_data;
?
> +#ifdef CONFIG_ARM_64
> + /* Disable MPAM as xen does not support it */
> + guest_cpuinfo.pfr64.mpam = 0;
> + guest_cpuinfo.pfr64.mpam_frac = 0;
> +
> + /* Disable SVE as Xen does not support it */
> + guest_cpuinfo.pfr64.sve = 0;
> + guest_cpuinfo.zfr64.bits[0] = 0;
> +
> + /* Disable MTE as Xen does not support it */
> + guest_cpuinfo.pfr64.mte = 0;
> +#endif
> +
> + /* Disable AMU */
> +#ifdef CONFIG_ARM_64
> + guest_cpuinfo.pfr64.amu = 0;
> +#endif
> + guest_cpuinfo.pfr32.amu = 0;
> +
> + /* Disable RAS as Xen does not support it */
> +#ifdef CONFIG_ARM_64
> + guest_cpuinfo.pfr64.ras = 0;
> + guest_cpuinfo.pfr64.ras_frac = 0;
> +#endif
> + guest_cpuinfo.pfr32.ras = 0;
> + guest_cpuinfo.pfr32.ras_frac = 0;
> +
> + return 0;
> +}
> +/*
> + * This function needs to be run after all smp are started to have
> + * cpuinfo structures for all cores.
> + */
> +__initcall(create_guest_cpuinfo);
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 64354c3f19..0ab6dd42a0 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *);
> extern struct cpuinfo_arm cpu_data[];
> #define current_cpu_data cpu_data[smp_processor_id()]
>
> +extern struct cpuinfo_arm guest_cpuinfo;
> +
> #endif /* __ASSEMBLY__ */
>
> #endif
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest
2020-12-04 23:57 ` Stefano Stabellini
@ 2020-12-07 17:24 ` Bertrand Marquis
0 siblings, 0 replies; 31+ messages in thread
From: Bertrand Marquis @ 2020-12-07 17:24 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, Julien Grall, Volodymyr Babchuk
Hi Stefano,
> On 4 Dec 2020, at 23:57, Stefano Stabellini <sstabellini@kernel.org> wrote:
>
> On Mon, 30 Nov 2020, Bertrand Marquis wrote:
>> Create a cpuinfo structure for guest and mask into it the features that
>> we do not support in Xen or that we do not want to publish to guests.
>>
>> Modify some values in the cpuinfo structure for guests to mask some
>> features which we do not want to allow to guests (like AMU) or we do not
>> support (like SVE).
>
> The first two sentences seem to say the same thing in two different
> ways.
>
>
>> The code is trying to group together registers modifications for the
>> same feature to be able in the long term to easily enable/disable a
>> feature depending on user parameters or add other registers modification
>> in the same place (like enabling/disabling HCR bits).
>>
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> Changes in V2: rebase
>> ---
>> xen/arch/arm/cpufeature.c | 51 ++++++++++++++++++++++++++++++++
>> xen/include/asm-arm/cpufeature.h | 2 ++
>> 2 files changed, 53 insertions(+)
>>
>> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
>> index 204be9b084..309941ff37 100644
>> --- a/xen/arch/arm/cpufeature.c
>> +++ b/xen/arch/arm/cpufeature.c
>> @@ -24,6 +24,8 @@
>>
>> DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>>
>> +struct cpuinfo_arm __read_mostly guest_cpuinfo;
>> +
>> void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>> const char *info)
>> {
>> @@ -156,6 +158,55 @@ void identify_cpu(struct cpuinfo_arm *c)
>> #endif
>> }
>>
>> +/*
>> + * This function is creating a cpuinfo structure with values modified to mask
>> + * all cpu features that should not be published to guest.
>> + * The created structure is then used to provide ID registers values to guests.
>> + */
>> +static int __init create_guest_cpuinfo(void)
>> +{
>> + /*
>> + * TODO: The code is currently using only the features detected on the boot
>> + * core. In the long term we should try to compute values containing only
>> + * features supported by all cores.
>> + */
>> + identify_cpu(&guest_cpuinfo);
>
> Given that we already have boot_cpu_data and current_cpu_data, which
> should be already initialized at this point, we could simply:
>
> guest_cpuinfo = current_cpu_data;
>
> or
>
> guest_cpuinfo = boot_cpu_data;
>
> ?
Ack, I will do that.
Cheers
Bertrand
>
>
>> +#ifdef CONFIG_ARM_64
>> + /* Disable MPAM as xen does not support it */
>> + guest_cpuinfo.pfr64.mpam = 0;
>> + guest_cpuinfo.pfr64.mpam_frac = 0;
>> +
>> + /* Disable SVE as Xen does not support it */
>> + guest_cpuinfo.pfr64.sve = 0;
>> + guest_cpuinfo.zfr64.bits[0] = 0;
>> +
>> + /* Disable MTE as Xen does not support it */
>> + guest_cpuinfo.pfr64.mte = 0;
>> +#endif
>> +
>> + /* Disable AMU */
>> +#ifdef CONFIG_ARM_64
>> + guest_cpuinfo.pfr64.amu = 0;
>> +#endif
>> + guest_cpuinfo.pfr32.amu = 0;
>> +
>> + /* Disable RAS as Xen does not support it */
>> +#ifdef CONFIG_ARM_64
>> + guest_cpuinfo.pfr64.ras = 0;
>> + guest_cpuinfo.pfr64.ras_frac = 0;
>> +#endif
>> + guest_cpuinfo.pfr32.ras = 0;
>> + guest_cpuinfo.pfr32.ras_frac = 0;
>> +
>> + return 0;
>> +}
>> +/*
>> + * This function needs to be run after all smp are started to have
>> + * cpuinfo structures for all cores.
>> + */
>> +__initcall(create_guest_cpuinfo);
>> +
>> /*
>> * Local variables:
>> * mode: C
>> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
>> index 64354c3f19..0ab6dd42a0 100644
>> --- a/xen/include/asm-arm/cpufeature.h
>> +++ b/xen/include/asm-arm/cpufeature.h
>> @@ -290,6 +290,8 @@ extern void identify_cpu(struct cpuinfo_arm *);
>> extern struct cpuinfo_arm cpu_data[];
>> #define current_cpu_data cpu_data[smp_processor_id()]
>>
>> +extern struct cpuinfo_arm guest_cpuinfo;
>> +
>> #endif /* __ASSEMBLY__ */
>>
>> #endif
>> --
>> 2.17.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64
2020-11-30 14:21 [PATCH v2 0/7] xen/arm: Emulate ID registers Bertrand Marquis
` (2 preceding siblings ...)
2020-11-30 14:21 ` [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest Bertrand Marquis
@ 2020-11-30 14:21 ` Bertrand Marquis
2020-11-30 20:22 ` Volodymyr Babchuk
2020-12-05 0:19 ` Stefano Stabellini
2020-11-30 14:21 ` [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers Bertrand Marquis
` (2 subsequent siblings)
6 siblings, 2 replies; 31+ messages in thread
From: Bertrand Marquis @ 2020-11-30 14:21 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk
Add vsysreg emulation for registers trapped when TID3 bit is activated
in HSR.
The emulation is returning the value stored in cpuinfo_guest structure
for most values and the hardware value for registers not stored in the
structure (those are mostly registers existing only as a provision for
feature use but who have no definition right now).
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: rebase
---
xen/arch/arm/arm64/vsysreg.c | 49 ++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
index 8a85507d9d..970ef51603 100644
--- a/xen/arch/arm/arm64/vsysreg.c
+++ b/xen/arch/arm/arm64/vsysreg.c
@@ -69,6 +69,14 @@ TVM_REG(CONTEXTIDR_EL1)
break; \
}
+/* Macro to generate easily case for ID co-processor emulation */
+#define GENERATE_TID3_INFO(reg,field,offset) \
+ case HSR_SYSREG_##reg: \
+ { \
+ return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, \
+ 1, guest_cpuinfo.field.bits[offset]); \
+ }
+
void do_sysreg(struct cpu_user_regs *regs,
const union hsr hsr)
{
@@ -259,6 +267,47 @@ void do_sysreg(struct cpu_user_regs *regs,
*/
return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
+ /*
+ * HCR_EL2.TID3
+ *
+ * This is trapping most Identification registers used by a guest
+ * to identify the processor features
+ */
+ GENERATE_TID3_INFO(ID_PFR0_EL1, pfr32, 0)
+ GENERATE_TID3_INFO(ID_PFR1_EL1, pfr32, 1)
+ GENERATE_TID3_INFO(ID_PFR2_EL1, pfr32, 2)
+ GENERATE_TID3_INFO(ID_DFR0_EL1, dbg32, 0)
+ GENERATE_TID3_INFO(ID_DFR1_EL1, dbg32, 1)
+ GENERATE_TID3_INFO(ID_AFR0_EL1, aux32, 0)
+ GENERATE_TID3_INFO(ID_MMFR0_EL1, mm32, 0)
+ GENERATE_TID3_INFO(ID_MMFR1_EL1, mm32, 1)
+ GENERATE_TID3_INFO(ID_MMFR2_EL1, mm32, 2)
+ GENERATE_TID3_INFO(ID_MMFR3_EL1, mm32, 3)
+ GENERATE_TID3_INFO(ID_MMFR4_EL1, mm32, 4)
+ GENERATE_TID3_INFO(ID_MMFR5_EL1, mm32, 5)
+ GENERATE_TID3_INFO(ID_ISAR0_EL1, isa32, 0)
+ GENERATE_TID3_INFO(ID_ISAR1_EL1, isa32, 1)
+ GENERATE_TID3_INFO(ID_ISAR2_EL1, isa32, 2)
+ GENERATE_TID3_INFO(ID_ISAR3_EL1, isa32, 3)
+ GENERATE_TID3_INFO(ID_ISAR4_EL1, isa32, 4)
+ GENERATE_TID3_INFO(ID_ISAR5_EL1, isa32, 5)
+ GENERATE_TID3_INFO(ID_ISAR6_EL1, isa32, 6)
+ GENERATE_TID3_INFO(MVFR0_EL1, mvfr, 0)
+ GENERATE_TID3_INFO(MVFR1_EL1, mvfr, 1)
+ GENERATE_TID3_INFO(MVFR2_EL1, mvfr, 2)
+ GENERATE_TID3_INFO(ID_AA64PFR0_EL1, pfr64, 0)
+ GENERATE_TID3_INFO(ID_AA64PFR1_EL1, pfr64, 1)
+ GENERATE_TID3_INFO(ID_AA64DFR0_EL1, dbg64, 0)
+ GENERATE_TID3_INFO(ID_AA64DFR1_EL1, dbg64, 1)
+ GENERATE_TID3_INFO(ID_AA64ISAR0_EL1, isa64, 0)
+ GENERATE_TID3_INFO(ID_AA64ISAR1_EL1, isa64, 1)
+ GENERATE_TID3_INFO(ID_AA64MMFR0_EL1, mm64, 0)
+ GENERATE_TID3_INFO(ID_AA64MMFR1_EL1, mm64, 1)
+ GENERATE_TID3_INFO(ID_AA64MMFR2_EL1, mm64, 2)
+ GENERATE_TID3_INFO(ID_AA64AFR0_EL1, aux64, 0)
+ GENERATE_TID3_INFO(ID_AA64AFR1_EL1, aux64, 1)
+ GENERATE_TID3_INFO(ID_AA64ZFR0_EL1, zfr64, 0)
+
/*
* HCR_EL2.TIDCP
*
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64
2020-11-30 14:21 ` [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64 Bertrand Marquis
@ 2020-11-30 20:22 ` Volodymyr Babchuk
2020-12-01 11:42 ` Bertrand Marquis
2020-12-05 0:19 ` Stefano Stabellini
1 sibling, 1 reply; 31+ messages in thread
From: Volodymyr Babchuk @ 2020-11-30 20:22 UTC (permalink / raw)
To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Bertrand Marquis writes:
> Add vsysreg emulation for registers trapped when TID3 bit is activated
> in HSR.
> The emulation is returning the value stored in cpuinfo_guest structure
> for most values and the hardware value for registers not stored in the
> structure (those are mostly registers existing only as a provision for
> feature use but who have no definition right now).
I can't see the code that returns values for the registers not stored in
the guest_cpuinfo. Perhaps you need to update the commit description?
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in V2: rebase
> ---
> xen/arch/arm/arm64/vsysreg.c | 49 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
> index 8a85507d9d..970ef51603 100644
> --- a/xen/arch/arm/arm64/vsysreg.c
> +++ b/xen/arch/arm/arm64/vsysreg.c
> @@ -69,6 +69,14 @@ TVM_REG(CONTEXTIDR_EL1)
> break; \
> }
>
> +/* Macro to generate easily case for ID co-processor emulation */
> +#define GENERATE_TID3_INFO(reg,field,offset) \
> + case HSR_SYSREG_##reg: \
> + { \
> + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, \
> + 1, guest_cpuinfo.field.bits[offset]); \
> + }
> +
> void do_sysreg(struct cpu_user_regs *regs,
> const union hsr hsr)
> {
> @@ -259,6 +267,47 @@ void do_sysreg(struct cpu_user_regs *regs,
> */
> return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>
> + /*
> + * HCR_EL2.TID3
> + *
> + * This is trapping most Identification registers used by a guest
> + * to identify the processor features
> + */
> + GENERATE_TID3_INFO(ID_PFR0_EL1, pfr32, 0)
> + GENERATE_TID3_INFO(ID_PFR1_EL1, pfr32, 1)
> + GENERATE_TID3_INFO(ID_PFR2_EL1, pfr32, 2)
> + GENERATE_TID3_INFO(ID_DFR0_EL1, dbg32, 0)
> + GENERATE_TID3_INFO(ID_DFR1_EL1, dbg32, 1)
> + GENERATE_TID3_INFO(ID_AFR0_EL1, aux32, 0)
> + GENERATE_TID3_INFO(ID_MMFR0_EL1, mm32, 0)
> + GENERATE_TID3_INFO(ID_MMFR1_EL1, mm32, 1)
> + GENERATE_TID3_INFO(ID_MMFR2_EL1, mm32, 2)
> + GENERATE_TID3_INFO(ID_MMFR3_EL1, mm32, 3)
> + GENERATE_TID3_INFO(ID_MMFR4_EL1, mm32, 4)
> + GENERATE_TID3_INFO(ID_MMFR5_EL1, mm32, 5)
> + GENERATE_TID3_INFO(ID_ISAR0_EL1, isa32, 0)
> + GENERATE_TID3_INFO(ID_ISAR1_EL1, isa32, 1)
> + GENERATE_TID3_INFO(ID_ISAR2_EL1, isa32, 2)
> + GENERATE_TID3_INFO(ID_ISAR3_EL1, isa32, 3)
> + GENERATE_TID3_INFO(ID_ISAR4_EL1, isa32, 4)
> + GENERATE_TID3_INFO(ID_ISAR5_EL1, isa32, 5)
> + GENERATE_TID3_INFO(ID_ISAR6_EL1, isa32, 6)
> + GENERATE_TID3_INFO(MVFR0_EL1, mvfr, 0)
> + GENERATE_TID3_INFO(MVFR1_EL1, mvfr, 1)
> + GENERATE_TID3_INFO(MVFR2_EL1, mvfr, 2)
> + GENERATE_TID3_INFO(ID_AA64PFR0_EL1, pfr64, 0)
> + GENERATE_TID3_INFO(ID_AA64PFR1_EL1, pfr64, 1)
> + GENERATE_TID3_INFO(ID_AA64DFR0_EL1, dbg64, 0)
> + GENERATE_TID3_INFO(ID_AA64DFR1_EL1, dbg64, 1)
> + GENERATE_TID3_INFO(ID_AA64ISAR0_EL1, isa64, 0)
> + GENERATE_TID3_INFO(ID_AA64ISAR1_EL1, isa64, 1)
> + GENERATE_TID3_INFO(ID_AA64MMFR0_EL1, mm64, 0)
> + GENERATE_TID3_INFO(ID_AA64MMFR1_EL1, mm64, 1)
> + GENERATE_TID3_INFO(ID_AA64MMFR2_EL1, mm64, 2)
> + GENERATE_TID3_INFO(ID_AA64AFR0_EL1, aux64, 0)
> + GENERATE_TID3_INFO(ID_AA64AFR1_EL1, aux64, 1)
> + GENERATE_TID3_INFO(ID_AA64ZFR0_EL1, zfr64, 0)
> +
> /*
> * HCR_EL2.TIDCP
> *
--
Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64
2020-11-30 20:22 ` Volodymyr Babchuk
@ 2020-12-01 11:42 ` Bertrand Marquis
2020-12-01 11:54 ` Volodymyr Babchuk
0 siblings, 1 reply; 31+ messages in thread
From: Bertrand Marquis @ 2020-12-01 11:42 UTC (permalink / raw)
To: Volodymyr Babchuk; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hi Volodymyr,
> On 30 Nov 2020, at 20:22, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>
>
>
> Bertrand Marquis writes:
>
>> Add vsysreg emulation for registers trapped when TID3 bit is activated
>> in HSR.
>> The emulation is returning the value stored in cpuinfo_guest structure
>> for most values and the hardware value for registers not stored in the
>> structure (those are mostly registers existing only as a provision for
>> feature use but who have no definition right now).
>
> I can't see the code that returns values for the registers not stored in
> the guest_cpuinfo. Perhaps you need to update the commit description?
You are right, i modified my code lately to handle all possible registers so
this case does not exist anymore.
I will update the commit message to fix this.
Cheers
Bertrand
>
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> Changes in V2: rebase
>> ---
>> xen/arch/arm/arm64/vsysreg.c | 49 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 49 insertions(+)
>>
>> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
>> index 8a85507d9d..970ef51603 100644
>> --- a/xen/arch/arm/arm64/vsysreg.c
>> +++ b/xen/arch/arm/arm64/vsysreg.c
>> @@ -69,6 +69,14 @@ TVM_REG(CONTEXTIDR_EL1)
>> break; \
>> }
>>
>> +/* Macro to generate easily case for ID co-processor emulation */
>> +#define GENERATE_TID3_INFO(reg,field,offset) \
>> + case HSR_SYSREG_##reg: \
>> + { \
>> + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, \
>> + 1, guest_cpuinfo.field.bits[offset]); \
>> + }
>> +
>> void do_sysreg(struct cpu_user_regs *regs,
>> const union hsr hsr)
>> {
>> @@ -259,6 +267,47 @@ void do_sysreg(struct cpu_user_regs *regs,
>> */
>> return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>>
>> + /*
>> + * HCR_EL2.TID3
>> + *
>> + * This is trapping most Identification registers used by a guest
>> + * to identify the processor features
>> + */
>> + GENERATE_TID3_INFO(ID_PFR0_EL1, pfr32, 0)
>> + GENERATE_TID3_INFO(ID_PFR1_EL1, pfr32, 1)
>> + GENERATE_TID3_INFO(ID_PFR2_EL1, pfr32, 2)
>> + GENERATE_TID3_INFO(ID_DFR0_EL1, dbg32, 0)
>> + GENERATE_TID3_INFO(ID_DFR1_EL1, dbg32, 1)
>> + GENERATE_TID3_INFO(ID_AFR0_EL1, aux32, 0)
>> + GENERATE_TID3_INFO(ID_MMFR0_EL1, mm32, 0)
>> + GENERATE_TID3_INFO(ID_MMFR1_EL1, mm32, 1)
>> + GENERATE_TID3_INFO(ID_MMFR2_EL1, mm32, 2)
>> + GENERATE_TID3_INFO(ID_MMFR3_EL1, mm32, 3)
>> + GENERATE_TID3_INFO(ID_MMFR4_EL1, mm32, 4)
>> + GENERATE_TID3_INFO(ID_MMFR5_EL1, mm32, 5)
>> + GENERATE_TID3_INFO(ID_ISAR0_EL1, isa32, 0)
>> + GENERATE_TID3_INFO(ID_ISAR1_EL1, isa32, 1)
>> + GENERATE_TID3_INFO(ID_ISAR2_EL1, isa32, 2)
>> + GENERATE_TID3_INFO(ID_ISAR3_EL1, isa32, 3)
>> + GENERATE_TID3_INFO(ID_ISAR4_EL1, isa32, 4)
>> + GENERATE_TID3_INFO(ID_ISAR5_EL1, isa32, 5)
>> + GENERATE_TID3_INFO(ID_ISAR6_EL1, isa32, 6)
>> + GENERATE_TID3_INFO(MVFR0_EL1, mvfr, 0)
>> + GENERATE_TID3_INFO(MVFR1_EL1, mvfr, 1)
>> + GENERATE_TID3_INFO(MVFR2_EL1, mvfr, 2)
>> + GENERATE_TID3_INFO(ID_AA64PFR0_EL1, pfr64, 0)
>> + GENERATE_TID3_INFO(ID_AA64PFR1_EL1, pfr64, 1)
>> + GENERATE_TID3_INFO(ID_AA64DFR0_EL1, dbg64, 0)
>> + GENERATE_TID3_INFO(ID_AA64DFR1_EL1, dbg64, 1)
>> + GENERATE_TID3_INFO(ID_AA64ISAR0_EL1, isa64, 0)
>> + GENERATE_TID3_INFO(ID_AA64ISAR1_EL1, isa64, 1)
>> + GENERATE_TID3_INFO(ID_AA64MMFR0_EL1, mm64, 0)
>> + GENERATE_TID3_INFO(ID_AA64MMFR1_EL1, mm64, 1)
>> + GENERATE_TID3_INFO(ID_AA64MMFR2_EL1, mm64, 2)
>> + GENERATE_TID3_INFO(ID_AA64AFR0_EL1, aux64, 0)
>> + GENERATE_TID3_INFO(ID_AA64AFR1_EL1, aux64, 1)
>> + GENERATE_TID3_INFO(ID_AA64ZFR0_EL1, zfr64, 0)
>> +
>> /*
>> * HCR_EL2.TIDCP
>> *
>
>
> --
> Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64
2020-12-01 11:42 ` Bertrand Marquis
@ 2020-12-01 11:54 ` Volodymyr Babchuk
0 siblings, 0 replies; 31+ messages in thread
From: Volodymyr Babchuk @ 2020-12-01 11:54 UTC (permalink / raw)
To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hi Bertrand,
Bertrand Marquis writes:
> Hi Volodymyr,
>
>> On 30 Nov 2020, at 20:22, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>
>>
>>
>> Bertrand Marquis writes:
>>
>>> Add vsysreg emulation for registers trapped when TID3 bit is activated
>>> in HSR.
>>> The emulation is returning the value stored in cpuinfo_guest structure
>>> for most values and the hardware value for registers not stored in the
>>> structure (those are mostly registers existing only as a provision for
>>> feature use but who have no definition right now).
>>
>> I can't see the code that returns values for the registers not stored in
>> the guest_cpuinfo. Perhaps you need to update the commit description?
>
> You are right, i modified my code lately to handle all possible registers so
> this case does not exist anymore.
You are covering all currently known registers. If I got reference
manual right, there are number of not assigned ID registers, that should
be RAZ. But with this patch access to them will trigger undefined
instruction abort in a guest.
> I will update the commit message to fix this.
I believe, you need to add cases for currently unassigned registers, so
access to them will yield 0.
> Cheers
> Bertrand
>
>>
>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>> ---
>>> Changes in V2: rebase
>>> ---
>>> xen/arch/arm/arm64/vsysreg.c | 49 ++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 49 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
>>> index 8a85507d9d..970ef51603 100644
>>> --- a/xen/arch/arm/arm64/vsysreg.c
>>> +++ b/xen/arch/arm/arm64/vsysreg.c
>>> @@ -69,6 +69,14 @@ TVM_REG(CONTEXTIDR_EL1)
>>> break; \
>>> }
>>>
>>> +/* Macro to generate easily case for ID co-processor emulation */
>>> +#define GENERATE_TID3_INFO(reg,field,offset) \
>>> + case HSR_SYSREG_##reg: \
>>> + { \
>>> + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, \
>>> + 1, guest_cpuinfo.field.bits[offset]); \
>>> + }
>>> +
>>> void do_sysreg(struct cpu_user_regs *regs,
>>> const union hsr hsr)
>>> {
>>> @@ -259,6 +267,47 @@ void do_sysreg(struct cpu_user_regs *regs,
>>> */
>>> return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>>>
>>> + /*
>>> + * HCR_EL2.TID3
>>> + *
>>> + * This is trapping most Identification registers used by a guest
>>> + * to identify the processor features
>>> + */
>>> + GENERATE_TID3_INFO(ID_PFR0_EL1, pfr32, 0)
>>> + GENERATE_TID3_INFO(ID_PFR1_EL1, pfr32, 1)
>>> + GENERATE_TID3_INFO(ID_PFR2_EL1, pfr32, 2)
>>> + GENERATE_TID3_INFO(ID_DFR0_EL1, dbg32, 0)
>>> + GENERATE_TID3_INFO(ID_DFR1_EL1, dbg32, 1)
>>> + GENERATE_TID3_INFO(ID_AFR0_EL1, aux32, 0)
>>> + GENERATE_TID3_INFO(ID_MMFR0_EL1, mm32, 0)
>>> + GENERATE_TID3_INFO(ID_MMFR1_EL1, mm32, 1)
>>> + GENERATE_TID3_INFO(ID_MMFR2_EL1, mm32, 2)
>>> + GENERATE_TID3_INFO(ID_MMFR3_EL1, mm32, 3)
>>> + GENERATE_TID3_INFO(ID_MMFR4_EL1, mm32, 4)
>>> + GENERATE_TID3_INFO(ID_MMFR5_EL1, mm32, 5)
>>> + GENERATE_TID3_INFO(ID_ISAR0_EL1, isa32, 0)
>>> + GENERATE_TID3_INFO(ID_ISAR1_EL1, isa32, 1)
>>> + GENERATE_TID3_INFO(ID_ISAR2_EL1, isa32, 2)
>>> + GENERATE_TID3_INFO(ID_ISAR3_EL1, isa32, 3)
>>> + GENERATE_TID3_INFO(ID_ISAR4_EL1, isa32, 4)
>>> + GENERATE_TID3_INFO(ID_ISAR5_EL1, isa32, 5)
>>> + GENERATE_TID3_INFO(ID_ISAR6_EL1, isa32, 6)
>>> + GENERATE_TID3_INFO(MVFR0_EL1, mvfr, 0)
>>> + GENERATE_TID3_INFO(MVFR1_EL1, mvfr, 1)
>>> + GENERATE_TID3_INFO(MVFR2_EL1, mvfr, 2)
>>> + GENERATE_TID3_INFO(ID_AA64PFR0_EL1, pfr64, 0)
>>> + GENERATE_TID3_INFO(ID_AA64PFR1_EL1, pfr64, 1)
>>> + GENERATE_TID3_INFO(ID_AA64DFR0_EL1, dbg64, 0)
>>> + GENERATE_TID3_INFO(ID_AA64DFR1_EL1, dbg64, 1)
>>> + GENERATE_TID3_INFO(ID_AA64ISAR0_EL1, isa64, 0)
>>> + GENERATE_TID3_INFO(ID_AA64ISAR1_EL1, isa64, 1)
>>> + GENERATE_TID3_INFO(ID_AA64MMFR0_EL1, mm64, 0)
>>> + GENERATE_TID3_INFO(ID_AA64MMFR1_EL1, mm64, 1)
>>> + GENERATE_TID3_INFO(ID_AA64MMFR2_EL1, mm64, 2)
>>> + GENERATE_TID3_INFO(ID_AA64AFR0_EL1, aux64, 0)
>>> + GENERATE_TID3_INFO(ID_AA64AFR1_EL1, aux64, 1)
>>> + GENERATE_TID3_INFO(ID_AA64ZFR0_EL1, zfr64, 0)
>>> +
>>> /*
>>> * HCR_EL2.TIDCP
>>> *
>>
>>
>> --
>> Volodymyr Babchuk at EPAM
--
Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64
2020-11-30 14:21 ` [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64 Bertrand Marquis
2020-11-30 20:22 ` Volodymyr Babchuk
@ 2020-12-05 0:19 ` Stefano Stabellini
1 sibling, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2020-12-05 0:19 UTC (permalink / raw)
To: Bertrand Marquis
Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk
On Mon, 30 Nov 2020, Bertrand Marquis wrote:
> Add vsysreg emulation for registers trapped when TID3 bit is activated
> in HSR.
> The emulation is returning the value stored in cpuinfo_guest structure
> for most values and the hardware value for registers not stored in the
> structure (those are mostly registers existing only as a provision for
> feature use but who have no definition right now).
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in V2: rebase
> ---
> xen/arch/arm/arm64/vsysreg.c | 49 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
> index 8a85507d9d..970ef51603 100644
> --- a/xen/arch/arm/arm64/vsysreg.c
> +++ b/xen/arch/arm/arm64/vsysreg.c
> @@ -69,6 +69,14 @@ TVM_REG(CONTEXTIDR_EL1)
> break; \
> }
>
> +/* Macro to generate easily case for ID co-processor emulation */
> +#define GENERATE_TID3_INFO(reg,field,offset) \
In addition to Volodymyr's comment, this should be for code style:
GENERATE_TID3_INFO(reg, field, offset)
> + case HSR_SYSREG_##reg: \
> + { \
> + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, \
> + 1, guest_cpuinfo.field.bits[offset]); \
> + }
> +
> void do_sysreg(struct cpu_user_regs *regs,
> const union hsr hsr)
> {
> @@ -259,6 +267,47 @@ void do_sysreg(struct cpu_user_regs *regs,
> */
> return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>
> + /*
> + * HCR_EL2.TID3
> + *
> + * This is trapping most Identification registers used by a guest
> + * to identify the processor features
> + */
> + GENERATE_TID3_INFO(ID_PFR0_EL1, pfr32, 0)
> + GENERATE_TID3_INFO(ID_PFR1_EL1, pfr32, 1)
> + GENERATE_TID3_INFO(ID_PFR2_EL1, pfr32, 2)
> + GENERATE_TID3_INFO(ID_DFR0_EL1, dbg32, 0)
> + GENERATE_TID3_INFO(ID_DFR1_EL1, dbg32, 1)
> + GENERATE_TID3_INFO(ID_AFR0_EL1, aux32, 0)
> + GENERATE_TID3_INFO(ID_MMFR0_EL1, mm32, 0)
> + GENERATE_TID3_INFO(ID_MMFR1_EL1, mm32, 1)
> + GENERATE_TID3_INFO(ID_MMFR2_EL1, mm32, 2)
> + GENERATE_TID3_INFO(ID_MMFR3_EL1, mm32, 3)
> + GENERATE_TID3_INFO(ID_MMFR4_EL1, mm32, 4)
> + GENERATE_TID3_INFO(ID_MMFR5_EL1, mm32, 5)
> + GENERATE_TID3_INFO(ID_ISAR0_EL1, isa32, 0)
> + GENERATE_TID3_INFO(ID_ISAR1_EL1, isa32, 1)
> + GENERATE_TID3_INFO(ID_ISAR2_EL1, isa32, 2)
> + GENERATE_TID3_INFO(ID_ISAR3_EL1, isa32, 3)
> + GENERATE_TID3_INFO(ID_ISAR4_EL1, isa32, 4)
> + GENERATE_TID3_INFO(ID_ISAR5_EL1, isa32, 5)
> + GENERATE_TID3_INFO(ID_ISAR6_EL1, isa32, 6)
> + GENERATE_TID3_INFO(MVFR0_EL1, mvfr, 0)
> + GENERATE_TID3_INFO(MVFR1_EL1, mvfr, 1)
> + GENERATE_TID3_INFO(MVFR2_EL1, mvfr, 2)
> + GENERATE_TID3_INFO(ID_AA64PFR0_EL1, pfr64, 0)
> + GENERATE_TID3_INFO(ID_AA64PFR1_EL1, pfr64, 1)
> + GENERATE_TID3_INFO(ID_AA64DFR0_EL1, dbg64, 0)
> + GENERATE_TID3_INFO(ID_AA64DFR1_EL1, dbg64, 1)
> + GENERATE_TID3_INFO(ID_AA64ISAR0_EL1, isa64, 0)
> + GENERATE_TID3_INFO(ID_AA64ISAR1_EL1, isa64, 1)
> + GENERATE_TID3_INFO(ID_AA64MMFR0_EL1, mm64, 0)
> + GENERATE_TID3_INFO(ID_AA64MMFR1_EL1, mm64, 1)
> + GENERATE_TID3_INFO(ID_AA64MMFR2_EL1, mm64, 2)
> + GENERATE_TID3_INFO(ID_AA64AFR0_EL1, aux64, 0)
> + GENERATE_TID3_INFO(ID_AA64AFR1_EL1, aux64, 1)
> + GENERATE_TID3_INFO(ID_AA64ZFR0_EL1, zfr64, 0)
> +
> /*
> * HCR_EL2.TIDCP
> *
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers
2020-11-30 14:21 [PATCH v2 0/7] xen/arm: Emulate ID registers Bertrand Marquis
` (3 preceding siblings ...)
2020-11-30 14:21 ` [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64 Bertrand Marquis
@ 2020-11-30 14:21 ` Bertrand Marquis
2020-11-30 20:31 ` Volodymyr Babchuk
2020-11-30 14:21 ` [PATCH v2 6/7] xen/arm: Add CP10 exception support to handle VMFR Bertrand Marquis
2020-11-30 14:21 ` [PATCH v2 7/7] xen/arm: Activate TID3 in HCR_EL2 Bertrand Marquis
6 siblings, 1 reply; 31+ messages in thread
From: Bertrand Marquis @ 2020-11-30 14:21 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk
Add support for emulation of cp15 based ID registers (on arm32 or when
running a 32bit guest on arm64).
The handlers are returning the values stored in the guest_cpuinfo
structure.
In the current status the MVFR registers are no supported.
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: rebase
---
xen/arch/arm/vcpreg.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
index cdc91cdf5b..d0c6406f34 100644
--- a/xen/arch/arm/vcpreg.c
+++ b/xen/arch/arm/vcpreg.c
@@ -155,6 +155,14 @@ TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
break; \
}
+/* Macro to generate easily case for ID co-processor emulation */
+#define GENERATE_TID3_INFO(reg,field,offset) \
+ case HSR_CPREG32(reg): \
+ { \
+ return handle_ro_read_val(regs, regidx, cp32.read, hsr, \
+ 1, guest_cpuinfo.field.bits[offset]); \
+ }
+
void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
{
const struct hsr_cp32 cp32 = hsr.cp32;
@@ -286,6 +294,33 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
*/
return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
+ /*
+ * HCR_EL2.TID3
+ *
+ * This is trapping most Identification registers used by a guest
+ * to identify the processor features
+ */
+ GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
+ GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
+ GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
+ GENERATE_TID3_INFO(ID_DFR0, dbg32, 0)
+ GENERATE_TID3_INFO(ID_DFR1, dbg32, 1)
+ GENERATE_TID3_INFO(ID_AFR0, aux32, 0)
+ GENERATE_TID3_INFO(ID_MMFR0, mm32, 0)
+ GENERATE_TID3_INFO(ID_MMFR1, mm32, 1)
+ GENERATE_TID3_INFO(ID_MMFR2, mm32, 2)
+ GENERATE_TID3_INFO(ID_MMFR3, mm32, 3)
+ GENERATE_TID3_INFO(ID_MMFR4, mm32, 4)
+ GENERATE_TID3_INFO(ID_MMFR5, mm32, 5)
+ GENERATE_TID3_INFO(ID_ISAR0, isa32, 0)
+ GENERATE_TID3_INFO(ID_ISAR1, isa32, 1)
+ GENERATE_TID3_INFO(ID_ISAR2, isa32, 2)
+ GENERATE_TID3_INFO(ID_ISAR3, isa32, 3)
+ GENERATE_TID3_INFO(ID_ISAR4, isa32, 4)
+ GENERATE_TID3_INFO(ID_ISAR5, isa32, 5)
+ GENERATE_TID3_INFO(ID_ISAR6, isa32, 6)
+ /* MVFR registers are in cp10 no cp15 */
+
/*
* HCR_EL2.TIDCP
*
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers
2020-11-30 14:21 ` [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers Bertrand Marquis
@ 2020-11-30 20:31 ` Volodymyr Babchuk
2020-12-01 11:46 ` Bertrand Marquis
0 siblings, 1 reply; 31+ messages in thread
From: Volodymyr Babchuk @ 2020-11-30 20:31 UTC (permalink / raw)
To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Bertrand Marquis writes:
> Add support for emulation of cp15 based ID registers (on arm32 or when
> running a 32bit guest on arm64).
> The handlers are returning the values stored in the guest_cpuinfo
> structure.
> In the current status the MVFR registers are no supported.
It is unclear what will happen with registers that are not covered by
guest_cpuinfo structure. According to ARM ARM, it is implementation
defined if such accesses will be trapped. On other hand, there are many
registers which are RAZ. So, good behaving guest can try to read one of
that registers and it will get undefined instruction exception, instead
of just reading all zeroes.
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in V2: rebase
> ---
> xen/arch/arm/vcpreg.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> index cdc91cdf5b..d0c6406f34 100644
> --- a/xen/arch/arm/vcpreg.c
> +++ b/xen/arch/arm/vcpreg.c
> @@ -155,6 +155,14 @@ TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
> break; \
> }
>
> +/* Macro to generate easily case for ID co-processor emulation */
> +#define GENERATE_TID3_INFO(reg,field,offset) \
> + case HSR_CPREG32(reg): \
> + { \
> + return handle_ro_read_val(regs, regidx, cp32.read, hsr, \
> + 1, guest_cpuinfo.field.bits[offset]); \
> + }
> +
> void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
> {
> const struct hsr_cp32 cp32 = hsr.cp32;
> @@ -286,6 +294,33 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
> */
> return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
>
> + /*
> + * HCR_EL2.TID3
> + *
> + * This is trapping most Identification registers used by a guest
> + * to identify the processor features
> + */
> + GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
> + GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
> + GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
> + GENERATE_TID3_INFO(ID_DFR0, dbg32, 0)
> + GENERATE_TID3_INFO(ID_DFR1, dbg32, 1)
> + GENERATE_TID3_INFO(ID_AFR0, aux32, 0)
> + GENERATE_TID3_INFO(ID_MMFR0, mm32, 0)
> + GENERATE_TID3_INFO(ID_MMFR1, mm32, 1)
> + GENERATE_TID3_INFO(ID_MMFR2, mm32, 2)
> + GENERATE_TID3_INFO(ID_MMFR3, mm32, 3)
> + GENERATE_TID3_INFO(ID_MMFR4, mm32, 4)
> + GENERATE_TID3_INFO(ID_MMFR5, mm32, 5)
> + GENERATE_TID3_INFO(ID_ISAR0, isa32, 0)
> + GENERATE_TID3_INFO(ID_ISAR1, isa32, 1)
> + GENERATE_TID3_INFO(ID_ISAR2, isa32, 2)
> + GENERATE_TID3_INFO(ID_ISAR3, isa32, 3)
> + GENERATE_TID3_INFO(ID_ISAR4, isa32, 4)
> + GENERATE_TID3_INFO(ID_ISAR5, isa32, 5)
> + GENERATE_TID3_INFO(ID_ISAR6, isa32, 6)
> + /* MVFR registers are in cp10 no cp15 */
> +
> /*
> * HCR_EL2.TIDCP
> *
--
Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers
2020-11-30 20:31 ` Volodymyr Babchuk
@ 2020-12-01 11:46 ` Bertrand Marquis
2020-12-01 12:07 ` Volodymyr Babchuk
0 siblings, 1 reply; 31+ messages in thread
From: Bertrand Marquis @ 2020-12-01 11:46 UTC (permalink / raw)
To: Volodymyr Babchuk; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hi,
> On 30 Nov 2020, at 20:31, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>
>
> Bertrand Marquis writes:
>
>> Add support for emulation of cp15 based ID registers (on arm32 or when
>> running a 32bit guest on arm64).
>> The handlers are returning the values stored in the guest_cpuinfo
>> structure.
>> In the current status the MVFR registers are no supported.
>
> It is unclear what will happen with registers that are not covered by
> guest_cpuinfo structure. According to ARM ARM, it is implementation
> defined if such accesses will be trapped. On other hand, there are many
> registers which are RAZ. So, good behaving guest can try to read one of
> that registers and it will get undefined instruction exception, instead
> of just reading all zeroes.
This is true in the status of this patch but this is solved by the next patch
which is adding proper handling of those registers (add CP10 exception
support), at least for MVFR ones.
From ARM ARM point of view, I did handle all registers listed I think.
If you think some are missing please point me to them as O do not
completely understand what are the “registers not covered” unless
you mean the MVFR ones.
Cheers
Bertrand
>
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> Changes in V2: rebase
>> ---
>> xen/arch/arm/vcpreg.c | 35 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>> index cdc91cdf5b..d0c6406f34 100644
>> --- a/xen/arch/arm/vcpreg.c
>> +++ b/xen/arch/arm/vcpreg.c
>> @@ -155,6 +155,14 @@ TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
>> break; \
>> }
>>
>> +/* Macro to generate easily case for ID co-processor emulation */
>> +#define GENERATE_TID3_INFO(reg,field,offset) \
>> + case HSR_CPREG32(reg): \
>> + { \
>> + return handle_ro_read_val(regs, regidx, cp32.read, hsr, \
>> + 1, guest_cpuinfo.field.bits[offset]); \
>> + }
>> +
>> void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>> {
>> const struct hsr_cp32 cp32 = hsr.cp32;
>> @@ -286,6 +294,33 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>> */
>> return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
>>
>> + /*
>> + * HCR_EL2.TID3
>> + *
>> + * This is trapping most Identification registers used by a guest
>> + * to identify the processor features
>> + */
>> + GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
>> + GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
>> + GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
>> + GENERATE_TID3_INFO(ID_DFR0, dbg32, 0)
>> + GENERATE_TID3_INFO(ID_DFR1, dbg32, 1)
>> + GENERATE_TID3_INFO(ID_AFR0, aux32, 0)
>> + GENERATE_TID3_INFO(ID_MMFR0, mm32, 0)
>> + GENERATE_TID3_INFO(ID_MMFR1, mm32, 1)
>> + GENERATE_TID3_INFO(ID_MMFR2, mm32, 2)
>> + GENERATE_TID3_INFO(ID_MMFR3, mm32, 3)
>> + GENERATE_TID3_INFO(ID_MMFR4, mm32, 4)
>> + GENERATE_TID3_INFO(ID_MMFR5, mm32, 5)
>> + GENERATE_TID3_INFO(ID_ISAR0, isa32, 0)
>> + GENERATE_TID3_INFO(ID_ISAR1, isa32, 1)
>> + GENERATE_TID3_INFO(ID_ISAR2, isa32, 2)
>> + GENERATE_TID3_INFO(ID_ISAR3, isa32, 3)
>> + GENERATE_TID3_INFO(ID_ISAR4, isa32, 4)
>> + GENERATE_TID3_INFO(ID_ISAR5, isa32, 5)
>> + GENERATE_TID3_INFO(ID_ISAR6, isa32, 6)
>> + /* MVFR registers are in cp10 no cp15 */
>> +
>> /*
>> * HCR_EL2.TIDCP
>> *
>
>
> --
> Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers
2020-12-01 11:46 ` Bertrand Marquis
@ 2020-12-01 12:07 ` Volodymyr Babchuk
2020-12-01 14:21 ` Bertrand Marquis
0 siblings, 1 reply; 31+ messages in thread
From: Volodymyr Babchuk @ 2020-12-01 12:07 UTC (permalink / raw)
To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hi,
Bertrand Marquis writes:
> Hi,
>
>> On 30 Nov 2020, at 20:31, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>
>>
>> Bertrand Marquis writes:
>>
>>> Add support for emulation of cp15 based ID registers (on arm32 or when
>>> running a 32bit guest on arm64).
>>> The handlers are returning the values stored in the guest_cpuinfo
>>> structure.
>>> In the current status the MVFR registers are no supported.
>>
>> It is unclear what will happen with registers that are not covered by
>> guest_cpuinfo structure. According to ARM ARM, it is implementation
>> defined if such accesses will be trapped. On other hand, there are many
>> registers which are RAZ. So, good behaving guest can try to read one of
>> that registers and it will get undefined instruction exception, instead
>> of just reading all zeroes.
>
> This is true in the status of this patch but this is solved by the next patch
> which is adding proper handling of those registers (add CP10 exception
> support), at least for MVFR ones.
>
> From ARM ARM point of view, I did handle all registers listed I think.
> If you think some are missing please point me to them as O do not
> completely understand what are the “registers not covered” unless
> you mean the MVFR ones.
Well, I may be wrong for aarch32 case, but for aarch64, there are number
of reserved registers in IDs range. Those registers should read as
zero. You can find them in the section "C5.1.6 op0==0b11, Moves to and
from non-debug System registers and Special-purpose registers" of ARM
DDI 0487B.a. Check out "Table C5-6 System instruction encodings for
non-Debug System register accesses".
>>
>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>> ---
>>> Changes in V2: rebase
>>> ---
>>> xen/arch/arm/vcpreg.c | 35 +++++++++++++++++++++++++++++++++++
>>> 1 file changed, 35 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>>> index cdc91cdf5b..d0c6406f34 100644
>>> --- a/xen/arch/arm/vcpreg.c
>>> +++ b/xen/arch/arm/vcpreg.c
>>> @@ -155,6 +155,14 @@ TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
>>> break; \
>>> }
>>>
>>> +/* Macro to generate easily case for ID co-processor emulation */
>>> +#define GENERATE_TID3_INFO(reg,field,offset) \
>>> + case HSR_CPREG32(reg): \
>>> + { \
>>> + return handle_ro_read_val(regs, regidx, cp32.read, hsr, \
>>> + 1, guest_cpuinfo.field.bits[offset]); \
>>> + }
>>> +
>>> void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>> {
>>> const struct hsr_cp32 cp32 = hsr.cp32;
>>> @@ -286,6 +294,33 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>> */
>>> return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
>>>
>>> + /*
>>> + * HCR_EL2.TID3
>>> + *
>>> + * This is trapping most Identification registers used by a guest
>>> + * to identify the processor features
>>> + */
>>> + GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
>>> + GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
>>> + GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
>>> + GENERATE_TID3_INFO(ID_DFR0, dbg32, 0)
>>> + GENERATE_TID3_INFO(ID_DFR1, dbg32, 1)
>>> + GENERATE_TID3_INFO(ID_AFR0, aux32, 0)
>>> + GENERATE_TID3_INFO(ID_MMFR0, mm32, 0)
>>> + GENERATE_TID3_INFO(ID_MMFR1, mm32, 1)
>>> + GENERATE_TID3_INFO(ID_MMFR2, mm32, 2)
>>> + GENERATE_TID3_INFO(ID_MMFR3, mm32, 3)
>>> + GENERATE_TID3_INFO(ID_MMFR4, mm32, 4)
>>> + GENERATE_TID3_INFO(ID_MMFR5, mm32, 5)
>>> + GENERATE_TID3_INFO(ID_ISAR0, isa32, 0)
>>> + GENERATE_TID3_INFO(ID_ISAR1, isa32, 1)
>>> + GENERATE_TID3_INFO(ID_ISAR2, isa32, 2)
>>> + GENERATE_TID3_INFO(ID_ISAR3, isa32, 3)
>>> + GENERATE_TID3_INFO(ID_ISAR4, isa32, 4)
>>> + GENERATE_TID3_INFO(ID_ISAR5, isa32, 5)
>>> + GENERATE_TID3_INFO(ID_ISAR6, isa32, 6)
>>> + /* MVFR registers are in cp10 no cp15 */
>>> +
>>> /*
>>> * HCR_EL2.TIDCP
>>> *
>>
>>
>> --
>> Volodymyr Babchuk at EPAM
--
Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers
2020-12-01 12:07 ` Volodymyr Babchuk
@ 2020-12-01 14:21 ` Bertrand Marquis
2020-12-01 16:54 ` Volodymyr Babchuk
0 siblings, 1 reply; 31+ messages in thread
From: Bertrand Marquis @ 2020-12-01 14:21 UTC (permalink / raw)
To: Volodymyr Babchuk; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hi Volodymyr,
> On 1 Dec 2020, at 12:07, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>
>
> Hi,
>
> Bertrand Marquis writes:
>
>> Hi,
>>
>>> On 30 Nov 2020, at 20:31, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>>
>>>
>>> Bertrand Marquis writes:
>>>
>>>> Add support for emulation of cp15 based ID registers (on arm32 or when
>>>> running a 32bit guest on arm64).
>>>> The handlers are returning the values stored in the guest_cpuinfo
>>>> structure.
>>>> In the current status the MVFR registers are no supported.
>>>
>>> It is unclear what will happen with registers that are not covered by
>>> guest_cpuinfo structure. According to ARM ARM, it is implementation
>>> defined if such accesses will be trapped. On other hand, there are many
>>> registers which are RAZ. So, good behaving guest can try to read one of
>>> that registers and it will get undefined instruction exception, instead
>>> of just reading all zeroes.
>>
>> This is true in the status of this patch but this is solved by the next patch
>> which is adding proper handling of those registers (add CP10 exception
>> support), at least for MVFR ones.
>>
>> From ARM ARM point of view, I did handle all registers listed I think.
>> If you think some are missing please point me to them as O do not
>> completely understand what are the “registers not covered” unless
>> you mean the MVFR ones.
>
> Well, I may be wrong for aarch32 case, but for aarch64, there are number
> of reserved registers in IDs range. Those registers should read as
> zero. You can find them in the section "C5.1.6 op0==0b11, Moves to and
> from non-debug System registers and Special-purpose registers" of ARM
> DDI 0487B.a. Check out "Table C5-6 System instruction encodings for
> non-Debug System register accesses".
The point of the serie is to handle all registers trapped due to TID3 bit in HCR_EL2.
And i think I handled all of them but I might be wrong.
Handling all registers for op0==0b11 will cover a lot more things.
This can be done of course but this was not the point of this serie.
The listing in HCR_EL2 documentation is pretty complete and if I miss any register
there please tell me but I do no understand from the documentation that all registers
with op0 3 are trapped by TID3.
Regards
Bertrand
>
>
>>>
>>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>>> ---
>>>> Changes in V2: rebase
>>>> ---
>>>> xen/arch/arm/vcpreg.c | 35 +++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 35 insertions(+)
>>>>
>>>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>>>> index cdc91cdf5b..d0c6406f34 100644
>>>> --- a/xen/arch/arm/vcpreg.c
>>>> +++ b/xen/arch/arm/vcpreg.c
>>>> @@ -155,6 +155,14 @@ TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
>>>> break; \
>>>> }
>>>>
>>>> +/* Macro to generate easily case for ID co-processor emulation */
>>>> +#define GENERATE_TID3_INFO(reg,field,offset) \
>>>> + case HSR_CPREG32(reg): \
>>>> + { \
>>>> + return handle_ro_read_val(regs, regidx, cp32.read, hsr, \
>>>> + 1, guest_cpuinfo.field.bits[offset]); \
>>>> + }
>>>> +
>>>> void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>>> {
>>>> const struct hsr_cp32 cp32 = hsr.cp32;
>>>> @@ -286,6 +294,33 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>>> */
>>>> return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
>>>>
>>>> + /*
>>>> + * HCR_EL2.TID3
>>>> + *
>>>> + * This is trapping most Identification registers used by a guest
>>>> + * to identify the processor features
>>>> + */
>>>> + GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
>>>> + GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
>>>> + GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
>>>> + GENERATE_TID3_INFO(ID_DFR0, dbg32, 0)
>>>> + GENERATE_TID3_INFO(ID_DFR1, dbg32, 1)
>>>> + GENERATE_TID3_INFO(ID_AFR0, aux32, 0)
>>>> + GENERATE_TID3_INFO(ID_MMFR0, mm32, 0)
>>>> + GENERATE_TID3_INFO(ID_MMFR1, mm32, 1)
>>>> + GENERATE_TID3_INFO(ID_MMFR2, mm32, 2)
>>>> + GENERATE_TID3_INFO(ID_MMFR3, mm32, 3)
>>>> + GENERATE_TID3_INFO(ID_MMFR4, mm32, 4)
>>>> + GENERATE_TID3_INFO(ID_MMFR5, mm32, 5)
>>>> + GENERATE_TID3_INFO(ID_ISAR0, isa32, 0)
>>>> + GENERATE_TID3_INFO(ID_ISAR1, isa32, 1)
>>>> + GENERATE_TID3_INFO(ID_ISAR2, isa32, 2)
>>>> + GENERATE_TID3_INFO(ID_ISAR3, isa32, 3)
>>>> + GENERATE_TID3_INFO(ID_ISAR4, isa32, 4)
>>>> + GENERATE_TID3_INFO(ID_ISAR5, isa32, 5)
>>>> + GENERATE_TID3_INFO(ID_ISAR6, isa32, 6)
>>>> + /* MVFR registers are in cp10 no cp15 */
>>>> +
>>>> /*
>>>> * HCR_EL2.TIDCP
>>>> *
>>>
>>>
>>> --
>>> Volodymyr Babchuk at EPAM
>
>
> --
> Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers
2020-12-01 14:21 ` Bertrand Marquis
@ 2020-12-01 16:54 ` Volodymyr Babchuk
2020-12-02 11:12 ` Bertrand Marquis
0 siblings, 1 reply; 31+ messages in thread
From: Volodymyr Babchuk @ 2020-12-01 16:54 UTC (permalink / raw)
To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hi,
Bertrand Marquis writes:
> Hi Volodymyr,
>
>> On 1 Dec 2020, at 12:07, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>
>>
>> Hi,
>>
>> Bertrand Marquis writes:
>>
>>> Hi,
>>>
>>>> On 30 Nov 2020, at 20:31, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>>>
>>>>
>>>> Bertrand Marquis writes:
>>>>
>>>>> Add support for emulation of cp15 based ID registers (on arm32 or when
>>>>> running a 32bit guest on arm64).
>>>>> The handlers are returning the values stored in the guest_cpuinfo
>>>>> structure.
>>>>> In the current status the MVFR registers are no supported.
>>>>
>>>> It is unclear what will happen with registers that are not covered by
>>>> guest_cpuinfo structure. According to ARM ARM, it is implementation
>>>> defined if such accesses will be trapped. On other hand, there are many
>>>> registers which are RAZ. So, good behaving guest can try to read one of
>>>> that registers and it will get undefined instruction exception, instead
>>>> of just reading all zeroes.
>>>
>>> This is true in the status of this patch but this is solved by the next patch
>>> which is adding proper handling of those registers (add CP10 exception
>>> support), at least for MVFR ones.
>>>
>>> From ARM ARM point of view, I did handle all registers listed I think.
>>> If you think some are missing please point me to them as O do not
>>> completely understand what are the “registers not covered” unless
>>> you mean the MVFR ones.
>>
>> Well, I may be wrong for aarch32 case, but for aarch64, there are number
>> of reserved registers in IDs range. Those registers should read as
>> zero. You can find them in the section "C5.1.6 op0==0b11, Moves to and
>> from non-debug System registers and Special-purpose registers" of ARM
>> DDI 0487B.a. Check out "Table C5-6 System instruction encodings for
>> non-Debug System register accesses".
>
> The point of the serie is to handle all registers trapped due to TID3 bit in HCR_EL2.
>
> And i think I handled all of them but I might be wrong.
>
> Handling all registers for op0==0b11 will cover a lot more things.
> This can be done of course but this was not the point of this serie.
>
> The listing in HCR_EL2 documentation is pretty complete and if I miss any register
> there please tell me but I do no understand from the documentation that all registers
> with op0 3 are trapped by TID3.
My concern is that the same code may observe different effects when
running in baremetal mode and as VM.
For example, we are trying to run a newer version of a kernel, that
supports some hypothetical ARMv8.9. And it tries to read a new ID
register which is absent in ARMv8.2. There are possible cases:
0. It runs as a baremetal code on a compatible architecture. So it just
accesses this register and all is fine.
1. It runs as baremetal code on older ARM8 architecture. Current
reference manual states that those registers should read as zero, so
all is fine, as well.
2. It runs as VM on an older architecture. It is IMPLEMENTATION DEFINED
if HCR.ID3 will cause traps when access to unassigned registers:
2a. Platform does not cause traps and software reads zeros directly from
real registers. This is a good outcome.
2b. Platform causes trap, and your code injects the undefined
instruction exception. This is a case that bothers me. Well written code
that is tries to be compatible with different versions of architecture
will fail there.
3. It runs as VM on a never architecture. I can only speculate there,
but I think, that list of registers trapped by HCR.ID3 will be extended
when new features are added. At least, this does not contradict with the
current IMPLEMENTATION DEFINED constraint. In this case, hypervisor will
inject exception when guest tries to access a valid register.
So, in my opinion and to be compatible with the reference manual, we
should allow guests to read "Reserved, RAZ" registers.
> Regards
> Bertrand
>
>
>>
>>
>>>>
>>>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>>>> ---
>>>>> Changes in V2: rebase
>>>>> ---
>>>>> xen/arch/arm/vcpreg.c | 35 +++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 35 insertions(+)
>>>>>
>>>>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>>>>> index cdc91cdf5b..d0c6406f34 100644
>>>>> --- a/xen/arch/arm/vcpreg.c
>>>>> +++ b/xen/arch/arm/vcpreg.c
>>>>> @@ -155,6 +155,14 @@ TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
>>>>> break; \
>>>>> }
>>>>>
>>>>> +/* Macro to generate easily case for ID co-processor emulation */
>>>>> +#define GENERATE_TID3_INFO(reg,field,offset) \
>>>>> + case HSR_CPREG32(reg): \
>>>>> + { \
>>>>> + return handle_ro_read_val(regs, regidx, cp32.read, hsr, \
>>>>> + 1, guest_cpuinfo.field.bits[offset]); \
>>>>> + }
>>>>> +
>>>>> void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>>>> {
>>>>> const struct hsr_cp32 cp32 = hsr.cp32;
>>>>> @@ -286,6 +294,33 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>>>> */
>>>>> return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
>>>>>
>>>>> + /*
>>>>> + * HCR_EL2.TID3
>>>>> + *
>>>>> + * This is trapping most Identification registers used by a guest
>>>>> + * to identify the processor features
>>>>> + */
>>>>> + GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
>>>>> + GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
>>>>> + GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
>>>>> + GENERATE_TID3_INFO(ID_DFR0, dbg32, 0)
>>>>> + GENERATE_TID3_INFO(ID_DFR1, dbg32, 1)
>>>>> + GENERATE_TID3_INFO(ID_AFR0, aux32, 0)
>>>>> + GENERATE_TID3_INFO(ID_MMFR0, mm32, 0)
>>>>> + GENERATE_TID3_INFO(ID_MMFR1, mm32, 1)
>>>>> + GENERATE_TID3_INFO(ID_MMFR2, mm32, 2)
>>>>> + GENERATE_TID3_INFO(ID_MMFR3, mm32, 3)
>>>>> + GENERATE_TID3_INFO(ID_MMFR4, mm32, 4)
>>>>> + GENERATE_TID3_INFO(ID_MMFR5, mm32, 5)
>>>>> + GENERATE_TID3_INFO(ID_ISAR0, isa32, 0)
>>>>> + GENERATE_TID3_INFO(ID_ISAR1, isa32, 1)
>>>>> + GENERATE_TID3_INFO(ID_ISAR2, isa32, 2)
>>>>> + GENERATE_TID3_INFO(ID_ISAR3, isa32, 3)
>>>>> + GENERATE_TID3_INFO(ID_ISAR4, isa32, 4)
>>>>> + GENERATE_TID3_INFO(ID_ISAR5, isa32, 5)
>>>>> + GENERATE_TID3_INFO(ID_ISAR6, isa32, 6)
>>>>> + /* MVFR registers are in cp10 no cp15 */
>>>>> +
>>>>> /*
>>>>> * HCR_EL2.TIDCP
>>>>> *
>>>>
>>>>
>>>> --
>>>> Volodymyr Babchuk at EPAM
>>
>>
>> --
>> Volodymyr Babchuk at EPAM
--
Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers
2020-12-01 16:54 ` Volodymyr Babchuk
@ 2020-12-02 11:12 ` Bertrand Marquis
2020-12-02 11:57 ` Bertrand Marquis
0 siblings, 1 reply; 31+ messages in thread
From: Bertrand Marquis @ 2020-12-02 11:12 UTC (permalink / raw)
To: Volodymyr Babchuk; +Cc: xen-devel, Stefano Stabellini, Julien Grall
HI Volodymyr,
> On 1 Dec 2020, at 16:54, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>
>
> Hi,
>
> Bertrand Marquis writes:
>
>> Hi Volodymyr,
>>
>>> On 1 Dec 2020, at 12:07, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>>
>>>
>>> Hi,
>>>
>>> Bertrand Marquis writes:
>>>
>>>> Hi,
>>>>
>>>>> On 30 Nov 2020, at 20:31, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>>>>
>>>>>
>>>>> Bertrand Marquis writes:
>>>>>
>>>>>> Add support for emulation of cp15 based ID registers (on arm32 or when
>>>>>> running a 32bit guest on arm64).
>>>>>> The handlers are returning the values stored in the guest_cpuinfo
>>>>>> structure.
>>>>>> In the current status the MVFR registers are no supported.
>>>>>
>>>>> It is unclear what will happen with registers that are not covered by
>>>>> guest_cpuinfo structure. According to ARM ARM, it is implementation
>>>>> defined if such accesses will be trapped. On other hand, there are many
>>>>> registers which are RAZ. So, good behaving guest can try to read one of
>>>>> that registers and it will get undefined instruction exception, instead
>>>>> of just reading all zeroes.
>>>>
>>>> This is true in the status of this patch but this is solved by the next patch
>>>> which is adding proper handling of those registers (add CP10 exception
>>>> support), at least for MVFR ones.
>>>>
>>>> From ARM ARM point of view, I did handle all registers listed I think.
>>>> If you think some are missing please point me to them as O do not
>>>> completely understand what are the “registers not covered” unless
>>>> you mean the MVFR ones.
>>>
>>> Well, I may be wrong for aarch32 case, but for aarch64, there are number
>>> of reserved registers in IDs range. Those registers should read as
>>> zero. You can find them in the section "C5.1.6 op0==0b11, Moves to and
>>> from non-debug System registers and Special-purpose registers" of ARM
>>> DDI 0487B.a. Check out "Table C5-6 System instruction encodings for
>>> non-Debug System register accesses".
>>
>> The point of the serie is to handle all registers trapped due to TID3 bit in HCR_EL2.
>>
>> And i think I handled all of them but I might be wrong.
>>
>> Handling all registers for op0==0b11 will cover a lot more things.
>> This can be done of course but this was not the point of this serie.
>>
>> The listing in HCR_EL2 documentation is pretty complete and if I miss any register
>> there please tell me but I do no understand from the documentation that all registers
>> with op0 3 are trapped by TID3.
>
> My concern is that the same code may observe different effects when
> running in baremetal mode and as VM.
>
> For example, we are trying to run a newer version of a kernel, that
> supports some hypothetical ARMv8.9. And it tries to read a new ID
> register which is absent in ARMv8.2. There are possible cases:
>
> 0. It runs as a baremetal code on a compatible architecture. So it just
> accesses this register and all is fine.
>
> 1. It runs as baremetal code on older ARM8 architecture. Current
> reference manual states that those registers should read as zero, so
> all is fine, as well.
>
> 2. It runs as VM on an older architecture. It is IMPLEMENTATION DEFINED
> if HCR.ID3 will cause traps when access to unassigned registers:
>
> 2a. Platform does not cause traps and software reads zeros directly from
> real registers. This is a good outcome.
>
> 2b. Platform causes trap, and your code injects the undefined
> instruction exception. This is a case that bothers me. Well written code
> that is tries to be compatible with different versions of architecture
> will fail there.
>
> 3. It runs as VM on a never architecture. I can only speculate there,
> but I think, that list of registers trapped by HCR.ID3 will be extended
> when new features are added. At least, this does not contradict with the
> current IMPLEMENTATION DEFINED constraint. In this case, hypervisor will
> inject exception when guest tries to access a valid register.
>
>
> So, in my opinion and to be compatible with the reference manual, we
> should allow guests to read "Reserved, RAZ" registers.
Thanks for the very detailed explanation, I know better understand what you
mean here.
I will try to check if we could return RAZ for “other” op0=3 registers and what
should be done on cp15 registers to have something equivalent.
Regards
Bertrand
>
>
>
>> Regards
>> Bertrand
>>
>>
>>>
>>>
>>>>>
>>>>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>>>>> ---
>>>>>> Changes in V2: rebase
>>>>>> ---
>>>>>> xen/arch/arm/vcpreg.c | 35 +++++++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 35 insertions(+)
>>>>>>
>>>>>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>>>>>> index cdc91cdf5b..d0c6406f34 100644
>>>>>> --- a/xen/arch/arm/vcpreg.c
>>>>>> +++ b/xen/arch/arm/vcpreg.c
>>>>>> @@ -155,6 +155,14 @@ TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
>>>>>> break; \
>>>>>> }
>>>>>>
>>>>>> +/* Macro to generate easily case for ID co-processor emulation */
>>>>>> +#define GENERATE_TID3_INFO(reg,field,offset) \
>>>>>> + case HSR_CPREG32(reg): \
>>>>>> + { \
>>>>>> + return handle_ro_read_val(regs, regidx, cp32.read, hsr, \
>>>>>> + 1, guest_cpuinfo.field.bits[offset]); \
>>>>>> + }
>>>>>> +
>>>>>> void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>>>>> {
>>>>>> const struct hsr_cp32 cp32 = hsr.cp32;
>>>>>> @@ -286,6 +294,33 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>>>>> */
>>>>>> return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
>>>>>>
>>>>>> + /*
>>>>>> + * HCR_EL2.TID3
>>>>>> + *
>>>>>> + * This is trapping most Identification registers used by a guest
>>>>>> + * to identify the processor features
>>>>>> + */
>>>>>> + GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
>>>>>> + GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
>>>>>> + GENERATE_TID3_INFO(ID_DFR0, dbg32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_DFR1, dbg32, 1)
>>>>>> + GENERATE_TID3_INFO(ID_AFR0, aux32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR0, mm32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR1, mm32, 1)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR2, mm32, 2)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR3, mm32, 3)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR4, mm32, 4)
>>>>>> + GENERATE_TID3_INFO(ID_MMFR5, mm32, 5)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR0, isa32, 0)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR1, isa32, 1)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR2, isa32, 2)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR3, isa32, 3)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR4, isa32, 4)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR5, isa32, 5)
>>>>>> + GENERATE_TID3_INFO(ID_ISAR6, isa32, 6)
>>>>>> + /* MVFR registers are in cp10 no cp15 */
>>>>>> +
>>>>>> /*
>>>>>> * HCR_EL2.TIDCP
>>>>>> *
>>>>>
>>>>>
>>>>> --
>>>>> Volodymyr Babchuk at EPAM
>>>
>>>
>>> --
>>> Volodymyr Babchuk at EPAM
>
>
> --
> Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers
2020-12-02 11:12 ` Bertrand Marquis
@ 2020-12-02 11:57 ` Bertrand Marquis
2020-12-05 0:36 ` Stefano Stabellini
0 siblings, 1 reply; 31+ messages in thread
From: Bertrand Marquis @ 2020-12-02 11:57 UTC (permalink / raw)
To: Volodymyr Babchuk; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hi,
> On 2 Dec 2020, at 11:12, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
>
> HI Volodymyr,
>
>> On 1 Dec 2020, at 16:54, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>
>>
>> Hi,
>>
>> Bertrand Marquis writes:
>>
>>> Hi Volodymyr,
>>>
>>>> On 1 Dec 2020, at 12:07, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> Bertrand Marquis writes:
>>>>
>>>>> Hi,
>>>>>
>>>>>> On 30 Nov 2020, at 20:31, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>>>>>>
>>>>>>
>>>>>> Bertrand Marquis writes:
>>>>>>
>>>>>>> Add support for emulation of cp15 based ID registers (on arm32 or when
>>>>>>> running a 32bit guest on arm64).
>>>>>>> The handlers are returning the values stored in the guest_cpuinfo
>>>>>>> structure.
>>>>>>> In the current status the MVFR registers are no supported.
>>>>>>
>>>>>> It is unclear what will happen with registers that are not covered by
>>>>>> guest_cpuinfo structure. According to ARM ARM, it is implementation
>>>>>> defined if such accesses will be trapped. On other hand, there are many
>>>>>> registers which are RAZ. So, good behaving guest can try to read one of
>>>>>> that registers and it will get undefined instruction exception, instead
>>>>>> of just reading all zeroes.
>>>>>
>>>>> This is true in the status of this patch but this is solved by the next patch
>>>>> which is adding proper handling of those registers (add CP10 exception
>>>>> support), at least for MVFR ones.
>>>>>
>>>>> From ARM ARM point of view, I did handle all registers listed I think.
>>>>> If you think some are missing please point me to them as O do not
>>>>> completely understand what are the “registers not covered” unless
>>>>> you mean the MVFR ones.
>>>>
>>>> Well, I may be wrong for aarch32 case, but for aarch64, there are number
>>>> of reserved registers in IDs range. Those registers should read as
>>>> zero. You can find them in the section "C5.1.6 op0==0b11, Moves to and
>>>> from non-debug System registers and Special-purpose registers" of ARM
>>>> DDI 0487B.a. Check out "Table C5-6 System instruction encodings for
>>>> non-Debug System register accesses".
>>>
>>> The point of the serie is to handle all registers trapped due to TID3 bit in HCR_EL2.
>>>
>>> And i think I handled all of them but I might be wrong.
>>>
>>> Handling all registers for op0==0b11 will cover a lot more things.
>>> This can be done of course but this was not the point of this serie.
>>>
>>> The listing in HCR_EL2 documentation is pretty complete and if I miss any register
>>> there please tell me but I do no understand from the documentation that all registers
>>> with op0 3 are trapped by TID3.
>>
>> My concern is that the same code may observe different effects when
>> running in baremetal mode and as VM.
>>
>> For example, we are trying to run a newer version of a kernel, that
>> supports some hypothetical ARMv8.9. And it tries to read a new ID
>> register which is absent in ARMv8.2. There are possible cases:
>>
>> 0. It runs as a baremetal code on a compatible architecture. So it just
>> accesses this register and all is fine.
>>
>> 1. It runs as baremetal code on older ARM8 architecture. Current
>> reference manual states that those registers should read as zero, so
>> all is fine, as well.
>>
>> 2. It runs as VM on an older architecture. It is IMPLEMENTATION DEFINED
>> if HCR.ID3 will cause traps when access to unassigned registers:
>>
>> 2a. Platform does not cause traps and software reads zeros directly from
>> real registers. This is a good outcome.
>>
>> 2b. Platform causes trap, and your code injects the undefined
>> instruction exception. This is a case that bothers me. Well written code
>> that is tries to be compatible with different versions of architecture
>> will fail there.
>>
>> 3. It runs as VM on a never architecture. I can only speculate there,
>> but I think, that list of registers trapped by HCR.ID3 will be extended
>> when new features are added. At least, this does not contradict with the
>> current IMPLEMENTATION DEFINED constraint. In this case, hypervisor will
>> inject exception when guest tries to access a valid register.
>>
>>
>> So, in my opinion and to be compatible with the reference manual, we
>> should allow guests to read "Reserved, RAZ" registers.
>
> Thanks for the very detailed explanation, I know better understand what you
> mean here.
>
> I will try to check if we could return RAZ for “other” op0=3 registers and what
> should be done on cp15 registers to have something equivalent.
>
In fact I need to add handling for other registers mentionned by the TID3
description in the armv8 architecture manual:
"This field traps all MRS accesses to registers in the following range that are not
already mentioned in this field description: Op0 == 3, op1 == 0, CRn == c0,
CRm == {c1-c7}, op2 == {0-7}.”
"This field traps all MRC accesses to encodings in the following range that are not
already mentioned in this field description: coproc == p15, opc1 == 0, CRn == c0,
CRm == {c2-c7}, opc2 == {0-7}.”
I will check how i can do that.
Thanks a lot for the review.
Regards
Bertrand
> Regards
> Bertrand
>
>>
>>
>>
>>> Regards
>>> Bertrand
>>>
>>>
>>>>
>>>>
>>>>>>
>>>>>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>>>>>> ---
>>>>>>> Changes in V2: rebase
>>>>>>> ---
>>>>>>> xen/arch/arm/vcpreg.c | 35 +++++++++++++++++++++++++++++++++++
>>>>>>> 1 file changed, 35 insertions(+)
>>>>>>>
>>>>>>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>>>>>>> index cdc91cdf5b..d0c6406f34 100644
>>>>>>> --- a/xen/arch/arm/vcpreg.c
>>>>>>> +++ b/xen/arch/arm/vcpreg.c
>>>>>>> @@ -155,6 +155,14 @@ TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
>>>>>>> break; \
>>>>>>> }
>>>>>>>
>>>>>>> +/* Macro to generate easily case for ID co-processor emulation */
>>>>>>> +#define GENERATE_TID3_INFO(reg,field,offset) \
>>>>>>> + case HSR_CPREG32(reg): \
>>>>>>> + { \
>>>>>>> + return handle_ro_read_val(regs, regidx, cp32.read, hsr, \
>>>>>>> + 1, guest_cpuinfo.field.bits[offset]); \
>>>>>>> + }
>>>>>>> +
>>>>>>> void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>>>>>> {
>>>>>>> const struct hsr_cp32 cp32 = hsr.cp32;
>>>>>>> @@ -286,6 +294,33 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>>>>>>> */
>>>>>>> return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
>>>>>>>
>>>>>>> + /*
>>>>>>> + * HCR_EL2.TID3
>>>>>>> + *
>>>>>>> + * This is trapping most Identification registers used by a guest
>>>>>>> + * to identify the processor features
>>>>>>> + */
>>>>>>> + GENERATE_TID3_INFO(ID_PFR0, pfr32, 0)
>>>>>>> + GENERATE_TID3_INFO(ID_PFR1, pfr32, 1)
>>>>>>> + GENERATE_TID3_INFO(ID_PFR2, pfr32, 2)
>>>>>>> + GENERATE_TID3_INFO(ID_DFR0, dbg32, 0)
>>>>>>> + GENERATE_TID3_INFO(ID_DFR1, dbg32, 1)
>>>>>>> + GENERATE_TID3_INFO(ID_AFR0, aux32, 0)
>>>>>>> + GENERATE_TID3_INFO(ID_MMFR0, mm32, 0)
>>>>>>> + GENERATE_TID3_INFO(ID_MMFR1, mm32, 1)
>>>>>>> + GENERATE_TID3_INFO(ID_MMFR2, mm32, 2)
>>>>>>> + GENERATE_TID3_INFO(ID_MMFR3, mm32, 3)
>>>>>>> + GENERATE_TID3_INFO(ID_MMFR4, mm32, 4)
>>>>>>> + GENERATE_TID3_INFO(ID_MMFR5, mm32, 5)
>>>>>>> + GENERATE_TID3_INFO(ID_ISAR0, isa32, 0)
>>>>>>> + GENERATE_TID3_INFO(ID_ISAR1, isa32, 1)
>>>>>>> + GENERATE_TID3_INFO(ID_ISAR2, isa32, 2)
>>>>>>> + GENERATE_TID3_INFO(ID_ISAR3, isa32, 3)
>>>>>>> + GENERATE_TID3_INFO(ID_ISAR4, isa32, 4)
>>>>>>> + GENERATE_TID3_INFO(ID_ISAR5, isa32, 5)
>>>>>>> + GENERATE_TID3_INFO(ID_ISAR6, isa32, 6)
>>>>>>> + /* MVFR registers are in cp10 no cp15 */
>>>>>>> +
>>>>>>> /*
>>>>>>> * HCR_EL2.TIDCP
>>>>>>> *
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Volodymyr Babchuk at EPAM
>>>>
>>>>
>>>> --
>>>> Volodymyr Babchuk at EPAM
>>
>>
>> --
>> Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers
2020-12-02 11:57 ` Bertrand Marquis
@ 2020-12-05 0:36 ` Stefano Stabellini
0 siblings, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2020-12-05 0:36 UTC (permalink / raw)
To: Bertrand Marquis
Cc: Volodymyr Babchuk, xen-devel, Stefano Stabellini, Julien Grall
[-- Attachment #1: Type: text/plain, Size: 5465 bytes --]
On Wed, 2 Dec 2020, Bertrand Marquis wrote:
> > On 2 Dec 2020, at 11:12, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
> >
> > HI Volodymyr,
> >
> >> On 1 Dec 2020, at 16:54, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
> >>
> >>
> >> Hi,
> >>
> >> Bertrand Marquis writes:
> >>
> >>> Hi Volodymyr,
> >>>
> >>>> On 1 Dec 2020, at 12:07, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
> >>>>
> >>>>
> >>>> Hi,
> >>>>
> >>>> Bertrand Marquis writes:
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>>> On 30 Nov 2020, at 20:31, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
> >>>>>>
> >>>>>>
> >>>>>> Bertrand Marquis writes:
> >>>>>>
> >>>>>>> Add support for emulation of cp15 based ID registers (on arm32 or when
> >>>>>>> running a 32bit guest on arm64).
> >>>>>>> The handlers are returning the values stored in the guest_cpuinfo
> >>>>>>> structure.
> >>>>>>> In the current status the MVFR registers are no supported.
> >>>>>>
> >>>>>> It is unclear what will happen with registers that are not covered by
> >>>>>> guest_cpuinfo structure. According to ARM ARM, it is implementation
> >>>>>> defined if such accesses will be trapped. On other hand, there are many
> >>>>>> registers which are RAZ. So, good behaving guest can try to read one of
> >>>>>> that registers and it will get undefined instruction exception, instead
> >>>>>> of just reading all zeroes.
> >>>>>
> >>>>> This is true in the status of this patch but this is solved by the next patch
> >>>>> which is adding proper handling of those registers (add CP10 exception
> >>>>> support), at least for MVFR ones.
> >>>>>
> >>>>> From ARM ARM point of view, I did handle all registers listed I think.
> >>>>> If you think some are missing please point me to them as O do not
> >>>>> completely understand what are the “registers not covered” unless
> >>>>> you mean the MVFR ones.
> >>>>
> >>>> Well, I may be wrong for aarch32 case, but for aarch64, there are number
> >>>> of reserved registers in IDs range. Those registers should read as
> >>>> zero. You can find them in the section "C5.1.6 op0==0b11, Moves to and
> >>>> from non-debug System registers and Special-purpose registers" of ARM
> >>>> DDI 0487B.a. Check out "Table C5-6 System instruction encodings for
> >>>> non-Debug System register accesses".
> >>>
> >>> The point of the serie is to handle all registers trapped due to TID3 bit in HCR_EL2.
> >>>
> >>> And i think I handled all of them but I might be wrong.
> >>>
> >>> Handling all registers for op0==0b11 will cover a lot more things.
> >>> This can be done of course but this was not the point of this serie.
> >>>
> >>> The listing in HCR_EL2 documentation is pretty complete and if I miss any register
> >>> there please tell me but I do no understand from the documentation that all registers
> >>> with op0 3 are trapped by TID3.
> >>
> >> My concern is that the same code may observe different effects when
> >> running in baremetal mode and as VM.
> >>
> >> For example, we are trying to run a newer version of a kernel, that
> >> supports some hypothetical ARMv8.9. And it tries to read a new ID
> >> register which is absent in ARMv8.2. There are possible cases:
> >>
> >> 0. It runs as a baremetal code on a compatible architecture. So it just
> >> accesses this register and all is fine.
> >>
> >> 1. It runs as baremetal code on older ARM8 architecture. Current
> >> reference manual states that those registers should read as zero, so
> >> all is fine, as well.
> >>
> >> 2. It runs as VM on an older architecture. It is IMPLEMENTATION DEFINED
> >> if HCR.ID3 will cause traps when access to unassigned registers:
> >>
> >> 2a. Platform does not cause traps and software reads zeros directly from
> >> real registers. This is a good outcome.
> >>
> >> 2b. Platform causes trap, and your code injects the undefined
> >> instruction exception. This is a case that bothers me. Well written code
> >> that is tries to be compatible with different versions of architecture
> >> will fail there.
> >>
> >> 3. It runs as VM on a never architecture. I can only speculate there,
> >> but I think, that list of registers trapped by HCR.ID3 will be extended
> >> when new features are added. At least, this does not contradict with the
> >> current IMPLEMENTATION DEFINED constraint. In this case, hypervisor will
> >> inject exception when guest tries to access a valid register.
> >>
> >>
> >> So, in my opinion and to be compatible with the reference manual, we
> >> should allow guests to read "Reserved, RAZ" registers.
> >
> > Thanks for the very detailed explanation, I know better understand what you
> > mean here.
> >
> > I will try to check if we could return RAZ for “other” op0=3 registers and what
> > should be done on cp15 registers to have something equivalent.
> >
>
> In fact I need to add handling for other registers mentionned by the TID3
> description in the armv8 architecture manual:
> "This field traps all MRS accesses to registers in the following range that are not
> already mentioned in this field description: Op0 == 3, op1 == 0, CRn == c0,
> CRm == {c1-c7}, op2 == {0-7}.”
> "This field traps all MRC accesses to encodings in the following range that are not
> already mentioned in this field description: coproc == p15, opc1 == 0, CRn == c0,
> CRm == {c2-c7}, opc2 == {0-7}.”
>
> I will check how i can do that.
>
> Thanks a lot for the review.
Well spotted Volodymyr!
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 6/7] xen/arm: Add CP10 exception support to handle VMFR
2020-11-30 14:21 [PATCH v2 0/7] xen/arm: Emulate ID registers Bertrand Marquis
` (4 preceding siblings ...)
2020-11-30 14:21 ` [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers Bertrand Marquis
@ 2020-11-30 14:21 ` Bertrand Marquis
2020-11-30 20:39 ` Volodymyr Babchuk
2020-11-30 14:21 ` [PATCH v2 7/7] xen/arm: Activate TID3 in HCR_EL2 Bertrand Marquis
6 siblings, 1 reply; 31+ messages in thread
From: Bertrand Marquis @ 2020-11-30 14:21 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk
Add support for cp10 exceptions decoding to be able to emulate the
values for VMFR0 and VMFR1 when TID3 bit of HSR is activated.
This is required for aarch32 guests accessing VMFR0 and VMFR1 using vmrs
and vmsr instructions.
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: rebase
---
xen/arch/arm/traps.c | 5 +++++
xen/arch/arm/vcpreg.c | 38 ++++++++++++++++++++++++++++++++
xen/include/asm-arm/perfc_defn.h | 1 +
xen/include/asm-arm/traps.h | 1 +
4 files changed, 45 insertions(+)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 22bd1bd4c6..28d9d64558 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2097,6 +2097,11 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
perfc_incr(trap_cp14_dbg);
do_cp14_dbg(regs, hsr);
break;
+ case HSR_EC_CP10:
+ GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+ perfc_incr(trap_cp10);
+ do_cp10(regs, hsr);
+ break;
case HSR_EC_CP:
GUEST_BUG_ON(!psr_mode_is_32bit(regs));
perfc_incr(trap_cp);
diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
index d0c6406f34..9d6a36ca5d 100644
--- a/xen/arch/arm/vcpreg.c
+++ b/xen/arch/arm/vcpreg.c
@@ -634,6 +634,44 @@ void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
inject_undef_exception(regs, hsr);
}
+void do_cp10(struct cpu_user_regs *regs, const union hsr hsr)
+{
+ const struct hsr_cp32 cp32 = hsr.cp32;
+ int regidx = cp32.reg;
+
+ if ( !check_conditional_instr(regs, hsr) )
+ {
+ advance_pc(regs, hsr);
+ return;
+ }
+
+ switch ( hsr.bits & HSR_CP32_REGS_MASK )
+ {
+ /*
+ * HSR.TID3 is trapping access to MVFR register used to identify the
+ * VFP/Simd using VMRS/VMSR instructions.
+ * In this case MVFR2 is not supported as the instruction does not support
+ * it.
+ * Exception encoding is using MRC/MCR standard with the reg field in Crn
+ * as are declared MVFR0 and MVFR1 in cpregs.h
+ */
+ GENERATE_TID3_INFO(MVFR0, mvfr, 0)
+ GENERATE_TID3_INFO(MVFR1, mvfr, 1)
+
+ default:
+ gdprintk(XENLOG_ERR,
+ "%s p10, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
+ cp32.read ? "mrc" : "mcr",
+ cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
+ gdprintk(XENLOG_ERR, "unhandled 32-bit CP10 access %#x\n",
+ hsr.bits & HSR_CP32_REGS_MASK);
+ inject_undef_exception(regs, hsr);
+ return;
+ }
+
+ advance_pc(regs, hsr);
+}
+
void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
{
const struct hsr_cp cp = hsr.cp;
diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h
index 6a83185163..31f071222b 100644
--- a/xen/include/asm-arm/perfc_defn.h
+++ b/xen/include/asm-arm/perfc_defn.h
@@ -11,6 +11,7 @@ PERFCOUNTER(trap_cp15_64, "trap: cp15 64-bit access")
PERFCOUNTER(trap_cp14_32, "trap: cp14 32-bit access")
PERFCOUNTER(trap_cp14_64, "trap: cp14 64-bit access")
PERFCOUNTER(trap_cp14_dbg, "trap: cp14 dbg access")
+PERFCOUNTER(trap_cp10, "trap: cp10 access")
PERFCOUNTER(trap_cp, "trap: cp access")
PERFCOUNTER(trap_smc32, "trap: 32-bit smc")
PERFCOUNTER(trap_hvc32, "trap: 32-bit hvc")
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
index 997c37884e..c4a3d0fb1b 100644
--- a/xen/include/asm-arm/traps.h
+++ b/xen/include/asm-arm/traps.h
@@ -62,6 +62,7 @@ void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr);
void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr);
void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr);
void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr);
+void do_cp10(struct cpu_user_regs *regs, const union hsr hsr);
void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
/* SMCCC handling */
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 6/7] xen/arm: Add CP10 exception support to handle VMFR
2020-11-30 14:21 ` [PATCH v2 6/7] xen/arm: Add CP10 exception support to handle VMFR Bertrand Marquis
@ 2020-11-30 20:39 ` Volodymyr Babchuk
2020-12-01 14:04 ` Bertrand Marquis
0 siblings, 1 reply; 31+ messages in thread
From: Volodymyr Babchuk @ 2020-11-30 20:39 UTC (permalink / raw)
To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Bertrand Marquis writes:
> Add support for cp10 exceptions decoding to be able to emulate the
> values for VMFR0 and VMFR1 when TID3 bit of HSR is activated.
> This is required for aarch32 guests accessing VMFR0 and VMFR1 using vmrs
> and vmsr instructions.
is it VMFR or MVFR? According to the reference manual, it is MVFR. Also,
you are missing MVFR2.
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in V2: rebase
> ---
> xen/arch/arm/traps.c | 5 +++++
> xen/arch/arm/vcpreg.c | 38 ++++++++++++++++++++++++++++++++
> xen/include/asm-arm/perfc_defn.h | 1 +
> xen/include/asm-arm/traps.h | 1 +
> 4 files changed, 45 insertions(+)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 22bd1bd4c6..28d9d64558 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2097,6 +2097,11 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
> perfc_incr(trap_cp14_dbg);
> do_cp14_dbg(regs, hsr);
> break;
> + case HSR_EC_CP10:
> + GUEST_BUG_ON(!psr_mode_is_32bit(regs));
> + perfc_incr(trap_cp10);
> + do_cp10(regs, hsr);
> + break;
> case HSR_EC_CP:
> GUEST_BUG_ON(!psr_mode_is_32bit(regs));
> perfc_incr(trap_cp);
> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> index d0c6406f34..9d6a36ca5d 100644
> --- a/xen/arch/arm/vcpreg.c
> +++ b/xen/arch/arm/vcpreg.c
> @@ -634,6 +634,44 @@ void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
> inject_undef_exception(regs, hsr);
> }
>
> +void do_cp10(struct cpu_user_regs *regs, const union hsr hsr)
> +{
> + const struct hsr_cp32 cp32 = hsr.cp32;
> + int regidx = cp32.reg;
> +
> + if ( !check_conditional_instr(regs, hsr) )
> + {
> + advance_pc(regs, hsr);
> + return;
> + }
> +
> + switch ( hsr.bits & HSR_CP32_REGS_MASK )
> + {
> + /*
> + * HSR.TID3 is trapping access to MVFR register used to identify the
> + * VFP/Simd using VMRS/VMSR instructions.
> + * In this case MVFR2 is not supported as the instruction does not support
> + * it.
> + * Exception encoding is using MRC/MCR standard with the reg field in Crn
> + * as are declared MVFR0 and MVFR1 in cpregs.h
> + */
> + GENERATE_TID3_INFO(MVFR0, mvfr, 0)
> + GENERATE_TID3_INFO(MVFR1, mvfr, 1)
> +
> + default:
> + gdprintk(XENLOG_ERR,
> + "%s p10, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
> + cp32.read ? "mrc" : "mcr",
> + cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
> + gdprintk(XENLOG_ERR, "unhandled 32-bit CP10 access %#x\n",
> + hsr.bits & HSR_CP32_REGS_MASK);
> + inject_undef_exception(regs, hsr);
> + return;
> + }
> +
> + advance_pc(regs, hsr);
> +}
> +
> void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
> {
> const struct hsr_cp cp = hsr.cp;
> diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h
> index 6a83185163..31f071222b 100644
> --- a/xen/include/asm-arm/perfc_defn.h
> +++ b/xen/include/asm-arm/perfc_defn.h
> @@ -11,6 +11,7 @@ PERFCOUNTER(trap_cp15_64, "trap: cp15 64-bit access")
> PERFCOUNTER(trap_cp14_32, "trap: cp14 32-bit access")
> PERFCOUNTER(trap_cp14_64, "trap: cp14 64-bit access")
> PERFCOUNTER(trap_cp14_dbg, "trap: cp14 dbg access")
> +PERFCOUNTER(trap_cp10, "trap: cp10 access")
> PERFCOUNTER(trap_cp, "trap: cp access")
> PERFCOUNTER(trap_smc32, "trap: 32-bit smc")
> PERFCOUNTER(trap_hvc32, "trap: 32-bit hvc")
> diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
> index 997c37884e..c4a3d0fb1b 100644
> --- a/xen/include/asm-arm/traps.h
> +++ b/xen/include/asm-arm/traps.h
> @@ -62,6 +62,7 @@ void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr);
> void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr);
> void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr);
> void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr);
> +void do_cp10(struct cpu_user_regs *regs, const union hsr hsr);
> void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
>
> /* SMCCC handling */
--
Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 6/7] xen/arm: Add CP10 exception support to handle VMFR
2020-11-30 20:39 ` Volodymyr Babchuk
@ 2020-12-01 14:04 ` Bertrand Marquis
0 siblings, 0 replies; 31+ messages in thread
From: Bertrand Marquis @ 2020-12-01 14:04 UTC (permalink / raw)
To: Volodymyr Babchuk; +Cc: xen-devel, Stefano Stabellini, Julien Grall
Hi,
> On 30 Nov 2020, at 20:39, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> wrote:
>
>
> Bertrand Marquis writes:
>
>> Add support for cp10 exceptions decoding to be able to emulate the
>> values for VMFR0 and VMFR1 when TID3 bit of HSR is activated.
>> This is required for aarch32 guests accessing VMFR0 and VMFR1 using vmrs
>> and vmsr instructions.
>
> is it VMFR or MVFR? According to the reference manual, it is MVFR. Also,
> you are missing MVFR2.
>
Thanks for the typo and it is MVFR (I will fix that).
Regarding MVFR2, it is in fact missing for 32bit implementation and in fact the
idea that it was not available on armv7 is wrong.
Thanks a lot for the finding, I will fix and test this and send a v3 patch.
Regards
Bertrand
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> Changes in V2: rebase
>> ---
>> xen/arch/arm/traps.c | 5 +++++
>> xen/arch/arm/vcpreg.c | 38 ++++++++++++++++++++++++++++++++
>> xen/include/asm-arm/perfc_defn.h | 1 +
>> xen/include/asm-arm/traps.h | 1 +
>> 4 files changed, 45 insertions(+)
>>
>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>> index 22bd1bd4c6..28d9d64558 100644
>> --- a/xen/arch/arm/traps.c
>> +++ b/xen/arch/arm/traps.c
>> @@ -2097,6 +2097,11 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
>> perfc_incr(trap_cp14_dbg);
>> do_cp14_dbg(regs, hsr);
>> break;
>> + case HSR_EC_CP10:
>> + GUEST_BUG_ON(!psr_mode_is_32bit(regs));
>> + perfc_incr(trap_cp10);
>> + do_cp10(regs, hsr);
>> + break;
>> case HSR_EC_CP:
>> GUEST_BUG_ON(!psr_mode_is_32bit(regs));
>> perfc_incr(trap_cp);
>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>> index d0c6406f34..9d6a36ca5d 100644
>> --- a/xen/arch/arm/vcpreg.c
>> +++ b/xen/arch/arm/vcpreg.c
>> @@ -634,6 +634,44 @@ void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
>> inject_undef_exception(regs, hsr);
>> }
>>
>> +void do_cp10(struct cpu_user_regs *regs, const union hsr hsr)
>> +{
>> + const struct hsr_cp32 cp32 = hsr.cp32;
>> + int regidx = cp32.reg;
>> +
>> + if ( !check_conditional_instr(regs, hsr) )
>> + {
>> + advance_pc(regs, hsr);
>> + return;
>> + }
>> +
>> + switch ( hsr.bits & HSR_CP32_REGS_MASK )
>> + {
>> + /*
>> + * HSR.TID3 is trapping access to MVFR register used to identify the
>> + * VFP/Simd using VMRS/VMSR instructions.
>> + * In this case MVFR2 is not supported as the instruction does not support
>> + * it.
>> + * Exception encoding is using MRC/MCR standard with the reg field in Crn
>> + * as are declared MVFR0 and MVFR1 in cpregs.h
>> + */
>> + GENERATE_TID3_INFO(MVFR0, mvfr, 0)
>> + GENERATE_TID3_INFO(MVFR1, mvfr, 1)
>> +
>> + default:
>> + gdprintk(XENLOG_ERR,
>> + "%s p10, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
>> + cp32.read ? "mrc" : "mcr",
>> + cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
>> + gdprintk(XENLOG_ERR, "unhandled 32-bit CP10 access %#x\n",
>> + hsr.bits & HSR_CP32_REGS_MASK);
>> + inject_undef_exception(regs, hsr);
>> + return;
>> + }
>> +
>> + advance_pc(regs, hsr);
>> +}
>> +
>> void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
>> {
>> const struct hsr_cp cp = hsr.cp;
>> diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h
>> index 6a83185163..31f071222b 100644
>> --- a/xen/include/asm-arm/perfc_defn.h
>> +++ b/xen/include/asm-arm/perfc_defn.h
>> @@ -11,6 +11,7 @@ PERFCOUNTER(trap_cp15_64, "trap: cp15 64-bit access")
>> PERFCOUNTER(trap_cp14_32, "trap: cp14 32-bit access")
>> PERFCOUNTER(trap_cp14_64, "trap: cp14 64-bit access")
>> PERFCOUNTER(trap_cp14_dbg, "trap: cp14 dbg access")
>> +PERFCOUNTER(trap_cp10, "trap: cp10 access")
>> PERFCOUNTER(trap_cp, "trap: cp access")
>> PERFCOUNTER(trap_smc32, "trap: 32-bit smc")
>> PERFCOUNTER(trap_hvc32, "trap: 32-bit hvc")
>> diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
>> index 997c37884e..c4a3d0fb1b 100644
>> --- a/xen/include/asm-arm/traps.h
>> +++ b/xen/include/asm-arm/traps.h
>> @@ -62,6 +62,7 @@ void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr);
>> void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr);
>> void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr);
>> void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr);
>> +void do_cp10(struct cpu_user_regs *regs, const union hsr hsr);
>> void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
>>
>> /* SMCCC handling */
>
>
> --
> Volodymyr Babchuk at EPAM
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 7/7] xen/arm: Activate TID3 in HCR_EL2
2020-11-30 14:21 [PATCH v2 0/7] xen/arm: Emulate ID registers Bertrand Marquis
` (5 preceding siblings ...)
2020-11-30 14:21 ` [PATCH v2 6/7] xen/arm: Add CP10 exception support to handle VMFR Bertrand Marquis
@ 2020-11-30 14:21 ` Bertrand Marquis
6 siblings, 0 replies; 31+ messages in thread
From: Bertrand Marquis @ 2020-11-30 14:21 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk
Activate TID3 bit in HSR register when starting a guest.
This will trap all coprecessor ID registers so that we can give to guest
values corresponding to what they can actually use and mask some
features to guests even though they would be supported by the underlying
hardware (like SVE or MPAM).
Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: rebase
---
xen/arch/arm/traps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 28d9d64558..c1a9ad6056 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -98,7 +98,7 @@ register_t get_default_hcr_flags(void)
{
return (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
(vwfi != NATIVE ? (HCR_TWI|HCR_TWE) : 0) |
- HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB|HCR_TSW);
+ HCR_TID3|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB|HCR_TSW);
}
static enum {
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread