[Buildroot] [PATCH v2 0/3] Update RISC-V Instruction Sets

[Buildroot] [PATCH v2 0/3] Update RISC-V Instruction Sets

[Jamie Gibbons via buildroot]

Hi all,

As per an earlier discussion with Thomas Petazzoni, we have decided that
it best to update the RISC-V instruction sets. This patch series updates
the kconfig to allow C and V to be selected as part of the general
purpose instruction set, thus allowing IMAFD + C and IMAFD + V to be a
part of a general purpose selection and keeping configurations as
custom.
With this I have updated the microchip_mpfs_icicle_defconfig to make use
of these changes.
Finally, I have included a fix to the post-image script to allow an
sdcard.img to be built for the Microchip MPFS Icicle kit. When merging,
some changes were made that broke this. The hss-payload-generator tool
looks for the u-boot.bin image in the directory this command is run
from. This binary file is located in the output images directory. All
changes to this script were made to try to ensure optimise paths and
methods while achieving a working solution.
This patch set is indended to be applied to the next branch as that is
where the Microchip Icicle kit patch series has previously been applied.

Looking forward to your feedback.

Regards,
Jamie.

v1 -> v2 changes:
- copied RVC and RVV kconfigs to both riscv_g and riscv_custom

Jamie Gibbons (3):
  arch/Config.in.riscv: update instruction set ext
  configs/microchip_mpfs_icicle_defconfig: update instruction sets
  board/microchip/mpfs_icicle: update post-image script

 arch/Config.in.riscv                      | 15 +++++++++++++++
 board/microchip/mpfs_icicle/post-image.sh |  6 ++++--
 configs/microchip_mpfs_icicle_defconfig   |  5 +----
 3 files changed, 20 insertions(+), 6 deletions(-)

-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 1/3] arch/Config.in.riscv: update instruction set ext

[Jamie Gibbons via buildroot]

Allow a RISC-V G core to support C and V. Copy custom RVC and RVV
instructions from RISC-V custom core to RISC-V general core.

v1 -> v2 changes:
- copied RVC and RVV kconfigs to both riscv_g and riscv_custom

Signed-off-by: Jamie Gibbons <jamie.gibbons@microchip.com>
---
 arch/Config.in.riscv | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/Config.in.riscv b/arch/Config.in.riscv
index 3dfbb4165f..48c3236fb6 100644
--- a/arch/Config.in.riscv
+++ b/arch/Config.in.riscv
@@ -35,6 +35,21 @@ config BR2_riscv_g
 	help
 	  General purpose (G) is equivalent to IMAFD.
 
+if BR2_riscv_g
+
+comment "Instruction Set Extensions"
+
+config BR2_RISCV_ISA_CUSTOM_RVC
+	bool "Compressed Instructions (C)"
+	select BR2_RISCV_ISA_RVC
+
+config BR2_RISCV_ISA_CUSTOM_RVV
+	bool "Vector Instructions (V)"
+	select BR2_RISCV_ISA_RVV
+	select BR2_ARCH_NEEDS_GCC_AT_LEAST_12
+
+endif
+
 config BR2_riscv_custom
 	bool "Custom architecture"
 	select BR2_RISCV_ISA_RVI
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/3] arch/Config.in.riscv: update instruction set ext

[Thomas Petazzoni via buildroot]

On Wed, 16 Aug 2023 10:24:37 +0100
Jamie Gibbons <jamie.gibbons@microchip.com> wrote:

> Allow a RISC-V G core to support C and V. Copy custom RVC and RVV
> instructions from RISC-V custom core to RISC-V general core.
> 
> v1 -> v2 changes:
> - copied RVC and RVV kconfigs to both riscv_g and riscv_custom

We don't want to *copy* them. We want to make them available in both
cases.

Also, the changelog shouldn't go inside the commit log but...

> 
> Signed-off-by: Jamie Gibbons <jamie.gibbons@microchip.com>
> ---

... here, after the "---" line.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/3] arch/Config.in.riscv: update instruction set ext

[Yann E. MORIN]

Jamie, Thomas, All,

On 2023-08-16 14:23 +0200, Thomas Petazzoni via buildroot spake thusly:
> On Wed, 16 Aug 2023 10:24:37 +0100
> Jamie Gibbons <jamie.gibbons@microchip.com> wrote:
> 
> > Allow a RISC-V G core to support C and V. Copy custom RVC and RVV
> > instructions from RISC-V custom core to RISC-V general core.
> > 
> > v1 -> v2 changes:
> > - copied RVC and RVV kconfigs to both riscv_g and riscv_custom
> 
> We don't want to *copy* them. We want to make them available in both
> cases.

Basically, I think what Thomas expects is something like:

    diff --git a/arch/Config.in.riscv b/arch/Config.in.riscv
    index 3dfbb4165f..997f7a631d 100644
    --- a/arch/Config.in.riscv
    +++ b/arch/Config.in.riscv
    @@ -1,26 +1,5 @@
     # RISC-V CPU ISA extensions.
     
    -config BR2_RISCV_ISA_RVI
    -    bool
    -
    -config BR2_RISCV_ISA_RVM
    -    bool
    -
    -config BR2_RISCV_ISA_RVA
    -    bool
    -
    -config BR2_RISCV_ISA_RVF
    -    bool
    -
    -config BR2_RISCV_ISA_RVD
    -    bool
    -
    -config BR2_RISCV_ISA_RVC
    -    bool
    -
    -config BR2_RISCV_ISA_RVV
    -    bool
    -
     choice
         prompt "Target Architecture Variant"
         default BR2_riscv_g
    @@ -41,38 +20,28 @@ config BR2_riscv_custom

     endchoice

    -if BR2_riscv_custom
    -
     comment "Instruction Set Extensions"

    -config BR2_RISCV_ISA_CUSTOM_RVM
    +config BR2_RISCV_ISA_RVM
         bool "Integer Multiplication and Division (M)"
    -    select BR2_RISCV_ISA_RVM

    -config BR2_RISCV_ISA_CUSTOM_RVA
    +config BR2_RISCV_ISA_RVA
         bool "Atomic Instructions (A)"
    -    select BR2_RISCV_ISA_RVA

    -config BR2_RISCV_ISA_CUSTOM_RVF
    +config BR2_RISCV_ISA_RVF
         bool "Single-precision Floating-point (F)"
    -    select BR2_RISCV_ISA_RVF

    -config BR2_RISCV_ISA_CUSTOM_RVD
    +config BR2_RISCV_ISA_RVD
         bool "Double-precision Floating-point (D)"
         depends on BR2_RISCV_ISA_RVF
    -    select BR2_RISCV_ISA_RVD

    -config BR2_RISCV_ISA_CUSTOM_RVC
    +config BR2_RISCV_ISA_RVC
         bool "Compressed Instructions (C)"
    -    select BR2_RISCV_ISA_RVC

    -config BR2_RISCV_ISA_CUSTOM_RVV
    +config BR2_RISCV_ISA_RVV
         bool "Vector Instructions (V)"
    -    select BR2_RISCV_ISA_RVV
         select BR2_ARCH_NEEDS_GCC_AT_LEAST_12

    -endif
    -
     choice
         prompt "Target Architecture Size"
         default BR2_RISCV_64

Thomas?

Regards,
Yann E. MORIN.

> Also, the changelog shouldn't go inside the commit log but...
> 
> > 
> > Signed-off-by: Jamie Gibbons <jamie.gibbons@microchip.com>
> > ---
> 
> ... here, after the "---" line.
> 
> Thanks!
> 
> Thomas
> -- 
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/3] arch/Config.in.riscv: update instruction set ext

[Thomas Petazzoni via buildroot]

On Wed, 16 Aug 2023 17:04:42 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Basically, I think what Thomas expects is something like:
> 
>     diff --git a/arch/Config.in.riscv b/arch/Config.in.riscv
>     index 3dfbb4165f..997f7a631d 100644
>     --- a/arch/Config.in.riscv
>     +++ b/arch/Config.in.riscv
>     @@ -1,26 +1,5 @@
>      # RISC-V CPU ISA extensions.
>      
>     -config BR2_RISCV_ISA_RVI
>     -    bool
>     -
>     -config BR2_RISCV_ISA_RVM
>     -    bool
>     -
>     -config BR2_RISCV_ISA_RVA
>     -    bool
>     -
>     -config BR2_RISCV_ISA_RVF
>     -    bool
>     -
>     -config BR2_RISCV_ISA_RVD
>     -    bool
>     -
>     -config BR2_RISCV_ISA_RVC
>     -    bool
>     -
>     -config BR2_RISCV_ISA_RVV
>     -    bool
>     -
>      choice
>          prompt "Target Architecture Variant"
>          default BR2_riscv_g
>     @@ -41,38 +20,28 @@ config BR2_riscv_custom
> 
>      endchoice
> 
>     -if BR2_riscv_custom
>     -
>      comment "Instruction Set Extensions"
> 
>     -config BR2_RISCV_ISA_CUSTOM_RVM
>     +config BR2_RISCV_ISA_RVM
>          bool "Integer Multiplication and Division (M)"
>     -    select BR2_RISCV_ISA_RVM
> 
>     -config BR2_RISCV_ISA_CUSTOM_RVA
>     +config BR2_RISCV_ISA_RVA
>          bool "Atomic Instructions (A)"
>     -    select BR2_RISCV_ISA_RVA
> 
>     -config BR2_RISCV_ISA_CUSTOM_RVF
>     +config BR2_RISCV_ISA_RVF
>          bool "Single-precision Floating-point (F)"
>     -    select BR2_RISCV_ISA_RVF
> 
>     -config BR2_RISCV_ISA_CUSTOM_RVD
>     +config BR2_RISCV_ISA_RVD
>          bool "Double-precision Floating-point (D)"
>          depends on BR2_RISCV_ISA_RVF
>     -    select BR2_RISCV_ISA_RVD
> 
>     -config BR2_RISCV_ISA_CUSTOM_RVC
>     +config BR2_RISCV_ISA_RVC
>          bool "Compressed Instructions (C)"
>     -    select BR2_RISCV_ISA_RVC
> 
>     -config BR2_RISCV_ISA_CUSTOM_RVV
>     +config BR2_RISCV_ISA_RVV
>          bool "Vector Instructions (V)"
>     -    select BR2_RISCV_ISA_RVV
>          select BR2_ARCH_NEEDS_GCC_AT_LEAST_12
> 
>     -endif
>     -
>      choice
>          prompt "Target Architecture Size"
>          default BR2_RISCV_64
> 
> Thomas?

Not quite, because we want the IMAFD options to remain under the
"custom" option.

Essentially what happens today is:

 - RISC-V G implies IMAFD, but there is no way to say I have G + C + V

 - RISC-V custom allows any combination of IMAFDCV

What we want is:

 - RISC-V G implies IMAFD, but also allows to select C and V

 - RISC-V custom allows any combination of IMAFDCV

So something like this:

diff --git a/arch/Config.in.riscv b/arch/Config.in.riscv
index 3dfbb4165f..df8499c7a0 100644
--- a/arch/Config.in.riscv
+++ b/arch/Config.in.riscv
@@ -41,10 +41,10 @@ config BR2_riscv_custom
 
 endchoice
 
-if BR2_riscv_custom
-
 comment "Instruction Set Extensions"
 
+if BR2_riscv_custom
+
 config BR2_RISCV_ISA_CUSTOM_RVM
 	bool "Integer Multiplication and Division (M)"
 	select BR2_RISCV_ISA_RVM
@@ -62,6 +62,8 @@ config BR2_RISCV_ISA_CUSTOM_RVD
 	depends on BR2_RISCV_ISA_RVF
 	select BR2_RISCV_ISA_RVD
 
+endif
+
 config BR2_RISCV_ISA_CUSTOM_RVC
 	bool "Compressed Instructions (C)"
 	select BR2_RISCV_ISA_RVC
@@ -71,8 +73,6 @@ config BR2_RISCV_ISA_CUSTOM_RVV
 	select BR2_RISCV_ISA_RVV
 	select BR2_ARCH_NEEDS_GCC_AT_LEAST_12
 
-endif
-
 choice
 	prompt "Target Architecture Size"
 	default BR2_RISCV_64

Note that indeed the blind options BR2_RISCV_ISA_RVC and
BR2_RISCV_ISA_RVV are no longer very useful, but I guess I would keep
them anyway to keep the symmetry with BR2_RISCV_ISA_RV{I,M,A,F,D} blind
options.

Thoughts?

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/3] arch/Config.in.riscv: update instruction set ext

[yann.morin]

Thomas, Jamie, All,

On 2023-08-17 09:40 +0200, Thomas Petazzoni via buildroot spake thusly:
> On Wed, 16 Aug 2023 17:04:42 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > Basically, I think what Thomas expects is something like:
[--SNIP--]
> > Thomas?
> Not quite, because we want the IMAFD options to remain under the
> "custom" option.
> Essentially what happens today is:
>  - RISC-V G implies IMAFD, but there is no way to say I have G + C + V
>  - RISC-V custom allows any combination of IMAFDCV
> What we want is:
>  - RISC-V G implies IMAFD, but also allows to select C and V
>  - RISC-V custom allows any combination of IMAFDCV
> So something like this:

With the change Yann@home suggests, it makes it obvious and visible that
generic forces IMAFD, without having to guess (or look at generic's help
to see what symbols it selects).

However, why do we want to expose those extra sets (C, V) to be optional
for generic? Isn't that really in fact just defining a custom silicon,
which is the reason we have a "custom" choice to begin with?

Or do we want to interpret "generic" as "base", e.g. "base that is able
to run a Linux system without too much hurdle, anything else less
featured will not cope very well at build or runtime, or may restrict
the set of packages you may enable; you may enable further extensions" ?

Note that G is anyway a superset of IMAFD, as it also contains the Zicsr
and Zifencei extensions. In Buildroot, as for the I set, we expect those
two extensions to always be available (see arch/arch.mk.riscv@33 and
commit d479264b34f2).

Regards,
Yann E. MORIN.

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/3] arch/Config.in.riscv: update instruction set ext

[Thomas Petazzoni via buildroot]

Hello,

On Thu, 17 Aug 2023 10:27:22 +0200
yann.morin@orange.com wrote:

> With the change Yann@home suggests, it makes it obvious and visible that
> generic forces IMAFD, without having to guess (or look at generic's help
> to see what symbols it selects).

Ah, I see what you propose. Yes, it also makes sense.

> However, why do we want to expose those extra sets (C, V) to be optional
> for generic? Isn't that really in fact just defining a custom silicon,
> which is the reason we have a "custom" choice to begin with?
> 
> Or do we want to interpret "generic" as "base", e.g. "base that is able
> to run a Linux system without too much hurdle, anything else less
> featured will not cope very well at build or runtime, or may restrict
> the set of packages you may enable; you may enable further extensions" ?

That was my idea indeed. To me "G" is a shortcut for IMAFD, but
potentially you can have extra extensions as well.

> Note that G is anyway a superset of IMAFD, as it also contains the Zicsr
> and Zifencei extensions. In Buildroot, as for the I set, we expect those
> two extensions to always be available (see arch/arch.mk.riscv@33 and
> commit d479264b34f2).

Agreed.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 2/3] configs/microchip_mpfs_icicle_defconfig: update instruction sets

[Jamie Gibbons via buildroot]

Update the instruction sets for MPFS icicle kit to mirror the
configuration update, i.e. It is now classed as a RISC-V G core with
support for C, IMAFDC.

Signed-off-by: Jamie Gibbons <jamie.gibbons@microchip.com>
---
 configs/microchip_mpfs_icicle_defconfig | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/configs/microchip_mpfs_icicle_defconfig b/configs/microchip_mpfs_icicle_defconfig
index b2568cae6a..a3b3093982 100644
--- a/configs/microchip_mpfs_icicle_defconfig
+++ b/configs/microchip_mpfs_icicle_defconfig
@@ -1,8 +1,5 @@
 BR2_riscv=y
-BR2_riscv_custom=y
-BR2_RISCV_ISA_CUSTOM_RVM=y
-BR2_RISCV_ISA_CUSTOM_RVF=y
-BR2_RISCV_ISA_CUSTOM_RVD=y
+BR2_riscv_g=y
 BR2_RISCV_ISA_CUSTOM_RVC=y
 BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_6_1=y
 BR2_TARGET_GENERIC_HOSTNAME="mpfs_icicle"
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 3/3] board/microchip/mpfs_icicle: update post-image script

[Jamie Gibbons via buildroot]

The hss-payload-generator could not find where u-boot.bin was when
looking for it using the config.yaml. This needs to be ran in the
same directory as the u-boot.bin file to enable it to find it. Fix the
post image script to allow an image to get built.

Signed-off-by: Jamie Gibbons <jamie.gibbons@microchip.com>
---
 board/microchip/mpfs_icicle/post-image.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/microchip/mpfs_icicle/post-image.sh b/board/microchip/mpfs_icicle/post-image.sh
index 04b34bbd40..2368aa42f6 100755
--- a/board/microchip/mpfs_icicle/post-image.sh
+++ b/board/microchip/mpfs_icicle/post-image.sh
@@ -2,7 +2,9 @@
 HSS_PAYLOAD_GENERATOR=${HOST_DIR}/bin/hss-payload-generator
 MKIMAGE=${HOST_DIR}/bin/mkimage
 
-"${HSS_PAYLOAD_GENERATOR}" -c board/microchip/mpfs_icicle/config.yaml "${BINARIES_DIR}"/payload.bin
-cp board/microchip/mpfs_icicle/mpfs_icicle.its "${BINARIES_DIR}"/mpfs_icicle.its
+cd "${BINARIES_DIR}" || exit
+"${HSS_PAYLOAD_GENERATOR}" -c "${CONFIG_DIR}"/board/microchip/mpfs_icicle/config.yaml payload.bin
+cd "${CONFIG_DIR}" || exit
+cp "${CONFIG_DIR}"/board/microchip/mpfs_icicle/mpfs_icicle.its "${BINARIES_DIR}"/mpfs_icicle.its
 (cd "${BINARIES_DIR}" && "${MKIMAGE}" -f mpfs_icicle.its mpfs_icicle.itb)
 support/scripts/genimage.sh -c board/microchip/mpfs_icicle/genimage.cfg
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot