Page MenuHomeFreeBSD

vmm: Lookup vcpu pointers in vmmdev_ioctl.
ClosedPublic

Authored by jhb on Oct 27 2022, 3:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 7, 9:02 AM
Unknown Object (File)
Thu, Nov 7, 5:29 AM
Unknown Object (File)
Wed, Nov 6, 3:29 PM
Unknown Object (File)
Tue, Nov 5, 10:05 AM
Unknown Object (File)
Oct 15 2024, 7:23 PM
Unknown Object (File)
Oct 14 2024, 9:42 PM
Unknown Object (File)
Oct 13 2024, 9:49 AM
Unknown Object (File)
Oct 12 2024, 7:09 AM

Details

Summary

Centralize mapping vCPU IDs to struct vcpu objects in vmmdev_ioctl and
pass vcpu pointers to the routines in vmm.c. For operations that want
to perform an action on all vCPUs or on a single vCPU, pass pointers
to both the VM and the vCPU using a NULL vCPU pointer to request
global actions.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 48040
Build 44927: arc lint + arc unit

Event Timeline

jhb requested review of this revision.Oct 27 2022, 3:05 PM
corvink added inline comments.
sys/amd64/vmm/vmm_dev.c
445

Nit

541–548
555

vcpu could be left uninitialized. Might be a good idea to always initialize it. Not sure if the compiler will complain about it when enabling more warnings.

sys/amd64/vmm/vmm_dev.c
435

vcpu is initialized to NULL here btw.

jhb marked 2 inline comments as done.Nov 4 2022, 9:23 PM
This revision is now accepted and ready to land.Nov 7 2022, 7:16 AM
markj added inline comments.
sys/amd64/vmm/vmm_lapic.c
80

This is a semantic change, right? Before, if vm_active_cpus() is empty, this function would do nothing and return 0 instead of ENOENT.

sys/amd64/vmm/vmm_lapic.c
80

Hmm, it is. Probably I shouldn't change the semantics in this commit (enough moving parts).

jhb marked an inline comment as done.Nov 11 2022, 5:37 PM
sys/amd64/vmm/vmm_lapic.c
80

Btw: why did you changed the layout of the function? The old layout avoids some nesting. This commit could change it to something like:

if (vcpu == NULL)
  dmask = vm_active_cpus(vm);
else
  CPU_SETOF(vcpu_vcpuid(vcpu), &dmask);
error = 0;
CPU_FOREACH_ISSET(cpuid, &dmask)
...
sys/amd64/vmm/vmm_lapic.c
80

I think it was just to avoid doing a round trip of vcpu = vm_vcpu(vm, vm_vcpuid(vcpu)).

sys/amd64/vmm/vmm_lapic.c
80

Yeah, this round trip looks unnecessary. The old layout avoids some code duplication for vlapic = vm_lapic(vcpu); error = vlapic_trigger_lvt(vlapic, vector);.

This revision was automatically updated to reflect the committed changes.