linux-kernel - Re: [PATCH 4/6] mips: bmips: setup: make CBR address configurable

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]

Date: Fri, 3 May 2024 14:24:49 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Christian Marangi <ansuelsmth@...il.com>,
 Florian Fainelli <florian.fainelli@...adcom.com>
Cc: Hauke Mehrtens <hauke@...ke-m.de>, Rafał Miłecki
 <zajec5@...il.com>, Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
 Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>,
 Álvaro Fernández Rojas <noltari@...il.com>,
 linux-mips@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Daniel González Cabanelas
 <dgcbueu@...il.com>
Subject: Re: [PATCH 4/6] mips: bmips: setup: make CBR address configurable

On 5/3/24 12:35, Christian Marangi wrote:
> On Fri, May 03, 2024 at 12:09:02PM -0700, Florian Fainelli wrote:
>> On 5/3/24 06:54, Christian Marangi wrote:
>>> Add support to provide CBR address from DT to handle broken
>>> SoC/Bootloader that doesn't correctly init it. This permits to use the
>>> RAC flush even in these condition.
>>>
>>> To provide a CBR address from DT, the property "mips-cbr-reg" needs to
>>> be set in the "cpus" node. On DT init, this property presence will be
>>> checked and will set the bmips_cbr_addr value accordingly. Also
>>> bmips_rac_flush_disable will be set to false as RAC flush can be
>>> correctly supported.
>>>
>>> The CBR address from DT will be applied only if the CBR address from the
>>> registers is 0, if the CBR address from the registers is not 0 and
>>> is not equal to the one set in DT (if provided) a WARN is printed.
>>>
>>> To ALWAYS overwrite the CBR address the additional property
>>> "mips-broken-cbr-reg" needs to be set.
>>>
>>> Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
>>> ---
>>>    arch/mips/bmips/setup.c | 30 +++++++++++++++++++++++++++---
>>>    1 file changed, 27 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c
>>> index 18561d426f89..bef84677248e 100644
>>> --- a/arch/mips/bmips/setup.c
>>> +++ b/arch/mips/bmips/setup.c
>>> @@ -34,7 +34,11 @@
>>>    #define REG_BCM6328_OTP		((void __iomem *)CKSEG1ADDR(0x1000062c))
>>>    #define BCM6328_TP1_DISABLED	BIT(9)
>>> -/* CBR addr doesn't change and we can cache it */
>>> +/*
>>> + * CBR addr doesn't change and we can cache it.
>>> + * For broken SoC/Bootloader CBR addr might also be provided via DT
>>> + * with "mips-cbr-reg" in the "cpus" node.
>>> + */
>>>    void __iomem *bmips_cbr_addr;
>>>    extern bool bmips_rac_flush_disable;
>>> @@ -212,8 +216,28 @@ void __init device_tree_init(void)
>>>    	/* Disable SMP boot unless both CPUs are listed in DT and !disabled */
>>>    	np = of_find_node_by_name(NULL, "cpus");
>>> -	if (np && of_get_available_child_count(np) <= 1)
>>> -		bmips_smp_enabled = 0;
>>> +	if (np) {
>>
>> Please reduce the indentation with early return/gotos. There might also be a
>> need to do some validation that the CBR is at least outside of the DRAM
>> window, that is we cannot blindly trust the DT to have gotten the CBR right
>> IMHO.
> 
> Do you have any hint on how to do the check if we are outside DRAM?
> 

I was going to suggest the use of memblock_start_of_DRAM() and 
memblock_end_of_DRAM() but before I sent that out your v2 already had it!
-- 
Florian