linux-kernel - Re: [PATCH v2 6/6] MIPS: cm: Probe GCR address from DeviceTree

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: <7j6cc5i4z4nwg73fowjz756eblnesglqm72jveygqfxngw26mc@sdy6xxomo3qe>
Date: Tue, 10 Sep 2024 23:07:37 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Jiaxun Yang <jiaxun.yang@...goat.com>
Cc: Thomas Bogendoerfer <tsbogend@...ha.franken.de>, 
	"paulburton@...nel.org" <paulburton@...nel.org>, "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>, 
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 6/6] MIPS: cm: Probe GCR address from DeviceTree

On Tue, Sep 10, 2024 at 08:23:25PM +0100, Jiaxun Yang wrote:
> 
> 
> 在2024年9月10日九月 下午1:36，Serge Semin写道：
> [...]
> >
> > This causes the kernel boot-up procedure to crash/hang-up because the
> > CM GCR base address is supposed to be redefined by means of the
> > already mapped CM GCR address space by accessing the
> > CM_GCR_BASE_GCRBASE register:
> > change_gcr_base()
> > +-> read_gcr_base()
> >     +-> addr_gcr_base()
> >         +-> return mips_gcr_base + CM_GCR_BASE_GCRBASE
> >
> > By the time of the change_gcr_base() call in mips_cm_phys_base(), the
> > mips_gcr_base variable hasn't been defined. So the IO operations
> > performed in the change_gcr_base() method would be accessing the
> > NULL-based memory space. That's why the kernel crash/hanging-up.
> 
> Thanks for the analysis!
> This path was not taken on my audience hardware, so I didn't catch this,
> will fix in next version.
> 
> >
> > In order to fix this we have to first map the CM GCR block at the
> > default base-address, then update the CM GCR-base CSR and after that
> > remap the CM GCR-space.
> >
> > Please also note, the GCR_BASE field might be RO. It depends on the
> > IP-core configuration. So it's possible that the CM_GCR_BASE_GCRBASE
> > field update won't work. Although that will be detected a bit later in
> > the mips_cm_probe() method by comparing the address returned from
> > mips_cm_phys_base() and retrieved from the CM GCR-base CSR.
> 

> Hmm, I just checked RTL and RDL for CM2 and CM3 and I didn't see it as a
> configurable option. It's possible to change hardware reset value but not make it RO.

Both MIPS P5600 and P6600 databooks define the GCR_BASE field as
optionally R/W:

GCR_BASE 31:15 This field sets the base address of the 32KB          R or R/W
               GCR block of the P5600 MPS.                           (IP Config-
               This register has a fixed value after reset if         uration)
               configured as Read-Only (an IP Configuration Option).

> 
> Maybe it was possible on earlier IP release,

I found the text above in the latest MIPS Warrior P-class software
manuals downloaded from the ImagTech site. Not sure why your RTL code
doesn't have such configs.

> in this case it's always
> user's responsibility to write correct address in DeviceTree :-)

Right. The system just won't work if the CM GCR base address couldn't
be updated.

-Serge(y)

> 
> Thanks
> 
> >
> > -Serge(y)
> 
> -- 
> - Jiaxun