Re: [PATCH] riscv: fix compitible with binutils 2.38

Simon Glass
2022-08-19T15:25:12Z

Hi Leo,

On Fri, 19 Aug 2022 at 03:09, Leo Liang <ycliang@andestech.com> wrote:
>
> On Thu, Aug 11, 2022 at 10:23:02PM +0000, Leo Liang wrote:
> > On Mon, May 30, 2022 at 11:05:54AM -0400, Tom Rini wrote:
> > > On Sat, May 28, 2022 at 09:02:09AM +0000, Leo Liang wrote:
> > > > On Fri, May 27, 2022 at 09:30:49AM -0400, Tom Rini wrote:
> > > > > On Fri, May 27, 2022 at 02:36:29AM +0000, Leo Liang wrote:
> > > > >
> > > > > > Hi Tom,
> > > > > >
> > > > > > The following changes since commit 7e0edcadb09d55d5319fdc862041fd1b874476f5:
> > > > > >
> > > > > > Merge branch 'master' of https://source.denx.de/u-boot/custodians/u-boot-sunxi (2022-05-24 23:29:00 -0400)
> > > > > >
> > > > > > are available in the Git repository at:
> > > > > >
> > > > > > https://source.denx.de/u-boot/custodians/u-boot-riscv.git
> > > > > >
> > > > > > for you to fetch changes up to c544b281cd3e549a4fcbf4ba9a05a5d72c9557dd:
> > > > > >
> > > > > > riscv: qemu: Set kernel_comp_addr_r for compressed kernel (2022-05-26 18:42:34 +0800)
> > > > > >
> > > > > > CI result shows no issue: https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/12131
> > > > >
> > > > > First, I've applied this to u-boot/master now. Second, will
> > > > > https://patchwork.ozlabs.org/project/uboot/patch/PH7PR14MB5594FD11D1BE74284F554BEBCED49@PH7PR14MB5594.namprd14.prod.outlook.com/
> > > > > be coming soon? Thanks!
> > > >
> > > > Hi Tom,
> > > >
> > > > This patch you mentioned will not pass CI, and the reason for that
> > > > is the toolchain used for RISC-V in CI does not have corresponding
> > > > settings for zifencei and zicsr.
> > > > (detailed disscussion: https://patchwork.ozlabs.org/project/uboot/patch/20220128134713.2322800-1-alexandre.ghiti@canonical.com/)
> > > > (CI result: https://source.denx.de/u-boot/custodians/u-boot-riscv/-/jobs/440735)
> > > >
> > > > The patch looks valid, but will fail CI on 32-bit configs.
> > > > If we use 32-bit toolchain to test 32-bit configs, then
> > > > problems solved.
> > > >
> > > > Do you have any comments?
> > >
> > > I guess I'm OK with saying we should use a 32bit toolchain for 32bit
> > > riscv, if that's how things should be handled moving forward for
> > > everyone else.
> > >
> > > --
> > > Tom
> >
> > Hi Tom,
> >
> > Sorry for taking such a long time to reply.
> >
> > Recap:
> > All the "riscv: fix compitible with binutils 2.38" patches that
> > try to support new RISC-V ISA extension will cause U-Boot CI to fail
> > because the toolchain used in U-Boot CI do not support the new multilib settings.
> > (original discussion:
> > https://patchwork.ozlabs.org/project/uboot/patch/20220128134713.2322800-1-alexandre.ghiti@canonical.com/)
> >
> > We found that current RISC-V toolchains from kernel.org do not
> > support zifencei and zicsr extensions' multilib settings,
> > regardless of the toolchain version.
> > (Both gcc 11.1.0, 12.1.0 do not support the needed settings.
> > https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/
> > https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/12.1.0/)
> >
> > But we also found that if we use recent upstream riscv-gnu-toolchain,
> > we could build an gcc-12.1.0 toolchain that does support multilib
> > settings and could fix this issue.
> >
> > We have provided a Dockerfile as a reference build script[1] and
> > a prebuilt toolchain[2] for U-Boot CI to use.
> >
> > We have also verified the CI process could execute successfully
> > with your base image and the provided riscv64-linux toolchain[3].
> >
> > I guess the coming update of the toolchain in kernel.org should
> > contain the new multilib settings, so I was wondering if we could
> > replace the riscv64-linux toolchain from kernel.org with this prebuilt
> > toolchain we've provided on github[2] temporarily?
> >
> > After studying a bit of the buildman tool, the earlier idea that
> > "use different toolchains for different board configs" would require
> > an amount of modification, thus we think its best to replace the toolchain
> > temporarily to fix this issue, then the patch could be applied without CI failure.
> >
> > [1] https://github.com/ycliang-andes/riscv-toolchain/blob/master/Dockerfile
> > [2] https://github.com/ycliang-andes/riscv-toolchain/releases/download/v1.0/x86_64-gcc-12.1.0-nolibc-riscv64-linux.tar.xz
> > [3] https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/13129
>
> Hi Tom,
>
> A gentle ping.
> Do you have any concern on changing the toolchain source for now?

It is easy enough for people to download the toolchain and update the
.buildman file to point to it. It would also be possible to patch
'buildman fetch-arch riscv64' to add a toolchain-prefix to the
.buildman file, as a temporary measure. This would need a later
buildman patch to remove the line.

I'm not sure if this sort of thing is a good idea though, when we can
just tell people to download the toolchain and update the .buildman
file.

Regards,
Simon