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

[Peter Maydell]

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.

-- PMM