Messages in this thread | | | Subject | Re: [PATCH 5/5] Powerpc/Watchpoint: Fix length calculation for unaligned target | From | Ravi Bangoria <> | Date | Wed, 19 Jun 2019 13:15:53 +0530 |
| |
On 6/18/19 7:02 PM, Michael Neuling wrote: > On Tue, 2019-06-18 at 09:57 +0530, Ravi Bangoria wrote: >> Watchpoint match range is always doubleword(8 bytes) aligned on >> powerpc. If the given range is crossing doubleword boundary, we >> need to increase the length such that next doubleword also get >> covered. Ex, >> >> address len = 6 bytes >> |=========. >> |------------v--|------v--------| >> | | | | | | | | | | | | | | | | | >> |---------------|---------------| >> <---8 bytes---> >> >> In such case, current code configures hw as: >> start_addr = address & ~HW_BREAKPOINT_ALIGN >> len = 8 bytes >> >> And thus read/write in last 4 bytes of the given range is ignored. >> Fix this by including next doubleword in the length. Watchpoint >> exception handler already ignores extraneous exceptions, so no >> changes required for that. > > Nice catch. Thanks. > > I assume this has been broken forever? Should we be CCing stable? If so, it > would be nice to have this self contained (separate from the refactor) so we can > more easily backport it.
Yes this has been broken forever. I'll add Fixes: tag and cc stable.
> > Also, can you update > tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c to catch this issue?
Sure, will add the test case.
[...]
>> +u16 hw_breakpoint_get_final_len(struct arch_hw_breakpoint *brk, >> + unsigned long *start_addr, >> + unsigned long *end_addr) > > I don't really like this. "final" is not a good name. Something like hardware > would be better. > > Also, can you put the start_addr and end addr in the arch_hw_breakpoint rather > than doing what you have above. Call them hw_start_addr, hw_end_addr. > > We could even set these two new addresses where we set the set of > arch_hw_breakpoint rather than having this late call.
Sure, will use 'hw_' prefix for them.
| |