On Fri, Aug 19, 2022 at 09:24:53AM -0600, Simon Glass wrote:
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/PH7PR14MB5594FD11D1BE74284F... 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-al...) (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-al...)
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_... [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.
Hi Simon,
Thank you for pointing this out!
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.
But agreed that this might not be a good idea.
Furthermore, I think this patch (https://patchwork.ozlabs.org/project/uboot/patch/BYAPR04MB48242371E5FBE6AC36...) should be able to support new RISC-V extension without breaking the CI process as long as we do not update the toolchain CI uses.
I will review this patch as soon as possible.
Thanks for your suggestion!
Best regards, Leo
Regards, Simon