All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/8] xen/arm: Emulate ID registers
@ 2020-12-17 15:38   ` Bertrand Marquis
  2020-12-17 15:38     ` [PATCH v4 1/8] xen/arm: Use READ_SYSREG instead of 32/64 versions Bertrand Marquis
                       ` (9 more replies)
  0 siblings, 10 replies; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-17 15:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

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.

Changes in V3:
  Add handling of reserved registers as RAZ
  Minor fixes described in each patch

Changes in V4:
  Add a patch to switch implementation to use READ_SYSREG instead of the
  32/64 bit version of it.
  Move cases for reserved register handling from macros to the code
  itself.
  Various typos fixes.

Bertrand Marquis (8):
  xen/arm: Use READ_SYSREG instead of 32/64 versions
  xen/arm: Add ID registers and complete cpuinfo
  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 MVFR
  xen/arm: Activate TID3 in HCR_EL2

 xen/arch/arm/arm64/vsysreg.c        |  82 ++++++++++++++++++++
 xen/arch/arm/cpufeature.c           | 113 ++++++++++++++++++++++------
 xen/arch/arm/traps.c                |   7 +-
 xen/arch/arm/vcpreg.c               | 102 +++++++++++++++++++++++++
 xen/include/asm-arm/arm64/hsr.h     |  37 +++++++++
 xen/include/asm-arm/arm64/sysregs.h |  28 +++++++
 xen/include/asm-arm/cpregs.h        |  15 ++++
 xen/include/asm-arm/cpufeature.h    |  58 +++++++++++---
 xen/include/asm-arm/perfc_defn.h    |   1 +
 xen/include/asm-arm/traps.h         |   1 +
 10 files changed, 409 insertions(+), 35 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v4 1/8] xen/arm: Use READ_SYSREG instead of 32/64 versions
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
@ 2020-12-17 15:38     ` Bertrand Marquis
  2020-12-17 23:17       ` Stefano Stabellini
  2020-12-17 15:38     ` [PATCH v4 2/8] xen/arm: Add ID registers and complete cpuinfo Bertrand Marquis
                       ` (8 subsequent siblings)
  9 siblings, 1 reply; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-17 15:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

Modify identify_cpu function to use READ_SYSREG instead of READ_SYSREG32
or READ_SYSREG64.
The aarch32 versions of the registers are 64bit on an aarch64 processor
so it was wrong to access them as 32bit registers.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Change in V4:
  This patch was introduced in v4.

---
 xen/arch/arm/cpufeature.c | 50 +++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 44126dbf07..115e1b164d 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -99,44 +99,44 @@ int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps)
 
 void identify_cpu(struct cpuinfo_arm *c)
 {
-        c->midr.bits = READ_SYSREG32(MIDR_EL1);
+        c->midr.bits = READ_SYSREG(MIDR_EL1);
         c->mpidr.bits = READ_SYSREG(MPIDR_EL1);
 
 #ifdef CONFIG_ARM_64
-        c->pfr64.bits[0] = READ_SYSREG64(ID_AA64PFR0_EL1);
-        c->pfr64.bits[1] = READ_SYSREG64(ID_AA64PFR1_EL1);
+        c->pfr64.bits[0] = READ_SYSREG(ID_AA64PFR0_EL1);
+        c->pfr64.bits[1] = READ_SYSREG(ID_AA64PFR1_EL1);
 
-        c->dbg64.bits[0] = READ_SYSREG64(ID_AA64DFR0_EL1);
-        c->dbg64.bits[1] = READ_SYSREG64(ID_AA64DFR1_EL1);
+        c->dbg64.bits[0] = READ_SYSREG(ID_AA64DFR0_EL1);
+        c->dbg64.bits[1] = READ_SYSREG(ID_AA64DFR1_EL1);
 
-        c->aux64.bits[0] = READ_SYSREG64(ID_AA64AFR0_EL1);
-        c->aux64.bits[1] = READ_SYSREG64(ID_AA64AFR1_EL1);
+        c->aux64.bits[0] = READ_SYSREG(ID_AA64AFR0_EL1);
+        c->aux64.bits[1] = READ_SYSREG(ID_AA64AFR1_EL1);
 
-        c->mm64.bits[0]  = READ_SYSREG64(ID_AA64MMFR0_EL1);
-        c->mm64.bits[1]  = READ_SYSREG64(ID_AA64MMFR1_EL1);
+        c->mm64.bits[0]  = READ_SYSREG(ID_AA64MMFR0_EL1);
+        c->mm64.bits[1]  = READ_SYSREG(ID_AA64MMFR1_EL1);
 
-        c->isa64.bits[0] = READ_SYSREG64(ID_AA64ISAR0_EL1);
-        c->isa64.bits[1] = READ_SYSREG64(ID_AA64ISAR1_EL1);
+        c->isa64.bits[0] = READ_SYSREG(ID_AA64ISAR0_EL1);
+        c->isa64.bits[1] = READ_SYSREG(ID_AA64ISAR1_EL1);
 #endif
 
-        c->pfr32.bits[0] = READ_SYSREG32(ID_PFR0_EL1);
-        c->pfr32.bits[1] = READ_SYSREG32(ID_PFR1_EL1);
+        c->pfr32.bits[0] = READ_SYSREG(ID_PFR0_EL1);
+        c->pfr32.bits[1] = READ_SYSREG(ID_PFR1_EL1);
 
-        c->dbg32.bits[0] = READ_SYSREG32(ID_DFR0_EL1);
+        c->dbg32.bits[0] = READ_SYSREG(ID_DFR0_EL1);
 
-        c->aux32.bits[0] = READ_SYSREG32(ID_AFR0_EL1);
+        c->aux32.bits[0] = READ_SYSREG(ID_AFR0_EL1);
 
-        c->mm32.bits[0]  = READ_SYSREG32(ID_MMFR0_EL1);
-        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[0]  = READ_SYSREG(ID_MMFR0_EL1);
+        c->mm32.bits[1]  = READ_SYSREG(ID_MMFR1_EL1);
+        c->mm32.bits[2]  = READ_SYSREG(ID_MMFR2_EL1);
+        c->mm32.bits[3]  = READ_SYSREG(ID_MMFR3_EL1);
 
-        c->isa32.bits[0] = READ_SYSREG32(ID_ISAR0_EL1);
-        c->isa32.bits[1] = READ_SYSREG32(ID_ISAR1_EL1);
-        c->isa32.bits[2] = READ_SYSREG32(ID_ISAR2_EL1);
-        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[0] = READ_SYSREG(ID_ISAR0_EL1);
+        c->isa32.bits[1] = READ_SYSREG(ID_ISAR1_EL1);
+        c->isa32.bits[2] = READ_SYSREG(ID_ISAR2_EL1);
+        c->isa32.bits[3] = READ_SYSREG(ID_ISAR3_EL1);
+        c->isa32.bits[4] = READ_SYSREG(ID_ISAR4_EL1);
+        c->isa32.bits[5] = READ_SYSREG(ID_ISAR5_EL1);
 }
 
 /*
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v4 2/8] xen/arm: Add ID registers and complete cpuinfo
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
  2020-12-17 15:38     ` [PATCH v4 1/8] xen/arm: Use READ_SYSREG instead of 32/64 versions Bertrand Marquis
@ 2020-12-17 15:38     ` Bertrand Marquis
  2020-12-17 23:22       ` Stefano Stabellini
  2020-12-17 15:38     ` [PATCH v4 3/8] xen/arm: Add arm64 ID registers definitions Bertrand Marquis
                       ` (7 subsequent siblings)
  9 siblings, 1 reply; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-17 15:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

Add definition and entries in cpuinfo for ID registers introduced in
newer Arm Architecture reference manual:
- ID_PFR2: processor feature register 2
- ID_DFR1: debug feature register 1
- ID_MMFR4 and ID_MMFR5: Memory model feature registers 4 and 5
- ID_ISA6: ISA Feature register 6
Add more bitfield definitions in PFR fields of cpuinfo.
Add MVFR2 register definition for aarch32.
Add MVFRx_EL1 defines 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.
Changes in V3:
  Fix typo in commit title
  Add MVFR2 definition and handling on aarch32 and remove specific case
  for mvfr field in cpuinfo (now the same on arm64 and arm32).
  Add MMFR4 definition if not known by the compiler.
Changes in V4:
  Add MVFRx_EL1 defines for aarch32
  Use READ_SYSREG instead of 32/64 versions of the function which
  removed the ifdef case for MVFR access.
  User register_t type for mvfr and zfr64 fields of cpuinfo structure.

---
 xen/arch/arm/cpufeature.c           | 12 +++++++
 xen/include/asm-arm/arm64/sysregs.h | 28 +++++++++++++++
 xen/include/asm-arm/cpregs.h        | 15 ++++++++
 xen/include/asm-arm/cpufeature.h    | 56 ++++++++++++++++++++++++-----
 4 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 115e1b164d..86b99ee960 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_SYSREG(ID_AA64MMFR0_EL1);
         c->mm64.bits[1]  = READ_SYSREG(ID_AA64MMFR1_EL1);
+        c->mm64.bits[2]  = READ_SYSREG(ID_AA64MMFR2_EL1);
 
         c->isa64.bits[0] = READ_SYSREG(ID_AA64ISAR0_EL1);
         c->isa64.bits[1] = READ_SYSREG(ID_AA64ISAR1_EL1);
+
+        c->zfr64.bits[0] = READ_SYSREG(ID_AA64ZFR0_EL1);
 #endif
 
         c->pfr32.bits[0] = READ_SYSREG(ID_PFR0_EL1);
         c->pfr32.bits[1] = READ_SYSREG(ID_PFR1_EL1);
+        c->pfr32.bits[2] = READ_SYSREG(ID_PFR2_EL1);
 
         c->dbg32.bits[0] = READ_SYSREG(ID_DFR0_EL1);
+        c->dbg32.bits[1] = READ_SYSREG(ID_DFR1_EL1);
 
         c->aux32.bits[0] = READ_SYSREG(ID_AFR0_EL1);
 
@@ -130,6 +135,8 @@ void identify_cpu(struct cpuinfo_arm *c)
         c->mm32.bits[1]  = READ_SYSREG(ID_MMFR1_EL1);
         c->mm32.bits[2]  = READ_SYSREG(ID_MMFR2_EL1);
         c->mm32.bits[3]  = READ_SYSREG(ID_MMFR3_EL1);
+        c->mm32.bits[4]  = READ_SYSREG(ID_MMFR4_EL1);
+        c->mm32.bits[5]  = READ_SYSREG(ID_MMFR5_EL1);
 
         c->isa32.bits[0] = READ_SYSREG(ID_ISAR0_EL1);
         c->isa32.bits[1] = READ_SYSREG(ID_ISAR1_EL1);
@@ -137,6 +144,11 @@ void identify_cpu(struct cpuinfo_arm *c)
         c->isa32.bits[3] = READ_SYSREG(ID_ISAR3_EL1);
         c->isa32.bits[4] = READ_SYSREG(ID_ISAR4_EL1);
         c->isa32.bits[5] = READ_SYSREG(ID_ISAR5_EL1);
+        c->isa32.bits[6] = READ_SYSREG(ID_ISAR6_EL1);
+
+        c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
+        c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
+        c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
 }
 
 /*
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index c60029d38f..077fd95fb7 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -57,6 +57,34 @@
 #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_MMFR4_EL1
+#define ID_MMFR4_EL1                S3_0_C0_C2_6
+#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..6daf2b1a30 100644
--- a/xen/include/asm-arm/cpregs.h
+++ b/xen/include/asm-arm/cpregs.h
@@ -63,6 +63,8 @@
 #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 MVFR2           p10,7,c5,c0,0   /* Media and VFP Feature Register 2 */
 #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 +110,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 +319,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
@@ -347,6 +359,9 @@
 #define VPIDR_EL2               VPIDR
 #define VTCR_EL2                VTCR
 #define VTTBR_EL2               VTTBR
+#define MVFR0_EL1               MVFR0
+#define MVFR1_EL1               MVFR1
+#define MVFR2_EL1               MVFR2
 #endif
 
 #endif
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index c7b5052992..74139be1cc 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 {
+        register_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,16 @@ struct cpuinfo_arm {
     } aux32;
 
     struct {
-        uint32_t bits[4];
+        uint32_t bits[6];
     } mm32;
 
     struct {
-        uint32_t bits[6];
+        uint32_t bits[7];
     } isa32;
+
+    struct {
+        register_t bits[3];
+    } mvfr;
 };
 
 extern struct cpuinfo_arm boot_cpu_data;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v4 3/8] xen/arm: Add arm64 ID registers definitions
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
  2020-12-17 15:38     ` [PATCH v4 1/8] xen/arm: Use READ_SYSREG instead of 32/64 versions Bertrand Marquis
  2020-12-17 15:38     ` [PATCH v4 2/8] xen/arm: Add ID registers and complete cpuinfo Bertrand Marquis
@ 2020-12-17 15:38     ` Bertrand Marquis
  2020-12-17 23:24       ` Stefano Stabellini
  2020-12-17 15:38     ` [PATCH v4 4/8] xen/arm: create a cpuinfo structure for guest Bertrand Marquis
                       ` (6 subsequent siblings)
  9 siblings, 1 reply; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-17 15:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

Add coprocessor registers definitions for all ID registers trapped
through the TID3 bit of HSR.
Those are the one that will be emulated in Xen to only publish to guests
the features that are supported by Xen and that are accessible to
guests.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: Rebase
Changes in V3:
  Add case definition for reserved registers.
Changes in V4:
  Remove case definition for reserved registers and move it to the code
  directly.

---
 xen/include/asm-arm/arm64/hsr.h | 37 +++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/xen/include/asm-arm/arm64/hsr.h b/xen/include/asm-arm/arm64/hsr.h
index ca931dd2fe..e691d41c17 100644
--- a/xen/include/asm-arm/arm64/hsr.h
+++ b/xen/include/asm-arm/arm64/hsr.h
@@ -110,6 +110,43 @@
 #define HSR_SYSREG_CNTP_CTL_EL0   HSR_SYSREG(3,3,c14,c2,1)
 #define HSR_SYSREG_CNTP_CVAL_EL0  HSR_SYSREG(3,3,c14,c2,2)
 
+/* Those registers are used when HCR_EL2.TID3 is set */
+#define HSR_SYSREG_ID_PFR0_EL1    HSR_SYSREG(3,0,c0,c1,0)
+#define HSR_SYSREG_ID_PFR1_EL1    HSR_SYSREG(3,0,c0,c1,1)
+#define HSR_SYSREG_ID_PFR2_EL1    HSR_SYSREG(3,0,c0,c3,4)
+#define HSR_SYSREG_ID_DFR0_EL1    HSR_SYSREG(3,0,c0,c1,2)
+#define HSR_SYSREG_ID_DFR1_EL1    HSR_SYSREG(3,0,c0,c3,5)
+#define HSR_SYSREG_ID_AFR0_EL1    HSR_SYSREG(3,0,c0,c1,3)
+#define HSR_SYSREG_ID_MMFR0_EL1   HSR_SYSREG(3,0,c0,c1,4)
+#define HSR_SYSREG_ID_MMFR1_EL1   HSR_SYSREG(3,0,c0,c1,5)
+#define HSR_SYSREG_ID_MMFR2_EL1   HSR_SYSREG(3,0,c0,c1,6)
+#define HSR_SYSREG_ID_MMFR3_EL1   HSR_SYSREG(3,0,c0,c1,7)
+#define HSR_SYSREG_ID_MMFR4_EL1   HSR_SYSREG(3,0,c0,c2,6)
+#define HSR_SYSREG_ID_MMFR5_EL1   HSR_SYSREG(3,0,c0,c3,6)
+#define HSR_SYSREG_ID_ISAR0_EL1   HSR_SYSREG(3,0,c0,c2,0)
+#define HSR_SYSREG_ID_ISAR1_EL1   HSR_SYSREG(3,0,c0,c2,1)
+#define HSR_SYSREG_ID_ISAR2_EL1   HSR_SYSREG(3,0,c0,c2,2)
+#define HSR_SYSREG_ID_ISAR3_EL1   HSR_SYSREG(3,0,c0,c2,3)
+#define HSR_SYSREG_ID_ISAR4_EL1   HSR_SYSREG(3,0,c0,c2,4)
+#define HSR_SYSREG_ID_ISAR5_EL1   HSR_SYSREG(3,0,c0,c2,5)
+#define HSR_SYSREG_ID_ISAR6_EL1   HSR_SYSREG(3,0,c0,c2,7)
+#define HSR_SYSREG_MVFR0_EL1      HSR_SYSREG(3,0,c0,c3,0)
+#define HSR_SYSREG_MVFR1_EL1      HSR_SYSREG(3,0,c0,c3,1)
+#define HSR_SYSREG_MVFR2_EL1      HSR_SYSREG(3,0,c0,c3,2)
+
+#define HSR_SYSREG_ID_AA64PFR0_EL1   HSR_SYSREG(3,0,c0,c4,0)
+#define HSR_SYSREG_ID_AA64PFR1_EL1   HSR_SYSREG(3,0,c0,c4,1)
+#define HSR_SYSREG_ID_AA64DFR0_EL1   HSR_SYSREG(3,0,c0,c5,0)
+#define HSR_SYSREG_ID_AA64DFR1_EL1   HSR_SYSREG(3,0,c0,c5,1)
+#define HSR_SYSREG_ID_AA64ISAR0_EL1  HSR_SYSREG(3,0,c0,c6,0)
+#define HSR_SYSREG_ID_AA64ISAR1_EL1  HSR_SYSREG(3,0,c0,c6,1)
+#define HSR_SYSREG_ID_AA64MMFR0_EL1  HSR_SYSREG(3,0,c0,c7,0)
+#define HSR_SYSREG_ID_AA64MMFR1_EL1  HSR_SYSREG(3,0,c0,c7,1)
+#define HSR_SYSREG_ID_AA64MMFR2_EL1  HSR_SYSREG(3,0,c0,c7,2)
+#define HSR_SYSREG_ID_AA64AFR0_EL1   HSR_SYSREG(3,0,c0,c5,4)
+#define HSR_SYSREG_ID_AA64AFR1_EL1   HSR_SYSREG(3,0,c0,c5,5)
+#define HSR_SYSREG_ID_AA64ZFR0_EL1   HSR_SYSREG(3,0,c0,c4,4)
+
 #endif /* __ASM_ARM_ARM64_HSR_H */
 
 /*
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v4 4/8] xen/arm: create a cpuinfo structure for guest
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
                       ` (2 preceding siblings ...)
  2020-12-17 15:38     ` [PATCH v4 3/8] xen/arm: Add arm64 ID registers definitions Bertrand Marquis
@ 2020-12-17 15:38     ` Bertrand Marquis
  2020-12-17 23:26       ` Stefano Stabellini
  2020-12-17 15:38     ` [PATCH v4 5/8] xen/arm: Add handler for ID registers on arm64 Bertrand Marquis
                       ` (5 subsequent siblings)
  9 siblings, 1 reply; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-17 15:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

Create a cpuinfo structure for guest and mask into it the features that
we do not support in Xen or that we do not want to publish to guests.

Modify some values in the cpuinfo structure for guests to mask some
features which we do not want to allow to guests (like AMU) or we do not
support (like SVE).
Modify some values in the guest cpuinfo structure to guests to hide some
processor features:
- SVE as this is not supported by Xen and guest are not allowed to use
this features (ZEN is set to 0 in CPTR_EL2).
- AMU as HCPTR_TAM is set in CPTR_EL2 so AMU cannot be used by guests
All other bits are left untouched.
- RAS as this is not supported by Xen.

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
Changes in V3:
  Use current_cpu_data info instead of recalling identify_cpu
Changes in V4:
  Use boot_cpu_data instead of current_cpu_data
  Use "hide XX support" instead of disable as this part of the code is
  actually only hidding feature to guests but not disabling them (this
  is done through the HCR register).
  Modify commit message to be more clear about what is done in
  guest_cpuinfo.

---
 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 86b99ee960..1f6a85aafe 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)
 {
@@ -151,6 +153,55 @@ void identify_cpu(struct cpuinfo_arm *c)
         c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
 }
 
+/*
+ * 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.
+     */
+    guest_cpuinfo = boot_cpu_data;
+
+#ifdef CONFIG_ARM_64
+    /* Hide MPAM support as xen does not support it */
+    guest_cpuinfo.pfr64.mpam = 0;
+    guest_cpuinfo.pfr64.mpam_frac = 0;
+
+    /* Hide SVE as Xen does not support it */
+    guest_cpuinfo.pfr64.sve = 0;
+    guest_cpuinfo.zfr64.bits[0] = 0;
+
+    /* Hide MTE support as Xen does not support it */
+    guest_cpuinfo.pfr64.mte = 0;
+#endif
+
+    /* Hide AMU support */
+#ifdef CONFIG_ARM_64
+    guest_cpuinfo.pfr64.amu = 0;
+#endif
+    guest_cpuinfo.pfr32.amu = 0;
+
+    /* Hide RAS support 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 74139be1cc..6058744c18 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -283,6 +283,8 @@ extern void identify_cpu(struct cpuinfo_arm *);
 extern struct cpuinfo_arm cpu_data[];
 #define current_cpu_data cpu_data[smp_processor_id()]
 
+extern struct cpuinfo_arm guest_cpuinfo;
+
 #endif /* __ASSEMBLY__ */
 
 #endif
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v4 5/8] xen/arm: Add handler for ID registers on arm64
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
                       ` (3 preceding siblings ...)
  2020-12-17 15:38     ` [PATCH v4 4/8] xen/arm: create a cpuinfo structure for guest Bertrand Marquis
@ 2020-12-17 15:38     ` Bertrand Marquis
  2020-12-17 23:31       ` Stefano Stabellini
  2020-12-17 15:38     ` [PATCH v4 6/8] xen/arm: Add handler for cp15 ID registers Bertrand Marquis
                       ` (4 subsequent siblings)
  9 siblings, 1 reply; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-17 15:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

Add vsysreg emulation for registers trapped when TID3 bit is activated
in HSR.
The emulation is returning the value stored in cpuinfo_guest structure
for know registers and is handling reserved registers as RAZ.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: Rebase
Changes in V3:
  Fix commit message
  Fix code style for GENERATE_TID3_INFO declaration
  Add handling of reserved registers as RAZ.
Changes in V4:
  Fix indentation in GENERATE_TID3_INFO macro
  Add explicit case code for reserved registers

---
 xen/arch/arm/arm64/vsysreg.c | 82 ++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
index 8a85507d9d..41f18612c6 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,80 @@ 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)
+
+    /*
+     * Those cases are catching all Reserved registers trapped by TID3 which
+     * currently have no assignment.
+     * HCR.TID3 is trapping all registers in the group 3:
+     * Op0 == 3, op1 == 0, CRn == c0,CRm == {c1-c7}, op2 == {0-7}.
+     * Those registers are defined as being RO in the Arm Architecture
+     * Reference manual Armv8 (Chapter D12.3.2 of issue F.c) so handle them
+     * as Read-only read as zero.
+     */
+    case HSR_SYSREG(3,0,c0,c3,3):
+    case HSR_SYSREG(3,0,c0,c3,7):
+    case HSR_SYSREG(3,0,c0,c4,2):
+    case HSR_SYSREG(3,0,c0,c4,3):
+    case HSR_SYSREG(3,0,c0,c4,5):
+    case HSR_SYSREG(3,0,c0,c4,6):
+    case HSR_SYSREG(3,0,c0,c4,7):
+    case HSR_SYSREG(3,0,c0,c5,2):
+    case HSR_SYSREG(3,0,c0,c5,3):
+    case HSR_SYSREG(3,0,c0,c5,6):
+    case HSR_SYSREG(3,0,c0,c5,7):
+    case HSR_SYSREG(3,0,c0,c6,2):
+    case HSR_SYSREG(3,0,c0,c6,3):
+    case HSR_SYSREG(3,0,c0,c6,4):
+    case HSR_SYSREG(3,0,c0,c6,5):
+    case HSR_SYSREG(3,0,c0,c6,6):
+    case HSR_SYSREG(3,0,c0,c6,7):
+    case HSR_SYSREG(3,0,c0,c7,3):
+    case HSR_SYSREG(3,0,c0,c7,4):
+    case HSR_SYSREG(3,0,c0,c7,5):
+    case HSR_SYSREG(3,0,c0,c7,6):
+    case HSR_SYSREG(3,0,c0,c7,7):
+        return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1);
+
     /*
      * HCR_EL2.TIDCP
      *
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v4 6/8] xen/arm: Add handler for cp15 ID registers
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
                       ` (4 preceding siblings ...)
  2020-12-17 15:38     ` [PATCH v4 5/8] xen/arm: Add handler for ID registers on arm64 Bertrand Marquis
@ 2020-12-17 15:38     ` Bertrand Marquis
  2020-12-17 23:37       ` Stefano Stabellini
  2020-12-17 15:38     ` [PATCH v4 7/8] xen/arm: Add CP10 exception support to handle MVFR Bertrand Marquis
                       ` (3 subsequent siblings)
  9 siblings, 1 reply; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-17 15:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

Add support for emulation of cp15 based ID registers (on arm32 or when
running a 32bit guest on arm64).
The handlers are returning the values stored in the guest_cpuinfo
structure for known registers and RAZ for all reserved registers.
In the current status the MVFR registers are no supported.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: Rebase
Changes in V3:
  Add case definition for reserved registers
  Add handling of reserved registers as RAZ.
  Fix code style in GENERATE_TID3_INFO declaration
Changes in V4:
  Fix comment for missing t (no to not)
  Put cases for reserved registers directly in the code instead of using
  a define in the cpregs.h header.

---
 xen/arch/arm/vcpreg.c | 65 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
index cdc91cdf5b..1fe07fe02a 100644
--- a/xen/arch/arm/vcpreg.c
+++ b/xen/arch/arm/vcpreg.c
@@ -155,6 +155,24 @@ 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]);     \
+    }
+
+/* helper to define cases for all registers for one CRm value */
+#define HSR_CPREG32_TID3_CASES(REG)     case HSR_CPREG32(p15,0,c0,REG,0): \
+                                        case HSR_CPREG32(p15,0,c0,REG,1): \
+                                        case HSR_CPREG32(p15,0,c0,REG,2): \
+                                        case HSR_CPREG32(p15,0,c0,REG,3): \
+                                        case HSR_CPREG32(p15,0,c0,REG,4): \
+                                        case HSR_CPREG32(p15,0,c0,REG,5): \
+                                        case HSR_CPREG32(p15,0,c0,REG,6): \
+                                        case HSR_CPREG32(p15,0,c0,REG,7)
+
 void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
 {
     const struct hsr_cp32 cp32 = hsr.cp32;
@@ -286,6 +304,53 @@ 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 not cp15 */
+
+    /*
+     * Those cases are catching all Reserved registers trapped by TID3 which
+     * currently have no assignment.
+     * HCR.TID3 is trapping all registers in the group 3:
+     * coproc == p15, opc1 == 0, CRn == c0, CRm == {c2-c7}, opc2 == {0-7}.
+     * Those registers are defined as being RO in the Arm Architecture
+     * Reference manual Armv8 (Chapter D12.3.2 of issue F.c) so handle them
+     * as Read-only read as zero.
+     */
+    case HSR_CPREG32(p15,0,c0,c3,0):
+    case HSR_CPREG32(p15,0,c0,c3,1):
+    case HSR_CPREG32(p15,0,c0,c3,2):
+    case HSR_CPREG32(p15,0,c0,c3,3):
+    case HSR_CPREG32(p15,0,c0,c3,7):
+    HSR_CPREG32_TID3_CASES(c4):
+    HSR_CPREG32_TID3_CASES(c5):
+    HSR_CPREG32_TID3_CASES(c6):
+    HSR_CPREG32_TID3_CASES(c7):
+        return handle_ro_raz(regs, regidx, cp32.read, hsr, 1);
+
     /*
      * HCR_EL2.TIDCP
      *
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v4 7/8] xen/arm: Add CP10 exception support to handle MVFR
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
                       ` (5 preceding siblings ...)
  2020-12-17 15:38     ` [PATCH v4 6/8] xen/arm: Add handler for cp15 ID registers Bertrand Marquis
@ 2020-12-17 15:38     ` Bertrand Marquis
  2020-12-17 23:40       ` Stefano Stabellini
  2020-12-17 15:38     ` [PATCH v4 8/8] xen/arm: Activate TID3 in HCR_EL2 Bertrand Marquis
                       ` (2 subsequent siblings)
  9 siblings, 1 reply; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-17 15:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

Add support for cp10 exceptions decoding to be able to emulate the
values for MVFR0, MVFR1 and MVFR2 when TID3 bit of HSR is activated.
This is required for aarch32 guests accessing MVFR registers using
vmrs and vmsr instructions.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Changes in V2: Rebase
Changes in V3:
  Add case for MVFR2, fix typo VMFR <-> MVFR.
Changes in V4:
  Fix typo HSR -> HCR
  Move no to not comment fix to previous patch

---
 xen/arch/arm/traps.c             |  5 +++++
 xen/arch/arm/vcpreg.c            | 37 ++++++++++++++++++++++++++++++++
 xen/include/asm-arm/perfc_defn.h |  1 +
 xen/include/asm-arm/traps.h      |  1 +
 4 files changed, 44 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 1fe07fe02a..cbad8f25a0 100644
--- a/xen/arch/arm/vcpreg.c
+++ b/xen/arch/arm/vcpreg.c
@@ -664,6 +664,43 @@ 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 )
+    {
+    /*
+     * HCR.TID3 is trapping access to MVFR register used to identify the
+     * VFP/Simd using VMRS/VMSR instructions.
+     * 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)
+    GENERATE_TID3_INFO(MVFR2, mvfr, 2)
+
+    default:
+        gdprintk(XENLOG_ERR,
+                 "%s p10, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
+                 cp32.read ? "mrc" : "mcr",
+                 cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
+        gdprintk(XENLOG_ERR, "unhandled 32-bit CP10 access %#x\n",
+                 hsr.bits & HSR_CP32_REGS_MASK);
+        inject_undef_exception(regs, hsr);
+        return;
+    }
+
+    advance_pc(regs, hsr);
+}
+
 void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
 {
     const struct hsr_cp cp = hsr.cp;
diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h
index 6a83185163..31f071222b 100644
--- a/xen/include/asm-arm/perfc_defn.h
+++ b/xen/include/asm-arm/perfc_defn.h
@@ -11,6 +11,7 @@ PERFCOUNTER(trap_cp15_64,  "trap: cp15 64-bit access")
 PERFCOUNTER(trap_cp14_32,  "trap: cp14 32-bit access")
 PERFCOUNTER(trap_cp14_64,  "trap: cp14 64-bit access")
 PERFCOUNTER(trap_cp14_dbg, "trap: cp14 dbg access")
+PERFCOUNTER(trap_cp10,     "trap: cp10 access")
 PERFCOUNTER(trap_cp,       "trap: cp access")
 PERFCOUNTER(trap_smc32,    "trap: 32-bit smc")
 PERFCOUNTER(trap_hvc32,    "trap: 32-bit hvc")
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
index 997c37884e..c4a3d0fb1b 100644
--- a/xen/include/asm-arm/traps.h
+++ b/xen/include/asm-arm/traps.h
@@ -62,6 +62,7 @@ void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr);
 void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr);
 void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr);
 void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr);
+void do_cp10(struct cpu_user_regs *regs, const union hsr hsr);
 void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
 
 /* SMCCC handling */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v4 8/8] xen/arm: Activate TID3 in HCR_EL2
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
                       ` (6 preceding siblings ...)
  2020-12-17 15:38     ` [PATCH v4 7/8] xen/arm: Add CP10 exception support to handle MVFR Bertrand Marquis
@ 2020-12-17 15:38     ` Bertrand Marquis
  2020-12-17 23:42       ` Stefano Stabellini
  2020-12-17 23:47     ` [PATCH v4 0/8] xen/arm: Emulate ID registers Stefano Stabellini
  2021-01-05 16:06     ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) Julien Grall
  9 siblings, 1 reply; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-17 15:38 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

Activate TID3 bit in HCR 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
Changes in V3: Rebase
Changes in V4: Rebase

---
 xen/arch/arm/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 28d9d64558..c1a9ad6056 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -98,7 +98,7 @@ register_t get_default_hcr_flags(void)
 {
     return  (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
              (vwfi != NATIVE ? (HCR_TWI|HCR_TWE) : 0) |
-             HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB|HCR_TSW);
+             HCR_TID3|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB|HCR_TSW);
 }
 
 static enum {
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 1/8] xen/arm: Use READ_SYSREG instead of 32/64 versions
  2020-12-17 15:38     ` [PATCH v4 1/8] xen/arm: Use READ_SYSREG instead of 32/64 versions Bertrand Marquis
@ 2020-12-17 23:17       ` Stefano Stabellini
  2020-12-18 10:23         ` Bertrand Marquis
  0 siblings, 1 reply; 35+ messages in thread
From: Stefano Stabellini @ 2020-12-17 23:17 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk

On Thu, 17 Dec 2020, Bertrand Marquis wrote:
> Modify identify_cpu function to use READ_SYSREG instead of READ_SYSREG32
> or READ_SYSREG64.
> The aarch32 versions of the registers are 64bit on an aarch64 processor
> so it was wrong to access them as 32bit registers.

This sentence is a bit confusing because, as an example, MIDR_EL1 is
also an aarch64 register, not only an aarch32 register. Maybe we should
clarify.

Aside from that:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>
> ---
> Change in V4:
>   This patch was introduced in v4.
> 
> ---
>  xen/arch/arm/cpufeature.c | 50 +++++++++++++++++++--------------------
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 44126dbf07..115e1b164d 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -99,44 +99,44 @@ int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps)
>  
>  void identify_cpu(struct cpuinfo_arm *c)
>  {
> -        c->midr.bits = READ_SYSREG32(MIDR_EL1);
> +        c->midr.bits = READ_SYSREG(MIDR_EL1);
>          c->mpidr.bits = READ_SYSREG(MPIDR_EL1);
>  
>  #ifdef CONFIG_ARM_64
> -        c->pfr64.bits[0] = READ_SYSREG64(ID_AA64PFR0_EL1);
> -        c->pfr64.bits[1] = READ_SYSREG64(ID_AA64PFR1_EL1);
> +        c->pfr64.bits[0] = READ_SYSREG(ID_AA64PFR0_EL1);
> +        c->pfr64.bits[1] = READ_SYSREG(ID_AA64PFR1_EL1);
>  
> -        c->dbg64.bits[0] = READ_SYSREG64(ID_AA64DFR0_EL1);
> -        c->dbg64.bits[1] = READ_SYSREG64(ID_AA64DFR1_EL1);
> +        c->dbg64.bits[0] = READ_SYSREG(ID_AA64DFR0_EL1);
> +        c->dbg64.bits[1] = READ_SYSREG(ID_AA64DFR1_EL1);
>  
> -        c->aux64.bits[0] = READ_SYSREG64(ID_AA64AFR0_EL1);
> -        c->aux64.bits[1] = READ_SYSREG64(ID_AA64AFR1_EL1);
> +        c->aux64.bits[0] = READ_SYSREG(ID_AA64AFR0_EL1);
> +        c->aux64.bits[1] = READ_SYSREG(ID_AA64AFR1_EL1);
>  
> -        c->mm64.bits[0]  = READ_SYSREG64(ID_AA64MMFR0_EL1);
> -        c->mm64.bits[1]  = READ_SYSREG64(ID_AA64MMFR1_EL1);
> +        c->mm64.bits[0]  = READ_SYSREG(ID_AA64MMFR0_EL1);
> +        c->mm64.bits[1]  = READ_SYSREG(ID_AA64MMFR1_EL1);
>  
> -        c->isa64.bits[0] = READ_SYSREG64(ID_AA64ISAR0_EL1);
> -        c->isa64.bits[1] = READ_SYSREG64(ID_AA64ISAR1_EL1);
> +        c->isa64.bits[0] = READ_SYSREG(ID_AA64ISAR0_EL1);
> +        c->isa64.bits[1] = READ_SYSREG(ID_AA64ISAR1_EL1);
>  #endif
>  
> -        c->pfr32.bits[0] = READ_SYSREG32(ID_PFR0_EL1);
> -        c->pfr32.bits[1] = READ_SYSREG32(ID_PFR1_EL1);
> +        c->pfr32.bits[0] = READ_SYSREG(ID_PFR0_EL1);
> +        c->pfr32.bits[1] = READ_SYSREG(ID_PFR1_EL1);
>  
> -        c->dbg32.bits[0] = READ_SYSREG32(ID_DFR0_EL1);
> +        c->dbg32.bits[0] = READ_SYSREG(ID_DFR0_EL1);
>  
> -        c->aux32.bits[0] = READ_SYSREG32(ID_AFR0_EL1);
> +        c->aux32.bits[0] = READ_SYSREG(ID_AFR0_EL1);
>  
> -        c->mm32.bits[0]  = READ_SYSREG32(ID_MMFR0_EL1);
> -        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[0]  = READ_SYSREG(ID_MMFR0_EL1);
> +        c->mm32.bits[1]  = READ_SYSREG(ID_MMFR1_EL1);
> +        c->mm32.bits[2]  = READ_SYSREG(ID_MMFR2_EL1);
> +        c->mm32.bits[3]  = READ_SYSREG(ID_MMFR3_EL1);
>  
> -        c->isa32.bits[0] = READ_SYSREG32(ID_ISAR0_EL1);
> -        c->isa32.bits[1] = READ_SYSREG32(ID_ISAR1_EL1);
> -        c->isa32.bits[2] = READ_SYSREG32(ID_ISAR2_EL1);
> -        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[0] = READ_SYSREG(ID_ISAR0_EL1);
> +        c->isa32.bits[1] = READ_SYSREG(ID_ISAR1_EL1);
> +        c->isa32.bits[2] = READ_SYSREG(ID_ISAR2_EL1);
> +        c->isa32.bits[3] = READ_SYSREG(ID_ISAR3_EL1);
> +        c->isa32.bits[4] = READ_SYSREG(ID_ISAR4_EL1);
> +        c->isa32.bits[5] = READ_SYSREG(ID_ISAR5_EL1);
>  }
>  
>  /*
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 2/8] xen/arm: Add ID registers and complete cpuinfo
  2020-12-17 15:38     ` [PATCH v4 2/8] xen/arm: Add ID registers and complete cpuinfo Bertrand Marquis
@ 2020-12-17 23:22       ` Stefano Stabellini
  0 siblings, 0 replies; 35+ messages in thread
From: Stefano Stabellini @ 2020-12-17 23:22 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk

On Thu, 17 Dec 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 MVFRx_EL1 defines 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: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes in V2:
>   Fix dbg32 table size and add proper initialisation of the second entry
>   of the table by reading ID_DFR1 register.
> Changes in V3:
>   Fix typo in commit title
>   Add MVFR2 definition and handling on aarch32 and remove specific case
>   for mvfr field in cpuinfo (now the same on arm64 and arm32).
>   Add MMFR4 definition if not known by the compiler.
> Changes in V4:
>   Add MVFRx_EL1 defines for aarch32
>   Use READ_SYSREG instead of 32/64 versions of the function which
>   removed the ifdef case for MVFR access.
>   User register_t type for mvfr and zfr64 fields of cpuinfo structure.
> 
> ---
>  xen/arch/arm/cpufeature.c           | 12 +++++++
>  xen/include/asm-arm/arm64/sysregs.h | 28 +++++++++++++++
>  xen/include/asm-arm/cpregs.h        | 15 ++++++++
>  xen/include/asm-arm/cpufeature.h    | 56 ++++++++++++++++++++++++-----
>  4 files changed, 102 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 115e1b164d..86b99ee960 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_SYSREG(ID_AA64MMFR0_EL1);
>          c->mm64.bits[1]  = READ_SYSREG(ID_AA64MMFR1_EL1);
> +        c->mm64.bits[2]  = READ_SYSREG(ID_AA64MMFR2_EL1);
>  
>          c->isa64.bits[0] = READ_SYSREG(ID_AA64ISAR0_EL1);
>          c->isa64.bits[1] = READ_SYSREG(ID_AA64ISAR1_EL1);
> +
> +        c->zfr64.bits[0] = READ_SYSREG(ID_AA64ZFR0_EL1);
>  #endif
>  
>          c->pfr32.bits[0] = READ_SYSREG(ID_PFR0_EL1);
>          c->pfr32.bits[1] = READ_SYSREG(ID_PFR1_EL1);
> +        c->pfr32.bits[2] = READ_SYSREG(ID_PFR2_EL1);
>  
>          c->dbg32.bits[0] = READ_SYSREG(ID_DFR0_EL1);
> +        c->dbg32.bits[1] = READ_SYSREG(ID_DFR1_EL1);
>  
>          c->aux32.bits[0] = READ_SYSREG(ID_AFR0_EL1);
>  
> @@ -130,6 +135,8 @@ void identify_cpu(struct cpuinfo_arm *c)
>          c->mm32.bits[1]  = READ_SYSREG(ID_MMFR1_EL1);
>          c->mm32.bits[2]  = READ_SYSREG(ID_MMFR2_EL1);
>          c->mm32.bits[3]  = READ_SYSREG(ID_MMFR3_EL1);
> +        c->mm32.bits[4]  = READ_SYSREG(ID_MMFR4_EL1);
> +        c->mm32.bits[5]  = READ_SYSREG(ID_MMFR5_EL1);
>  
>          c->isa32.bits[0] = READ_SYSREG(ID_ISAR0_EL1);
>          c->isa32.bits[1] = READ_SYSREG(ID_ISAR1_EL1);
> @@ -137,6 +144,11 @@ void identify_cpu(struct cpuinfo_arm *c)
>          c->isa32.bits[3] = READ_SYSREG(ID_ISAR3_EL1);
>          c->isa32.bits[4] = READ_SYSREG(ID_ISAR4_EL1);
>          c->isa32.bits[5] = READ_SYSREG(ID_ISAR5_EL1);
> +        c->isa32.bits[6] = READ_SYSREG(ID_ISAR6_EL1);
> +
> +        c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
> +        c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
> +        c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
>  }
>  
>  /*
> diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
> index c60029d38f..077fd95fb7 100644
> --- a/xen/include/asm-arm/arm64/sysregs.h
> +++ b/xen/include/asm-arm/arm64/sysregs.h
> @@ -57,6 +57,34 @@
>  #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_MMFR4_EL1
> +#define ID_MMFR4_EL1                S3_0_C0_C2_6
> +#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..6daf2b1a30 100644
> --- a/xen/include/asm-arm/cpregs.h
> +++ b/xen/include/asm-arm/cpregs.h
> @@ -63,6 +63,8 @@
>  #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 MVFR2           p10,7,c5,c0,0   /* Media and VFP Feature Register 2 */
>  #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 +110,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 +319,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
> @@ -347,6 +359,9 @@
>  #define VPIDR_EL2               VPIDR
>  #define VTCR_EL2                VTCR
>  #define VTTBR_EL2               VTTBR
> +#define MVFR0_EL1               MVFR0
> +#define MVFR1_EL1               MVFR1
> +#define MVFR2_EL1               MVFR2
>  #endif
>  
>  #endif
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index c7b5052992..74139be1cc 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 {
> +        register_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,16 @@ struct cpuinfo_arm {
>      } aux32;
>  
>      struct {
> -        uint32_t bits[4];
> +        uint32_t bits[6];
>      } mm32;
>  
>      struct {
> -        uint32_t bits[6];
> +        uint32_t bits[7];
>      } isa32;
> +
> +    struct {
> +        register_t bits[3];
> +    } mvfr;
>  };
>  
>  extern struct cpuinfo_arm boot_cpu_data;
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 3/8] xen/arm: Add arm64 ID registers definitions
  2020-12-17 15:38     ` [PATCH v4 3/8] xen/arm: Add arm64 ID registers definitions Bertrand Marquis
@ 2020-12-17 23:24       ` Stefano Stabellini
  0 siblings, 0 replies; 35+ messages in thread
From: Stefano Stabellini @ 2020-12-17 23:24 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk

On Thu, 17 Dec 2020, Bertrand Marquis wrote:
> 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: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes in V2: Rebase
> Changes in V3:
>   Add case definition for reserved registers.
> Changes in V4:
>   Remove case definition for reserved registers and move it to the code
>   directly.
> 
> ---
>  xen/include/asm-arm/arm64/hsr.h | 37 +++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/xen/include/asm-arm/arm64/hsr.h b/xen/include/asm-arm/arm64/hsr.h
> index ca931dd2fe..e691d41c17 100644
> --- a/xen/include/asm-arm/arm64/hsr.h
> +++ b/xen/include/asm-arm/arm64/hsr.h
> @@ -110,6 +110,43 @@
>  #define HSR_SYSREG_CNTP_CTL_EL0   HSR_SYSREG(3,3,c14,c2,1)
>  #define HSR_SYSREG_CNTP_CVAL_EL0  HSR_SYSREG(3,3,c14,c2,2)
>  
> +/* Those registers are used when HCR_EL2.TID3 is set */
> +#define HSR_SYSREG_ID_PFR0_EL1    HSR_SYSREG(3,0,c0,c1,0)
> +#define HSR_SYSREG_ID_PFR1_EL1    HSR_SYSREG(3,0,c0,c1,1)
> +#define HSR_SYSREG_ID_PFR2_EL1    HSR_SYSREG(3,0,c0,c3,4)
> +#define HSR_SYSREG_ID_DFR0_EL1    HSR_SYSREG(3,0,c0,c1,2)
> +#define HSR_SYSREG_ID_DFR1_EL1    HSR_SYSREG(3,0,c0,c3,5)
> +#define HSR_SYSREG_ID_AFR0_EL1    HSR_SYSREG(3,0,c0,c1,3)
> +#define HSR_SYSREG_ID_MMFR0_EL1   HSR_SYSREG(3,0,c0,c1,4)
> +#define HSR_SYSREG_ID_MMFR1_EL1   HSR_SYSREG(3,0,c0,c1,5)
> +#define HSR_SYSREG_ID_MMFR2_EL1   HSR_SYSREG(3,0,c0,c1,6)
> +#define HSR_SYSREG_ID_MMFR3_EL1   HSR_SYSREG(3,0,c0,c1,7)
> +#define HSR_SYSREG_ID_MMFR4_EL1   HSR_SYSREG(3,0,c0,c2,6)
> +#define HSR_SYSREG_ID_MMFR5_EL1   HSR_SYSREG(3,0,c0,c3,6)
> +#define HSR_SYSREG_ID_ISAR0_EL1   HSR_SYSREG(3,0,c0,c2,0)
> +#define HSR_SYSREG_ID_ISAR1_EL1   HSR_SYSREG(3,0,c0,c2,1)
> +#define HSR_SYSREG_ID_ISAR2_EL1   HSR_SYSREG(3,0,c0,c2,2)
> +#define HSR_SYSREG_ID_ISAR3_EL1   HSR_SYSREG(3,0,c0,c2,3)
> +#define HSR_SYSREG_ID_ISAR4_EL1   HSR_SYSREG(3,0,c0,c2,4)
> +#define HSR_SYSREG_ID_ISAR5_EL1   HSR_SYSREG(3,0,c0,c2,5)
> +#define HSR_SYSREG_ID_ISAR6_EL1   HSR_SYSREG(3,0,c0,c2,7)
> +#define HSR_SYSREG_MVFR0_EL1      HSR_SYSREG(3,0,c0,c3,0)
> +#define HSR_SYSREG_MVFR1_EL1      HSR_SYSREG(3,0,c0,c3,1)
> +#define HSR_SYSREG_MVFR2_EL1      HSR_SYSREG(3,0,c0,c3,2)
> +
> +#define HSR_SYSREG_ID_AA64PFR0_EL1   HSR_SYSREG(3,0,c0,c4,0)
> +#define HSR_SYSREG_ID_AA64PFR1_EL1   HSR_SYSREG(3,0,c0,c4,1)
> +#define HSR_SYSREG_ID_AA64DFR0_EL1   HSR_SYSREG(3,0,c0,c5,0)
> +#define HSR_SYSREG_ID_AA64DFR1_EL1   HSR_SYSREG(3,0,c0,c5,1)
> +#define HSR_SYSREG_ID_AA64ISAR0_EL1  HSR_SYSREG(3,0,c0,c6,0)
> +#define HSR_SYSREG_ID_AA64ISAR1_EL1  HSR_SYSREG(3,0,c0,c6,1)
> +#define HSR_SYSREG_ID_AA64MMFR0_EL1  HSR_SYSREG(3,0,c0,c7,0)
> +#define HSR_SYSREG_ID_AA64MMFR1_EL1  HSR_SYSREG(3,0,c0,c7,1)
> +#define HSR_SYSREG_ID_AA64MMFR2_EL1  HSR_SYSREG(3,0,c0,c7,2)
> +#define HSR_SYSREG_ID_AA64AFR0_EL1   HSR_SYSREG(3,0,c0,c5,4)
> +#define HSR_SYSREG_ID_AA64AFR1_EL1   HSR_SYSREG(3,0,c0,c5,5)
> +#define HSR_SYSREG_ID_AA64ZFR0_EL1   HSR_SYSREG(3,0,c0,c4,4)
> +
>  #endif /* __ASM_ARM_ARM64_HSR_H */
>  
>  /*
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 4/8] xen/arm: create a cpuinfo structure for guest
  2020-12-17 15:38     ` [PATCH v4 4/8] xen/arm: create a cpuinfo structure for guest Bertrand Marquis
@ 2020-12-17 23:26       ` Stefano Stabellini
  0 siblings, 0 replies; 35+ messages in thread
From: Stefano Stabellini @ 2020-12-17 23:26 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk

On Thu, 17 Dec 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).
> Modify some values in the guest cpuinfo structure to guests to hide some
> processor features:
> - SVE as this is not supported by Xen and guest are not allowed to use
> this features (ZEN is set to 0 in CPTR_EL2).
> - AMU as HCPTR_TAM is set in CPTR_EL2 so AMU cannot be used by guests
> All other bits are left untouched.
> - RAS as this is not supported by Xen.
> 
> 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>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes in V2: Rebase
> Changes in V3:
>   Use current_cpu_data info instead of recalling identify_cpu
> Changes in V4:
>   Use boot_cpu_data instead of current_cpu_data
>   Use "hide XX support" instead of disable as this part of the code is
>   actually only hidding feature to guests but not disabling them (this
>   is done through the HCR register).
>   Modify commit message to be more clear about what is done in
>   guest_cpuinfo.
> 
> ---
>  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 86b99ee960..1f6a85aafe 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)
>  {
> @@ -151,6 +153,55 @@ void identify_cpu(struct cpuinfo_arm *c)
>          c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
>  }
>  
> +/*
> + * 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.
> +     */
> +    guest_cpuinfo = boot_cpu_data;
> +
> +#ifdef CONFIG_ARM_64
> +    /* Hide MPAM support as xen does not support it */
> +    guest_cpuinfo.pfr64.mpam = 0;
> +    guest_cpuinfo.pfr64.mpam_frac = 0;
> +
> +    /* Hide SVE as Xen does not support it */
> +    guest_cpuinfo.pfr64.sve = 0;
> +    guest_cpuinfo.zfr64.bits[0] = 0;
> +
> +    /* Hide MTE support as Xen does not support it */
> +    guest_cpuinfo.pfr64.mte = 0;
> +#endif
> +
> +    /* Hide AMU support */
> +#ifdef CONFIG_ARM_64
> +    guest_cpuinfo.pfr64.amu = 0;
> +#endif
> +    guest_cpuinfo.pfr32.amu = 0;
> +
> +    /* Hide RAS support 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 74139be1cc..6058744c18 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -283,6 +283,8 @@ extern void identify_cpu(struct cpuinfo_arm *);
>  extern struct cpuinfo_arm cpu_data[];
>  #define current_cpu_data cpu_data[smp_processor_id()]
>  
> +extern struct cpuinfo_arm guest_cpuinfo;
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 5/8] xen/arm: Add handler for ID registers on arm64
  2020-12-17 15:38     ` [PATCH v4 5/8] xen/arm: Add handler for ID registers on arm64 Bertrand Marquis
@ 2020-12-17 23:31       ` Stefano Stabellini
  0 siblings, 0 replies; 35+ messages in thread
From: Stefano Stabellini @ 2020-12-17 23:31 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk

On Thu, 17 Dec 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 know registers and is handling reserved registers as RAZ.
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes in V2: Rebase
> Changes in V3:
>   Fix commit message
>   Fix code style for GENERATE_TID3_INFO declaration
>   Add handling of reserved registers as RAZ.
> Changes in V4:
>   Fix indentation in GENERATE_TID3_INFO macro
>   Add explicit case code for reserved registers
> 
> ---
>  xen/arch/arm/arm64/vsysreg.c | 82 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
> index 8a85507d9d..41f18612c6 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,80 @@ 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)
> +
> +    /*
> +     * Those cases are catching all Reserved registers trapped by TID3 which
> +     * currently have no assignment.
> +     * HCR.TID3 is trapping all registers in the group 3:
> +     * Op0 == 3, op1 == 0, CRn == c0,CRm == {c1-c7}, op2 == {0-7}.
> +     * Those registers are defined as being RO in the Arm Architecture
> +     * Reference manual Armv8 (Chapter D12.3.2 of issue F.c) so handle them
> +     * as Read-only read as zero.
> +     */
> +    case HSR_SYSREG(3,0,c0,c3,3):
> +    case HSR_SYSREG(3,0,c0,c3,7):
> +    case HSR_SYSREG(3,0,c0,c4,2):
> +    case HSR_SYSREG(3,0,c0,c4,3):
> +    case HSR_SYSREG(3,0,c0,c4,5):
> +    case HSR_SYSREG(3,0,c0,c4,6):
> +    case HSR_SYSREG(3,0,c0,c4,7):
> +    case HSR_SYSREG(3,0,c0,c5,2):
> +    case HSR_SYSREG(3,0,c0,c5,3):
> +    case HSR_SYSREG(3,0,c0,c5,6):
> +    case HSR_SYSREG(3,0,c0,c5,7):
> +    case HSR_SYSREG(3,0,c0,c6,2):
> +    case HSR_SYSREG(3,0,c0,c6,3):
> +    case HSR_SYSREG(3,0,c0,c6,4):
> +    case HSR_SYSREG(3,0,c0,c6,5):
> +    case HSR_SYSREG(3,0,c0,c6,6):
> +    case HSR_SYSREG(3,0,c0,c6,7):
> +    case HSR_SYSREG(3,0,c0,c7,3):
> +    case HSR_SYSREG(3,0,c0,c7,4):
> +    case HSR_SYSREG(3,0,c0,c7,5):
> +    case HSR_SYSREG(3,0,c0,c7,6):
> +    case HSR_SYSREG(3,0,c0,c7,7):
> +        return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1);
> +
>      /*
>       * HCR_EL2.TIDCP
>       *
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 6/8] xen/arm: Add handler for cp15 ID registers
  2020-12-17 15:38     ` [PATCH v4 6/8] xen/arm: Add handler for cp15 ID registers Bertrand Marquis
@ 2020-12-17 23:37       ` Stefano Stabellini
  2020-12-18 10:14         ` Bertrand Marquis
  0 siblings, 1 reply; 35+ messages in thread
From: Stefano Stabellini @ 2020-12-17 23:37 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk

On Thu, 17 Dec 2020, Bertrand Marquis wrote:
> 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 for known registers and RAZ for all reserved registers.
> In the current status the MVFR registers are no supported.
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
> Changes in V2: Rebase
> Changes in V3:
>   Add case definition for reserved registers
>   Add handling of reserved registers as RAZ.
>   Fix code style in GENERATE_TID3_INFO declaration
> Changes in V4:
>   Fix comment for missing t (no to not)
>   Put cases for reserved registers directly in the code instead of using
>   a define in the cpregs.h header.
> 
> ---
>  xen/arch/arm/vcpreg.c | 65 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> index cdc91cdf5b..1fe07fe02a 100644
> --- a/xen/arch/arm/vcpreg.c
> +++ b/xen/arch/arm/vcpreg.c
> @@ -155,6 +155,24 @@ 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]);     \

This line is misaligned, but it can be adjusted on commit

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>



> +    }
> +
> +/* helper to define cases for all registers for one CRm value */
> +#define HSR_CPREG32_TID3_CASES(REG)     case HSR_CPREG32(p15,0,c0,REG,0): \
> +                                        case HSR_CPREG32(p15,0,c0,REG,1): \
> +                                        case HSR_CPREG32(p15,0,c0,REG,2): \
> +                                        case HSR_CPREG32(p15,0,c0,REG,3): \
> +                                        case HSR_CPREG32(p15,0,c0,REG,4): \
> +                                        case HSR_CPREG32(p15,0,c0,REG,5): \
> +                                        case HSR_CPREG32(p15,0,c0,REG,6): \
> +                                        case HSR_CPREG32(p15,0,c0,REG,7)
> +
>  void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>  {
>      const struct hsr_cp32 cp32 = hsr.cp32;
> @@ -286,6 +304,53 @@ 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 not cp15 */
> +
> +    /*
> +     * Those cases are catching all Reserved registers trapped by TID3 which
> +     * currently have no assignment.
> +     * HCR.TID3 is trapping all registers in the group 3:
> +     * coproc == p15, opc1 == 0, CRn == c0, CRm == {c2-c7}, opc2 == {0-7}.
> +     * Those registers are defined as being RO in the Arm Architecture
> +     * Reference manual Armv8 (Chapter D12.3.2 of issue F.c) so handle them
> +     * as Read-only read as zero.
> +     */
> +    case HSR_CPREG32(p15,0,c0,c3,0):
> +    case HSR_CPREG32(p15,0,c0,c3,1):
> +    case HSR_CPREG32(p15,0,c0,c3,2):
> +    case HSR_CPREG32(p15,0,c0,c3,3):
> +    case HSR_CPREG32(p15,0,c0,c3,7):
> +    HSR_CPREG32_TID3_CASES(c4):
> +    HSR_CPREG32_TID3_CASES(c5):
> +    HSR_CPREG32_TID3_CASES(c6):
> +    HSR_CPREG32_TID3_CASES(c7):
> +        return handle_ro_raz(regs, regidx, cp32.read, hsr, 1);
> +
>      /*
>       * HCR_EL2.TIDCP
>       *
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 7/8] xen/arm: Add CP10 exception support to handle MVFR
  2020-12-17 15:38     ` [PATCH v4 7/8] xen/arm: Add CP10 exception support to handle MVFR Bertrand Marquis
@ 2020-12-17 23:40       ` Stefano Stabellini
  0 siblings, 0 replies; 35+ messages in thread
From: Stefano Stabellini @ 2020-12-17 23:40 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk

On Thu, 17 Dec 2020, Bertrand Marquis wrote:
> Add support for cp10 exceptions decoding to be able to emulate the
> values for MVFR0, MVFR1 and MVFR2 when TID3 bit of HSR is activated.
> This is required for aarch32 guests accessing MVFR registers using
> vmrs and vmsr instructions.
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes in V2: Rebase
> Changes in V3:
>   Add case for MVFR2, fix typo VMFR <-> MVFR.
> Changes in V4:
>   Fix typo HSR -> HCR
>   Move no to not comment fix to previous patch
> 
> ---
>  xen/arch/arm/traps.c             |  5 +++++
>  xen/arch/arm/vcpreg.c            | 37 ++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/perfc_defn.h |  1 +
>  xen/include/asm-arm/traps.h      |  1 +
>  4 files changed, 44 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 1fe07fe02a..cbad8f25a0 100644
> --- a/xen/arch/arm/vcpreg.c
> +++ b/xen/arch/arm/vcpreg.c
> @@ -664,6 +664,43 @@ 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 )
> +    {
> +    /*
> +     * HCR.TID3 is trapping access to MVFR register used to identify the
> +     * VFP/Simd using VMRS/VMSR instructions.
> +     * 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)
> +    GENERATE_TID3_INFO(MVFR2, mvfr, 2)
> +
> +    default:
> +        gdprintk(XENLOG_ERR,
> +                 "%s p10, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
> +                 cp32.read ? "mrc" : "mcr",
> +                 cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
> +        gdprintk(XENLOG_ERR, "unhandled 32-bit CP10 access %#x\n",
> +                 hsr.bits & HSR_CP32_REGS_MASK);
> +        inject_undef_exception(regs, hsr);
> +        return;
> +    }
> +
> +    advance_pc(regs, hsr);
> +}
> +
>  void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
>  {
>      const struct hsr_cp cp = hsr.cp;
> diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h
> index 6a83185163..31f071222b 100644
> --- a/xen/include/asm-arm/perfc_defn.h
> +++ b/xen/include/asm-arm/perfc_defn.h
> @@ -11,6 +11,7 @@ PERFCOUNTER(trap_cp15_64,  "trap: cp15 64-bit access")
>  PERFCOUNTER(trap_cp14_32,  "trap: cp14 32-bit access")
>  PERFCOUNTER(trap_cp14_64,  "trap: cp14 64-bit access")
>  PERFCOUNTER(trap_cp14_dbg, "trap: cp14 dbg access")
> +PERFCOUNTER(trap_cp10,     "trap: cp10 access")
>  PERFCOUNTER(trap_cp,       "trap: cp access")
>  PERFCOUNTER(trap_smc32,    "trap: 32-bit smc")
>  PERFCOUNTER(trap_hvc32,    "trap: 32-bit hvc")
> diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
> index 997c37884e..c4a3d0fb1b 100644
> --- a/xen/include/asm-arm/traps.h
> +++ b/xen/include/asm-arm/traps.h
> @@ -62,6 +62,7 @@ void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr);
>  void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr);
>  void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr);
>  void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr);
> +void do_cp10(struct cpu_user_regs *regs, const union hsr hsr);
>  void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
>  
>  /* SMCCC handling */
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 8/8] xen/arm: Activate TID3 in HCR_EL2
  2020-12-17 15:38     ` [PATCH v4 8/8] xen/arm: Activate TID3 in HCR_EL2 Bertrand Marquis
@ 2020-12-17 23:42       ` Stefano Stabellini
  0 siblings, 0 replies; 35+ messages in thread
From: Stefano Stabellini @ 2020-12-17 23:42 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk

On Thu, 17 Dec 2020, Bertrand Marquis wrote:
> Activate TID3 bit in HCR 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>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes in V2: Rebase
> Changes in V3: Rebase
> Changes in V4: Rebase
> 
> ---
>  xen/arch/arm/traps.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 28d9d64558..c1a9ad6056 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -98,7 +98,7 @@ register_t get_default_hcr_flags(void)
>  {
>      return  (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
>               (vwfi != NATIVE ? (HCR_TWI|HCR_TWE) : 0) |
> -             HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB|HCR_TSW);
> +             HCR_TID3|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB|HCR_TSW);
>  }
>  
>  static enum {
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 0/8] xen/arm: Emulate ID registers
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
                       ` (7 preceding siblings ...)
  2020-12-17 15:38     ` [PATCH v4 8/8] xen/arm: Activate TID3 in HCR_EL2 Bertrand Marquis
@ 2020-12-17 23:47     ` Stefano Stabellini
  2020-12-18 10:12       ` Bertrand Marquis
  2021-01-05 16:06     ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) Julien Grall
  9 siblings, 1 reply; 35+ messages in thread
From: Stefano Stabellini @ 2020-12-17 23:47 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
	andrew.cooper3, george.dunlap, iwj, wl

On Thu, 17 Dec 2020, Bertrand Marquis wrote:
> 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.

As per our discussion I think we want to add this to the series.

---

xen/arm: clarify support status for various ARMv8.x CPUs

ARMv8.1+ is not security supported for now, as it would require more
investigation on hardware features that Xen has to hide from the guest.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>

diff --git a/SUPPORT.md b/SUPPORT.md
index ab02aca5f4..d95ce3a411 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -37,7 +37,8 @@ supported in this document.
 
 ### ARM v8
 
-    Status: Supported
+    Status, ARMv8.0: Supported
+    Status, ARMv8.1+: Supported, not security supported
     Status, Cortex A57 r0p0-r1p1: Supported, not security supported
 
 For the Cortex A57 r0p0 - r1p1, see Errata 832075.


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 0/8] xen/arm: Emulate ID registers
  2020-12-17 23:47     ` [PATCH v4 0/8] xen/arm: Emulate ID registers Stefano Stabellini
@ 2020-12-18 10:12       ` Bertrand Marquis
  0 siblings, 0 replies; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-18 10:12 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Xen-devel, Julien Grall, Volodymyr Babchuk, andrew.cooper3,
	george.dunlap, iwj, wl

Hi Stefano,

> On 17 Dec 2020, at 23:47, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Thu, 17 Dec 2020, Bertrand Marquis wrote:
>> 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.
> 
> As per our discussion I think we want to add this to the series.

Fully agree.

> 
> ---
> 
> xen/arm: clarify support status for various ARMv8.x CPUs
> 
> ARMv8.1+ is not security supported for now, as it would require more
> investigation on hardware features that Xen has to hide from the guest.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> 
> diff --git a/SUPPORT.md b/SUPPORT.md
> index ab02aca5f4..d95ce3a411 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -37,7 +37,8 @@ supported in this document.
> 
> ### ARM v8
> 
> -    Status: Supported
> +    Status, ARMv8.0: Supported
> +    Status, ARMv8.1+: Supported, not security supported
>     Status, Cortex A57 r0p0-r1p1: Supported, not security supported
> 
> For the Cortex A57 r0p0 - r1p1, see Errata 832075.



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 6/8] xen/arm: Add handler for cp15 ID registers
  2020-12-17 23:37       ` Stefano Stabellini
@ 2020-12-18 10:14         ` Bertrand Marquis
  0 siblings, 0 replies; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-18 10:14 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Julien Grall, Volodymyr Babchuk

Hi Stefano,

> On 17 Dec 2020, at 23:37, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Thu, 17 Dec 2020, Bertrand Marquis wrote:
>> 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 for known registers and RAZ for all reserved registers.
>> In the current status the MVFR registers are no supported.
>> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> Changes in V2: Rebase
>> Changes in V3:
>>  Add case definition for reserved registers
>>  Add handling of reserved registers as RAZ.
>>  Fix code style in GENERATE_TID3_INFO declaration
>> Changes in V4:
>>  Fix comment for missing t (no to not)
>>  Put cases for reserved registers directly in the code instead of using
>>  a define in the cpregs.h header.
>> 
>> ---
>> xen/arch/arm/vcpreg.c | 65 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 65 insertions(+)
>> 
>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>> index cdc91cdf5b..1fe07fe02a 100644
>> --- a/xen/arch/arm/vcpreg.c
>> +++ b/xen/arch/arm/vcpreg.c
>> @@ -155,6 +155,24 @@ 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]);     \
> 
> This line is misaligned, but it can be adjusted on commit

Oh yes this was spotted by Julien on the previous serie and I forgot to fix it.

> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 

Thanks
Bertrand

> 
> 
>> +    }
>> +
>> +/* helper to define cases for all registers for one CRm value */
>> +#define HSR_CPREG32_TID3_CASES(REG)     case HSR_CPREG32(p15,0,c0,REG,0): \
>> +                                        case HSR_CPREG32(p15,0,c0,REG,1): \
>> +                                        case HSR_CPREG32(p15,0,c0,REG,2): \
>> +                                        case HSR_CPREG32(p15,0,c0,REG,3): \
>> +                                        case HSR_CPREG32(p15,0,c0,REG,4): \
>> +                                        case HSR_CPREG32(p15,0,c0,REG,5): \
>> +                                        case HSR_CPREG32(p15,0,c0,REG,6): \
>> +                                        case HSR_CPREG32(p15,0,c0,REG,7)
>> +
>> void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>> {
>>     const struct hsr_cp32 cp32 = hsr.cp32;
>> @@ -286,6 +304,53 @@ 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 not cp15 */
>> +
>> +    /*
>> +     * Those cases are catching all Reserved registers trapped by TID3 which
>> +     * currently have no assignment.
>> +     * HCR.TID3 is trapping all registers in the group 3:
>> +     * coproc == p15, opc1 == 0, CRn == c0, CRm == {c2-c7}, opc2 == {0-7}.
>> +     * Those registers are defined as being RO in the Arm Architecture
>> +     * Reference manual Armv8 (Chapter D12.3.2 of issue F.c) so handle them
>> +     * as Read-only read as zero.
>> +     */
>> +    case HSR_CPREG32(p15,0,c0,c3,0):
>> +    case HSR_CPREG32(p15,0,c0,c3,1):
>> +    case HSR_CPREG32(p15,0,c0,c3,2):
>> +    case HSR_CPREG32(p15,0,c0,c3,3):
>> +    case HSR_CPREG32(p15,0,c0,c3,7):
>> +    HSR_CPREG32_TID3_CASES(c4):
>> +    HSR_CPREG32_TID3_CASES(c5):
>> +    HSR_CPREG32_TID3_CASES(c6):
>> +    HSR_CPREG32_TID3_CASES(c7):
>> +        return handle_ro_raz(regs, regidx, cp32.read, hsr, 1);
>> +
>>     /*
>>      * HCR_EL2.TIDCP
>>      *
>> -- 
>> 2.17.1



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 1/8] xen/arm: Use READ_SYSREG instead of 32/64 versions
  2020-12-17 23:17       ` Stefano Stabellini
@ 2020-12-18 10:23         ` Bertrand Marquis
  0 siblings, 0 replies; 35+ messages in thread
From: Bertrand Marquis @ 2020-12-18 10:23 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, Julien Grall, Volodymyr Babchuk

Hi Stefano,

> On 17 Dec 2020, at 23:17, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Thu, 17 Dec 2020, Bertrand Marquis wrote:
>> Modify identify_cpu function to use READ_SYSREG instead of READ_SYSREG32
>> or READ_SYSREG64.
>> The aarch32 versions of the registers are 64bit on an aarch64 processor
>> so it was wrong to access them as 32bit registers.
> 
> This sentence is a bit confusing because, as an example, MIDR_EL1 is
> also an aarch64 register, not only an aarch32 register. Maybe we should
> clarify.

You are right the sentence is not very clear.

Maybe the following would be better:

All aarch32 specific registers (for example ID_PFR0_EL1) are 64bit when
accessed from aarch64 with upper bits read as 0, so it is right to access them
as 64bit registers on a 64bit platform.

> 
> Aside from that:
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 

Thanks

Cheers
Bertrand

> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> 
>> ---
>> Change in V4:
>>  This patch was introduced in v4.
>> 
>> ---
>> xen/arch/arm/cpufeature.c | 50 +++++++++++++++++++--------------------
>> 1 file changed, 25 insertions(+), 25 deletions(-)
>> 
>> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
>> index 44126dbf07..115e1b164d 100644
>> --- a/xen/arch/arm/cpufeature.c
>> +++ b/xen/arch/arm/cpufeature.c
>> @@ -99,44 +99,44 @@ int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps)
>> 
>> void identify_cpu(struct cpuinfo_arm *c)
>> {
>> -        c->midr.bits = READ_SYSREG32(MIDR_EL1);
>> +        c->midr.bits = READ_SYSREG(MIDR_EL1);
>>         c->mpidr.bits = READ_SYSREG(MPIDR_EL1);
>> 
>> #ifdef CONFIG_ARM_64
>> -        c->pfr64.bits[0] = READ_SYSREG64(ID_AA64PFR0_EL1);
>> -        c->pfr64.bits[1] = READ_SYSREG64(ID_AA64PFR1_EL1);
>> +        c->pfr64.bits[0] = READ_SYSREG(ID_AA64PFR0_EL1);
>> +        c->pfr64.bits[1] = READ_SYSREG(ID_AA64PFR1_EL1);
>> 
>> -        c->dbg64.bits[0] = READ_SYSREG64(ID_AA64DFR0_EL1);
>> -        c->dbg64.bits[1] = READ_SYSREG64(ID_AA64DFR1_EL1);
>> +        c->dbg64.bits[0] = READ_SYSREG(ID_AA64DFR0_EL1);
>> +        c->dbg64.bits[1] = READ_SYSREG(ID_AA64DFR1_EL1);
>> 
>> -        c->aux64.bits[0] = READ_SYSREG64(ID_AA64AFR0_EL1);
>> -        c->aux64.bits[1] = READ_SYSREG64(ID_AA64AFR1_EL1);
>> +        c->aux64.bits[0] = READ_SYSREG(ID_AA64AFR0_EL1);
>> +        c->aux64.bits[1] = READ_SYSREG(ID_AA64AFR1_EL1);
>> 
>> -        c->mm64.bits[0]  = READ_SYSREG64(ID_AA64MMFR0_EL1);
>> -        c->mm64.bits[1]  = READ_SYSREG64(ID_AA64MMFR1_EL1);
>> +        c->mm64.bits[0]  = READ_SYSREG(ID_AA64MMFR0_EL1);
>> +        c->mm64.bits[1]  = READ_SYSREG(ID_AA64MMFR1_EL1);
>> 
>> -        c->isa64.bits[0] = READ_SYSREG64(ID_AA64ISAR0_EL1);
>> -        c->isa64.bits[1] = READ_SYSREG64(ID_AA64ISAR1_EL1);
>> +        c->isa64.bits[0] = READ_SYSREG(ID_AA64ISAR0_EL1);
>> +        c->isa64.bits[1] = READ_SYSREG(ID_AA64ISAR1_EL1);
>> #endif
>> 
>> -        c->pfr32.bits[0] = READ_SYSREG32(ID_PFR0_EL1);
>> -        c->pfr32.bits[1] = READ_SYSREG32(ID_PFR1_EL1);
>> +        c->pfr32.bits[0] = READ_SYSREG(ID_PFR0_EL1);
>> +        c->pfr32.bits[1] = READ_SYSREG(ID_PFR1_EL1);
>> 
>> -        c->dbg32.bits[0] = READ_SYSREG32(ID_DFR0_EL1);
>> +        c->dbg32.bits[0] = READ_SYSREG(ID_DFR0_EL1);
>> 
>> -        c->aux32.bits[0] = READ_SYSREG32(ID_AFR0_EL1);
>> +        c->aux32.bits[0] = READ_SYSREG(ID_AFR0_EL1);
>> 
>> -        c->mm32.bits[0]  = READ_SYSREG32(ID_MMFR0_EL1);
>> -        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[0]  = READ_SYSREG(ID_MMFR0_EL1);
>> +        c->mm32.bits[1]  = READ_SYSREG(ID_MMFR1_EL1);
>> +        c->mm32.bits[2]  = READ_SYSREG(ID_MMFR2_EL1);
>> +        c->mm32.bits[3]  = READ_SYSREG(ID_MMFR3_EL1);
>> 
>> -        c->isa32.bits[0] = READ_SYSREG32(ID_ISAR0_EL1);
>> -        c->isa32.bits[1] = READ_SYSREG32(ID_ISAR1_EL1);
>> -        c->isa32.bits[2] = READ_SYSREG32(ID_ISAR2_EL1);
>> -        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[0] = READ_SYSREG(ID_ISAR0_EL1);
>> +        c->isa32.bits[1] = READ_SYSREG(ID_ISAR1_EL1);
>> +        c->isa32.bits[2] = READ_SYSREG(ID_ISAR2_EL1);
>> +        c->isa32.bits[3] = READ_SYSREG(ID_ISAR3_EL1);
>> +        c->isa32.bits[4] = READ_SYSREG(ID_ISAR4_EL1);
>> +        c->isa32.bits[5] = READ_SYSREG(ID_ISAR5_EL1);
>> }
>> 
>> /*
>> -- 
>> 2.17.1
>> 



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers)
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
                       ` (8 preceding siblings ...)
  2020-12-17 23:47     ` [PATCH v4 0/8] xen/arm: Emulate ID registers Stefano Stabellini
@ 2021-01-05 16:06     ` Julien Grall
  2021-01-05 16:28       ` André Przywara
  9 siblings, 1 reply; 35+ messages in thread
From: Julien Grall @ 2021-01-05 16:06 UTC (permalink / raw)
  To: Bertrand Marquis, xen-devel
  Cc: Stefano Stabellini, André Przywara, Ian Jackson

(+ Ian and Andre)

Hi Bertrand,

On IRC, Ian pointed out that the smoke test was failing on Cubietruck. 
The only patches because the last success and the failure are your series.

This seems to be a very early failure as there is no output from Xen [1].

I originally thought the problem was because some of the ID registers 
(such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.

But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be 
RAZ. So it would result to a crash. Andre confirmed that PFR2 can be 
accessed by writing a small baremetal code and booted on Cortex-A7 and 
Cortex-A20.

So I am not entirely sure what's the problem. Andre kindly accepted to 
try to boot Xen on his board. Hopefully, this will give us a clue on the 
problem.

If not, I will borrow a Cubietruck in OssTest and see if I can reproduce 
it and debug it.

Cheers,

On 17/12/2020 15:38, Bertrand Marquis wrote:
> 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.
> 
> Changes in V3:
>    Add handling of reserved registers as RAZ
>    Minor fixes described in each patch
> 
> Changes in V4:
>    Add a patch to switch implementation to use READ_SYSREG instead of the
>    32/64 bit version of it.
>    Move cases for reserved register handling from macros to the code
>    itself.
>    Various typos fixes.
> 
> Bertrand Marquis (8):
>    xen/arm: Use READ_SYSREG instead of 32/64 versions
>    xen/arm: Add ID registers and complete cpuinfo
>    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 MVFR
>    xen/arm: Activate TID3 in HCR_EL2
> 
>   xen/arch/arm/arm64/vsysreg.c        |  82 ++++++++++++++++++++
>   xen/arch/arm/cpufeature.c           | 113 ++++++++++++++++++++++------
>   xen/arch/arm/traps.c                |   7 +-
>   xen/arch/arm/vcpreg.c               | 102 +++++++++++++++++++++++++
>   xen/include/asm-arm/arm64/hsr.h     |  37 +++++++++
>   xen/include/asm-arm/arm64/sysregs.h |  28 +++++++
>   xen/include/asm-arm/cpregs.h        |  15 ++++
>   xen/include/asm-arm/cpufeature.h    |  58 +++++++++++---
>   xen/include/asm-arm/perfc_defn.h    |   1 +
>   xen/include/asm-arm/traps.h         |   1 +
>   10 files changed, 409 insertions(+), 35 deletions(-)
> 

[1] 
http://logs.test-lab.xenproject.org/osstest/logs/158152/test-armhf-armhf-xl/info.html

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers)
  2021-01-05 16:06     ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) Julien Grall
@ 2021-01-05 16:28       ` André Przywara
  2021-01-05 18:44         ` Stefano Stabellini
  0 siblings, 1 reply; 35+ messages in thread
From: André Przywara @ 2021-01-05 16:28 UTC (permalink / raw)
  To: Julien Grall, Bertrand Marquis, xen-devel; +Cc: Stefano Stabellini, Ian Jackson

On 05/01/2021 16:06, Julien Grall wrote:
> (+ Ian and Andre)
> 
> Hi Bertrand,
> 
> On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
> The only patches because the last success and the failure are your series.
> 
> This seems to be a very early failure as there is no output from Xen [1].
> 
> I originally thought the problem was because some of the ID registers
> (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
> 
> But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
> RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
> accessed by writing a small baremetal code and booted on Cortex-A7 and
> Cortex-A20.
> 
> So I am not entirely sure what's the problem. Andre kindly accepted to
> try to boot Xen on his board. Hopefully, this will give us a clue on the
> problem.
> 
> If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
> it and debug it.


So I just compiled master and staging and ran just that on an Allwinner
H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
missing Dom0, as expected). However staging indeed fails:

(XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
(arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y  Tue Jan  5
16:09:40 GMT 2021
(XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
(XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
(XEN) CPU0: Unexpected Trap: Undefined Instruction
(XEN) ----[ Xen-4.15-unstable  arm32  debug=y   Not tainted ]----
(XEN) CPU:    0
(XEN) PC:     0026b8c8 identify_cpu+0xc0/0xd4
(XEN) CPSR:   600001da MODE:Hypervisor
(XEN)      R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
(XEN)      R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
(XEN)      R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c R12:00000080
(XEN) HYP: SP: 002f7e68 LR: 002c419c
(XEN)
(XEN)   VTCR_EL2: 80002646
(XEN)  VTTBR_EL2: 00000018e628bb80
(XEN)
(XEN)  SCTLR_EL2: 30cd187f
(XEN)    HCR_EL2: 00000038
(XEN)  TTBR0_EL2: 000000004013a000
(XEN)
(XEN)    ESR_EL2: 00000000
(XEN)  HPFAR_EL2: 0003fff0
(XEN)      HDFAR: 9d110000
(XEN)      HIFAR: 0000a04a
(XEN)
(XEN) Xen stack trace from sp=002f7e68:
(XEN)    00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
00000000
(XEN)    00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
00000001
(XEN)    00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
50000000
(XEN)    4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
00000000
(XEN)    50000000 00000000 50000000 00000000 00000000 00000000 00000000
00000000
(XEN)    00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
00000000
(XEN)    00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
49ff5000
(XEN)    40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
(XEN)    00000000 00000000 00000000 00000000 00000000 00000000
(XEN) Xen call trace:
(XEN)    [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
(XEN)    [<002c419c>] start_xen+0x778/0xe50 (LR)
(XEN)    [<002f7f54>] 002f7f54
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) CPU0: Unexpected Trap: Undefined Instruction
(XEN) ****************************************
(XEN)
(XEN) Reboot in five seconds...


The code in question:
  26b8c0:       eef63a10        vmrs    r3, mvfr1
  26b8c4:       e5803058        str     r3, [r0, #88]   ; 0x58
> 26b8c8:       eef53a10        vmrs    r3, mvfr2
  26b8cc:       e580305c        str     r3, [r0, #92]   ; 0x5c
  26b8d0:       e28bd000        add     sp, fp, #0
  26b8d4:       e49db004        pop     {fp}       ; (ldr fp, [sp], #4)
  26b8d8:       e12fff1e        bx      lr

And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
the CP15 CPUID registers this is using the VMRS instruction, so it's not
protected by future-proof CPUID register scheme.

Not sure what to do about this, maybe #ifdef'ing this register access
with arm64?
I guess this comes from the slightly too optimistic code-sharing between
arm32 and arm64?

Cheers,
Andre

> On 17/12/2020 15:38, Bertrand Marquis wrote:
>> 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.
>>
>> Changes in V3:
>>    Add handling of reserved registers as RAZ
>>    Minor fixes described in each patch
>>
>> Changes in V4:
>>    Add a patch to switch implementation to use READ_SYSREG instead of the
>>    32/64 bit version of it.
>>    Move cases for reserved register handling from macros to the code
>>    itself.
>>    Various typos fixes.
>>
>> Bertrand Marquis (8):
>>    xen/arm: Use READ_SYSREG instead of 32/64 versions
>>    xen/arm: Add ID registers and complete cpuinfo
>>    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 MVFR
>>    xen/arm: Activate TID3 in HCR_EL2
>>
>>   xen/arch/arm/arm64/vsysreg.c        |  82 ++++++++++++++++++++
>>   xen/arch/arm/cpufeature.c           | 113 ++++++++++++++++++++++------
>>   xen/arch/arm/traps.c                |   7 +-
>>   xen/arch/arm/vcpreg.c               | 102 +++++++++++++++++++++++++
>>   xen/include/asm-arm/arm64/hsr.h     |  37 +++++++++
>>   xen/include/asm-arm/arm64/sysregs.h |  28 +++++++
>>   xen/include/asm-arm/cpregs.h        |  15 ++++
>>   xen/include/asm-arm/cpufeature.h    |  58 +++++++++++---
>>   xen/include/asm-arm/perfc_defn.h    |   1 +
>>   xen/include/asm-arm/traps.h         |   1 +
>>   10 files changed, 409 insertions(+), 35 deletions(-)
>>
> 
> [1]
> http://logs.test-lab.xenproject.org/osstest/logs/158152/test-armhf-armhf-xl/info.html
> 
> 



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers)
  2021-01-05 16:28       ` André Przywara
@ 2021-01-05 18:44         ` Stefano Stabellini
  2021-01-05 19:05           ` [PATCH] xen/arm: do not read MVFR2 on arm32 Stefano Stabellini
  2021-01-05 19:15           ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) Julien Grall
  0 siblings, 2 replies; 35+ messages in thread
From: Stefano Stabellini @ 2021-01-05 18:44 UTC (permalink / raw)
  To: André Przywara
  Cc: Julien Grall, Bertrand Marquis, xen-devel, Stefano Stabellini,
	Ian Jackson

[-- Attachment #1: Type: text/plain, Size: 5742 bytes --]

On Tue, 5 Jan 2021, André Przywara wrote:
> On 05/01/2021 16:06, Julien Grall wrote:
> > (+ Ian and Andre)
> > 
> > Hi Bertrand,
> > 
> > On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
> > The only patches because the last success and the failure are your series.
> > 
> > This seems to be a very early failure as there is no output from Xen [1].
> > 
> > I originally thought the problem was because some of the ID registers
> > (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
> > 
> > But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
> > RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
> > accessed by writing a small baremetal code and booted on Cortex-A7 and
> > Cortex-A20.
> > 
> > So I am not entirely sure what's the problem. Andre kindly accepted to
> > try to boot Xen on his board. Hopefully, this will give us a clue on the
> > problem.
> > 
> > If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
> > it and debug it.
> 
> 
> So I just compiled master and staging and ran just that on an Allwinner
> H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
> missing Dom0, as expected). However staging indeed fails:
> 
> (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
> (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y  Tue Jan  5
> 16:09:40 GMT 2021
> (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
> (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
> (XEN) CPU0: Unexpected Trap: Undefined Instruction
> (XEN) ----[ Xen-4.15-unstable  arm32  debug=y   Not tainted ]----
> (XEN) CPU:    0
> (XEN) PC:     0026b8c8 identify_cpu+0xc0/0xd4
> (XEN) CPSR:   600001da MODE:Hypervisor
> (XEN)      R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
> (XEN)      R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
> (XEN)      R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c R12:00000080
> (XEN) HYP: SP: 002f7e68 LR: 002c419c
> (XEN)
> (XEN)   VTCR_EL2: 80002646
> (XEN)  VTTBR_EL2: 00000018e628bb80
> (XEN)
> (XEN)  SCTLR_EL2: 30cd187f
> (XEN)    HCR_EL2: 00000038
> (XEN)  TTBR0_EL2: 000000004013a000
> (XEN)
> (XEN)    ESR_EL2: 00000000
> (XEN)  HPFAR_EL2: 0003fff0
> (XEN)      HDFAR: 9d110000
> (XEN)      HIFAR: 0000a04a
> (XEN)
> (XEN) Xen stack trace from sp=002f7e68:
> (XEN)    00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
> 00000000
> (XEN)    00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
> 00000001
> (XEN)    00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
> 50000000
> (XEN)    4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
> 00000000
> (XEN)    50000000 00000000 50000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN)    00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
> 00000000
> (XEN)    00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
> 49ff5000
> (XEN)    40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
> 00000000
> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
> (XEN) Xen call trace:
> (XEN)    [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
> (XEN)    [<002c419c>] start_xen+0x778/0xe50 (LR)
> (XEN)    [<002f7f54>] 002f7f54
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) CPU0: Unexpected Trap: Undefined Instruction
> (XEN) ****************************************
> (XEN)
> (XEN) Reboot in five seconds...
> 
> 
> The code in question:
>   26b8c0:       eef63a10        vmrs    r3, mvfr1
>   26b8c4:       e5803058        str     r3, [r0, #88]   ; 0x58
> > 26b8c8:       eef53a10        vmrs    r3, mvfr2
>   26b8cc:       e580305c        str     r3, [r0, #92]   ; 0x5c
>   26b8d0:       e28bd000        add     sp, fp, #0
>   26b8d4:       e49db004        pop     {fp}       ; (ldr fp, [sp], #4)
>   26b8d8:       e12fff1e        bx      lr
> 
> And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
> the CP15 CPUID registers this is using the VMRS instruction, so it's not
> protected by future-proof CPUID register scheme.
> 
> Not sure what to do about this, maybe #ifdef'ing this register access
> with arm64?
> I guess this comes from the slightly too optimistic code-sharing between
> arm32 and arm64?

Yes and #ifdef'ing is what we have been doing with the other registers.
It looks OK in cpufeature.c; it looks ugly in cpufeature.h but I
couldn't come up with something nicer at the moment.

diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 1f6a85aafe..9e3377eae3 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -150,7 +150,9 @@ void identify_cpu(struct cpuinfo_arm *c)
 
         c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
         c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
+#ifdef CONFIG_ARM_64
         c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
+#endif
 }
 
 /*
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 6058744c18..29a63a91c8 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -271,9 +271,15 @@ struct cpuinfo_arm {
         uint32_t bits[7];
     } isa32;
 
+#ifdef CONFIG_ARM_64
     struct {
         register_t bits[3];
     } mvfr;
+#else
+    struct {
+        register_t bits[2];
+    } mvfr;
+#endif
 };
 
 extern struct cpuinfo_arm boot_cpu_data;

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH] xen/arm: do not read MVFR2 on arm32
  2021-01-05 18:44         ` Stefano Stabellini
@ 2021-01-05 19:05           ` Stefano Stabellini
  2021-01-05 19:20             ` Julien Grall
  2021-01-05 19:15           ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) Julien Grall
  1 sibling, 1 reply; 35+ messages in thread
From: Stefano Stabellini @ 2021-01-05 19:05 UTC (permalink / raw)
  To: xen-devel
  Cc: sstabellini, julien, bertrand.marquis, iwj, Andrew.Cooper3,
	Volodymyr_Babchuk, Stefano Stabellini

MVFR2 is not available on arm32. Don't try to read it. Make MVFR2 arm64
only.

Fixes: 9cfdb489af81 ("xen/arm: Add ID registers and complete cpuinfo")
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/arch/arm/cpufeature.c        | 2 ++
 xen/include/asm-arm/cpufeature.h | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 1f6a85aafe..9e3377eae3 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -150,7 +150,9 @@ void identify_cpu(struct cpuinfo_arm *c)
 
         c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
         c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
+#ifdef CONFIG_ARM_64
         c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
+#endif
 }
 
 /*
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 6058744c18..fa20cb493a 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -272,7 +272,7 @@ struct cpuinfo_arm {
     } isa32;
 
     struct {
-        register_t bits[3];
+        register_t bits[2 + IS_ENABLED(CONFIG_ARM_64)];
     } mvfr;
 };
 
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers)
  2021-01-05 18:44         ` Stefano Stabellini
  2021-01-05 19:05           ` [PATCH] xen/arm: do not read MVFR2 on arm32 Stefano Stabellini
@ 2021-01-05 19:15           ` Julien Grall
  2021-01-05 22:43             ` Stefano Stabellini
  1 sibling, 1 reply; 35+ messages in thread
From: Julien Grall @ 2021-01-05 19:15 UTC (permalink / raw)
  To: Stefano Stabellini, André Przywara
  Cc: Bertrand Marquis, xen-devel, Ian Jackson

Hi Stefano,

On 05/01/2021 18:44, Stefano Stabellini wrote:
> On Tue, 5 Jan 2021, André Przywara wrote:
>> On 05/01/2021 16:06, Julien Grall wrote:
>>> (+ Ian and Andre)
>>>
>>> Hi Bertrand,
>>>
>>> On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
>>> The only patches because the last success and the failure are your series.
>>>
>>> This seems to be a very early failure as there is no output from Xen [1].
>>>
>>> I originally thought the problem was because some of the ID registers
>>> (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
>>>
>>> But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
>>> RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
>>> accessed by writing a small baremetal code and booted on Cortex-A7 and
>>> Cortex-A20.
>>>
>>> So I am not entirely sure what's the problem. Andre kindly accepted to
>>> try to boot Xen on his board. Hopefully, this will give us a clue on the
>>> problem.
>>>
>>> If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
>>> it and debug it.
>>
>>
>> So I just compiled master and staging and ran just that on an Allwinner
>> H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
>> missing Dom0, as expected). However staging indeed fails:
>>
>> (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
>> (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y  Tue Jan  5
>> 16:09:40 GMT 2021
>> (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
>> (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>> (XEN) ----[ Xen-4.15-unstable  arm32  debug=y   Not tainted ]----
>> (XEN) CPU:    0
>> (XEN) PC:     0026b8c8 identify_cpu+0xc0/0xd4
>> (XEN) CPSR:   600001da MODE:Hypervisor
>> (XEN)      R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
>> (XEN)      R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
>> (XEN)      R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c R12:00000080
>> (XEN) HYP: SP: 002f7e68 LR: 002c419c
>> (XEN)
>> (XEN)   VTCR_EL2: 80002646
>> (XEN)  VTTBR_EL2: 00000018e628bb80
>> (XEN)
>> (XEN)  SCTLR_EL2: 30cd187f
>> (XEN)    HCR_EL2: 00000038
>> (XEN)  TTBR0_EL2: 000000004013a000
>> (XEN)
>> (XEN)    ESR_EL2: 00000000
>> (XEN)  HPFAR_EL2: 0003fff0
>> (XEN)      HDFAR: 9d110000
>> (XEN)      HIFAR: 0000a04a
>> (XEN)
>> (XEN) Xen stack trace from sp=002f7e68:
>> (XEN)    00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
>> 00000000
>> (XEN)    00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
>> 00000001
>> (XEN)    00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
>> 50000000
>> (XEN)    4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
>> 00000000
>> (XEN)    50000000 00000000 50000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN)    00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
>> 00000000
>> (XEN)    00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
>> 49ff5000
>> (XEN)    40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
>> 00000000
>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
>> (XEN) Xen call trace:
>> (XEN)    [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
>> (XEN)    [<002c419c>] start_xen+0x778/0xe50 (LR)
>> (XEN)    [<002f7f54>] 002f7f54
>> (XEN)
>> (XEN)
>> (XEN) ****************************************
>> (XEN) Panic on CPU 0:
>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>> (XEN) ****************************************
>> (XEN)
>> (XEN) Reboot in five seconds...
>>
>>
>> The code in question:
>>    26b8c0:       eef63a10        vmrs    r3, mvfr1
>>    26b8c4:       e5803058        str     r3, [r0, #88]   ; 0x58
>>> 26b8c8:       eef53a10        vmrs    r3, mvfr2
>>    26b8cc:       e580305c        str     r3, [r0, #92]   ; 0x5c
>>    26b8d0:       e28bd000        add     sp, fp, #0
>>    26b8d4:       e49db004        pop     {fp}       ; (ldr fp, [sp], #4)
>>    26b8d8:       e12fff1e        bx      lr
>>
>> And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
>> the CP15 CPUID registers this is using the VMRS instruction, so it's not
>> protected by future-proof CPUID register scheme.
>>
>> Not sure what to do about this, maybe #ifdef'ing this register access
>> with arm64?
>> I guess this comes from the slightly too optimistic code-sharing between
>> arm32 and arm64?
> 
> Yes and #ifdef'ing is what we have been doing with the other registers.

There is a catch here. This register is accessible from AArch32 on all 
Armv8 HW. It is just not accessible on Armv7.

So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not correct.

I know that we said that we don't officially support Xen on Arm32 on 
Armv8 HW (I can't find where it is written though). So we could argue 
that shadowing MVFR2 is not worth it.

I do use Armv8 HW to test 32-bit, so I would at least like to get Xen 
booting. In addition to that, Linux 32-bit doesn't access MVFR2 at the 
moment.

Therefore, a #ifdef may be acceptable for now. However, I would suggest 
to introduce name it MAY_BE_UNDEFINED (or similar) that will be used to 
avoid reading the system register on 32-bit.

For the 32-bit case, I would just hardcode the value based on the Arm 
(it looks like for Armv8-A there is only one valid value).

IOW, the hack would be self-contained in cpufeature.c.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] xen/arm: do not read MVFR2 on arm32
  2021-01-05 19:05           ` [PATCH] xen/arm: do not read MVFR2 on arm32 Stefano Stabellini
@ 2021-01-05 19:20             ` Julien Grall
  0 siblings, 0 replies; 35+ messages in thread
From: Julien Grall @ 2021-01-05 19:20 UTC (permalink / raw)
  To: Stefano Stabellini, xen-devel
  Cc: bertrand.marquis, iwj, Andrew.Cooper3, Volodymyr_Babchuk,
	Stefano Stabellini



On 05/01/2021 19:05, Stefano Stabellini wrote:
> MVFR2 is not available on arm32. Don't try to read it. Make MVFR2 arm64
> only.

Not really, MVFR2 is allocated when running in AArch32 mode on Armv8. It 
just doesn't exist on Armv7. See my answer your previous e-mail for more 
details.

> 
> Fixes: 9cfdb489af81 ("xen/arm: Add ID registers and complete cpuinfo")
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> ---
>   xen/arch/arm/cpufeature.c        | 2 ++
>   xen/include/asm-arm/cpufeature.h | 2 +-
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 1f6a85aafe..9e3377eae3 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -150,7 +150,9 @@ void identify_cpu(struct cpuinfo_arm *c)
>   
>           c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
>           c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
> +#ifdef CONFIG_ARM_64
>           c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
> +#endif
>   }
>   
>   /*
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 6058744c18..fa20cb493a 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -272,7 +272,7 @@ struct cpuinfo_arm {
>       } isa32;
>   
>       struct {
> -        register_t bits[3];
> +        register_t bits[2 + IS_ENABLED(CONFIG_ARM_64)];

mvfr.bits[2] will be accessed from arch/arm/vcpreg.c, so you will 
provide garbagge to guest if the user happen to run Xen on Arm32 on Armv8.

Given that MVFR2 exists, I think it would be best to keep the definition 
in cpuinfo_arm around and only hardcode the value in cpufeature.c.

Please see my previous e-mail for more rationale on this approach.


Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers)
  2021-01-05 19:15           ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) Julien Grall
@ 2021-01-05 22:43             ` Stefano Stabellini
  2021-01-06 18:29               ` Julien Grall
  0 siblings, 1 reply; 35+ messages in thread
From: Stefano Stabellini @ 2021-01-05 22:43 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, André Przywara, Bertrand Marquis,
	xen-devel, Ian Jackson

[-- Attachment #1: Type: text/plain, Size: 7199 bytes --]

On Tue, 5 Jan 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 05/01/2021 18:44, Stefano Stabellini wrote:
> > On Tue, 5 Jan 2021, André Przywara wrote:
> > > On 05/01/2021 16:06, Julien Grall wrote:
> > > > (+ Ian and Andre)
> > > > 
> > > > Hi Bertrand,
> > > > 
> > > > On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
> > > > The only patches because the last success and the failure are your
> > > > series.
> > > > 
> > > > This seems to be a very early failure as there is no output from Xen
> > > > [1].
> > > > 
> > > > I originally thought the problem was because some of the ID registers
> > > > (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
> > > > 
> > > > But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
> > > > RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
> > > > accessed by writing a small baremetal code and booted on Cortex-A7 and
> > > > Cortex-A20.
> > > > 
> > > > So I am not entirely sure what's the problem. Andre kindly accepted to
> > > > try to boot Xen on his board. Hopefully, this will give us a clue on the
> > > > problem.
> > > > 
> > > > If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
> > > > it and debug it.
> > > 
> > > 
> > > So I just compiled master and staging and ran just that on an Allwinner
> > > H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
> > > missing Dom0, as expected). However staging indeed fails:
> > > 
> > > (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
> > > (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y  Tue Jan  5
> > > 16:09:40 GMT 2021
> > > (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
> > > (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
> > > (XEN) CPU0: Unexpected Trap: Undefined Instruction
> > > (XEN) ----[ Xen-4.15-unstable  arm32  debug=y   Not tainted ]----
> > > (XEN) CPU:    0
> > > (XEN) PC:     0026b8c8 identify_cpu+0xc0/0xd4
> > > (XEN) CPSR:   600001da MODE:Hypervisor
> > > (XEN)      R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
> > > (XEN)      R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
> > > (XEN)      R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c
> > > R12:00000080
> > > (XEN) HYP: SP: 002f7e68 LR: 002c419c
> > > (XEN)
> > > (XEN)   VTCR_EL2: 80002646
> > > (XEN)  VTTBR_EL2: 00000018e628bb80
> > > (XEN)
> > > (XEN)  SCTLR_EL2: 30cd187f
> > > (XEN)    HCR_EL2: 00000038
> > > (XEN)  TTBR0_EL2: 000000004013a000
> > > (XEN)
> > > (XEN)    ESR_EL2: 00000000
> > > (XEN)  HPFAR_EL2: 0003fff0
> > > (XEN)      HDFAR: 9d110000
> > > (XEN)      HIFAR: 0000a04a
> > > (XEN)
> > > (XEN) Xen stack trace from sp=002f7e68:
> > > (XEN)    00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
> > > 00000000
> > > (XEN)    00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
> > > 00000001
> > > (XEN)    00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
> > > 50000000
> > > (XEN)    4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
> > > 00000000
> > > (XEN)    50000000 00000000 50000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN)    00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
> > > 00000000
> > > (XEN)    00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
> > > 49ff5000
> > > (XEN)    40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
> > > 00000000
> > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
> > > (XEN) Xen call trace:
> > > (XEN)    [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
> > > (XEN)    [<002c419c>] start_xen+0x778/0xe50 (LR)
> > > (XEN)    [<002f7f54>] 002f7f54
> > > (XEN)
> > > (XEN)
> > > (XEN) ****************************************
> > > (XEN) Panic on CPU 0:
> > > (XEN) CPU0: Unexpected Trap: Undefined Instruction
> > > (XEN) ****************************************
> > > (XEN)
> > > (XEN) Reboot in five seconds...
> > > 
> > > 
> > > The code in question:
> > >    26b8c0:       eef63a10        vmrs    r3, mvfr1
> > >    26b8c4:       e5803058        str     r3, [r0, #88]   ; 0x58
> > > > 26b8c8:       eef53a10        vmrs    r3, mvfr2
> > >    26b8cc:       e580305c        str     r3, [r0, #92]   ; 0x5c
> > >    26b8d0:       e28bd000        add     sp, fp, #0
> > >    26b8d4:       e49db004        pop     {fp}       ; (ldr fp, [sp], #4)
> > >    26b8d8:       e12fff1e        bx      lr
> > > 
> > > And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
> > > the CP15 CPUID registers this is using the VMRS instruction, so it's not
> > > protected by future-proof CPUID register scheme.
> > > 
> > > Not sure what to do about this, maybe #ifdef'ing this register access
> > > with arm64?
> > > I guess this comes from the slightly too optimistic code-sharing between
> > > arm32 and arm64?
> > 
> > Yes and #ifdef'ing is what we have been doing with the other registers.
> 
> There is a catch here. This register is accessible from AArch32 on all Armv8
> HW. It is just not accessible on Armv7.
> 
> So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not correct.
> 
> I know that we said that we don't officially support Xen on Arm32 on Armv8 HW
> (I can't find where it is written though). So we could argue that shadowing
> MVFR2 is not worth it.
> 
> I do use Armv8 HW to test 32-bit, so I would at least like to get Xen booting.

Yep, me too.


> In addition to that, Linux 32-bit doesn't access MVFR2 at the moment.
>
> Therefore, a #ifdef may be acceptable for now. However, I would suggest to
> introduce name it MAY_BE_UNDEFINED (or similar) that will be used to avoid
> reading the system register on 32-bit.
> 
> For the 32-bit case, I would just hardcode the value based on the Arm (it
> looks like for Armv8-A there is only one valid value).
> 
> IOW, the hack would be self-contained in cpufeature.c.

I think it makes sense that the hack should be self-contained in
cpufeature.c, leaving the definition of struct mvfr to 3 register_t also
on arm32 as it is today. Also leaving vcpreg.c as it is today so that a
guest can try to read mvfr2 without crashing thanks to
GENERATE_TID3_INFO(MVFR2, mvfr, 2).

For the arm32 case in cpufeature.c:identify_cpu, the two permitted
values are 0 and 0b0100, which one did you have in mind? I take you
meant 0 which stands for "not implemented, or no support for
miscellaneous features"?


In regards to:
> I would suggest to introduce name it MAY_BE_UNDEFINED (or similar)
> that will be used to avoid reading the system register on 32-bit.

Did you mean something like the following on arm32 (maybe to
xen/include/asm-arm/arm32/sysregs.h):

#define MVFR2_MAYBE_UNDEFINED


Then in identify_cpu:

#ifndef MVFR2_MAYBE_UNDEFINED
    c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
#endif

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers)
  2021-01-05 22:43             ` Stefano Stabellini
@ 2021-01-06 18:29               ` Julien Grall
  2021-01-06 20:55                 ` Stefano Stabellini
  0 siblings, 1 reply; 35+ messages in thread
From: Julien Grall @ 2021-01-06 18:29 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: André Przywara, Bertrand Marquis, xen-devel, Ian Jackson

Hi Stefano,

On 05/01/2021 22:43, Stefano Stabellini wrote:
> On Tue, 5 Jan 2021, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 05/01/2021 18:44, Stefano Stabellini wrote:
>>> On Tue, 5 Jan 2021, André Przywara wrote:
>>>> On 05/01/2021 16:06, Julien Grall wrote:
>>>>> (+ Ian and Andre)
>>>>>
>>>>> Hi Bertrand,
>>>>>
>>>>> On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
>>>>> The only patches because the last success and the failure are your
>>>>> series.
>>>>>
>>>>> This seems to be a very early failure as there is no output from Xen
>>>>> [1].
>>>>>
>>>>> I originally thought the problem was because some of the ID registers
>>>>> (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
>>>>>
>>>>> But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
>>>>> RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
>>>>> accessed by writing a small baremetal code and booted on Cortex-A7 and
>>>>> Cortex-A20.
>>>>>
>>>>> So I am not entirely sure what's the problem. Andre kindly accepted to
>>>>> try to boot Xen on his board. Hopefully, this will give us a clue on the
>>>>> problem.
>>>>>
>>>>> If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
>>>>> it and debug it.
>>>>
>>>>
>>>> So I just compiled master and staging and ran just that on an Allwinner
>>>> H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
>>>> missing Dom0, as expected). However staging indeed fails:
>>>>
>>>> (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
>>>> (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y  Tue Jan  5
>>>> 16:09:40 GMT 2021
>>>> (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
>>>> (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
>>>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>>> (XEN) ----[ Xen-4.15-unstable  arm32  debug=y   Not tainted ]----
>>>> (XEN) CPU:    0
>>>> (XEN) PC:     0026b8c8 identify_cpu+0xc0/0xd4
>>>> (XEN) CPSR:   600001da MODE:Hypervisor
>>>> (XEN)      R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
>>>> (XEN)      R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
>>>> (XEN)      R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c
>>>> R12:00000080
>>>> (XEN) HYP: SP: 002f7e68 LR: 002c419c
>>>> (XEN)
>>>> (XEN)   VTCR_EL2: 80002646
>>>> (XEN)  VTTBR_EL2: 00000018e628bb80
>>>> (XEN)
>>>> (XEN)  SCTLR_EL2: 30cd187f
>>>> (XEN)    HCR_EL2: 00000038
>>>> (XEN)  TTBR0_EL2: 000000004013a000
>>>> (XEN)
>>>> (XEN)    ESR_EL2: 00000000
>>>> (XEN)  HPFAR_EL2: 0003fff0
>>>> (XEN)      HDFAR: 9d110000
>>>> (XEN)      HIFAR: 0000a04a
>>>> (XEN)
>>>> (XEN) Xen stack trace from sp=002f7e68:
>>>> (XEN)    00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
>>>> 00000000
>>>> (XEN)    00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
>>>> 00000001
>>>> (XEN)    00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
>>>> 50000000
>>>> (XEN)    4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
>>>> 00000000
>>>> (XEN)    50000000 00000000 50000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN)    00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
>>>> 00000000
>>>> (XEN)    00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
>>>> 49ff5000
>>>> (XEN)    40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
>>>> (XEN) Xen call trace:
>>>> (XEN)    [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
>>>> (XEN)    [<002c419c>] start_xen+0x778/0xe50 (LR)
>>>> (XEN)    [<002f7f54>] 002f7f54
>>>> (XEN)
>>>> (XEN)
>>>> (XEN) ****************************************
>>>> (XEN) Panic on CPU 0:
>>>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>>> (XEN) ****************************************
>>>> (XEN)
>>>> (XEN) Reboot in five seconds...
>>>>
>>>>
>>>> The code in question:
>>>>     26b8c0:       eef63a10        vmrs    r3, mvfr1
>>>>     26b8c4:       e5803058        str     r3, [r0, #88]   ; 0x58
>>>>> 26b8c8:       eef53a10        vmrs    r3, mvfr2
>>>>     26b8cc:       e580305c        str     r3, [r0, #92]   ; 0x5c
>>>>     26b8d0:       e28bd000        add     sp, fp, #0
>>>>     26b8d4:       e49db004        pop     {fp}       ; (ldr fp, [sp], #4)
>>>>     26b8d8:       e12fff1e        bx      lr
>>>>
>>>> And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
>>>> the CP15 CPUID registers this is using the VMRS instruction, so it's not
>>>> protected by future-proof CPUID register scheme.
>>>>
>>>> Not sure what to do about this, maybe #ifdef'ing this register access
>>>> with arm64?
>>>> I guess this comes from the slightly too optimistic code-sharing between
>>>> arm32 and arm64?
>>>
>>> Yes and #ifdef'ing is what we have been doing with the other registers.
>>
>> There is a catch here. This register is accessible from AArch32 on all Armv8
>> HW. It is just not accessible on Armv7.
>>
>> So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not correct.
>>
>> I know that we said that we don't officially support Xen on Arm32 on Armv8 HW
>> (I can't find where it is written though). So we could argue that shadowing
>> MVFR2 is not worth it.
>>
>> I do use Armv8 HW to test 32-bit, so I would at least like to get Xen booting.
> 
> Yep, me too.
> 
> 
>> In addition to that, Linux 32-bit doesn't access MVFR2 at the moment.
>>
>> Therefore, a #ifdef may be acceptable for now. However, I would suggest to
>> introduce name it MAY_BE_UNDEFINED (or similar) that will be used to avoid
>> reading the system register on 32-bit.
>>
>> For the 32-bit case, I would just hardcode the value based on the Arm (it
>> looks like for Armv8-A there is only one valid value).
>>
>> IOW, the hack would be self-contained in cpufeature.c.
> 
> I think it makes sense that the hack should be self-contained in
> cpufeature.c, leaving the definition of struct mvfr to 3 register_t also
> on arm32 as it is today. Also leaving vcpreg.c as it is today so that a
> guest can try to read mvfr2 without crashing thanks to
> GENERATE_TID3_INFO(MVFR2, mvfr, 2).
> 
> For the arm32 case in cpufeature.c:identify_cpu, the two permitted
> values are 0 and 0b0100, which one did you have in mind? I take you
> meant 0 which stands for "not implemented, or no support for
> miscellaneous features"?

It looks like I misread the spec. :/ I thought it would only be a single 
possible value.

Hmmm... I am not sure about the value here as I don't know what would be 
the impact.

It might be better to detect whether we are running on Armv8 HW or Armv7 
HW. I haven't figured out an obvious way for that yet.

On IRC, Andre suggested a couple of options on IRC

  1) read ISAR5: This doesn't  seem to work as it is UNK on Armv7
  2) detect the VFP subarchitecture: It is not clear how this can be 
leveraged

While skimming through the spec, I noticed that ID_DFR0.CopDbg requires 
a specific value for Armv8.x spec. This is the debug architecture, but 
maybe we can leverage it?

Any better ideas?

> 
> 
> In regards to:
>> I would suggest to introduce name it MAY_BE_UNDEFINED (or similar)
>> that will be used to avoid reading the system register on 32-bit.
> 
> Did you mean something like the following on arm32 (maybe to
> xen/include/asm-arm/arm32/sysregs.h):
> 
> #define MVFR2_MAYBE_UNDEFINED
> 
> 
> Then in identify_cpu:
> 
> #ifndef MVFR2_MAYBE_UNDEFINED
>      c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
> #endif
> 

Yes. But this may not work if we can't decide on the fixed value (see 
above).

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers)
  2021-01-06 18:29               ` Julien Grall
@ 2021-01-06 20:55                 ` Stefano Stabellini
  2021-01-08 11:12                   ` Julien Grall
  0 siblings, 1 reply; 35+ messages in thread
From: Stefano Stabellini @ 2021-01-06 20:55 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, André Przywara, Bertrand Marquis,
	xen-devel, Ian Jackson

[-- Attachment #1: Type: text/plain, Size: 10209 bytes --]

On Wed, 6 Jan 2021, Julien Grall wrote:
> On 05/01/2021 22:43, Stefano Stabellini wrote:
> > On Tue, 5 Jan 2021, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 05/01/2021 18:44, Stefano Stabellini wrote:
> > > > On Tue, 5 Jan 2021, André Przywara wrote:
> > > > > On 05/01/2021 16:06, Julien Grall wrote:
> > > > > > (+ Ian and Andre)
> > > > > > 
> > > > > > Hi Bertrand,
> > > > > > 
> > > > > > On IRC, Ian pointed out that the smoke test was failing on
> > > > > > Cubietruck.
> > > > > > The only patches because the last success and the failure are your
> > > > > > series.
> > > > > > 
> > > > > > This seems to be a very early failure as there is no output from Xen
> > > > > > [1].
> > > > > > 
> > > > > > I originally thought the problem was because some of the ID
> > > > > > registers
> > > > > > (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
> > > > > > 
> > > > > > But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should
> > > > > > be
> > > > > > RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
> > > > > > accessed by writing a small baremetal code and booted on Cortex-A7
> > > > > > and
> > > > > > Cortex-A20.
> > > > > > 
> > > > > > So I am not entirely sure what's the problem. Andre kindly accepted
> > > > > > to
> > > > > > try to boot Xen on his board. Hopefully, this will give us a clue on
> > > > > > the
> > > > > > problem.
> > > > > > 
> > > > > > If not, I will borrow a Cubietruck in OssTest and see if I can
> > > > > > reproduce
> > > > > > it and debug it.
> > > > > 
> > > > > 
> > > > > So I just compiled master and staging and ran just that on an
> > > > > Allwinner
> > > > > H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
> > > > > missing Dom0, as expected). However staging indeed fails:
> > > > > 
> > > > > (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
> > > > > (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y  Tue Jan  5
> > > > > 16:09:40 GMT 2021
> > > > > (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
> > > > > (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
> > > > > (XEN) CPU0: Unexpected Trap: Undefined Instruction
> > > > > (XEN) ----[ Xen-4.15-unstable  arm32  debug=y   Not tainted ]----
> > > > > (XEN) CPU:    0
> > > > > (XEN) PC:     0026b8c8 identify_cpu+0xc0/0xd4
> > > > > (XEN) CPSR:   600001da MODE:Hypervisor
> > > > > (XEN)      R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
> > > > > (XEN)      R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
> > > > > (XEN)      R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c
> > > > > R12:00000080
> > > > > (XEN) HYP: SP: 002f7e68 LR: 002c419c
> > > > > (XEN)
> > > > > (XEN)   VTCR_EL2: 80002646
> > > > > (XEN)  VTTBR_EL2: 00000018e628bb80
> > > > > (XEN)
> > > > > (XEN)  SCTLR_EL2: 30cd187f
> > > > > (XEN)    HCR_EL2: 00000038
> > > > > (XEN)  TTBR0_EL2: 000000004013a000
> > > > > (XEN)
> > > > > (XEN)    ESR_EL2: 00000000
> > > > > (XEN)  HPFAR_EL2: 0003fff0
> > > > > (XEN)      HDFAR: 9d110000
> > > > > (XEN)      HIFAR: 0000a04a
> > > > > (XEN)
> > > > > (XEN) Xen stack trace from sp=002f7e68:
> > > > > (XEN)    00000000 002f7f54 00008000 00000000 00002000 002a4584
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN)    00000000 00008000 49ff5000 002d81f0 40000000 00000000
> > > > > 00002000
> > > > > 00000001
> > > > > (XEN)    00000000 50000000 49ffd000 00000000 50000000 00000000
> > > > > 00000000
> > > > > 50000000
> > > > > (XEN)    4c000000 00000000 4e000000 00000000 ffffffff ffffffff
> > > > > 50000000
> > > > > 00000000
> > > > > (XEN)    50000000 00000000 50000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN)    00000000 00000003 00000000 00000000 ffffffff 00000040
> > > > > ffffffff
> > > > > 00000000
> > > > > (XEN)    00000000 00000000 00000000 002a7000 40008050 0000001a
> > > > > 00000000
> > > > > 49ff5000
> > > > > (XEN)    40008000 3fe08000 00000004 0020006c 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
> > > > > (XEN) Xen call trace:
> > > > > (XEN)    [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
> > > > > (XEN)    [<002c419c>] start_xen+0x778/0xe50 (LR)
> > > > > (XEN)    [<002f7f54>] 002f7f54
> > > > > (XEN)
> > > > > (XEN)
> > > > > (XEN) ****************************************
> > > > > (XEN) Panic on CPU 0:
> > > > > (XEN) CPU0: Unexpected Trap: Undefined Instruction
> > > > > (XEN) ****************************************
> > > > > (XEN)
> > > > > (XEN) Reboot in five seconds...
> > > > > 
> > > > > 
> > > > > The code in question:
> > > > >     26b8c0:       eef63a10        vmrs    r3, mvfr1
> > > > >     26b8c4:       e5803058        str     r3, [r0, #88]   ; 0x58
> > > > > > 26b8c8:       eef53a10        vmrs    r3, mvfr2
> > > > >     26b8cc:       e580305c        str     r3, [r0, #92]   ; 0x5c
> > > > >     26b8d0:       e28bd000        add     sp, fp, #0
> > > > >     26b8d4:       e49db004        pop     {fp}       ; (ldr fp, [sp],
> > > > > #4)
> > > > >     26b8d8:       e12fff1e        bx      lr
> > > > > 
> > > > > And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
> > > > > the CP15 CPUID registers this is using the VMRS instruction, so it's
> > > > > not
> > > > > protected by future-proof CPUID register scheme.
> > > > > 
> > > > > Not sure what to do about this, maybe #ifdef'ing this register access
> > > > > with arm64?
> > > > > I guess this comes from the slightly too optimistic code-sharing
> > > > > between
> > > > > arm32 and arm64?
> > > > 
> > > > Yes and #ifdef'ing is what we have been doing with the other registers.
> > > 
> > > There is a catch here. This register is accessible from AArch32 on all
> > > Armv8
> > > HW. It is just not accessible on Armv7.
> > > 
> > > So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not
> > > correct.
> > > 
> > > I know that we said that we don't officially support Xen on Arm32 on Armv8
> > > HW
> > > (I can't find where it is written though). So we could argue that
> > > shadowing
> > > MVFR2 is not worth it.
> > > 
> > > I do use Armv8 HW to test 32-bit, so I would at least like to get Xen
> > > booting.
> > 
> > Yep, me too.
> > 
> > 
> > > In addition to that, Linux 32-bit doesn't access MVFR2 at the moment.
> > > 
> > > Therefore, a #ifdef may be acceptable for now. However, I would suggest to
> > > introduce name it MAY_BE_UNDEFINED (or similar) that will be used to avoid
> > > reading the system register on 32-bit.
> > > 
> > > For the 32-bit case, I would just hardcode the value based on the Arm (it
> > > looks like for Armv8-A there is only one valid value).
> > > 
> > > IOW, the hack would be self-contained in cpufeature.c.
> > 
> > I think it makes sense that the hack should be self-contained in
> > cpufeature.c, leaving the definition of struct mvfr to 3 register_t also
> > on arm32 as it is today. Also leaving vcpreg.c as it is today so that a
> > guest can try to read mvfr2 without crashing thanks to
> > GENERATE_TID3_INFO(MVFR2, mvfr, 2).
> > 
> > For the arm32 case in cpufeature.c:identify_cpu, the two permitted
> > values are 0 and 0b0100, which one did you have in mind? I take you
> > meant 0 which stands for "not implemented, or no support for
> > miscellaneous features"?
> 
> It looks like I misread the spec. :/ I thought it would only be a single
> possible value.
> 
> Hmmm... I am not sure about the value here as I don't know what would be the
> impact.
> 
> It might be better to detect whether we are running on Armv8 HW or Armv7 HW. I
> haven't figured out an obvious way for that yet.
> 
> On IRC, Andre suggested a couple of options on IRC
> 
>  1) read ISAR5: This doesn't  seem to work as it is UNK on Armv7
>  2) detect the VFP subarchitecture: It is not clear how this can be leveraged
> 
> While skimming through the spec, I noticed that ID_DFR0.CopDbg requires a
> specific value for Armv8.x spec. This is the debug architecture, but maybe we
> can leverage it?
> 
> Any better ideas?

One thing to note is that if Xen has difficulties in distinguishing
between armv7 and armv8/aarch32 a guest will have the same issue. Given
that mvfr2 is not available on armv7, how can a guest safely attempt to
read it?

If somehow the guest figures out that it is running on armv8/aarch32,
then it could read mvfr2, but we could return "not implemented" in that
case, right? In other words, part of me thinks that maybe we are
overcomplicating this, especially because we don't officially support
Xen on armv8/aarch32 as you wrote (we need to update SUPPORT.md).

So I would be tempted to only do the following:

- read mvfr2 on arm64
- never read mvfr2 on aarch32 (no matter armv7 or armv8)
- keep c->mvfr.bits[2] as zero on armv7 and armv8/aarch32 so that if the
  guest attempts to read it, it won't crash but simply return zero


The most important things in regards to armv8/aarch32 are:

- be able to boot Xen armv8/aarch32 for our own arm32 testing of Xen
- be able to boot an armv7 and armv8/aarch32 guest on Xen armv8/aarch64

Both these things should work correctly with the simple approach above.


Your idea of trying to figure out if we are running on armv8/aarch32 via
ID_DFR0.CopDbg is clever but I think it is a bit overkill for a mode of
execution that we don't support. I don't think is worth the effort.

But if you think differently, I can invest some time in investigating
this further, following your line of thought on finding a register to
detect the mode of execution. (However I don't have any hardware to test
this anymore.) Otherwise, if you agree that the simpler approach is
sufficient, then I'll send a patch as described above.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers)
  2021-01-06 20:55                 ` Stefano Stabellini
@ 2021-01-08 11:12                   ` Julien Grall
  2021-01-11 11:19                     ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) [and 2 more messages] Ian Jackson
  0 siblings, 1 reply; 35+ messages in thread
From: Julien Grall @ 2021-01-08 11:12 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: André Przywara, Bertrand Marquis, xen-devel, Ian Jackson



On 06/01/2021 20:55, Stefano Stabellini wrote:
> On Wed, 6 Jan 2021, Julien Grall wrote:
>> On 05/01/2021 22:43, Stefano Stabellini wrote:
>>> On Tue, 5 Jan 2021, Julien Grall wrote:
>>>> Hi Stefano,
>>>>
>>>> On 05/01/2021 18:44, Stefano Stabellini wrote:
>>>>> On Tue, 5 Jan 2021, André Przywara wrote:
>>>>>> On 05/01/2021 16:06, Julien Grall wrote:
>>>>>>> (+ Ian and Andre)
>>>>>>>
>>>>>>> Hi Bertrand,
>>>>>>>
>>>>>>> On IRC, Ian pointed out that the smoke test was failing on
>>>>>>> Cubietruck.
>>>>>>> The only patches because the last success and the failure are your
>>>>>>> series.
>>>>>>>
>>>>>>> This seems to be a very early failure as there is no output from Xen
>>>>>>> [1].
>>>>>>>
>>>>>>> I originally thought the problem was because some of the ID
>>>>>>> registers
>>>>>>> (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
>>>>>>>
>>>>>>> But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should
>>>>>>> be
>>>>>>> RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
>>>>>>> accessed by writing a small baremetal code and booted on Cortex-A7
>>>>>>> and
>>>>>>> Cortex-A20.
>>>>>>>
>>>>>>> So I am not entirely sure what's the problem. Andre kindly accepted
>>>>>>> to
>>>>>>> try to boot Xen on his board. Hopefully, this will give us a clue on
>>>>>>> the
>>>>>>> problem.
>>>>>>>
>>>>>>> If not, I will borrow a Cubietruck in OssTest and see if I can
>>>>>>> reproduce
>>>>>>> it and debug it.
>>>>>>
>>>>>>
>>>>>> So I just compiled master and staging and ran just that on an
>>>>>> Allwinner
>>>>>> H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
>>>>>> missing Dom0, as expected). However staging indeed fails:
>>>>>>
>>>>>> (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
>>>>>> (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y  Tue Jan  5
>>>>>> 16:09:40 GMT 2021
>>>>>> (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
>>>>>> (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
>>>>>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>>>>> (XEN) ----[ Xen-4.15-unstable  arm32  debug=y   Not tainted ]----
>>>>>> (XEN) CPU:    0
>>>>>> (XEN) PC:     0026b8c8 identify_cpu+0xc0/0xd4
>>>>>> (XEN) CPSR:   600001da MODE:Hypervisor
>>>>>> (XEN)      R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
>>>>>> (XEN)      R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
>>>>>> (XEN)      R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c
>>>>>> R12:00000080
>>>>>> (XEN) HYP: SP: 002f7e68 LR: 002c419c
>>>>>> (XEN)
>>>>>> (XEN)   VTCR_EL2: 80002646
>>>>>> (XEN)  VTTBR_EL2: 00000018e628bb80
>>>>>> (XEN)
>>>>>> (XEN)  SCTLR_EL2: 30cd187f
>>>>>> (XEN)    HCR_EL2: 00000038
>>>>>> (XEN)  TTBR0_EL2: 000000004013a000
>>>>>> (XEN)
>>>>>> (XEN)    ESR_EL2: 00000000
>>>>>> (XEN)  HPFAR_EL2: 0003fff0
>>>>>> (XEN)      HDFAR: 9d110000
>>>>>> (XEN)      HIFAR: 0000a04a
>>>>>> (XEN)
>>>>>> (XEN) Xen stack trace from sp=002f7e68:
>>>>>> (XEN)    00000000 002f7f54 00008000 00000000 00002000 002a4584
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN)    00000000 00008000 49ff5000 002d81f0 40000000 00000000
>>>>>> 00002000
>>>>>> 00000001
>>>>>> (XEN)    00000000 50000000 49ffd000 00000000 50000000 00000000
>>>>>> 00000000
>>>>>> 50000000
>>>>>> (XEN)    4c000000 00000000 4e000000 00000000 ffffffff ffffffff
>>>>>> 50000000
>>>>>> 00000000
>>>>>> (XEN)    50000000 00000000 50000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN)    00000000 00000003 00000000 00000000 ffffffff 00000040
>>>>>> ffffffff
>>>>>> 00000000
>>>>>> (XEN)    00000000 00000000 00000000 002a7000 40008050 0000001a
>>>>>> 00000000
>>>>>> 49ff5000
>>>>>> (XEN)    40008000 3fe08000 00000004 0020006c 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN)    00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> (XEN) Xen call trace:
>>>>>> (XEN)    [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
>>>>>> (XEN)    [<002c419c>] start_xen+0x778/0xe50 (LR)
>>>>>> (XEN)    [<002f7f54>] 002f7f54
>>>>>> (XEN)
>>>>>> (XEN)
>>>>>> (XEN) ****************************************
>>>>>> (XEN) Panic on CPU 0:
>>>>>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>>>>> (XEN) ****************************************
>>>>>> (XEN)
>>>>>> (XEN) Reboot in five seconds...
>>>>>>
>>>>>>
>>>>>> The code in question:
>>>>>>      26b8c0:       eef63a10        vmrs    r3, mvfr1
>>>>>>      26b8c4:       e5803058        str     r3, [r0, #88]   ; 0x58
>>>>>>> 26b8c8:       eef53a10        vmrs    r3, mvfr2
>>>>>>      26b8cc:       e580305c        str     r3, [r0, #92]   ; 0x5c
>>>>>>      26b8d0:       e28bd000        add     sp, fp, #0
>>>>>>      26b8d4:       e49db004        pop     {fp}       ; (ldr fp, [sp],
>>>>>> #4)
>>>>>>      26b8d8:       e12fff1e        bx      lr
>>>>>>
>>>>>> And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
>>>>>> the CP15 CPUID registers this is using the VMRS instruction, so it's
>>>>>> not
>>>>>> protected by future-proof CPUID register scheme.
>>>>>>
>>>>>> Not sure what to do about this, maybe #ifdef'ing this register access
>>>>>> with arm64?
>>>>>> I guess this comes from the slightly too optimistic code-sharing
>>>>>> between
>>>>>> arm32 and arm64?
>>>>>
>>>>> Yes and #ifdef'ing is what we have been doing with the other registers.
>>>>
>>>> There is a catch here. This register is accessible from AArch32 on all
>>>> Armv8
>>>> HW. It is just not accessible on Armv7.
>>>>
>>>> So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not
>>>> correct.
>>>>
>>>> I know that we said that we don't officially support Xen on Arm32 on Armv8
>>>> HW
>>>> (I can't find where it is written though). So we could argue that
>>>> shadowing
>>>> MVFR2 is not worth it.
>>>>
>>>> I do use Armv8 HW to test 32-bit, so I would at least like to get Xen
>>>> booting.
>>>
>>> Yep, me too.
>>>
>>>
>>>> In addition to that, Linux 32-bit doesn't access MVFR2 at the moment.
>>>>
>>>> Therefore, a #ifdef may be acceptable for now. However, I would suggest to
>>>> introduce name it MAY_BE_UNDEFINED (or similar) that will be used to avoid
>>>> reading the system register on 32-bit.
>>>>
>>>> For the 32-bit case, I would just hardcode the value based on the Arm (it
>>>> looks like for Armv8-A there is only one valid value).
>>>>
>>>> IOW, the hack would be self-contained in cpufeature.c.
>>>
>>> I think it makes sense that the hack should be self-contained in
>>> cpufeature.c, leaving the definition of struct mvfr to 3 register_t also
>>> on arm32 as it is today. Also leaving vcpreg.c as it is today so that a
>>> guest can try to read mvfr2 without crashing thanks to
>>> GENERATE_TID3_INFO(MVFR2, mvfr, 2).
>>>
>>> For the arm32 case in cpufeature.c:identify_cpu, the two permitted
>>> values are 0 and 0b0100, which one did you have in mind? I take you
>>> meant 0 which stands for "not implemented, or no support for
>>> miscellaneous features"?
>>
>> It looks like I misread the spec. :/ I thought it would only be a single
>> possible value.
>>
>> Hmmm... I am not sure about the value here as I don't know what would be the
>> impact.
>>
>> It might be better to detect whether we are running on Armv8 HW or Armv7 HW. I
>> haven't figured out an obvious way for that yet.
>>
>> On IRC, Andre suggested a couple of options on IRC
>>
>>   1) read ISAR5: This doesn't  seem to work as it is UNK on Armv7
>>   2) detect the VFP subarchitecture: It is not clear how this can be leveraged
>>
>> While skimming through the spec, I noticed that ID_DFR0.CopDbg requires a
>> specific value for Armv8.x spec. This is the debug architecture, but maybe we
>> can leverage it?
>>
>> Any better ideas?
> 
> One thing to note is that if Xen has difficulties in distinguishing
> between armv7 and armv8/aarch32 a guest will have the same issue. Given
> that mvfr2 is not available on armv7, how can a guest safely attempt to
> read it?

I am not sure I would call it difficulties yet. I would say it is more 
that I haven't looked too much in the spec to understand if there is a 
trivial way to distinguish between v7 and v8.

Maybe André or Bertrand can help here?

> 
> If somehow the guest figures out that it is running on armv8/aarch32,
> then it could read mvfr2, but we could return "not implemented" in that
> case, right? 

Nothing in the Armv8 say the register is optional. So we technically 
cannot return "not implemented".

However, we could consider to return "not implemented" (which would 
likely result to a crash) if we are not confident with the fixed value 
returned.

> In other words, part of me thinks that maybe we are
> overcomplicating this, especially because we don't officially support
> Xen on armv8/aarch32 as you wrote (we need to update SUPPORT.md).

Right, at the same time we don't want to introduce problem hard to 
diagnostic.

This is quite similar to the discussion about IS{ACT,PEND}R GIC register 
we had in the past. If we return dummy value, then the guest may 
continue to run until a point but we don't know for how long. An error 
may come up a really long time after and it would be difficult to find 
the exact root cause.

So we need to be careful in our choice.

> So I would be tempted to only do the following:
> 
> - read mvfr2 on arm64
> - never read mvfr2 on aarch32 (no matter armv7 or armv8)
> - keep c->mvfr.bits[2] as zero on armv7 and armv8/aarch32 so that if the
>    guest attempts to read it, it won't crash but simply return zero
I agree that we should return 0, but I disagree with the justification 
(see more below).

Reading the Armv8 spec again, 0 means there is no support for 
miscellaneous features. So the guest should behave correctly if we 
return 0. Therefore returning 0 for AArch32 should be fine.

> 
> The most important things in regards to armv8/aarch32 are:
> 
> - be able to boot Xen armv8/aarch32 for our own arm32 testing of Xen
> - be able to boot an armv7 and armv8/aarch32 guest on Xen armv8/aarch64

The most important things here is to not shoot ourself in the foot by 
returning value we don't understand :). It is best to crash the guest 
rather than continuing until it become nearly impossible to track the 
root cause of an issue.

> 
> Both these things should work correctly with the simple approach above.
> 
> 
> Your idea of trying to figure out if we are running on armv8/aarch32 via
> ID_DFR0.CopDbg is clever but I think it is a bit overkill for a mode of
> execution that we don't support. I don't think is worth the effort.

I made this suggestion because I originally thought it would not be 
possible to find a sensible value. I believe we found one, so this 
suggestion can be discarded.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [xen-unstable test] 158303: regressions - FAIL
@ 2021-01-10 11:56 ` osstest service owner
  2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
  0 siblings, 1 reply; 35+ messages in thread
From: osstest service owner @ 2021-01-10 11:56 UTC (permalink / raw)
  To: xen-devel, osstest-admin

flight 158303 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/158303/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-xl-thunderx  8 xen-boot                 fail REGR. vs. 158290
 test-arm64-arm64-examine      8 reboot                   fail REGR. vs. 158290
 test-arm64-arm64-xl-xsm       8 xen-boot                 fail REGR. vs. 158290
 test-arm64-arm64-xl-credit1   8 xen-boot                 fail REGR. vs. 158290

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 20 guest-start/debianhvm.repeat fail in 158296 pass in 158303
 test-arm64-arm64-xl           8 xen-boot                   fail pass in 158296
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 17 depriv-audit-qemu/create fail pass in 158296

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl         15 migrate-support-check fail in 158296 never pass
 test-arm64-arm64-xl     16 saverestore-support-check fail in 158296 never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stop            fail like 158290
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stop            fail like 158290
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop             fail like 158290
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stop            fail like 158290
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop             fail like 158290
 test-armhf-armhf-libvirt-raw 15 saverestore-support-check    fail  like 158290
 test-armhf-armhf-libvirt     16 saverestore-support-check    fail  like 158290
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stop            fail like 158290
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop             fail like 158290
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop             fail like 158290
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 158290
 test-amd64-i386-xl-pvshim    14 guest-start                  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-check        fail   never pass
 test-amd64-amd64-libvirt     15 migrate-support-check        fail   never pass
 test-amd64-i386-libvirt      15 migrate-support-check        fail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-check        fail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-check        fail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-check    fail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-check        fail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-check    fail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-check        fail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-check    fail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-check        fail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-check    fail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-check        fail   never pass
 test-armhf-armhf-xl          15 migrate-support-check        fail   never pass
 test-armhf-armhf-xl          16 saverestore-support-check    fail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-check        fail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-check    fail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-check        fail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-check    fail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-check        fail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-check    fail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-check        fail   never pass
 test-armhf-armhf-libvirt     15 migrate-support-check        fail   never pass
 test-armhf-armhf-xl-rtds     15 migrate-support-check        fail   never pass
 test-armhf-armhf-xl-rtds     16 saverestore-support-check    fail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-check        fail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-check    fail  never pass
 test-armhf-armhf-xl-vhd      14 migrate-support-check        fail   never pass
 test-armhf-armhf-xl-vhd      15 saverestore-support-check    fail   never pass

version targeted for testing:
 xen                  ce59e3dda5f99afbe7257e1e9a22dffd5c4d033c
baseline version:
 xen                  7ba2ab495be54f608cb47440e1497b2795bd301a

Last test of basis   158290  2021-01-09 01:51:29 Z    1 days
Testing same since   158296  2021-01-09 11:08:42 Z    1 days    2 attempts

------------------------------------------------------------
People who touched revisions under test:
  Andrew Cooper <andrew.cooper3@citrix.com>
  Bertrand Marquis <bertrand.marquis@arm.com>
  Jan Beulich <jbeulich@suse.com>
  Julien Grall <jgrall@amazon.com>
  Manuel Bouyer <bouyer@antioche.eu.org>
  Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
  Olaf Hering <olaf@aepfle.de>
  Roger Pau Monne <roger.pau@citrix.com>
  Roger Pau Monné <roger.pau@citrix.com>
  Roman Skakun <roman_skakun@epam.com>
  Stefano Stabellini <sstabellini@kernel.org>
  Stefano Stabellini <stefano.stabellini@xilinx.com>
  Tamas K Lengyel <tamas.lengyel@intel.com>
  Tamas K Lengyel <tamas@tklengyel.com>
  Tim Deegan <tim@xen.org>
  Volodymyr Babchyk <volodymyr_babchuk@epam.com>
  Wei Liu <wl@xen.org>

jobs:
 build-amd64-xsm                                              pass    
 build-arm64-xsm                                              pass    
 build-i386-xsm                                               pass    
 build-amd64-xtf                                              pass    
 build-amd64                                                  pass    
 build-arm64                                                  pass    
 build-armhf                                                  pass    
 build-i386                                                   pass    
 build-amd64-libvirt                                          pass    
 build-arm64-libvirt                                          pass    
 build-armhf-libvirt                                          pass    
 build-i386-libvirt                                           pass    
 build-amd64-prev                                             pass    
 build-i386-prev                                              pass    
 build-amd64-pvops                                            pass    
 build-arm64-pvops                                            pass    
 build-armhf-pvops                                            pass    
 build-i386-pvops                                             pass    
 test-xtf-amd64-amd64-1                                       pass    
 test-xtf-amd64-amd64-2                                       pass    
 test-xtf-amd64-amd64-3                                       pass    
 test-xtf-amd64-amd64-4                                       pass    
 test-xtf-amd64-amd64-5                                       pass    
 test-amd64-amd64-xl                                          pass    
 test-amd64-coresched-amd64-xl                                pass    
 test-arm64-arm64-xl                                          fail    
 test-armhf-armhf-xl                                          pass    
 test-amd64-i386-xl                                           pass    
 test-amd64-coresched-i386-xl                                 pass    
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm           pass    
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm            pass    
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm        pass    
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm         pass    
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm                 pass    
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm                  pass    
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm                 pass    
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm                  pass    
 test-amd64-amd64-libvirt-xsm                                 pass    
 test-arm64-arm64-libvirt-xsm                                 pass    
 test-amd64-i386-libvirt-xsm                                  pass    
 test-amd64-amd64-xl-xsm                                      pass    
 test-arm64-arm64-xl-xsm                                      fail    
 test-amd64-i386-xl-xsm                                       pass    
 test-amd64-amd64-qemuu-nested-amd                            fail    
 test-amd64-amd64-xl-pvhv2-amd                                pass    
 test-amd64-i386-qemut-rhel6hvm-amd                           pass    
 test-amd64-i386-qemuu-rhel6hvm-amd                           pass    
 test-amd64-amd64-dom0pvh-xl-amd                              pass    
 test-amd64-amd64-xl-qemut-debianhvm-amd64                    pass    
 test-amd64-i386-xl-qemut-debianhvm-amd64                     pass    
 test-amd64-amd64-xl-qemuu-debianhvm-amd64                    pass    
 test-amd64-i386-xl-qemuu-debianhvm-amd64                     pass    
 test-amd64-i386-freebsd10-amd64                              pass    
 test-amd64-amd64-qemuu-freebsd11-amd64                       pass    
 test-amd64-amd64-qemuu-freebsd12-amd64                       pass    
 test-amd64-amd64-xl-qemuu-ovmf-amd64                         pass    
 test-amd64-i386-xl-qemuu-ovmf-amd64                          pass    
 test-amd64-amd64-xl-qemut-win7-amd64                         fail    
 test-amd64-i386-xl-qemut-win7-amd64                          fail    
 test-amd64-amd64-xl-qemuu-win7-amd64                         fail    
 test-amd64-i386-xl-qemuu-win7-amd64                          fail    
 test-amd64-amd64-xl-qemut-ws16-amd64                         fail    
 test-amd64-i386-xl-qemut-ws16-amd64                          fail    
 test-amd64-amd64-xl-qemuu-ws16-amd64                         fail    
 test-amd64-i386-xl-qemuu-ws16-amd64                          fail    
 test-armhf-armhf-xl-arndale                                  pass    
 test-amd64-amd64-xl-credit1                                  pass    
 test-arm64-arm64-xl-credit1                                  fail    
 test-armhf-armhf-xl-credit1                                  pass    
 test-amd64-amd64-xl-credit2                                  pass    
 test-arm64-arm64-xl-credit2                                  pass    
 test-armhf-armhf-xl-credit2                                  pass    
 test-armhf-armhf-xl-cubietruck                               pass    
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict        fail    
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict         pass    
 test-amd64-amd64-examine                                     pass    
 test-arm64-arm64-examine                                     fail    
 test-armhf-armhf-examine                                     pass    
 test-amd64-i386-examine                                      pass    
 test-amd64-i386-freebsd10-i386                               pass    
 test-amd64-amd64-qemuu-nested-intel                          pass    
 test-amd64-amd64-xl-pvhv2-intel                              pass    
 test-amd64-i386-qemut-rhel6hvm-intel                         pass    
 test-amd64-i386-qemuu-rhel6hvm-intel                         pass    
 test-amd64-amd64-dom0pvh-xl-intel                            pass    
 test-amd64-amd64-libvirt                                     pass    
 test-armhf-armhf-libvirt                                     pass    
 test-amd64-i386-libvirt                                      pass    
 test-amd64-amd64-livepatch                                   pass    
 test-amd64-i386-livepatch                                    pass    
 test-amd64-amd64-migrupgrade                                 pass    
 test-amd64-i386-migrupgrade                                  pass    
 test-amd64-amd64-xl-multivcpu                                pass    
 test-armhf-armhf-xl-multivcpu                                pass    
 test-amd64-amd64-pair                                        pass    
 test-amd64-i386-pair                                         pass    
 test-amd64-amd64-libvirt-pair                                pass    
 test-amd64-i386-libvirt-pair                                 pass    
 test-amd64-amd64-amd64-pvgrub                                pass    
 test-amd64-amd64-i386-pvgrub                                 pass    
 test-amd64-amd64-xl-pvshim                                   pass    
 test-amd64-i386-xl-pvshim                                    fail    
 test-amd64-amd64-pygrub                                      pass    
 test-amd64-amd64-xl-qcow2                                    pass    
 test-armhf-armhf-libvirt-raw                                 pass    
 test-amd64-i386-xl-raw                                       pass    
 test-amd64-amd64-xl-rtds                                     pass    
 test-armhf-armhf-xl-rtds                                     pass    
 test-arm64-arm64-xl-seattle                                  pass    
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow             pass    
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow              pass    
 test-amd64-amd64-xl-shadow                                   pass    
 test-amd64-i386-xl-shadow                                    pass    
 test-arm64-arm64-xl-thunderx                                 fail    
 test-amd64-amd64-libvirt-vhd                                 pass    
 test-armhf-armhf-xl-vhd                                      pass    


------------------------------------------------------------
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
    http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
    http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
    http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
    http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 750 lines long.)


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [xen-unstable bisection] complete test-arm64-arm64-examine
@ 2021-01-10 11:56 osstest service owner
  2021-01-10 11:56 ` [xen-unstable test] 158303: regressions - FAIL osstest service owner
  0 siblings, 1 reply; 35+ messages in thread
From: osstest service owner @ 2021-01-10 11:56 UTC (permalink / raw)
  To: xen-devel, osstest-admin

branch xen-unstable
xenbranch xen-unstable
job test-arm64-arm64-examine
testid reboot

Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  xen git://xenbits.xen.org/xen.git
  Bug introduced:  9cfdb489af810f71acb7dcdb87075dc7b3b313a0
  Bug not present: a9f1f03b2710f5ce84f69c1c4516349531053fac
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/158321/


  commit 9cfdb489af810f71acb7dcdb87075dc7b3b313a0
  Author: Bertrand Marquis <bertrand.marquis@arm.com>
  Date:   Thu Dec 17 15:38:02 2020 +0000
  
      xen/arm: Add ID registers and complete cpuinfo
      
      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 MVFRx_EL1 defines 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: Stefano Stabellini <sstabellini@kernel.org>


For bisection revision-tuple graph see:
   http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable/test-arm64-arm64-examine.reboot.html
Revision IDs in each graph node refer, respectively, to the Trees above.

----------------------------------------
Running cs-bisection-step --graph-out=/home/logs/results/bisect/xen-unstable/test-arm64-arm64-examine.reboot --summary-out=tmp/158321.bisection-summary --basis-template=158290 --blessings=real,real-bisect,real-retry xen-unstable test-arm64-arm64-examine reboot
Searching for failure / basis pass:
 158303 fail [host=rochester1] / 158290 [host=laxton1] 158269 [host=laxton0] 158231 [host=rochester0] 158183 ok.
Failure / basis pass flights: 158303 / 158183
Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Latest a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 ce59e3dda5f99afbe7257e1e9a22dffd5c4d033c
Basis pass a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 7ba2ab495be54f608cb47440e1497b2795bd301a
Generating revisions with ./adhoc-revtuple-generator  git://xenbits.xen.org/linux-pvops.git#a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9-a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/qemu-xen.git#7ea428895af2840d85c524f0bd11a38aac308308-7ea428895af2840d85c524f0bd11a38aac308308 git://xenbits.xen.org/xen.git#7ba2ab495be54f608cb47440e1497b2795bd301a-ce59e3d\
 da5f99afbe7257e1e9a22dffd5c4d033c
Loaded 5001 nodes in revision graph
Searching for test results:
 158132 [host=laxton0]
 158146 [host=laxton1]
 158183 pass a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 7ba2ab495be54f608cb47440e1497b2795bd301a
 158231 [host=rochester0]
 158269 [host=laxton0]
 158290 [host=laxton1]
 158296 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 ce59e3dda5f99afbe7257e1e9a22dffd5c4d033c
 158302 pass a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 7ba2ab495be54f608cb47440e1497b2795bd301a
 158305 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 ce59e3dda5f99afbe7257e1e9a22dffd5c4d033c
 158306 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 c7115531ea8ede5c6ab27f972c1be6ecad388f55
 158308 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 7d2d7a43d0141de69d837854ce139bcd47a7b859
 158309 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 07b9acea116e8329d613004766b8606756986db5
 158318 pass a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 a9f1f03b2710f5ce84f69c1c4516349531053fac
 158311 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 9cfdb489af810f71acb7dcdb87075dc7b3b313a0
 158313 pass a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 a9f1f03b2710f5ce84f69c1c4516349531053fac
 158314 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 9cfdb489af810f71acb7dcdb87075dc7b3b313a0
 158316 pass a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 a9f1f03b2710f5ce84f69c1c4516349531053fac
 158317 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 9cfdb489af810f71acb7dcdb87075dc7b3b313a0
 158303 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 ce59e3dda5f99afbe7257e1e9a22dffd5c4d033c
 158321 fail a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 9cfdb489af810f71acb7dcdb87075dc7b3b313a0
Searching for interesting versions
 Result found: flight 158183 (pass), for basis pass
 For basis failure, parent search stopping at a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 a9f1f03b2710f5ce84f69c1c4516349531053fac, results HASH(0x562171ce2c68) HASH(0x562171c8fab0) HASH(0x562171c7def8) For basis failure, parent search stopping at a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 7ba2ab495be54f608cb47440e1497b2795bd301a, results \
 HASH(0x562171c7e9a0) HASH(0x562171c7f120) Result found: flight 158296 (fail), for basis failure (at ancestor ~771)
 Repro found: flight 158302 (pass), for basis pass
 Repro found: flight 158303 (fail), for basis failure
 0 revisions at a6c5dd1dbaffe4cc398d8454546ba9246b9a95c9 c530a75c1e6a472b0eb9558310b518f0dfcd8860 7ea428895af2840d85c524f0bd11a38aac308308 a9f1f03b2710f5ce84f69c1c4516349531053fac
No revisions left to test, checking graph state.
 Result found: flight 158313 (pass), for last pass
 Result found: flight 158314 (fail), for first failure
 Repro found: flight 158316 (pass), for last pass
 Repro found: flight 158317 (fail), for first failure
 Repro found: flight 158318 (pass), for last pass
 Repro found: flight 158321 (fail), for first failure

*** Found and reproduced problem changeset ***

  Bug is in tree:  xen git://xenbits.xen.org/xen.git
  Bug introduced:  9cfdb489af810f71acb7dcdb87075dc7b3b313a0
  Bug not present: a9f1f03b2710f5ce84f69c1c4516349531053fac
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/158321/


  commit 9cfdb489af810f71acb7dcdb87075dc7b3b313a0
  Author: Bertrand Marquis <bertrand.marquis@arm.com>
  Date:   Thu Dec 17 15:38:02 2020 +0000
  
      xen/arm: Add ID registers and complete cpuinfo
      
      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 MVFRx_EL1 defines 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: Stefano Stabellini <sstabellini@kernel.org>

Revision graph left in /home/logs/results/bisect/xen-unstable/test-arm64-arm64-examine.reboot.{dot,ps,png,html,svg}.
----------------------------------------
158321: tolerable ALL FAIL

flight 158321 xen-unstable real-bisect [real]
http://logs.test-lab.xenproject.org/osstest/logs/158321/

Failures :-/ but no regressions.

Tests which did not succeed,
including tests which could not be run:
 test-arm64-arm64-examine      8 reboot                  fail baseline untested


jobs:
 test-arm64-arm64-examine                                     fail    


------------------------------------------------------------
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
    http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
    http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
    http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
    http://xenbits.xen.org/gitweb?p=osstest.git;a=summary



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) [and 2 more messages]
  2021-01-08 11:12                   ` Julien Grall
@ 2021-01-11 11:19                     ` Ian Jackson
  2021-01-11 11:25                       ` Julien Grall
  0 siblings, 1 reply; 35+ messages in thread
From: Ian Jackson @ 2021-01-11 11:19 UTC (permalink / raw)
  To: Julien Grall, Stefano Stabellini, André Przywara,
	Bertrand Marquis, xen-devel

It seems there is still something wrong with ARM in staging.  Whatever
change was made passed the smoke test but now the main flights are
failing:

osstest service owner writes:
> flight 158303 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/158303/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-arm64-arm64-xl-thunderx  8 xen-boot             fail REGR. vs. 158290
>  test-arm64-arm64-examine      8 reboot               fail REGR. vs. 158290
>  test-arm64-arm64-xl-xsm       8 xen-boot             fail REGR. vs. 158290
>  test-arm64-arm64-xl-credit1   8 xen-boot             fail REGR. vs. 158290

The bisector has fingered the same commit in unstable as it did in the
smoke tests.  That might be because the fix made to staging, to get
the smoke tests to pass, was not complete enough.  It also might be
because something different broke the other tests and the tree was
briefly working in between.

fx: puts on Release Manager hat.  Can one of you ARM folks please take
a look at this and fix it ASAP ?

Thanks,
Ian.

osstest service owner writes ("[xen-unstable bisection] complete test-arm64-arm64-xl-xsm"):
> branch xen-unstable
> xenbranch xen-unstable
> job test-arm64-arm64-xl-xsm
> testid xen-boot
> 
> Tree: linux git://xenbits.xen.org/linux-pvops.git
> Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
> Tree: qemuu git://xenbits.xen.org/qemu-xen.git
> Tree: xen git://xenbits.xen.org/xen.git
> 
> *** Found and reproduced problem changeset ***
> 
>   Bug is in tree:  xen git://xenbits.xen.org/xen.git
>   Bug introduced:  9cfdb489af810f71acb7dcdb87075dc7b3b313a0
>   Bug not present: a9f1f03b2710f5ce84f69c1c4516349531053fac
>   Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/158347/
> 
> 
>   commit 9cfdb489af810f71acb7dcdb87075dc7b3b313a0
>   Author: Bertrand Marquis <bertrand.marquis@arm.com>
>   Date:   Thu Dec 17 15:38:02 2020 +0000
>   
>       xen/arm: Add ID registers and complete cpuinfo
>       
>       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 MVFRx_EL1 defines 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: Stefano Stabellini <sstabellini@kernel.org>


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) [and 2 more messages]
  2021-01-11 11:19                     ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) [and 2 more messages] Ian Jackson
@ 2021-01-11 11:25                       ` Julien Grall
  0 siblings, 0 replies; 35+ messages in thread
From: Julien Grall @ 2021-01-11 11:25 UTC (permalink / raw)
  To: Ian Jackson, Stefano Stabellini, André Przywara,
	Bertrand Marquis, xen-devel

Hi Ian,

On 11/01/2021 11:19, Ian Jackson wrote:
> It seems there is still something wrong with ARM in staging.  Whatever
> change was made passed the smoke test but now the main flights are
> failing:
> 
> osstest service owner writes:
>> flight 158303 xen-unstable real [real]
>> http://logs.test-lab.xenproject.org/osstest/logs/158303/
>>
>> Regressions :-(
>>
>> Tests which did not succeed and are blocking,
>> including tests which could not be run:
>>   test-arm64-arm64-xl-thunderx  8 xen-boot             fail REGR. vs. 158290
>>   test-arm64-arm64-examine      8 reboot               fail REGR. vs. 158290
>>   test-arm64-arm64-xl-xsm       8 xen-boot             fail REGR. vs. 158290
>>   test-arm64-arm64-xl-credit1   8 xen-boot             fail REGR. vs. 158290
> 
> The bisector has fingered the same commit in unstable as it did in the
> smoke tests.  That might be because the fix made to staging, to get
> the smoke tests to pass, was not complete enough.  It also might be
> because something different broke the other tests and the tree was
> briefly working in between.

The smoke test ran on laxton0 (AMD Seattle) while the unstable test ran 
on rochester0 (Cavium Thunder-X).

The issue seems to be processor specific.

> 
> fx: puts on Release Manager hat.  Can one of you ARM folks please take
> a look at this and fix it ASAP ?

Jan sent an e-mail this morning about this. See [1].

Cheers,

[1] <dce8bed2-db66-b032-06a9-86c8f80d26aa@xen.org>

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2021-01-11 11:26 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-10 11:56 [xen-unstable bisection] complete test-arm64-arm64-examine osstest service owner
2021-01-10 11:56 ` [xen-unstable test] 158303: regressions - FAIL osstest service owner
2020-12-17 15:38   ` [PATCH v4 0/8] xen/arm: Emulate ID registers Bertrand Marquis
2020-12-17 15:38     ` [PATCH v4 1/8] xen/arm: Use READ_SYSREG instead of 32/64 versions Bertrand Marquis
2020-12-17 23:17       ` Stefano Stabellini
2020-12-18 10:23         ` Bertrand Marquis
2020-12-17 15:38     ` [PATCH v4 2/8] xen/arm: Add ID registers and complete cpuinfo Bertrand Marquis
2020-12-17 23:22       ` Stefano Stabellini
2020-12-17 15:38     ` [PATCH v4 3/8] xen/arm: Add arm64 ID registers definitions Bertrand Marquis
2020-12-17 23:24       ` Stefano Stabellini
2020-12-17 15:38     ` [PATCH v4 4/8] xen/arm: create a cpuinfo structure for guest Bertrand Marquis
2020-12-17 23:26       ` Stefano Stabellini
2020-12-17 15:38     ` [PATCH v4 5/8] xen/arm: Add handler for ID registers on arm64 Bertrand Marquis
2020-12-17 23:31       ` Stefano Stabellini
2020-12-17 15:38     ` [PATCH v4 6/8] xen/arm: Add handler for cp15 ID registers Bertrand Marquis
2020-12-17 23:37       ` Stefano Stabellini
2020-12-18 10:14         ` Bertrand Marquis
2020-12-17 15:38     ` [PATCH v4 7/8] xen/arm: Add CP10 exception support to handle MVFR Bertrand Marquis
2020-12-17 23:40       ` Stefano Stabellini
2020-12-17 15:38     ` [PATCH v4 8/8] xen/arm: Activate TID3 in HCR_EL2 Bertrand Marquis
2020-12-17 23:42       ` Stefano Stabellini
2020-12-17 23:47     ` [PATCH v4 0/8] xen/arm: Emulate ID registers Stefano Stabellini
2020-12-18 10:12       ` Bertrand Marquis
2021-01-05 16:06     ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) Julien Grall
2021-01-05 16:28       ` André Przywara
2021-01-05 18:44         ` Stefano Stabellini
2021-01-05 19:05           ` [PATCH] xen/arm: do not read MVFR2 on arm32 Stefano Stabellini
2021-01-05 19:20             ` Julien Grall
2021-01-05 19:15           ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) Julien Grall
2021-01-05 22:43             ` Stefano Stabellini
2021-01-06 18:29               ` Julien Grall
2021-01-06 20:55                 ` Stefano Stabellini
2021-01-08 11:12                   ` Julien Grall
2021-01-11 11:19                     ` Smoke test failure on Arm (was Re: [PATCH v4 0/8] xen/arm: Emulate ID registers) [and 2 more messages] Ian Jackson
2021-01-11 11:25                       ` Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.