linux-kernel - Re: [PATCH 1/3] soc: apple: Add hardware tunable 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: <c8a88007-aaa0-4f8c-bf47-186b41428502@kernel.org>
Date: Sun, 2 Nov 2025 14:09:42 +0100
From: Sven Peter <sven@...nel.org>
To: Janne Grunau <j@...nau.net>
Cc: Neal Gompa <neal@...pa.dev>, Vinod Koul <vkoul@...nel.org>,
 Kishon Vijay Abraham I <kishon@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>,
 asahi@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Alyssa Rosenzweig <alyssa@...enzweig.io>
Subject: Re: [PATCH 1/3] soc: apple: Add hardware tunable support

Hi,

On 29.10.25 20:21, Janne Grunau wrote:
> Hej,
> 
> On Sun, Oct 26, 2025 at 01:52:01PM +0000, Sven Peter wrote:
>> Various hardware, like the Type-C PHY or the Thunderbolt/USB4 NHI,
>> present on Apple SoCs need machine-specific tunables passed from our
>> bootloader m1n1 to the device tree. Add generic helpers so that we
>> don't have to duplicate this across multiple drivers.
>>
>> Reviewed-by: Alyssa Rosenzweig <alyssa@...enzweig.io>
>> Reviewed-by: Neal Gompa <neal@...pa.dev>
>> Signed-off-by: Sven Peter <sven@...nel.org>

[...]

>> +
>> +	tunable = devm_kzalloc(dev,
>> +			       sizeof(*tunable) + sz * sizeof(*tunable->values),
> 
> There is a struct_size macro in linux/overflow.h for this calculation.
> We do not have to care about overflows as as struct property.length
> remains (signed) int. I would expect there is a much smaller limit for of
> properties in place anyway. The macro looks nicer though:
> 
> struct_size(tunable, values, sz)

Nice, I'll use that!

> 
>> +			       GFP_KERNEL);
>> +	if (!tunable)
>> +		return ERR_PTR(-ENOMEM);
>> +	tunable->sz = sz;
>> +
>> +	for (i = 0, p = NULL; i < tunable->sz; ++i) {
>> +		p = of_prop_next_u32(prop, p, &tunable->values[i].offset);
> 
> Does it make sense to add an size argument either here or in
> apple_tunable_apply() to check that the offset is within the expect MMIO
> region? Not really important but might catch a bug someday.

I would've usually said this was overkill but given that we just found a 
bug in our bootloader which caused us to copy random memory as tunables 
a week or two ago because of a stale fdt node id I'll add some sanity 
checks here.

> 
>> +		p = of_prop_next_u32(prop, p, &tunable->values[i].mask);
>> +		p = of_prop_next_u32(prop, p, &tunable->values[i].value);

[...]

>> +/**
>> + * Apply a previously loaded hardware tunable.
>> + *
>> + * @param regs: MMIO to which the tunable will be applied.
>> + * @param tunable: Pointer to the tunable.
>> + */
>> +void apple_tunable_apply(void __iomem *regs, struct apple_tunable *tunable);
>> +
>> +#endif
> 
> Reviewed-by: Janne Grunau <j@...nau.net>

thanks!


Sven