dell / dkms

Dynamic Kernel Module Support
GNU General Public License v2.0
640 stars 150 forks source link

improper archirecture detection #196

Open benthetechguy opened 2 years ago

benthetechguy commented 2 years ago

I'm trying to build some modules on my Allwinner Nezha D1 running Arch Linux for RISC-V, and I get this error every time I try: Makefile:702: arch/riscv64/Makefile: No such file or directory

Looking at the build command, it's using ARCH=riscv64 which makes it look for arch/riscv64 in the kernel source. I'm assuming it gets this from uname which lists the architecture as riscv64. The kernel's name for RISC-V, both 32 and 64 bit, is riscv, so it should be looking in arch/riscv instead. Considering that on x86_64 DKMS knows to look in arch/x86 instead of arch/x86_64, I'm assuming we already have this logic in place, just not for RISC-V yet.

benthetechguy commented 2 years ago

I have also tried adding --arch riscv to the dkms command, but this doesn't change anything and ARCH=riscv64 is still put into the build command.

scaronni commented 2 years ago

Interesting! Can you provide a patch to fix the detection? Thanks.

benthetechguy commented 2 years ago

If I knew how to fix it, this would be a pull request, not an issue ;)

benthetechguy commented 2 years ago

Considering that on x86_64 DKMS knows to look in arch/x86 instead of arch/x86_64, I'm assuming we already have this logic in place, just not for RISC-V yet.

Got curious, checked out which ARCH variable is being set on x86_64. Turns out it's not ARCH=x86 as I thought, but ARCH=x86_64. This means that the logic mentioned above does not exist in DKMS, and that if you build with ARCH=x86_64, the kernel just knows to use the arch/x86 folder, while this association does not exist for ARCH=riscv64 and arch/riscv. This makes me think it might actually be a kernel issue rather than a DKMS one, but I'm not closing this yet for two reasons:

  1. I've never contributed to or reported bugs for the kernel before, so any assistance with that process would be nice.
  2. This issue could still technically be fixed in DKMS by adding logic to set ARCH=riscv if uname -m says riscv64.
scaronni commented 2 years ago

have you tried passing -a riscv to the DKMS command line?

Example:

dkms remove -m module/x.x.x
dkms add -a riscv -m module/x.x.x
dkms build -a riscv -m module/x.x.x
dkms install -a riscv -m module/x.x.x
benthetechguy commented 2 years ago

Yes, as mentioned above.

benthetechguy commented 2 years ago

I now have some new information. I have been talking with some kernel maintainers, and the issue does in fact lie within DKMS. Here is an excerpt from some email correspondance.

On 3/30/22 9:34 PM, Masahiro Yamada wrote:

On Thu, Mar 31, 2022 at 3:40 AM Ben Westover

On 3/30/22 11:31 AM, Masahiro Yamada wrote:

On Wed, Mar 30, 2022 at 11:34 PM Ben Westover wrote:

When riscv64 or riscv32 are used as the value for ARCH during compilation, like in tools that get the ARCH value from uname, set SRCARCH to riscv instead of failing because the riscv64 and riscv32 targets don't exist.

Can you refer to the code that really needs this?

Some software like DKMS compiles out-of-tree modules by running uname -m and using that for the ARCH value. Without this patch, that compilation fails because uname shows either riscv64 or riscv32 while riscv should be used.

It is a bug in DKMS.

The ARCH=* in linux kernel does not necessarily match to 'uname -m'.

For example, we use ARCH=arm64 for arm 64-bit (so called aarch64), but it does not match "aarch64".

The kernel has freedom to determine the supported string for ARCH=.

DKMS must adjust to the kernel code.

Ah, I see. Originally, I opened an issue in DKMS, but was led to believe it was a kernel issue. Now I see that they are the ones that need to change.

This code already exists for sparc and parisc, as well as x86 of course.

This is because there is a historical reason.

If you look at the old code (e.g. 2.6.x,) arch/i386/ and arch/x86_64 were separate directories.

They were unified into arch/x86/ now, but we still support ARCH=i386/x86_64. It helps to choose a different defconfig. See arch/x86/Makefile.

This makes more sense now. DKMS based their ARCH value off of uname, and because of this vestigial code, it worked on x86. Now, trouble is being run into on other architectures.

There are two main takeaways here:

  1. This issue isn't exclusive to RISC-V and happens on all architectures named differently from their uname values, such as ARM. I have tested and verified that this issue occurs on both ARM and PowerPC.
  2. The way DKMS has detected architectures has always been wrong. There was simply a coincidental check that redirected x86_64 and i386 to ARCH=x86 for historical reasons, and because of that the code worked. Now that other architectures are being used, this vestigial coincidence no longer exists; the code needs to be rewritten to actually follow the way the kernel does things.

To start you off, the current arch detection code is here: https://github.com/dell/dkms/blob/1194db850c4d2391daf69cfb1a30ed17517a2bf9/dkms.in#L271-L276

kalashnlkov commented 2 years ago

Considering that on x86_64 DKMS knows to look in arch/x86 instead of arch/x86_64, I'm assuming we already have this logic in place, just not for RISC-V yet.

Got curious, checked out which ARCH variable is being set on x86_64. Turns out it's not ARCH=x86 as I thought, but ARCH=x86_64. This means that the logic mentioned above does not exist in DKMS, and that if you build with ARCH=x86_64, the kernel just knows to use the arch/x86 folder, while this association does not exist for ARCH=riscv64 and arch/riscv. This makes me think it might actually be a kernel issue rather than a DKMS one, but I'm not closing this yet for two reasons:

  1. I've never contributed to or reported bugs for the kernel before, so any assistance with that process would be nice.
  2. This issue could still technically be fixed in DKMS by adding logic to set ARCH=riscv if uname -m says riscv64.

exactly. kernel do some thing about arch like following:

SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
                  -e s/sun4u/sparc64/ \
                  -e s/arm.*/arm/ -e s/sa110/arm/ \
                  -e s/s390x/s390/ \
                  -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
                  -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \
                  -e s/riscv.*/riscv/ -e s/loongarch.*/loongarch/)

ARCH        ?= $(SUBARCH)

ifeq ($(ARCH),x86_64)
        SRCARCH := x86
endif

https://github.com/torvalds/linux/blob/master/Makefile#L397