Re: [PATCH V2] mips: kernel: fix detect_memory_region() function

[Shiji Yang <yangshiji66@outlook.com>]

On Tue, 25 Jun 2024 09:46:34 +0200, Thomas Bogendoerfer wrote:
>>  	for (size = sz_min; size < sz_max; size <<= 1) {
>> -		if (!memcmp(dm, dm + size, sizeof(detect_magic)))
>> -			break;
>> +		__raw_writel(MIPS_MEM_TEST_PATTERN, dm);
>> +		if (__raw_readl(dm) == __raw_readl(dm + size)) {
>> +			__raw_writel(~MIPS_MEM_TEST_PATTERN, dm);
>> +			if (__raw_readl(dm) == __raw_readl(dm + size))
>> +				break;
>> +		}
>
>you are testing memory, so just use pointers. __raw_readl and __raw_writel
>are for io regions (which should be ioremppaed before usage).

Fixed in v3:
https://lore.kernel.org/linux-mips/TYCP286MB0895F65439037ED134FEA7DDBCD12@TYCP286MB0895.JPNP286.PROD.OUTLOOK.COM/


>
>> The detect_memory_region() has been broken on 6.6 kernel[1]. This
>> patch fixes it by:
>> 1. Do not use memcmp() on unallocated memory, as the new introduced
>>    fortify dynamic object size check[2] will return unexpected result.
>
>hmm, so there should a new way for doing memory probing without
>triggering this fortify check. How do other platforms deal with this ?

I guess __builtin_memcmp() should work. But this patch also fixes a
potential wrong memory size issue[1] by fliping magic number and
double checking it. And there is no need to ues memcmp() to check
an u32 variable.

It seems that other ARCHs directly read some registers to judge
the memory size. However, the theory in mips ARCH is checking the
overlapping memory address.

>
>> 2. Use a fixed pattern instead of a random function pointer as the
>>    magic value.
>
>why is this better ?

Just engineering experience. Byte 0xaa/0x55 has the largest information
entropy and is widely used in memory testing. Most codes in v2 patch
are copied from 'arch/mips/ralink/mt7621.c'.

>
>Thomas.
>

[1] https://github.com/openwrt/openwrt/pull/14282


Regards,
Shiji Yang