Re: [PATCH v3 2/4] lib: sbi: give platform choice of using single memregion to cover OpenSBI

[Bo Gan <ganboing@gmail.com>]

Hi Peter,

On 12/1/25 00:11, Yu-Chien Peter Lin wrote:
> Hi Bo,
> 
> Maybe use a FW option to enable single FW region at compile
> or runtime options.
> 
> compile time [1]:
> make PLATFORM=generic FW_OPTIONS=0x4
> 
> runtime [2]:
> Use fw dynamic and pass 0x4 via ((struct fw_dynamic_info *)$a2)->options
> 

I could be wrong, but I don't think it's the right approach. The strategy
of covering OpenSBI itself with single memregion is something internal to
OpenSBI, and shouldn't be allowed to be controlled by previous stage boot-
loader. Even if we allows it, previous stage bootloader will likely still
use options=0, and expect it to just boot, not to mention EIC7700 doesn't
have a previous stage bootloader. (It directly boots with OpenSBI) This
means we'll have to specify at compile time, not so different than my
previous version that got NAK'ed by Anup.

Bo

> [1] https://github.com/riscv-software-src/opensbi/blob/master/docs/firmware/fw.md#options-for-opensbi-firmware-behaviors
> [2] https://github.com/riscv-software-src/opensbi/blob/825d0e918a9e41cc57097a8cb913f26550699911/docs/firmware/fw_dynamic.md?plain=1#L10
> 
> diff --git a/include/sbi/sbi_scratch.h b/include/sbi/sbi_scratch.h
> index f1b4155d..4f40fedd 100644
> --- a/include/sbi/sbi_scratch.h
> +++ b/include/sbi/sbi_scratch.h
> @@ -115,6 +115,13 @@ enum sbi_scratch_options {
>          SBI_SCRATCH_NO_BOOT_PRINTS = (1 << 0),
>          /** Enable runtime debug prints */
>          SBI_SCRATCH_DEBUG_PRINTS = (1 << 1),
> +       /**
> +        * Use single PMP entry covering entire firmware region.
> +        * Saves one PMP entry on non-Smepmp platforms at the cost
> +        * of coarser PMP granularity.
> +        * Not compatible with Smepmp (M-mode RWX is not supported).
> +        */
> +       SBI_SCRATCH_SINGLE_FW_PMP = (1 << 2),
>   };
> 
>   /** Get pointer to sbi_scratch for current HART */
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index da0f0557..0222ca93 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -904,18 +904,27 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
>          root.possible_harts = root_hmask;
> 
>          /* Root domain firmware memory region */
> -       sbi_domain_memregion_init(scratch->fw_start, scratch->fw_rw_offset,
> -                                 (SBI_DOMAIN_MEMREGION_M_READABLE |
> - SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
> -                                  SBI_DOMAIN_MEMREGION_FW),
> -  &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 |
> -                                  SBI_DOMAIN_MEMREGION_FW),
> -  &root_memregs[root_memregs_count++]);
> +       if (scratch->options & SBI_SCRATCH_SINGLE_FW_PMP) {
> +               sbi_domain_memregion_init(scratch->fw_start, scratch->fw_size,
> +  (SBI_DOMAIN_MEMREGION_M_READABLE |
> + SBI_DOMAIN_MEMREGION_M_WRITABLE |
> + SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
> + SBI_DOMAIN_MEMREGION_FW),
> +  &root_memregs[root_memregs_count++]);
> +       } else {
> +               sbi_domain_memregion_init(scratch->fw_start, scratch->fw_rw_offset,
> +  (SBI_DOMAIN_MEMREGION_M_READABLE |
> + SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
> + SBI_DOMAIN_MEMREGION_FW),
> +  &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 |
> + SBI_DOMAIN_MEMREGION_FW),
> +  &root_memregs[root_memregs_count++]);
> +       }
> 
>          root.fw_region_inited = true;
> 
> Best regards,
> Peter Lin
> 
> On 11/20/25 5:34 PM, Bo Gan wrote:
>> ...

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