On 04/11/2018 08:17 AM, Christian König wrote:
Am 11.04.2018 um 14:03 schrieb Tom St Denis:
On 04/11/2018 07:58 AM, Christian König wrote:
- if (size & 3 || *pos & 3)
+ if (size & 3 || size > (4 * AMDGPU_DEBUGFS_MAX_SGPR_READ))
I think checking the position alignment here is still necessary,
cause we can't read from not dw boundaries don't we?
The index is a dword index as fed into SQ_IND_INDEX (offset => start
=> regno as you trace from the debugfs entry to
gfx_v8_0_read_wave_sgprs to wave_read_regs (or analogues...)).
SQ_IND_INDEX doesn't take a byte offset but a dword offset, for instance:
include/asic_reg/gc/gc_9_0_offset.h:#define ixSQ_WAVE_TTMP0 0x026c
include/asic_reg/gc/gc_9_0_offset.h:#define ixSQ_WAVE_TTMP1 0x026d
include/asic_reg/gc/gc_9_0_offset.h:#define ixSQ_WAVE_TTMP2 0x026e
The current way for instance would prohibit reading (directly)
SQ_WAVE_TTMP1.
I agree it's not really how a typical file device works but it's not a
typical file device :-). It's assumed every read would be preceded by
a seek to set the higher order bits anyways.
So pos=0 is one dw and pos=1 is another one? If that's the case then
that would be a bug since pos is by definition a byte offset.
I suggest to keep the limitation and instead fix how pos is interpreted
instead.
It would break copies of umr out there already (in ways that won't be
obvious and therefore result in lost time/etc). Since umr is probably
the only user of this it's not really a big deal. Ideally, everyone
uses the latest umr each day :-) but that's not realistically the case.
I agree that had I written this today I would keep the file offset
consistent with the byte offset into the register file (shift down by 4
before calling the callback).
Since you need to seek to a very specific address to use this device
it's not the sort of thing that someone would cat and then expect
sensible output anyways.
Tom
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
