qemu-devel
[Top][All Lists]
Advanced

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

Re: [RFC PATCH v2 02/30] target/loongarch: Add CSR registers definition


From: Richard Henderson
Subject: Re: [RFC PATCH v2 02/30] target/loongarch: Add CSR registers definition
Date: Thu, 11 Nov 2021 14:29:06 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0

On 11/11/21 2:35 AM, Xiaojuan Yang wrote:
+#define LOONGARCH_CSR_MISC           0x3 /* Misc config */
+

Missing bitfield definitions for misc.

+#define  EXCODE_IP                   64

What's this?

+#define  EXCCODE_INT                 0
+#define  EXCCODE_PIL                 1
+#define  EXCCODE_PIS                 2
+#define  EXCCODE_PIF                 3
+#define  EXCCODE_PME                 4
+#define  EXCCODE_PNR                 5
+#define  EXCCODE_PNX                 6
+#define  EXCCODE_PPI                 7
+#define  EXCCODE_ADE                 8

ADEF vs ADEM?

+#define  EXCCODE_ALE                 9
+#define  EXCCODE_BCE                 10
+#define  EXCCODE_SYS                 11
+#define  EXCCODE_BRK                 12
+#define  EXCCODE_INE                 13
+#define  EXCCODE_IPE                 14
+#define  EXCCODE_FPD                 15
+#define  EXCCODE_SXD                 16
+#define  EXCCODE_ASXD                17
+#define  EXCCODE_FPE                 18 /* Have different expsubcode */
+#define  EXCCODE_VFPE                18
+#define  EXCCODE_WPEF                19 /* Have different expsubcode */
+#define  EXCCODE_WPEM                19
+#define  EXCCODE_BTD                 20
+#define  EXCCODE_BTE                 21

Missing BSPR, HVC, GCSC, GCHC.

+#define LOONGARCH_CSR_ERA            0x6 /* ERA */

Not really helpful to name the acronym with the acronym.
"Exception return address".

+#define LOONGARCH_CSR_TLBELO0        0x12 /* TLB EntryLo0 */
+FIELD(CSR_TLBELO0, V, 0, 1)
+FIELD(CSR_TLBELO0, D, 1, 1)
+FIELD(CSR_TLBELO0, PLV, 2, 2)
+FIELD(CSR_TLBELO0, MAT, 4, 2)
+FIELD(CSR_TLBELO0, G, 6, 1)
+FIELD(CSR_TLBELO0, PPN, 12, 36)
+FIELD(CSR_TLBELO0, NR, 61, 1)
+FIELD(CSR_TLBELO0, NX, 62, 1)
+FIELD(CSR_TLBELO0, RPLV, 63, 1)
+
+#define LOONGARCH_CSR_TLBELO1        0x13 /* 64 TLB EntryLo1 */
+FIELD(CSR_TLBELO1, V, 0, 1)
+FIELD(CSR_TLBELO1, D, 1, 1)
+FIELD(CSR_TLBELO1, PLV, 2, 2)
+FIELD(CSR_TLBELO1, MAT, 4, 2)
+FIELD(CSR_TLBELO1, G, 6, 1)
+FIELD(CSR_TLBELO1, PPN, 12, 36)
+FIELD(CSR_TLBELO1, NR, 61, 1)
+FIELD(CSR_TLBELO1, NX, 62, 1)
+FIELD(CSR_TLBELO1, RPLV, 63, 1)

Better to define the fields once, dropping the 0/1 from the name.

+#define LOONGARCH_CSR_PWCL           0x1c /* PWCl */

"Page walk controller, low addr"

+#define LOONGARCH_CSR_PWCH           0x1d /* PWCh */

"Page walk controller, high addr"

+#define LOONGARCH_CSR_STLBPS     0x1e /* 64 */

64?  "STLB Page size".

+#define LOONGARCH_CSR_RVACFG         0x1f

"Reduced virtual address config"

+/* Save registers */
+#define LOONGARCH_CSR_SAVE0            0x30
+#define LOONGARCH_CSR_SAVE1            0x31
+#define LOONGARCH_CSR_SAVE2            0x32
+#define LOONGARCH_CSR_SAVE3            0x33
+#define LOONGARCH_CSR_SAVE4            0x34
+#define LOONGARCH_CSR_SAVE5            0x35
+#define LOONGARCH_CSR_SAVE6            0x36
+#define LOONGARCH_CSR_SAVE7            0x37

Might as well must define SAVE0, and comment that the count is in 
PRCFG1.SAVE_NUM.

+#define  CSR_DMW_BASE_SH             48

What's this?  It looks like you should be using TARGET_VIRT_ADDR_SPACE_BITS 
anyway.

+#define dmwin_va2pa(va) \
+    (va & (((unsigned long)1 << CSR_DMW_BASE_SH) - 1))

Using unsigned long is wrong, breaking 32-bit hosts.
You want

    ((va) & MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS))

+/* Performance Counter registers */
+#define LOONGARCH_CSR_PERFCTRL0      0x200 /* 32 perf event 0 config */
+#define LOONGARCH_CSR_PERFCNTR0      0x201 /* 64 perf event 0 count value */
+#define LOONGARCH_CSR_PERFCTRL1      0x202 /* 32 perf event 1 config */
+#define LOONGARCH_CSR_PERFCNTR1      0x203 /* 64 perf event 1 count value */
+#define LOONGARCH_CSR_PERFCTRL2      0x204 /* 32 perf event 2 config */
+#define LOONGARCH_CSR_PERFCNTR2      0x205 /* 64 perf event 2 count value */
+#define LOONGARCH_CSR_PERFCTRL3      0x206 /* 32 perf event 3 config */
+#define LOONGARCH_CSR_PERFCNTR3      0x207 /* 64 perf event 3 count value */

Perhaps better to define

#define LOONGARCH_CSR_PERFCTRL(N)  (0x200 + 2 * N)

etc.

+#define LOONGARCH_CSR_DB0ADDR        0x310 /* data breakpoint 0 address */
+#define LOONGARCH_CSR_DB0MASK        0x311 /* data breakpoint 0 mask */
+#define LOONGARCH_CSR_DB0CTL         0x312 /* data breakpoint 0 control */
+#define LOONGARCH_CSR_DB0ASID        0x313 /* data breakpoint 0 asid */

Likewise.

+    uint64_t CSR_SAVE0;
+    uint64_t CSR_SAVE1;
+    uint64_t CSR_SAVE2;
+    uint64_t CSR_SAVE3;
+    uint64_t CSR_SAVE4;
+    uint64_t CSR_SAVE5;
+    uint64_t CSR_SAVE6;
+    uint64_t CSR_SAVE7;

Better as an array.

+    uint64_t CSR_PERFCTRL0;
+    uint64_t CSR_PERFCNTR0;
+    uint64_t CSR_PERFCTRL1;
+    uint64_t CSR_PERFCNTR1;
+    uint64_t CSR_PERFCTRL2;
+    uint64_t CSR_PERFCNTR2;
+    uint64_t CSR_PERFCTRL3;
+    uint64_t CSR_PERFCNTR3;

Likewise.

+    uint64_t CSR_DB0ADDR;
+    uint64_t CSR_DB0MASK;
+    uint64_t CSR_DB0CTL;
+    uint64_t CSR_DB0ASID;
+    uint64_t CSR_DB1ADDR;
+    uint64_t CSR_DB1MASK;
+    uint64_t CSR_DB1CTL;
+    uint64_t CSR_DB1ASID;
+    uint64_t CSR_DB2ADDR;
+    uint64_t CSR_DB2MASK;
+    uint64_t CSR_DB2CTL;
+    uint64_t CSR_DB2ASID;
+    uint64_t CSR_DB3ADDR;
+    uint64_t CSR_DB3MASK;
+    uint64_t CSR_DB3CTL;
+    uint64_t CSR_DB3ASID;

Likewise.

+    uint64_t CSR_IB0ADDR;
+    uint64_t CSR_IB0MASK;
+    uint64_t CSR_IB0CTL;
+    uint64_t CSR_IB0ASID;
+    uint64_t CSR_IB1ADDR;
+    uint64_t CSR_IB1MASK;
+    uint64_t CSR_IB1CTL;
+    uint64_t CSR_IB1ASID;
+    uint64_t CSR_IB2ADDR;
+    uint64_t CSR_IB2MASK;
+    uint64_t CSR_IB2CTL;
+    uint64_t CSR_IB2ASID;
+    uint64_t CSR_IB3ADDR;
+    uint64_t CSR_IB3MASK;
+    uint64_t CSR_IB3CTL;
+    uint64_t CSR_IB3ASID;
+    uint64_t CSR_IB4ADDR;
+    uint64_t CSR_IB4MASK;
+    uint64_t CSR_IB4CTL;
+    uint64_t CSR_IB4ASID;
+    uint64_t CSR_IB5ADDR;
+    uint64_t CSR_IB5MASK;
+    uint64_t CSR_IB5CTL;
+    uint64_t CSR_IB5ASID;
+    uint64_t CSR_IB6ADDR;
+    uint64_t CSR_IB6MASK;
+    uint64_t CSR_IB6CTL;
+    uint64_t CSR_IB6ASID;
+    uint64_t CSR_IB7ADDR;
+    uint64_t CSR_IB7MASK;
+    uint64_t CSR_IB7CTL;
+    uint64_t CSR_IB7ASID;

Likewise.


r~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]