Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

point kvm/qemu/libkvmi to kvmi-v12 branches #127

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

adlazar
Copy link
Collaborator

@adlazar adlazar commented Oct 7, 2021

Point QEMU, KVM and libkvmi submodules to kvmi-v12 branches.
kvmi-v12 has less features than kvmi-v7, but it is two bits closer to upstream :)

@Wenzel
Copy link
Member

Wenzel commented Oct 7, 2021

Surprisingly, it's not libkvmi which failed, it's the self tests:

https://app.travis-ci.com/github/KVM-VMI/kvm-vmi/builds/239353934#L1105

TASK [run kvm self-tests] ******************************************************

fatal: [kvmi]: FAILED! => changed=true 
  cmd:
  - ./tools/testing/selftests/kvm/x86_64/kvmi_test
  delta: '0:00:00.003372'
  end: '2021-10-07 18:22:17.241996'
  msg: non-zero return code
  rc: 4
  start: '2021-10-07 18:22:17.238624'
  stderr: ''
  stderr_lines: []
  stdout: KVM_CAP_INTROSPECTION not available, skipping test
  stdout_lines: <omitted>

@Wenzel
Copy link
Member

Wenzel commented Oct 8, 2021

Libvmi compilation failed

https://app.travis-ci.com/github/KVM-VMI/kvm-vmi/builds/239413034#L1181

    /vagrant/libvmi/libvmi/driver/kvm/kvm_private.h:64:30: error: KVMI_NUM_EVENTS undeclared here (not in a function); did you mean KVMI_VM_EVENT?
         status_t (*process_event[KVMI_NUM_EVENTS])(vmi_instance_t vmi, struct kvmi_dom_event *event);
                                  ^~~~~~~~~~~~~~~
                                  KVMI_VM_EVENT

@adlazar adlazar force-pushed the kvmi-v12 branch 2 times, most recently from e097b00 to 02f54d9 Compare October 8, 2021 17:39
@Wenzel
Copy link
Member

Wenzel commented Oct 11, 2021

The build timed out:
https://app.travis-ci.com/github/KVM-VMI/kvm-vmi/builds/239455542#L1089

Looks like building the kernel takes more time than it used to be.
Before:
kernel + modules compilation: ~7313 seconds
https://app.travis-ci.com/github/KVM-VMI/kvm-vmi/builds/237909810#L1011

now:
kernel + modules compilation: ~9014 seconds
https://app.travis-ci.com/github/KVM-VMI/kvm-vmi/builds/239455542#L1036

The build time increased by +30 minutes which is too much

@adlazar adlazar force-pushed the kvmi-v12 branch 2 times, most recently from 98786ea to b3a5096 Compare October 11, 2021 10:48
@Wenzel
Copy link
Member

Wenzel commented Oct 11, 2021

*** No rule to make target 'kvmconfig'. Stop.

@tklengyel
Copy link

Something is weird, QEMU tells me VMI: missing kernel built with CONFIG_KVM_INTROSPECTION even after booting the v12 kernel with kvm.introspection=1.

@adlazar
Copy link
Collaborator Author

adlazar commented Oct 11, 2021

Something is weird, QEMU tells me VMI: missing kernel built with CONFIG_KVM_INTROSPECTION even after booting the v12 kernel with kvm.introspection=1.

If both (QEMU, kernel) are built with the same KVM_CAP_INTROSPECTION value, double check that you use the right QEMU (/usr/ vs /usr/local/).

@tklengyel
Copy link

Ah yes, that was it :)

@tklengyel
Copy link

@adlazar Any chance you could rebase your libvmi branch on the latest master of libvmi? I tried to do that but I'm getting:

-o libvmi/driver/kvm/.libs/libvmi_la-kvm_events.o
libvmi/driver/kvm/kvm_events.c: In function ‘process_cpuid’:
libvmi/driver/kvm/kvm_events.c:716:59: error: ‘struct kvmi_event’ has no member named ‘arch’
  716 |     struct kvm_regs *kvmi_regs = &kvmi_event->event.common.arch.regs;
      |                                                           ^
libvmi/driver/kvm/kvm_events.c:717:61: error: ‘struct kvmi_event’ has no member named ‘arch’
  717 |     struct kvm_sregs *kvmi_sregs = &kvmi_event->event.common.arch.sregs;
      |                                                             ^
libvmi/driver/kvm/kvm_events.c:720:53: error: ‘struct kvmi_event’ has no member named ‘vcpu’
  720 |     libvmi_event->vcpu_id = kvmi_event->event.common.vcpu;
      |                                                     ^
libvmi/driver/kvm/kvm_events.c:734:44: error: ‘struct kvmi_event’ has no member named ‘vcpu’
  734 |     rpl.hdr.vcpu = kvmi_event->event.common.vcpu;
      |                                            ^
libvmi/driver/kvm/kvm_events.c:735:48: error: ‘struct kvmi_event’ has no member named ‘event’
  735 |     rpl.common.event = kvmi_event->event.common.event;
      |                                                ^
make[1]: *** [Makefile:1973: libvmi/driver/kvm/libvmi_la-kvm_events.lo] Error 1
make[1]: Leaving directory '/shared/kvm-vmi/libvmi'
make: *** [Makefile:1218: all] Error 2

@tklengyel
Copy link

Also, is KVMI_GET_MAX_GFN no longer available? That's going to be an issue since LibVMI needs that to initialize.

@adlazar
Copy link
Collaborator Author

adlazar commented Oct 11, 2021

@adlazar Any chance you could rebase your libvmi branch on the latest master of libvmi? I tried to do that but I'm getting:

-o libvmi/driver/kvm/.libs/libvmi_la-kvm_events.o
libvmi/driver/kvm/kvm_events.c: In function ‘process_cpuid’:
libvmi/driver/kvm/kvm_events.c:716:59: error: ‘struct kvmi_event’ has no member named ‘arch’
  716 |     struct kvm_regs *kvmi_regs = &kvmi_event->event.common.arch.regs;
      |                                                           ^

First, kvmi-v12 is not a replacement for kvmi-v7 (far from it) :(. It has only basic introspection features. While KVMI_GET_MAX_GFN is one of these, I'm not sure is the proper way to obtain that value.

I've used the libvmi commit from kvm-vmi@master as a base (I wasn't sure if upstream was ready for my changes :) ), but the errors are caused by the fact that the wire-protocol changed a bit and it's summarized by this commit message (https://github.com/adlazar/libkvmi/commit/bf95b407c8bdfba07504489603422a68a13012d0). For examples/hookguest.c I've hardcoded 2GB (if kvmi_get_maximum_gfn() fails). I can add back the two kernel patches related to this feature.

For libvmi, most of the changes can be done almost automatically (event.common.event -> event.common.hdr.event and any other event.common.X => event.common.ev.X). See https://github.com/adlazar/libvmi/commit/16f7aa4df2bf5c3d4c39522fd195af4ae5b24169 . Also, struct kvmi_event_pf_reply is empty and some libvmi features won't work (https://github.com/adlazar/libvmi/commit/5e4f9d11bc1ad1e7ab1087e612ebd47a179a0e69).

I've tried to keep the source changes (to libkvmi and libvmi) as small as possible, but to still be able to test the new kernel.

@adlazar
Copy link
Collaborator Author

adlazar commented Oct 11, 2021

@adlazar Any chance you could rebase your libvmi branch on the latest master of libvmi?

Done. Waiting for Travis to build it :)

@adlazar
Copy link
Collaborator Author

adlazar commented Oct 11, 2021

but the errors are caused by the fact that the wire-protocol changed a bit

@tklengyel , not quite, but now I think I've done all the necessarily changes.

@tklengyel
Copy link

tklengyel commented Oct 11, 2021

@adlazar LibVMI right now needs to know the max address or have a complete memory map to determine if a requested access is sane or not. I'm not actually sure this is something absolutely necessary, its just been how LibVMI was setup from the start. Right now without it init will completely fail.

@Wenzel
Copy link
Member

Wenzel commented Oct 12, 2021

Somehow it wasn't able to start the second playbook:

==> kvmi: Machine already provisioned. Run `vagrant provision` or use the `--provision`
==> kvmi: flag to force provisioning. Provisioners marked to run always will still run.
==> kvmi: Attempting direct shutdown of domain...

compared to a normal build

==> kvmi: Machine already provisioned. Run `vagrant provision` or use the `--provision`
==> kvmi: flag to force provisioning. Provisioners marked to run always will still run.
==> kvmi: Running provisioner: ansible_local...

@adlazar
Copy link
Collaborator Author

adlazar commented Oct 12, 2021

Somehow it wasn't able to start the second playbook:

It fails on task/vm. I saw an error related to docker on a local run. My guess is that x86_64_defconfig is not enough for docker.

@adlazar
Copy link
Collaborator Author

adlazar commented Oct 12, 2021

@adlazar LibVMI right now needs to know the max address or have a complete memory map to determine if a requested access is sane or not.

I'll add support for max address, somehow...

@Wenzel
Copy link
Member

Wenzel commented Oct 12, 2021

I tried to rebuild this branch locally with vagrant, and the kvmi-test hangs:

TASK [run kvm self-tests] ******************************************************


I'm running on AMD Ryzen Pro here.
When I executed the kvmi-test manually, i get:

vagrant@debian10:/vagrant/kvm/tools/testing/selftests/kvm/x86_64$ sudo ./kvmi_test 
TODO: test_cmd_vcpu_inject_exception() - make it work with AMD, skipping test

adlazar and others added 6 commits October 12, 2021 15:24
Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>
Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>
and hope that build time will be smaller

Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>
The test can hang and we want to avoid burning Travis CI
credits
@Wenzel
Copy link
Member

Wenzel commented Oct 14, 2021

So, packer depend on docker, which refuses to start

I just tested with vagrant up --no-provision and sudo apt-get packer, it works.

So Docker can't start on the KVMi kernel
I assume there is an kernel option missing here.

@Wenzel
Copy link
Member

Wenzel commented Oct 14, 2021

Just pushed a fix where we install Packer from the zip archive distributed by packer.io instead of the Debian package

@Wenzel
Copy link
Member

Wenzel commented Oct 14, 2021

Yeah, the build is passing ✔️ ❗

@tklengyel
Copy link

To get LibVMI working it still needs this: libvmi/libvmi@8d0fabf

@adlazar
Copy link
Collaborator Author

adlazar commented Oct 26, 2021

@tklengyel , I thought these two commits fixed the memory size issue:

  • QEMU sends the max gpa during handshake
  • libkvmi will use that value from kvmi_get_maximum_gfn()

@tklengyel
Copy link

Ah, didn't catch those commits. Is the QEMU patch going to work even if the guest is booted with ovmf and there is no e820 map?

@adlazar
Copy link
Collaborator Author

adlazar commented Oct 26, 2021

Is the QEMU patch going to work even if the guest is booted with ovmf and there is no e820 map?

I guess so - https://github.com/KVM-VMI/qemu/blob/kvmi-v12/hw/i386/pc.c#L1160

@tklengyel
Copy link

@adlazar is remote mapping no longer available in the 5.15 kernel?

@Wenzel
Copy link
Member

Wenzel commented Nov 4, 2021

@tklengyel I believe kvmi-v12 is a subset of kvmi-v7 features to faciliate the review for KVM maintainers.

@tklengyel
Copy link

I don't know what's the right move here then, I don't see a reason to move to v12 if it has less features. If v12 gets merged to upstream that's a different story, then yes, but with it being out-of-tree just as v7 I would prefer a v7 that's rebased on 5.15 tbh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants