* [PATCH 0/2] Refresh the dynamic CSR xml after updating the state of the cpu. @ 2023-05-23 11:44 Tommy Wu 2023-05-23 11:44 ` [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml Tommy Wu 2023-05-23 11:44 ` [PATCH 2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu Tommy Wu 0 siblings, 2 replies; 15+ messages in thread From: Tommy Wu @ 2023-05-23 11:44 UTC (permalink / raw) To: qemu-devel, qemu-riscv Cc: frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, liweiwei, zhiwei_liu, Tommy Wu The realize function of IMSIC will force select AIA feature and setup CSRs, but the dynamic CSR xml is not updated, so we cannot print the AIA CSRs in the remote gdb debugger. In the patches, we provide a function to refresh the dynamic CSR xml, and refresh the xml in the realize function of IMSIC. Tommy Wu (2): target/riscv: Add a function to refresh the dynamic CSRs xml. hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu. hw/intc/riscv_imsic.c | 4 ++++ target/riscv/cpu.h | 2 ++ target/riscv/gdbstub.c | 12 ++++++++++++ 3 files changed, 18 insertions(+) -- 2.38.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml. 2023-05-23 11:44 [PATCH 0/2] Refresh the dynamic CSR xml after updating the state of the cpu Tommy Wu @ 2023-05-23 11:44 ` Tommy Wu 2023-05-23 14:37 ` Weiwei Li 2023-05-25 2:33 ` Alistair Francis 2023-05-23 11:44 ` [PATCH 2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu Tommy Wu 1 sibling, 2 replies; 15+ messages in thread From: Tommy Wu @ 2023-05-23 11:44 UTC (permalink / raw) To: qemu-devel, qemu-riscv Cc: frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, liweiwei, zhiwei_liu, Tommy Wu When we change the cpu extension state after the cpu is realized, we cannot print the value of some CSRs in the remote gdb debugger. The root cause is that the dynamic CSR xml is generated when the cpu is realized. This patch add a function to refresh the dynamic CSR xml after the cpu is realized. Signed-off-by: Tommy Wu <tommy.wu@sifive.com> Reviewed-by: Frank Chang <frank.chang@sifive.com> --- target/riscv/cpu.h | 2 ++ target/riscv/gdbstub.c | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index de7e43126a..dc8e592275 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -494,6 +494,7 @@ struct ArchCPU { CPUNegativeOffsetState neg; CPURISCVState env; + int dyn_csr_base_reg; char *dyn_csr_xml; char *dyn_vreg_xml; @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops); void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops); void riscv_cpu_register_gdb_regs_for_features(CPUState *cs); +void riscv_refresh_dynamic_csr_xml(CPUState *cs); uint8_t satp_mode_max_from_map(uint32_t map); const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit); diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c index 524bede865..9e97ee2c35 100644 --- a/target/riscv/gdbstub.c +++ b/target/riscv/gdbstub.c @@ -230,6 +230,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) bitsize = 64; } + cpu->dyn_csr_base_reg = base_reg; + g_string_printf(s, "<?xml version=\"1.0\"?>"); g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">"); g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">"); @@ -349,3 +351,13 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs) "riscv-csr.xml", 0); } } + +void riscv_refresh_dynamic_csr_xml(CPUState *cs) +{ + RISCVCPU *cpu = RISCV_CPU(cs); + if (!cpu->dyn_csr_xml) { + g_assert_not_reached(); + } + g_free(cpu->dyn_csr_xml); + riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg); +} -- 2.38.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml. 2023-05-23 11:44 ` [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml Tommy Wu @ 2023-05-23 14:37 ` Weiwei Li 2023-05-24 1:59 ` Tommy Wu 2023-05-25 2:33 ` Alistair Francis 1 sibling, 1 reply; 15+ messages in thread From: Weiwei Li @ 2023-05-23 14:37 UTC (permalink / raw) To: Tommy Wu, qemu-devel, qemu-riscv Cc: liweiwei, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, zhiwei_liu On 2023/5/23 19:44, Tommy Wu wrote: > When we change the cpu extension state after the cpu is > realized, we cannot print the value of some CSRs in the remote > gdb debugger. The root cause is that the dynamic CSR xml is > generated when the cpu is realized. > > This patch add a function to refresh the dynamic CSR xml after > the cpu is realized. > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > Reviewed-by: Frank Chang <frank.chang@sifive.com> > --- > target/riscv/cpu.h | 2 ++ > target/riscv/gdbstub.c | 12 ++++++++++++ > 2 files changed, 14 insertions(+) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index de7e43126a..dc8e592275 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -494,6 +494,7 @@ struct ArchCPU { > CPUNegativeOffsetState neg; > CPURISCVState env; > > + int dyn_csr_base_reg; dyn_csr_base_reg seems have no additional effect on the function. And It remains unmodified in following riscv_refresh_dynamic_csr_xml(). Regards, Weiwei Li > char *dyn_csr_xml; > char *dyn_vreg_xml; > > @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops); > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops); > > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs); > +void riscv_refresh_dynamic_csr_xml(CPUState *cs); > > uint8_t satp_mode_max_from_map(uint32_t map); > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit); > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > index 524bede865..9e97ee2c35 100644 > --- a/target/riscv/gdbstub.c > +++ b/target/riscv/gdbstub.c > @@ -230,6 +230,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) > bitsize = 64; > } > > + cpu->dyn_csr_base_reg = base_reg; > + > g_string_printf(s, "<?xml version=\"1.0\"?>"); > g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">"); > g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">"); > @@ -349,3 +351,13 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs) > "riscv-csr.xml", 0); > } > } > + > +void riscv_refresh_dynamic_csr_xml(CPUState *cs) > +{ > + RISCVCPU *cpu = RISCV_CPU(cs); > + if (!cpu->dyn_csr_xml) { > + g_assert_not_reached(); > + } > + g_free(cpu->dyn_csr_xml); > + riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg); > +} ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml. 2023-05-23 14:37 ` Weiwei Li @ 2023-05-24 1:59 ` Tommy Wu 2023-05-24 2:10 ` Weiwei Li 0 siblings, 1 reply; 15+ messages in thread From: Tommy Wu @ 2023-05-24 1:59 UTC (permalink / raw) To: Weiwei Li Cc: qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, zhiwei_liu [-- Attachment #1: Type: text/plain, Size: 2991 bytes --] Hi Weiwei Li, `dyn_csr_base_reg` will be used in `riscv_refresh_dynamic_csr_xml` We can initialize this variable when the cpu is realized. And used this variable in `riscv_refresh_dynamic_csr_xml`. Best regards, Tommy On Tue, May 23, 2023 at 10:38 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > On 2023/5/23 19:44, Tommy Wu wrote: > > When we change the cpu extension state after the cpu is > > realized, we cannot print the value of some CSRs in the remote > > gdb debugger. The root cause is that the dynamic CSR xml is > > generated when the cpu is realized. > > > > This patch add a function to refresh the dynamic CSR xml after > > the cpu is realized. > > > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > > Reviewed-by: Frank Chang <frank.chang@sifive.com> > > --- > > target/riscv/cpu.h | 2 ++ > > target/riscv/gdbstub.c | 12 ++++++++++++ > > 2 files changed, 14 insertions(+) > > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > index de7e43126a..dc8e592275 100644 > > --- a/target/riscv/cpu.h > > +++ b/target/riscv/cpu.h > > @@ -494,6 +494,7 @@ struct ArchCPU { > > CPUNegativeOffsetState neg; > > CPURISCVState env; > > > > + int dyn_csr_base_reg; > > dyn_csr_base_reg seems have no additional effect on the function. > > And It remains unmodified in following riscv_refresh_dynamic_csr_xml(). > > Regards, > > Weiwei Li > > > char *dyn_csr_xml; > > char *dyn_vreg_xml; > > > > @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, > riscv_csr_operations *ops); > > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops); > > > > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs); > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs); > > > > uint8_t satp_mode_max_from_map(uint32_t map); > > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit); > > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > > index 524bede865..9e97ee2c35 100644 > > --- a/target/riscv/gdbstub.c > > +++ b/target/riscv/gdbstub.c > > @@ -230,6 +230,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, > int base_reg) > > bitsize = 64; > > } > > > > + cpu->dyn_csr_base_reg = base_reg; > > + > > g_string_printf(s, "<?xml version=\"1.0\"?>"); > > g_string_append_printf(s, "<!DOCTYPE feature SYSTEM > \"gdb-target.dtd\">"); > > g_string_append_printf(s, "<feature > name=\"org.gnu.gdb.riscv.csr\">"); > > @@ -349,3 +351,13 @@ void > riscv_cpu_register_gdb_regs_for_features(CPUState *cs) > > "riscv-csr.xml", 0); > > } > > } > > + > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs) > > +{ > > + RISCVCPU *cpu = RISCV_CPU(cs); > > + if (!cpu->dyn_csr_xml) { > > + g_assert_not_reached(); > > + } > > + g_free(cpu->dyn_csr_xml); > > + riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg); > > +} > > [-- Attachment #2: Type: text/html, Size: 4280 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml. 2023-05-24 1:59 ` Tommy Wu @ 2023-05-24 2:10 ` Weiwei Li 2023-05-24 5:35 ` Tommy Wu 0 siblings, 1 reply; 15+ messages in thread From: Weiwei Li @ 2023-05-24 2:10 UTC (permalink / raw) To: Tommy Wu Cc: liweiwei, qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, zhiwei_liu On 2023/5/24 09:59, Tommy Wu wrote: > Hi Weiwei Li, > > `dyn_csr_base_reg` will be used in `riscv_refresh_dynamic_csr_xml` > We can initialize this variable when the cpu is realized. I didn't find this initialization in following code. > And used this variable in `riscv_refresh_dynamic_csr_xml`. That's my question. In riscv_refresh_dynamic_csr_xml() , cpu->dyn_csr_base_reg is passed to riscv_gen_dynamic_csr_xml() as base_reg. And then base_reg is assigned to cpu->dyn_csr_base_reg again in it. So it's unchanged in this progress. Another question is dyn_csr_base_reg seems have no additional function currently. Regards, Weiwei Li > > Best regards, > Tommy > > > On Tue, May 23, 2023 at 10:38 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > On 2023/5/23 19:44, Tommy Wu wrote: > > When we change the cpu extension state after the cpu is > > realized, we cannot print the value of some CSRs in the remote > > gdb debugger. The root cause is that the dynamic CSR xml is > > generated when the cpu is realized. > > > > This patch add a function to refresh the dynamic CSR xml after > > the cpu is realized. > > > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > > Reviewed-by: Frank Chang <frank.chang@sifive.com> > > --- > > target/riscv/cpu.h | 2 ++ > > target/riscv/gdbstub.c | 12 ++++++++++++ > > 2 files changed, 14 insertions(+) > > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > index de7e43126a..dc8e592275 100644 > > --- a/target/riscv/cpu.h > > +++ b/target/riscv/cpu.h > > @@ -494,6 +494,7 @@ struct ArchCPU { > > CPUNegativeOffsetState neg; > > CPURISCVState env; > > > > + int dyn_csr_base_reg; > > dyn_csr_base_reg seems have no additional effect on the function. > > And It remains unmodified in following > riscv_refresh_dynamic_csr_xml(). > > Regards, > > Weiwei Li > > > char *dyn_csr_xml; > > char *dyn_vreg_xml; > > > > @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, > riscv_csr_operations *ops); > > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops); > > > > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs); > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs); > > > > uint8_t satp_mode_max_from_map(uint32_t map); > > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit); > > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > > index 524bede865..9e97ee2c35 100644 > > --- a/target/riscv/gdbstub.c > > +++ b/target/riscv/gdbstub.c > > @@ -230,6 +230,8 @@ static int > riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) > > bitsize = 64; > > } > > > > + cpu->dyn_csr_base_reg = base_reg; > > + > > g_string_printf(s, "<?xml version=\"1.0\"?>"); > > g_string_append_printf(s, "<!DOCTYPE feature SYSTEM > \"gdb-target.dtd\">"); > > g_string_append_printf(s, "<feature > name=\"org.gnu.gdb.riscv.csr\">"); > > @@ -349,3 +351,13 @@ void > riscv_cpu_register_gdb_regs_for_features(CPUState *cs) > > "riscv-csr.xml", 0); > > } > > } > > + > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs) > > +{ > > + RISCVCPU *cpu = RISCV_CPU(cs); > > + if (!cpu->dyn_csr_xml) { > > + g_assert_not_reached(); > > + } > > + g_free(cpu->dyn_csr_xml); > > + riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg); > > +} > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml. 2023-05-24 2:10 ` Weiwei Li @ 2023-05-24 5:35 ` Tommy Wu 2023-05-24 6:18 ` Weiwei Li 0 siblings, 1 reply; 15+ messages in thread From: Tommy Wu @ 2023-05-24 5:35 UTC (permalink / raw) To: Weiwei Li Cc: qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, zhiwei_liu [-- Attachment #1: Type: text/plain, Size: 4765 bytes --] Hi WeiWei Li, When the CPU is realizing, it will call `riscv_gen_dynamic_csr_xml` for the first time with the correct `base_reg` value. code flow : riscv_cpu_realize → riscv_cpu_register_gdb_regs_for_features → riscv_gen_dynamic_csr_xml The functionality of `cpu->dyn_csr_base_reg` is to record the `base_reg` from `riscv_cpu_register_gdb_regs_for_features`. When we try to refresh the dynamic CSRs xml, we will call the function `riscv_gen_dynamic_csr_xml` for the second time, and then we can give the correct `base_reg` value to the function `riscv_gen_dynamic_csr_xml`, because we've record this value in the ` cpu->dyn_csr_base_reg`. Best Regards, Tommy On Wed, May 24, 2023 at 10:10 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > On 2023/5/24 09:59, Tommy Wu wrote: > > Hi Weiwei Li, > > > > `dyn_csr_base_reg` will be used in `riscv_refresh_dynamic_csr_xml` > > We can initialize this variable when the cpu is realized. > I didn't find this initialization in following code. > > And used this variable in `riscv_refresh_dynamic_csr_xml`. > > That's my question. In riscv_refresh_dynamic_csr_xml() , > cpu->dyn_csr_base_reg is passed to riscv_gen_dynamic_csr_xml() as base_reg. > > And then base_reg is assigned to cpu->dyn_csr_base_reg again in it. So > it's unchanged in this progress. > > Another question is dyn_csr_base_reg seems have no additional function > currently. > > Regards, > > Weiwei Li > > > > > Best regards, > > Tommy > > > > > > On Tue, May 23, 2023 at 10:38 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > > > > On 2023/5/23 19:44, Tommy Wu wrote: > > > When we change the cpu extension state after the cpu is > > > realized, we cannot print the value of some CSRs in the remote > > > gdb debugger. The root cause is that the dynamic CSR xml is > > > generated when the cpu is realized. > > > > > > This patch add a function to refresh the dynamic CSR xml after > > > the cpu is realized. > > > > > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > > > Reviewed-by: Frank Chang <frank.chang@sifive.com> > > > --- > > > target/riscv/cpu.h | 2 ++ > > > target/riscv/gdbstub.c | 12 ++++++++++++ > > > 2 files changed, 14 insertions(+) > > > > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > > index de7e43126a..dc8e592275 100644 > > > --- a/target/riscv/cpu.h > > > +++ b/target/riscv/cpu.h > > > @@ -494,6 +494,7 @@ struct ArchCPU { > > > CPUNegativeOffsetState neg; > > > CPURISCVState env; > > > > > > + int dyn_csr_base_reg; > > > > dyn_csr_base_reg seems have no additional effect on the function. > > > > And It remains unmodified in following > > riscv_refresh_dynamic_csr_xml(). > > > > Regards, > > > > Weiwei Li > > > > > char *dyn_csr_xml; > > > char *dyn_vreg_xml; > > > > > > @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, > > riscv_csr_operations *ops); > > > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops); > > > > > > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs); > > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs); > > > > > > uint8_t satp_mode_max_from_map(uint32_t map); > > > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit); > > > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > > > index 524bede865..9e97ee2c35 100644 > > > --- a/target/riscv/gdbstub.c > > > +++ b/target/riscv/gdbstub.c > > > @@ -230,6 +230,8 @@ static int > > riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) > > > bitsize = 64; > > > } > > > > > > + cpu->dyn_csr_base_reg = base_reg; > > > + > > > g_string_printf(s, "<?xml version=\"1.0\"?>"); > > > g_string_append_printf(s, "<!DOCTYPE feature SYSTEM > > \"gdb-target.dtd\">"); > > > g_string_append_printf(s, "<feature > > name=\"org.gnu.gdb.riscv.csr\">"); > > > @@ -349,3 +351,13 @@ void > > riscv_cpu_register_gdb_regs_for_features(CPUState *cs) > > > "riscv-csr.xml", 0); > > > } > > > } > > > + > > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs) > > > +{ > > > + RISCVCPU *cpu = RISCV_CPU(cs); > > > + if (!cpu->dyn_csr_xml) { > > > + g_assert_not_reached(); > > > + } > > > + g_free(cpu->dyn_csr_xml); > > > + riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg); > > > +} > > > > [-- Attachment #2: Type: text/html, Size: 11889 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml. 2023-05-24 5:35 ` Tommy Wu @ 2023-05-24 6:18 ` Weiwei Li 0 siblings, 0 replies; 15+ messages in thread From: Weiwei Li @ 2023-05-24 6:18 UTC (permalink / raw) To: Tommy Wu Cc: liweiwei, qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, zhiwei_liu On 2023/5/24 13:35, Tommy Wu wrote: > > Hi WeiWei Li, > > When the CPU is realizing, it will call `riscv_gen_dynamic_csr_xml` > for the first time with the correct `base_reg` value. > > > code flow : > > riscv_cpu_realize > > → riscv_cpu_register_gdb_regs_for_features > > → riscv_gen_dynamic_csr_xml > > > The functionality of `cpu->dyn_csr_base_reg` is to record the > `base_reg` from > > `riscv_cpu_register_gdb_regs_for_features`. > > > When we try to refresh the dynamic CSRs xml, we will call the function > `riscv_gen_dynamic_csr_xml` > > for the second time, and then we can give the correct `base_reg` value > to the function > > `riscv_gen_dynamic_csr_xml`, because we've record this value in the > `cpu->dyn_csr_base_reg`. > Oh. Sorry, I stuck. Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> Weiwei Li > > Best Regards, > > Tommy > > > > On Wed, May 24, 2023 at 10:10 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > On 2023/5/24 09:59, Tommy Wu wrote: > > Hi Weiwei Li, > > > > `dyn_csr_base_reg` will be used in `riscv_refresh_dynamic_csr_xml` > > We can initialize this variable when the cpu is realized. > I didn't find this initialization in following code. > > And used this variable in `riscv_refresh_dynamic_csr_xml`. > > That's my question. In riscv_refresh_dynamic_csr_xml() , > cpu->dyn_csr_base_reg is passed to riscv_gen_dynamic_csr_xml() as > base_reg. > > And then base_reg is assigned to cpu->dyn_csr_base_reg again in > it. So > it's unchanged in this progress. > > Another question is dyn_csr_base_reg seems have no additional > function > currently. > > Regards, > > Weiwei Li > > > > > Best regards, > > Tommy > > > > > > On Tue, May 23, 2023 at 10:38 PM Weiwei Li > <liweiwei@iscas.ac.cn> wrote: > > > > > > On 2023/5/23 19:44, Tommy Wu wrote: > > > When we change the cpu extension state after the cpu is > > > realized, we cannot print the value of some CSRs in the remote > > > gdb debugger. The root cause is that the dynamic CSR xml is > > > generated when the cpu is realized. > > > > > > This patch add a function to refresh the dynamic CSR xml after > > > the cpu is realized. > > > > > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > > > Reviewed-by: Frank Chang <frank.chang@sifive.com> > > > --- > > > target/riscv/cpu.h | 2 ++ > > > target/riscv/gdbstub.c | 12 ++++++++++++ > > > 2 files changed, 14 insertions(+) > > > > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > > index de7e43126a..dc8e592275 100644 > > > --- a/target/riscv/cpu.h > > > +++ b/target/riscv/cpu.h > > > @@ -494,6 +494,7 @@ struct ArchCPU { > > > CPUNegativeOffsetState neg; > > > CPURISCVState env; > > > > > > + int dyn_csr_base_reg; > > > > dyn_csr_base_reg seems have no additional effect on the > function. > > > > And It remains unmodified in following > > riscv_refresh_dynamic_csr_xml(). > > > > Regards, > > > > Weiwei Li > > > > > char *dyn_csr_xml; > > > char *dyn_vreg_xml; > > > > > > @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, > > riscv_csr_operations *ops); > > > void riscv_set_csr_ops(int csrno, riscv_csr_operations > *ops); > > > > > > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs); > > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs); > > > > > > uint8_t satp_mode_max_from_map(uint32_t map); > > > const char *satp_mode_str(uint8_t satp_mode, bool > is_32_bit); > > > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > > > index 524bede865..9e97ee2c35 100644 > > > --- a/target/riscv/gdbstub.c > > > +++ b/target/riscv/gdbstub.c > > > @@ -230,6 +230,8 @@ static int > > riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) > > > bitsize = 64; > > > } > > > > > > + cpu->dyn_csr_base_reg = base_reg; > > > + > > > g_string_printf(s, "<?xml version=\"1.0\"?>"); > > > g_string_append_printf(s, "<!DOCTYPE feature SYSTEM > > \"gdb-target.dtd\">"); > > > g_string_append_printf(s, "<feature > > name=\"org.gnu.gdb.riscv.csr\">"); > > > @@ -349,3 +351,13 @@ void > > riscv_cpu_register_gdb_regs_for_features(CPUState *cs) > > > "riscv-csr.xml", 0); > > > } > > > } > > > + > > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs) > > > +{ > > > + RISCVCPU *cpu = RISCV_CPU(cs); > > > + if (!cpu->dyn_csr_xml) { > > > + g_assert_not_reached(); > > > + } > > > + g_free(cpu->dyn_csr_xml); > > > + riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg); > > > +} > > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml. 2023-05-23 11:44 ` [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml Tommy Wu 2023-05-23 14:37 ` Weiwei Li @ 2023-05-25 2:33 ` Alistair Francis 2023-06-09 8:37 ` Tommy Wu 1 sibling, 1 reply; 15+ messages in thread From: Alistair Francis @ 2023-05-25 2:33 UTC (permalink / raw) To: Tommy Wu Cc: qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, liweiwei, zhiwei_liu On Tue, May 23, 2023 at 9:46 PM Tommy Wu <tommy.wu@sifive.com> wrote: > > When we change the cpu extension state after the cpu is > realized, we cannot print the value of some CSRs in the remote > gdb debugger. The root cause is that the dynamic CSR xml is > generated when the cpu is realized. > > This patch add a function to refresh the dynamic CSR xml after > the cpu is realized. > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > Reviewed-by: Frank Chang <frank.chang@sifive.com> > --- > target/riscv/cpu.h | 2 ++ > target/riscv/gdbstub.c | 12 ++++++++++++ > 2 files changed, 14 insertions(+) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index de7e43126a..dc8e592275 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -494,6 +494,7 @@ struct ArchCPU { > CPUNegativeOffsetState neg; > CPURISCVState env; > > + int dyn_csr_base_reg; > char *dyn_csr_xml; > char *dyn_vreg_xml; > > @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops); > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops); > > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs); > +void riscv_refresh_dynamic_csr_xml(CPUState *cs); > > uint8_t satp_mode_max_from_map(uint32_t map); > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit); > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > index 524bede865..9e97ee2c35 100644 > --- a/target/riscv/gdbstub.c > +++ b/target/riscv/gdbstub.c > @@ -230,6 +230,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) > bitsize = 64; > } > > + cpu->dyn_csr_base_reg = base_reg; > + > g_string_printf(s, "<?xml version=\"1.0\"?>"); > g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">"); > g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">"); > @@ -349,3 +351,13 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs) > "riscv-csr.xml", 0); > } > } > + > +void riscv_refresh_dynamic_csr_xml(CPUState *cs) > +{ > + RISCVCPU *cpu = RISCV_CPU(cs); > + if (!cpu->dyn_csr_xml) { > + g_assert_not_reached(); > + } > + g_free(cpu->dyn_csr_xml); > + riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg); I don't really understand why we need dyn_csr_base_reg, could we just use cs->gdb_num_regs directly here? Alistair > +} > -- > 2.38.1 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml. 2023-05-25 2:33 ` Alistair Francis @ 2023-06-09 8:37 ` Tommy Wu 2023-06-12 2:52 ` Alistair Francis 0 siblings, 1 reply; 15+ messages in thread From: Tommy Wu @ 2023-06-09 8:37 UTC (permalink / raw) To: Alistair Francis Cc: qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, liweiwei, zhiwei_liu [-- Attachment #1: Type: text/plain, Size: 3083 bytes --] Hi Alistair, Thanks for the suggestion! Do you mean ``` ... g_free(cpu->dyn_csr_xml); riscv_gen_dynamic_csr_xml(cs, cpu-> gdb_num_regs - CSR_TABLE_SIZE); ... ``` ? Or maybe we don't need this refresh function, and just add ext_ssaia & ext_smaia in the command line. Best Regards, Tommy On Thu, May 25, 2023 at 10:33 AM Alistair Francis <alistair23@gmail.com> wrote: > On Tue, May 23, 2023 at 9:46 PM Tommy Wu <tommy.wu@sifive.com> wrote: > > > > When we change the cpu extension state after the cpu is > > realized, we cannot print the value of some CSRs in the remote > > gdb debugger. The root cause is that the dynamic CSR xml is > > generated when the cpu is realized. > > > > This patch add a function to refresh the dynamic CSR xml after > > the cpu is realized. > > > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > > Reviewed-by: Frank Chang <frank.chang@sifive.com> > > --- > > target/riscv/cpu.h | 2 ++ > > target/riscv/gdbstub.c | 12 ++++++++++++ > > 2 files changed, 14 insertions(+) > > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > index de7e43126a..dc8e592275 100644 > > --- a/target/riscv/cpu.h > > +++ b/target/riscv/cpu.h > > @@ -494,6 +494,7 @@ struct ArchCPU { > > CPUNegativeOffsetState neg; > > CPURISCVState env; > > > > + int dyn_csr_base_reg; > > char *dyn_csr_xml; > > char *dyn_vreg_xml; > > > > @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, > riscv_csr_operations *ops); > > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops); > > > > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs); > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs); > > > > uint8_t satp_mode_max_from_map(uint32_t map); > > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit); > > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > > index 524bede865..9e97ee2c35 100644 > > --- a/target/riscv/gdbstub.c > > +++ b/target/riscv/gdbstub.c > > @@ -230,6 +230,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, > int base_reg) > > bitsize = 64; > > } > > > > + cpu->dyn_csr_base_reg = base_reg; > > + > > g_string_printf(s, "<?xml version=\"1.0\"?>"); > > g_string_append_printf(s, "<!DOCTYPE feature SYSTEM > \"gdb-target.dtd\">"); > > g_string_append_printf(s, "<feature > name=\"org.gnu.gdb.riscv.csr\">"); > > @@ -349,3 +351,13 @@ void > riscv_cpu_register_gdb_regs_for_features(CPUState *cs) > > "riscv-csr.xml", 0); > > } > > } > > + > > +void riscv_refresh_dynamic_csr_xml(CPUState *cs) > > +{ > > + RISCVCPU *cpu = RISCV_CPU(cs); > > + if (!cpu->dyn_csr_xml) { > > + g_assert_not_reached(); > > + } > > + g_free(cpu->dyn_csr_xml); > > + riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg); > > I don't really understand why we need dyn_csr_base_reg, could we just > use cs->gdb_num_regs directly here? > > Alistair > > > +} > > -- > > 2.38.1 > > > > > [-- Attachment #2: Type: text/html, Size: 4188 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml. 2023-06-09 8:37 ` Tommy Wu @ 2023-06-12 2:52 ` Alistair Francis 0 siblings, 0 replies; 15+ messages in thread From: Alistair Francis @ 2023-06-12 2:52 UTC (permalink / raw) To: Tommy Wu Cc: qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, liweiwei, zhiwei_liu On Fri, Jun 9, 2023 at 6:37 PM Tommy Wu <tommy.wu@sifive.com> wrote: > > Hi Alistair, > Thanks for the suggestion! Do you mean > ``` > ... > g_free(cpu->dyn_csr_xml); > riscv_gen_dynamic_csr_xml(cs, cpu-> gdb_num_regs - CSR_TABLE_SIZE); > ... > ``` ? Yeah, pretty much. We already have cpu-> gdb_num_regs we should be able to use it directly. > > Or maybe we don't need this refresh function, and just add ext_ssaia & ext_smaia in the command line. That also works! Alistair > > Best Regards, > Tommy > > On Thu, May 25, 2023 at 10:33 AM Alistair Francis <alistair23@gmail.com> wrote: >> >> On Tue, May 23, 2023 at 9:46 PM Tommy Wu <tommy.wu@sifive.com> wrote: >> > >> > When we change the cpu extension state after the cpu is >> > realized, we cannot print the value of some CSRs in the remote >> > gdb debugger. The root cause is that the dynamic CSR xml is >> > generated when the cpu is realized. >> > >> > This patch add a function to refresh the dynamic CSR xml after >> > the cpu is realized. >> > >> > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> >> > Reviewed-by: Frank Chang <frank.chang@sifive.com> >> > --- >> > target/riscv/cpu.h | 2 ++ >> > target/riscv/gdbstub.c | 12 ++++++++++++ >> > 2 files changed, 14 insertions(+) >> > >> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h >> > index de7e43126a..dc8e592275 100644 >> > --- a/target/riscv/cpu.h >> > +++ b/target/riscv/cpu.h >> > @@ -494,6 +494,7 @@ struct ArchCPU { >> > CPUNegativeOffsetState neg; >> > CPURISCVState env; >> > >> > + int dyn_csr_base_reg; >> > char *dyn_csr_xml; >> > char *dyn_vreg_xml; >> > >> > @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops); >> > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops); >> > >> > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs); >> > +void riscv_refresh_dynamic_csr_xml(CPUState *cs); >> > >> > uint8_t satp_mode_max_from_map(uint32_t map); >> > const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit); >> > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c >> > index 524bede865..9e97ee2c35 100644 >> > --- a/target/riscv/gdbstub.c >> > +++ b/target/riscv/gdbstub.c >> > @@ -230,6 +230,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) >> > bitsize = 64; >> > } >> > >> > + cpu->dyn_csr_base_reg = base_reg; >> > + >> > g_string_printf(s, "<?xml version=\"1.0\"?>"); >> > g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">"); >> > g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">"); >> > @@ -349,3 +351,13 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs) >> > "riscv-csr.xml", 0); >> > } >> > } >> > + >> > +void riscv_refresh_dynamic_csr_xml(CPUState *cs) >> > +{ >> > + RISCVCPU *cpu = RISCV_CPU(cs); >> > + if (!cpu->dyn_csr_xml) { >> > + g_assert_not_reached(); >> > + } >> > + g_free(cpu->dyn_csr_xml); >> > + riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg); >> >> I don't really understand why we need dyn_csr_base_reg, could we just >> use cs->gdb_num_regs directly here? >> >> Alistair >> >> > +} >> > -- >> > 2.38.1 >> > >> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu. 2023-05-23 11:44 [PATCH 0/2] Refresh the dynamic CSR xml after updating the state of the cpu Tommy Wu 2023-05-23 11:44 ` [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml Tommy Wu @ 2023-05-23 11:44 ` Tommy Wu 2023-05-23 14:43 ` Weiwei Li 1 sibling, 1 reply; 15+ messages in thread From: Tommy Wu @ 2023-05-23 11:44 UTC (permalink / raw) To: qemu-devel, qemu-riscv Cc: frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, liweiwei, zhiwei_liu, Tommy Wu Originally, when we set the ext_smaia to true, we still cannot print the AIA CSRs in the remote gdb debugger, because the dynamic CSR xml is generated when the cpu is realized. This patch refreshes the dynamic CSR xml after we update the ext_smaia, so that we can print the AIA CSRs in the remote gdb debugger. Signed-off-by: Tommy Wu <tommy.wu@sifive.com> Reviewed-by: Frank Chang <frank.chang@sifive.com> --- hw/intc/riscv_imsic.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c index fea3385b51..97a51d535b 100644 --- a/hw/intc/riscv_imsic.c +++ b/hw/intc/riscv_imsic.c @@ -350,6 +350,10 @@ static void riscv_imsic_realize(DeviceState *dev, Error **errp) } else { rcpu->cfg.ext_smaia = true; } + + /* Refresh the dynamic csr xml for the gdbstub. */ + riscv_refresh_dynamic_csr_xml(cpu); + riscv_cpu_set_aia_ireg_rmw_fn(env, (imsic->mmode) ? PRV_M : PRV_S, riscv_imsic_rmw, imsic); } -- 2.38.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu. 2023-05-23 11:44 ` [PATCH 2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu Tommy Wu @ 2023-05-23 14:43 ` Weiwei Li 2023-05-24 1:51 ` Tommy Wu 0 siblings, 1 reply; 15+ messages in thread From: Weiwei Li @ 2023-05-23 14:43 UTC (permalink / raw) To: Tommy Wu, qemu-devel, qemu-riscv Cc: liweiwei, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, zhiwei_liu On 2023/5/23 19:44, Tommy Wu wrote: > Originally, when we set the ext_smaia to true, we still cannot print the > AIA CSRs in the remote gdb debugger, because the dynamic CSR xml is > generated when the cpu is realized. > > This patch refreshes the dynamic CSR xml after we update the ext_smaia, > so that we can print the AIA CSRs in the remote gdb debugger. > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > Reviewed-by: Frank Chang <frank.chang@sifive.com> > --- > hw/intc/riscv_imsic.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c > index fea3385b51..97a51d535b 100644 > --- a/hw/intc/riscv_imsic.c > +++ b/hw/intc/riscv_imsic.c > @@ -350,6 +350,10 @@ static void riscv_imsic_realize(DeviceState *dev, Error **errp) > } else { > rcpu->cfg.ext_smaia = true; > } > + > + /* Refresh the dynamic csr xml for the gdbstub. */ > + riscv_refresh_dynamic_csr_xml(cpu); > + There is an assert in riscv_refresh_dynamic_csr_xml(): + if (!cpu->dyn_csr_xml) { + g_assert_not_reached(); + } So I think riscv_refresh_dynamic_csr_xml() can only be called when cpu->dyn_csr_xml is true here. Regards, Weiwei Li > riscv_cpu_set_aia_ireg_rmw_fn(env, (imsic->mmode) ? PRV_M : PRV_S, > riscv_imsic_rmw, imsic); > } ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu. 2023-05-23 14:43 ` Weiwei Li @ 2023-05-24 1:51 ` Tommy Wu 2023-05-24 2:34 ` Weiwei Li 0 siblings, 1 reply; 15+ messages in thread From: Tommy Wu @ 2023-05-24 1:51 UTC (permalink / raw) To: Weiwei Li Cc: qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, zhiwei_liu [-- Attachment #1: Type: text/plain, Size: 1833 bytes --] Hi Weiwei Li, Yes, you're right, `riscv_refresh_dynamic_csr_xml()` can only be called when cpu->dyn_csr_xml isn't a NULL pointer here. The cpu->dyn_csr_xml will be set when the cpu is realized. Best Regards, Tommy On Tue, May 23, 2023 at 10:44 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > On 2023/5/23 19:44, Tommy Wu wrote: > > Originally, when we set the ext_smaia to true, we still cannot print the > > AIA CSRs in the remote gdb debugger, because the dynamic CSR xml is > > generated when the cpu is realized. > > > > This patch refreshes the dynamic CSR xml after we update the ext_smaia, > > so that we can print the AIA CSRs in the remote gdb debugger. > > > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > > Reviewed-by: Frank Chang <frank.chang@sifive.com> > > --- > > hw/intc/riscv_imsic.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c > > index fea3385b51..97a51d535b 100644 > > --- a/hw/intc/riscv_imsic.c > > +++ b/hw/intc/riscv_imsic.c > > @@ -350,6 +350,10 @@ static void riscv_imsic_realize(DeviceState *dev, > Error **errp) > > } else { > > rcpu->cfg.ext_smaia = true; > > } > > + > > + /* Refresh the dynamic csr xml for the gdbstub. */ > > + riscv_refresh_dynamic_csr_xml(cpu); > > + > > There is an assert in riscv_refresh_dynamic_csr_xml(): > > + if (!cpu->dyn_csr_xml) { > + g_assert_not_reached(); > + } > > So I think riscv_refresh_dynamic_csr_xml() can only be called when > cpu->dyn_csr_xml is true here. > > Regards, > > Weiwei Li > > > riscv_cpu_set_aia_ireg_rmw_fn(env, (imsic->mmode) ? PRV_M : > PRV_S, > > riscv_imsic_rmw, imsic); > > } > > [-- Attachment #2: Type: text/html, Size: 2581 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu. 2023-05-24 1:51 ` Tommy Wu @ 2023-05-24 2:34 ` Weiwei Li 2023-05-24 5:46 ` Tommy Wu 0 siblings, 1 reply; 15+ messages in thread From: Weiwei Li @ 2023-05-24 2:34 UTC (permalink / raw) To: Tommy Wu Cc: liweiwei, qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, zhiwei_liu On 2023/5/24 09:51, Tommy Wu wrote: > Hi Weiwei Li, > Yes, you're right, `riscv_refresh_dynamic_csr_xml()` can only be > called when > cpu->dyn_csr_xml isn't a NULL pointer here. > > The cpu->dyn_csr_xml will be set when the cpu is realized. Yeah, It will be set only when Zicsr is supported. And I think aia requires Zicsr. However, another question: whether we should add a check in riscv_imsic.c that it requires Zicsr? Regards, Weiwei Li > > Best Regards, > Tommy > > > On Tue, May 23, 2023 at 10:44 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > On 2023/5/23 19:44, Tommy Wu wrote: > > Originally, when we set the ext_smaia to true, we still cannot > print the > > AIA CSRs in the remote gdb debugger, because the dynamic CSR xml is > > generated when the cpu is realized. > > > > This patch refreshes the dynamic CSR xml after we update the > ext_smaia, > > so that we can print the AIA CSRs in the remote gdb debugger. > > > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > > Reviewed-by: Frank Chang <frank.chang@sifive.com> > > --- > > hw/intc/riscv_imsic.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c > > index fea3385b51..97a51d535b 100644 > > --- a/hw/intc/riscv_imsic.c > > +++ b/hw/intc/riscv_imsic.c > > @@ -350,6 +350,10 @@ static void riscv_imsic_realize(DeviceState > *dev, Error **errp) > > } else { > > rcpu->cfg.ext_smaia = true; > > } > > + > > + /* Refresh the dynamic csr xml for the gdbstub. */ > > + riscv_refresh_dynamic_csr_xml(cpu); > > + > > There is an assert in riscv_refresh_dynamic_csr_xml(): > > + if (!cpu->dyn_csr_xml) { > + g_assert_not_reached(); > + } > > So I think riscv_refresh_dynamic_csr_xml() can only be called when > cpu->dyn_csr_xml is true here. > > Regards, > > Weiwei Li > > > riscv_cpu_set_aia_ireg_rmw_fn(env, (imsic->mmode) ? > PRV_M : PRV_S, > > riscv_imsic_rmw, imsic); > > } > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu. 2023-05-24 2:34 ` Weiwei Li @ 2023-05-24 5:46 ` Tommy Wu 0 siblings, 0 replies; 15+ messages in thread From: Tommy Wu @ 2023-05-24 5:46 UTC (permalink / raw) To: Weiwei Li Cc: qemu-devel, qemu-riscv, frank.chang, alistair.francis, apatel, palmer, dbarboza, bin.meng, zhiwei_liu [-- Attachment #1: Type: text/plain, Size: 2641 bytes --] Hi Weiwei Li, You're right, it seems that we need to add a check in riscv_imsic.c Thanks for the advice! Best Regards, Tommy On Wed, May 24, 2023 at 10:35 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > On 2023/5/24 09:51, Tommy Wu wrote: > > Hi Weiwei Li, > > Yes, you're right, `riscv_refresh_dynamic_csr_xml()` can only be > > called when > > cpu->dyn_csr_xml isn't a NULL pointer here. > > > > The cpu->dyn_csr_xml will be set when the cpu is realized. > > Yeah, It will be set only when Zicsr is supported. And I think aia > requires Zicsr. > > However, another question: whether we should add a check in > riscv_imsic.c that it requires Zicsr? > > Regards, > > Weiwei Li > > > > > Best Regards, > > Tommy > > > > > > On Tue, May 23, 2023 at 10:44 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > > > > On 2023/5/23 19:44, Tommy Wu wrote: > > > Originally, when we set the ext_smaia to true, we still cannot > > print the > > > AIA CSRs in the remote gdb debugger, because the dynamic CSR xml is > > > generated when the cpu is realized. > > > > > > This patch refreshes the dynamic CSR xml after we update the > > ext_smaia, > > > so that we can print the AIA CSRs in the remote gdb debugger. > > > > > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com> > > > Reviewed-by: Frank Chang <frank.chang@sifive.com> > > > --- > > > hw/intc/riscv_imsic.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/hw/intc/riscv_imsic.c b/hw/intc/riscv_imsic.c > > > index fea3385b51..97a51d535b 100644 > > > --- a/hw/intc/riscv_imsic.c > > > +++ b/hw/intc/riscv_imsic.c > > > @@ -350,6 +350,10 @@ static void riscv_imsic_realize(DeviceState > > *dev, Error **errp) > > > } else { > > > rcpu->cfg.ext_smaia = true; > > > } > > > + > > > + /* Refresh the dynamic csr xml for the gdbstub. */ > > > + riscv_refresh_dynamic_csr_xml(cpu); > > > + > > > > There is an assert in riscv_refresh_dynamic_csr_xml(): > > > > + if (!cpu->dyn_csr_xml) { > > + g_assert_not_reached(); > > + } > > > > So I think riscv_refresh_dynamic_csr_xml() can only be called when > > cpu->dyn_csr_xml is true here. > > > > Regards, > > > > Weiwei Li > > > > > riscv_cpu_set_aia_ireg_rmw_fn(env, (imsic->mmode) ? > > PRV_M : PRV_S, > > > riscv_imsic_rmw, imsic); > > > } > > > > [-- Attachment #2: Type: text/html, Size: 3813 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-06-12 2:53 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-23 11:44 [PATCH 0/2] Refresh the dynamic CSR xml after updating the state of the cpu Tommy Wu 2023-05-23 11:44 ` [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml Tommy Wu 2023-05-23 14:37 ` Weiwei Li 2023-05-24 1:59 ` Tommy Wu 2023-05-24 2:10 ` Weiwei Li 2023-05-24 5:35 ` Tommy Wu 2023-05-24 6:18 ` Weiwei Li 2023-05-25 2:33 ` Alistair Francis 2023-06-09 8:37 ` Tommy Wu 2023-06-12 2:52 ` Alistair Francis 2023-05-23 11:44 ` [PATCH 2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu Tommy Wu 2023-05-23 14:43 ` Weiwei Li 2023-05-24 1:51 ` Tommy Wu 2023-05-24 2:34 ` Weiwei Li 2023-05-24 5:46 ` Tommy Wu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).