vmm: Refactor storage of CPU-dependent per-vCPU data.
ClosedPublic
Actions

Authored by jhb on Oct 27 2022, 3:03 PM.

Details

Reviewers

markj
corvink
manu

Group Reviewers

bhyve

Commits

rG6fc2d2dbe235: vmm: Refactor storage of CPU-dependent per-vCPU data.
rG1aa5150479bf: vmm: Refactor storage of CPU-dependent per-vCPU data.

Summary

Rather than storing static arrays of per-vCPU data in the CPU-specific
per-VM structure, adopt a more dynamic model similar to that used to
manage CPU-specific per-VM data.

That is, add new vmmops methods to init and cleanup a single vCPU.
The init method returns a pointer that is stored in 'struct vcpu' as a
cookie pointer. This cookie pointer is now passed to other vmmops
callbacks in place of the integer index. The index is now only used
in KTR traces and when calling back into the CPU-independent layer.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 48023
Build 44910: arc lint + arc unit

Event Timeline

jhb created this revision.Oct 27 2022, 3:03 PM

Herald added a reviewer: bhyve. · View Herald TranscriptOct 27 2022, 3:03 PM

Herald added subscribers: bcran, rgrimes, imp. · View Herald Transcript

jhb requested review of this revision.Oct 27 2022, 3:03 PM

jhb added a parent revision: D37150: vmm: Rework snapshotting of CPU-specific per-vCPU data..

jhb added a child revision: D37152: vmm: Remove the per-vm cookie argument from vmmops taking a vcpu..

Harbormaster completed remote builds in B48023: Diff 112260.Oct 27 2022, 3:03 PM

corvink added a subscriber: corvink.Nov 2 2022, 9:54 AM

corvink added inline comments.

sys/amd64/vmm/amd/svm.c
385	`sc` is unused. It's a vcpu specific function. So, I would drop the sc parameter. The same applies to other vcpu specific functions.
sys/amd64/vmm/intel/vmx.h
170	Looks like this change is not related to the commit. The change is fine but I would split it into another commit.
sys/amd64/vmm/vmm.c
2877–2879
2970–2971

corvink added inline comments.Nov 2 2022, 9:57 AM

sys/amd64/vmm/amd/svm.c
295

corvink added inline comments.Nov 2 2022, 10:03 AM

sys/amd64/vmm/amd/svm.c
385	Sry, haven't looked at your next commit.

jhb added inline comments.Nov 3 2022, 10:46 PM

sys/amd64/vmm/intel/vmx.h
170	It's related in that the vcpus[] array was removed in this commit. I could maybe split out just the new bool though.
sys/amd64/vmm/vmm.c
2877–2879	I've assumed that vmm.c itself could directly access members of `struct vcpu`. At the end of this series `vcpu_cookie` is no longer present. It was more a helper function for intermediate commits in the series.

corvink accepted this revision.Nov 4 2022, 6:30 AM

corvink added inline comments.

sys/amd64/vmm/intel/vmx.h
170	Got it. Thanks. If you split out the new bool, we could merge it earlier but that's you're decision.
sys/amd64/vmm/vmm.c
2877–2879	You're usage of `vcpu_cookie` is inconsistent. Some functions of `vmm.c` directly access it (like this one) and others are using `vcpu_cookie` to access it (like `vm_set_capability`). As it's only an intermediate helper you can leave it like it is.

This revision is now accepted and ready to land.Nov 4 2022, 6:30 AM

corvink added a reviewer: manu.Nov 4 2022, 7:02 AM

jhb added inline comments.Nov 4 2022, 2:42 PM

sys/amd64/vmm/intel/vmx.h
170	I will probably split it out into a new commit just before this one in the series.
sys/amd64/vmm/vmm.c
2877–2879	Hmm, I re-read the patch again and I see what I did. If I had an existing `struct vcpu ` then I used `vcpu->cookie`. If there wasn't an existing `struct vcpu `, then the code uses `vcpu_cookie()` as a transition aid. Eventually the functions that use the helper are changed to take a `struct vcpu vcpu` as a function argument instead of `struct vm vm, int vcpu` and at that point the functions are changed to use `vcpu->cookie` directly.