linux-kernel - Re: [PATCH] drm/ast: Fix big-endian support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20251211.153101.411672428832661296.rene@exactco.de>
Date: Thu, 11 Dec 2025 15:31:01 +0100 (CET)
From: René Rebe <rene@...ctco.de>
To: tzimmermann@...e.de
Cc: tpearson@...torengineering.com, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, airlied@...hat.com
Subject: Re: [PATCH] drm/ast: Fix big-endian support

Hi,

On Thu, 11 Dec 2025 15:03:48 +0100, Thomas Zimmermann <tzimmermann@...e.de> wrote:

> Hi,
> 
> Am 11.12.25 um 13:43 schrieb René Rebe:
> [...]
> >> The code for the primary plane should be fine now. But we also need
> >> something for the cursor plane as well. There's a
> >> ast_set_cursor_image() with a memcpy_toio() [1] and several additional
> >> writes. IIUC they all have to be swapped as well.
> > Of course, any obvious style issue or endianess swapping linux-kernel
> > would like to see differently? You did not answer if I should just
> > conditionalize on the chip id. I used a bool to avoid intermangled
> > #ifdef conditionals to hopefully match kernel style.
> > Btw. checkpatch.pl warns:
> >
> > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> >
> > I could add this if desired while at it.
> >
> > Only compile tested, will do a final hw test once patch is approved in
> > general.
> 
> It's all a bit excessive. There's a patch attached that will hopefully
> fix the issues.
> 
> If you could test it, I'll send it out for official review. The
> easiest way of testing cursor support is to run Xorg and see if the
> cursor looks correct.
> 
> The Co-developed-by tag requires your Signed-off-by.

Ok, so you are not a fan of using the hw swapping. I think I asked two
emails ago if having both pathes is acceptable. To be honest this
driver is for some reason already annoyingly slow. Buf of course we
can just keep the sw swapping for now.

>  	/* write checksum + signature */
> +	writel(swab32(csum), dst);
> +	writel(swab32(width), dst + AST_HWC_SIGNATURE_SizeX);
> +	writel(swab32(height), dst + AST_HWC_SIGNATURE_SizeY);
> +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTX);
> +	writel(swab32(0), dst + AST_HWC_SIGNATURE_HOTSPOTY);
> +#else
> +	memcpy_toio(dst, src, AST_HWC_SIZE);
>  	dst += AST_HWC_SIZE;
> +
> +	/* write checksum + signature */
>  	writel(csum, dst);
>  	writel(width, dst + AST_HWC_SIGNATURE_SizeX);
>  	writel(height, dst + AST_HWC_SIGNATURE_SizeY);
>  	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTX);
>  	writel(0, dst + AST_HWC_SIGNATURE_HOTSPOTY);
> +#endif

I'm pretty sure this will break the cursor, as the position was
working correctly and I only had to swap the cursor image data. The
csum will also not be identical anyway, as the checksum function
computes it in native byte order. Theoretically that would have to be
changed. However, I do not see where it is really used, maybe only
some special remote desktop vendor protocol that I'm not using. Maybe
the exact checksum does not even matter and is only used as
optimization to not resend an unchanged cursor image.

I'll send a final version after validating it w/ HW later.

Thanks,
	René

-- 
René Rebe, ExactCODE GmbH, Berlin, Germany
https://exactco.de • https://t2linux.com • https://patreon.com/renerebe