On 2021-12-11 5:39 a.m., Mark Kettenis wrote:
>> Date: Sat, 11 Dec 2021 05:10:41 -0700
>> From: Ted Bullock <[email protected]>
>
> The are several reasons why that test can fail though. It can be an
> endian-ness issue or on sparc64 it could also be an IOMMU issue where
> the wrong address is programmed into the hardware because CPU
> addresses aren't properly translated into device virtual addresses.
>
Trying to figure out what the hell is happening here is making my eyes
bleed a little... there are lots of preprocessor stuff in this code
that looks fragile to me. I've not written much in the last few years
but surely this isn't a normal way of programming or maybe the authors
are smarter than me. :( Anyway I'm looking for where things could get
broken.
>> sys/dev/pci/drm/radeon/r100.c:3651
>> WREG32(scratch, 0xCAFEDEAD);
Starting here this is a macro that calls an inline function:
#define WREG32(reg, v) r100_mm_wreg(rdev, (reg), (v), false)
fwiw r100_mm_wreg is called only by one other thing, the macro:
#define WREG32_IDX(reg, v) r100_mm_wreg(rdev, (reg), (v), true)
I don't know why they wrapped an inline function that is called in only
2 different ways behind a macro but they did so ok, then looking at
r100_mm_wreg:
static inline void r100_mm_wreg(struct radeon_device *rdev, uint32_t reg,
uint32_t v,
bool always_indirect)
{
if ((reg < rdev->rmmio_size || reg < RADEON_MIN_MMIO_SIZE) &&
!always_indirect)
writel(v, ((void __iomem *)rdev->rmmio) + reg);
else
r100_mm_wreg_slow(rdev, reg, v);
}
This has some pointer math but this doesn't look like it has anything to
cause endian issues, so I suppose it's fine.
>> r = radeon_ring_lock(rdev, ring, 2);
>> if (r) {
>> DRM_ERROR("radeon: cp failed to lock ring (%d).\n", r);
>> radeon_scratch_free(rdev, scratch);
>> return r;
>> }
This locking stuff has nothing that looks problematic to me for endian
issues.
>> radeon_ring_write(ring, PACKET0(scratch, 0));
Until we get to this ^, this is kind of a nightmare for me to grasp.
The driver uses ring, or circular queue data structures as I'm familiar
with for interacting with the gpu, work items are written to the queue
and read by the gpu. The queue code doesn't look like it should have
endian issues and obviously it's working on other platforms so can
probably ignore it. It's weird to me that linux, bsd et al have circular
queue data structures pre-rolled so I don't know why they made their
own, perhaps ignorance or hubris, or they are super smart?
PACKET0(scratch, 0) this is kind of a monster though.
#define PACKET0(reg, n) (CP_PACKET0 | \
REG_SET(PACKET0_BASE_INDEX, (reg) >> 2) | \
REG_SET(PACKET0_COUNT, (n)))
and also here:
#define REG_SET(FIELD, v) (((v) << FIELD##_SHIFT) & FIELD##_MASK)
Think that maybe this is a big candidate for some sort of endian bug? I
do. hmmm. This looks insanely fragile but maybe I'm crazy and probably a
moron or something.
>> radeon_ring_write(ring, 0xDEADBEEF);
>> radeon_ring_unlock_commit(rdev, ring, false);
>> for (i = 0; i < rdev->usec_timeout; i++) {
>> tmp = RREG32(scratch);
>> if (tmp == 0xDEADBEEF) {
>> break;
>> }
>> udelay(1);
>> }
>> if (i < rdev->usec_timeout) {
>> DRM_INFO("ring test succeeded in %d usecs\n", i);
>> } else {
>> DRM_ERROR("radeon: ring test failed (scratch(0x%04X)=0x%08X)\n",
>> scratch, tmp);
>> r = -EINVAL;
>> }
--
Ted Bullock <[email protected]>
