On 11/7/22 18:08, Sean Christopherson wrote:
On Mon, Nov 07, 2022, Paolo Bonzini wrote:
Having inline functions confuses the compilation of asm-offsets.c,
which cannot find kvm_cache_regs.h because arch/x86/kvm is not in
asm-offset.c's include path. Just extract the functions to a
new file.
No functional change intended.
Cc: stable@xxxxxxxxxxxxxxx
Fixes: f14eec0a3203 ("KVM: SVM: move more vmentry code to assembly")
Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
---
arch/x86/kvm/svm/avic.c | 1 +
arch/x86/kvm/svm/nested.c | 1 +
arch/x86/kvm/svm/sev.c | 1 +
arch/x86/kvm/svm/svm.c | 1 +
arch/x86/kvm/svm/svm.h | 200 ------------------------------
arch/x86/kvm/svm/svm_onhyperv.c | 1 +
arch/x86/kvm/svm/vmcb.h | 211 ++++++++++++++++++++++++++++++++
I don't think vmcb.h is a good name. The logical inclusion sequence would be for
svm.h to include vmcb.h, e.g. SVM requires knowledge about VMCBs, but this requires
vmcb.h to include svm.h to dereference "struct vcpu_svm".
Unlike VMX's vmcs.h, the new file isn't a "pure" VMCB helper, it also holds a
decent amount of KVM's SVM logic.
Yes, it's basically the wrappers that KVM uses to access the VMCB fields.
What about making KVM self-sufficient?
You mean having a different asm-offsets.h file just for arch/x86/kvm/?
The includes in asm-offsets.c are quite ugly
#include "../kvm/vmx/vmx.h"
#include "../kvm/svm/svm.h"
or as a stopgap to make backporting easier, just include kvm_cache_regs.h?
The problem is that the _existing_ include of kvm_cache_regs.h in svm.h
fails, with
arch/x86/kernel/../kvm/svm/svm.h:25:10: fatal error: kvm_cache_regs.h:
No such file or directory
25 | #include "kvm_cache_regs.h"
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
The other two solutions here are:
1) move kvm_cache_regs.h to arch/x86/include/asm/ so it can be included
normally
2) extract the structs to arch/x86/kvm/svm/svm_types.h and include that
from asm-offsets.h, basically the opposite of this patch.
(2) is my preference if having a different asm-offsets.h file turns out
to be too complex. We can do the same for VMX as well.
Paolo
void svm_leave_nested(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/svm/svm_onhyperv.c b/arch/x86/kvm/svm/svm_onhyperv.c
index 8cdc62c74a96..ae0a101329e6 100644
--- a/arch/x86/kvm/svm/svm_onhyperv.c
+++ b/arch/x86/kvm/svm/svm_onhyperv.c
@@ -8,6 +8,7 @@
#include <asm/mshyperv.h>
#include "svm.h"
+#include "vmcb.h"
#include "svm_ops.h"
#include "hyperv.h"
diff --git a/arch/x86/kvm/svm/vmcb.h b/arch/x86/kvm/svm/vmcb.h
new file mode 100644
index 000000000000..8757cda27e3a
--- /dev/null
+++ b/arch/x86/kvm/svm/vmcb.h
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Kernel-based Virtual Machine driver for Linux
+ *
+ * AMD SVM support - VMCB accessors
+ */
+
+#ifndef __SVM_VMCB_H
+#define __SVM_VMCB_H
+
+#include "kvm_cache_regs.h"
This should include "svm.h" instead of relying on the parent to include said file.
