[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





On 10/12/2020 02:30, Stefano Stabellini wrote:
On Wed, 9 Dec 2020, Julien Grall wrote:
Hi Bertrand,

On 09/12/2020 16:30, 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.
Also define a case to catch all reserved registers that should be
handled as RAZ.

Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
---
Changes in V2: Rebase
Changes in V3:
    Add case definition for reserved registers.

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

diff --git a/xen/include/asm-arm/arm64/hsr.h
b/xen/include/asm-arm/arm64/hsr.h
index ca931dd2fe..ffe0f0007e 100644
--- a/xen/include/asm-arm/arm64/hsr.h
+++ b/xen/include/asm-arm/arm64/hsr.h
@@ -110,6 +110,72 @@
   #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)
+
+/*
+ * 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}.
+ */
+#define HSR_SYSREG_TID3_RESERVED_CASE  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)

I don't like the idea to define the list of case in a header that is used by
multiple source. Please define it directly in the source file that use it.

At that point it might be best to open-coding it in do_sysreg? I mean no
#define at all.

I am happpy with that.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.