Re: [PATCH 3/3] [AArch64] Remove tag from address for watchpoint

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2017-11-09 15:30, Pedro Alves wrote:
> On 10/26/2017 09:29 AM, Yao Qi wrote:
> 
>> +typedef CORE_ADDR (gdbarch_addr_tag_remove_ftype) (struct gdbarch 
>> *gdbarch, CORE_ADDR addr);
>> +extern CORE_ADDR gdbarch_addr_tag_remove (struct gdbarch *gdbarch, 
>> CORE_ADDR addr);
>> +extern void set_gdbarch_addr_tag_remove (struct gdbarch *gdbarch, 
>> gdbarch_addr_tag_remove_ftype *addr_tag_remove);
>> +
>>  /* FIXME/cagney/2001-01-18: This should be split in two.  A target 
>> method that
>>     indicates if the target needs software single step.  An ISA method 
>> to
>>     implement it.
>> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
>> index 6459b12..1f673e7 100755
>> --- a/gdb/gdbarch.sh
>> +++ b/gdb/gdbarch.sh
>> @@ -621,6 +621,11 @@ m;CORE_ADDR;convert_from_func_ptr_addr;CORE_ADDR 
>> addr, struct target_ops *targ;a
>>  # possible it should be in TARGET_READ_PC instead).
>>  m;CORE_ADDR;addr_bits_remove;CORE_ADDR 
>> addr;addr;;core_addr_identity;;0
>> 
>> +# On some machines, there are bits in address which are ignored by 
>> the
>> +# kernel, the hardeware, etc.  They are called "tag", which can be
>> +# regarded as additional data associated with the address.
>> +m;CORE_ADDR;addr_tag_remove;CORE_ADDR 
>> addr;addr;;core_addr_identity;;0
> 
> typo: "hardeware".
> 
> Hmmm.  We have gdbarch_addr_bit / addr_bit to represent the size
> of a target address.  I'm thinking that instead of addr_tag_remove,
> this would a bit more in line with the current scheme if this were
> a new "significant_addr_bit" gdbarch property?  I.e.:
> 
>  /* On some machines, not all bits of an address word are significant.
>     For example, on Aarch64, the top bits of an address known as the 
> "tag"
>     are ignored by the kernel, the hardware, etc. and can be regarded 
> as
>     additional data associated with the address.  */
>  int gdbarch_significant_addr_bit (struct gdbarch *gdbarch);
> 
> significant_addr_bit would default to addr_bit.
> 
> And then at places where we need to save or compare memory addresses,
> like in the watchpoint location addresses case we strip out / ignore
> non-significant bits.
> 
> And the next question is: if you're adding a gdbarch hook such as
> this one (either significant_addr_bit or addr_tag_remove)
> why not use it for all the cases handled by the different patches in
> this series, instead of using different solutions for each case?
> I.e., for memory access, saving breakpoint and watchpoint
> location addresses, the dcache, and any other future case we run
> into, like e.g., maybe agent expressions.
> 
> Thanks,
> Pedro Alves

There's gdbarch_addr_bits_remove already as well, I don't know if that 
has the same goal.

Simon