Update x86_64 and arm64 bindings to v5.13

[sboeuf]

This patch is a proposal to simplify kvm-bindings crate, moving to the
latest Linux version 5.13.

First thing, instead of maintaining multiple versions (4.14, 4.20 and
now 5.13), this patch only keeps 5.13, and moving forward, we would only
keep the most up to date version.

Signed-off-by: Sebastien Boeuf [email protected]

[sboeuf]

@andreeaflorescu I know I'm removing lots of things through this PR, but I thought I'd give it a try to see what y'all think about it.

[andreeaflorescu]

I was thinking we can still keep the arm & x86 bindings. I am a bit unsure who is using this crate, I saw some other dependencies on it from project I was not aware of before. Is it too much trouble to keep them?

Can you also test this with kvm-ioctls to be sure it still works? We can open an RFC PR in kvm-ioctls to patch kvm-bindings to use your fork and do a dry run of the tests.

We also need to check that this still works in the Firecracker use case, but I would assume it does.

I think it would be better to split the PR in multiple commits:

• one that removes the features, and first just keeps v4.20
• and one that does the update from v4.20 to v5.13
  It should be clear that way what are the changes.

[sboeuf]

I was thinking we can still keep the arm & x86 bindings. I am a bit unsure who is using this crate, I saw some other dependencies on it from project I was not aware of before. Is it too much trouble to keep them?

No worries I can keep them. That being said, I couldn't find kvm.h in the headers generated along with the kernel for arm arch. Should I simply use the one from arm64? @MrXinWang could you give some input there?

Can you also test this with kvm-ioctls to be sure it still works? We can open an RFC PR in kvm-ioctls to patch kvm-bindings to use your fork and do a dry run of the tests.

Yes I can open a draft PR in kvm-ioctls based on my fork.

We also need to check that this still works in the Firecracker use case, but I would assume it does.

Can you try this out on your side?

I think it would be better to split the PR in multiple commits:

• one that removes the features, and first just keeps v4.20
• and one that does the update from v4.20 to v5.13
  It should be clear that way what are the changes.

Yes that sounds like a good plan :)

[MrXinWang]

I was thinking we can still keep the arm & x86 bindings. I am a bit unsure who is using this crate, I saw some other dependencies on it from project I was not aware of before. Is it too much trouble to keep them?

No worries I can keep them. That being said, I couldn't find kvm.h in the headers generated along with the kernel for arm arch. Should I simply use the one from arm64? @MrXinWang could you give some input there?

Hi both, I spent some time digging into the patchwork, and found that the arm32 hypervisor support in kvm has been removed - It explains why @sboeuf cannot found the kvm.h for arm32 . See https://patchwork.kernel.org/project/kvm/cover/[email protected]/.

So I think the idea is you cannot create a 32-bit kvm host, but you always can start a 32-bit kvm guest on 64-bit host.

Therefore I think yes?

[sboeuf]

@MrXinWang thanks for checking. So I'll do the same thing we do for x86 (which means we use the same bindings for both 32 and 64 bits).

[MrXinWang]

As I am not familiar with x86 system (sorry...), maybe for the x86 system @andreeaflorescu can give some input or suggestions :)) Thanks!

[sboeuf]

@andreeaflorescu #43 is the first step (cleaning up 4.14).

[andreeaflorescu]

I'll look into the changes, but I am going on vacation starting with Monday, and I have a few other things I need to get done by then. I'll let you know about my progress.

My plan is to:

• do a diff to see what's changed between 4.20 and 5.13 in terms of structures (and other binding specific defines)
• update the Firecracker's bindings
• run tests to see if it works with the current bindings
• check if upgrading from one version to another works

[sboeuf]

@andreeaflorescu ok this PR is up to date now, based on the previous discussions. Take your time to test it on Firecracker, and once you can confirm it is working fine, we can move forward and merge it (when you're back from your vacation).
In the meantime I'll send a draft PR on kvm-ioctls relying on this branch.

[sboeuf]

Here is the kvm-ioctls draft: rust-vmm/kvm#160

[sboeuf]

@andreeaflorescu if you agree I can simply close #43 and we can merge (when ready) this one directly since it includes #43.

[andreeaflorescu]

Ah, I mistakenly approved this one, I meant to approve the other one. I didn't yet got to integrate the changes in Firecracker. I just did a read up on the diffs. It looks like some structures we're using in Firecracker are changing state (like the kvm_irqchip), but I believe they can be easily addressed.

@alexandruag said he'll take a look at this one as well while I am off.

[andreeaflorescu]

@andreeaflorescu if you agree I can simply close #43 and we can merge (when ready) this one directly since it includes #43.

We can also do as you propose.

Approved by mistake.

[andreeaflorescu]

@sboeuf what bindgen version did you use for generating the new bindings? I think we might need to update this in the documentation as well. Locally it looks like for me it installs v0.59.0.

Looks like I am not fully back from vacation 🤣 I notice now that you're adding the version in the docs, I was just looking at the wrong PR.

[andreeaflorescu]

I did a re-run with the latest version of bindgen. My comments reflect the changes. I think it might be worth it to use the latest version to generate the bindings as there are quite a few more structures that can derive partialeq & debug and those are helpful when writing tests/debugging.

I also finished my analysis of changes on both x86_64 and aarch64, and I'll publish it (still need to figure out where/how 😆); tl;dr: the changes look safe.

We still need to integrate the changes with versionize & firecracker, but I am not expecting any problems.

[sboeuf]

@andreeaflorescu sounds good, I'll update the PR with bindings generated from bindgen v0.59.0

[sboeuf]

@andreeaflorescu PR updated! Please have a look.

[AlexandruCihodaru]

Hello, I have tested it with Firecracker, so far all integration tests are passing.

[andreeaflorescu]

The coverage also needs to be updated.

Once that is fixed, I think we can merge the PR.

[andreeaflorescu]

Hello, I have tested it with Firecracker, so far all integration tests are passing.

Thanks, @AlexandruCihodaru!

[sboeuf]

@andreeaflorescu PR updated accordingly :)

[sboeuf]

@andreeaflorescu can we find someone else to approve and merge this PR?

[acatangiu]

Changes look good!

[alexandruag]

Looks good, apologies for the delay.