[PATCH v3 5/5] Add two entries:

[Jessica Clarke]

On 25 Jan 2023, at 20:14, Jessica Clarke <jrtc27 at jrtc27.com> wrote:
> 
> On 25 Jan 2023, at 19:42, Lad, Prabhakar <prabhakar.csengg at gmail.com> wrote:
>> 
>> Hi Himanshu,
>> 
>> On Thu, Jan 19, 2023 at 3:19 PM Himanshu Chauhan
>> <hchauhan at ventanamicro.com> wrote:
>>> 
>>> 1. TEXT: fw_start to _fw_rw_offset with RX permissions
>>> 2. DATA: _fw_rw_offset to fw_size with RW permissions
>>> 
>>> These permissions are still not enforced from M-mode but lay
>>> the ground work for enforcing them for M-mode. SU-mode don't
>>> have any access to these regions.
>>> 
>>> Sample output:
>>> Domain0 Region01  : 0x0000000080000000-0x000000008001ffff M: (R,X) S/U: ()
>>> Domain0 Region02  : 0x0000000080020000-0x000000008003ffff M: (R,W) S/U: ()
>>> 
>>> Signed-off-by: Himanshu Chauhan <hchauhan at ventanamicro.com>
>>> ---
>>> lib/sbi/sbi_domain.c | 24 ++++++++++++++++++++++--
>>> 1 file changed, 22 insertions(+), 2 deletions(-)
>>> 
>> After updating to the latest master, RZ/Five SoC doesn't boot up.
>> Bisecting pointed me to this commit 230278d "lib: sbi: Add separate
>> entries for firmware RX and RW regions"
>> 
>> I get no prints from OpenSBI ; the only prints I get are from SPL.
>> After reverting this patch I get RZ/Five working as expected (log
>> below). Do you see any reason why this fails on RZ/Five ?
>> 
>> U-Boot SPL 2023.01-00206-g1026336bc6 (Jan 25 2023 - 19:36:23 +0000)
>> Trying to boot from NOR
>> 
>> OpenSBI v1.2-40-g031b6db
>>  ____                    _____ ____ _____
>> / __ \                  / ____|  _ \_   _|
>> | |  | |_ __   ___ _ __ | (___ | |_) || |
>> | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
>> | |__| | |_) |  __/ | | |____) | |_) || |_
>> \____/| .__/ \___|_| |_|_____/|____/_____|
>>       | |
>>       |_|
>> 
>> Platform Name             : Renesas SMARC EVK based on r9a07g043f01
>> Platform Features         : medeleg
>> Platform HART Count       : 1
>> Platform IPI Device       : andes_plicsw
>> Platform Timer Device     : andes_plmt @ 12000000Hz
>> Platform Console Device   : renesas_scif
>> Platform HSM Device       : ---
>> Platform PMU Device       : ---
>> Platform Reboot Device    : ---
>> Platform Shutdown Device  : ---
>> Firmware Base             : 0x44000000
>> Firmware Size             : 228 KB
>> Firmware RW Offset        : 0xffffffffc4020000
> 
> This is clearly nonsense. What does readelf -Ws show for _fw_rw_offset?
> 
> Worth noting that 0x44000000 + 0xffffffffc4020000 = 0x8020000, so it
> sounds like relocation inconsistency to me somehow…

Ok, figured it out, this is an LLD vs BFD linker script difference. For
whatever reason, BFD decides that . - _fw_start is not an absolute
address, whereas LLD gives you the more sensible (or at least what we
want here) behaviour, probably because . = in the various output
section statements makes it a relative address, but _fw_start remains
absolute, and so it thinks you’re trying to compute relative -
absolute, which gives a relative address. This then means that the
value located at __fw_rw_offset is subject to relocation and so loading
at an address other than 0x80000000 breaks things like you see (your
observed _fw_rw_offset value is out by 0x44000000 - 0x80000000). The
following diff should fix it (I’ve verified it with a toy example with
BFD that reproduced the symbol not being absolute); let me know if it
does and I’ll make a proper patch out of it.

Jess

diff --git a/firmware/fw_base.ldS b/firmware/fw_base.ldS
index 9a1304e..918357d 100644
--- a/firmware/fw_base.ldS
+++ b/firmware/fw_base.ldS
@@ -64,7 +64,7 @@
 	. = ALIGN(1 << LOG2CEIL((SIZEOF(.rodata) + SIZEOF(.text)
 				+ SIZEOF(.dynsym) + SIZEOF(.rela.dyn))));
 
-	PROVIDE(_fw_rw_offset = (.  - _fw_start));
+	PROVIDE(_fw_rw_offset = ABSOLUTE(. - _fw_start));
 
 	/* Beginning of the read-write data sections */
 

Jess