[PATCH v2 0/7] xen/arm: Emulate ID registers

[PATCH v2 0/7] xen/arm: Emulate ID registers

[Bertrand Marquis]

The goal of this serie is to emulate coprocessor ID registers so that
Xen only publish to guest features that are supported by Xen and can
actually be used by guests.
One practical example where this is required are SVE support which is
forbidden by Xen as it is not supported, but if Linux is compiled with
it, it will crash on boot. An other one is AMU which is also forbidden
by Xen but one Linux compiled with it would crash if the platform
supports it.

To be able to emulate the coprocessor registers defining what features
are supported by the hardware, the TID3 bit of HCR must be disabled and
Xen must emulated the values of those registers when an exception is
catched when a guest is accessing it.

This serie is first creating a guest cpuinfo structure which will
contain the values that we want to publish to the guests and then
provides the proper emulationg for those registers when Xen is getting
an exception due to an access to any of those registers.

This is a first simple implementation to solve the problem and the way
to define the values that we provide to guests and which features are
disabled will be in a future patchset enhance so that we could decide
per guest what can be used or not and depending on this deduce the bits
to activate in HCR and the values that we must publish on ID registers.

---
Changes in V2:
  Fix First patch to properly handle DFR1 register and increase dbg32
  size. Other patches have just been rebased.

Bertrand Marquis (7):
  xen/arm: Add ID registers and complete cpufinfo
  xen/arm: Add arm64 ID registers definitions
  xen/arm: create a cpuinfo structure for guest
  xen/arm: Add handler for ID registers on arm64
  xen/arm: Add handler for cp15 ID registers
  xen/arm: Add CP10 exception support to handle VMFR
  xen/arm: Activate TID3 in HCR_EL2

 xen/arch/arm/arm64/vsysreg.c        | 49 +++++++++++++++++++
 xen/arch/arm/cpufeature.c           | 68 +++++++++++++++++++++++++++
 xen/arch/arm/traps.c                |  7 ++-
 xen/arch/arm/vcpreg.c               | 73 +++++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/hsr.h     | 37 +++++++++++++++
 xen/include/asm-arm/arm64/sysregs.h | 25 ++++++++++
 xen/include/asm-arm/cpregs.h        | 11 +++++
 xen/include/asm-arm/cpufeature.h    | 65 +++++++++++++++++++++----
 xen/include/asm-arm/perfc_defn.h    |  1 +
 xen/include/asm-arm/traps.h         |  1 +
 10 files changed, 327 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo

[Bertrand Marquis]

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

[PATCH v2 2/7] xen/arm: Add arm64 ID registers definitions

[Bertrand Marquis]

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

[PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest

[Bertrand Marquis]

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

[PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64

[Bertrand Marquis]

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

[PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers

[Bertrand Marquis]

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

[PATCH v2 6/7] xen/arm: Add CP10 exception support to handle VMFR

[Bertrand Marquis]

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

[PATCH v2 7/7] xen/arm: Activate TID3 in HCR_EL2

[Bertrand Marquis]

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

Re: [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo

[Volodymyr Babchuk]

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

Re: [PATCH v2 2/7] xen/arm: Add arm64 ID registers definitions

[Volodymyr Babchuk]

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

Re: [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest

[Volodymyr Babchuk]

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

Re: [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64

[Volodymyr Babchuk]

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

Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers

[Volodymyr Babchuk]

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

Re: [PATCH v2 6/7] xen/arm: Add CP10 exception support to handle VMFR

[Volodymyr Babchuk]

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

Re: [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest

[Bertrand Marquis]

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

Re: [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64

[Bertrand Marquis]

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

Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers

[Bertrand Marquis]

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

Re: [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64

[Volodymyr Babchuk]

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

Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers

[Volodymyr Babchuk]

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

Re: [PATCH v2 6/7] xen/arm: Add CP10 exception support to handle VMFR

[Bertrand Marquis]

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

Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers

[Bertrand Marquis]

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

Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers

[Volodymyr Babchuk]

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

Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers

[Bertrand Marquis]

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

Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers

[Bertrand Marquis]

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

Re: [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo

[Stefano Stabellini]

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
> 

Re: [PATCH v2 2/7] xen/arm: Add arm64 ID registers definitions

[Stefano Stabellini]

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

Re: [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest

[Stefano Stabellini]

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
> 

Re: [PATCH v2 4/7] xen/arm: Add handler for ID registers on arm64

[Stefano Stabellini]

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
> 

Re: [PATCH v2 5/7] xen/arm: Add handler for cp15 ID registers

[Stefano Stabellini]

[-- 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!

Re: [PATCH v2 3/7] xen/arm: create a cpuinfo structure for guest

[Bertrand Marquis]

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

Re: [PATCH v2 1/7] xen/arm: Add ID registers and complete cpufinfo

[Bertrand Marquis]

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