Split RX and RW regions for separate pmp entries

Split the RO/RX and RW sections so that they can have independent pmp entries with required permissions. The split size is ensured to be a power-of-2 as required by pmp.

_fw_rw_offset symbol marks the beginning of the data section.

Signed-off-by Himanshu Chauhan <[email protected]>

---
 firmware/fw_base.ldS | 8 ++++++++
 1 file changed, 8 insertions(+)

firmware/fw_base.ldS +8 -1

index 563ae25..5411bbd 100644
@@ -42,6 +42,14 @@
 
        . = ALIGN(0x1000); /* Ensure next section is page aligned */
 
+       /*
+        * PMP regions must be to be power-of-2. RO/RW will have separate
+        * regions, so ensure that the split is power-of-2.
+        */
+       . = ALIGN(1 << LOG2CEIL(SIZEOF(.rodata) + SIZEOF(.text)));
+
+       PROVIDE(_fw_rw_offset = (.  - _fw_start));
+
        /* Beginning of the read-write data sections */
 
        .data :
--

2.39.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Currently, the dynsym and reladyn sections are under RW data. They are moved to the Read-only/Executable region.

Signed-off-by Himanshu Chauhan <[email protected]>

---
 firmware/fw_base.ldS | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

firmware/fw_base.ldS +22 -20

index 5411bbd..9a1304e 100644
@@ -30,23 +30,39 @@
 
        /* Beginning of the read-only data sections */
 
+       PROVIDE(_rodata_start = .);
+
        .rodata :
        {
-               PROVIDE(_rodata_start = .);
                *(.rodata .rodata.*)
                . = ALIGN(8);
-               PROVIDE(_rodata_end = .);
        }
 
-       /* End of the read-only data sections */
-
        . = ALIGN(0x1000); /* Ensure next section is page aligned */
 
+       .dynsym : {
+               PROVIDE(__dyn_sym_start = .);
+               *(.dynsym)
+               PROVIDE(__dyn_sym_end = .);
+       }
+
+       .rela.dyn : {
+               PROVIDE(__rel_dyn_start = .);
+               *(.rela*)
+               . = ALIGN(8);
+               PROVIDE(__rel_dyn_end = .);
+       }
+
+       PROVIDE(_rodata_end = .);
+
+       /* End of the read-only data sections */
+
        /*
-        * PMP regions must be to be power-of-2. RO/RW will have separate
+        * PMP regions must be to be power-of-2. RX/RW will have separate
         * regions, so ensure that the split is power-of-2.
         */
-       . = ALIGN(1 << LOG2CEIL(SIZEOF(.rodata) + SIZEOF(.text)));
+       . = ALIGN(1 << LOG2CEIL((SIZEOF(.rodata) + SIZEOF(.text)
+                               + SIZEOF(.dynsym) + SIZEOF(.rela.dyn))));
 
        PROVIDE(_fw_rw_offset = (.  - _fw_start));
 

@@ -67,19 +83,6 @@
                PROVIDE(_data_end = .);
        }
 
-       .dynsym : {
-               PROVIDE(__dyn_sym_start = .);
-               *(.dynsym)
-               PROVIDE(__dyn_sym_end = .);
-       }
-
-       .rela.dyn : {
-               PROVIDE(__rel_dyn_start = .);
-               *(.rela*)
-               . = ALIGN(8);
-               PROVIDE(__rel_dyn_end = .);
-       }
-
        . = ALIGN(0x1000); /* Ensure next section is page aligned */
 
        .bss :
--

2.39.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Add the RW section offset, provided by _fw_rw_offset symbol, to the scratch structure. This will be used to program separate pmp entry for RW section.

Signed-off-by Himanshu Chauhan <[email protected]>

---
 firmware/fw_base.S        |  8 ++++++++
 include/sbi/sbi_scratch.h | 24 ++++++++++++++----------
 2 files changed, 22 insertions(+), 10 deletions(-)

firmware/fw_base.S +8 -0

index 3f622b3..e04b683 100644
@@ -298,6 +298,12 @@ _scratch_init:
        sub     a5, t3, a4
        REG_S   a4, SBI_SCRATCH_FW_START_OFFSET(tp)
        REG_S   a5, SBI_SCRATCH_FW_SIZE_OFFSET(tp)
+
+       /* Store R/W section's offset in scratch space */
+       lla     a4, __fw_rw_offset
+       REG_L   a5, 0(a4)
+       REG_S   a5, SBI_SCRATCH_FW_RW_OFFSET(tp)
+
        /* Store next arg1 in scratch space */
        MOV_3R  s0, a0, s1, a1, s2, a2
        call    fw_next_arg1

@@ -519,6 +525,8 @@ _link_start:
        RISCV_PTR       FW_TEXT_START
 _link_end:
        RISCV_PTR       _fw_reloc_end
+__fw_rw_offset:
+       RISCV_PTR       _fw_rw_offset
 
        .section .entry, "ax", %progbits
        .align 3

include/sbi/sbi_scratch.h +14 -11

index 40a3bc9..2966188 100644
@@ -18,26 +18,28 @@
 #define SBI_SCRATCH_FW_START_OFFSET            (0 * __SIZEOF_POINTER__)
 /** Offset of fw_size member in sbi_scratch */
 #define SBI_SCRATCH_FW_SIZE_OFFSET             (1 * __SIZEOF_POINTER__)
+/** Offset (in sbi_scratch) of the R/W Offset */
+#define SBI_SCRATCH_FW_RW_OFFSET               (2 * __SIZEOF_POINTER__)
 /** Offset of next_arg1 member in sbi_scratch */
-#define SBI_SCRATCH_NEXT_ARG1_OFFSET           (2 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_NEXT_ARG1_OFFSET           (3 * __SIZEOF_POINTER__)
 /** Offset of next_addr member in sbi_scratch */
-#define SBI_SCRATCH_NEXT_ADDR_OFFSET           (3 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_NEXT_ADDR_OFFSET           (4 * __SIZEOF_POINTER__)
 /** Offset of next_mode member in sbi_scratch */
-#define SBI_SCRATCH_NEXT_MODE_OFFSET           (4 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_NEXT_MODE_OFFSET           (5 * __SIZEOF_POINTER__)
 /** Offset of warmboot_addr member in sbi_scratch */
-#define SBI_SCRATCH_WARMBOOT_ADDR_OFFSET       (5 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_WARMBOOT_ADDR_OFFSET       (6 * __SIZEOF_POINTER__)
 /** Offset of platform_addr member in sbi_scratch */
-#define SBI_SCRATCH_PLATFORM_ADDR_OFFSET       (6 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_PLATFORM_ADDR_OFFSET       (7 * __SIZEOF_POINTER__)
 /** Offset of hartid_to_scratch member in sbi_scratch */
-#define SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET   (7 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET   (8 * __SIZEOF_POINTER__)
 /** Offset of trap_exit member in sbi_scratch */
-#define SBI_SCRATCH_TRAP_EXIT_OFFSET           (8 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_TRAP_EXIT_OFFSET           (9 * __SIZEOF_POINTER__)
 /** Offset of tmp0 member in sbi_scratch */
-#define SBI_SCRATCH_TMP0_OFFSET                        (9 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_TMP0_OFFSET                        (10 * __SIZEOF_POINTER__)
 /** Offset of options member in sbi_scratch */
-#define SBI_SCRATCH_OPTIONS_OFFSET             (10 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_OPTIONS_OFFSET             (11 * __SIZEOF_POINTER__)
 /** Offset of extra space in sbi_scratch */
-#define SBI_SCRATCH_EXTRA_SPACE_OFFSET         (11 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_EXTRA_SPACE_OFFSET         (12 * __SIZEOF_POINTER__)
 /** Maximum size of sbi_scratch (4KB) */
 #define SBI_SCRATCH_SIZE                       (0x1000)
 

@@ -53,6 +55,8 @@ struct sbi_scratch {
        unsigned long fw_start;
        /** Size (in bytes) of firmware linked to OpenSBI library */
        unsigned long fw_size;
+       /** Offset (in bytes) of the R/W section */
+       unsigned long fw_rw_offset;
        /** Arg1 (or 'a1' register) of next booting stage for this HART */
        unsigned long next_arg1;
        /** Address of next booting stage for this HART */
--

2.39.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Print the RW section offset when firmware base and size is being printed.

Signed-off-by Himanshu Chauhan <[email protected]>

---
 lib/sbi/sbi_init.c | 1 +
 1 file changed, 1 insertion(+)

lib/sbi/sbi_init.c +1 -1

index 259a191..e353c33 100644
@@ -108,6 +108,7 @@ static void sbi_boot_print_general(struct sbi_scratch *scratch)
        sbi_printf("Firmware Base             : 0x%lx\n", scratch->fw_start);
        sbi_printf("Firmware Size             : %d KB\n",
                   (u32)(scratch->fw_size / 1024));
+       sbi_printf("Firmware RW Offset        : 0x%lx\n", scratch->fw_rw_offset);
 
        /* SBI details */
        sbi_printf("Runtime SBI Version       : %d.%d\n",
--

2.39.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

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 <[email protected]>

---
 lib/sbi/sbi_domain.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

lib/sbi/sbi_domain.c +22 -3

index 60fda01..9ddc1a7 100644
@@ -638,12 +638,32 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
        u32 i;
        const struct sbi_platform *plat = sbi_platform_ptr(scratch);
 
+       if (scratch->fw_rw_offset != 0
+           && (scratch->fw_rw_offset & (scratch->fw_rw_offset - 1)) != 0) {
+               sbi_printf("%s: fw_rw_offset not a power of 2 (0x%lx)\n",
+                          __func__, scratch->fw_rw_offset);
+               return SBI_EINVAL;
+       }
+
+       if ((scratch->fw_start & (scratch->fw_rw_offset - 1)) != 0) {
+               sbi_printf("%s: fw_start and fw_rw_offset not aligned\n",
+                          __func__);
+               return SBI_EINVAL;
+       }
+
        /* Root domain firmware memory region */
-       sbi_domain_memregion_init(scratch->fw_start, scratch->fw_size,
-                                 SBI_DOMAIN_MEMREGION_M_RWX,
+       sbi_domain_memregion_init(scratch->fw_start, scratch->fw_rw_offset,
+                                 (SBI_DOMAIN_MEMREGION_M_READABLE |
+                                  SBI_DOMAIN_MEMREGION_M_EXECUTABLE),
                                  &root_fw_region);
        domain_memregion_initfw(&root_memregs[root_memregs_count++]);
 
+       sbi_domain_memregion_init((scratch->fw_start + scratch->fw_rw_offset),
+                                 (scratch->fw_size - scratch->fw_rw_offset),
+                                 (SBI_DOMAIN_MEMREGION_M_READABLE |
+                                  SBI_DOMAIN_MEMREGION_M_WRITABLE),
+                                 &root_memregs[root_memregs_count++]);
+
        /* Root domain allow everything memory region */
        sbi_domain_memregion_init(0, ~0UL,
                                  (SBI_DOMAIN_MEMREGION_READABLE |
--

2.39.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

? 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 <[email protected]>

---
?lib/sbi/sbi_domain.c | 24 ++++++++++++++++++++++-- ?1 file changed, 22 insertions(+), 2 deletions(-)

lib/sbi/sbi_domain.c +0 -0

index 60fda01..9ddc1a7 100644
@@ -638,12 +638,32 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
 ????????u32 i;
 ????????const struct sbi_platform *plat = sbi_platform_ptr(scratch);

?
+???????if (scratch->fw_rw_offset != 0
+?????????? && (scratch->fw_rw_offset & (scratch->fw_rw_offset - 1)) != 0) {

+???????????????sbi_printf("%s: fw_rw_offset not a power of 2 (0x%lx)\n", +????????????????????????? __func__, scratch->fw_rw_offset); +???????????????return SBI_EINVAL;
+???????}

lib/sbi/sbi_domain.c +7 -0

+
+???????if ((scratch->fw_start & (scratch->fw_rw_offset - 1)) != 0) {
+???????????????sbi_printf("%s: fw_start and fw_rw_offset not aligned\n",
+????????????????????????? __func__);
+???????????????return SBI_EINVAL;
+???????}
+

????????/* Root domain firmware memory region */ -???????sbi_domain_memregion_init(scratch->fw_start, scratch->fw_size, -???????????????????????????????? SBI_DOMAIN_MEMREGION_M_RWX, +???????sbi_domain_memregion_init(scratch->fw_start, scratch->fw_rw_offset, +???????????????????????????????? (SBI_DOMAIN_MEMREGION_M_READABLE | +????????????????????????????????? SBI_DOMAIN_MEMREGION_M_EXECUTABLE), ????????????????????????????????? &root_fw_region); ????????domain_memregion_initfw(&root_memregs[root_memregs_count++]); ?
+???????sbi_domain_memregion_init((scratch->fw_start + scratch->fw_rw_offset), +???????????????????????????????? (scratch->fw_size - scratch->fw_rw_offset), +???????????????????????????????? (SBI_DOMAIN_MEMREGION_M_READABLE | +????????????????????????????????? SBI_DOMAIN_MEMREGION_M_WRITABLE), +???????????????????????????????? &root_memregs[root_memregs_count++]); +
????????/* Root domain allow everything memory region */ ????????sbi_domain_memregion_init(0, ~0UL, ????????????????????????????????? (SBI_DOMAIN_MEMREGION_READABLE |

Signature · 1 line

2.39.1

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 <[email protected]>

---
 lib/sbi/sbi_domain.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

Hi Himanshu,

Quoted 8 lines · Himanshu Chauhan

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.

Quoted 9 lines

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 <[email protected]>

---
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

Runtime SBI Version : 1.0

Domain0 Name : root
Domain0 Boot HART : 0
Domain0 HARTs : 0*
Domain0 Region00 : 0x0000000000030000-0x000000000003ffff M:
(R,W,X) S/U: ()
Domain0 Region01 : 0x0000000000040000-0x000000000004ffff M:
(R,W,X) S/U: ()
Domain0 Region02 : 0x00000000110c0000-0x00000000110fffff M:
(I,R,W,X) S/U: (R)
Domain0 Region03 : 0x0000000011180000-0x00000000111bffff M:
(I,R,W,X) S/U: (R)
Domain0 Region04 : 0x0000000044000000-0x000000004403ffff M:
(R,W,X) S/U: ()
Domain0 Region05 : 0x0000000011100000-0x000000001117ffff M:
(I,R,W,X) S/U: (R)
Domain0 Region06 : 0x0000000013000000-0x00000000133fffff M: (I) S/U: ()
Domain0 Region07 : 0x0000000000000000-0xffffffffffffffff M:
(R,W,X) S/U: (R,W,X)
Domain0 Next Address : 0x0000000050000000
Domain0 Next Arg1 : 0x00000000500beca0
Domain0 Next Mode : S-mode
Domain0 SysReset : yes

Boot HART ID : 0
Boot HART Domain : root
Boot HART Priv Version : v1.11
Boot HART Base ISA : rv64imafdcnx
Boot HART ISA Extensions : none
Boot HART PMP Count : 16
Boot HART PMP Granularity : 8
Boot HART PMP Address Bits: 36
Boot HART MHPM Count : 4
Boot HART MIDELEG : 0x0000000000000222
Boot HART MEDELEG : 0x000000000000b109

U-Boot 2023.01-00206-g1026336bc6 (Jan 25 2023 - 19:36:23 +0000)

CPU:   rv64imafdc
Model: Renesas SMARC EVK based on r9a07g043f01
DRAM:  896 MiB
SW_ET0_EN: OFFCore:  30 devices, 18 uclasses, devicetree: separate
MMC:   sd at 11c00000: 0, sd at 11c10000: 1
Loading Environment from MMC... OK
In:    serial at 1004b800
Out:   serial at 1004b800
Err:   serial at 1004b800
Net:   eth0: ethernet at 11c30000
Hit any key to stop autoboot:  0

Cheers,
Prabhakar

Signature · 3 lines

opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Quoted 43 lines · Prabhakar

Hi Himanshu,

Quoted 8 lines · Himanshu Chauhan

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.

Quoted 9 lines

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 <[email protected]>

---
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
____ _____ ____ _____

/ __ \                  / ____|  _ \_   _|
| |  | |_ __   ___ _ __ | (___ | |_) || |
| |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
| |__| | |_) |  __/ | | |____) | |_) || |_
\____/| .__/ \___|_| |_|_____/|____/_____|
      | |
      |_|

Quoted 13 lines

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?

Quoted 59 lines · Jessica Clarke

Quoted 43 lines · Prabhakar

Hi Himanshu,

Quoted 8 lines · Himanshu Chauhan

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.

Quoted 9 lines

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 <[email protected]>

---
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
____ _____ ____ _____

/ __ \                  / ____|  _ \_   _|
| |  | |_ __   ___ _ __ | (___ | |_) || |
| |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
| |__| | |_) |  __/ | | |____) | |_) || |_
\____/| .__/ \___|_| |_|_____/|____/_____|
      | |
      |_|

Quoted 13 lines

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

Quoted 4 lines

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

firmware/fw_base.ldS +0 -0

index 9a1304e..918357d 100644

Hi,

Thank you for the feedback.

Quoted 88 lines · Jessica Clarke

Quoted 59 lines · Jessica Clarke

Quoted 43 lines · Prabhakar

Hi Himanshu,

Quoted 8 lines · Himanshu Chauhan

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.

Quoted 9 lines

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 <[email protected]>

---
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

____                    _____ ____ _____
/ __ \                  / ____|  _ \_   _|
| |  | |_ __   ___ _ __ | (___ | |_) || |
| |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
| |__| | |_) |  __/ | | |____) | |_) || |_
\____/| .__/ \___|_| |_|_____/|____/_____|
     | |
     |_|

Quoted 13 lines

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

Quoted 4 lines

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

firmware/fw_base.ldS +0 -0

index 9a1304e..918357d 100644

firmware/fw_base.ldS line 70

Quoted 8 lines

firmware/fw_base.ldS +1 -1

@@ -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 */
 

I did try the above diff and was able to see the same issue.

After this change:
readelf -Ws build/platform/generic/firmware/fw_dynamic.elf | grep -w _fw_rw_offset
805: 0000000000020000 0 NOTYPE GLOBAL DEFAULT ABS _fw_rw_offset

Before this change:

$ readelf -Ws build/platform/generic/firmware/fw_dynamic.elf | grep -w
_fw_rw_offset
  805: 0000000000020000     0 NOTYPE  GLOBAL DEFAULT    8 _fw_rw_offset

Quoted 109 lines · Prabhakar

Hi,

Thank you for the feedback.

Quoted 88 lines · Jessica Clarke

Quoted 59 lines · Jessica Clarke

Quoted 43 lines · Prabhakar

Hi Himanshu,

Quoted 8 lines · Himanshu Chauhan

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.

Quoted 9 lines

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 <[email protected]>

---
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

____                    _____ ____ _____
/ __ \                  / ____|  _ \_   _|
| |  | |_ __   ___ _ __ | (___ | |_) || |
| |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
| |__| | |_) |  __/ | | |____) | |_) || |_
\____/| .__/ \___|_| |_|_____/|____/_____|
    | |
    |_|

Quoted 13 lines

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

Quoted 4 lines

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

firmware/fw_base.ldS +0 -0

index 9a1304e..918357d 100644

firmware/fw_base.ldS line 70

Quoted 8 lines

firmware/fw_base.ldS +1 -1

@@ -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 */
 

I did try the above diff and was able to see the same issue.

After this change:
readelf -Ws build/platform/generic/firmware/fw_dynamic.elf | grep -w _fw_rw_offset
805: 0000000000020000 0 NOTYPE GLOBAL DEFAULT ABS _fw_rw_offset

Quoted 4 lines

Before this change:

$ readelf -Ws build/platform/generic/firmware/fw_dynamic.elf | grep -w
_fw_rw_offset
 805: 0000000000020000     0 NOTYPE  GLOBAL DEFAULT    8 _fw_rw_offset

Are you sure you tested the new file? That looks like the exact change I was intending and should have an effect at run time on platforms that load OpenSBI to an address other than 0x80000000 (well, whatever the config set FW_TEXT_START to, which is that for generic by default).

Quoted 116 lines · Jessica Clarke

Quoted 109 lines · Prabhakar

Hi,

Thank you for the feedback.

Quoted 88 lines · Jessica Clarke

Quoted 59 lines · Jessica Clarke

Quoted 43 lines · Prabhakar

Hi Himanshu,

Quoted 8 lines · Himanshu Chauhan

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.

Quoted 9 lines

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 <[email protected]>

---
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

____                    _____ ____ _____
/ __ \                  / ____|  _ \_   _|
| |  | |_ __   ___ _ __ | (___ | |_) || |
| |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
| |__| | |_) |  __/ | | |____) | |_) || |_
\____/| .__/ \___|_| |_|_____/|____/_____|
    | |
    |_|

Quoted 13 lines

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

Quoted 4 lines

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

firmware/fw_base.ldS +0 -0

index 9a1304e..918357d 100644

firmware/fw_base.ldS line 70

Quoted 8 lines

firmware/fw_base.ldS +1 -1

@@ -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 */
 

I did try the above diff and was able to see the same issue.

After this change:
readelf -Ws build/platform/generic/firmware/fw_dynamic.elf | grep -w _fw_rw_offset
805: 0000000000020000 0 NOTYPE GLOBAL DEFAULT ABS _fw_rw_offset

Quoted 4 lines

Before this change:

$ readelf -Ws build/platform/generic/firmware/fw_dynamic.elf | grep -w
_fw_rw_offset
 805: 0000000000020000     0 NOTYPE  GLOBAL DEFAULT    8 _fw_rw_offset

Quoted 4 lines

Are you sure you tested the new file? That looks like the exact change I was intending and should have an effect at run time on platforms that load OpenSBI to an address other than 0x80000000 (well, whatever the config set FW_TEXT_START to, which is that for generic by default).

I guess you could be getting further and hanging for some other reason. Despite the sbi_printf calls, sbi_domain_init will never actually print anything, because it?s run before sbi_console_init and thus the prints are just slow no-ops.

What do you see with the problematic patch reverted but my fix applied?

Quoted 127 lines · Jessica Clarke

Quoted 116 lines · Jessica Clarke

Quoted 109 lines · Prabhakar

Hi,

Thank you for the feedback.

Quoted 88 lines · Jessica Clarke

Quoted 59 lines · Jessica Clarke

Quoted 43 lines · Prabhakar

Hi Himanshu,

Quoted 8 lines · Himanshu Chauhan

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.

Quoted 9 lines

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 <[email protected]>

---
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

____                    _____ ____ _____
/ __ \                  / ____|  _ \_   _|
| |  | |_ __   ___ _ __ | (___ | |_) || |
| |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
| |__| | |_) |  __/ | | |____) | |_) || |_
\____/| .__/ \___|_| |_|_____/|____/_____|
   | |
   |_|

Quoted 13 lines

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

Quoted 4 lines

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

firmware/fw_base.ldS +0 -0

index 9a1304e..918357d 100644

firmware/fw_base.ldS line 70

Quoted 8 lines

firmware/fw_base.ldS +1 -1

@@ -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 */
 

I did try the above diff and was able to see the same issue.

After this change:
readelf -Ws build/platform/generic/firmware/fw_dynamic.elf | grep -w _fw_rw_offset
805: 0000000000020000 0 NOTYPE GLOBAL DEFAULT ABS _fw_rw_offset

Quoted 4 lines

Before this change:

$ readelf -Ws build/platform/generic/firmware/fw_dynamic.elf | grep -w
_fw_rw_offset
805: 0000000000020000     0 NOTYPE  GLOBAL DEFAULT    8 _fw_rw_offset

Quoted 4 lines

Are you sure you tested the new file? That looks like the exact change I was intending and should have an effect at run time on platforms that load OpenSBI to an address other than 0x80000000 (well, whatever the config set FW_TEXT_START to, which is that for generic by default).

I guess you could be getting further and hanging for some other reason. Despite the sbi_printf calls, sbi_domain_init will never actually print anything, because it?s run before sbi_console_init and thus the prints are just slow no-ops.

Quoted 1 line

What do you see with the problematic patch reverted but my fix applied?

The same patch breaks booting for me as well on Allwinner D1. I get the same value reported with or without your linker script change:

Firmware RW Offset : 0xffffffffc0020000

(OpenSBI is loaded at 0x40000000)

Quoted 2 lines · Samuel Holland

Quoted 1 line · Jessica Clarke

On 25 Jan 2023, at 20:56, Jessica Clarke <[email protected]> wrote:

Quoted 5 lines

Quoted 5 lines

I guess you could be getting further and hanging for some other reason. Despite the sbi_printf calls, sbi_domain_init will never actually print anything, because it?s run before sbi_console_init and thus the prints are just slow no-ops.

Quoted 8 lines

Quoted 1 line

What do you see with the problematic patch reverted but my fix applied?

The same patch breaks booting for me as well on Allwinner D1. I get the same value reported with or without your linker script change:

Firmware RW Offset : 0xffffffffc0020000

(OpenSBI is loaded at 0x40000000)

Ok, so it seems BFD just does totally nonsensical things for SHN_ABS symbols when producing position-independent outputs (both -pie and -shared) for various historical reasons, and so SHN_ABS symbols are still subject to relocation as far as BFD is concerned (except AArch64, which fixes it in limited cases that don?t apply here...). I think the only way to make this work with BFD is:

firmware/fw_base.S +1 -1

index e04b683..fb6ac92 100644
@@ -526,7 +526,7 @@ _link_start:
 _link_end:
        RISCV_PTR       _fw_reloc_end
 __fw_rw_offset:
-       RISCV_PTR       _fw_rw_offset
+       RISCV_PTR       _fw_rw_start - _fw_start
 
        .section .entry, "ax", %progbits
        .align 3

firmware/fw_base.ldS +0 -0

index 9a1304e..3d68484 100644

This is second installment of the patch series sent earlier with the subject: Split region permissions into M-mode and SU-mode. The version 2 was recently posted, for which below is the link:
http://lists.infradead.org/pipermail/opensbi/2023-January/004127.html

This patch series splits the RX (text and read-only data) and RW data regions so that each can have its independent pmp entries with correct permissions. The linker script provides a new symbol _fw_rw_offset that marks the beginning of the RW section. It also ensures that the split is a power-of-2 as required by pmp.

This lays ground work for enforcing the permissions from M-mode.

Changes in v2:
* Added a check if fw_rw_offset is a power of w
* Added a check if fw_start and fw_rw_offset are aligned

Changes in v3:
* Use of local variable to latch the value in fw_rw_offset linker symbol
* Check that fw_rw_offset is non-zero before power-of-2 check

Himanshu Chauhan (5):
firmware: Split RO/RX and RW sections
firmware: Move dynsym and reladyn sections to RX section
firmware: Add RW section offset in scratch
lib: sbi: Print the RW section offset
Add two entries:

 firmware/fw_base.S        |  8 ++++++++
 firmware/fw_base.ldS      | 43 ++++++++++++++++++++++++---------------
 include/sbi/sbi_scratch.h | 24 +++++++++++++---------
 lib/sbi/sbi_domain.c      | 24 ++++++++++++++++++++--
 lib/sbi/sbi_init.c        |  1 +
 5 files changed, 72 insertions(+), 28 deletions(-)

--
2.39.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Quoted 29 lines · Himanshu Chauhan

This is second installment of the patch series sent earlier with the subject: Split region permissions into M-mode and SU-mode. The version 2 was recently posted, for which below is the link:
http://lists.infradead.org/pipermail/opensbi/2023-January/004127.html

This patch series splits the RX (text and read-only data) and RW data regions so that each can have its independent pmp entries with correct permissions. The linker script provides a new symbol _fw_rw_offset that marks the beginning of the RW section. It also ensures that the split is a power-of-2 as required by pmp.

This lays ground work for enforcing the permissions from M-mode.

Changes in v2:
* Added a check if fw_rw_offset is a power of w
* Added a check if fw_start and fw_rw_offset are aligned

Changes in v3:
* Use of local variable to latch the value in fw_rw_offset linker symbol
* Check that fw_rw_offset is non-zero before power-of-2 check

Himanshu Chauhan (5):
firmware: Split RO/RX and RW sections
firmware: Move dynsym and reladyn sections to RX section
firmware: Add RW section offset in scratch
lib: sbi: Print the RW section offset
Add two entries:

Quoted 6 lines

 firmware/fw_base.S        |  8 ++++++++
 firmware/fw_base.ldS      | 43 ++++++++++++++++++++++++---------------
 include/sbi/sbi_scratch.h | 24 +++++++++++++---------
 lib/sbi/sbi_domain.c      | 24 ++++++++++++++++++++--
 lib/sbi/sbi_init.c        |  1 +
 5 files changed, 72 insertions(+), 28 deletions(-)

Please include Reviewed-by tags in your PATCHs whenever you send out a new version of your series.

Regards,
Anup

Quoted 9 lines

--
2.39.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

This is second installment of the patch series sent earlier with the subject: Split region permissions into M-mode and SU-mode. The version 2 was recently posted, for which below is the link:
http://lists.infradead.org/pipermail/opensbi/2023-January/004127.html

This patch series splits the RX (text and read-only data) and RW data regions so that each can have its independent pmp entries with correct permissions. The linker script provides a new symbol _fw_rw_offset that marks the beginning of the RW section. It also ensures that the split is a power-of-2 as required by pmp.

This lays ground work for enforcing the permissions from M-mode.

Changes in v2:
* Added a check if fw_rw_offset is a power of w
* Added a check if fw_start and fw_rw_offset are aligned

Changes in v3:
* Use of local variable to latch the value in fw_rw_offset linker symbol
* Check that fw_rw_offset is non-zero before power-of-2 check

Himanshu Chauhan (5):
firmware: Split RO/RX and RW sections
firmware: Move dynsym and reladyn sections to RX section
firmware: Add RW section offset in scratch
lib: sbi: Print the RW section offset
Add two entries:

 firmware/fw_base.S        |  8 ++++++++
 firmware/fw_base.ldS      | 43 ++++++++++++++++++++++++---------------
 include/sbi/sbi_scratch.h | 24 +++++++++++++---------
 lib/sbi/sbi_domain.c      | 24 ++++++++++++++++++++--
 lib/sbi/sbi_init.c        |  1 +
 5 files changed, 72 insertions(+), 28 deletions(-)

--
2.39.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi