Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description

[Alistair Francis <alistair23@gmail.com>]

On Thu, Apr 21, 2022 at 9:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Thu, Apr 21, 2022 at 6:45 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Wed, Apr 20, 2022 at 7:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Alistair,
> > >
> > > On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > > >
> > > > > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > From: Bin Meng <bin.meng@windriver.com>
> > > > > >
> > > > > > Add a subsection to machine.c to migrate debug CSR state.
> > > > > >
> > > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > > > > > ---
> > > > > >
> > > > > > (no changes since v2)
> > > > > >
> > > > > > Changes in v2:
> > > > > > - new patch: add debug state description
> > > > > >
> > > > > >  target/riscv/machine.c | 32 ++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 32 insertions(+)
> > > > > >
> > > > > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > > > > index 5178b3fec9..4921dad09d 100644
> > > > > > --- a/target/riscv/machine.c
> > > > > > +++ b/target/riscv/machine.c
> > > > > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > > > > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > > > > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > > > > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > > > > +        VMSTATE_END_OF_LIST()
> > > > > > +    }
> > > > > > +};
> > > > > > +
> > > > > > +static bool debug_needed(void *opaque)
> > > > > > +{
> > > > > > +    RISCVCPU *cpu = opaque;
> > > > > > +    CPURISCVState *env = &cpu->env;
> > > > > > +
> > > > > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > >
> > > > > This fails to build:
> > > > >
> > > > > ../target/riscv/machine.c: In function ‘debug_needed’:
> > > > > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > > > > undeclared (first use in this function); did you mean
> > > > > ‘RISCV_FEATURE_EPMP’?
> > > > >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > >      |                               ^~~~~~~~~~~~~~~~~~~
> > > > >      |                               RISCV_FEATURE_EPMP
> > > > > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > > > > reported only once for each function it appears in
> > > > > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > > > > non-void function [-Wreturn-type]
> > > > >  229 | }
> > > > >      | ^
> > > >
> > > > That's weird. Maybe it's out of sync or merge conflict? I will take a look.
> > > >
> > >
> > > I rebased the v4 series on top of your riscv-to-apply.next branch,
> > > indeed there is a merge conflict of target/riscv/machine.c. After I
> > > resolved the conflict, the build succeeded.
> >
> > Looking at this patch series RISCV_FEATURE_DEBUG is only defined in
> > patch 4, it doesn't currently exist in the tree. I'm not sure how this
> > can build.
>
> Ah, it looks like I should adjust the patch order to have patch 4 come first.
>
> >
> > Are you sure you looked at just this patch and not the entire series?
>
> I see. I was looking at the series not this patch.
>
> It seems you were trying to build every commit for bisectabliity? Is
> there an easy way to do such automatically?

Yep, I build test every patch.

I do this automatically with an internal Jenkins server, unfortunately
I can't really share it publically

Alistair

>
> >
> > >
> > > I suspect you missed something during your handling of the merge conflict?
> >
> > That's entirely possible. Can you send a rebased version please
>
> Regards,
> Bin