diff mbox series

[2/2] hw/intc: riscv_imsic: Refresh the CSRs xml after updating the state of the cpu.

Message ID 20230523114454.717708-3-tommy.wu@sifive.com (mailing list archive)
State New, archived
Headers show
Series Refresh the dynamic CSR xml after updating the state of the cpu. | expand

Commit Message

Tommy Wu May 23, 2023, 11:44 a.m. UTC
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(+)

Comments

Weiwei Li May 23, 2023, 2:43 p.m. UTC | #1
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);
>       }
Tommy Wu May 24, 2023, 1:51 a.m. UTC | #2
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);
> >       }
>
>
Weiwei Li May 24, 2023, 2:34 a.m. UTC | #3
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);
>     >       }
>
Tommy Wu May 24, 2023, 5:46 a.m. UTC | #4
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 mbox series

Patch

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);
     }