Re: [PATCH v2 0/5] Initial ESWIN/EIC7700 support

[Bo Gan <ganboing@gmail.com>]

Hi Anup,

Thanks for the prompt reply. See inline.

On 11/17/25 00:04, Anup Patel wrote:
> On Mon, Nov 17, 2025 at 11:20 AM Bo Gan <ganboing@gmail.com> wrote:
>>
>> EIC7700 is the SoC used in HiFive P550 and Milk-V Megrez. This SoC is
>> currently one of the only off-the-shelf board/chips that support H
>> extension, although it's v0.6.1. It also supports pre-ratified N-trace.
>> Add support for it so people can benefit from latest OpenSBI features.
>>
>> The device-tree of HiFive P550 has been upstreamed to Linux:
>> https://lore.kernel.org/all/20250825132427.1618089-1-pinkesh.vaghela@einfochips.com/
>> However U-boot is not, and there are bugs in vendor U-boot device-tree,
>> and also inconsistencies between the two. Thus, this patch is coded with
>> the upstreamed device-tree as the reference, but tested with the patched
>> vendor U-boot device tree as `FW_FDT_PATH`. The patched vendor U-boot is
>> hosted here: https://github.com/ganboing/u-boot-eic7x/tree/eic7x-dt-fix
>> Refer to PATCH 5/5 for the instructions on building the firmware blob
>> and launch it through UART boot.
>>
>> The major complication of this chip is that it requires certain memory
>> regions to be blocked with PMP entries to prevent speculative execution
>> or HW prefetcher from touching them to avoid bus errors. Also due to the
>> fact that this SoC handles cache incoherent DMA by mapping memory twice,
>> one as cached, and the other as uncached, we also need an extra PMP to
>> protect the OpenSBI in the uncached portion in address space. Following
>> changes are made to lib/ and firmware/ to make it possible:
>>
>>   - Allow platform to override pmp_(un)configure
>>   - Add helper function for settings PMP TOR
>>   - Add helper function to check if memregions are disjoint
>>   - Introduce FIRMWARE_PACKED_RXRW for disabling power-of-2 RW split
> 
> PMP TOR entries are not going to fly. Also, adding separate config
> option FIRMWARE_PACKED_RXRW and separate defconfig for
> EIC770X  means distros can't use the same fw_dyanmic.bin on
> EIC770X  and other platforms.
> 
> If PMP TOR is being used only to save one PMP entry then there
> are better ways to achieve this.
> 

Okay. I think I've found a way to use NAPOT to achieve the same effect.

Memory Port (die 0)
   ├─ 0x00_8000_0000 - 0x10_8000_0000 die 0 memory (cached)
   └─ 0x10_8000_0000 - 0x80_0000_0000 (hole)

Memory Port (die 1)
   ├─ 0x00_8000_0000 - 0x20_0000_0000 (hole)
   ├─ 0x20_0000_0000 - 0x30_0000_0000 die 1 memory (cached)
   └─ 0x30_0000_0000 - 0x80_0000_0000 (hole)

By default, for both die 0/1:

   NAPOT[0]   ......   MRWX,SU()  ____ OpenSBI in cached mem
   NAPOT[1]   ......   MRWX,SU()  ____ OpenSBI in uncached mem
   NAPOT[2-5]                     <free>
   NAPOT[6] [0x0, 0x80_0000_0000) L___
   NAPOT[7]       ......          <reserved>

For die 0 in root domain:

   NAPOT[0]       ......          M(RWX)SU() ____ OpenSBI in cached mem
   NAPOT[1]       ......          M(RWX)SU() ____ OpenSBI in uncached mem
   NAPOT[2]         [0x200_0000, 0x201_0000) ____ M(RW)SU() CLINT
   NAPOT[3]       [0x2000_0000, 0x4000_0000) L___ for die 1 p550
   NAPOT[4]            [0x0, 0x10_0000_0000) _RWX M() SU(RWX)
   NAPOT[5] [0x10_0000_0000, 0x10_8000_0000) _RWX M() SU(RWX)
   NAPOT[6]            [0x0, 0x80_0000_0000) L___
   NAPOT[7]        [0x0, 0xffffffffffffffff) _RWX M() SU(RWX)

Similarly, for die 1, use:


  NAPOT[0]       ......          M(RWX)SU() ____ OpenSBI in cached mem
  NAPOT[1]       ......          M(RWX)SU() ____ OpenSBI in uncached mem
  NAPOT[2]         [0x400_0000, 0x401_0000) ____ M(RW)SU() CLINT
  NAPOT[3]               [0x0, 0x2000_0000) L___ for die 0 p550
  NAPOT[4]               [0x0, 0x8000_0000) _RWX M() SU(RWX)
  NAPOT[5] [0x20_0000_0000, 0x30_8000_0000) _RWX M() SU(RWX)
  NAPOT[6]            [0x0, 0x80_0000_0000) L___
  NAPOT[7]        [0x0, 0xffffffffffffffff) _RWX M() SU(RWX)

It'll work also for non-root domain harts, and actually better if it
doesn't contain a "catch all" region (order == __riscv_xlen). However,
compared to the TOR approach, it consumes 1 more PMP on die 0 for root
domain or any domain with a "catch all" region, and we've used up all
entries. There's no room for more, so we can't do something like a
trusted/untrusted-domain scenario as documented in domain_support.md,
because we can't block the access of tdomain memory region in udomain.
I'm not sure if this is an issue at this point. Perhaps it could be
when someone tries out TEE? I don't have the answer today.

> For example, you can do the following ...
> 
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index d75c12de..9932fe0c 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -76,6 +76,9 @@ struct sbi_platform_operations {
>       /* Check if specified HART is allowed to do cold boot */
>       bool (*cold_boot_allowed)(u32 hartid);
> 
> +    /* Check if platform requires single firmware region */
> +    bool (*single_fw_region)(void);
> +

Sounds good. I'll make the change in next version.

Bo

>       /* Platform nascent initialization */
>       int (*nascent_init)(void);
> 
> @@ -347,6 +350,23 @@ static inline bool sbi_platform_cold_boot_allowed(
>       return true;
>   }
> 
> +/**
> + * Check whether platform requires single firmware region
> + *
> + * Note: Single firmware region only works with legacy PMP because with
> + * Smepmp M-mode only regions can't have RWX permissions.
> + *
> + * @param plat pointer to struct sbi_platform
> + *
> + * @return true if single firmware region required and false otherwise
> + */
> +static inline bool sbi_platform_single_fw_region(const struct
> sbi_platform *plat)
> +{
> +    if (plat && sbi_platform_ops(plat)->single_fw_region)
> +        return sbi_platform_ops(plat)->single_fw_region();
> +    return false;
> +}
> +
>   /**
>    * Nascent (very early) initialization for current HART
>    *
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index da0f0557..bf74ba67 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 (sbi_platform_single_fw_region(sbi_platform_ptr(scratch))) {
> +        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;
> 
> Regards,
> Anup


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