diff mbox series

[1/2] target/riscv: Register vendors CSR

Message ID 20240130111159.532-2-zhiwei_liu@linux.alibaba.com
State New
Headers show
Series target/riscv: Support mxstatus CSR for thead-c906 | expand

Commit Message

LIU Zhiwei Jan. 30, 2024, 11:11 a.m. UTC 
riscv specification allows custom CSRs in decode area. So we should
register all vendor CSRs in cpu realize stage.

Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 target/riscv/cpu.c         |  3 +++
 target/riscv/tcg/tcg-cpu.c | 26 ++++++++++++++++++++++++++
 target/riscv/tcg/tcg-cpu.h |  1 +
 3 files changed, 30 insertions(+)

Comments

Richard Henderson Jan. 31, 2024, 5:06 a.m. UTC | #1 
On 1/30/24 21:11, LIU Zhiwei wrote:
> +/* This stub just works for making vendors array not empty */
> +riscv_csr_operations stub_csr_ops[CSR_TABLE_SIZE];
> +static inline bool never_p(const RISCVCPUConfig *cfg)
> +{
> +    return false;
> +}
> +
> +void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu)
> +{
> +    static const struct {
> +        bool (*guard_func)(const RISCVCPUConfig *);
> +        riscv_csr_operations *csr_ops;
> +    } vendors[] = {
> +        { never_p, stub_csr_ops },
> +    };
> +    for (size_t i = 0; i < ARRAY_SIZE(vendors); ++i) {

Presumably you did this to avoid a Werror for "i < 0", since i is unsigned.

It would be better to either use "int i", or

   for (size_t i = 0, n = ARRAY_SIZE(vendors); i < n; ++i)

either of which will not Werror.

Especially considering the size of stub_csr_ops.

r~
LIU Zhiwei Jan. 31, 2024, 6:14 a.m. UTC | #2 
On 2024/1/31 13:06, Richard Henderson wrote:
> On 1/30/24 21:11, LIU Zhiwei wrote:
>> +/* This stub just works for making vendors array not empty */
>> +riscv_csr_operations stub_csr_ops[CSR_TABLE_SIZE];
>> +static inline bool never_p(const RISCVCPUConfig *cfg)
>> +{
>> +    return false;
>> +}
>> +
>> +void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu)
>> +{
>> +    static const struct {
>> +        bool (*guard_func)(const RISCVCPUConfig *);
>> +        riscv_csr_operations *csr_ops;
>> +    } vendors[] = {
>> +        { never_p, stub_csr_ops },
>> +    };
>> +    for (size_t i = 0; i < ARRAY_SIZE(vendors); ++i) {
>
> Presumably you did this to avoid a Werror for "i < 0", since i is 
> unsigned.
Yes. That's the gcc complains.
>
> It would be better to either use "int i"
OK
> , or
>
>   for (size_t i = 0, n = ARRAY_SIZE(vendors); i < n; ++i)
>
> either of which will not Werror.
This works.  I don't know why it works, because n is 0 and never changes.
>
> Especially considering the size of stub_csr_ops.

Do you mean we should remove the stub_csr_ops? I don't know how to 
relate your two solving ways to stub_csr_ops.

Thanks,
Zhiwei

>
> r~
>
diff mbox series

Patch

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8cbfc7e781..2dcbc9ff32 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1113,6 +1113,9 @@  static void riscv_cpu_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (tcg_enabled()) {
+        riscv_tcg_cpu_register_vendor_csr(cpu);
+    }
     riscv_cpu_register_gdb_regs_for_features(cs);
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 994ca1cdf9..408b2ebffa 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -871,6 +871,32 @@  static void riscv_cpu_validate_profiles(RISCVCPU *cpu)
     }
 }
 
+/* This stub just works for making vendors array not empty */
+riscv_csr_operations stub_csr_ops[CSR_TABLE_SIZE];
+static inline bool never_p(const RISCVCPUConfig *cfg)
+{
+    return false;
+}
+
+void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu)
+{
+    static const struct {
+        bool (*guard_func)(const RISCVCPUConfig *);
+        riscv_csr_operations *csr_ops;
+    } vendors[] = {
+        { never_p, stub_csr_ops },
+    };
+    for (size_t i = 0; i < ARRAY_SIZE(vendors); ++i) {
+        if (!vendors[i].guard_func(&cpu->cfg)) {
+            continue;
+        }
+        for (size_t j = 0; j < CSR_TABLE_SIZE &&
+                           vendors[i].csr_ops[j].name; j++) {
+            csr_ops[j] = vendors[i].csr_ops[j];
+        }
+    }
+}
+
 void riscv_tcg_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
 {
     CPURISCVState *env = &cpu->env;
diff --git a/target/riscv/tcg/tcg-cpu.h b/target/riscv/tcg/tcg-cpu.h
index f7b32417f8..3daaebf4fb 100644
--- a/target/riscv/tcg/tcg-cpu.h
+++ b/target/riscv/tcg/tcg-cpu.h
@@ -25,5 +25,6 @@ 
 void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp);
 void riscv_tcg_cpu_finalize_features(RISCVCPU *cpu, Error **errp);
 bool riscv_cpu_tcg_compatible(RISCVCPU *cpu);
+void riscv_tcg_cpu_register_vendor_csr(RISCVCPU *cpu);
 
 #endif