llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.34k stars 11.7k forks source link

clang, sparc: missing compiler command line flags for linux kernel build #40792

Open 9816046d-d175-4de2-95d1-ce389e5190e8 opened 5 years ago

9816046d-d175-4de2-95d1-ce389e5190e8 commented 5 years ago
Bugzilla Link 41447
Version 8.0
OS Linux
Blocks llvm/llvm-project#4440
CC @arndb,@nickdesaulniers

Extended Description

I tried to build the Linux kernel for sparc32 and sparc64. Both work with few changes, but the Makefiles pass a number of options to the compiler that clang does not understand.

In particular, linux-5.1 passes

-mno-fpu
-fcall-used-g5
-fcall-used-g7
-mcpu=ultrasparc
-mcmodel=medlow
-ffixed-g4
-ffixed-g5
-fcall-used-g7
-mv8plus

Removing all of these from the Makefile gets it to compile almost all files, but I'm fairly sure this will not boot because some of the options are in fact required.

brad0 commented 2 years ago

For OpenBSD/sparc64 we ran into -mno-fpu being missing when building the kernel.

llvmbot commented 2 years ago

@llvm/issue-subscribers-clang-driver

ihiasi commented 2 years ago

I see in the Sparc GCC options document that :

-mno-fpu
-msoft-float
Generate output containing library calls for floating point. Warning: the requisite libraries are not available for all SPARC targets. Normally the facilities of the machine’s usual C compiler are used, but this cannot be done directly in cross-compilation. You must make your own arrangements to provide suitable library functions for cross-compilation. The embedded targets ‘sparc-*-aout’ and ‘sparclite-*-*’ do provide software floating-point support.

I changed arch/sparc/Makefile to use soft-float (supported by LLVM) and seems to compile (fails on the other flags you listed). FYI.

ihiasi commented 2 years ago

@arndb, @nickdesaulniers - I'm hitting the following after I turn off the -ffixed/-fcall flags :

+ ld.lld -m elf64_sparc -z noexecstack --build-id=sha1 -X --script=./arch/sparc/kernel/vmlinux.lds --strip-debug -o .tmp_vmlinux.kallsyms1 --whole-archive arch/sparc/kernel/head_64.o init/built-in.a usr/built-in.a arch/sparc/built-in.a kernel/built-in.a certs/built-in.a mm/built-in.a fs/built-in.a ipc/built-in.a security/built-in.a crypto/built-in.a block/built-in.a io_uring/built-in.a lib/built-in.a arch/sparc/prom/built-in.a arch/sparc/lib/built-in.a lib/lib.a arch/sparc/prom/lib.a arch/sparc/lib/lib.a drivers/built-in.a sound/built-in.a net/built-in.a virt/built-in.a arch/sparc/power/built-in.a .vmlinux.export.o --no-whole-archive --start-group --end-group
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xccc): unknown relocation (41) against symbol user_rtt_fill_fixup_common
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xcd4): unknown relocation (41) against symbol user_rtt_fill_fixup_common
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xcdc): unknown relocation (41) against symbol user_rtt_fill_fixup_common
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xe04): unknown relocation (41) against symbol etrap
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xe14): unknown relocation (41) against symbol rtrap
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xf04): unknown relocation (41) against symbol etrap
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xf14): unknown relocation (41) against symbol rtrap
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xf34): unknown relocation (41) against symbol etrap
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xf5c): unknown relocation (41) against symbol rtrap
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xf70): unknown relocation (41) against symbol rtrap
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xf90): unknown relocation (41) against symbol etrap
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xfbc): unknown relocation (41) against symbol rtrap
ld.lld: error: arch/sparc/kernel/head_64.o:(.text+0xfcc): unknown relocation (41) against symbol rtrap
ld.lld: error: arch/sparc/kernel/head_64.o:(function do_fpdis: .text+0xffc): unknown relocation (41) against symbol etrap
ld.lld: error: arch/sparc/kernel/head_64.o:(function do_fpdis: .text+0x1008): unknown relocation (41) against symbol rtrap
ld.lld: error: arch/sparc/kernel/head_64.o:(function fp_other_bounce: .text+0x1228): unknown relocation (41) against symbol rtrap
ld.lld: error: arch/sparc/kernel/head_64.o:(function do_fptrap: .text+0x147c): unknown relocation (41) against symbol etrap
ld.lld: error: arch/sparc/kernel/head_64.o:(function utrap_trap: .text+0x157c): unknown relocation (41) against symbol etrap
ld.lld: error: arch/sparc/kernel/head_64.o:(function utrap_trap: .text+0x1590): unknown relocation (41) against symbol rtrap
ld.lld: error: arch/sparc/kernel/head_64.o:(function __spitfire_access_error: .text+0x1668): unknown relocation (41) against symbol etraptl1
ld.lld: error: too many errors emitted, stopping now (use -error-limit=0 to see all errors)
make: *** [../Makefile:1168: vmlinux] Error 1

Any ideas what might cause this ? I'd like to get the Linux SPARC LLVM port to completion.

arndb commented 2 years ago

I would recommend you try ld.bfd instead of ld.lld to reduce the number of variables in your build setup compared to the gcc setup.

For https://github.com/ClangBuiltLinux, we pretty much ignore sparc now, as there is very little interest in the architecture upstream compared to the other clang supported targets.

MaskRay commented 2 years ago

The lld support state is similar: the port is quite incomplete. I am also concerned of sparc doing things too differently and lld support would introduce maintenance burden.

ihiasi commented 2 years ago

actually, if I compile ONLY for 64-bit :

make -j 20 CC=clang LD=ld.bfd ARCH=sparc64 CROSS_COMPILE=sparc64-linux-gnu- O=./build_clang_ldbfd_64 sparc64_defconfig
cd build_clang_ldbfd_64
make -j 20 CC=clang LD=ld.bfd ARCH=sparc64 CROSS_COMPILE=sparc64-linux-gnu- V=1 vmlinux modules zImage

I'm getting different errors (regardless of if I use LLVM=1 or CC=clang LD=ld.bfd - since the errors are in the LLVM assembly reader/parser) :

../arch/sparc/include/asm/irqflags_64.h:33:3: error: invalid operand for instruction
                "wrpr   %0, %%pil"
                ^
<inline asm>:1:12: note: instantiated into assembly here
        wrpr    %i0, %pil
                     ^
...
...
../arch/sparc/include/asm/irqflags_64.h:53:3: error: invalid operand for instruction
                "wrpr   0, %%pil"
                ^
<inline asm>:1:7: note: instantiated into assembly here
        wrpr    0, %pil

This is happening because SparcInstrInfo.td declares wrpr as :

// Section A.62 - Write Privileged Register Instructions
let Predicates = [HasV9] in {
  def WRPRrr : F3_1<2, 0b110010,
                   (outs PRRegs:$rd), (ins IntRegs:$rs1, IntRegs:$rs2),
                   "wrpr $rs1, $rs2, $rd", []>;
  def WRPRri : F3_2<2, 0b110010,
                   (outs PRRegs:$rd), (ins IntRegs:$rs1, simm13Op:$simm13),
                   "wrpr $rs1, $simm13, $rd", []>;
}

ie, no wrpr <register>, <register> or wrpr <immediate>, <register>. Both the original SparcV9 spec and the Oracle updated SparcV9 spec do not mention wrpr imm, %reg - but ld.bfd seem to accept it and I see the same in the mtrace Sparc code, for example.

I'll fix that and update here.

ihiasi commented 2 years ago

The lld support state is similar: the port is quite incomplete. I am also concerned of sparc doing things too differently and lld support would introduce maintenance burden.

The hope is that we won't introduce maintenance burden since Sparc is in many ways very similar to Mips for example and Mips support is already in and pretty strong.

brad0 commented 1 year ago

@koachan

koachan commented 1 year ago

From a quick look about the various issues in the report:

-mfpu/-mno-fpu

This is already implemented in clang as -mhard-float/-msoft-float, but I guess an alias could be made for those.

-mcmodel=medlow

I don't think clang has any option to set the 64-bit SPARC code model now, but from the assembly output I'm guessing that currently it compiles in a mode corresponding to GCC's medmid. If Linux has a hard requirement on medlow code model then yeah, this is a problem. (Though from my understanding medmid is a strict superset of medlow so this should not be a problem in practice)

-mv8plus

32-bit V8+ ABI extensions is also unsupported by clang right now :(

-fcall-used-g5
-fcall-used-g7
-ffixed-g4
-ffixed-g5
-fcall-used-g7

I have no idea what do those options do and I can't find it in GCC's documentation, so no comment on those. As an aside, I find it very weird that Linux passes both -mcmodel (which is a 64-bit only option) and -mv8plus (which is a 32-bit only option). Here on my local GCC 12.1, passing the option for the wrong target bitness results in it throwing an error, at least.

As for the instructions, seems like GNU assembler has those two nonstandard aliases for WRPR:

wrpr %reg, %rd -> wrpr %reg, %g0, %rd
wrpr  imm, %rd -> wrpr  %g0, imm, %rd

God knows why did they decide to place the %g0 in separate places in the expansion but I guess we just have to put up with it.

About LLD, I'm unfamiliar with it and its internals but at least I can confirm that as of now it can only work on very basic use cases, so binutils' LD is still the way to go for SPARC linking needs.

brad0 commented 1 year ago

As an aside, I find it very weird that Linux passes both -mcmodel (which is a 64-bit only option) and -mv8plus (which is a 32-bit only option). Here on my local GCC 12.1, passing the option for the wrong target bitness results in it throwing an error, at least.

The OP said above he was building both sparc and sparc64 kernels. So I think the confusion is the fact the options are all clumped together instead of being separated into the options being passed to a sparc kernel vs a sparc64 kernel.

brad0 commented 1 year ago

From a quick look about the various issues in the report:

-mfpu/-mno-fpu

This is already implemented in clang as -mhard-float/-msoft-float, but I guess an alias could be made for those.

Please provide a diff. Both *BSD and Linux use this.

nickdesaulniers commented 1 year ago

but I guess an alias could be made for those.

Yep!

I have no idea what do those options do

They reserve registers from the register allocator (so you can ensure "important" global variables are always in specific registers for fast access) and change the calling convention. We have to implement them per target in llvm; a few other targets already support these, but support is a per architecture job since the flag contains arch specific registers.

seems like GNU assembler has those two nonstandard aliases for WRPR:

Supporting the 32b ARM target linux kernel builds was a similar story... (see some of my commits that mention ARM in LLVM). GAS has some psuedo instructions when operands make sense to be implied, for shorthand, IIUC.

so binutils' LD is still the way to go for SPARC linking needs.

LLD support for sparc linux kernels would be of interest. Perhaps a dedicated bug for that would be worth pursuing if we could get compiler support first.

brad0 commented 1 year ago

so binutils' LD is still the way to go for SPARC linking needs.

LLD support for sparc linux kernels would be of interest. Perhaps a dedicated bug for that would be worth pursuing if we could get compiler support first.

Even more so on OpenBSD/sparc64. We'll see if we can hobble along with our binutils 2.17 for now. But pushing for Clang + IAS to mature enough so we can switch to Clang as the system compiler first.

koachan commented 1 year ago

Here are the review tickets for the WRPR and FPU flags:

brad0 commented 1 year ago

@ihiasi It would be good to have further feedback based on testing the latest.

brad0 commented 1 year ago

Looking at the Linux kernel Makefile these are the flags for sparc / sparc64 kernel builds..

ifeq ($(CONFIG_SPARC32),y)
#####
# sparc32
#

CHECKFLAGS     += -D__sparc__
KBUILD_LDFLAGS := -m elf32_sparc

# We are adding -Wa,-Av8 to KBUILD_CFLAGS to deal with a specs bug in some
# versions of gcc.  Some gcc versions won't pass -Av8 to binutils when you
# give -mcpu=v8.  This silently worked with older bintutils versions but
# does not any more.
KBUILD_CFLAGS  += -m32 -mcpu=v8 -pipe -mno-fpu -fcall-used-g5 -fcall-used-g7
KBUILD_CFLAGS  += -Wa,-Av8

KBUILD_AFLAGS  += -m32 -Wa,-Av8

else
#####
# sparc64
#

CHECKFLAGS    += -D__sparc__ -D__sparc_v9__ -D__arch64__
KBUILD_LDFLAGS := -m elf64_sparc

KBUILD_CFLAGS += -m64 -pipe -mno-fpu -mcpu=ultrasparc -mcmodel=medlow
KBUILD_CFLAGS += -ffixed-g4 -ffixed-g5 -fcall-used-g7 -Wno-sign-compare
KBUILD_CFLAGS += -Wa,--undeclared-regs
KBUILD_CFLAGS += $(call cc-option,-mtune=ultrasparc3)
KBUILD_AFLAGS += -m64 -mcpu=ultrasparc -Wa,--undeclared-regs

endif
brad0 commented 1 year ago

@koachan Anything else planned after the recent bits went in?

koachan commented 1 year ago

@koachan Anything else planned after the recent bits went in?

From looking at the flags, at least, there's two things that is still missing:

Also, @nickdesaulniers, could you please guide me to how the register allocation flags is implemented in other arches?

nickdesaulniers commented 1 year ago

commit 287a3be37997 ("[AArch64] Support reserving x1-7 registers.")

should be helpful for -fffixed I believe.

brad0 commented 7 months ago
  • Support for register allocation tweaks (-ffixed-g4 -ffixed-g5 -fcall-used-g7 etc.)

Do you have a diff for implementing the -fcall-used-reg options?

brad0 commented 7 months ago

With 18 going out soon, it would be nice to have more feedback. Any other issues building the kernel or other components like glibc.

koachan commented 7 months ago
  • Support for register allocation tweaks (-ffixed-g4 -ffixed-g5 -fcall-used-g7 etc.)

Do you have a diff for implementing the -fcall-used-reg options?

Hmm, not yet, unfortunately, will do if there's some free time opening up~

With 18 going out soon, it would be nice to have more feedback. Any other issues building the kernel or other components like glibc.

I say that at least when it comes to Linux kernel, as soon as the flags issues have been cleaned up we can try doing a test build or two and see what happens? My feeling is that IAS is still probably missing some instruction/register definitions, but fixing that should be able to be done in parallel with testing on builds w/o IAS.

(But then this thread is about compiler flag support so it's probably better to track IAS issues, if any, on a separate thread? I dunno...)

koachan commented 7 months ago

Support for register allocation tweaks (-ffixed-g4 -ffixed-g5 -fcall-used-g7 etc.)

Do you have a diff for implementing the -fcall-used-reg options?

Hmm, not yet, unfortunately, will do if there's some free time opening up~

Or... we could take the -fcall-used-* out of Linux build flags instead.

Looking into it more, it seems like it's a microoptimization attempt rather than something needed for correctness. Building the kernel (using GCC) with those flags results in a working binary, and since LLVM does not have any support for -fcall-used-* for any architecture I feel like adding it in would probably be waaay too invasive to add support for what is essentially just one flag in one codebase (and one that is not even actually needed).

Also, looking at the Linux kernel port, all LLVM-supported architectures do not use those flags too...

What do you think? @brad0 @nickdesaulniers

nickdesaulniers commented 7 months ago

It would be good to do some archeology in git to find the kernel commit that introduced the usage of that flag. That commit's commit message probably has important information and historical context as to why it was ever added in the first place.

Building the kernel (using GCC) with those flags results in a working binary

Did you mean without? If so, then kbuild does have cc-option make macro for adding compiler flags only if they're supported (as in, they are treated as optional).

brad0 commented 7 months ago

Support for register allocation tweaks (-ffixed-g4 -ffixed-g5 -fcall-used-g7 etc.)

Do you have a diff for implementing the -fcall-used-reg options?

Hmm, not yet, unfortunately, will do if there's some free time opening up~

Or... we could take the -fcall-used-* out of Linux build flags instead.

Looking into it more, it seems like it's a microoptimization attempt rather than something needed for correctness. Building the kernel (using GCC) with those flags results in a working binary, and since LLVM does not have any support for -fcall-used-* for any architecture I feel like adding it in would probably be waaay too invasive to add support for what is essentially just one flag in one codebase (and one that is not even actually needed).

Also, looking at the Linux kernel port, all LLVM-supported architectures do not use those flags too...

What do you think? @brad0 @nickdesaulniers

If it's just essentially a micro-optimization then I tend to agree with you.

I am more interested in truly functional issues. Missing bits from IAS and other code building and generation issues.

nickdesaulniers commented 7 months ago

When that flag was used for aarch64, the entire kernel had its own custom calling convention. I don't see how stealing registers from the register allocator could ever be considered an optimization.

koachan commented 6 months ago

It would be good to do some archeology in git to find the kernel commit that introduced the usage of that flag. That commit's commit message probably has important information and historical context as to why it was ever added in the first place.

It seems to date back to the first commit inside git (sparc sparc64), but it's probably even older and the git commit message does not say anything about it.

Building the kernel (using GCC) with those flags results in a working binary

Did you mean without? If so, then kbuild does have cc-option make macro for adding compiler flags only if they're supported (as in, they are treated as optional).

Yep, should be "without"! Sorry for the typo...

When that flag was used for aarch64, the entire kernel had its own custom calling convention. I don't see how stealing registers from the register allocator could ever be considered an optimization.

In the 32-bit ABI, %g5 and %g7 is normally reserved, and in the 64-bit ABI, %g7 is the reserved one. Linux turns them into volatile registers by the way of -fcall-used-*, but on the other hand, omitting the flags shouldn't be harmful either - compilers will now simply refuse to touch them, and any assembly code that happens to touch them would still work like usual (because Linux' conventions already treats them as volatile anyway).

On the other hand, the -ffixed-* ones are indeed vital and support for those have already been implemented.

nickdesaulniers commented 6 months ago

It seems to date back to the first commit inside git (sparc sparc64), but it's probably even older and the git commit message does not say anything about it.

Try checking the history trees?

In the 32-bit ABI, %g5 and %g7 is normally reserved, and in the 64-bit ABI, %g7 is the reserved one. Linux turns them into volatile registers by the way of -fcall-used- On the other hand, the -ffixed- ones are indeed vital and support for those have already been implemented.

Ah, I'm mixing those two flags up! Then yes, it sounds like the kernel is modifying the default calling convention to potentially have additional registers for register allocation available that aren't reserved in the same way that they are for userspace.

Still, it would be helpful to do the archaeology to confirm that that is the case. If so, then cc-option is probably relevant to use in Kbuild in upstream kernel sources, with that archaeological findings in the commit message as justification.

brad0 commented 6 months ago

It would be good to do some archeology in git to find the kernel commit that introduced the usage of that flag. That commit's commit message probably has important information and historical context as to why it was ever added in the first place.

Linux 2.1.29 is the first kernel release / commit based on this archive.

https://github.com/mpe/linux-fullhistory/commit/3dbc14f57b47026fb569d4db4ee20cbddd94fc58

nickdesaulniers commented 6 months ago

Wow, so this predates even the full history tree. Ok, thanks for your due diligence; it will be helpful in the kernel discussions. I guess the next thing to do would be to either

It's probably better to start with the general question to the list. I'd use the kernel's MAINTAINERS file to cc:

koachan commented 6 months ago

So it seems that clang still does not recognize the split L/H inline asm register naming on these functions?

notrace static __always_inline u64 vread_tick(void)
{
    register unsigned long long ret asm("o4");

    __asm__ __volatile__("rd %%tick, %L0\n\t"
                 "srlx %L0, 32, %H0"
                 : "=r" (ret));
    return ret;
}

notrace static __always_inline u64 vread_tick_stick(void)
{
    register unsigned long long ret asm("o4");

    __asm__ __volatile__("rd %%asr24, %L0\n\t"
                 "srlx %L0, 32, %H0"
                 : "=r" (ret));
    return ret;
}

It returns a SPARC "twinword" datatype so %H0 needs to be assigned to an even-numbered register (%o4 in this case as specified by the ret declaration) and %L0 is %H0+1 (%o5 in this case).

Any pointers where do I need to look for if I want to implement this?

s-barannikov commented 6 months ago

Any pointers where do I need to look for if I want to implement this?

Those are called operand modifiers. In LLVM, they are documented here. You will need to modify SparcAsmPrinter::PrintAsmOperand to support 'L' and 'H' in ExtraCode string. I don't think they need special processing anywhere else.

brad0 commented 3 months ago

@koachan What is the status of building the kernel with the latest and greatest?

koachan commented 3 months ago

@koachan What is the status of building the kernel with the latest and greatest?

Sorta mixed, I suppose?

glaubitz commented 3 months ago
* clang can already build a working kernel, but you need to disable IAS and apply two OOT patches to the kernel ([1](https://github.com/koachan/linux-clang/commit/9099c6f66fd32dfba1ca714015ad3b6a637dc05f), [2](https://github.com/koachan/linux-clang/commit/c0114bfc7a4f64bc4d3e63eca6582ec827a8e2a2)) (will try upstreaming them once I have the time to figure out their workflow, basically)

I'm a kernel maintainer and I can help you with the upstreaming process. I also happen to know the SPARC maintainer in the kernel.

* LLD is... pretty much untested, though I wouldn't be surprised if it doesn't work

I assume that LLD is going to be replaced with mold in the long term anyway, isn't it?

glaubitz commented 3 months ago

@koachan Btw, if you have any idea how to fix the LLVM build hanging on the Linux SPARC buildbot, I'd highly appreciate any input.

See: https://lab.llvm.org/staging/#/builders/186

Excerpt from one build:

[914/931] Linking CXX shared module unittests/Passes/Plugins/DoublerPlugin.so
[915/931] Linking CXX executable bin/opt
[916/931] Building CXX object tools/clang/lib/ASTMatchers/Dynamic/CMakeFiles/obj.clangDynamicASTMatchers.dir/Registry.cpp.o
[917/931] Linking CXX static library lib/libclangDynamicASTMatchers.a
command timed out: 1200 seconds without output running [b'ninja'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=5876.102218
s-barannikov commented 3 months ago

The buildbot is likely out of RAM, which usually happens when using bfd linker. Consider using a different linker (e.g. -DLLVM_USE_LINKER=lld), if appropriate.

glaubitz commented 3 months ago

The buildbot is likely out of RAM, which usually happens when using bfd linker. Consider using a different linker (e.g. -DLLVM_USE_LINKER=lld), if appropriate.

Thanks. I just switched to mold, see: https://github.com/llvm/llvm-zorg/pull/203

If that's not enough, I'll add -DLLVM_SPLIT_DWARF=ON.

brad0 commented 3 months ago

I assume that LLD is going to be replaced with mold in the long term anyway, isn't it?

I've never heard that and with only ELF support that's not possible.

brad0 commented 3 months ago
  • IAS is almost there; it can assemble all the instructions but still needs some work (still seeing relocation-related errors)

Ok, please continue to fix any issues here.

  • LLD is... pretty much untested, though I wouldn't be surprised if it doesn't work

It definitely needs work. But don't worry about LLD just yet.

s-barannikov commented 3 months ago

If that's not enough, I'll add -DLLVM_SPLIT_DWARF=ON.

Reducing LLVM_PARALLEL_LINK_JOBS might also help.

glaubitz commented 3 months ago

I assume that LLD is going to be replaced with mold in the long term anyway, isn't it?

I've never heard that and with only ELF support that's not possible.

I see. I thought that LLD would be replaced in the long-term since Apple is also using their own new linker these days.

koachan commented 3 months ago

I'm a kernel maintainer and I can help you with the upstreaming process. I also happen to know the SPARC maintainer in the kernel.

Thank you! I guess I have two questions for now: Where and who should I CC when I submit those patches, and is it okay I submit them separately? I also have a couple questions related to the code itself but I guess it is better talked in another channel?

glaubitz commented 3 months ago

Where and who should I CC when I submit those patches:

There is a script called get_maintainer.pl which will automatically tell you whom to CC with your patch.

This article covers everything you need to know:

http://nickdesaulniers.github.io/blog/2017/05/16/submitting-your-first-patch-to-the-linux-kernel-and-responding-to-feedback/

and is it okay I submit them separately?

Separate to what? Each patch should normally make one logical change.

I also have a couple questions related to the code itself but I guess it is better talked in another channel?

Just post them to the corresponding mailing list according to get_maintainer.pl.