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:
 <PN3PR01MB9597E607DC472FA07A82CAF4B8C02@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>
Date: Mon, 24 Feb 2025 16:38:58 +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:45 PM, andriy.shevchenko@...ux.intel.com wrote:
> 
> On Mon, Feb 24, 2025 at 03:56:36PM +0000, Aditya Garg 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.
> 
> For now I think we better save the energy and wait for PRINTK people to tell
> if they are okay in general. Otherwise it makes no sense to develop and review
> something that will go to the trash bin.
> 

I'd rather keep %p4cc here for now. I don't know in detail about FOURCC tbh, so probably just let Asahi Linux manage that part if more changes are needed. In case we get better printk formats, a subsequent patch can be sent later. %p4cc still works just fine.