arm64: add KVM_REG_ARM64_SVE_VLS const

[xuejun-xj]

Summary of the PR

For using SVE scenario, KVM_REG_ARM64_SVE_VLS is used by set_one_reg/get_one_reg ioctls, which allows the set of vector lengths supported by the vcpu to be discovered and configured by userspace.
See https://www.kernel.org/doc/html/latest/virt/kvm/api.html#kvm-set-one-reg

When using bindgen, this const is missing, so add it according to the definition in linux.

[xuejun-xj]

Hi! This PR adds missing macro "KVM_REG_ARM64_SVE_VLS", which is used to customize vector length selector of SVE extension. Is there anyone help to review this PR?

[roypat]

Hey, sorry for missing this one. Do we know why this constant is missing when using bindgen? I'd prefer if we could fix our bindgen invocation to include this constant, instead of manually modifying the generated code (as if we do this, we need to write it down as a step to do everytime we regenerate, to prevent the constant from getting lost again).

[xuejun-xj]

Hey, sorry for missing this one. Do we know why this constant is missing when using bindgen? I'd prefer if we could fix our bindgen invocation to include this constant, instead of manually modifying the generated code (as if we do this, we need to write it down as a step to do everytime we regenerate, to prevent the constant from getting lost again).

Yes, I doubt it may be caused by the definition format in C code macros. When I found this const missing, I looked it in kernel code and it is defined in multiple lines linked with "\". Like this:

#define KVM_REG_ARM64_SVE_VLS		(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
					 KVM_REG_SIZE_U512 | 0xffff)

I also looked for other multi-line macros and didn't find them in bindgen.rs. Therefore, I guess maybe bindgen skips macros defined with multiple lines? Can you have a check whether bindgen can handle this kind of macros?

[roypat]

Hey, sorry for missing this one. Do we know why this constant is missing when using bindgen? I'd prefer if we could fix our bindgen invocation to include this constant, instead of manually modifying the generated code (as if we do this, we need to write it down as a step to do everytime we regenerate, to prevent the constant from getting lost again).

Yes, I doubt it may be caused by the definition format in C code macros. When I found this const missing, I looked it in kernel code and it is defined in multiple lines linked with "". Like this:

#define KVM_REG_ARM64_SVE_VLS		(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
					 KVM_REG_SIZE_U512 | 0xffff)

I also looked for other multi-line macros and didn't find them in bindgen.rs. Therefore, I guess maybe bindgen skips macros defined with multiple lines? Can you have a check whether bindgen can handle this kind of macros?

Mh, I don't think its the multi-line definition, I copied the relevant defines into a stand-alone header and everything worked. However if I try to generate bindings from kvm.h they are indeed missing KVM_REG_ARM64_SVE_VLS. I think this is instead related to the definition of KVM_REG_ARM64_SVE_VLS in the linux header arch/arm64/include/uapi/asm/kvm.h using KVM_REG_SIZE_U512 and KVM_REG_ARM64 from include/uapi/linux/kvm.h, however, the arch specific kvm file gets included before those two constants are #defined. It seems that bindgen.rs in this case considers those constants undefined and will omit anything that depends on them

[xuejun-xj]

Hey, sorry for missing this one. Do we know why this constant is missing when using bindgen? I'd prefer if we could fix our bindgen invocation to include this constant, instead of manually modifying the generated code (as if we do this, we need to write it down as a step to do everytime we regenerate, to prevent the constant from getting lost again).

Yes, I doubt it may be caused by the definition format in C code macros. When I found this const missing, I looked it in kernel code and it is defined in multiple lines linked with "". Like this:

#define KVM_REG_ARM64_SVE_VLS		(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
					 KVM_REG_SIZE_U512 | 0xffff)

I also looked for other multi-line macros and didn't find them in bindgen.rs. Therefore, I guess maybe bindgen skips macros defined with multiple lines? Can you have a check whether bindgen can handle this kind of macros?

Mh, I don't think its the multi-line definition, I copied the relevant defines into a stand-alone header and everything worked. However if I try to generate bindings from kvm.h they are indeed missing KVM_REG_ARM64_SVE_VLS. I think this is instead related to the definition of KVM_REG_ARM64_SVE_VLS in the linux header arch/arm64/include/uapi/asm/kvm.h using KVM_REG_SIZE_U512 and KVM_REG_ARM64 from include/uapi/linux/kvm.h, however, the arch specific kvm file gets included before those two constants are #defined. It seems that bindgen.rs in this case considers those constants undefined and will omit anything that depends on them

Yeah, thanks @roypat .
I have got your point. I try to move KVM_REG_ARM64_SVE_VLS into include/linux/kvm.h and generate the binding.rs. This time it contains the const KVM_REG_ARM64_SVE_VLS. I think you are right.
However, another question confuses me that, how does kernel knows these values during compilation? As we all known, KVM_REG_ARM64 and KVM_REG_SIZE_U512 are defined in include/uapi/linux/kvm.h, while KVM_REG_ARM64_SVE_VLS is defined in arch/arm64/include/uapi/asm/kvm.h, which is included by include/uapi/linux/kvm.h.

[xuejun-xj]

Hey, sorry for missing this one. Do we know why this constant is missing when using bindgen? I'd prefer if we could fix our bindgen invocation to include this constant, instead of manually modifying the generated code (as if we do this, we need to write it down as a step to do everytime we regenerate, to prevent the constant from getting lost again).

Yes, I doubt it may be caused by the definition format in C code macros. When I found this const missing, I looked it in kernel code and it is defined in multiple lines linked with "". Like this:

#define KVM_REG_ARM64_SVE_VLS		(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
					 KVM_REG_SIZE_U512 | 0xffff)

I also looked for other multi-line macros and didn't find them in bindgen.rs. Therefore, I guess maybe bindgen skips macros defined with multiple lines? Can you have a check whether bindgen can handle this kind of macros?

Mh, I don't think its the multi-line definition, I copied the relevant defines into a stand-alone header and everything worked. However if I try to generate bindings from kvm.h they are indeed missing KVM_REG_ARM64_SVE_VLS. I think this is instead related to the definition of KVM_REG_ARM64_SVE_VLS in the linux header arch/arm64/include/uapi/asm/kvm.h using KVM_REG_SIZE_U512 and KVM_REG_ARM64 from include/uapi/linux/kvm.h, however, the arch specific kvm file gets included before those two constants are #defined. It seems that bindgen.rs in this case considers those constants undefined and will omit anything that depends on them

Yeah, thanks @roypat . I have got your point. I try to move KVM_REG_ARM64_SVE_VLS into include/linux/kvm.h and generate the binding.rs. This time it contains the const KVM_REG_ARM64_SVE_VLS. I think you are right. However, another question confuses me that, how does kernel knows these values during compilation? As we all known, KVM_REG_ARM64 and KVM_REG_SIZE_U512 are defined in include/uapi/linux/kvm.h, while KVM_REG_ARM64_SVE_VLS is defined in arch/arm64/include/uapi/asm/kvm.h, which is included by include/uapi/linux/kvm.h.

I have checked all the places which use KVM_REG_ARM64_SVE_VLS, and all these files includes include/uapi/linux/kvm.h. This can explain why we can build kernel correctly.

[roypat]

Hey, sorry for missing this one. Do we know why this constant is missing when using bindgen? I'd prefer if we could fix our bindgen invocation to include this constant, instead of manually modifying the generated code (as if we do this, we need to write it down as a step to do everytime we regenerate, to prevent the constant from getting lost again).

Yes, I doubt it may be caused by the definition format in C code macros. When I found this const missing, I looked it in kernel code and it is defined in multiple lines linked with "". Like this:

#define KVM_REG_ARM64_SVE_VLS		(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
					 KVM_REG_SIZE_U512 | 0xffff)

I also looked for other multi-line macros and didn't find them in bindgen.rs. Therefore, I guess maybe bindgen skips macros defined with multiple lines? Can you have a check whether bindgen can handle this kind of macros?

Mh, I don't think its the multi-line definition, I copied the relevant defines into a stand-alone header and everything worked. However if I try to generate bindings from kvm.h they are indeed missing KVM_REG_ARM64_SVE_VLS. I think this is instead related to the definition of KVM_REG_ARM64_SVE_VLS in the linux header arch/arm64/include/uapi/asm/kvm.h using KVM_REG_SIZE_U512 and KVM_REG_ARM64 from include/uapi/linux/kvm.h, however, the arch specific kvm file gets included before those two constants are #defined. It seems that bindgen.rs in this case considers those constants undefined and will omit anything that depends on them

Yeah, thanks @roypat . I have got your point. I try to move KVM_REG_ARM64_SVE_VLS into include/linux/kvm.h and generate the binding.rs. This time it contains the const KVM_REG_ARM64_SVE_VLS. I think you are right. However, another question confuses me that, how does kernel knows these values during compilation? As we all known, KVM_REG_ARM64 and KVM_REG_SIZE_U512 are defined in include/uapi/linux/kvm.h, while KVM_REG_ARM64_SVE_VLS is defined in arch/arm64/include/uapi/asm/kvm.h, which is included by include/uapi/linux/kvm.h.

I have checked all the places which use KVM_REG_ARM64_SVE_VLS, and all these files includes include/uapi/linux/kvm.h. This can explain why we can build kernel correctly.

Mh, does this explain it though? Because include/uapi/linux/kvm.h includes asm/kvm.h before KVM_REG_SIZE_U512 and friends are defined, so just including include/uapi/linux/kvm.h should also fail (I tried this locally but it failed for reasons that are more related to me not knowing enough about C and linux compilation). The only way I can see this working if there is some #include cycle (e.g. somehow asm/kvm.h tries to include include/uapi/linux/kvm.h again before it tries to define KVM_REG_ARM64_SVE_VLS, and due to include guards, this time the pre-processor reaches the point where KVM_REG_USIZE_512 is defined, and from there on it is present for asm/kvm.h), which I guess is too sophisticated for bindgen. I also cant find such a cycle though :(

[xuejun-xj]

Hey, sorry for missing this one. Do we know why this constant is missing when using bindgen? I'd prefer if we could fix our bindgen invocation to include this constant, instead of manually modifying the generated code (as if we do this, we need to write it down as a step to do everytime we regenerate, to prevent the constant from getting lost again).

Yes, I doubt it may be caused by the definition format in C code macros. When I found this const missing, I looked it in kernel code and it is defined in multiple lines linked with "". Like this:

#define KVM_REG_ARM64_SVE_VLS		(KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
					 KVM_REG_SIZE_U512 | 0xffff)

I also looked for other multi-line macros and didn't find them in bindgen.rs. Therefore, I guess maybe bindgen skips macros defined with multiple lines? Can you have a check whether bindgen can handle this kind of macros?

Mh, I don't think its the multi-line definition, I copied the relevant defines into a stand-alone header and everything worked. However if I try to generate bindings from kvm.h they are indeed missing KVM_REG_ARM64_SVE_VLS. I think this is instead related to the definition of KVM_REG_ARM64_SVE_VLS in the linux header arch/arm64/include/uapi/asm/kvm.h using KVM_REG_SIZE_U512 and KVM_REG_ARM64 from include/uapi/linux/kvm.h, however, the arch specific kvm file gets included before those two constants are #defined. It seems that bindgen.rs in this case considers those constants undefined and will omit anything that depends on them

Yeah, thanks @roypat . I have got your point. I try to move KVM_REG_ARM64_SVE_VLS into include/linux/kvm.h and generate the binding.rs. This time it contains the const KVM_REG_ARM64_SVE_VLS. I think you are right. However, another question confuses me that, how does kernel knows these values during compilation? As we all known, KVM_REG_ARM64 and KVM_REG_SIZE_U512 are defined in include/uapi/linux/kvm.h, while KVM_REG_ARM64_SVE_VLS is defined in arch/arm64/include/uapi/asm/kvm.h, which is included by include/uapi/linux/kvm.h.

I have checked all the places which use KVM_REG_ARM64_SVE_VLS, and all these files includes include/uapi/linux/kvm.h. This can explain why we can build kernel correctly.

Mh, does this explain it though? Because include/uapi/linux/kvm.h includes asm/kvm.h before KVM_REG_SIZE_U512 and friends are defined, so just including include/uapi/linux/kvm.h should also fail (I tried this locally but it failed for reasons that are more related to me not knowing enough about C and linux compilation). The only way I can see this working if there is some #include cycle (e.g. somehow asm/kvm.h tries to include include/uapi/linux/kvm.h again before it tries to define KVM_REG_ARM64_SVE_VLS, and due to include guards, this time the pre-processor reaches the point where KVM_REG_USIZE_512 is defined, and from there on it is present for asm/kvm.h), which I guess is too sophisticated for bindgen. I also cant find such a cycle though :(

For C compilation, gcc will include headers and replace const value defined by macro "#define" during pre-process procedure, which won't truly compile the code and just do the replacement. That means we just need to ensure all the symbols can be found in the same file(after expanding "#include" and "#define"...) and binary will be correctly compiled later.
I don't know a lot about bindgen implementation, so maybe it cannot handle this kind of sophisticated case.
I also tried to add "#include <linux/kvm.h>" in asm/kvm.h file before generating binding.rs locally, but failed, too.

[roypat]

For C compilation, gcc will include headers and replace const value defined by macro "#define" during pre-process procedure, which won't truly compile the code and just do the replacement. That means we just need to ensure all the symbols can be found in the same file(after expanding "#include" and "#define"...) and binary will be correctly compiled later. I don't know a lot about bindgen implementation, so maybe it cannot handle this kind of sophisticated case. I also tried to add "#include <linux/kvm.h>" in asm/kvm.h file before generating binding.rs locally, but failed, too.

Ahh, thanks for explaining! I managed to reproduce this outside of the linux tree with this info:

$ cat a.h 
#define MISSING_CONSTANT PRESENT_CONSTANT

$ cat main.c
#include <stdio.h>
#include "a.h"

#define PRESENT_CONSTANT 2

int main() {
    printf("%d\n", MISSING_CONSTANT);
    return 0;
}

$ gcc main.c -o main
$ ./main
2

$ bindgen main.c | grep MISSING
$ bindgen main.c | grep PRESENT
pub const PRESENT_CONSTANT: u32 = 2;

In fact, this already happens if two constants are defined out of order in just a single header:

$ cat b.h
#define A B
#define B 1
$ bindgen b.h 
/* automatically generated by rust-bindgen 0.66.1 */

pub const B: u32 = 1;

I'll file a bug report with bindgen-rs :)

[roypat]

Accidentally pressed "close", sorry

[xuejun-xj]

For C compilation, gcc will include headers and replace const value defined by macro "#define" during pre-process procedure, which won't truly compile the code and just do the replacement. That means we just need to ensure all the symbols can be found in the same file(after expanding "#include" and "#define"...) and binary will be correctly compiled later. I don't know a lot about bindgen implementation, so maybe it cannot handle this kind of sophisticated case. I also tried to add "#include <linux/kvm.h>" in asm/kvm.h file before generating binding.rs locally, but failed, too.

Ahh, thanks for explaining! I managed to reproduce this outside of the linux tree with this info:

$ cat a.h 
#define MISSING_CONSTANT PRESENT_CONSTANT

$ cat main.c
#include <stdio.h>
#include "a.h"

#define PRESENT_CONSTANT 2

int main() {
    printf("%d\n", MISSING_CONSTANT);
    return 0;
}

$ gcc main.c -o main
$ ./main
2

$ bindgen main.c | grep MISSING
$ bindgen main.c | grep PRESENT
pub const PRESENT_CONSTANT: u32 = 2;

In fact, this already happens if two constants are defined out of order in just a single header:

$ cat b.h
#define A B
#define B 1
$ bindgen b.h 
/* automatically generated by rust-bindgen 0.66.1 */

pub const B: u32 = 1;

I'll file a bug report with bindgen-rs :)

Yeah, thanks! I think we have found the bug indeed.
This const is mainly used by user to set customized vector length selector. We may leave this PR later after bindgen-rs fixes the bug. :)