linux-kernel - Re: [PATCH v4 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs

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:
 <PN3PR01MB9597922FEF631E75EC0F0CE6B8C02@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>
Date: Mon, 24 Feb 2025 16:14:21 +0000
From: Aditya Garg <gargaditya08@...e.com>
To: "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>
CC: "maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
	"mripard@...nel.org" <mripard@...nel.org>, "tzimmermann@...e.de"
	<tzimmermann@...e.de>, "airlied@...il.com" <airlied@...il.com>,
	"simona@...ll.ch" <simona@...ll.ch>, Kerem Karabay <kekrby@...il.com>,
	Atharva Tiwari <evepolonium@...il.com>, Aun-Ali Zaidi <admin@...eit.net>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v4 2/2] drm/tiny: add driver for Apple Touch Bars in x86
 Macs



> On 24 Feb 2025, at 9:26 PM, Aditya Garg <gargaditya08@...e.com> wrote:
> 
> 
> 
>> On 24 Feb 2025, at 9:23 PM, andriy.shevchenko@...ux.intel.com wrote:
>> 
>> On Mon, Feb 24, 2025 at 03:40:29PM +0000, Aditya Garg wrote:
>>>>> On 24 Feb 2025, at 9:08 PM, andriy.shevchenko@...ux.intel.com wrote:
>>>> On Mon, Feb 24, 2025 at 03:32:56PM +0000, Aditya Garg wrote:
>>>>>> On 24 Feb 2025, at 8:50 PM, Aditya Garg <gargaditya08@...e.com> wrote:
>>>>>>> On 24 Feb 2025, at 8:41 PM, andriy.shevchenko@...ux.intel.com wrote:
>>>>>>> On Mon, Feb 24, 2025 at 03:03:40PM +0000, Aditya Garg wrote:
>>>>>>>>>> On 24 Feb 2025, at 8:27 PM, andriy.shevchenko@...ux.intel.com wrote:
>>>>>>>>> On Mon, Feb 24, 2025 at 02:32:37PM +0000, Aditya Garg wrote:
>>>>>>>>>>> On 24 Feb 2025, at 7:30 PM, andriy.shevchenko@...ux.intel.com wrote:
>>>>>>>>>>>>>> On Mon, Feb 24, 2025 at 01:40:20PM +0000, Aditya Garg wrote:
>>> 
>>> ...
>>> 
>>>>>>>>>>>>>>> +#define __APPLETBDRM_MSG_STR4(str4) ((__le32 __force)((str4[0] << 24) | (str4[1] << 16) | (str4[2] << 8) | str4[3]))
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> As commented previously this is quite strange what's going on with endianess in
>>>>>>>>>>>>>> this driver. Especially the above weirdness when get_unaligned_be32() is being
>>>>>>>>>>>>>> open coded and force-cast to __le32.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I would assume it was also mimicked from the Windows driver, though I haven't
>>>>>>>>>>>>> really tried exploring this there.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I’d rather be happy if you give me code change suggestions and let me review
>>>>>>>>>>>>> and test them
>>>>>>>>>>>> 
>>>>>>>>>>>> For the starter I would do the following for all related constants and
>>>>>>>>>>>> drop that weird and ugly macros at the top (it also has an issue with
>>>>>>>>>>>> the str4 length as it is 5 bytes long, not 4, btw):
>>>>>>>>>>>> 
>>>>>>>>>>>> #define APPLETBDRM_MSG_CLEAR_DISPLAY cpu_to_le32(0x434c5244) /* CLRD */
>>>>>>>>>> 
>>>>>>>>>> Lemme test this.
>>>>>>>>> 
>>>>>>>>> Just in case it won't work, reverse bytes in the integer. Because I was lost in
>>>>>>>>> this conversion.
>>>>>>> 
>>>>>>> It works. What I understand is that you used the macro to get the final hex and converted it into little endian, which on the x86 macs would technically remain the same.
>>>>>> 
>>>>>> And now that I oberved again, %p4cc is actually printing these CLRD, REDY etc
>>>>>> in reverse order, probably the reason %p4ch was chosen. And I am unable to
>>>>>> find what macro upstream can be used.
>>>>> 
>>>>> %.4s should work as it technically not DRM 4cc, but specifics of the protocol
>>>>> (that reminds me about ACPI that uses 4cc a lot).
>>> 
>>> I still get reverse order in that.
>> 
>> Ah, right, it will give you the first letter as LSB, indeed. At the end of the
>> day if it's so important, there are ways how to solve that without using %p4cc.
>> But if others (and esp. PRINTK maintainers) want to have / don't object having
>> that extension, why not?
> 
> Right, but what to do about the case of little endian and host endian? I remember the statement "for the sake of completeness" for them. Do you think just host endian and reverse endian should be just fine? Or you got any "no sparse warning" way to get it done? The macros to convert to le32/be32 expect a u32 value, but in those cases we actually are passing a le32/be32 value.

We can convert the __le32 to be printed by %p4cc to __be32.

__be32 expected_response_be = cpu_to_be32(le32_to_cpu(expected_response));

That's seems to be the best possible option to fix this revert.