Re: [Qemu-devel] [PATCH v1 1/4] target/arm64: properly handle DBGVR RESS bits

[Peter Maydell]

On 26 September 2018 at 12:20, Alex Bennée <address@hidden> wrote:
> This only fails with some (broken) versions of gdb but we should
> treat the top bits of DBGBVR as RESS. As the hardware may have IMPDEF
> approaches to writes to this register we apply the sign extension when
> checking breakpoints.
>
> Signed-off-by: Alex Bennée <address@hidden>
> ---
>  target/arm/kvm64.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index e0b8246283..80ad07ed0c 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -356,13 +356,23 @@ bool kvm_arm_hw_debug_active(CPUState *cs)
>      return ((cur_hw_wps > 0) || (cur_hw_bps > 0));
>  }
>
> +/*
> + * We shouldn't rely on gdb correctly setting the top bits of DBGBVR
> + * and the HW lists the top bits a RESS - sign-extending the top bit
> + * of the VA address. As it is IMPDEF if the write is either a sign
> + * extension or kept as is we might fix it up before we compare with
> + * the correctly reported and sign extended address.
> + */
> +
>  static bool find_hw_breakpoint(CPUState *cpu, target_ulong pc)
>  {
>      int i;
>
>      for (i = 0; i < cur_hw_bps; i++) {
>          HWBreakpoint *bp = get_hw_bp(i);
> -        if (bp->bvr == pc) {
> +        target_ulong bvr = bp->bvr;
> +        bvr |= extract64(bvr, 52, 1) ? MAKE_64BIT_MASK(53, 11) : 0;
> +        if (bvr == pc) {
>              return true;
>          }
>      }

Shouldn't we be sanitizing the addresses we get from gdb
before we put them into the hardware watchpoint registers,
rather than doing the sign extension when we read the registers?

thanks
-- PMM