| Message ID | 20200331092158.5787-1-ynezz@true.cz |
|---|---|
| State | Not Applicable |
| Delegated to: | Petr Štetiar |
| Headers | show |
| Series | [OpenWrt-Devel] mvebu, tegra: make CPU subtype default to vfp3-d16 | expand |
Tested on mvebu and fixes opkg issue the issue for me, thanks! Best, Paul
W dniu 31.03.2020 o 11:21, Petr Štetiar pisze: > Armada 370 and Tegra2 processors have only 16 double-precision > registers. The change introduced by commit 8dcc1087602e ("toolchain: > ARM: Fix toolchain compilation for gcc 8.x") switched accidentally the > toolchain for mvebu cortexa9 subtarget to cpu type with 32 > double-precision registers. > > This stems from gcc defaults which assume "vfpv3-d32" if only "vfpv3" as > mfpu is specified. That change resulted in unusable image, in which > kernel will kill userspace as soon as it causing "Illegal instruction". > > In order to fix those issues Tomas in commit 2d61f8821c7c ("mvebu: > cortexa9: correct cpu subtype") and commit 43d1d8851062 ("tegra: correct > cpu subtype") changed the CPU subtype to explicit vfpv3-d16 which fixed > the above mentioned issue, but on the other end it has resulted in the > need of building packages for this new CPU subtype which is not wanted > due to the increased infrastructure costs, like disk space and > additional build time which is huge for packages feed. > > So lets just take a step back and make the vfp3-d16 explicit again. > > Cc: Tomasz Maciej Nowak <tomek_n@o2.pl> > Cc: Christian Lamparter <chunkeey@gmail.com> > Reported-by: Paul Spooren <mail@aparcar.org> > Fixes: 43d1d8851062 ("tegra: correct cpu subtype") > Fixes: 2d61f8821c7c ("mvebu: cortexa9: correct cpu subtype") > Fixes: 8dcc1087602e ("toolchain: ARM: Fix toolchain compilation for gcc 8.x") > Signed-off-by: Petr Štetiar <ynezz@true.cz> For the tegra target Tested-by: Tomasz Maciej Nowak <tomek_n@o2.pl> > --- > include/target.mk | 3 +++ > target/linux/mvebu/cortexa9/target.mk | 2 +- > target/linux/tegra/Makefile | 2 +- > 3 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/include/target.mk b/include/target.mk > index 9bd4c14936c1..94ea1a9e0001 100644 > --- a/include/target.mk > +++ b/include/target.mk > @@ -179,6 +179,9 @@ ifeq ($(DUMP),1) > endif > ifneq ($(findstring arm,$(ARCH)),) > CPU_TYPE ?= xscale > + ifeq ($(CONFIG_SOFT_FLOAT),) > + CPU_CFLAGS_vfpv3 = -mfpu=vfpv3-d16 > + endif > endif > ifeq ($(ARCH),powerpc) > CPU_CFLAGS_603e:=-mcpu=603e > diff --git a/target/linux/mvebu/cortexa9/target.mk b/target/linux/mvebu/cortexa9/target.mk > index cdd4d86e4936..2a75599bc9a3 100644 > --- a/target/linux/mvebu/cortexa9/target.mk > +++ b/target/linux/mvebu/cortexa9/target.mk > @@ -10,5 +10,5 @@ include $(TOPDIR)/rules.mk > ARCH:=arm > BOARDNAME:=Marvell Armada 37x/38x/XP > CPU_TYPE:=cortex-a9 > -CPU_SUBTYPE:=vfpv3-d16 > +CPU_SUBTYPE:=vfpv3 > KERNELNAME:=zImage dtbs > diff --git a/target/linux/tegra/Makefile b/target/linux/tegra/Makefile > index 5dd4d439849e..0b48fc16baa2 100644 > --- a/target/linux/tegra/Makefile > +++ b/target/linux/tegra/Makefile > @@ -11,7 +11,7 @@ BOARD := tegra > BOARDNAME := NVIDIA Tegra > FEATURES := audio boot-part display ext4 fpu gpio pci pcie rootfs-part rtc squashfs usb > CPU_TYPE := cortex-a9 > -CPU_SUBTYPE := vfpv3-d16 > +CPU_SUBTYPE := vfpv3 > > KERNEL_PATCHVER := 5.4 > KERNEL_TESTING_PATCHVER := 5.4 > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel >
On Wednesday, 1 April 2020 15:17:31 CEST Tomasz Maciej Nowak wrote: > W dniu 31.03.2020 o 11:21, Petr Štetiar pisze: > > Armada 370 and Tegra2 processors have only 16 double-precision > > registers. The change introduced by commit 8dcc1087602e ("toolchain: > > ARM: Fix toolchain compilation for gcc 8.x") switched accidentally the > > toolchain for mvebu cortexa9 subtarget to cpu type with 32 > > double-precision registers. > > > > This stems from gcc defaults which assume "vfpv3-d32" if only "vfpv3" as > > mfpu is specified. That change resulted in unusable image, in which > > kernel will kill userspace as soon as it causing "Illegal instruction". > > > > In order to fix those issues Tomas in commit 2d61f8821c7c ("mvebu: > > cortexa9: correct cpu subtype") and commit 43d1d8851062 ("tegra: correct > > cpu subtype") changed the CPU subtype to explicit vfpv3-d16 which fixed > > the above mentioned issue, but on the other end it has resulted in the > > need of building packages for this new CPU subtype which is not wanted > > due to the increased infrastructure costs, like disk space and > > additional build time which is huge for packages feed. > > > > So lets just take a step back and make the vfp3-d16 explicit again. [...] > > --- > > include/target.mk | 3 +++ > > target/linux/mvebu/cortexa9/target.mk | 2 +- > > target/linux/tegra/Makefile | 2 +- > > 3 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/include/target.mk b/include/target.mk > > index 9bd4c14936c1..94ea1a9e0001 100644 > > --- a/include/target.mk > > +++ b/include/target.mk > > @@ -179,6 +179,9 @@ ifeq ($(DUMP),1) > > endif > > ifneq ($(findstring arm,$(ARCH)),) > > CPU_TYPE ?= xscale > > + ifeq ($(CONFIG_SOFT_FLOAT),) > > + CPU_CFLAGS_vfpv3 = -mfpu=vfpv3-d16 > > + endif The question I'm pondering is: Do you really want to maintain this specifc maps for this (only this arch) or not? Because I do think this will creep up sooner or later again. So we could also defuse this a bit by replacing that monicer "vfpv3" (as this is a gcc/binutils thing and not under our control) with something "clearly from OpenWrt". like CPU_CFLAGS_fpu = -mfpu=vfpv3-d16 (if that doesn't clash with something else). But then again, I'm not here to tell you what to do. If you want your patch as-is then go for it! :D Cheers, Christian
Christian Lamparter <chunkeey@gmail.com> [2020-04-01 15:43:40]: Hi Christian, > > > diff --git a/include/target.mk b/include/target.mk > > > index 9bd4c14936c1..94ea1a9e0001 100644 > > > --- a/include/target.mk > > > +++ b/include/target.mk > > > @@ -179,6 +179,9 @@ ifeq ($(DUMP),1) > > > endif > > > ifneq ($(findstring arm,$(ARCH)),) > > > CPU_TYPE ?= xscale > > > + ifeq ($(CONFIG_SOFT_FLOAT),) > > > + CPU_CFLAGS_vfpv3 = -mfpu=vfpv3-d16 > > > + endif > > The question I'm pondering is: Do you really want to maintain this specifc maps > for this (only this arch) or not? Ideally no, sure, but I wasn't aware about the other options. I don't own HW with this CPUs, so have no prior experience. > Because I do think this will creep up sooner or later again. Yeah, thats likely going to happen :-) > So we could also defuse this a bit by replacing that monicer "vfpv3" (as > this is a gcc/binutils thing and not under our control) with something > "clearly from OpenWrt". like CPU_CFLAGS_fpu = -mfpu=vfpv3-d16 (if that > doesn't clash with something else). You mean put this explicitly in the target/linux/tegra/Makefile? > But then again, I'm not here to tell you what to do. I'm all in to fix this properly. > If you want your patch as-is then go for it! :D Then I would commit it without even asking for review, right? :-) Thanks! Cheers, Petr
W dniu 01.04.2020 o 16:52, Klaus Kudielka pisze: > Hi Petr & Tomasz, > >> In order to fix those issues Tomas in commit 2d61f8821c7c ("mvebu: >> cortexa9: correct cpu subtype") and commit 43d1d8851062 ("tegra: correct >> cpu subtype") changed the CPU subtype to explicit vfpv3-d16 which fixed >> the above mentioned issue, but on the other end it has resulted in the >> need of building packages for this new CPU subtype which is not wanted >> due to the increased infrastructure costs, like disk space and >> additional build time which is huge for packages feed. > > I certainly don't understand all aspects of the build system, but I > checked over and over again. > > [0]$ find target -type f -exec grep -H CPU_SUBTYPE {} \; | grep vfpv3 > target/linux/mvebu/cortexa9/target.mk:CPU_SUBTYPE:=vfpv3-d16 > target/linux/omap/Makefile:CPU_SUBTYPE:=vfpv3 > target/linux/sunxi/cortexa8/target.mk:CPU_SUBTYPE:=vfpv3 > target/linux/tegra/Makefile:CPU_SUBTYPE := vfpv3-d16 > > omap & sunxi/cortexa8 are both cortex-a8. > > So, tegra and mvebu/cortexa9 are the *only* targets with arm_cortex-a9_vfpv3 > (-d16) packages? > > If we switch both, like Tomasz did, arm_cortex-a9_vfpv3 would not be needed > anymore, and be replaced by -d16. Why does that cost more resources? Indeed, I always forget about omap and zynq being cortex-a8. > > (Okay, a few snapshot users would have to upgrade if they want to install > packages, but this is the master branch anyway) This commit also would make it un-portable to 19.07 branch, since package directories structure wouldn't change. So if OpenWrt would announce 19.07.3 users of 19.07.2 or earlier would possibly end up with incompatible packages, when installing something from packages repository. > > Nevermind, just a stupid idea, to avoid unneeded complexity. > Please ignore it, if I'm wrong. Nothing wrong with what You wrote, thanks for pointing all of it out. > > Klaus > Sorry for noise, added OpenWrt list.
Klaus Kudielka <klaus.kudielka@gmail.com> [2020-04-01 16:52:41]: Hi, > omap & sunxi/cortexa8 are both cortex-a8. good point, I've missed that. > So, tegra and mvebu/cortexa9 are the *only* targets with arm_cortex-a9_vfpv3 > (-d16) packages? Seems so. > If we switch both, like Tomasz did, arm_cortex-a9_vfpv3 would not be needed > anymore, and be replaced by -d16. Why does that cost more resources? It doesn't, you're right. > (Okay, a few snapshot users would have to upgrade if they want to install > packages, but this is the master branch anyway) That master branch is going to be next release at some point. Just wondering now if this change could have some impact during upgrade from 19.07.y release. > Nevermind, just a stupid idea, to avoid unneeded complexity. But so far the best idea :-) So I've marked this patch as 'Rejected', solved, thanks. -- ynezz
diff --git a/include/target.mk b/include/target.mk index 9bd4c14936c1..94ea1a9e0001 100644 --- a/include/target.mk +++ b/include/target.mk @@ -179,6 +179,9 @@ ifeq ($(DUMP),1) endif ifneq ($(findstring arm,$(ARCH)),) CPU_TYPE ?= xscale + ifeq ($(CONFIG_SOFT_FLOAT),) + CPU_CFLAGS_vfpv3 = -mfpu=vfpv3-d16 + endif endif ifeq ($(ARCH),powerpc) CPU_CFLAGS_603e:=-mcpu=603e diff --git a/target/linux/mvebu/cortexa9/target.mk b/target/linux/mvebu/cortexa9/target.mk index cdd4d86e4936..2a75599bc9a3 100644 --- a/target/linux/mvebu/cortexa9/target.mk +++ b/target/linux/mvebu/cortexa9/target.mk @@ -10,5 +10,5 @@ include $(TOPDIR)/rules.mk ARCH:=arm BOARDNAME:=Marvell Armada 37x/38x/XP CPU_TYPE:=cortex-a9 -CPU_SUBTYPE:=vfpv3-d16 +CPU_SUBTYPE:=vfpv3 KERNELNAME:=zImage dtbs diff --git a/target/linux/tegra/Makefile b/target/linux/tegra/Makefile index 5dd4d439849e..0b48fc16baa2 100644 --- a/target/linux/tegra/Makefile +++ b/target/linux/tegra/Makefile @@ -11,7 +11,7 @@ BOARD := tegra BOARDNAME := NVIDIA Tegra FEATURES := audio boot-part display ext4 fpu gpio pci pcie rootfs-part rtc squashfs usb CPU_TYPE := cortex-a9 -CPU_SUBTYPE := vfpv3-d16 +CPU_SUBTYPE := vfpv3 KERNEL_PATCHVER := 5.4 KERNEL_TESTING_PATCHVER := 5.4
Armada 370 and Tegra2 processors have only 16 double-precision registers. The change introduced by commit 8dcc1087602e ("toolchain: ARM: Fix toolchain compilation for gcc 8.x") switched accidentally the toolchain for mvebu cortexa9 subtarget to cpu type with 32 double-precision registers. This stems from gcc defaults which assume "vfpv3-d32" if only "vfpv3" as mfpu is specified. That change resulted in unusable image, in which kernel will kill userspace as soon as it causing "Illegal instruction". In order to fix those issues Tomas in commit 2d61f8821c7c ("mvebu: cortexa9: correct cpu subtype") and commit 43d1d8851062 ("tegra: correct cpu subtype") changed the CPU subtype to explicit vfpv3-d16 which fixed the above mentioned issue, but on the other end it has resulted in the need of building packages for this new CPU subtype which is not wanted due to the increased infrastructure costs, like disk space and additional build time which is huge for packages feed. So lets just take a step back and make the vfp3-d16 explicit again. Cc: Tomasz Maciej Nowak <tomek_n@o2.pl> Cc: Christian Lamparter <chunkeey@gmail.com> Reported-by: Paul Spooren <mail@aparcar.org> Fixes: 43d1d8851062 ("tegra: correct cpu subtype") Fixes: 2d61f8821c7c ("mvebu: cortexa9: correct cpu subtype") Fixes: 8dcc1087602e ("toolchain: ARM: Fix toolchain compilation for gcc 8.x") Signed-off-by: Petr Štetiar <ynezz@true.cz> --- include/target.mk | 3 +++ target/linux/mvebu/cortexa9/target.mk | 2 +- target/linux/tegra/Makefile | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-)