| Message ID | 20230523114454.717708-3-tommy.wu@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | Refresh the dynamic CSR xml after updating the state of the cpu. | expand |
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); > }
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); > > } > >
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); > > } >
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); > > > } > > > >
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); }