[Top][All Lists]

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

Re: [PATCH v4 17/17] target/riscv: actual functions to realize crs 128-b

From: Richard Henderson
Subject: Re: [PATCH v4 17/17] target/riscv: actual functions to realize crs 128-bit insns
Date: Tue, 2 Nov 2021 09:22:14 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0

On 10/25/21 8:28 AM, Frédéric Pétrot wrote:
+typedef RISCVException (*riscv_csr_op128_fn)(CPURISCVState *env, int csrno,
+                                             Int128 *ret_value,
+                                             Int128 new_value,
+                                             Int128 write_mask);
 typedef struct {
     const char *name;
     riscv_csr_predicate_fn predicate;
     riscv_csr_read_fn read;
     riscv_csr_write_fn write;
     riscv_csr_op_fn op;
+    riscv_csr_read128_fn read128;
+    riscv_csr_write128_fn write128;
+    riscv_csr_op128_fn op128;

I think you should drop op128 until there is a need for it.

+static RISCVException read_mstatus_i128(CPURISCVState *env, int csrno,
+                                        Int128 *val)
+    *val = int128_make128(env->mstatus, add_status_sd(riscv_cpu_mxl(env), 0));
+    return RISCV_EXCP_NONE;

Well, you don't need to read mxl here, as you cannot arrive to this function without it being MXL_RV128.

I think you need a similar read_sstatus_i128 function.

-    /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
+    /* check privileges and return -1 if check fails */

Bad rebase change; the return type is RISCVException.

+    /* write value if writable and write mask set, otherwise drop writes */
+    if (int128_nz(write_mask)) {
+        new_value = int128_or(int128_and(old_value, int128_not(write_mask)),
+                              int128_and(new_value, write_mask));
+        if (csr_ops[csrno].write128) {
+            ret = csr_ops[csrno].write128(env, csrno, new_value);
+            if (ret != RISCV_EXCP_NONE) {
+                return ret;
+            }
+        }
+    }

This is wrong; you certainly don't want to drop writes; you want to forward writes to the 64-bit version. At least...

+    [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     
write_mstatus, NULL,
+                                               read_mstatus_i128               
+    [CSR_MISA]        = { "misa",       any,   read_misa,        write_misa, 
+                                               read_misa_i128                  

... that is certainly the case for these CSRs. Alternately, you've got to implement these write functions.


reply via email to

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