linux-kernel - Re: [PATCH 7/9] nvme: apple: Add Apple A11 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] [day] [month] [year] [list]

Message-ID: <e67860f1-bec4-4a55-91e8-61ade069f0a5@gmail.com>
Date: Mon, 18 Aug 2025 00:36:55 +0800
From: Nick Chan <towinchenmi@...il.com>
To: Sven Peter <sven@...nel.org>
Cc: asahi@...ts.linux.dev, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Rob Herring <robh@...nel.org>, Jassi Brar <jassisinghbrar@...il.com>,
 Neal Gompa <neal@...pa.dev>, Janne Grunau <j@...nau.net>,
 Christoph Hellwig <hch@....de>, Keith Busch <kbusch@...nel.org>,
 Robin Murphy <robin.murphy@....com>, Will Deacon <will@...nel.org>,
 Joerg Roedel <joro@...tes.org>, Sagi Grimberg <sagi@...mberg.me>,
 Hector Martin <marcan@...can.st>, Jens Axboe <axboe@...nel.dk>,
 Conor Dooley <conor+dt@...nel.org>, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 iommu@...ts.linux.dev, linux-nvme@...ts.infradead.org,
 Alyssa Rosenzweig <alyssa@...enzweig.io>
Subject: Re: [PATCH 7/9] nvme: apple: Add Apple A11 support



On 17/8/2025 18:47, Sven Peter wrote:
> On 11.08.25 15:50, Nick Chan wrote:
>> Add support for ANS2 NVMe on Apple A11 SoC.
>>
>> This version of ANS2 is less quirky than the one in M1, and does not have
>> NVMMU or Linear SQ. However, it still requires a non-standard 128-byte
>> SQE.
>>
>> Signed-off-by: Nick Chan <towinchenmi@...il.com>
>> ---
>>   drivers/nvme/host/apple.c | 228 +++++++++++++++++++++++++++++++---------------
> 
> [...]
> 
>>   }
>>   
>>   static void apple_nvme_rtkit_crashed(void *cookie, const void *crashlog, size_t crashlog_size)
>> @@ -284,21 +294,8 @@ static void apple_nvme_submit_cmd(struct apple_nvme_queue *q,
>>   				  struct nvme_command *cmd)
>>   {
> 
> Please just create a separate submit function here.
> There's just not much code that's shared between the two variants.
Will do in v2.

> 
> [...]
> 
>>   }
>>   
>> @@ -587,10 +618,17 @@ static inline void apple_nvme_handle_cqe(struct apple_nvme_queue *q,
>>   {
>>   	struct apple_nvme *anv = queue_to_apple_nvme(q);
>>   	struct nvme_completion *cqe = &q->cqes[idx];
>> -	__u16 command_id = READ_ONCE(cqe->command_id);
>>   	struct request *req;
>>   
>> -	apple_nvmmu_inval(q, command_id);
>> +	if (!anv->hw->has_lsq_nvmmu)
>> +		cqe->command_id--;
>> +
>> +	__u16 command_id = READ_ONCE(cqe->command_id);
>> +
>> +	if (anv->hw->has_lsq_nvmmu)
>> +		apple_nvmmu_inval(q, command_id);
>> +	else
>> +		command_id++;
> 
> This entire block here looks weird. First you decrease the command_id
> directly inside the shared memory structure, then you read it with
> READ_ONCE to a local variable only to increase it again. Why?
Thanks for spotting! Looks like this is merely an artifact of how the code
is hacked to work that slipped under my radar, so I will remove the useless
codes in v2.

> 
> 
> 
> Sven

Nick Chan