Re: [PATCH-for-8.0 2/4] gdbstub: Use vaddr type for generic insert/remove_breakpoint() API

[Philippe Mathieu-Daudé]

On 7/12/22 21:15, Peter Maydell wrote:
> On Wed, 7 Dec 2022 at 18:27, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > On 7/12/22 19:23, Peter Maydell wrote:
> > > On Wed, 7 Dec 2022 at 17:42, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > > > Both insert/remove_breakpoint() handlers are used in system and
> > > > user emulation. We can not use the 'hwaddr' type on user emulation,
> > > > we have to use 'vaddr' which is defined as "wide enough to contain
> > > > any #target_ulong virtual address".
>
> > > > @@ -48,8 +48,8 @@ struct AccelOpsClass {
> > > >
> > > >        /* gdbstub hooks */
> > > >        bool (*supports_guest_debug)(void);
> > > > -    int (*insert_breakpoint)(CPUState *cpu, int type, hwaddr addr, hwaddr len);
> > > > -    int (*remove_breakpoint)(CPUState *cpu, int type, hwaddr addr, hwaddr len);
> > > > +    int (*insert_breakpoint)(CPUState *cpu, int type, vaddr addr, vaddr len);
> > > > +    int (*remove_breakpoint)(CPUState *cpu, int type, vaddr addr, vaddr len);
> > > >        void (*remove_all_breakpoints)(CPUState *cpu);
> > > >    };
> > >
> > > If you're changing the prototype of these methods on AccelOpsClass
> > > don't you also want to change the implementations, eg tcg_breakpoint_insert()?
> > > Interestingly that function calls cpu_breakpoint_insert() which
> > > already takes a 'vaddr' rather than a 'hwaddr'.
> >
> > Yes I neglected to include these changes here:
> >
> > -- >8 --
> > diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> > diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c
> > diff --git a/gdbstub/user.c b/gdbstub/user.c
>
> Those are the callsites to the methods, not the implementations, I think.

Oh correct, I missed that :/ Thanks!