* [PATCH v3 01/14] x86/svm: move declarations used only by svm code from svm.h to private header
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
@ 2023-02-24 18:49 ` Xenia Ragiadakou
2023-02-24 18:49 ` [PATCH v3 02/14] x86/svm: make asid.h private Xenia Ragiadakou
` (13 subsequent siblings)
14 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:49 UTC (permalink / raw)
To: xen-devel; +Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu
Create a new private header in arch/x86/hvm/svm called svm.h and move there
all definitions and declarations that are used solely by svm code.
Take the opportunity to remove the forward declaration of struct vcpu, that is
a leftover since the removal of svm_update_guest_cr()'s declaration.
Take the opportunity to re-arrange the header as follows, all structures first,
then all variable decalarations, all function delarations, and finally all
inline functions.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- add SPDX identifier in priv header, reported by Andrew
- add #ifndef header guard, reported by Andrew and Jan
- move svm_invlpga() as well, it was not called anyway
- fold patch removing redundant forward declaration of struct vcpu into
this patch, suggested by Andrew
- rearrange the header in the way Jan's proposed
- update commit message
xen/arch/x86/hvm/svm/nestedsvm.c | 1 +
xen/arch/x86/hvm/svm/svm.c | 2 +
xen/arch/x86/hvm/svm/svm.h | 62 ++++++++++++++++++++++++++
xen/arch/x86/include/asm/hvm/svm/svm.h | 41 -----------------
4 files changed, 65 insertions(+), 41 deletions(-)
create mode 100644 xen/arch/x86/hvm/svm/svm.h
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 77f7547360..a341ccc876 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -27,6 +27,7 @@
#include <asm/event.h> /* for local_event_delivery_(en|dis)able */
#include <asm/p2m.h> /* p2m_get_pagetable, p2m_get_nestedp2m */
+#include "svm.h"
#define NSVM_ERROR_VVMCB 1
#define NSVM_ERROR_VMENTRY 2
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 9c43227b76..6d394e4fe3 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -55,6 +55,8 @@
#include <public/sched.h>
+#include "svm.h"
+
void noreturn svm_asm_do_resume(void);
u32 svm_feature_flags;
diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
new file mode 100644
index 0000000000..9e65919757
--- /dev/null
+++ b/xen/arch/x86/hvm/svm/svm.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * svm.h: SVM Architecture related definitions
+ *
+ * Copyright (c) 2005, AMD Corporation.
+ * Copyright (c) 2004, Intel Corporation.
+ */
+
+#ifndef __X86_HVM_SVM_SVM_PRIV_H__
+#define __X86_HVM_SVM_SVM_PRIV_H__
+
+#include <xen/types.h>
+
+struct cpu_user_regs;
+
+unsigned long *svm_msrbit(unsigned long *msr_bitmap, uint32_t msr);
+void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_len);
+
+static inline void svm_vmload_pa(paddr_t vmcb)
+{
+ asm volatile (
+ ".byte 0x0f,0x01,0xda" /* vmload */
+ : : "a" (vmcb) : "memory" );
+}
+
+static inline void svm_vmsave_pa(paddr_t vmcb)
+{
+ asm volatile (
+ ".byte 0x0f,0x01,0xdb" /* vmsave */
+ : : "a" (vmcb) : "memory" );
+}
+
+static inline void svm_invlpga(unsigned long linear, uint32_t asid)
+{
+ asm volatile (
+ ".byte 0x0f,0x01,0xdf"
+ : /* output */
+ : /* input */
+ "a" (linear), "c" (asid));
+}
+
+/* TSC rate */
+#define DEFAULT_TSC_RATIO 0x0000000100000000ULL
+#define TSC_RATIO_RSVD_BITS 0xffffff0000000000ULL
+
+/* EXITINFO1 fields on NPT faults */
+#define _NPT_PFEC_with_gla 32
+#define NPT_PFEC_with_gla (1UL<<_NPT_PFEC_with_gla)
+#define _NPT_PFEC_in_gpt 33
+#define NPT_PFEC_in_gpt (1UL<<_NPT_PFEC_in_gpt)
+
+#endif /* __X86_HVM_SVM_SVM_PRIV_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/include/asm/hvm/svm/svm.h b/xen/arch/x86/include/asm/hvm/svm/svm.h
index cf9ed517d5..7d5de0122a 100644
--- a/xen/arch/x86/include/asm/hvm/svm/svm.h
+++ b/xen/arch/x86/include/asm/hvm/svm/svm.h
@@ -20,37 +20,6 @@
#ifndef __ASM_X86_HVM_SVM_H__
#define __ASM_X86_HVM_SVM_H__
-#include <xen/types.h>
-
-static inline void svm_vmload_pa(paddr_t vmcb)
-{
- asm volatile (
- ".byte 0x0f,0x01,0xda" /* vmload */
- : : "a" (vmcb) : "memory" );
-}
-
-static inline void svm_vmsave_pa(paddr_t vmcb)
-{
- asm volatile (
- ".byte 0x0f,0x01,0xdb" /* vmsave */
- : : "a" (vmcb) : "memory" );
-}
-
-static inline void svm_invlpga(unsigned long linear, uint32_t asid)
-{
- asm volatile (
- ".byte 0x0f,0x01,0xdf"
- : /* output */
- : /* input */
- "a" (linear), "c" (asid));
-}
-
-struct cpu_user_regs;
-struct vcpu;
-
-unsigned long *svm_msrbit(unsigned long *msr_bitmap, uint32_t msr);
-void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_len);
-
/*
* PV context switch helpers. Prefetching the VMCB area itself has been shown
* to be useful for performance.
@@ -96,14 +65,4 @@ extern u32 svm_feature_flags;
#define cpu_has_svm_sss cpu_has_svm_feature(SVM_FEATURE_SSS)
#define cpu_has_svm_spec_ctrl cpu_has_svm_feature(SVM_FEATURE_SPEC_CTRL)
-/* TSC rate */
-#define DEFAULT_TSC_RATIO 0x0000000100000000ULL
-#define TSC_RATIO_RSVD_BITS 0xffffff0000000000ULL
-
-/* EXITINFO1 fields on NPT faults */
-#define _NPT_PFEC_with_gla 32
-#define NPT_PFEC_with_gla (1UL<<_NPT_PFEC_with_gla)
-#define _NPT_PFEC_in_gpt 33
-#define NPT_PFEC_in_gpt (1UL<<_NPT_PFEC_in_gpt)
-
#endif /* __ASM_X86_HVM_SVM_H__ */
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 02/14] x86/svm: make asid.h private
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
2023-02-24 18:49 ` [PATCH v3 01/14] x86/svm: move declarations used only by svm code from svm.h to private header Xenia Ragiadakou
@ 2023-02-24 18:49 ` Xenia Ragiadakou
2023-02-24 19:42 ` [PATCH v3 02/14 - ALT] x86/svm: Remove the asm/hvm/svm/asid.h header Andrew Cooper
2023-02-24 18:49 ` [PATCH v3 03/14] x86/svm: delete header asm/hvm/svm/intr.h Xenia Ragiadakou
` (12 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:49 UTC (permalink / raw)
To: xen-devel; +Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu
The asm/hvm/svm/asid.h is used only internally by the SVM code, so it can be
changed into a private header.
Take the opportunity to use an SPDX tag for the licence and remove included
but unused xen/types.h.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- new patch
xen/arch/x86/hvm/svm/asid.c | 3 +-
xen/arch/x86/hvm/svm/asid.h | 38 +++++++++++++++++++
xen/arch/x86/hvm/svm/svm.c | 2 +-
xen/arch/x86/include/asm/hvm/svm/asid.h | 49 -------------------------
4 files changed, 41 insertions(+), 51 deletions(-)
create mode 100644 xen/arch/x86/hvm/svm/asid.h
delete mode 100644 xen/arch/x86/include/asm/hvm/svm/asid.h
diff --git a/xen/arch/x86/hvm/svm/asid.c b/xen/arch/x86/hvm/svm/asid.c
index ab06dd3f3a..1128434878 100644
--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -17,9 +17,10 @@
#include <asm/amd.h>
#include <asm/hvm/nestedhvm.h>
-#include <asm/hvm/svm/asid.h>
#include <asm/hvm/svm/svm.h>
+#include "asid.h"
+
void svm_asid_init(const struct cpuinfo_x86 *c)
{
int nasids = 0;
diff --git a/xen/arch/x86/hvm/svm/asid.h b/xen/arch/x86/hvm/svm/asid.h
new file mode 100644
index 0000000000..4dd6abb5fb
--- /dev/null
+++ b/xen/arch/x86/hvm/svm/asid.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * asid.h: handling ASIDs in SVM.
+ *
+ * Copyright (c) 2007, Advanced Micro Devices, Inc.
+ */
+
+#ifndef __X86_HVM_SVM_ASID_PRIV_H__
+#define __X86_HVM_SVM_ASID_PRIV_H__
+
+#include <asm/hvm/asid.h>
+#include <asm/processor.h>
+
+void svm_asid_init(const struct cpuinfo_x86 *c);
+void svm_asid_handle_vmrun(void);
+
+static inline void svm_asid_g_invlpg(struct vcpu *v, unsigned long g_linear)
+{
+#if 0
+ /* Optimization? */
+ svm_invlpga(g_linear, v->arch.hvm.svm.vmcb->guest_asid);
+#endif
+
+ /* Safe fallback. Take a new ASID. */
+ hvm_asid_flush_vcpu(v);
+}
+
+#endif /* __X86_HVM_SVM_ASID_PRIV_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 6d394e4fe3..793a10eaca 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -37,7 +37,6 @@
#include <asm/hvm/monitor.h>
#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/support.h>
-#include <asm/hvm/svm/asid.h>
#include <asm/hvm/svm/emulate.h>
#include <asm/hvm/svm/nestedsvm.h>
#include <asm/hvm/svm/svm.h>
@@ -55,6 +54,7 @@
#include <public/sched.h>
+#include "asid.h"
#include "svm.h"
void noreturn svm_asm_do_resume(void);
diff --git a/xen/arch/x86/include/asm/hvm/svm/asid.h b/xen/arch/x86/include/asm/hvm/svm/asid.h
deleted file mode 100644
index 0e5ec3ab78..0000000000
--- a/xen/arch/x86/include/asm/hvm/svm/asid.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * asid.h: handling ASIDs in SVM.
- * Copyright (c) 2007, Advanced Micro Devices, Inc.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef __ASM_X86_HVM_SVM_ASID_H__
-#define __ASM_X86_HVM_SVM_ASID_H__
-
-#include <xen/types.h>
-#include <asm/hvm/asid.h>
-#include <asm/processor.h>
-
-void svm_asid_init(const struct cpuinfo_x86 *c);
-void svm_asid_handle_vmrun(void);
-
-static inline void svm_asid_g_invlpg(struct vcpu *v, unsigned long g_linear)
-{
-#if 0
- /* Optimization? */
- svm_invlpga(g_linear, v->arch.hvm.svm.vmcb->guest_asid);
-#endif
-
- /* Safe fallback. Take a new ASID. */
- hvm_asid_flush_vcpu(v);
-}
-
-#endif /* __ASM_X86_HVM_SVM_ASID_H__ */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 02/14 - ALT] x86/svm: Remove the asm/hvm/svm/asid.h header
2023-02-24 18:49 ` [PATCH v3 02/14] x86/svm: make asid.h private Xenia Ragiadakou
@ 2023-02-24 19:42 ` Andrew Cooper
2023-02-24 19:59 ` Xenia Ragiadakou
0 siblings, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2023-02-24 19:42 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné,
Wei Liu, Xenia Ragiadakou
Fold svm_asid_g_invlpg() into its single caller, deleting the #if 0 which has
been present for the entire 16 years this helper has existed: c/s 322a078ab140
"[HVM][SVM] Reintroduce ASIDs."
Move the two remaining prototypes into the private svm.h, forward declaring
cpuinfo_x86 in preference to including asm/processor.h
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Xenia Ragiadakou <burzalodowa@gmail.com>
---
xen/arch/x86/hvm/svm/asid.c | 3 +-
xen/arch/x86/hvm/svm/svm.c | 4 +-
xen/arch/x86/hvm/svm/svm.h | 4 ++
xen/arch/x86/include/asm/hvm/svm/asid.h | 49 -------------------------
4 files changed, 8 insertions(+), 52 deletions(-)
delete mode 100644 xen/arch/x86/include/asm/hvm/svm/asid.h
diff --git a/xen/arch/x86/hvm/svm/asid.c b/xen/arch/x86/hvm/svm/asid.c
index ab06dd3f3ae2..150d8dfc8178 100644
--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -17,9 +17,10 @@
#include <asm/amd.h>
#include <asm/hvm/nestedhvm.h>
-#include <asm/hvm/svm/asid.h>
#include <asm/hvm/svm/svm.h>
+#include "svm.h"
+
void svm_asid_init(const struct cpuinfo_x86 *c)
{
int nasids = 0;
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 6d394e4fe3bc..46ae0b6602e2 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -37,7 +37,6 @@
#include <asm/hvm/monitor.h>
#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/support.h>
-#include <asm/hvm/svm/asid.h>
#include <asm/hvm/svm/emulate.h>
#include <asm/hvm/svm/nestedsvm.h>
#include <asm/hvm/svm/svm.h>
@@ -2421,7 +2420,8 @@ static bool cf_check is_invlpg(
static void cf_check svm_invlpg(struct vcpu *v, unsigned long linear)
{
- svm_asid_g_invlpg(v, linear);
+ /* Safe fallback. Take a new ASID. */
+ hvm_asid_flush_vcpu(v);
}
static bool cf_check svm_get_pending_event(
diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
index f700f26f9082..b8178f62161b 100644
--- a/xen/arch/x86/hvm/svm/svm.h
+++ b/xen/arch/x86/hvm/svm/svm.h
@@ -12,6 +12,10 @@
#include <xen/types.h>
struct cpu_user_regs;
+struct cpuinfo_x86;
+
+void svm_asid_init(const struct cpuinfo_x86 *c);
+void svm_asid_handle_vmrun(void);
unsigned long *svm_msrbit(unsigned long *msr_bitmap, uint32_t msr);
void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_len);
diff --git a/xen/arch/x86/include/asm/hvm/svm/asid.h b/xen/arch/x86/include/asm/hvm/svm/asid.h
deleted file mode 100644
index 0e5ec3ab788a..000000000000
--- a/xen/arch/x86/include/asm/hvm/svm/asid.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * asid.h: handling ASIDs in SVM.
- * Copyright (c) 2007, Advanced Micro Devices, Inc.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef __ASM_X86_HVM_SVM_ASID_H__
-#define __ASM_X86_HVM_SVM_ASID_H__
-
-#include <xen/types.h>
-#include <asm/hvm/asid.h>
-#include <asm/processor.h>
-
-void svm_asid_init(const struct cpuinfo_x86 *c);
-void svm_asid_handle_vmrun(void);
-
-static inline void svm_asid_g_invlpg(struct vcpu *v, unsigned long g_linear)
-{
-#if 0
- /* Optimization? */
- svm_invlpga(g_linear, v->arch.hvm.svm.vmcb->guest_asid);
-#endif
-
- /* Safe fallback. Take a new ASID. */
- hvm_asid_flush_vcpu(v);
-}
-
-#endif /* __ASM_X86_HVM_SVM_ASID_H__ */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
--
2.30.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v3 02/14 - ALT] x86/svm: Remove the asm/hvm/svm/asid.h header
2023-02-24 19:42 ` [PATCH v3 02/14 - ALT] x86/svm: Remove the asm/hvm/svm/asid.h header Andrew Cooper
@ 2023-02-24 19:59 ` Xenia Ragiadakou
0 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 19:59 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich, Roger Pau Monné, Wei Liu
On 2/24/23 21:42, Andrew Cooper wrote:
> Fold svm_asid_g_invlpg() into its single caller, deleting the #if 0 which has
> been present for the entire 16 years this helper has existed: c/s 322a078ab140
> "[HVM][SVM] Reintroduce ASIDs."
>
> Move the two remaining prototypes into the private svm.h, forward declaring
> cpuinfo_x86 in preference to including asm/processor.h
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Xenia Ragiadakou <burzalodowa@gmail.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v3 03/14] x86/svm: delete header asm/hvm/svm/intr.h
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
2023-02-24 18:49 ` [PATCH v3 01/14] x86/svm: move declarations used only by svm code from svm.h to private header Xenia Ragiadakou
2023-02-24 18:49 ` [PATCH v3 02/14] x86/svm: make asid.h private Xenia Ragiadakou
@ 2023-02-24 18:49 ` Xenia Ragiadakou
2023-02-24 18:50 ` [PATCH v3 04/14] x86/svm: make emulate.h private Xenia Ragiadakou
` (11 subsequent siblings)
14 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:49 UTC (permalink / raw)
To: xen-devel; +Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu
Delete asm/hvm/svm/intr.h because it contains only the declaration of
svm_intr_assist() which is referenced only by assembly.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- new patch, suggested by Andrew
xen/arch/x86/hvm/svm/intr.c | 1 -
xen/arch/x86/include/asm/hvm/svm/intr.h | 25 -------------------------
2 files changed, 26 deletions(-)
delete mode 100644 xen/arch/x86/include/asm/hvm/svm/intr.h
diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index 9525f35593..d21e930af0 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -29,7 +29,6 @@
#include <asm/hvm/io.h>
#include <asm/hvm/vlapic.h>
#include <asm/hvm/svm/svm.h>
-#include <asm/hvm/svm/intr.h>
#include <asm/hvm/nestedhvm.h> /* for nestedhvm_vcpu_in_guestmode */
#include <asm/vm_event.h>
#include <xen/event.h>
diff --git a/xen/arch/x86/include/asm/hvm/svm/intr.h b/xen/arch/x86/include/asm/hvm/svm/intr.h
deleted file mode 100644
index ae52d9f948..0000000000
--- a/xen/arch/x86/include/asm/hvm/svm/intr.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * intr.h: SVM Architecture related definitions
- * Copyright (c) 2005, AMD Corporation.
- * Copyright (c) 2004, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-#ifndef __ASM_X86_HVM_SVM_INTR_H__
-#define __ASM_X86_HVM_SVM_INTR_H__
-
-void svm_intr_assist(void);
-
-#endif /* __ASM_X86_HVM_SVM_INTR_H__ */
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 04/14] x86/svm: make emulate.h private
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (2 preceding siblings ...)
2023-02-24 18:49 ` [PATCH v3 03/14] x86/svm: delete header asm/hvm/svm/intr.h Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-24 19:50 ` Andrew Cooper
2023-02-24 19:58 ` [PATCH v3 04/14 - ALT] x86/svm: Remove the asm/hvm/svm/emulate.h header Andrew Cooper
2023-02-24 18:50 ` [PATCH v3 05/14] x86/svm: move nestedsvm declarations used only by svm code to private header Xenia Ragiadakou
` (10 subsequent siblings)
14 siblings, 2 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel; +Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu
The header asm/hvm/svm/emulate.h is used only internally by the SVM code,
so it can be changed into a private header.
Take the opportunity to use an SPDX tag for the licence.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- new patch
xen/arch/x86/hvm/svm/emulate.c | 3 ++-
.../x86/{include/asm => }/hvm/svm/emulate.h | 20 +++++--------------
xen/arch/x86/hvm/svm/nestedsvm.c | 2 +-
xen/arch/x86/hvm/svm/svm.c | 2 +-
4 files changed, 9 insertions(+), 18 deletions(-)
rename xen/arch/x86/{include/asm => }/hvm/svm/emulate.h (73%)
diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c
index 16fc134883..4a84b4e761 100644
--- a/xen/arch/x86/hvm/svm/emulate.c
+++ b/xen/arch/x86/hvm/svm/emulate.c
@@ -24,7 +24,8 @@
#include <asm/hvm/hvm.h>
#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/vmcb.h>
-#include <asm/hvm/svm/emulate.h>
+
+#include "emulate.h"
static unsigned long svm_nextrip_insn_length(struct vcpu *v)
{
diff --git a/xen/arch/x86/include/asm/hvm/svm/emulate.h b/xen/arch/x86/hvm/svm/emulate.h
similarity index 73%
rename from xen/arch/x86/include/asm/hvm/svm/emulate.h
rename to xen/arch/x86/hvm/svm/emulate.h
index eb1a8c24af..c0d27772a5 100644
--- a/xen/arch/x86/include/asm/hvm/svm/emulate.h
+++ b/xen/arch/x86/hvm/svm/emulate.h
@@ -1,23 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* emulate.h: SVM instruction emulation bits.
+ *
* Copyright (c) 2005, AMD Corporation.
* Copyright (c) 2004, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
*/
-#ifndef __ASM_X86_HVM_SVM_EMULATE_H__
-#define __ASM_X86_HVM_SVM_EMULATE_H__
+#ifndef __X86_HVM_SVM_EMULATE_PRIV_H__
+#define __X86_HVM_SVM_EMULATE_PRIV_H__
/*
* Encoding for svm_get_insn_len(). We take X86EMUL_OPC() for the main
@@ -53,7 +43,7 @@ struct vcpu;
unsigned int svm_get_insn_len(struct vcpu *v, unsigned int instr_enc);
unsigned int svm_get_task_switch_insn_len(void);
-#endif /* __ASM_X86_HVM_SVM_EMULATE_H__ */
+#endif /* __X86_HVM_SVM_EMULATE_PRIV_H__ */
/*
* Local variables:
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index a341ccc876..5f5752ce21 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -17,7 +17,6 @@
*/
#include <asm/hvm/support.h>
-#include <asm/hvm/svm/emulate.h>
#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/vmcb.h>
#include <asm/hvm/nestedhvm.h>
@@ -27,6 +26,7 @@
#include <asm/event.h> /* for local_event_delivery_(en|dis)able */
#include <asm/p2m.h> /* p2m_get_pagetable, p2m_get_nestedp2m */
+#include "emulate.h"
#include "svm.h"
#define NSVM_ERROR_VVMCB 1
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 793a10eaca..c767a3eb76 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -37,7 +37,6 @@
#include <asm/hvm/monitor.h>
#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/support.h>
-#include <asm/hvm/svm/emulate.h>
#include <asm/hvm/svm/nestedsvm.h>
#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/svmdebug.h>
@@ -55,6 +54,7 @@
#include <public/sched.h>
#include "asid.h"
+#include "emulate.h"
#include "svm.h"
void noreturn svm_asm_do_resume(void);
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v3 04/14] x86/svm: make emulate.h private
2023-02-24 18:50 ` [PATCH v3 04/14] x86/svm: make emulate.h private Xenia Ragiadakou
@ 2023-02-24 19:50 ` Andrew Cooper
2023-02-24 19:58 ` [PATCH v3 04/14 - ALT] x86/svm: Remove the asm/hvm/svm/emulate.h header Andrew Cooper
1 sibling, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2023-02-24 19:50 UTC (permalink / raw)
To: Xenia Ragiadakou, xen-devel; +Cc: Jan Beulich, Roger Pau Monné, Wei Liu
On 24/02/2023 6:50 pm, Xenia Ragiadakou wrote:
> The header asm/hvm/svm/emulate.h is used only internally by the SVM code,
> so it can be changed into a private header.
>
> Take the opportunity to use an SPDX tag for the licence.
>
> No functional change intended.
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
The name emulate.h is rather stale now. We used to have a full ad-hoc
x86 emulator in emulate.{h,c}, before the work to use one single
emulator (rather than the 4(?) we had at the time).
Nowadays, it's just the the instruction length helpers, which you can
see are wrappers around x86_insn_length() which is the main emulator.
Given that it's now just two function declarations and a few constants
for the instr_enc field, it would be better to just move them into the
private svm.h (like I did the asid declarations) and remove the header
entirely.
~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread* [PATCH v3 04/14 - ALT] x86/svm: Remove the asm/hvm/svm/emulate.h header
2023-02-24 18:50 ` [PATCH v3 04/14] x86/svm: make emulate.h private Xenia Ragiadakou
2023-02-24 19:50 ` Andrew Cooper
@ 2023-02-24 19:58 ` Andrew Cooper
2023-02-24 20:04 ` Xenia Ragiadakou
1 sibling, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2023-02-24 19:58 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Xenia Ragiadakou
These days, this is just two length helpers. Move into the private svm.h
No functional change intended.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Xenia Ragiadakou <burzalodowa@gmail.com>
---
xen/arch/x86/hvm/svm/emulate.c | 3 +-
xen/arch/x86/hvm/svm/nestedsvm.c | 1 -
xen/arch/x86/hvm/svm/svm.c | 1 -
xen/arch/x86/hvm/svm/svm.h | 33 +++++++++++
xen/arch/x86/include/asm/hvm/svm/emulate.h | 66 ----------------------
5 files changed, 35 insertions(+), 69 deletions(-)
delete mode 100644 xen/arch/x86/include/asm/hvm/svm/emulate.h
diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c
index 16fc134883cf..391f0255162e 100644
--- a/xen/arch/x86/hvm/svm/emulate.c
+++ b/xen/arch/x86/hvm/svm/emulate.c
@@ -24,7 +24,8 @@
#include <asm/hvm/hvm.h>
#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/vmcb.h>
-#include <asm/hvm/svm/emulate.h>
+
+#include "svm.h"
static unsigned long svm_nextrip_insn_length(struct vcpu *v)
{
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index a341ccc8760e..c0b5474756f4 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -17,7 +17,6 @@
*/
#include <asm/hvm/support.h>
-#include <asm/hvm/svm/emulate.h>
#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/vmcb.h>
#include <asm/hvm/nestedhvm.h>
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 46ae0b6602e2..97783b7f1118 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -37,7 +37,6 @@
#include <asm/hvm/monitor.h>
#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/support.h>
-#include <asm/hvm/svm/emulate.h>
#include <asm/hvm/svm/nestedsvm.h>
#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/svmdebug.h>
diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
index b8178f62161b..d2a781fc3fb5 100644
--- a/xen/arch/x86/hvm/svm/svm.h
+++ b/xen/arch/x86/hvm/svm/svm.h
@@ -13,6 +13,7 @@
struct cpu_user_regs;
struct cpuinfo_x86;
+struct vcpu;
void svm_asid_init(const struct cpuinfo_x86 *c);
void svm_asid_handle_vmrun(void);
@@ -43,6 +44,38 @@ static inline void svm_invlpga(unsigned long linear, uint32_t asid)
"a" (linear), "c" (asid) );
}
+/*
+ * Encoding for svm_get_insn_len(). We take X86EMUL_OPC() for the main
+ * opcode, shifted left to make room for the ModRM byte.
+ *
+ * The Grp7 instructions have their ModRM byte expressed in octal for easier
+ * cross referencing with the opcode extension table.
+ */
+#define INSTR_ENC(opc, modrm) (((opc) << 8) | (modrm))
+
+#define INSTR_PAUSE INSTR_ENC(X86EMUL_OPC_F3(0, 0x90), 0)
+#define INSTR_INT3 INSTR_ENC(X86EMUL_OPC( 0, 0xcc), 0)
+#define INSTR_ICEBP INSTR_ENC(X86EMUL_OPC( 0, 0xf1), 0)
+#define INSTR_HLT INSTR_ENC(X86EMUL_OPC( 0, 0xf4), 0)
+#define INSTR_XSETBV INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0321)
+#define INSTR_VMRUN INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0330)
+#define INSTR_VMCALL INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0331)
+#define INSTR_VMLOAD INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0332)
+#define INSTR_VMSAVE INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0333)
+#define INSTR_STGI INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0334)
+#define INSTR_CLGI INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0335)
+#define INSTR_INVLPGA INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0337)
+#define INSTR_RDTSCP INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0371)
+#define INSTR_INVD INSTR_ENC(X86EMUL_OPC(0x0f, 0x08), 0)
+#define INSTR_WBINVD INSTR_ENC(X86EMUL_OPC(0x0f, 0x09), 0)
+#define INSTR_WRMSR INSTR_ENC(X86EMUL_OPC(0x0f, 0x30), 0)
+#define INSTR_RDTSC INSTR_ENC(X86EMUL_OPC(0x0f, 0x31), 0)
+#define INSTR_RDMSR INSTR_ENC(X86EMUL_OPC(0x0f, 0x32), 0)
+#define INSTR_CPUID INSTR_ENC(X86EMUL_OPC(0x0f, 0xa2), 0)
+
+unsigned int svm_get_insn_len(struct vcpu *v, unsigned int instr_enc);
+unsigned int svm_get_task_switch_insn_len(void);
+
/* TSC rate */
#define DEFAULT_TSC_RATIO 0x0000000100000000ULL
#define TSC_RATIO_RSVD_BITS 0xffffff0000000000ULL
diff --git a/xen/arch/x86/include/asm/hvm/svm/emulate.h b/xen/arch/x86/include/asm/hvm/svm/emulate.h
deleted file mode 100644
index eb1a8c24af6d..000000000000
--- a/xen/arch/x86/include/asm/hvm/svm/emulate.h
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * emulate.h: SVM instruction emulation bits.
- * Copyright (c) 2005, AMD Corporation.
- * Copyright (c) 2004, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef __ASM_X86_HVM_SVM_EMULATE_H__
-#define __ASM_X86_HVM_SVM_EMULATE_H__
-
-/*
- * Encoding for svm_get_insn_len(). We take X86EMUL_OPC() for the main
- * opcode, shifted left to make room for the ModRM byte.
- *
- * The Grp7 instructions have their ModRM byte expressed in octal for easier
- * cross referencing with the opcode extension table.
- */
-#define INSTR_ENC(opc, modrm) (((opc) << 8) | (modrm))
-
-#define INSTR_PAUSE INSTR_ENC(X86EMUL_OPC_F3(0, 0x90), 0)
-#define INSTR_INT3 INSTR_ENC(X86EMUL_OPC( 0, 0xcc), 0)
-#define INSTR_ICEBP INSTR_ENC(X86EMUL_OPC( 0, 0xf1), 0)
-#define INSTR_HLT INSTR_ENC(X86EMUL_OPC( 0, 0xf4), 0)
-#define INSTR_XSETBV INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0321)
-#define INSTR_VMRUN INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0330)
-#define INSTR_VMCALL INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0331)
-#define INSTR_VMLOAD INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0332)
-#define INSTR_VMSAVE INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0333)
-#define INSTR_STGI INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0334)
-#define INSTR_CLGI INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0335)
-#define INSTR_INVLPGA INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0337)
-#define INSTR_RDTSCP INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0371)
-#define INSTR_INVD INSTR_ENC(X86EMUL_OPC(0x0f, 0x08), 0)
-#define INSTR_WBINVD INSTR_ENC(X86EMUL_OPC(0x0f, 0x09), 0)
-#define INSTR_WRMSR INSTR_ENC(X86EMUL_OPC(0x0f, 0x30), 0)
-#define INSTR_RDTSC INSTR_ENC(X86EMUL_OPC(0x0f, 0x31), 0)
-#define INSTR_RDMSR INSTR_ENC(X86EMUL_OPC(0x0f, 0x32), 0)
-#define INSTR_CPUID INSTR_ENC(X86EMUL_OPC(0x0f, 0xa2), 0)
-
-struct vcpu;
-
-unsigned int svm_get_insn_len(struct vcpu *v, unsigned int instr_enc);
-unsigned int svm_get_task_switch_insn_len(void);
-
-#endif /* __ASM_X86_HVM_SVM_EMULATE_H__ */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
--
2.30.2
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v3 05/14] x86/svm: move nestedsvm declarations used only by svm code to private header
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (3 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 04/14] x86/svm: make emulate.h private Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-24 20:12 ` Andrew Cooper
2023-02-24 21:06 ` [PATCH v3 05.5/14] x86/svm: Decouple types in struct nestedsvm Andrew Cooper
2023-02-24 18:50 ` [PATCH v3 06/14] x86/svm: move vmcb declarations used only by svm code to private header Xenia Ragiadakou
` (9 subsequent siblings)
14 siblings, 2 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel; +Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu
Create a new private header in arch/x86/hvm/svm called nestedsvm.h and move
there all definitions and declarations that are used only by svm code and
don't need to reside in an external header.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- new patch
xen/arch/x86/hvm/svm/intr.c | 2 +
xen/arch/x86/hvm/svm/nestedhvm.h | 77 ++++++++++++++++++++
xen/arch/x86/hvm/svm/nestedsvm.c | 2 +-
xen/arch/x86/hvm/svm/svm.c | 2 +-
xen/arch/x86/hvm/svm/svm.h | 2 +-
xen/arch/x86/include/asm/hvm/svm/nestedsvm.h | 53 +-------------
6 files changed, 86 insertions(+), 52 deletions(-)
create mode 100644 xen/arch/x86/hvm/svm/nestedhvm.h
diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index d21e930af0..dbb0022190 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -37,6 +37,8 @@
#include <xen/domain_page.h>
#include <asm/hvm/trace.h>
+#include "nestedhvm.h"
+
static void svm_inject_nmi(struct vcpu *v)
{
struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h b/xen/arch/x86/hvm/svm/nestedhvm.h
new file mode 100644
index 0000000000..43245e13de
--- /dev/null
+++ b/xen/arch/x86/hvm/svm/nestedhvm.h
@@ -0,0 +1,77 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * nestedsvm.h: Nested Virtualization
+ *
+ * Copyright (c) 2011, Advanced Micro Devices, Inc
+ */
+
+#ifndef __X86_HVM_SVM_NESTEDHVM_PRIV_H__
+#define __X86_HVM_SVM_NESTEDHVM_PRIV_H__
+
+#include <xen/mm.h>
+#include <xen/types.h>
+
+#include <asm/hvm/vcpu.h>
+#include <asm/hvm/hvm.h>
+#include <asm/hvm/nestedhvm.h>
+#include <asm/msr-index.h>
+
+/* SVM specific intblk types, cannot be an enum because gcc 4.5 complains */
+/* GIF cleared */
+#define hvm_intblk_svm_gif hvm_intblk_arch
+
+#define vcpu_nestedsvm(v) (vcpu_nestedhvm(v).u.nsvm)
+
+/* True when l1 guest enabled SVM in EFER */
+#define nsvm_efer_svm_enabled(v) \
+ (!!((v)->arch.hvm.guest_efer & EFER_SVME))
+
+int nestedsvm_vmcb_map(struct vcpu *v, uint64_t vmcbaddr);
+void nestedsvm_vmexit_defer(struct vcpu *v,
+ uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2);
+enum nestedhvm_vmexits
+nestedsvm_vmexit_n2n1(struct vcpu *v, struct cpu_user_regs *regs);
+enum nestedhvm_vmexits
+nestedsvm_check_intercepts(struct vcpu *v, struct cpu_user_regs *regs,
+ uint64_t exitcode);
+void svm_nested_features_on_efer_update(struct vcpu *v);
+
+/* Interface methods */
+void cf_check nsvm_vcpu_destroy(struct vcpu *v);
+int cf_check nsvm_vcpu_initialise(struct vcpu *v);
+int cf_check nsvm_vcpu_reset(struct vcpu *v);
+int nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs);
+int cf_check nsvm_vcpu_vmexit_event(struct vcpu *v,
+ const struct x86_event *event);
+uint64_t cf_check nsvm_vcpu_hostcr3(struct vcpu *v);
+bool cf_check nsvm_vmcb_guest_intercepts_event(
+ struct vcpu *v, unsigned int vector, int errcode);
+bool cf_check nsvm_vmcb_hap_enabled(struct vcpu *v);
+enum hvm_intblk cf_check nsvm_intr_blocked(struct vcpu *v);
+
+/* Interrupts, vGIF */
+void svm_vmexit_do_clgi(struct cpu_user_regs *regs, struct vcpu *v);
+void svm_vmexit_do_stgi(struct cpu_user_regs *regs, struct vcpu *v);
+bool nestedsvm_gif_isset(struct vcpu *v);
+int cf_check nsvm_hap_walk_L1_p2m(
+ struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa, unsigned int *page_order,
+ uint8_t *p2m_acc, struct npfec npfec);
+
+#define NSVM_INTR_NOTHANDLED 3
+#define NSVM_INTR_NOTINTERCEPTED 2
+#define NSVM_INTR_FORCEVMEXIT 1
+#define NSVM_INTR_MASKED 0
+
+int nestedsvm_vcpu_interrupt(struct vcpu *v, const struct hvm_intack intack);
+
+#endif /* __X86_HVM_SVM_NESTEDHVM_PRIV_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 5f5752ce21..80b72b5dee 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -20,13 +20,13 @@
#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/vmcb.h>
#include <asm/hvm/nestedhvm.h>
-#include <asm/hvm/svm/nestedsvm.h>
#include <asm/hvm/svm/svmdebug.h>
#include <asm/paging.h> /* paging_mode_hap */
#include <asm/event.h> /* for local_event_delivery_(en|dis)able */
#include <asm/p2m.h> /* p2m_get_pagetable, p2m_get_nestedp2m */
#include "emulate.h"
+#include "nestedhvm.h"
#include "svm.h"
#define NSVM_ERROR_VVMCB 1
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index c767a3eb76..4b74ee3d7c 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -37,7 +37,6 @@
#include <asm/hvm/monitor.h>
#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/support.h>
-#include <asm/hvm/svm/nestedsvm.h>
#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/svmdebug.h>
#include <asm/hvm/svm/vmcb.h>
@@ -55,6 +54,7 @@
#include "asid.h"
#include "emulate.h"
+#include "nestedhvm.h"
#include "svm.h"
void noreturn svm_asm_do_resume(void);
diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
index 9e65919757..f700f26f90 100644
--- a/xen/arch/x86/hvm/svm/svm.h
+++ b/xen/arch/x86/hvm/svm/svm.h
@@ -36,7 +36,7 @@ static inline void svm_invlpga(unsigned long linear, uint32_t asid)
".byte 0x0f,0x01,0xdf"
: /* output */
: /* input */
- "a" (linear), "c" (asid));
+ "a" (linear), "c" (asid) );
}
/* TSC rate */
diff --git a/xen/arch/x86/include/asm/hvm/svm/nestedsvm.h b/xen/arch/x86/include/asm/hvm/svm/nestedsvm.h
index 656d7d1a9a..94d45d2e8d 100644
--- a/xen/arch/x86/include/asm/hvm/svm/nestedsvm.h
+++ b/xen/arch/x86/include/asm/hvm/svm/nestedsvm.h
@@ -18,15 +18,12 @@
#ifndef __ASM_X86_HVM_SVM_NESTEDSVM_H__
#define __ASM_X86_HVM_SVM_NESTEDSVM_H__
-#include <asm/hvm/hvm.h>
-#include <asm/hvm/svm/vmcb.h>
+#include <xen/types.h>
-/* SVM specific intblk types, cannot be an enum because gcc 4.5 complains */
-/* GIF cleared */
-#define hvm_intblk_svm_gif hvm_intblk_arch
+#include <asm/hvm/svm/vmcb.h>
struct nestedsvm {
- bool_t ns_gif;
+ bool ns_gif;
uint64_t ns_msr_hsavepa; /* MSR HSAVE_PA value */
/* l1 guest physical address of virtual vmcb used by prior VMRUN.
@@ -72,7 +69,7 @@ struct nestedsvm {
uint64_t ns_vmcb_guestcr3, ns_vmcb_hostcr3;
uint32_t ns_guest_asid;
- bool_t ns_hap_enabled;
+ bool ns_hap_enabled;
/* Only meaningful when vmexit_pending flag is set */
struct {
@@ -90,48 +87,6 @@ struct nestedsvm {
} ns_hostflags;
};
-#define vcpu_nestedsvm(v) (vcpu_nestedhvm(v).u.nsvm)
-
-/* True when l1 guest enabled SVM in EFER */
-#define nsvm_efer_svm_enabled(v) \
- (!!((v)->arch.hvm.guest_efer & EFER_SVME))
-
-int nestedsvm_vmcb_map(struct vcpu *v, uint64_t vmcbaddr);
-void nestedsvm_vmexit_defer(struct vcpu *v,
- uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2);
-enum nestedhvm_vmexits
-nestedsvm_vmexit_n2n1(struct vcpu *v, struct cpu_user_regs *regs);
-enum nestedhvm_vmexits
-nestedsvm_check_intercepts(struct vcpu *v, struct cpu_user_regs *regs,
- uint64_t exitcode);
-void svm_nested_features_on_efer_update(struct vcpu *v);
-
-/* Interface methods */
-void cf_check nsvm_vcpu_destroy(struct vcpu *v);
-int cf_check nsvm_vcpu_initialise(struct vcpu *v);
-int cf_check nsvm_vcpu_reset(struct vcpu *v);
-int nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs);
-int cf_check nsvm_vcpu_vmexit_event(struct vcpu *v, const struct x86_event *event);
-uint64_t cf_check nsvm_vcpu_hostcr3(struct vcpu *v);
-bool cf_check nsvm_vmcb_guest_intercepts_event(
- struct vcpu *v, unsigned int vector, int errcode);
-bool cf_check nsvm_vmcb_hap_enabled(struct vcpu *v);
-enum hvm_intblk cf_check nsvm_intr_blocked(struct vcpu *v);
-
-/* Interrupts, vGIF */
-void svm_vmexit_do_clgi(struct cpu_user_regs *regs, struct vcpu *v);
-void svm_vmexit_do_stgi(struct cpu_user_regs *regs, struct vcpu *v);
-bool_t nestedsvm_gif_isset(struct vcpu *v);
-int cf_check nsvm_hap_walk_L1_p2m(
- struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa, unsigned int *page_order,
- uint8_t *p2m_acc, struct npfec npfec);
-
-#define NSVM_INTR_NOTHANDLED 3
-#define NSVM_INTR_NOTINTERCEPTED 2
-#define NSVM_INTR_FORCEVMEXIT 1
-#define NSVM_INTR_MASKED 0
-int nestedsvm_vcpu_interrupt(struct vcpu *v, const struct hvm_intack intack);
-
#endif /* ASM_X86_HVM_SVM_NESTEDSVM_H__ */
/*
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v3 05/14] x86/svm: move nestedsvm declarations used only by svm code to private header
2023-02-24 18:50 ` [PATCH v3 05/14] x86/svm: move nestedsvm declarations used only by svm code to private header Xenia Ragiadakou
@ 2023-02-24 20:12 ` Andrew Cooper
2023-02-24 20:28 ` Xenia Ragiadakou
2023-02-24 21:06 ` [PATCH v3 05.5/14] x86/svm: Decouple types in struct nestedsvm Andrew Cooper
1 sibling, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2023-02-24 20:12 UTC (permalink / raw)
To: Xenia Ragiadakou, xen-devel; +Cc: Jan Beulich, Roger Pau Monné, Wei Liu
On 24/02/2023 6:50 pm, Xenia Ragiadakou wrote:
> diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h b/xen/arch/x86/hvm/svm/nestedhvm.h
> new file mode 100644
> index 0000000000..43245e13de
> --- /dev/null
> +++ b/xen/arch/x86/hvm/svm/nestedhvm.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * nestedsvm.h: Nested Virtualization
> + *
> + * Copyright (c) 2011, Advanced Micro Devices, Inc
> + */
> +
> +#ifndef __X86_HVM_SVM_NESTEDHVM_PRIV_H__
> +#define __X86_HVM_SVM_NESTEDHVM_PRIV_H__
> +
> +#include <xen/mm.h>
> +#include <xen/types.h>
> +
> +#include <asm/hvm/vcpu.h>
> +#include <asm/hvm/hvm.h>
> +#include <asm/hvm/nestedhvm.h>
> +#include <asm/msr-index.h>
> +
> +/* SVM specific intblk types, cannot be an enum because gcc 4.5 complains */
> +/* GIF cleared */
> +#define hvm_intblk_svm_gif hvm_intblk_arch
I know you're just moving code, but I simply don't believe this comment.
This additional delta seems to compile fine:
diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index dbb0022190a8..111b10673cf4 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -154,7 +154,7 @@ void svm_intr_assist(void)
return;
intblk = hvm_interrupt_blocked(v, intack);
- if ( intblk == hvm_intblk_svm_gif )
+ if ( intblk == hvm_intblk_arch ) /* GIF */
{
ASSERT(nestedhvm_enabled(v->domain));
return;
diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h
b/xen/arch/x86/hvm/svm/nestedhvm.h
index 43245e13deb7..c7747daae24a 100644
--- a/xen/arch/x86/hvm/svm/nestedhvm.h
+++ b/xen/arch/x86/hvm/svm/nestedhvm.h
@@ -16,10 +16,6 @@
#include <asm/hvm/nestedhvm.h>
#include <asm/msr-index.h>
-/* SVM specific intblk types, cannot be an enum because gcc 4.5
complains */
-/* GIF cleared */
-#define hvm_intblk_svm_gif hvm_intblk_arch
-
#define vcpu_nestedsvm(v) (vcpu_nestedhvm(v).u.nsvm)
/* True when l1 guest enabled SVM in EFER */
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c
b/xen/arch/x86/hvm/svm/nestedsvm.c
index 92316c6624ce..1794eb2105be 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1247,7 +1247,7 @@ enum hvm_intblk cf_check nsvm_intr_blocked(struct
vcpu *v)
ASSERT(nestedhvm_enabled(v->domain));
if ( !nestedsvm_gif_isset(v) )
- return hvm_intblk_svm_gif;
+ return hvm_intblk_arch;
if ( nestedhvm_vcpu_in_guestmode(v) )
{
but the first hunk demonstrates an error in the original logic.
Architecturally, GIF can become set for SKINIT, and in systems where SVM
isn't available.
I'm unsure whether its better to fix this up in this patch, or defer it
for later.
> +
> +#define vcpu_nestedsvm(v) (vcpu_nestedhvm(v).u.nsvm)
> +
> +/* True when l1 guest enabled SVM in EFER */
> +#define nsvm_efer_svm_enabled(v) \
> + (!!((v)->arch.hvm.guest_efer & EFER_SVME))
This seems to be the only use of asm/msr-index.h, and it's a macro so
the header is actually unused.
I'd drop the include - its a very common header anyway.
~Andrew
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v3 05/14] x86/svm: move nestedsvm declarations used only by svm code to private header
2023-02-24 20:12 ` Andrew Cooper
@ 2023-02-24 20:28 ` Xenia Ragiadakou
2023-02-24 20:34 ` Andrew Cooper
0 siblings, 1 reply; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 20:28 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: Jan Beulich, Roger Pau Monné, Wei Liu
On 2/24/23 22:12, Andrew Cooper wrote:
> On 24/02/2023 6:50 pm, Xenia Ragiadakou wrote:
>> diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h b/xen/arch/x86/hvm/svm/nestedhvm.h
>> new file mode 100644
>> index 0000000000..43245e13de
>> --- /dev/null
>> +++ b/xen/arch/x86/hvm/svm/nestedhvm.h
>> @@ -0,0 +1,77 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * nestedsvm.h: Nested Virtualization
>> + *
>> + * Copyright (c) 2011, Advanced Micro Devices, Inc
>> + */
>> +
>> +#ifndef __X86_HVM_SVM_NESTEDHVM_PRIV_H__
>> +#define __X86_HVM_SVM_NESTEDHVM_PRIV_H__
>> +
>> +#include <xen/mm.h>
>> +#include <xen/types.h>
>> +
>> +#include <asm/hvm/vcpu.h>
>> +#include <asm/hvm/hvm.h>
>> +#include <asm/hvm/nestedhvm.h>
>> +#include <asm/msr-index.h>
>> +
>> +/* SVM specific intblk types, cannot be an enum because gcc 4.5 complains */
>> +/* GIF cleared */
>> +#define hvm_intblk_svm_gif hvm_intblk_arch
>
> I know you're just moving code, but I simply don't believe this comment.
>
> This additional delta seems to compile fine:
>
> diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
> index dbb0022190a8..111b10673cf4 100644
> --- a/xen/arch/x86/hvm/svm/intr.c
> +++ b/xen/arch/x86/hvm/svm/intr.c
> @@ -154,7 +154,7 @@ void svm_intr_assist(void)
> return;
>
> intblk = hvm_interrupt_blocked(v, intack);
> - if ( intblk == hvm_intblk_svm_gif )
> + if ( intblk == hvm_intblk_arch ) /* GIF */
> {
> ASSERT(nestedhvm_enabled(v->domain));
> return;
> diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h
> b/xen/arch/x86/hvm/svm/nestedhvm.h
> index 43245e13deb7..c7747daae24a 100644
> --- a/xen/arch/x86/hvm/svm/nestedhvm.h
> +++ b/xen/arch/x86/hvm/svm/nestedhvm.h
> @@ -16,10 +16,6 @@
> #include <asm/hvm/nestedhvm.h>
> #include <asm/msr-index.h>
>
> -/* SVM specific intblk types, cannot be an enum because gcc 4.5
> complains */
> -/* GIF cleared */
> -#define hvm_intblk_svm_gif hvm_intblk_arch
> -
> #define vcpu_nestedsvm(v) (vcpu_nestedhvm(v).u.nsvm)
>
> /* True when l1 guest enabled SVM in EFER */
> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c
> b/xen/arch/x86/hvm/svm/nestedsvm.c
> index 92316c6624ce..1794eb2105be 100644
> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -1247,7 +1247,7 @@ enum hvm_intblk cf_check nsvm_intr_blocked(struct
> vcpu *v)
> ASSERT(nestedhvm_enabled(v->domain));
>
> if ( !nestedsvm_gif_isset(v) )
> - return hvm_intblk_svm_gif;
> + return hvm_intblk_arch;
>
> if ( nestedhvm_vcpu_in_guestmode(v) )
> {
>
>
> but the first hunk demonstrates an error in the original logic.
> Architecturally, GIF can become set for SKINIT, and in systems where SVM
> isn't available.
>
> I'm unsure whether its better to fix this up in this patch, or defer it
> for later.
I think this change merits its own patch.
>
>> +
>> +#define vcpu_nestedsvm(v) (vcpu_nestedhvm(v).u.nsvm)
>> +
>> +/* True when l1 guest enabled SVM in EFER */
>> +#define nsvm_efer_svm_enabled(v) \
>> + (!!((v)->arch.hvm.guest_efer & EFER_SVME))
>
> This seems to be the only use of asm/msr-index.h, and it's a macro so
> the header is actually unused.
>
> I'd drop the include - its a very common header anyway.
Feel free to drop it. There was not in the other header. I have a
tendency to include headers for everything.
>
> ~Andrew
--
Xenia
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 05/14] x86/svm: move nestedsvm declarations used only by svm code to private header
2023-02-24 20:28 ` Xenia Ragiadakou
@ 2023-02-24 20:34 ` Andrew Cooper
0 siblings, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2023-02-24 20:34 UTC (permalink / raw)
To: Xenia Ragiadakou, xen-devel; +Cc: Jan Beulich, Roger Pau Monné, Wei Liu
On 24/02/2023 8:28 pm, Xenia Ragiadakou wrote:
>
> On 2/24/23 22:12, Andrew Cooper wrote:
>> On 24/02/2023 6:50 pm, Xenia Ragiadakou wrote:
>>> diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h
>>> b/xen/arch/x86/hvm/svm/nestedhvm.h
>>> new file mode 100644
>>> index 0000000000..43245e13de
>>> --- /dev/null
>>> +++ b/xen/arch/x86/hvm/svm/nestedhvm.h
>>> @@ -0,0 +1,77 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * nestedsvm.h: Nested Virtualization
>>> + *
>>> + * Copyright (c) 2011, Advanced Micro Devices, Inc
>>> + */
>>> +
>>> +#ifndef __X86_HVM_SVM_NESTEDHVM_PRIV_H__
>>> +#define __X86_HVM_SVM_NESTEDHVM_PRIV_H__
>>> +
>>> +#include <xen/mm.h>
>>> +#include <xen/types.h>
>>> +
>>> +#include <asm/hvm/vcpu.h>
>>> +#include <asm/hvm/hvm.h>
>>> +#include <asm/hvm/nestedhvm.h>
>>> +#include <asm/msr-index.h>
>>> +
>>> +/* SVM specific intblk types, cannot be an enum because gcc 4.5
>>> complains */
>>> +/* GIF cleared */
>>> +#define hvm_intblk_svm_gif hvm_intblk_arch
>>
>> I know you're just moving code, but I simply don't believe this comment.
>>
>> This additional delta seems to compile fine:
>>
>> diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
>> index dbb0022190a8..111b10673cf4 100644
>> --- a/xen/arch/x86/hvm/svm/intr.c
>> +++ b/xen/arch/x86/hvm/svm/intr.c
>> @@ -154,7 +154,7 @@ void svm_intr_assist(void)
>> return;
>> intblk = hvm_interrupt_blocked(v, intack);
>> - if ( intblk == hvm_intblk_svm_gif )
>> + if ( intblk == hvm_intblk_arch ) /* GIF */
>> {
>> ASSERT(nestedhvm_enabled(v->domain));
>> return;
>> diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h
>> b/xen/arch/x86/hvm/svm/nestedhvm.h
>> index 43245e13deb7..c7747daae24a 100644
>> --- a/xen/arch/x86/hvm/svm/nestedhvm.h
>> +++ b/xen/arch/x86/hvm/svm/nestedhvm.h
>> @@ -16,10 +16,6 @@
>> #include <asm/hvm/nestedhvm.h>
>> #include <asm/msr-index.h>
>> -/* SVM specific intblk types, cannot be an enum because gcc 4.5
>> complains */
>> -/* GIF cleared */
>> -#define hvm_intblk_svm_gif hvm_intblk_arch
>> -
>> #define vcpu_nestedsvm(v) (vcpu_nestedhvm(v).u.nsvm)
>> /* True when l1 guest enabled SVM in EFER */
>> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c
>> b/xen/arch/x86/hvm/svm/nestedsvm.c
>> index 92316c6624ce..1794eb2105be 100644
>> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
>> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
>> @@ -1247,7 +1247,7 @@ enum hvm_intblk cf_check nsvm_intr_blocked(struct
>> vcpu *v)
>> ASSERT(nestedhvm_enabled(v->domain));
>> if ( !nestedsvm_gif_isset(v) )
>> - return hvm_intblk_svm_gif;
>> + return hvm_intblk_arch;
>> if ( nestedhvm_vcpu_in_guestmode(v) )
>> {
>>
>>
>> but the first hunk demonstrates an error in the original logic.
>> Architecturally, GIF can become set for SKINIT, and in systems where SVM
>> isn't available.
>>
>> I'm unsure whether its better to fix this up in this patch, or defer it
>> for later.
>
> I think this change merits its own patch.
Yeah, it probably does.
Seeing as you've reviewed my two alt patches, I'll commit some more as
I've already resolved the conflicts around adjacent headers.
~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v3 05.5/14] x86/svm: Decouple types in struct nestedsvm
2023-02-24 18:50 ` [PATCH v3 05/14] x86/svm: move nestedsvm declarations used only by svm code to private header Xenia Ragiadakou
2023-02-24 20:12 ` Andrew Cooper
@ 2023-02-24 21:06 ` Andrew Cooper
2023-02-27 8:52 ` Xenia Ragiadakou
1 sibling, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2023-02-24 21:06 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Xenia Ragiadakou
struct nestedvm uses mostly plain integer types, except for virt_ext_t which
is a union wrapping two bitfield names. But the nested virt logic only ever
deals with it as full opaque register.
Switch it to being a plain uint64_t, allowing us to hide even more of the SVM
internal infrastructure.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Xenia Ragiadakou <burzalodowa@gmail.com>
This allows virt_ext_t to move out of the public vmcb.h along with all other
vmcb types.
---
xen/arch/x86/hvm/svm/nestedsvm.c | 4 ++--
xen/arch/x86/include/asm/hvm/svm/nestedsvm.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 92316c6624ce..153a37f2f227 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -164,7 +164,7 @@ int cf_check nsvm_vcpu_reset(struct vcpu *v)
svm->ns_exception_intercepts = 0;
svm->ns_general1_intercepts = 0;
svm->ns_general2_intercepts = 0;
- svm->ns_virt_ext.bytes = 0;
+ svm->ns_virt_ext = 0;
svm->ns_hap_enabled = 0;
svm->ns_vmcb_guestcr3 = 0;
@@ -526,7 +526,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
/* LBR and other virtualization */
if ( !clean.lbr )
- svm->ns_virt_ext = ns_vmcb->virt_ext;
+ svm->ns_virt_ext = ns_vmcb->virt_ext.bytes;
n2vmcb->virt_ext.bytes =
n1vmcb->virt_ext.bytes | ns_vmcb->virt_ext.bytes;
diff --git a/xen/arch/x86/include/asm/hvm/svm/nestedsvm.h b/xen/arch/x86/include/asm/hvm/svm/nestedsvm.h
index 94d45d2e8d47..184248bbd7c5 100644
--- a/xen/arch/x86/include/asm/hvm/svm/nestedsvm.h
+++ b/xen/arch/x86/include/asm/hvm/svm/nestedsvm.h
@@ -44,7 +44,7 @@ struct nestedsvm {
uint32_t ns_general2_intercepts;
/* Cached real lbr and other virtual extentions of the l2 guest */
- virt_ext_t ns_virt_ext;
+ uint64_t ns_virt_ext;
/* Cached real MSR permission bitmaps of the l2 guest */
unsigned long *ns_cached_msrpm;
--
2.30.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v3 05.5/14] x86/svm: Decouple types in struct nestedsvm
2023-02-24 21:06 ` [PATCH v3 05.5/14] x86/svm: Decouple types in struct nestedsvm Andrew Cooper
@ 2023-02-27 8:52 ` Xenia Ragiadakou
2023-02-27 10:47 ` Andrew Cooper
0 siblings, 1 reply; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-27 8:52 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich
On 2/24/23 23:06, Andrew Cooper wrote:
> struct nestedvm uses mostly plain integer types, except for virt_ext_t which
> is a union wrapping two bitfield names. But the nested virt logic only ever
> deals with it as full opaque register.
>
> Switch it to being a plain uint64_t, allowing us to hide even more of the SVM
> internal infrastructure.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Xenia Ragiadakou <burzalodowa@gmail.com>
--
Xenia
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v3 05.5/14] x86/svm: Decouple types in struct nestedsvm
2023-02-27 8:52 ` Xenia Ragiadakou
@ 2023-02-27 10:47 ` Andrew Cooper
0 siblings, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2023-02-27 10:47 UTC (permalink / raw)
To: Xenia Ragiadakou, Xen-devel; +Cc: Jan Beulich
On 27/02/2023 8:52 am, Xenia Ragiadakou wrote:
>
> On 2/24/23 23:06, Andrew Cooper wrote:
>> struct nestedvm uses mostly plain integer types, except for
>> virt_ext_t which
>> is a union wrapping two bitfield names. But the nested virt logic
>> only ever
>> deals with it as full opaque register.
>>
>> Switch it to being a plain uint64_t, allowing us to hide even more of
>> the SVM
>> internal infrastructure.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Reviewed-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>
On second thoughts, the fact that this patch compiles means its a
write-only variable.
Lemme experiment quickly with an alternate patch.
~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v3 06/14] x86/svm: move vmcb declarations used only by svm code to private header
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (4 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 05/14] x86/svm: move nestedsvm declarations used only by svm code to private header Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-24 18:50 ` [PATCH v3 07/14] x86/svm: move svmdebug.h declarations to private vmcb.h and delete it Xenia Ragiadakou
` (8 subsequent siblings)
14 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel; +Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu
Create a new private header in arch/x86/hvm/svm called vmcb.h and move
there all definitions and declarations that are used only by svm code and
don't need to reside in an external header.
Take the opportunity to replace u* with uint*_t and remove redundant blank
lines.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- new patch
xen/arch/x86/hvm/svm/asid.c | 1 +
xen/arch/x86/hvm/svm/emulate.c | 1 +
xen/arch/x86/hvm/svm/intr.c | 1 +
xen/arch/x86/hvm/svm/nestedsvm.c | 1 +
xen/arch/x86/hvm/svm/svm.c | 1 +
xen/arch/x86/hvm/svm/svmdebug.c | 2 +
xen/arch/x86/hvm/svm/vmcb.c | 2 +
xen/arch/x86/hvm/svm/vmcb.h | 591 ++++++++++++++++++++++++
xen/arch/x86/include/asm/hvm/svm/vmcb.h | 575 +----------------------
9 files changed, 604 insertions(+), 571 deletions(-)
create mode 100644 xen/arch/x86/hvm/svm/vmcb.h
diff --git a/xen/arch/x86/hvm/svm/asid.c b/xen/arch/x86/hvm/svm/asid.c
index 1128434878..05ba2df9d5 100644
--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -20,6 +20,7 @@
#include <asm/hvm/svm/svm.h>
#include "asid.h"
+#include "vmcb.h"
void svm_asid_init(const struct cpuinfo_x86 *c)
{
diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c
index 4a84b4e761..573d005127 100644
--- a/xen/arch/x86/hvm/svm/emulate.c
+++ b/xen/arch/x86/hvm/svm/emulate.c
@@ -26,6 +26,7 @@
#include <asm/hvm/svm/vmcb.h>
#include "emulate.h"
+#include "vmcb.h"
static unsigned long svm_nextrip_insn_length(struct vcpu *v)
{
diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index dbb0022190..2655c5b4c8 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -38,6 +38,7 @@
#include <asm/hvm/trace.h>
#include "nestedhvm.h"
+#include "vmcb.h"
static void svm_inject_nmi(struct vcpu *v)
{
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 80b72b5dee..efbd9bbb77 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -28,6 +28,7 @@
#include "emulate.h"
#include "nestedhvm.h"
#include "svm.h"
+#include "vmcb.h"
#define NSVM_ERROR_VVMCB 1
#define NSVM_ERROR_VMENTRY 2
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 4b74ee3d7c..86b1bf3242 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -56,6 +56,7 @@
#include "emulate.h"
#include "nestedhvm.h"
#include "svm.h"
+#include "vmcb.h"
void noreturn svm_asm_do_resume(void);
diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
index bce86f0ef7..ade74dfd8f 100644
--- a/xen/arch/x86/hvm/svm/svmdebug.c
+++ b/xen/arch/x86/hvm/svm/svmdebug.c
@@ -21,6 +21,8 @@
#include <asm/msr-index.h>
#include <asm/hvm/svm/svmdebug.h>
+#include "vmcb.h"
+
static void svm_dump_sel(const char *name, const struct segment_register *s)
{
printk("%s: %04x %04x %08x %016"PRIx64"\n",
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index ba93375e87..1d512fedb0 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -30,6 +30,8 @@
#include <asm/hvm/svm/svmdebug.h>
#include <asm/spec_ctrl.h>
+#include "vmcb.h"
+
struct vmcb_struct *alloc_vmcb(void)
{
struct vmcb_struct *vmcb;
diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
new file mode 100644
index 0000000000..c58625fd80
--- /dev/null
+++ b/xen/arch/x86/hvm/svm/vmcb.h
@@ -0,0 +1,591 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * vmcb.h: VMCB related definitions
+ *
+ * Copyright (c) 2005-2007, Advanced Micro Devices, Inc
+ * Copyright (c) 2004, Intel Corporation.
+ */
+
+#ifndef __X86_HVM_SVM_VMCB_PRIV_H__
+#define __X86_HVM_SVM_VMCB_PRIV_H__
+
+#include <xen/types.h>
+
+#include <asm/x86_emulate.h>
+
+/* general 1 intercepts */
+enum GenericIntercept1bits
+{
+ GENERAL1_INTERCEPT_INTR = 1 << 0,
+ GENERAL1_INTERCEPT_NMI = 1 << 1,
+ GENERAL1_INTERCEPT_SMI = 1 << 2,
+ GENERAL1_INTERCEPT_INIT = 1 << 3,
+ GENERAL1_INTERCEPT_VINTR = 1 << 4,
+ GENERAL1_INTERCEPT_CR0_SEL_WRITE = 1 << 5,
+ GENERAL1_INTERCEPT_IDTR_READ = 1 << 6,
+ GENERAL1_INTERCEPT_GDTR_READ = 1 << 7,
+ GENERAL1_INTERCEPT_LDTR_READ = 1 << 8,
+ GENERAL1_INTERCEPT_TR_READ = 1 << 9,
+ GENERAL1_INTERCEPT_IDTR_WRITE = 1 << 10,
+ GENERAL1_INTERCEPT_GDTR_WRITE = 1 << 11,
+ GENERAL1_INTERCEPT_LDTR_WRITE = 1 << 12,
+ GENERAL1_INTERCEPT_TR_WRITE = 1 << 13,
+ GENERAL1_INTERCEPT_RDTSC = 1 << 14,
+ GENERAL1_INTERCEPT_RDPMC = 1 << 15,
+ GENERAL1_INTERCEPT_PUSHF = 1 << 16,
+ GENERAL1_INTERCEPT_POPF = 1 << 17,
+ GENERAL1_INTERCEPT_CPUID = 1 << 18,
+ GENERAL1_INTERCEPT_RSM = 1 << 19,
+ GENERAL1_INTERCEPT_IRET = 1 << 20,
+ GENERAL1_INTERCEPT_SWINT = 1 << 21,
+ GENERAL1_INTERCEPT_INVD = 1 << 22,
+ GENERAL1_INTERCEPT_PAUSE = 1 << 23,
+ GENERAL1_INTERCEPT_HLT = 1 << 24,
+ GENERAL1_INTERCEPT_INVLPG = 1 << 25,
+ GENERAL1_INTERCEPT_INVLPGA = 1 << 26,
+ GENERAL1_INTERCEPT_IOIO_PROT = 1 << 27,
+ GENERAL1_INTERCEPT_MSR_PROT = 1 << 28,
+ GENERAL1_INTERCEPT_TASK_SWITCH = 1 << 29,
+ GENERAL1_INTERCEPT_FERR_FREEZE = 1 << 30,
+ GENERAL1_INTERCEPT_SHUTDOWN_EVT = 1u << 31
+};
+
+/* general 2 intercepts */
+enum GenericIntercept2bits
+{
+ GENERAL2_INTERCEPT_VMRUN = 1 << 0,
+ GENERAL2_INTERCEPT_VMMCALL = 1 << 1,
+ GENERAL2_INTERCEPT_VMLOAD = 1 << 2,
+ GENERAL2_INTERCEPT_VMSAVE = 1 << 3,
+ GENERAL2_INTERCEPT_STGI = 1 << 4,
+ GENERAL2_INTERCEPT_CLGI = 1 << 5,
+ GENERAL2_INTERCEPT_SKINIT = 1 << 6,
+ GENERAL2_INTERCEPT_RDTSCP = 1 << 7,
+ GENERAL2_INTERCEPT_ICEBP = 1 << 8,
+ GENERAL2_INTERCEPT_WBINVD = 1 << 9,
+ GENERAL2_INTERCEPT_MONITOR = 1 << 10,
+ GENERAL2_INTERCEPT_MWAIT = 1 << 11,
+ GENERAL2_INTERCEPT_MWAIT_CONDITIONAL = 1 << 12,
+ GENERAL2_INTERCEPT_XSETBV = 1 << 13,
+ GENERAL2_INTERCEPT_RDPRU = 1 << 14,
+};
+
+/* control register intercepts */
+enum CRInterceptBits
+{
+ CR_INTERCEPT_CR0_READ = 1 << 0,
+ CR_INTERCEPT_CR1_READ = 1 << 1,
+ CR_INTERCEPT_CR2_READ = 1 << 2,
+ CR_INTERCEPT_CR3_READ = 1 << 3,
+ CR_INTERCEPT_CR4_READ = 1 << 4,
+ CR_INTERCEPT_CR5_READ = 1 << 5,
+ CR_INTERCEPT_CR6_READ = 1 << 6,
+ CR_INTERCEPT_CR7_READ = 1 << 7,
+ CR_INTERCEPT_CR8_READ = 1 << 8,
+ CR_INTERCEPT_CR9_READ = 1 << 9,
+ CR_INTERCEPT_CR10_READ = 1 << 10,
+ CR_INTERCEPT_CR11_READ = 1 << 11,
+ CR_INTERCEPT_CR12_READ = 1 << 12,
+ CR_INTERCEPT_CR13_READ = 1 << 13,
+ CR_INTERCEPT_CR14_READ = 1 << 14,
+ CR_INTERCEPT_CR15_READ = 1 << 15,
+ CR_INTERCEPT_CR0_WRITE = 1 << 16,
+ CR_INTERCEPT_CR1_WRITE = 1 << 17,
+ CR_INTERCEPT_CR2_WRITE = 1 << 18,
+ CR_INTERCEPT_CR3_WRITE = 1 << 19,
+ CR_INTERCEPT_CR4_WRITE = 1 << 20,
+ CR_INTERCEPT_CR5_WRITE = 1 << 21,
+ CR_INTERCEPT_CR6_WRITE = 1 << 22,
+ CR_INTERCEPT_CR7_WRITE = 1 << 23,
+ CR_INTERCEPT_CR8_WRITE = 1 << 24,
+ CR_INTERCEPT_CR9_WRITE = 1 << 25,
+ CR_INTERCEPT_CR10_WRITE = 1 << 26,
+ CR_INTERCEPT_CR11_WRITE = 1 << 27,
+ CR_INTERCEPT_CR12_WRITE = 1 << 28,
+ CR_INTERCEPT_CR13_WRITE = 1 << 29,
+ CR_INTERCEPT_CR14_WRITE = 1 << 30,
+ CR_INTERCEPT_CR15_WRITE = 1u << 31,
+};
+
+/* debug register intercepts */
+enum DRInterceptBits
+{
+ DR_INTERCEPT_DR0_READ = 1 << 0,
+ DR_INTERCEPT_DR1_READ = 1 << 1,
+ DR_INTERCEPT_DR2_READ = 1 << 2,
+ DR_INTERCEPT_DR3_READ = 1 << 3,
+ DR_INTERCEPT_DR4_READ = 1 << 4,
+ DR_INTERCEPT_DR5_READ = 1 << 5,
+ DR_INTERCEPT_DR6_READ = 1 << 6,
+ DR_INTERCEPT_DR7_READ = 1 << 7,
+ DR_INTERCEPT_DR8_READ = 1 << 8,
+ DR_INTERCEPT_DR9_READ = 1 << 9,
+ DR_INTERCEPT_DR10_READ = 1 << 10,
+ DR_INTERCEPT_DR11_READ = 1 << 11,
+ DR_INTERCEPT_DR12_READ = 1 << 12,
+ DR_INTERCEPT_DR13_READ = 1 << 13,
+ DR_INTERCEPT_DR14_READ = 1 << 14,
+ DR_INTERCEPT_DR15_READ = 1 << 15,
+ DR_INTERCEPT_DR0_WRITE = 1 << 16,
+ DR_INTERCEPT_DR1_WRITE = 1 << 17,
+ DR_INTERCEPT_DR2_WRITE = 1 << 18,
+ DR_INTERCEPT_DR3_WRITE = 1 << 19,
+ DR_INTERCEPT_DR4_WRITE = 1 << 20,
+ DR_INTERCEPT_DR5_WRITE = 1 << 21,
+ DR_INTERCEPT_DR6_WRITE = 1 << 22,
+ DR_INTERCEPT_DR7_WRITE = 1 << 23,
+ DR_INTERCEPT_DR8_WRITE = 1 << 24,
+ DR_INTERCEPT_DR9_WRITE = 1 << 25,
+ DR_INTERCEPT_DR10_WRITE = 1 << 26,
+ DR_INTERCEPT_DR11_WRITE = 1 << 27,
+ DR_INTERCEPT_DR12_WRITE = 1 << 28,
+ DR_INTERCEPT_DR13_WRITE = 1 << 29,
+ DR_INTERCEPT_DR14_WRITE = 1 << 30,
+ DR_INTERCEPT_DR15_WRITE = 1u << 31,
+};
+
+enum VMEXIT_EXITCODE
+{
+ /* control register read exitcodes */
+ VMEXIT_CR0_READ = 0, /* 0x0 */
+ VMEXIT_CR1_READ = 1, /* 0x1 */
+ VMEXIT_CR2_READ = 2, /* 0x2 */
+ VMEXIT_CR3_READ = 3, /* 0x3 */
+ VMEXIT_CR4_READ = 4, /* 0x4 */
+ VMEXIT_CR5_READ = 5, /* 0x5 */
+ VMEXIT_CR6_READ = 6, /* 0x6 */
+ VMEXIT_CR7_READ = 7, /* 0x7 */
+ VMEXIT_CR8_READ = 8, /* 0x8 */
+ VMEXIT_CR9_READ = 9, /* 0x9 */
+ VMEXIT_CR10_READ = 10, /* 0xa */
+ VMEXIT_CR11_READ = 11, /* 0xb */
+ VMEXIT_CR12_READ = 12, /* 0xc */
+ VMEXIT_CR13_READ = 13, /* 0xd */
+ VMEXIT_CR14_READ = 14, /* 0xe */
+ VMEXIT_CR15_READ = 15, /* 0xf */
+
+ /* control register write exitcodes */
+ VMEXIT_CR0_WRITE = 16, /* 0x10 */
+ VMEXIT_CR1_WRITE = 17, /* 0x11 */
+ VMEXIT_CR2_WRITE = 18, /* 0x12 */
+ VMEXIT_CR3_WRITE = 19, /* 0x13 */
+ VMEXIT_CR4_WRITE = 20, /* 0x14 */
+ VMEXIT_CR5_WRITE = 21, /* 0x15 */
+ VMEXIT_CR6_WRITE = 22, /* 0x16 */
+ VMEXIT_CR7_WRITE = 23, /* 0x17 */
+ VMEXIT_CR8_WRITE = 24, /* 0x18 */
+ VMEXIT_CR9_WRITE = 25, /* 0x19 */
+ VMEXIT_CR10_WRITE = 26, /* 0x1a */
+ VMEXIT_CR11_WRITE = 27, /* 0x1b */
+ VMEXIT_CR12_WRITE = 28, /* 0x1c */
+ VMEXIT_CR13_WRITE = 29, /* 0x1d */
+ VMEXIT_CR14_WRITE = 30, /* 0x1e */
+ VMEXIT_CR15_WRITE = 31, /* 0x1f */
+
+ /* debug register read exitcodes */
+ VMEXIT_DR0_READ = 32, /* 0x20 */
+ VMEXIT_DR1_READ = 33, /* 0x21 */
+ VMEXIT_DR2_READ = 34, /* 0x22 */
+ VMEXIT_DR3_READ = 35, /* 0x23 */
+ VMEXIT_DR4_READ = 36, /* 0x24 */
+ VMEXIT_DR5_READ = 37, /* 0x25 */
+ VMEXIT_DR6_READ = 38, /* 0x26 */
+ VMEXIT_DR7_READ = 39, /* 0x27 */
+ VMEXIT_DR8_READ = 40, /* 0x28 */
+ VMEXIT_DR9_READ = 41, /* 0x29 */
+ VMEXIT_DR10_READ = 42, /* 0x2a */
+ VMEXIT_DR11_READ = 43, /* 0x2b */
+ VMEXIT_DR12_READ = 44, /* 0x2c */
+ VMEXIT_DR13_READ = 45, /* 0x2d */
+ VMEXIT_DR14_READ = 46, /* 0x2e */
+ VMEXIT_DR15_READ = 47, /* 0x2f */
+
+ /* debug register write exitcodes */
+ VMEXIT_DR0_WRITE = 48, /* 0x30 */
+ VMEXIT_DR1_WRITE = 49, /* 0x31 */
+ VMEXIT_DR2_WRITE = 50, /* 0x32 */
+ VMEXIT_DR3_WRITE = 51, /* 0x33 */
+ VMEXIT_DR4_WRITE = 52, /* 0x34 */
+ VMEXIT_DR5_WRITE = 53, /* 0x35 */
+ VMEXIT_DR6_WRITE = 54, /* 0x36 */
+ VMEXIT_DR7_WRITE = 55, /* 0x37 */
+ VMEXIT_DR8_WRITE = 56, /* 0x38 */
+ VMEXIT_DR9_WRITE = 57, /* 0x39 */
+ VMEXIT_DR10_WRITE = 58, /* 0x3a */
+ VMEXIT_DR11_WRITE = 59, /* 0x3b */
+ VMEXIT_DR12_WRITE = 60, /* 0x3c */
+ VMEXIT_DR13_WRITE = 61, /* 0x3d */
+ VMEXIT_DR14_WRITE = 62, /* 0x3e */
+ VMEXIT_DR15_WRITE = 63, /* 0x3f */
+
+ /* processor exception exitcodes (VMEXIT_EXCP[0-31]) */
+ VMEXIT_EXCEPTION_DE = 64, /* 0x40, divide-by-zero-error */
+ VMEXIT_EXCEPTION_DB = 65, /* 0x41, debug */
+ VMEXIT_EXCEPTION_NMI = 66, /* 0x42, non-maskable-interrupt */
+ VMEXIT_EXCEPTION_BP = 67, /* 0x43, breakpoint */
+ VMEXIT_EXCEPTION_OF = 68, /* 0x44, overflow */
+ VMEXIT_EXCEPTION_BR = 69, /* 0x45, bound-range */
+ VMEXIT_EXCEPTION_UD = 70, /* 0x46, invalid-opcode*/
+ VMEXIT_EXCEPTION_NM = 71, /* 0x47, device-not-available */
+ VMEXIT_EXCEPTION_DF = 72, /* 0x48, double-fault */
+ VMEXIT_EXCEPTION_09 = 73, /* 0x49, unsupported (reserved) */
+ VMEXIT_EXCEPTION_TS = 74, /* 0x4a, invalid-tss */
+ VMEXIT_EXCEPTION_NP = 75, /* 0x4b, segment-not-present */
+ VMEXIT_EXCEPTION_SS = 76, /* 0x4c, stack */
+ VMEXIT_EXCEPTION_GP = 77, /* 0x4d, general-protection */
+ VMEXIT_EXCEPTION_PF = 78, /* 0x4e, page-fault */
+ VMEXIT_EXCEPTION_15 = 79, /* 0x4f, reserved */
+ VMEXIT_EXCEPTION_MF = 80, /* 0x50, x87 floating-point exception-pending */
+ VMEXIT_EXCEPTION_AC = 81, /* 0x51, alignment-check */
+ VMEXIT_EXCEPTION_MC = 82, /* 0x52, machine-check */
+ VMEXIT_EXCEPTION_XF = 83, /* 0x53, simd floating-point */
+/* VMEXIT_EXCEPTION_20 = 84, 0x54, #VE (Intel specific) */
+ VMEXIT_EXCEPTION_CP = 85, /* 0x55, controlflow protection */
+
+ /* exceptions 20-31 (exitcodes 84-95) are reserved */
+
+ /* ...and the rest of the #VMEXITs */
+ VMEXIT_INTR = 96, /* 0x60 */
+ VMEXIT_NMI = 97, /* 0x61 */
+ VMEXIT_SMI = 98, /* 0x62 */
+ VMEXIT_INIT = 99, /* 0x63 */
+ VMEXIT_VINTR = 100, /* 0x64 */
+ VMEXIT_CR0_SEL_WRITE = 101, /* 0x65 */
+ VMEXIT_IDTR_READ = 102, /* 0x66 */
+ VMEXIT_GDTR_READ = 103, /* 0x67 */
+ VMEXIT_LDTR_READ = 104, /* 0x68 */
+ VMEXIT_TR_READ = 105, /* 0x69 */
+ VMEXIT_IDTR_WRITE = 106, /* 0x6a */
+ VMEXIT_GDTR_WRITE = 107, /* 0x6b */
+ VMEXIT_LDTR_WRITE = 108, /* 0x6c */
+ VMEXIT_TR_WRITE = 109, /* 0x6d */
+ VMEXIT_RDTSC = 110, /* 0x6e */
+ VMEXIT_RDPMC = 111, /* 0x6f */
+ VMEXIT_PUSHF = 112, /* 0x70 */
+ VMEXIT_POPF = 113, /* 0x71 */
+ VMEXIT_CPUID = 114, /* 0x72 */
+ VMEXIT_RSM = 115, /* 0x73 */
+ VMEXIT_IRET = 116, /* 0x74 */
+ VMEXIT_SWINT = 117, /* 0x75 */
+ VMEXIT_INVD = 118, /* 0x76 */
+ VMEXIT_PAUSE = 119, /* 0x77 */
+ VMEXIT_HLT = 120, /* 0x78 */
+ VMEXIT_INVLPG = 121, /* 0x79 */
+ VMEXIT_INVLPGA = 122, /* 0x7a */
+ VMEXIT_IOIO = 123, /* 0x7b */
+ VMEXIT_MSR = 124, /* 0x7c */
+ VMEXIT_TASK_SWITCH = 125, /* 0x7d */
+ VMEXIT_FERR_FREEZE = 126, /* 0x7e */
+ VMEXIT_SHUTDOWN = 127, /* 0x7f */
+ VMEXIT_VMRUN = 128, /* 0x80 */
+ VMEXIT_VMMCALL = 129, /* 0x81 */
+ VMEXIT_VMLOAD = 130, /* 0x82 */
+ VMEXIT_VMSAVE = 131, /* 0x83 */
+ VMEXIT_STGI = 132, /* 0x84 */
+ VMEXIT_CLGI = 133, /* 0x85 */
+ VMEXIT_SKINIT = 134, /* 0x86 */
+ VMEXIT_RDTSCP = 135, /* 0x87 */
+ VMEXIT_ICEBP = 136, /* 0x88 */
+ VMEXIT_WBINVD = 137, /* 0x89 */
+ VMEXIT_MONITOR = 138, /* 0x8a */
+ VMEXIT_MWAIT = 139, /* 0x8b */
+ VMEXIT_MWAIT_CONDITIONAL= 140, /* 0x8c */
+ VMEXIT_XSETBV = 141, /* 0x8d */
+ VMEXIT_RDPRU = 142, /* 0x8e */
+ /* Remember to also update VMEXIT_NPF_PERFC! */
+ VMEXIT_NPF = 1024, /* 0x400, nested paging fault */
+ /* Remember to also update SVM_PERF_EXIT_REASON_SIZE! */
+ VMEXIT_INVALID = -1
+};
+
+enum
+{
+ /* Available on all SVM-capable hardware. */
+ TLB_CTRL_NO_FLUSH = 0,
+ TLB_CTRL_FLUSH_ALL = 1,
+
+ /* Available with the FlushByASID feature. */
+ TLB_CTRL_FLUSH_ASID = 3,
+ TLB_CTRL_FLUSH_ASID_NONGLOBAL = 7,
+};
+
+typedef union
+{
+ struct
+ {
+ uint8_t vector;
+ uint8_t type:3;
+ bool ev:1;
+ uint32_t resvd1:19;
+ bool v:1;
+ uint32_t ec;
+ };
+ uint64_t raw;
+} intinfo_t;
+
+typedef union {
+ struct {
+ bool intr_shadow: 1;
+ bool guest_intr_mask:1;
+ };
+ uint64_t raw;
+} intstat_t;
+
+typedef union
+{
+ uint64_t bytes;
+ struct
+ {
+ uint64_t tpr: 8;
+ uint64_t irq: 1;
+ uint64_t vgif: 1;
+ uint64_t rsvd0: 6;
+ uint64_t prio: 4;
+ uint64_t ign_tpr: 1;
+ uint64_t rsvd1: 3;
+ uint64_t intr_masking: 1;
+ uint64_t vgif_enable: 1;
+ uint64_t rsvd2: 6;
+ uint64_t vector: 8;
+ uint64_t rsvd3: 24;
+ } fields;
+} vintr_t;
+
+typedef union
+{
+ uint64_t bytes;
+ struct
+ {
+ uint64_t type: 1;
+ uint64_t rsv0: 1;
+ uint64_t str: 1;
+ uint64_t rep: 1;
+ uint64_t sz8: 1;
+ uint64_t sz16: 1;
+ uint64_t sz32: 1;
+ uint64_t rsv1: 9;
+ uint64_t port: 16;
+ } fields;
+} ioio_info_t;
+
+typedef union
+{
+ struct {
+ bool intercepts:1; /* 0: cr/dr/exception/general intercepts,
+ * pause_filter_count, tsc_offset */
+ bool iopm:1; /* 1: iopm_base_pa, msrpm_base_pa */
+ bool asid:1; /* 2: guest_asid */
+ bool tpr:1; /* 3: vintr */
+ bool np:1; /* 4: np_enable, h_cr3, g_pat */
+ bool cr:1; /* 5: cr0, cr3, cr4, efer */
+ bool dr:1; /* 6: dr6, dr7 */
+ bool dt:1; /* 7: gdtr, idtr */
+ bool seg:1; /* 8: cs, ds, es, ss, cpl */
+ bool cr2:1; /* 9: cr2 */
+ bool lbr:1; /* 10: debugctlmsr, last{branch,int}{to,from}ip */
+ bool :1;
+ bool cet:1; /* 12: msr_s_set, ssp, msr_isst */
+ };
+ uint32_t raw;
+} vmcbcleanbits_t;
+
+#define IOPM_SIZE (12 * 1024)
+#define MSRPM_SIZE (8 * 1024)
+
+struct vmcb_struct {
+ uint32_t _cr_intercepts; /* offset 0x00 - cleanbit 0 */
+ uint32_t _dr_intercepts; /* offset 0x04 - cleanbit 0 */
+ uint32_t _exception_intercepts; /* offset 0x08 - cleanbit 0 */
+ uint32_t _general1_intercepts; /* offset 0x0C - cleanbit 0 */
+ uint32_t _general2_intercepts; /* offset 0x10 - cleanbit 0 */
+ uint32_t res01[10];
+ uint16_t _pause_filter_thresh; /* offset 0x3C - cleanbit 0 */
+ uint16_t _pause_filter_count; /* offset 0x3E - cleanbit 0 */
+ uint64_t _iopm_base_pa; /* offset 0x40 - cleanbit 1 */
+ uint64_t _msrpm_base_pa; /* offset 0x48 - cleanbit 1 */
+ uint64_t _tsc_offset; /* offset 0x50 - cleanbit 0 */
+ uint32_t _guest_asid; /* offset 0x58 - cleanbit 2 */
+ uint8_t tlb_control; /* offset 0x5C - TLB_CTRL_* */
+ uint8_t res07[3];
+ vintr_t _vintr; /* offset 0x60 - cleanbit 3 */
+ intstat_t int_stat; /* offset 0x68 */
+ uint64_t exitcode; /* offset 0x70 */
+ union {
+ struct {
+ uint64_t exitinfo1; /* offset 0x78 */
+ uint64_t exitinfo2; /* offset 0x80 */
+ };
+ union {
+ struct {
+ uint16_t sel;
+ uint64_t :48;
+
+ uint32_t ec;
+ uint32_t :4;
+ bool iret:1;
+ uint32_t :1;
+ bool jmp:1;
+ uint32_t :5;
+ bool ev:1;
+ uint32_t :3;
+ bool rf:1;
+ } task_switch;
+ } ei;
+ };
+ intinfo_t exit_int_info; /* offset 0x88 */
+ union { /* offset 0x90 - cleanbit 4 */
+ struct {
+ bool _np_enable :1;
+ bool _sev_enable :1;
+ bool _sev_es_enable :1;
+ bool _gmet :1;
+ bool _np_sss :1;
+ bool _vte :1;
+ };
+ uint64_t _np_ctrl;
+ };
+ uint64_t res08[2];
+ intinfo_t event_inj; /* offset 0xA8 */
+ uint64_t _h_cr3; /* offset 0xB0 - cleanbit 4 */
+ virt_ext_t virt_ext; /* offset 0xB8 */
+ vmcbcleanbits_t cleanbits; /* offset 0xC0 */
+ uint32_t res09; /* offset 0xC4 */
+ uint64_t nextrip; /* offset 0xC8 */
+ uint8_t guest_ins_len; /* offset 0xD0 */
+ uint8_t guest_ins[15]; /* offset 0xD1 */
+ uint64_t res10a[100]; /* offset 0xE0 pad to save area */
+
+ union {
+ struct segment_register sreg[6];
+ struct {
+ struct segment_register es; /* offset 0x400 - cleanbit 8 */
+ struct segment_register cs; /* cleanbit 8 */
+ struct segment_register ss; /* cleanbit 8 */
+ struct segment_register ds; /* cleanbit 8 */
+ struct segment_register fs;
+ struct segment_register gs;
+ };
+ };
+ struct segment_register gdtr; /* cleanbit 7 */
+ struct segment_register ldtr;
+ struct segment_register idtr; /* cleanbit 7 */
+ struct segment_register tr;
+ uint64_t res10[5];
+ uint8_t res11[3];
+ uint8_t _cpl; /* cleanbit 8 */
+ uint32_t res12;
+ uint64_t _efer; /* offset 0x400 + 0xD0 - cleanbit 5 */
+ uint64_t res13[14];
+ uint64_t _cr4; /* offset 0x400 + 0x148 - cleanbit 5 */
+ uint64_t _cr3; /* cleanbit 5 */
+ uint64_t _cr0; /* cleanbit 5 */
+ uint64_t _dr7; /* cleanbit 6 */
+ uint64_t _dr6; /* cleanbit 6 */
+ uint64_t rflags;
+ uint64_t rip;
+ uint64_t res14[11];
+ uint64_t rsp;
+ uint64_t _msr_s_cet; /* offset 0x400 + 0x1E0 - cleanbit 12 */
+ uint64_t _ssp; /* offset 0x400 + 0x1E8 | */
+ uint64_t _msr_isst; /* offset 0x400 + 0x1F0 v */
+ uint64_t rax;
+ uint64_t star;
+ uint64_t lstar;
+ uint64_t cstar;
+ uint64_t sfmask;
+ uint64_t kerngsbase;
+ uint64_t sysenter_cs;
+ uint64_t sysenter_esp;
+ uint64_t sysenter_eip;
+ uint64_t _cr2; /* cleanbit 9 */
+ uint64_t res16[4];
+ uint64_t _g_pat; /* cleanbit 4 */
+ uint64_t _debugctlmsr; /* cleanbit 10 */
+ uint64_t _lastbranchfromip; /* cleanbit 10 */
+ uint64_t _lastbranchtoip; /* cleanbit 10 */
+ uint64_t _lastintfromip; /* cleanbit 10 */
+ uint64_t _lastinttoip; /* cleanbit 10 */
+ uint64_t res17[9];
+ uint64_t spec_ctrl;
+ uint64_t res18[291];
+};
+
+struct vmcb_struct *alloc_vmcb(void);
+void free_vmcb(struct vmcb_struct *vmcb);
+
+int svm_create_vmcb(struct vcpu *v);
+void svm_destroy_vmcb(struct vcpu *v);
+
+void setup_vmcb_dump(void);
+
+/*
+ * VMCB accessor functions.
+ */
+
+#define VMCB_ACCESSORS_(name, type, cleanbit) \
+static inline void \
+vmcb_set_ ## name(struct vmcb_struct *vmcb, \
+ type value) \
+{ \
+ vmcb->_ ## name = value; \
+ vmcb->cleanbits.cleanbit = false; \
+} \
+static inline type \
+vmcb_get_ ## name(const struct vmcb_struct *vmcb) \
+{ \
+ return vmcb->_ ## name; \
+}
+
+#define VMCB_ACCESSORS(name, cleanbit) \
+ VMCB_ACCESSORS_(name, typeof(alloc_vmcb()->_ ## name), cleanbit)
+
+VMCB_ACCESSORS(cr_intercepts, intercepts)
+VMCB_ACCESSORS(dr_intercepts, intercepts)
+VMCB_ACCESSORS(exception_intercepts, intercepts)
+VMCB_ACCESSORS(general1_intercepts, intercepts)
+VMCB_ACCESSORS(general2_intercepts, intercepts)
+VMCB_ACCESSORS(pause_filter_count, intercepts)
+VMCB_ACCESSORS(pause_filter_thresh, intercepts)
+VMCB_ACCESSORS(tsc_offset, intercepts)
+VMCB_ACCESSORS(iopm_base_pa, iopm)
+VMCB_ACCESSORS(msrpm_base_pa, iopm)
+VMCB_ACCESSORS(guest_asid, asid)
+VMCB_ACCESSORS(vintr, tpr)
+VMCB_ACCESSORS(np_ctrl, np)
+VMCB_ACCESSORS_(np_enable, bool, np)
+VMCB_ACCESSORS_(sev_enable, bool, np)
+VMCB_ACCESSORS_(sev_es_enable, bool, np)
+VMCB_ACCESSORS_(gmet, bool, np)
+VMCB_ACCESSORS_(vte, bool, np)
+VMCB_ACCESSORS(h_cr3, np)
+VMCB_ACCESSORS(g_pat, np)
+VMCB_ACCESSORS(cr0, cr)
+VMCB_ACCESSORS(cr3, cr)
+VMCB_ACCESSORS(cr4, cr)
+VMCB_ACCESSORS(efer, cr)
+VMCB_ACCESSORS(dr6, dr)
+VMCB_ACCESSORS(dr7, dr)
+VMCB_ACCESSORS(cpl, seg)
+VMCB_ACCESSORS(cr2, cr2)
+VMCB_ACCESSORS(debugctlmsr, lbr)
+VMCB_ACCESSORS(lastbranchfromip, lbr)
+VMCB_ACCESSORS(lastbranchtoip, lbr)
+VMCB_ACCESSORS(lastintfromip, lbr)
+VMCB_ACCESSORS(lastinttoip, lbr)
+VMCB_ACCESSORS(msr_s_cet, cet)
+VMCB_ACCESSORS(ssp, cet)
+VMCB_ACCESSORS(msr_isst, cet)
+
+#undef VMCB_ACCESSORS
+
+#endif /* __X86_HVM_SVM_VMCB_PRIV_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/include/asm/hvm/svm/vmcb.h b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
index e87728fa81..593badc651 100644
--- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
+++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
@@ -21,515 +21,16 @@
#include <xen/types.h>
-/* general 1 intercepts */
-enum GenericIntercept1bits
-{
- GENERAL1_INTERCEPT_INTR = 1 << 0,
- GENERAL1_INTERCEPT_NMI = 1 << 1,
- GENERAL1_INTERCEPT_SMI = 1 << 2,
- GENERAL1_INTERCEPT_INIT = 1 << 3,
- GENERAL1_INTERCEPT_VINTR = 1 << 4,
- GENERAL1_INTERCEPT_CR0_SEL_WRITE = 1 << 5,
- GENERAL1_INTERCEPT_IDTR_READ = 1 << 6,
- GENERAL1_INTERCEPT_GDTR_READ = 1 << 7,
- GENERAL1_INTERCEPT_LDTR_READ = 1 << 8,
- GENERAL1_INTERCEPT_TR_READ = 1 << 9,
- GENERAL1_INTERCEPT_IDTR_WRITE = 1 << 10,
- GENERAL1_INTERCEPT_GDTR_WRITE = 1 << 11,
- GENERAL1_INTERCEPT_LDTR_WRITE = 1 << 12,
- GENERAL1_INTERCEPT_TR_WRITE = 1 << 13,
- GENERAL1_INTERCEPT_RDTSC = 1 << 14,
- GENERAL1_INTERCEPT_RDPMC = 1 << 15,
- GENERAL1_INTERCEPT_PUSHF = 1 << 16,
- GENERAL1_INTERCEPT_POPF = 1 << 17,
- GENERAL1_INTERCEPT_CPUID = 1 << 18,
- GENERAL1_INTERCEPT_RSM = 1 << 19,
- GENERAL1_INTERCEPT_IRET = 1 << 20,
- GENERAL1_INTERCEPT_SWINT = 1 << 21,
- GENERAL1_INTERCEPT_INVD = 1 << 22,
- GENERAL1_INTERCEPT_PAUSE = 1 << 23,
- GENERAL1_INTERCEPT_HLT = 1 << 24,
- GENERAL1_INTERCEPT_INVLPG = 1 << 25,
- GENERAL1_INTERCEPT_INVLPGA = 1 << 26,
- GENERAL1_INTERCEPT_IOIO_PROT = 1 << 27,
- GENERAL1_INTERCEPT_MSR_PROT = 1 << 28,
- GENERAL1_INTERCEPT_TASK_SWITCH = 1 << 29,
- GENERAL1_INTERCEPT_FERR_FREEZE = 1 << 30,
- GENERAL1_INTERCEPT_SHUTDOWN_EVT = 1u << 31
-};
-
-/* general 2 intercepts */
-enum GenericIntercept2bits
-{
- GENERAL2_INTERCEPT_VMRUN = 1 << 0,
- GENERAL2_INTERCEPT_VMMCALL = 1 << 1,
- GENERAL2_INTERCEPT_VMLOAD = 1 << 2,
- GENERAL2_INTERCEPT_VMSAVE = 1 << 3,
- GENERAL2_INTERCEPT_STGI = 1 << 4,
- GENERAL2_INTERCEPT_CLGI = 1 << 5,
- GENERAL2_INTERCEPT_SKINIT = 1 << 6,
- GENERAL2_INTERCEPT_RDTSCP = 1 << 7,
- GENERAL2_INTERCEPT_ICEBP = 1 << 8,
- GENERAL2_INTERCEPT_WBINVD = 1 << 9,
- GENERAL2_INTERCEPT_MONITOR = 1 << 10,
- GENERAL2_INTERCEPT_MWAIT = 1 << 11,
- GENERAL2_INTERCEPT_MWAIT_CONDITIONAL = 1 << 12,
- GENERAL2_INTERCEPT_XSETBV = 1 << 13,
- GENERAL2_INTERCEPT_RDPRU = 1 << 14,
-};
-
-
-/* control register intercepts */
-enum CRInterceptBits
-{
- CR_INTERCEPT_CR0_READ = 1 << 0,
- CR_INTERCEPT_CR1_READ = 1 << 1,
- CR_INTERCEPT_CR2_READ = 1 << 2,
- CR_INTERCEPT_CR3_READ = 1 << 3,
- CR_INTERCEPT_CR4_READ = 1 << 4,
- CR_INTERCEPT_CR5_READ = 1 << 5,
- CR_INTERCEPT_CR6_READ = 1 << 6,
- CR_INTERCEPT_CR7_READ = 1 << 7,
- CR_INTERCEPT_CR8_READ = 1 << 8,
- CR_INTERCEPT_CR9_READ = 1 << 9,
- CR_INTERCEPT_CR10_READ = 1 << 10,
- CR_INTERCEPT_CR11_READ = 1 << 11,
- CR_INTERCEPT_CR12_READ = 1 << 12,
- CR_INTERCEPT_CR13_READ = 1 << 13,
- CR_INTERCEPT_CR14_READ = 1 << 14,
- CR_INTERCEPT_CR15_READ = 1 << 15,
- CR_INTERCEPT_CR0_WRITE = 1 << 16,
- CR_INTERCEPT_CR1_WRITE = 1 << 17,
- CR_INTERCEPT_CR2_WRITE = 1 << 18,
- CR_INTERCEPT_CR3_WRITE = 1 << 19,
- CR_INTERCEPT_CR4_WRITE = 1 << 20,
- CR_INTERCEPT_CR5_WRITE = 1 << 21,
- CR_INTERCEPT_CR6_WRITE = 1 << 22,
- CR_INTERCEPT_CR7_WRITE = 1 << 23,
- CR_INTERCEPT_CR8_WRITE = 1 << 24,
- CR_INTERCEPT_CR9_WRITE = 1 << 25,
- CR_INTERCEPT_CR10_WRITE = 1 << 26,
- CR_INTERCEPT_CR11_WRITE = 1 << 27,
- CR_INTERCEPT_CR12_WRITE = 1 << 28,
- CR_INTERCEPT_CR13_WRITE = 1 << 29,
- CR_INTERCEPT_CR14_WRITE = 1 << 30,
- CR_INTERCEPT_CR15_WRITE = 1u << 31,
-};
-
-
-/* debug register intercepts */
-enum DRInterceptBits
-{
- DR_INTERCEPT_DR0_READ = 1 << 0,
- DR_INTERCEPT_DR1_READ = 1 << 1,
- DR_INTERCEPT_DR2_READ = 1 << 2,
- DR_INTERCEPT_DR3_READ = 1 << 3,
- DR_INTERCEPT_DR4_READ = 1 << 4,
- DR_INTERCEPT_DR5_READ = 1 << 5,
- DR_INTERCEPT_DR6_READ = 1 << 6,
- DR_INTERCEPT_DR7_READ = 1 << 7,
- DR_INTERCEPT_DR8_READ = 1 << 8,
- DR_INTERCEPT_DR9_READ = 1 << 9,
- DR_INTERCEPT_DR10_READ = 1 << 10,
- DR_INTERCEPT_DR11_READ = 1 << 11,
- DR_INTERCEPT_DR12_READ = 1 << 12,
- DR_INTERCEPT_DR13_READ = 1 << 13,
- DR_INTERCEPT_DR14_READ = 1 << 14,
- DR_INTERCEPT_DR15_READ = 1 << 15,
- DR_INTERCEPT_DR0_WRITE = 1 << 16,
- DR_INTERCEPT_DR1_WRITE = 1 << 17,
- DR_INTERCEPT_DR2_WRITE = 1 << 18,
- DR_INTERCEPT_DR3_WRITE = 1 << 19,
- DR_INTERCEPT_DR4_WRITE = 1 << 20,
- DR_INTERCEPT_DR5_WRITE = 1 << 21,
- DR_INTERCEPT_DR6_WRITE = 1 << 22,
- DR_INTERCEPT_DR7_WRITE = 1 << 23,
- DR_INTERCEPT_DR8_WRITE = 1 << 24,
- DR_INTERCEPT_DR9_WRITE = 1 << 25,
- DR_INTERCEPT_DR10_WRITE = 1 << 26,
- DR_INTERCEPT_DR11_WRITE = 1 << 27,
- DR_INTERCEPT_DR12_WRITE = 1 << 28,
- DR_INTERCEPT_DR13_WRITE = 1 << 29,
- DR_INTERCEPT_DR14_WRITE = 1 << 30,
- DR_INTERCEPT_DR15_WRITE = 1u << 31,
-};
-
-enum VMEXIT_EXITCODE
-{
- /* control register read exitcodes */
- VMEXIT_CR0_READ = 0, /* 0x0 */
- VMEXIT_CR1_READ = 1, /* 0x1 */
- VMEXIT_CR2_READ = 2, /* 0x2 */
- VMEXIT_CR3_READ = 3, /* 0x3 */
- VMEXIT_CR4_READ = 4, /* 0x4 */
- VMEXIT_CR5_READ = 5, /* 0x5 */
- VMEXIT_CR6_READ = 6, /* 0x6 */
- VMEXIT_CR7_READ = 7, /* 0x7 */
- VMEXIT_CR8_READ = 8, /* 0x8 */
- VMEXIT_CR9_READ = 9, /* 0x9 */
- VMEXIT_CR10_READ = 10, /* 0xa */
- VMEXIT_CR11_READ = 11, /* 0xb */
- VMEXIT_CR12_READ = 12, /* 0xc */
- VMEXIT_CR13_READ = 13, /* 0xd */
- VMEXIT_CR14_READ = 14, /* 0xe */
- VMEXIT_CR15_READ = 15, /* 0xf */
-
- /* control register write exitcodes */
- VMEXIT_CR0_WRITE = 16, /* 0x10 */
- VMEXIT_CR1_WRITE = 17, /* 0x11 */
- VMEXIT_CR2_WRITE = 18, /* 0x12 */
- VMEXIT_CR3_WRITE = 19, /* 0x13 */
- VMEXIT_CR4_WRITE = 20, /* 0x14 */
- VMEXIT_CR5_WRITE = 21, /* 0x15 */
- VMEXIT_CR6_WRITE = 22, /* 0x16 */
- VMEXIT_CR7_WRITE = 23, /* 0x17 */
- VMEXIT_CR8_WRITE = 24, /* 0x18 */
- VMEXIT_CR9_WRITE = 25, /* 0x19 */
- VMEXIT_CR10_WRITE = 26, /* 0x1a */
- VMEXIT_CR11_WRITE = 27, /* 0x1b */
- VMEXIT_CR12_WRITE = 28, /* 0x1c */
- VMEXIT_CR13_WRITE = 29, /* 0x1d */
- VMEXIT_CR14_WRITE = 30, /* 0x1e */
- VMEXIT_CR15_WRITE = 31, /* 0x1f */
-
- /* debug register read exitcodes */
- VMEXIT_DR0_READ = 32, /* 0x20 */
- VMEXIT_DR1_READ = 33, /* 0x21 */
- VMEXIT_DR2_READ = 34, /* 0x22 */
- VMEXIT_DR3_READ = 35, /* 0x23 */
- VMEXIT_DR4_READ = 36, /* 0x24 */
- VMEXIT_DR5_READ = 37, /* 0x25 */
- VMEXIT_DR6_READ = 38, /* 0x26 */
- VMEXIT_DR7_READ = 39, /* 0x27 */
- VMEXIT_DR8_READ = 40, /* 0x28 */
- VMEXIT_DR9_READ = 41, /* 0x29 */
- VMEXIT_DR10_READ = 42, /* 0x2a */
- VMEXIT_DR11_READ = 43, /* 0x2b */
- VMEXIT_DR12_READ = 44, /* 0x2c */
- VMEXIT_DR13_READ = 45, /* 0x2d */
- VMEXIT_DR14_READ = 46, /* 0x2e */
- VMEXIT_DR15_READ = 47, /* 0x2f */
-
- /* debug register write exitcodes */
- VMEXIT_DR0_WRITE = 48, /* 0x30 */
- VMEXIT_DR1_WRITE = 49, /* 0x31 */
- VMEXIT_DR2_WRITE = 50, /* 0x32 */
- VMEXIT_DR3_WRITE = 51, /* 0x33 */
- VMEXIT_DR4_WRITE = 52, /* 0x34 */
- VMEXIT_DR5_WRITE = 53, /* 0x35 */
- VMEXIT_DR6_WRITE = 54, /* 0x36 */
- VMEXIT_DR7_WRITE = 55, /* 0x37 */
- VMEXIT_DR8_WRITE = 56, /* 0x38 */
- VMEXIT_DR9_WRITE = 57, /* 0x39 */
- VMEXIT_DR10_WRITE = 58, /* 0x3a */
- VMEXIT_DR11_WRITE = 59, /* 0x3b */
- VMEXIT_DR12_WRITE = 60, /* 0x3c */
- VMEXIT_DR13_WRITE = 61, /* 0x3d */
- VMEXIT_DR14_WRITE = 62, /* 0x3e */
- VMEXIT_DR15_WRITE = 63, /* 0x3f */
-
- /* processor exception exitcodes (VMEXIT_EXCP[0-31]) */
- VMEXIT_EXCEPTION_DE = 64, /* 0x40, divide-by-zero-error */
- VMEXIT_EXCEPTION_DB = 65, /* 0x41, debug */
- VMEXIT_EXCEPTION_NMI = 66, /* 0x42, non-maskable-interrupt */
- VMEXIT_EXCEPTION_BP = 67, /* 0x43, breakpoint */
- VMEXIT_EXCEPTION_OF = 68, /* 0x44, overflow */
- VMEXIT_EXCEPTION_BR = 69, /* 0x45, bound-range */
- VMEXIT_EXCEPTION_UD = 70, /* 0x46, invalid-opcode*/
- VMEXIT_EXCEPTION_NM = 71, /* 0x47, device-not-available */
- VMEXIT_EXCEPTION_DF = 72, /* 0x48, double-fault */
- VMEXIT_EXCEPTION_09 = 73, /* 0x49, unsupported (reserved) */
- VMEXIT_EXCEPTION_TS = 74, /* 0x4a, invalid-tss */
- VMEXIT_EXCEPTION_NP = 75, /* 0x4b, segment-not-present */
- VMEXIT_EXCEPTION_SS = 76, /* 0x4c, stack */
- VMEXIT_EXCEPTION_GP = 77, /* 0x4d, general-protection */
- VMEXIT_EXCEPTION_PF = 78, /* 0x4e, page-fault */
- VMEXIT_EXCEPTION_15 = 79, /* 0x4f, reserved */
- VMEXIT_EXCEPTION_MF = 80, /* 0x50, x87 floating-point exception-pending */
- VMEXIT_EXCEPTION_AC = 81, /* 0x51, alignment-check */
- VMEXIT_EXCEPTION_MC = 82, /* 0x52, machine-check */
- VMEXIT_EXCEPTION_XF = 83, /* 0x53, simd floating-point */
-/* VMEXIT_EXCEPTION_20 = 84, 0x54, #VE (Intel specific) */
- VMEXIT_EXCEPTION_CP = 85, /* 0x55, controlflow protection */
-
- /* exceptions 20-31 (exitcodes 84-95) are reserved */
-
- /* ...and the rest of the #VMEXITs */
- VMEXIT_INTR = 96, /* 0x60 */
- VMEXIT_NMI = 97, /* 0x61 */
- VMEXIT_SMI = 98, /* 0x62 */
- VMEXIT_INIT = 99, /* 0x63 */
- VMEXIT_VINTR = 100, /* 0x64 */
- VMEXIT_CR0_SEL_WRITE = 101, /* 0x65 */
- VMEXIT_IDTR_READ = 102, /* 0x66 */
- VMEXIT_GDTR_READ = 103, /* 0x67 */
- VMEXIT_LDTR_READ = 104, /* 0x68 */
- VMEXIT_TR_READ = 105, /* 0x69 */
- VMEXIT_IDTR_WRITE = 106, /* 0x6a */
- VMEXIT_GDTR_WRITE = 107, /* 0x6b */
- VMEXIT_LDTR_WRITE = 108, /* 0x6c */
- VMEXIT_TR_WRITE = 109, /* 0x6d */
- VMEXIT_RDTSC = 110, /* 0x6e */
- VMEXIT_RDPMC = 111, /* 0x6f */
- VMEXIT_PUSHF = 112, /* 0x70 */
- VMEXIT_POPF = 113, /* 0x71 */
- VMEXIT_CPUID = 114, /* 0x72 */
- VMEXIT_RSM = 115, /* 0x73 */
- VMEXIT_IRET = 116, /* 0x74 */
- VMEXIT_SWINT = 117, /* 0x75 */
- VMEXIT_INVD = 118, /* 0x76 */
- VMEXIT_PAUSE = 119, /* 0x77 */
- VMEXIT_HLT = 120, /* 0x78 */
- VMEXIT_INVLPG = 121, /* 0x79 */
- VMEXIT_INVLPGA = 122, /* 0x7a */
- VMEXIT_IOIO = 123, /* 0x7b */
- VMEXIT_MSR = 124, /* 0x7c */
- VMEXIT_TASK_SWITCH = 125, /* 0x7d */
- VMEXIT_FERR_FREEZE = 126, /* 0x7e */
- VMEXIT_SHUTDOWN = 127, /* 0x7f */
- VMEXIT_VMRUN = 128, /* 0x80 */
- VMEXIT_VMMCALL = 129, /* 0x81 */
- VMEXIT_VMLOAD = 130, /* 0x82 */
- VMEXIT_VMSAVE = 131, /* 0x83 */
- VMEXIT_STGI = 132, /* 0x84 */
- VMEXIT_CLGI = 133, /* 0x85 */
- VMEXIT_SKINIT = 134, /* 0x86 */
- VMEXIT_RDTSCP = 135, /* 0x87 */
- VMEXIT_ICEBP = 136, /* 0x88 */
- VMEXIT_WBINVD = 137, /* 0x89 */
- VMEXIT_MONITOR = 138, /* 0x8a */
- VMEXIT_MWAIT = 139, /* 0x8b */
- VMEXIT_MWAIT_CONDITIONAL= 140, /* 0x8c */
- VMEXIT_XSETBV = 141, /* 0x8d */
- VMEXIT_RDPRU = 142, /* 0x8e */
- /* Remember to also update VMEXIT_NPF_PERFC! */
- VMEXIT_NPF = 1024, /* 0x400, nested paging fault */
- /* Remember to also update SVM_PERF_EXIT_REASON_SIZE! */
- VMEXIT_INVALID = -1
-};
-
-enum
-{
- /* Available on all SVM-capable hardware. */
- TLB_CTRL_NO_FLUSH = 0,
- TLB_CTRL_FLUSH_ALL = 1,
-
- /* Available with the FlushByASID feature. */
- TLB_CTRL_FLUSH_ASID = 3,
- TLB_CTRL_FLUSH_ASID_NONGLOBAL = 7,
-};
-
typedef union
{
+ uint64_t bytes;
struct
{
- uint8_t vector;
- uint8_t type:3;
- bool ev:1;
- uint32_t resvd1:19;
- bool v:1;
- uint32_t ec;
- };
- uint64_t raw;
-} intinfo_t;
-
-typedef union {
- struct {
- bool intr_shadow: 1;
- bool guest_intr_mask:1;
- };
- uint64_t raw;
-} intstat_t;
-
-typedef union
-{
- u64 bytes;
- struct
- {
- u64 tpr: 8;
- u64 irq: 1;
- u64 vgif: 1;
- u64 rsvd0: 6;
- u64 prio: 4;
- u64 ign_tpr: 1;
- u64 rsvd1: 3;
- u64 intr_masking: 1;
- u64 vgif_enable: 1;
- u64 rsvd2: 6;
- u64 vector: 8;
- u64 rsvd3: 24;
- } fields;
-} vintr_t;
-
-typedef union
-{
- u64 bytes;
- struct
- {
- u64 type: 1;
- u64 rsv0: 1;
- u64 str: 1;
- u64 rep: 1;
- u64 sz8: 1;
- u64 sz16: 1;
- u64 sz32: 1;
- u64 rsv1: 9;
- u64 port: 16;
- } fields;
-} ioio_info_t;
-
-typedef union
-{
- u64 bytes;
- struct
- {
- u64 lbr_enable:1;
- u64 vloadsave_enable:1;
+ uint64_t lbr_enable:1;
+ uint64_t vloadsave_enable:1;
} fields;
} virt_ext_t;
-typedef union
-{
- struct {
- bool intercepts:1; /* 0: cr/dr/exception/general intercepts,
- * pause_filter_count, tsc_offset */
- bool iopm:1; /* 1: iopm_base_pa, msrpm_base_pa */
- bool asid:1; /* 2: guest_asid */
- bool tpr:1; /* 3: vintr */
- bool np:1; /* 4: np_enable, h_cr3, g_pat */
- bool cr:1; /* 5: cr0, cr3, cr4, efer */
- bool dr:1; /* 6: dr6, dr7 */
- bool dt:1; /* 7: gdtr, idtr */
- bool seg:1; /* 8: cs, ds, es, ss, cpl */
- bool cr2:1; /* 9: cr2 */
- bool lbr:1; /* 10: debugctlmsr, last{branch,int}{to,from}ip */
- bool :1;
- bool cet:1; /* 12: msr_s_set, ssp, msr_isst */
- };
- uint32_t raw;
-} vmcbcleanbits_t;
-
-#define IOPM_SIZE (12 * 1024)
-#define MSRPM_SIZE (8 * 1024)
-
-struct vmcb_struct {
- u32 _cr_intercepts; /* offset 0x00 - cleanbit 0 */
- u32 _dr_intercepts; /* offset 0x04 - cleanbit 0 */
- u32 _exception_intercepts; /* offset 0x08 - cleanbit 0 */
- u32 _general1_intercepts; /* offset 0x0C - cleanbit 0 */
- u32 _general2_intercepts; /* offset 0x10 - cleanbit 0 */
- u32 res01[10];
- u16 _pause_filter_thresh; /* offset 0x3C - cleanbit 0 */
- u16 _pause_filter_count; /* offset 0x3E - cleanbit 0 */
- u64 _iopm_base_pa; /* offset 0x40 - cleanbit 1 */
- u64 _msrpm_base_pa; /* offset 0x48 - cleanbit 1 */
- u64 _tsc_offset; /* offset 0x50 - cleanbit 0 */
- u32 _guest_asid; /* offset 0x58 - cleanbit 2 */
- u8 tlb_control; /* offset 0x5C - TLB_CTRL_* */
- u8 res07[3];
- vintr_t _vintr; /* offset 0x60 - cleanbit 3 */
- intstat_t int_stat; /* offset 0x68 */
- u64 exitcode; /* offset 0x70 */
- union {
- struct {
- uint64_t exitinfo1; /* offset 0x78 */
- uint64_t exitinfo2; /* offset 0x80 */
- };
- union {
- struct {
- uint16_t sel;
- uint64_t :48;
-
- uint32_t ec;
- uint32_t :4;
- bool iret:1;
- uint32_t :1;
- bool jmp:1;
- uint32_t :5;
- bool ev:1;
- uint32_t :3;
- bool rf:1;
- } task_switch;
- } ei;
- };
- intinfo_t exit_int_info; /* offset 0x88 */
- union { /* offset 0x90 - cleanbit 4 */
- struct {
- bool _np_enable :1;
- bool _sev_enable :1;
- bool _sev_es_enable :1;
- bool _gmet :1;
- bool _np_sss :1;
- bool _vte :1;
- };
- uint64_t _np_ctrl;
- };
- u64 res08[2];
- intinfo_t event_inj; /* offset 0xA8 */
- u64 _h_cr3; /* offset 0xB0 - cleanbit 4 */
- virt_ext_t virt_ext; /* offset 0xB8 */
- vmcbcleanbits_t cleanbits; /* offset 0xC0 */
- u32 res09; /* offset 0xC4 */
- u64 nextrip; /* offset 0xC8 */
- u8 guest_ins_len; /* offset 0xD0 */
- u8 guest_ins[15]; /* offset 0xD1 */
- u64 res10a[100]; /* offset 0xE0 pad to save area */
-
- union {
- struct segment_register sreg[6];
- struct {
- struct segment_register es; /* offset 0x400 - cleanbit 8 */
- struct segment_register cs; /* cleanbit 8 */
- struct segment_register ss; /* cleanbit 8 */
- struct segment_register ds; /* cleanbit 8 */
- struct segment_register fs;
- struct segment_register gs;
- };
- };
- struct segment_register gdtr; /* cleanbit 7 */
- struct segment_register ldtr;
- struct segment_register idtr; /* cleanbit 7 */
- struct segment_register tr;
- u64 res10[5];
- u8 res11[3];
- u8 _cpl; /* cleanbit 8 */
- u32 res12;
- u64 _efer; /* offset 0x400 + 0xD0 - cleanbit 5 */
- u64 res13[14];
- u64 _cr4; /* offset 0x400 + 0x148 - cleanbit 5 */
- u64 _cr3; /* cleanbit 5 */
- u64 _cr0; /* cleanbit 5 */
- u64 _dr7; /* cleanbit 6 */
- u64 _dr6; /* cleanbit 6 */
- u64 rflags;
- u64 rip;
- u64 res14[11];
- u64 rsp;
- u64 _msr_s_cet; /* offset 0x400 + 0x1E0 - cleanbit 12 */
- u64 _ssp; /* offset 0x400 + 0x1E8 | */
- u64 _msr_isst; /* offset 0x400 + 0x1F0 v */
- u64 rax;
- u64 star;
- u64 lstar;
- u64 cstar;
- u64 sfmask;
- u64 kerngsbase;
- u64 sysenter_cs;
- u64 sysenter_esp;
- u64 sysenter_eip;
- u64 _cr2; /* cleanbit 9 */
- u64 res16[4];
- u64 _g_pat; /* cleanbit 4 */
- u64 _debugctlmsr; /* cleanbit 10 */
- u64 _lastbranchfromip; /* cleanbit 10 */
- u64 _lastbranchtoip; /* cleanbit 10 */
- u64 _lastintfromip; /* cleanbit 10 */
- u64 _lastinttoip; /* cleanbit 10 */
- u64 res17[9];
- u64 spec_ctrl;
- u64 res18[291];
-};
-
struct svm_domain {
/* OSVW MSRs */
union {
@@ -558,7 +59,7 @@ enum vmcb_sync_state {
struct svm_vcpu {
struct vmcb_struct *vmcb;
- u64 vmcb_pa;
+ uint64_t vmcb_pa;
unsigned long *msrpm;
int launch_core;
@@ -577,14 +78,6 @@ struct svm_vcpu {
uint64_t guest_sysenter_eip;
};
-struct vmcb_struct *alloc_vmcb(void);
-void free_vmcb(struct vmcb_struct *vmcb);
-
-int svm_create_vmcb(struct vcpu *v);
-void svm_destroy_vmcb(struct vcpu *v);
-
-void setup_vmcb_dump(void);
-
#define MSR_INTERCEPT_NONE 0
#define MSR_INTERCEPT_READ 1
#define MSR_INTERCEPT_WRITE 2
@@ -593,66 +86,6 @@ void svm_intercept_msr(struct vcpu *v, uint32_t msr, int enable);
#define svm_disable_intercept_for_msr(v, msr) svm_intercept_msr((v), (msr), MSR_INTERCEPT_NONE)
#define svm_enable_intercept_for_msr(v, msr) svm_intercept_msr((v), (msr), MSR_INTERCEPT_RW)
-/*
- * VMCB accessor functions.
- */
-
-#define VMCB_ACCESSORS_(name, type, cleanbit) \
-static inline void \
-vmcb_set_ ## name(struct vmcb_struct *vmcb, \
- type value) \
-{ \
- vmcb->_ ## name = value; \
- vmcb->cleanbits.cleanbit = false; \
-} \
-static inline type \
-vmcb_get_ ## name(const struct vmcb_struct *vmcb) \
-{ \
- return vmcb->_ ## name; \
-}
-
-#define VMCB_ACCESSORS(name, cleanbit) \
- VMCB_ACCESSORS_(name, typeof(alloc_vmcb()->_ ## name), cleanbit)
-
-VMCB_ACCESSORS(cr_intercepts, intercepts)
-VMCB_ACCESSORS(dr_intercepts, intercepts)
-VMCB_ACCESSORS(exception_intercepts, intercepts)
-VMCB_ACCESSORS(general1_intercepts, intercepts)
-VMCB_ACCESSORS(general2_intercepts, intercepts)
-VMCB_ACCESSORS(pause_filter_count, intercepts)
-VMCB_ACCESSORS(pause_filter_thresh, intercepts)
-VMCB_ACCESSORS(tsc_offset, intercepts)
-VMCB_ACCESSORS(iopm_base_pa, iopm)
-VMCB_ACCESSORS(msrpm_base_pa, iopm)
-VMCB_ACCESSORS(guest_asid, asid)
-VMCB_ACCESSORS(vintr, tpr)
-VMCB_ACCESSORS(np_ctrl, np)
-VMCB_ACCESSORS_(np_enable, bool, np)
-VMCB_ACCESSORS_(sev_enable, bool, np)
-VMCB_ACCESSORS_(sev_es_enable, bool, np)
-VMCB_ACCESSORS_(gmet, bool, np)
-VMCB_ACCESSORS_(vte, bool, np)
-VMCB_ACCESSORS(h_cr3, np)
-VMCB_ACCESSORS(g_pat, np)
-VMCB_ACCESSORS(cr0, cr)
-VMCB_ACCESSORS(cr3, cr)
-VMCB_ACCESSORS(cr4, cr)
-VMCB_ACCESSORS(efer, cr)
-VMCB_ACCESSORS(dr6, dr)
-VMCB_ACCESSORS(dr7, dr)
-VMCB_ACCESSORS(cpl, seg)
-VMCB_ACCESSORS(cr2, cr2)
-VMCB_ACCESSORS(debugctlmsr, lbr)
-VMCB_ACCESSORS(lastbranchfromip, lbr)
-VMCB_ACCESSORS(lastbranchtoip, lbr)
-VMCB_ACCESSORS(lastintfromip, lbr)
-VMCB_ACCESSORS(lastinttoip, lbr)
-VMCB_ACCESSORS(msr_s_cet, cet)
-VMCB_ACCESSORS(ssp, cet)
-VMCB_ACCESSORS(msr_isst, cet)
-
-#undef VMCB_ACCESSORS
-
#endif /* ASM_X86_HVM_SVM_VMCS_H__ */
/*
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 07/14] x86/svm: move svmdebug.h declarations to private vmcb.h and delete it
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (5 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 06/14] x86/svm: move vmcb declarations used only by svm code to private header Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-24 18:50 ` [PATCH v3 08/14] x86/vmx: move vmx_update_debug_state() in vmcs.c and declare it static Xenia Ragiadakou
` (7 subsequent siblings)
14 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel; +Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu
Move the declarations in svmdebug.h to private vmcb.h because are vmcb
specific and are used only by internal svm code, and delete svmdebug.h.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- new patch, suggested by Andrew
xen/arch/x86/hvm/svm/nestedsvm.c | 1 -
xen/arch/x86/hvm/svm/svm.c | 1 -
xen/arch/x86/hvm/svm/svmdebug.c | 1 -
xen/arch/x86/hvm/svm/vmcb.c | 1 -
xen/arch/x86/hvm/svm/vmcb.h | 6 +++++
xen/arch/x86/include/asm/hvm/svm/svmdebug.h | 30 ---------------------
6 files changed, 6 insertions(+), 34 deletions(-)
delete mode 100644 xen/arch/x86/include/asm/hvm/svm/svmdebug.h
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index efbd9bbb77..201da7d531 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -20,7 +20,6 @@
#include <asm/hvm/svm/svm.h>
#include <asm/hvm/svm/vmcb.h>
#include <asm/hvm/nestedhvm.h>
-#include <asm/hvm/svm/svmdebug.h>
#include <asm/paging.h> /* paging_mode_hap */
#include <asm/event.h> /* for local_event_delivery_(en|dis)able */
#include <asm/p2m.h> /* p2m_get_pagetable, p2m_get_nestedp2m */
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 86b1bf3242..0a1b447e36 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -38,7 +38,6 @@
#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/support.h>
#include <asm/hvm/svm/svm.h>
-#include <asm/hvm/svm/svmdebug.h>
#include <asm/hvm/svm/vmcb.h>
#include <asm/hvm/trace.h>
#include <asm/iocap.h>
diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
index ade74dfd8f..7fd0753116 100644
--- a/xen/arch/x86/hvm/svm/svmdebug.c
+++ b/xen/arch/x86/hvm/svm/svmdebug.c
@@ -19,7 +19,6 @@
#include <xen/sched.h>
#include <asm/processor.h>
#include <asm/msr-index.h>
-#include <asm/hvm/svm/svmdebug.h>
#include "vmcb.h"
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 1d512fedb0..657b4b1670 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -27,7 +27,6 @@
#include <asm/msr-index.h>
#include <asm/p2m.h>
#include <asm/hvm/svm/svm.h>
-#include <asm/hvm/svm/svmdebug.h>
#include <asm/spec_ctrl.h>
#include "vmcb.h"
diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
index c58625fd80..80143164e5 100644
--- a/xen/arch/x86/hvm/svm/vmcb.h
+++ b/xen/arch/x86/hvm/svm/vmcb.h
@@ -11,6 +11,7 @@
#include <xen/types.h>
+#include <asm/hvm/svm/vmcb.h>
#include <asm/x86_emulate.h>
/* general 1 intercepts */
@@ -518,6 +519,11 @@ void svm_destroy_vmcb(struct vcpu *v);
void setup_vmcb_dump(void);
+void svm_sync_vmcb(struct vcpu *v, enum vmcb_sync_state new_state);
+void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb);
+bool svm_vmcb_isvalid(const char *from, const struct vmcb_struct *vmcb,
+ const struct vcpu *v, bool verbose);
+
/*
* VMCB accessor functions.
*/
diff --git a/xen/arch/x86/include/asm/hvm/svm/svmdebug.h b/xen/arch/x86/include/asm/hvm/svm/svmdebug.h
deleted file mode 100644
index 330c1d91aa..0000000000
--- a/xen/arch/x86/include/asm/hvm/svm/svmdebug.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * svmdebug.h: SVM related debug defintions
- * Copyright (c) 2011, AMD Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-#ifndef __ASM_X86_HVM_SVM_SVMDEBUG_H__
-#define __ASM_X86_HVM_SVM_SVMDEBUG_H__
-
-#include <asm/types.h>
-#include <asm/hvm/svm/vmcb.h>
-
-void svm_sync_vmcb(struct vcpu *v, enum vmcb_sync_state new_state);
-void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb);
-bool svm_vmcb_isvalid(const char *from, const struct vmcb_struct *vmcb,
- const struct vcpu *v, bool verbose);
-
-#endif /* __ASM_X86_HVM_SVM_SVMDEBUG_H__ */
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 08/14] x86/vmx: move vmx_update_debug_state() in vmcs.c and declare it static
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (6 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 07/14] x86/svm: move svmdebug.h declarations to private vmcb.h and delete it Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-27 15:59 ` Andrew Cooper
2023-02-24 18:50 ` [PATCH v3 09/14] x86/vmx: remove unused included headers from vmx.h Xenia Ragiadakou
` (6 subsequent siblings)
14 siblings, 1 reply; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel
Cc: Jun Nakajima, Kevin Tian, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu
Move vmx_update_debug_state() in vmcs.c because it is used only in this
file and limit its scope to this file by declaring it static and removing
its declaration from vmx.h.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- apply the change before moving the declarations into private headers
to avoid churn, suggested by Jan
xen/arch/x86/hvm/vmx/vmcs.c | 12 ++++++++++++
xen/arch/x86/hvm/vmx/vmx.c | 12 ------------
xen/arch/x86/include/asm/hvm/vmx/vmx.h | 1 -
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index ed71ecfb62..d3c75b3803 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1868,6 +1868,18 @@ void vmx_vmentry_failure(void)
void noreturn vmx_asm_do_vmentry(void);
+static void vmx_update_debug_state(struct vcpu *v)
+{
+ if ( v->arch.hvm.debug_state_latch )
+ v->arch.hvm.vmx.exception_bitmap |= 1U << TRAP_int3;
+ else
+ v->arch.hvm.vmx.exception_bitmap &= ~(1U << TRAP_int3);
+
+ vmx_vmcs_enter(v);
+ vmx_update_exception_bitmap(v);
+ vmx_vmcs_exit(v);
+}
+
void cf_check vmx_do_resume(void)
{
struct vcpu *v = current;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 0ec33bcc18..294c8490b4 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1613,18 +1613,6 @@ static void cf_check vmx_update_host_cr3(struct vcpu *v)
vmx_vmcs_exit(v);
}
-void vmx_update_debug_state(struct vcpu *v)
-{
- if ( v->arch.hvm.debug_state_latch )
- v->arch.hvm.vmx.exception_bitmap |= 1U << TRAP_int3;
- else
- v->arch.hvm.vmx.exception_bitmap &= ~(1U << TRAP_int3);
-
- vmx_vmcs_enter(v);
- vmx_update_exception_bitmap(v);
- vmx_vmcs_exit(v);
-}
-
static void cf_check vmx_update_guest_cr(
struct vcpu *v, unsigned int cr, unsigned int flags)
{
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmx.h b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
index f6308ed656..82a9487b40 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
@@ -88,7 +88,6 @@ void cf_check vmx_vlapic_msr_changed(struct vcpu *v);
struct hvm_emulate_ctxt;
void vmx_realmode_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt);
void vmx_realmode(struct cpu_user_regs *regs);
-void vmx_update_debug_state(struct vcpu *v);
void vmx_update_exception_bitmap(struct vcpu *v);
void vmx_update_cpu_exec_control(struct vcpu *v);
void vmx_update_secondary_exec_control(struct vcpu *v);
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 09/14] x86/vmx: remove unused included headers from vmx.h
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (7 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 08/14] x86/vmx: move vmx_update_debug_state() in vmcs.c and declare it static Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-24 18:50 ` [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers Xenia Ragiadakou
` (5 subsequent siblings)
14 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel
Cc: Jun Nakajima, Kevin Tian, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu
Do not include the headers:
asm/i387.h
asm/hvm/trace.h
asm/processor.h
asm/regs.h
because none of the declarations and macro definitions in them is used in
this file. Sort the rest of the headers alphabetically.
Include asm/i387.h in vmx.c, needed for vcpu_restore_fpu_lazy().
Take the opportunity to include xen/types.h in the place of asm/types.h and
replace u* with uint*_t and bool_t with bool.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- reword the commit message to not look like a bug fix, reported by JAn
- include xen/types.h instead of asm/types.h and use uint*_t instead of u*,
suggested by Jan
xen/arch/x86/hvm/vmx/vmx.c | 1 +
xen/arch/x86/include/asm/hvm/vmx/vmx.h | 15 ++++++---------
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 294c8490b4..232107bd79 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -43,6 +43,7 @@
#include <asm/hvm/vmx/vmcs.h>
#include <public/sched.h>
#include <public/hvm/ioreq.h>
+#include <asm/i387.h>
#include <asm/hvm/vpic.h>
#include <asm/hvm/vlapic.h>
#include <asm/x86_emulate.h>
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmx.h b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
index 82a9487b40..f5720c393c 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
@@ -19,20 +19,17 @@
#define __ASM_X86_HVM_VMX_VMX_H__
#include <xen/sched.h>
-#include <asm/types.h>
-#include <asm/regs.h>
+#include <xen/types.h>
+
#include <asm/asm_defns.h>
-#include <asm/processor.h>
-#include <asm/p2m.h>
-#include <asm/i387.h>
-#include <asm/hvm/trace.h>
#include <asm/hvm/vmx/vmcs.h>
+#include <asm/p2m.h>
extern int8_t opt_ept_exec_sp;
typedef union {
struct {
- u64 r : 1, /* bit 0 - Read permission */
+ uint64_t r : 1, /* bit 0 - Read permission */
w : 1, /* bit 1 - Write permission */
x : 1, /* bit 2 - Execute permission */
emt : 3, /* bits 5:3 - EPT Memory type */
@@ -49,7 +46,7 @@ typedef union {
_rsvd : 1, /* bit 62 - reserved */
suppress_ve : 1; /* bit 63 - suppress #VE */
};
- u64 epte;
+ uint64_t epte;
} ept_entry_t;
typedef struct {
@@ -595,7 +592,7 @@ void vmx_inject_extint(int trap, uint8_t source);
void vmx_inject_nmi(void);
void ept_walk_table(struct domain *d, unsigned long gfn);
-bool_t ept_handle_misconfig(uint64_t gpa);
+bool ept_handle_misconfig(uint64_t gpa);
int epte_get_entry_emt(struct domain *d, gfn_t gfn, mfn_t mfn,
unsigned int order, bool *ipat, p2m_type_t type);
void setup_ept_dump(void);
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (8 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 09/14] x86/vmx: remove unused included headers from vmx.h Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-27 15:25 ` Jan Beulich
2023-02-27 16:26 ` Andrew Cooper
2023-02-24 18:50 ` [PATCH v3 11/14] x86/vmx: remove unused included headers from vmx.c Xenia Ragiadakou
` (4 subsequent siblings)
14 siblings, 2 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel
Cc: Jun Nakajima, Kevin Tian, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu
Create two new private headers in arch/x86/hvm/vmx called vmx.h and pi.h.
Move all the definitions and declarations that are used solely by vmx code
into the private vmx.h, apart from the ones related to posted interrupts that
are moved into pi.h.
EPT related declarations and definitions stay in asm/hvm/vmx/vmx.h because
they are used in arch/x86/mm and drivers/passthrough/vtd.
Also, __vmread(), used in arch/x86/cpu, and consequently the opcodes stay in
asm/hvm/vmx/vmx.h.
Take the opportunity during the movement to replace u* with uint*_t, fix minor
coding style issues and reduce scope of GAS_VMX_OP definition.
Also, rearrange the code in the following way, place all structures first,
then all variable decalarations, all function delarations, and finally all
inline functions.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- add SPDX identifier, reported by Andrew
- add #ifndef header guard, reported by Andrew and Jan
- fold patch reducing the scope of GAS_VMX_OP definition into this,
suggested by Jan
- put pi related declarations in a separate priv header, suggested by Jan
- perform minor coding style fixes pointed out by Jan
- replace u* with uint*_t, suggested by Jan
- rearrange the header in the way Jan's proposed
- rebase to the latest staging
xen/arch/x86/hvm/vmx/intr.c | 2 +
xen/arch/x86/hvm/vmx/pi.h | 78 +++++
xen/arch/x86/hvm/vmx/realmode.c | 2 +
xen/arch/x86/hvm/vmx/vmcs.c | 2 +
xen/arch/x86/hvm/vmx/vmx.c | 3 +
xen/arch/x86/hvm/vmx/vmx.h | 416 +++++++++++++++++++++++
xen/arch/x86/hvm/vmx/vvmx.c | 2 +
xen/arch/x86/include/asm/hvm/vmx/vmx.h | 439 -------------------------
8 files changed, 505 insertions(+), 439 deletions(-)
create mode 100644 xen/arch/x86/hvm/vmx/pi.h
create mode 100644 xen/arch/x86/hvm/vmx/vmx.h
diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
index 6a8316de0e..c8db501759 100644
--- a/xen/arch/x86/hvm/vmx/intr.c
+++ b/xen/arch/x86/hvm/vmx/intr.c
@@ -38,6 +38,8 @@
#include <asm/hvm/trace.h>
#include <asm/vm_event.h>
+#include "vmx.h"
+
/*
* A few notes on virtual NMI and INTR delivery, and interactions with
* interruptibility states:
diff --git a/xen/arch/x86/hvm/vmx/pi.h b/xen/arch/x86/hvm/vmx/pi.h
new file mode 100644
index 0000000000..c72cc511da
--- /dev/null
+++ b/xen/arch/x86/hvm/vmx/pi.h
@@ -0,0 +1,78 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * pi.h: VMX Posted Interrupts
+ *
+ * Copyright (c) 2004, Intel Corporation.
+ */
+
+#ifndef __X86_HVM_VMX_PI_PRIV_H__
+#define __X86_HVM_VMX_PI_PRIV_H__
+
+#include <xen/types.h>
+
+#include <asm/hvm/vmx/vmcs.h>
+
+#define POSTED_INTR_ON 0
+#define POSTED_INTR_SN 1
+
+static inline int pi_test_and_set_pir(uint8_t vector, struct pi_desc *pi_desc)
+{
+ return test_and_set_bit(vector, pi_desc->pir);
+}
+
+static inline int pi_test_pir(uint8_t vector, const struct pi_desc *pi_desc)
+{
+ return test_bit(vector, pi_desc->pir);
+}
+
+static inline int pi_test_and_set_on(struct pi_desc *pi_desc)
+{
+ return test_and_set_bit(POSTED_INTR_ON, &pi_desc->control);
+}
+
+static inline void pi_set_on(struct pi_desc *pi_desc)
+{
+ set_bit(POSTED_INTR_ON, &pi_desc->control);
+}
+
+static inline int pi_test_and_clear_on(struct pi_desc *pi_desc)
+{
+ return test_and_clear_bit(POSTED_INTR_ON, &pi_desc->control);
+}
+
+static inline int pi_test_on(struct pi_desc *pi_desc)
+{
+ return pi_desc->on;
+}
+
+static inline unsigned long pi_get_pir(struct pi_desc *pi_desc, int group)
+{
+ return xchg(&pi_desc->pir[group], 0);
+}
+
+static inline int pi_test_sn(struct pi_desc *pi_desc)
+{
+ return pi_desc->sn;
+}
+
+static inline void pi_set_sn(struct pi_desc *pi_desc)
+{
+ set_bit(POSTED_INTR_SN, &pi_desc->control);
+}
+
+static inline void pi_clear_sn(struct pi_desc *pi_desc)
+{
+ clear_bit(POSTED_INTR_SN, &pi_desc->control);
+}
+
+#endif /* __X86_HVM_VMX_PI_PRIV_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/hvm/vmx/realmode.c b/xen/arch/x86/hvm/vmx/realmode.c
index 4ac93e0810..5591660230 100644
--- a/xen/arch/x86/hvm/vmx/realmode.c
+++ b/xen/arch/x86/hvm/vmx/realmode.c
@@ -23,6 +23,8 @@
#include <asm/hvm/vmx/vmx.h>
#include <asm/hvm/vmx/vmcs.h>
+#include "vmx.h"
+
static void realmode_deliver_exception(
unsigned int vector,
unsigned int insn_len,
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index d3c75b3803..4eb2571abb 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -43,6 +43,8 @@
#include <asm/tboot.h>
#include <asm/apic.h>
+#include "vmx.h"
+
static bool_t __read_mostly opt_vpid_enabled = 1;
boolean_param("vpid", opt_vpid_enabled);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 232107bd79..cb8b133ed5 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -62,6 +62,9 @@
#include <asm/prot-key.h>
#include <public/arch-x86/cpuid.h>
+#include "pi.h"
+#include "vmx.h"
+
static bool_t __initdata opt_force_ept;
boolean_param("force-ept", opt_force_ept);
diff --git a/xen/arch/x86/hvm/vmx/vmx.h b/xen/arch/x86/hvm/vmx/vmx.h
new file mode 100644
index 0000000000..7f8a00952b
--- /dev/null
+++ b/xen/arch/x86/hvm/vmx/vmx.h
@@ -0,0 +1,416 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * vmx.h: VMX Architecture related definitions
+ *
+ * Copyright (c) 2004, Intel Corporation.
+ */
+
+#ifndef __X86_HVM_VMX_VMX_PRIV_H__
+#define __X86_HVM_VMX_VMX_PRIV_H__
+
+#include <xen/sched.h>
+#include <xen/types.h>
+
+#include <asm/bug.h>
+#include <asm/asm_defns.h>
+#include <asm/hvm/vmx/vmcs.h>
+#include <asm/hvm/vmx/vmx.h>
+
+#define PI_xAPIC_NDST_MASK 0xFF00
+
+/*
+ * Interruption-information format
+ *
+ * Note INTR_INFO_NMI_UNBLOCKED_BY_IRET is also used with Exit Qualification
+ * field for EPT violations, PML full and SPP-related event vmexits.
+ */
+#define INTR_INFO_VECTOR_MASK 0xff /* 7:0 */
+#define INTR_INFO_INTR_TYPE_MASK 0x700 /* 10:8 */
+#define INTR_INFO_DELIVER_CODE_MASK 0x800 /* 11 */
+#define INTR_INFO_NMI_UNBLOCKED_BY_IRET 0x1000 /* 12 */
+#define INTR_INFO_VALID_MASK 0x80000000 /* 31 */
+#define INTR_INFO_RESVD_BITS_MASK 0x7ffff000
+
+/*
+ * Exit Qualifications for NOTIFY VM EXIT
+ */
+#define NOTIFY_VM_CONTEXT_INVALID 1u
+
+/*
+ * Exit Qualifications for MOV for Control Register Access
+ */
+enum {
+ VMX_CR_ACCESS_TYPE_MOV_TO_CR,
+ VMX_CR_ACCESS_TYPE_MOV_FROM_CR,
+ VMX_CR_ACCESS_TYPE_CLTS,
+ VMX_CR_ACCESS_TYPE_LMSW,
+};
+
+typedef union cr_access_qual {
+ unsigned long raw;
+ struct {
+ uint16_t cr:4,
+ access_type:2, /* VMX_CR_ACCESS_TYPE_* */
+ lmsw_op_type:1, /* 0 => reg, 1 => mem */
+ :1,
+ gpr:4,
+ :4;
+ uint16_t lmsw_data;
+ uint32_t :32;
+ };
+} __transparent__ cr_access_qual_t;
+
+/*
+ * Access Rights
+ */
+#define X86_SEG_AR_SEG_TYPE 0xf /* 3:0, segment type */
+#define X86_SEG_AR_DESC_TYPE (1u << 4) /* 4, descriptor type */
+#define X86_SEG_AR_DPL 0x60 /* 6:5, descriptor privilege level */
+#define X86_SEG_AR_SEG_PRESENT (1u << 7) /* 7, segment present */
+#define X86_SEG_AR_AVL (1u << 12) /* 12, available for system software */
+#define X86_SEG_AR_CS_LM_ACTIVE (1u << 13) /* 13, long mode active (CS only) */
+#define X86_SEG_AR_DEF_OP_SIZE (1u << 14) /* 14, default operation size */
+#define X86_SEG_AR_GRANULARITY (1u << 15) /* 15, granularity */
+#define X86_SEG_AR_SEG_UNUSABLE (1u << 16) /* 16, segment unusable */
+
+#define APIC_INVALID_DEST 0xffffffff
+
+/* #VE information page */
+typedef struct {
+ uint32_t exit_reason;
+ uint32_t semaphore;
+ uint64_t exit_qualification;
+ uint64_t gla;
+ uint64_t gpa;
+ uint16_t eptp_index;
+} ve_info_t;
+
+/* VM-Exit instruction info for LIDT, LGDT, SIDT, SGDT */
+typedef union idt_or_gdt_instr_info {
+ unsigned long raw;
+ struct {
+ unsigned long scaling :2, /* bits 0:1 - Scaling */
+ :5, /* bits 6:2 - Undefined */
+ addr_size :3, /* bits 9:7 - Address size */
+ :1, /* bit 10 - Cleared to 0 */
+ operand_size :1, /* bit 11 - Operand size */
+ :3, /* bits 14:12 - Undefined */
+ segment_reg :3, /* bits 17:15 - Segment register */
+ index_reg :4, /* bits 21:18 - Index register */
+ index_reg_invalid :1, /* bit 22 - Index register invalid */
+ base_reg :4, /* bits 26:23 - Base register */
+ base_reg_invalid :1, /* bit 27 - Base register invalid */
+ instr_identity :1, /* bit 28 - 0:GDT, 1:IDT */
+ instr_write :1, /* bit 29 - 0:store, 1:load */
+ :34; /* bits 30:63 - Undefined */
+ };
+} idt_or_gdt_instr_info_t;
+
+/* VM-Exit instruction info for LLDT, LTR, SLDT, STR */
+typedef union ldt_or_tr_instr_info {
+ unsigned long raw;
+ struct {
+ unsigned long scaling :2, /* bits 0:1 - Scaling */
+ :1, /* bit 2 - Undefined */
+ reg1 :4, /* bits 6:3 - Reg1 */
+ addr_size :3, /* bits 9:7 - Address size */
+ mem_reg :1, /* bit 10 - Mem/Reg */
+ :4, /* bits 14:11 - Undefined */
+ segment_reg :3, /* bits 17:15 - Segment register */
+ index_reg :4, /* bits 21:18 - Index register */
+ index_reg_invalid :1, /* bit 22 - Index register invalid */
+ base_reg :4, /* bits 26:23 - Base register */
+ base_reg_invalid :1, /* bit 27 - Base register invalid */
+ instr_identity :1, /* bit 28 - 0:LDT, 1:TR */
+ instr_write :1, /* bit 29 - 0:store, 1:load */
+ :34; /* bits 31:63 - Undefined */
+ };
+} ldt_or_tr_instr_info_t;
+
+extern int8_t opt_ept_exec_sp;
+
+extern uint8_t posted_intr_vector;
+
+void vmx_intr_assist(void);
+void noreturn cf_check vmx_do_resume(void);
+void cf_check vmx_vlapic_msr_changed(struct vcpu *v);
+void vmx_realmode(struct cpu_user_regs *regs);
+void vmx_update_exception_bitmap(struct vcpu *v);
+void vmx_update_cpu_exec_control(struct vcpu *v);
+void vmx_update_secondary_exec_control(struct vcpu *v);
+
+int cf_check vmx_guest_x86_mode(struct vcpu *v);
+unsigned int vmx_get_cpl(void);
+void vmx_inject_extint(int trap, uint8_t source);
+void vmx_inject_nmi(void);
+
+void update_guest_eip(void);
+
+void vmx_pi_per_cpu_init(unsigned int cpu);
+void vmx_pi_desc_fixup(unsigned int cpu);
+
+void vmx_sync_exit_bitmap(struct vcpu *v);
+
+static always_inline void __vmptrld(uint64_t addr)
+{
+ asm volatile (
+#ifdef HAVE_AS_VMX
+ "vmptrld %0\n"
+#else
+ VMPTRLD_OPCODE MODRM_EAX_06
+#endif
+ /* CF==1 or ZF==1 --> BUG() */
+ UNLIKELY_START(be, vmptrld)
+ _ASM_BUGFRAME_TEXT(0)
+ UNLIKELY_END_SECTION
+ :
+#ifdef HAVE_AS_VMX
+ : "m" (addr),
+#else
+ : "a" (&addr),
+#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
+ : "memory" );
+}
+
+static always_inline void __vmpclear(uint64_t addr)
+{
+ asm volatile (
+#ifdef HAVE_AS_VMX
+ "vmclear %0\n"
+#else
+ VMCLEAR_OPCODE MODRM_EAX_06
+#endif
+ /* CF==1 or ZF==1 --> BUG() */
+ UNLIKELY_START(be, vmclear)
+ _ASM_BUGFRAME_TEXT(0)
+ UNLIKELY_END_SECTION
+ :
+#ifdef HAVE_AS_VMX
+ : "m" (addr),
+#else
+ : "a" (&addr),
+#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
+ : "memory" );
+}
+
+static always_inline void __vmwrite(unsigned long field, unsigned long value)
+{
+ asm volatile (
+#ifdef HAVE_AS_VMX
+ "vmwrite %1, %0\n"
+#else
+ VMWRITE_OPCODE MODRM_EAX_ECX
+#endif
+ /* CF==1 or ZF==1 --> BUG() */
+ UNLIKELY_START(be, vmwrite)
+ _ASM_BUGFRAME_TEXT(0)
+ UNLIKELY_END_SECTION
+ :
+#ifdef HAVE_AS_VMX
+ : "r" (field) , "rm" (value),
+#else
+ : "a" (field) , "c" (value),
+#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
+ );
+}
+
+#ifdef HAVE_AS_VMX
+# define GAS_VMX_OP(yes, no) yes
+#else
+# define GAS_VMX_OP(yes, no) no
+#endif
+
+static inline enum vmx_insn_errno vmread_safe(unsigned long field,
+ unsigned long *value)
+{
+ unsigned long ret = VMX_INSN_SUCCEED;
+ bool fail_invalid, fail_valid;
+
+ asm volatile ( GAS_VMX_OP("vmread %[field], %[value]\n\t",
+ VMREAD_OPCODE MODRM_EAX_ECX)
+ ASM_FLAG_OUT(, "setc %[invalid]\n\t")
+ ASM_FLAG_OUT(, "setz %[valid]\n\t")
+ : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid),
+ ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid),
+ [value] GAS_VMX_OP("=rm", "=c") (*value)
+ : [field] GAS_VMX_OP("r", "a") (field) );
+
+ if ( unlikely(fail_invalid) )
+ ret = VMX_INSN_FAIL_INVALID;
+ else if ( unlikely(fail_valid) )
+ __vmread(VM_INSTRUCTION_ERROR, &ret);
+
+ return ret;
+}
+
+static inline enum vmx_insn_errno vmwrite_safe(unsigned long field,
+ unsigned long value)
+{
+ unsigned long ret = VMX_INSN_SUCCEED;
+ bool fail_invalid, fail_valid;
+
+ asm volatile ( GAS_VMX_OP("vmwrite %[value], %[field]\n\t",
+ VMWRITE_OPCODE MODRM_EAX_ECX)
+ ASM_FLAG_OUT(, "setc %[invalid]\n\t")
+ ASM_FLAG_OUT(, "setz %[valid]\n\t")
+ : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid),
+ ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid)
+ : [field] GAS_VMX_OP("r", "a") (field),
+ [value] GAS_VMX_OP("rm", "c") (value) );
+
+ if ( unlikely(fail_invalid) )
+ ret = VMX_INSN_FAIL_INVALID;
+ else if ( unlikely(fail_valid) )
+ __vmread(VM_INSTRUCTION_ERROR, &ret);
+
+ return ret;
+}
+
+#undef GAS_VMX_OP
+
+#define INVEPT_SINGLE_CONTEXT 1
+#define INVEPT_ALL_CONTEXT 2
+
+static always_inline void __invept(unsigned long type, uint64_t eptp)
+{
+ struct {
+ uint64_t eptp, rsvd;
+ } operand = { eptp };
+
+ /*
+ * If single context invalidation is not supported, we escalate to
+ * use all context invalidation.
+ */
+ if ( (type == INVEPT_SINGLE_CONTEXT) &&
+ !cpu_has_vmx_ept_invept_single_context )
+ type = INVEPT_ALL_CONTEXT;
+
+ asm volatile (
+#ifdef HAVE_AS_EPT
+ "invept %0, %1\n"
+#else
+ INVEPT_OPCODE MODRM_EAX_08
+#endif
+ /* CF==1 or ZF==1 --> BUG() */
+ UNLIKELY_START(be, invept)
+ _ASM_BUGFRAME_TEXT(0)
+ UNLIKELY_END_SECTION
+ :
+#ifdef HAVE_AS_EPT
+ : "m" (operand), "r" (type),
+#else
+ : "a" (&operand), "c" (type),
+#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
+ : "memory" );
+}
+
+#define INVVPID_INDIVIDUAL_ADDR 0
+#define INVVPID_SINGLE_CONTEXT 1
+#define INVVPID_ALL_CONTEXT 2
+#define INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL 3
+
+static always_inline void __invvpid(unsigned long type, uint16_t vpid,
+ uint64_t gva)
+{
+ struct __packed {
+ uint64_t vpid:16;
+ uint64_t rsvd:48;
+ uint64_t gva;
+ } operand = {vpid, 0, gva};
+
+ /* Fix up #UD exceptions which occur when TLBs are flushed before VMXON. */
+ asm volatile ( "1: "
+#ifdef HAVE_AS_EPT
+ "invvpid %0, %1\n"
+#else
+ INVVPID_OPCODE MODRM_EAX_08
+#endif
+ /* CF==1 or ZF==1 --> BUG() */
+ UNLIKELY_START(be, invvpid)
+ _ASM_BUGFRAME_TEXT(0)
+ UNLIKELY_END_SECTION "\n"
+ "2:"
+ _ASM_EXTABLE(1b, 2b)
+ :
+#ifdef HAVE_AS_EPT
+ : "m" (operand), "r" (type),
+#else
+ : "a" (&operand), "c" (type),
+#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
+ : "memory" );
+}
+
+static inline void ept_sync_all(void)
+{
+ __invept(INVEPT_ALL_CONTEXT, 0);
+}
+
+static inline void vpid_sync_vcpu_gva(struct vcpu *v, unsigned long gva)
+{
+ int type = INVVPID_INDIVIDUAL_ADDR;
+
+ /*
+ * If individual address invalidation is not supported, we escalate to
+ * use single context invalidation.
+ */
+ if ( likely(cpu_has_vmx_vpid_invvpid_individual_addr) )
+ goto execute_invvpid;
+
+ type = INVVPID_SINGLE_CONTEXT;
+
+ /*
+ * If single context invalidation is not supported, we escalate to
+ * use all context invalidation.
+ */
+ if ( !cpu_has_vmx_vpid_invvpid_single_context )
+ type = INVVPID_ALL_CONTEXT;
+
+ execute_invvpid:
+ __invvpid(type, v->arch.hvm.n1asid.asid, (uint64_t) gva);
+}
+
+static inline void vpid_sync_all(void)
+{
+ __invvpid(INVVPID_ALL_CONTEXT, 0, 0);
+}
+
+static inline void __vmxoff(void)
+{
+ asm volatile ( VMXOFF_OPCODE : : : "memory" );
+}
+
+static inline int __vmxon(uint64_t addr)
+{
+ int rc;
+
+ asm volatile (
+ "1: " VMXON_OPCODE MODRM_EAX_06 "\n"
+ " setna %b0 ; neg %0\n" /* CF==1 or ZF==1 --> rc = -1 */
+ "2:\n"
+ ".section .fixup,\"ax\"\n"
+ "3: sub $2,%0 ; jmp 2b\n" /* #UD or #GP --> rc = -2 */
+ ".previous\n"
+ _ASM_EXTABLE(1b, 3b)
+ : "=q" (rc)
+ : "0" (0), "a" (&addr)
+ : "memory" );
+
+ return rc;
+}
+
+#endif /* __X86_HVM_VMX_VMX_PRIV_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 674cdabb07..0bda8430b9 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -29,6 +29,8 @@
#include <asm/hvm/vmx/vvmx.h>
#include <asm/hvm/nestedhvm.h>
+#include "vmx.h"
+
static DEFINE_PER_CPU(u64 *, vvmcs_buf);
static void nvmx_purge_vvmcs(struct vcpu *v);
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmx.h b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
index f5720c393c..3b32001149 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
@@ -21,12 +21,8 @@
#include <xen/sched.h>
#include <xen/types.h>
-#include <asm/asm_defns.h>
-#include <asm/hvm/vmx/vmcs.h>
#include <asm/p2m.h>
-extern int8_t opt_ept_exec_sp;
-
typedef union {
struct {
uint64_t r : 1, /* bit 0 - Read permission */
@@ -77,69 +73,8 @@ typedef enum {
#define EPTE_RWX_MASK 0x7
#define EPTE_FLAG_MASK 0x7f
-#define PI_xAPIC_NDST_MASK 0xFF00
-
-void vmx_intr_assist(void);
-void noreturn cf_check vmx_do_resume(void);
-void cf_check vmx_vlapic_msr_changed(struct vcpu *v);
struct hvm_emulate_ctxt;
void vmx_realmode_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt);
-void vmx_realmode(struct cpu_user_regs *regs);
-void vmx_update_exception_bitmap(struct vcpu *v);
-void vmx_update_cpu_exec_control(struct vcpu *v);
-void vmx_update_secondary_exec_control(struct vcpu *v);
-
-#define POSTED_INTR_ON 0
-#define POSTED_INTR_SN 1
-static inline int pi_test_and_set_pir(uint8_t vector, struct pi_desc *pi_desc)
-{
- return test_and_set_bit(vector, pi_desc->pir);
-}
-
-static inline int pi_test_pir(uint8_t vector, const struct pi_desc *pi_desc)
-{
- return test_bit(vector, pi_desc->pir);
-}
-
-static inline int pi_test_and_set_on(struct pi_desc *pi_desc)
-{
- return test_and_set_bit(POSTED_INTR_ON, &pi_desc->control);
-}
-
-static inline void pi_set_on(struct pi_desc *pi_desc)
-{
- set_bit(POSTED_INTR_ON, &pi_desc->control);
-}
-
-static inline int pi_test_and_clear_on(struct pi_desc *pi_desc)
-{
- return test_and_clear_bit(POSTED_INTR_ON, &pi_desc->control);
-}
-
-static inline int pi_test_on(struct pi_desc *pi_desc)
-{
- return pi_desc->on;
-}
-
-static inline unsigned long pi_get_pir(struct pi_desc *pi_desc, int group)
-{
- return xchg(&pi_desc->pir[group], 0);
-}
-
-static inline int pi_test_sn(struct pi_desc *pi_desc)
-{
- return pi_desc->sn;
-}
-
-static inline void pi_set_sn(struct pi_desc *pi_desc)
-{
- set_bit(POSTED_INTR_SN, &pi_desc->control);
-}
-
-static inline void pi_clear_sn(struct pi_desc *pi_desc)
-{
- clear_bit(POSTED_INTR_SN, &pi_desc->control);
-}
/*
* Exit Reasons
@@ -210,60 +145,6 @@ static inline void pi_clear_sn(struct pi_desc *pi_desc)
#define EXIT_REASON_NOTIFY 75
/* Remember to also update VMX_PERF_EXIT_REASON_SIZE! */
-/*
- * Interruption-information format
- *
- * Note INTR_INFO_NMI_UNBLOCKED_BY_IRET is also used with Exit Qualification
- * field for EPT violations, PML full and SPP-related event vmexits.
- */
-#define INTR_INFO_VECTOR_MASK 0xff /* 7:0 */
-#define INTR_INFO_INTR_TYPE_MASK 0x700 /* 10:8 */
-#define INTR_INFO_DELIVER_CODE_MASK 0x800 /* 11 */
-#define INTR_INFO_NMI_UNBLOCKED_BY_IRET 0x1000 /* 12 */
-#define INTR_INFO_VALID_MASK 0x80000000 /* 31 */
-#define INTR_INFO_RESVD_BITS_MASK 0x7ffff000
-
-/*
- * Exit Qualifications for NOTIFY VM EXIT
- */
-#define NOTIFY_VM_CONTEXT_INVALID 1u
-
-/*
- * Exit Qualifications for MOV for Control Register Access
- */
-enum {
- VMX_CR_ACCESS_TYPE_MOV_TO_CR,
- VMX_CR_ACCESS_TYPE_MOV_FROM_CR,
- VMX_CR_ACCESS_TYPE_CLTS,
- VMX_CR_ACCESS_TYPE_LMSW,
-};
-typedef union cr_access_qual {
- unsigned long raw;
- struct {
- uint16_t cr:4,
- access_type:2, /* VMX_CR_ACCESS_TYPE_* */
- lmsw_op_type:1, /* 0 => reg, 1 => mem */
- :1,
- gpr:4,
- :4;
- uint16_t lmsw_data;
- uint32_t :32;
- };
-} __transparent__ cr_access_qual_t;
-
-/*
- * Access Rights
- */
-#define X86_SEG_AR_SEG_TYPE 0xf /* 3:0, segment type */
-#define X86_SEG_AR_DESC_TYPE (1u << 4) /* 4, descriptor type */
-#define X86_SEG_AR_DPL 0x60 /* 6:5, descriptor privilege level */
-#define X86_SEG_AR_SEG_PRESENT (1u << 7) /* 7, segment present */
-#define X86_SEG_AR_AVL (1u << 12) /* 12, available for system software */
-#define X86_SEG_AR_CS_LM_ACTIVE (1u << 13) /* 13, long mode active (CS only) */
-#define X86_SEG_AR_DEF_OP_SIZE (1u << 14) /* 14, default operation size */
-#define X86_SEG_AR_GRANULARITY (1u << 15) /* 15, granularity */
-#define X86_SEG_AR_SEG_UNUSABLE (1u << 16) /* 16, segment unusable */
-
#define VMCALL_OPCODE ".byte 0x0f,0x01,0xc1\n"
#define VMCLEAR_OPCODE ".byte 0x66,0x0f,0xc7\n" /* reg/opcode: /6 */
#define VMLAUNCH_OPCODE ".byte 0x0f,0x01,0xc2\n"
@@ -282,8 +163,6 @@ typedef union cr_access_qual {
#define MODRM_EAX_07 ".byte 0x38\n" /* [EAX], with reg/opcode: /7 */
#define MODRM_EAX_ECX ".byte 0xc1\n" /* EAX, ECX */
-extern uint8_t posted_intr_vector;
-
#define cpu_has_vmx_ept_exec_only_supported \
(vmx_ept_vpid_cap & VMX_EPT_EXEC_ONLY_SUPPORTED)
@@ -302,9 +181,6 @@ extern uint8_t posted_intr_vector;
#define ept_has_2mb(c) ((c >> EPT_2MB_SHIFT) & 1)
#define ept_has_1gb(c) ((c >> EPT_1GB_SHIFT) & 1)
-#define INVEPT_SINGLE_CONTEXT 1
-#define INVEPT_ALL_CONTEXT 2
-
#define cpu_has_vmx_vpid_invvpid_individual_addr \
(vmx_ept_vpid_cap & VMX_VPID_INVVPID_INDIVIDUAL_ADDR)
#define cpu_has_vmx_vpid_invvpid_single_context \
@@ -312,61 +188,6 @@ extern uint8_t posted_intr_vector;
#define cpu_has_vmx_vpid_invvpid_single_context_retaining_global \
(vmx_ept_vpid_cap & VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL)
-#define INVVPID_INDIVIDUAL_ADDR 0
-#define INVVPID_SINGLE_CONTEXT 1
-#define INVVPID_ALL_CONTEXT 2
-#define INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL 3
-
-#ifdef HAVE_AS_VMX
-# define GAS_VMX_OP(yes, no) yes
-#else
-# define GAS_VMX_OP(yes, no) no
-#endif
-
-static always_inline void __vmptrld(u64 addr)
-{
- asm volatile (
-#ifdef HAVE_AS_VMX
- "vmptrld %0\n"
-#else
- VMPTRLD_OPCODE MODRM_EAX_06
-#endif
- /* CF==1 or ZF==1 --> BUG() */
- UNLIKELY_START(be, vmptrld)
- _ASM_BUGFRAME_TEXT(0)
- UNLIKELY_END_SECTION
- :
-#ifdef HAVE_AS_VMX
- : "m" (addr),
-#else
- : "a" (&addr),
-#endif
- _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
- : "memory");
-}
-
-static always_inline void __vmpclear(u64 addr)
-{
- asm volatile (
-#ifdef HAVE_AS_VMX
- "vmclear %0\n"
-#else
- VMCLEAR_OPCODE MODRM_EAX_06
-#endif
- /* CF==1 or ZF==1 --> BUG() */
- UNLIKELY_START(be, vmclear)
- _ASM_BUGFRAME_TEXT(0)
- UNLIKELY_END_SECTION
- :
-#ifdef HAVE_AS_VMX
- : "m" (addr),
-#else
- : "a" (&addr),
-#endif
- _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
- : "memory");
-}
-
static always_inline void __vmread(unsigned long field, unsigned long *value)
{
asm volatile (
@@ -390,207 +211,8 @@ static always_inline void __vmread(unsigned long field, unsigned long *value)
);
}
-static always_inline void __vmwrite(unsigned long field, unsigned long value)
-{
- asm volatile (
-#ifdef HAVE_AS_VMX
- "vmwrite %1, %0\n"
-#else
- VMWRITE_OPCODE MODRM_EAX_ECX
-#endif
- /* CF==1 or ZF==1 --> BUG() */
- UNLIKELY_START(be, vmwrite)
- _ASM_BUGFRAME_TEXT(0)
- UNLIKELY_END_SECTION
- :
-#ifdef HAVE_AS_VMX
- : "r" (field) , "rm" (value),
-#else
- : "a" (field) , "c" (value),
-#endif
- _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
- );
-}
-
-static inline enum vmx_insn_errno vmread_safe(unsigned long field,
- unsigned long *value)
-{
- unsigned long ret = VMX_INSN_SUCCEED;
- bool fail_invalid, fail_valid;
-
- asm volatile ( GAS_VMX_OP("vmread %[field], %[value]\n\t",
- VMREAD_OPCODE MODRM_EAX_ECX)
- ASM_FLAG_OUT(, "setc %[invalid]\n\t")
- ASM_FLAG_OUT(, "setz %[valid]\n\t")
- : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid),
- ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid),
- [value] GAS_VMX_OP("=rm", "=c") (*value)
- : [field] GAS_VMX_OP("r", "a") (field));
-
- if ( unlikely(fail_invalid) )
- ret = VMX_INSN_FAIL_INVALID;
- else if ( unlikely(fail_valid) )
- __vmread(VM_INSTRUCTION_ERROR, &ret);
-
- return ret;
-}
-
-static inline enum vmx_insn_errno vmwrite_safe(unsigned long field,
- unsigned long value)
-{
- unsigned long ret = VMX_INSN_SUCCEED;
- bool fail_invalid, fail_valid;
-
- asm volatile ( GAS_VMX_OP("vmwrite %[value], %[field]\n\t",
- VMWRITE_OPCODE MODRM_EAX_ECX)
- ASM_FLAG_OUT(, "setc %[invalid]\n\t")
- ASM_FLAG_OUT(, "setz %[valid]\n\t")
- : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid),
- ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid)
- : [field] GAS_VMX_OP("r", "a") (field),
- [value] GAS_VMX_OP("rm", "c") (value));
-
- if ( unlikely(fail_invalid) )
- ret = VMX_INSN_FAIL_INVALID;
- else if ( unlikely(fail_valid) )
- __vmread(VM_INSTRUCTION_ERROR, &ret);
-
- return ret;
-}
-
-static always_inline void __invept(unsigned long type, uint64_t eptp)
-{
- struct {
- uint64_t eptp, rsvd;
- } operand = { eptp };
-
- /*
- * If single context invalidation is not supported, we escalate to
- * use all context invalidation.
- */
- if ( (type == INVEPT_SINGLE_CONTEXT) &&
- !cpu_has_vmx_ept_invept_single_context )
- type = INVEPT_ALL_CONTEXT;
-
- asm volatile (
-#ifdef HAVE_AS_EPT
- "invept %0, %1\n"
-#else
- INVEPT_OPCODE MODRM_EAX_08
-#endif
- /* CF==1 or ZF==1 --> BUG() */
- UNLIKELY_START(be, invept)
- _ASM_BUGFRAME_TEXT(0)
- UNLIKELY_END_SECTION
- :
-#ifdef HAVE_AS_EPT
- : "m" (operand), "r" (type),
-#else
- : "a" (&operand), "c" (type),
-#endif
- _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
- : "memory" );
-}
-
-static always_inline void __invvpid(unsigned long type, u16 vpid, u64 gva)
-{
- struct __packed {
- u64 vpid:16;
- u64 rsvd:48;
- u64 gva;
- } operand = {vpid, 0, gva};
-
- /* Fix up #UD exceptions which occur when TLBs are flushed before VMXON. */
- asm volatile ( "1: "
-#ifdef HAVE_AS_EPT
- "invvpid %0, %1\n"
-#else
- INVVPID_OPCODE MODRM_EAX_08
-#endif
- /* CF==1 or ZF==1 --> BUG() */
- UNLIKELY_START(be, invvpid)
- _ASM_BUGFRAME_TEXT(0)
- UNLIKELY_END_SECTION "\n"
- "2:"
- _ASM_EXTABLE(1b, 2b)
- :
-#ifdef HAVE_AS_EPT
- : "m" (operand), "r" (type),
-#else
- : "a" (&operand), "c" (type),
-#endif
- _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
- : "memory" );
-}
-
-static inline void ept_sync_all(void)
-{
- __invept(INVEPT_ALL_CONTEXT, 0);
-}
-
void ept_sync_domain(struct p2m_domain *p2m);
-static inline void vpid_sync_vcpu_gva(struct vcpu *v, unsigned long gva)
-{
- int type = INVVPID_INDIVIDUAL_ADDR;
-
- /*
- * If individual address invalidation is not supported, we escalate to
- * use single context invalidation.
- */
- if ( likely(cpu_has_vmx_vpid_invvpid_individual_addr) )
- goto execute_invvpid;
-
- type = INVVPID_SINGLE_CONTEXT;
-
- /*
- * If single context invalidation is not supported, we escalate to
- * use all context invalidation.
- */
- if ( !cpu_has_vmx_vpid_invvpid_single_context )
- type = INVVPID_ALL_CONTEXT;
-
-execute_invvpid:
- __invvpid(type, v->arch.hvm.n1asid.asid, (u64)gva);
-}
-
-static inline void vpid_sync_all(void)
-{
- __invvpid(INVVPID_ALL_CONTEXT, 0, 0);
-}
-
-static inline void __vmxoff(void)
-{
- asm volatile (
- VMXOFF_OPCODE
- : : : "memory" );
-}
-
-static inline int __vmxon(u64 addr)
-{
- int rc;
-
- asm volatile (
- "1: " VMXON_OPCODE MODRM_EAX_06 "\n"
- " setna %b0 ; neg %0\n" /* CF==1 or ZF==1 --> rc = -1 */
- "2:\n"
- ".section .fixup,\"ax\"\n"
- "3: sub $2,%0 ; jmp 2b\n" /* #UD or #GP --> rc = -2 */
- ".previous\n"
- _ASM_EXTABLE(1b, 3b)
- : "=q" (rc)
- : "0" (0), "a" (&addr)
- : "memory");
-
- return rc;
-}
-
-int cf_check vmx_guest_x86_mode(struct vcpu *v);
-unsigned int vmx_get_cpl(void);
-
-void vmx_inject_extint(int trap, uint8_t source);
-void vmx_inject_nmi(void);
-
void ept_walk_table(struct domain *d, unsigned long gfn);
bool ept_handle_misconfig(uint64_t gpa);
int epte_get_entry_emt(struct domain *d, gfn_t gfn, mfn_t mfn,
@@ -599,13 +221,6 @@ void setup_ept_dump(void);
/* Locate an alternate p2m by its EPTP */
unsigned int p2m_find_altp2m_by_eptp(struct domain *d, uint64_t eptp);
-void update_guest_eip(void);
-
-void vmx_pi_per_cpu_init(unsigned int cpu);
-void vmx_pi_desc_fixup(unsigned int cpu);
-
-void vmx_sync_exit_bitmap(struct vcpu *v);
-
#ifdef CONFIG_HVM
void vmx_pi_hooks_assign(struct domain *d);
void vmx_pi_hooks_deassign(struct domain *d);
@@ -614,8 +229,6 @@ static inline void vmx_pi_hooks_assign(struct domain *d) {}
static inline void vmx_pi_hooks_deassign(struct domain *d) {}
#endif
-#define APIC_INVALID_DEST 0xffffffff
-
/* EPT violation qualifications definitions */
typedef union ept_qual {
unsigned long raw;
@@ -631,56 +244,4 @@ typedef union ept_qual {
#define EPT_L4_PAGETABLE_SHIFT 39
#define EPT_PAGETABLE_ENTRIES 512
-/* #VE information page */
-typedef struct {
- u32 exit_reason;
- u32 semaphore;
- u64 exit_qualification;
- u64 gla;
- u64 gpa;
- u16 eptp_index;
-} ve_info_t;
-
-/* VM-Exit instruction info for LIDT, LGDT, SIDT, SGDT */
-typedef union idt_or_gdt_instr_info {
- unsigned long raw;
- struct {
- unsigned long scaling :2, /* bits 0:1 - Scaling */
- :5, /* bits 6:2 - Undefined */
- addr_size :3, /* bits 9:7 - Address size */
- :1, /* bit 10 - Cleared to 0 */
- operand_size :1, /* bit 11 - Operand size */
- :3, /* bits 14:12 - Undefined */
- segment_reg :3, /* bits 17:15 - Segment register */
- index_reg :4, /* bits 21:18 - Index register */
- index_reg_invalid :1, /* bit 22 - Index register invalid */
- base_reg :4, /* bits 26:23 - Base register */
- base_reg_invalid :1, /* bit 27 - Base register invalid */
- instr_identity :1, /* bit 28 - 0:GDT, 1:IDT */
- instr_write :1, /* bit 29 - 0:store, 1:load */
- :34; /* bits 30:63 - Undefined */
- };
-} idt_or_gdt_instr_info_t;
-
-/* VM-Exit instruction info for LLDT, LTR, SLDT, STR */
-typedef union ldt_or_tr_instr_info {
- unsigned long raw;
- struct {
- unsigned long scaling :2, /* bits 0:1 - Scaling */
- :1, /* bit 2 - Undefined */
- reg1 :4, /* bits 6:3 - Reg1 */
- addr_size :3, /* bits 9:7 - Address size */
- mem_reg :1, /* bit 10 - Mem/Reg */
- :4, /* bits 14:11 - Undefined */
- segment_reg :3, /* bits 17:15 - Segment register */
- index_reg :4, /* bits 21:18 - Index register */
- index_reg_invalid :1, /* bit 22 - Index register invalid */
- base_reg :4, /* bits 26:23 - Base register */
- base_reg_invalid :1, /* bit 27 - Base register invalid */
- instr_identity :1, /* bit 28 - 0:LDT, 1:TR */
- instr_write :1, /* bit 29 - 0:store, 1:load */
- :34; /* bits 31:63 - Undefined */
- };
-} ldt_or_tr_instr_info_t;
-
#endif /* __ASM_X86_HVM_VMX_VMX_H__ */
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers
2023-02-24 18:50 ` [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers Xenia Ragiadakou
@ 2023-02-27 15:25 ` Jan Beulich
2023-02-28 7:36 ` Xenia Ragiadakou
2023-02-27 16:26 ` Andrew Cooper
1 sibling, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2023-02-27 15:25 UTC (permalink / raw)
To: Xenia Ragiadakou
Cc: Jun Nakajima, Kevin Tian, Andrew Cooper, Roger Pau Monné,
Wei Liu, xen-devel
On 24.02.2023 19:50, Xenia Ragiadakou wrote:
> --- /dev/null
> +++ b/xen/arch/x86/hvm/vmx/pi.h
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * pi.h: VMX Posted Interrupts
> + *
> + * Copyright (c) 2004, Intel Corporation.
> + */
> +
> +#ifndef __X86_HVM_VMX_PI_PRIV_H__
> +#define __X86_HVM_VMX_PI_PRIV_H__
I can see that you need something to disambiguate the two vmx.h. But
here you don't need the PRIV infix, do you? Even in the private vmx.h
I'd like to ask to consider e.g. __VMX_H__ as the guard (and then
__PI_H__ here), rather than one which doesn't really match the
filename.
Jan
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers
2023-02-27 15:25 ` Jan Beulich
@ 2023-02-28 7:36 ` Xenia Ragiadakou
2023-02-28 8:05 ` Jan Beulich
0 siblings, 1 reply; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-28 7:36 UTC (permalink / raw)
To: Jan Beulich
Cc: Jun Nakajima, Kevin Tian, Andrew Cooper, Roger Pau Monné,
Wei Liu, xen-devel
On 2/27/23 17:25, Jan Beulich wrote:
> On 24.02.2023 19:50, Xenia Ragiadakou wrote:
>> --- /dev/null
>> +++ b/xen/arch/x86/hvm/vmx/pi.h
>> @@ -0,0 +1,78 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * pi.h: VMX Posted Interrupts
>> + *
>> + * Copyright (c) 2004, Intel Corporation.
>> + */
>> +
>> +#ifndef __X86_HVM_VMX_PI_PRIV_H__
>> +#define __X86_HVM_VMX_PI_PRIV_H__
>
> I can see that you need something to disambiguate the two vmx.h. But
> here you don't need the PRIV infix, do you? Even in the private vmx.h
> I'd like to ask to consider e.g. __VMX_H__ as the guard (and then
> __PI_H__ here), rather than one which doesn't really match the
> filename.
I do agree that adding _PRIV_ is off target.
For the purpose of disambiguation, the header guards need to conform to
a well specified pattern guaranteed not to be used for anything else.
For that reason I would suggest the guard to include the path, not just
the file name.
In any case, the pattern that is used to generate the header guards
should be mentioned in the coding style doc.
>
> Jan
--
Xenia
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers
2023-02-28 7:36 ` Xenia Ragiadakou
@ 2023-02-28 8:05 ` Jan Beulich
0 siblings, 0 replies; 45+ messages in thread
From: Jan Beulich @ 2023-02-28 8:05 UTC (permalink / raw)
To: Xenia Ragiadakou
Cc: Jun Nakajima, Kevin Tian, Andrew Cooper, Roger Pau Monné,
Wei Liu, xen-devel
On 28.02.2023 08:36, Xenia Ragiadakou wrote:
>
> On 2/27/23 17:25, Jan Beulich wrote:
>> On 24.02.2023 19:50, Xenia Ragiadakou wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/x86/hvm/vmx/pi.h
>>> @@ -0,0 +1,78 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * pi.h: VMX Posted Interrupts
>>> + *
>>> + * Copyright (c) 2004, Intel Corporation.
>>> + */
>>> +
>>> +#ifndef __X86_HVM_VMX_PI_PRIV_H__
>>> +#define __X86_HVM_VMX_PI_PRIV_H__
>>
>> I can see that you need something to disambiguate the two vmx.h. But
>> here you don't need the PRIV infix, do you? Even in the private vmx.h
>> I'd like to ask to consider e.g. __VMX_H__ as the guard (and then
>> __PI_H__ here), rather than one which doesn't really match the
>> filename.
>
> I do agree that adding _PRIV_ is off target.
> For the purpose of disambiguation, the header guards need to conform to
> a well specified pattern guaranteed not to be used for anything else.
> For that reason I would suggest the guard to include the path, not just
> the file name.
But as we see the path can be ambiguous, unless all non-private headers
had an "INCLUDE" infix. Hence my proposal would be no path at all for
private headers, and path (as we frequently but perhaps not consistently
have it) for non-private ones.
> In any case, the pattern that is used to generate the header guards
> should be mentioned in the coding style doc.
Perhaps.
Jan
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers
2023-02-24 18:50 ` [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers Xenia Ragiadakou
2023-02-27 15:25 ` Jan Beulich
@ 2023-02-27 16:26 ` Andrew Cooper
2023-02-27 16:41 ` Andrew Cooper
2023-02-28 7:47 ` Jan Beulich
1 sibling, 2 replies; 45+ messages in thread
From: Andrew Cooper @ 2023-02-27 16:26 UTC (permalink / raw)
To: Xenia Ragiadakou, xen-devel
Cc: Jun Nakajima, Kevin Tian, Jan Beulich, Roger Pau Monné, Wei Liu
On 24/02/2023 6:50 pm, Xenia Ragiadakou wrote:
> Create two new private headers in arch/x86/hvm/vmx called vmx.h and pi.h.
> Move all the definitions and declarations that are used solely by vmx code
> into the private vmx.h, apart from the ones related to posted interrupts that
> are moved into pi.h.
>
> EPT related declarations and definitions stay in asm/hvm/vmx/vmx.h because
> they are used in arch/x86/mm and drivers/passthrough/vtd.
>
> Also, __vmread(), used in arch/x86/cpu, and consequently the opcodes stay in
> asm/hvm/vmx/vmx.h.
Every time I read the vpmu code, I get increasingly sad.
That is dangerously unsafe, and comes with a chance of exploding completely.
That __vmread() is in NMI context, which means `current` isn't safe to
deference (we might hit in the middle of a context switch), and more
generally there's no guarantee that the loaded VMCS is the one
associated with `current` (we might hit in the middle of a remote VMCS
access).
vpmu is generally not supported, and BTS needs further custom enablement
because it is only useable with a custom bus analyser.
The __vmread() needs deleting - its absolutely not safe to say.
I'm tempted to hardwire the return 0, and punt the problem to whomever
next uses BTS.
Alternatively, MSR_DBGCTL needs wiring into the hvm_get_reg()
infrastructure, but I'm not convinced this will actually work in either
of the two problem cases above, hence preferring the previous option.
Thoughts?
~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers
2023-02-27 16:26 ` Andrew Cooper
@ 2023-02-27 16:41 ` Andrew Cooper
2023-02-28 7:47 ` Jan Beulich
1 sibling, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2023-02-27 16:41 UTC (permalink / raw)
To: Xenia Ragiadakou, xen-devel
Cc: Jun Nakajima, Kevin Tian, Jan Beulich, Roger Pau Monné, Wei Liu
On 27/02/2023 4:26 pm, Andrew Cooper wrote:
> On 24/02/2023 6:50 pm, Xenia Ragiadakou wrote:
>> Create two new private headers in arch/x86/hvm/vmx called vmx.h and pi.h.
>> Move all the definitions and declarations that are used solely by vmx code
>> into the private vmx.h, apart from the ones related to posted interrupts that
>> are moved into pi.h.
>>
>> EPT related declarations and definitions stay in asm/hvm/vmx/vmx.h because
>> they are used in arch/x86/mm and drivers/passthrough/vtd.
>>
>> Also, __vmread(), used in arch/x86/cpu, and consequently the opcodes stay in
>> asm/hvm/vmx/vmx.h.
> Every time I read the vpmu code, I get increasingly sad.
>
> That is dangerously unsafe, and comes with a chance of exploding completely.
>
> That __vmread() is in NMI context, which means `current` isn't safe to
> deference (we might hit in the middle of a context switch), and more
> generally there's no guarantee that the loaded VMCS is the one
> associated with `current` (we might hit in the middle of a remote VMCS
> access).
>
> vpmu is generally not supported, and BTS needs further custom enablement
> because it is only useable with a custom bus analyser.
>
>
> The __vmread() needs deleting - its absolutely not safe to say.
to stay*
>
> I'm tempted to hardwire the return 0, and punt the problem to whomever
> next uses BTS.
>
> Alternatively, MSR_DBGCTL needs wiring into the hvm_get_reg()
> infrastructure, but I'm not convinced this will actually work in either
> of the two problem cases above, hence preferring the previous option.
>
> Thoughts?
>
> ~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers
2023-02-27 16:26 ` Andrew Cooper
2023-02-27 16:41 ` Andrew Cooper
@ 2023-02-28 7:47 ` Jan Beulich
1 sibling, 0 replies; 45+ messages in thread
From: Jan Beulich @ 2023-02-28 7:47 UTC (permalink / raw)
To: Andrew Cooper
Cc: Jun Nakajima, Kevin Tian, Roger Pau Monné,
Wei Liu, xen-devel, Xenia Ragiadakou
On 27.02.2023 17:26, Andrew Cooper wrote:
> On 24/02/2023 6:50 pm, Xenia Ragiadakou wrote:
>> Create two new private headers in arch/x86/hvm/vmx called vmx.h and pi.h.
>> Move all the definitions and declarations that are used solely by vmx code
>> into the private vmx.h, apart from the ones related to posted interrupts that
>> are moved into pi.h.
>>
>> EPT related declarations and definitions stay in asm/hvm/vmx/vmx.h because
>> they are used in arch/x86/mm and drivers/passthrough/vtd.
>>
>> Also, __vmread(), used in arch/x86/cpu, and consequently the opcodes stay in
>> asm/hvm/vmx/vmx.h.
>
> Every time I read the vpmu code, I get increasingly sad.
>
> That is dangerously unsafe, and comes with a chance of exploding completely.
>
> That __vmread() is in NMI context, which means `current` isn't safe to
> deference (we might hit in the middle of a context switch), and more
> generally there's no guarantee that the loaded VMCS is the one
> associated with `current` (we might hit in the middle of a remote VMCS
> access).
Are you mixing up oprofile (using NMI) and vPMU (using an ordinary vectored
interrupt)? Or am I overlooking a vPMU mode of operation where NMI could be
used (i.e. other than apic_intr_init()'s calling of set_direct_apic_vector()
and other than pmu_interrupt() invoking vpmu_do_interrupt() /after/ acking
the IRQ at the LAPIC)?
Jan
> vpmu is generally not supported, and BTS needs further custom enablement
> because it is only useable with a custom bus analyser.
>
>
> The __vmread() needs deleting - its absolutely not safe to say.
>
> I'm tempted to hardwire the return 0, and punt the problem to whomever
> next uses BTS.
>
> Alternatively, MSR_DBGCTL needs wiring into the hvm_get_reg()
> infrastructure, but I'm not convinced this will actually work in either
> of the two problem cases above, hence preferring the previous option.
>
> Thoughts?
>
> ~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v3 11/14] x86/vmx: remove unused included headers from vmx.c
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (9 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 10/14] x86/vmx: move declarations used only by vmx code from vmx.h to private headers Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-24 18:50 ` [PATCH v3 12/14] x86/vmx: declare nvmx_enqueue_n2_exceptions() static Xenia Ragiadakou
` (3 subsequent siblings)
14 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel
Cc: Jun Nakajima, Kevin Tian, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu
Do not include the headers:
asm/hvm/vpic.h
asm/hvm/vpt.h
asm/io.h
asm/mce.h
asm/mem_sharing.h
asm/regs.h
public/arch-x86/cpuid.h
public/hvm/save.h
because none of the declarations and macro definitions in them is used.
Sort the rest of the headers alphabetically.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- none
xen/arch/x86/hvm/vmx/vmx.c | 56 +++++++++++++++++---------------------
1 file changed, 25 insertions(+), 31 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index cb8b133ed5..9b009ebcef 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -15,52 +15,46 @@
* this program; If not, see <http://www.gnu.org/licenses/>.
*/
+#include <xen/domain_page.h>
#include <xen/guest_access.h>
+#include <xen/hypercall.h>
#include <xen/init.h>
+#include <xen/irq.h>
#include <xen/lib.h>
#include <xen/param.h>
-#include <xen/trace.h>
+#include <xen/perfc.h>
#include <xen/sched.h>
-#include <xen/irq.h>
#include <xen/softirq.h>
-#include <xen/domain_page.h>
-#include <xen/hypercall.h>
-#include <xen/perfc.h>
-#include <asm/current.h>
-#include <asm/io.h>
-#include <asm/iocap.h>
-#include <asm/regs.h>
+#include <xen/trace.h>
+
+#include <asm/altp2m.h>
+#include <asm/apic.h>
#include <asm/cpufeature.h>
-#include <asm/processor.h>
+#include <asm/current.h>
+#include <asm/gdbsx.h>
#include <asm/debugreg.h>
-#include <asm/msr.h>
-#include <asm/p2m.h>
-#include <asm/mem_sharing.h>
+#include <asm/event.h>
#include <asm/hvm/emulate.h>
#include <asm/hvm/hvm.h>
+#include <asm/hvm/monitor.h>
+#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/support.h>
-#include <asm/hvm/vmx/vmx.h>
+#include <asm/hvm/trace.h>
+#include <asm/hvm/vlapic.h>
#include <asm/hvm/vmx/vmcs.h>
-#include <public/sched.h>
-#include <public/hvm/ioreq.h>
+#include <asm/hvm/vmx/vmx.h>
+#include <asm/iocap.h>
#include <asm/i387.h>
-#include <asm/hvm/vpic.h>
-#include <asm/hvm/vlapic.h>
-#include <asm/x86_emulate.h>
-#include <asm/hvm/vpt.h>
-#include <public/hvm/save.h>
-#include <asm/hvm/trace.h>
-#include <asm/hvm/monitor.h>
-#include <asm/xenoprof.h>
-#include <asm/gdbsx.h>
-#include <asm/apic.h>
-#include <asm/hvm/nestedhvm.h>
-#include <asm/altp2m.h>
-#include <asm/event.h>
-#include <asm/mce.h>
#include <asm/monitor.h>
+#include <asm/msr.h>
+#include <asm/processor.h>
#include <asm/prot-key.h>
-#include <public/arch-x86/cpuid.h>
+#include <asm/p2m.h>
+#include <asm/xenoprof.h>
+#include <asm/x86_emulate.h>
+
+#include <public/sched.h>
+#include <public/hvm/ioreq.h>
#include "pi.h"
#include "vmx.h"
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 12/14] x86/vmx: declare nvmx_enqueue_n2_exceptions() static
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (10 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 11/14] x86/vmx: remove unused included headers from vmx.c Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-24 18:50 ` [PATCH v3 13/14] x86/vmx: move vvmx declarations used only by vmx code to private header Xenia Ragiadakou
` (2 subsequent siblings)
14 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel
Cc: Jun Nakajima, Kevin Tian, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu
Reduce the scope of nvmx_enqueue_n2_exceptions() to static because it is used
only in this file.
Take the opportunity to remove a trailing space.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v3:
- add Jan's R-b tag
xen/arch/x86/hvm/vmx/vmx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 9b009ebcef..72d8f058f7 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1897,7 +1897,7 @@ static void cf_check vmx_update_guest_efer(struct vcpu *v)
vmx_set_msr_intercept(v, MSR_EFER, VMX_MSR_R);
}
-void nvmx_enqueue_n2_exceptions(struct vcpu *v,
+static void nvmx_enqueue_n2_exceptions(struct vcpu *v,
unsigned long intr_fields, int error_code, uint8_t source)
{
struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 13/14] x86/vmx: move vvmx declarations used only by vmx code to private header
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (11 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 12/14] x86/vmx: declare nvmx_enqueue_n2_exceptions() static Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-24 18:50 ` [PATCH v3 14/14] x86/vmx: move vmcs " Xenia Ragiadakou
2023-02-24 19:29 ` [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Andrew Cooper
14 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel
Cc: Jun Nakajima, Kevin Tian, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu
Create a new private header in arch/x86/hvm/vmx called vvmx.h and move
there all definitions and declarations that are used only by vmx code and
don't need to reside in an external header.
Take the opportunity to replace u* with uint*_t, bool_t with bool and to
re-arrange the header as follows, all structures first, then all variable
decalarations, all function delarations, and finally all inline functions.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- new patch
xen/arch/x86/hvm/vmx/intr.c | 1 +
xen/arch/x86/hvm/vmx/vmcs.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 1 +
xen/arch/x86/hvm/vmx/vvmx.c | 1 +
xen/arch/x86/hvm/vmx/vvmx.h | 187 ++++++++++++++++++++++++
xen/arch/x86/include/asm/hvm/vmx/vvmx.h | 165 +--------------------
6 files changed, 198 insertions(+), 159 deletions(-)
create mode 100644 xen/arch/x86/hvm/vmx/vvmx.h
diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
index c8db501759..8431937f42 100644
--- a/xen/arch/x86/hvm/vmx/intr.c
+++ b/xen/arch/x86/hvm/vmx/intr.c
@@ -39,6 +39,7 @@
#include <asm/vm_event.h>
#include "vmx.h"
+#include "vvmx.h"
/*
* A few notes on virtual NMI and INTR delivery, and interactions with
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 4eb2571abb..3d0f6be5bb 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -34,7 +34,6 @@
#include <asm/hvm/io.h>
#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/vmx/vmx.h>
-#include <asm/hvm/vmx/vvmx.h>
#include <asm/hvm/vmx/vmcs.h>
#include <asm/flushtlb.h>
#include <asm/monitor.h>
@@ -44,6 +43,7 @@
#include <asm/apic.h>
#include "vmx.h"
+#include "vvmx.h"
static bool_t __read_mostly opt_vpid_enabled = 1;
boolean_param("vpid", opt_vpid_enabled);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 72d8f058f7..73ab4e9816 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -58,6 +58,7 @@
#include "pi.h"
#include "vmx.h"
+#include "vvmx.h"
static bool_t __initdata opt_force_ept;
boolean_param("force-ept", opt_force_ept);
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 0bda8430b9..0af5411076 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -30,6 +30,7 @@
#include <asm/hvm/nestedhvm.h>
#include "vmx.h"
+#include "vvmx.h"
static DEFINE_PER_CPU(u64 *, vvmcs_buf);
diff --git a/xen/arch/x86/hvm/vmx/vvmx.h b/xen/arch/x86/hvm/vmx/vvmx.h
new file mode 100644
index 0000000000..0367fae42a
--- /dev/null
+++ b/xen/arch/x86/hvm/vmx/vvmx.h
@@ -0,0 +1,187 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * vvmx.h: Support virtual VMX for nested virtualization.
+ *
+ * Copyright (c) 2010, Intel Corporation.
+ * Author: Qing He <qing.he@intel.com>
+ * Eddie Dong <eddie.dong@intel.com>
+ */
+
+#ifndef __X86_HVM_VMX_VVMX_PRIV_H__
+#define __X86_HVM_VMX_VVMX_PRIV_H__
+
+#include <xen/list.h>
+#include <xen/sched.h>
+#include <xen/types.h>
+
+#include <asm/hvm/vcpu.h>
+#include <asm/hvm/hvm.h>
+#include <asm/hvm/vmx/vmcs.h>
+
+struct vvmcs_list {
+ unsigned long vvmcs_mfn;
+ struct list_head node;
+};
+
+#define vcpu_2_nvmx(v) (vcpu_nestedhvm(v).u.nvmx)
+
+/* bit 1, 2, 4 must be 1 */
+#define VMX_PINBASED_CTLS_DEFAULT1 0x16
+/* bit 1, 4-6,8,13-16,26 must be 1 */
+#define VMX_PROCBASED_CTLS_DEFAULT1 0x401e172
+/* bit 0-8, 10,11,13,14,16,17 must be 1 */
+#define VMX_EXIT_CTLS_DEFAULT1 0x36dff
+/* bit 0-8, and 12 must be 1 */
+#define VMX_ENTRY_CTLS_DEFAULT1 0x11ff
+
+union vmx_inst_info {
+ struct {
+ unsigned int scaling :2; /* bit 0-1 */
+ unsigned int __rsvd0 :1; /* bit 2 */
+ unsigned int reg1 :4; /* bit 3-6 */
+ unsigned int addr_size :3; /* bit 7-9 */
+ unsigned int memreg :1; /* bit 10 */
+ unsigned int __rsvd1 :4; /* bit 11-14 */
+ unsigned int segment :3; /* bit 15-17 */
+ unsigned int index_reg :4; /* bit 18-21 */
+ unsigned int index_reg_invalid :1; /* bit 22 */
+ unsigned int base_reg :4; /* bit 23-26 */
+ unsigned int base_reg_invalid :1; /* bit 27 */
+ unsigned int reg2 :4; /* bit 28-31 */
+ } fields;
+ uint32_t word;
+};
+
+/*
+ * Virtual VMCS layout
+ *
+ * Since physical VMCS layout is unknown, a custom layout is used
+ * for virtual VMCS seen by guest. It occupies a 4k page, and the
+ * field is offset by an 9-bit offset into u64[], The offset is as
+ * follow, which means every <width, type> pair has a max of 32
+ * fields available.
+ *
+ * 9 7 5 0
+ * --------------------------------
+ * offset: | width | type | index |
+ * --------------------------------
+ *
+ * Also, since the lower range <width=0, type={0,1}> has only one
+ * field: VPID, it is moved to a higher offset (63), and leaves the
+ * lower range to non-indexed field like VMCS revision.
+ *
+ */
+
+struct vvmcs_header {
+ uint32_t revision;
+ uint32_t abort;
+};
+
+union vmcs_encoding {
+ struct {
+ uint32_t access_type : 1;
+ uint32_t index : 9;
+ uint32_t type : 2;
+ uint32_t rsv1 : 1;
+ uint32_t width : 2;
+ uint32_t rsv2 : 17;
+ };
+ uint32_t word;
+};
+
+enum vvmcs_encoding_width {
+ VVMCS_WIDTH_16 = 0,
+ VVMCS_WIDTH_64,
+ VVMCS_WIDTH_32,
+ VVMCS_WIDTH_NATURAL,
+};
+
+enum vvmcs_encoding_type {
+ VVMCS_TYPE_CONTROL = 0,
+ VVMCS_TYPE_RO,
+ VVMCS_TYPE_GSTATE,
+ VVMCS_TYPE_HSTATE,
+};
+
+int cf_check nvmx_vcpu_initialise(struct vcpu *v);
+void cf_check nvmx_vcpu_destroy(struct vcpu *v);
+int cf_check nvmx_vcpu_reset(struct vcpu *v);
+uint64_t cf_check nvmx_vcpu_eptp_base(struct vcpu *v);
+enum hvm_intblk cf_check nvmx_intr_blocked(struct vcpu *v);
+bool cf_check nvmx_intercepts_exception(struct vcpu *v, unsigned int vector,
+ int error_code);
+void cf_check nvmx_domain_relinquish_resources(struct domain *d);
+
+bool cf_check nvmx_ept_enabled(struct vcpu *v);
+
+int cf_check nvmx_hap_walk_L1_p2m(
+ struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa, unsigned int *page_order,
+ uint8_t *p2m_acc, struct npfec npfec);
+
+uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t encoding);
+uint64_t get_vvmcs_real(const struct vcpu *, uint32_t encoding);
+void set_vvmcs_virtual(void *vvmcs, uint32_t encoding, uint64_t val);
+void set_vvmcs_real(const struct vcpu *, uint32_t encoding, uint64_t val);
+enum vmx_insn_errno get_vvmcs_virtual_safe(void *vvmcs, uint32_t encoding,
+ uint64_t *val);
+enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *, uint32_t encoding,
+ uint64_t *val);
+enum vmx_insn_errno set_vvmcs_virtual_safe(void *vvmcs, uint32_t encoding,
+ uint64_t val);
+enum vmx_insn_errno set_vvmcs_real_safe(const struct vcpu *, uint32_t encoding,
+ uint64_t val);
+
+#define get_vvmcs(vcpu, encoding) \
+ (cpu_has_vmx_vmcs_shadowing ? \
+ get_vvmcs_real(vcpu, encoding) : \
+ get_vvmcs_virtual(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding))
+
+#define set_vvmcs(vcpu, encoding, val) \
+ (cpu_has_vmx_vmcs_shadowing ? \
+ set_vvmcs_real(vcpu, encoding, val) : \
+ set_vvmcs_virtual(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding, val))
+
+#define get_vvmcs_safe(vcpu, encoding, val) \
+ (cpu_has_vmx_vmcs_shadowing ? \
+ get_vvmcs_real_safe(vcpu, encoding, val) : \
+ get_vvmcs_virtual_safe(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding, val))
+
+#define set_vvmcs_safe(vcpu, encoding, val) \
+ (cpu_has_vmx_vmcs_shadowing ? \
+ set_vvmcs_real_safe(vcpu, encoding, val) : \
+ set_vvmcs_virtual_safe(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding, val))
+
+void nvmx_destroy_vmcs(struct vcpu *v);
+int nvmx_handle_vmx_insn(struct cpu_user_regs *regs, unsigned int exit_reason);
+int nvmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content);
+
+void nvmx_update_exec_control(struct vcpu *v, uint32_t value);
+void nvmx_update_secondary_exec_control(struct vcpu *v, unsigned long value);
+void nvmx_update_exception_bitmap(struct vcpu *v, unsigned long value);
+void nvmx_switch_guest(void);
+void nvmx_idtv_handling(void);
+uint64_t nvmx_get_tsc_offset(struct vcpu *v);
+int nvmx_n2_vmexit_handler(struct cpu_user_regs *regs,
+ unsigned int exit_reason);
+void nvmx_set_cr_read_shadow(struct vcpu *v, unsigned int cr);
+
+uint64_t nept_get_ept_vpid_cap(void);
+
+int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
+ unsigned int *page_order, uint32_t rwx_acc,
+ unsigned long *l1gfn, uint8_t *p2m_acc,
+ uint64_t *exit_qual, uint32_t *exit_reason);
+int nvmx_cpu_up_prepare(unsigned int cpu);
+void nvmx_cpu_dead(unsigned int cpu);
+
+#endif /* __X86_HVM_VMX_VVMX_PRIV_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vvmx.h b/xen/arch/x86/include/asm/hvm/vmx/vvmx.h
index 2c3adb5dd6..2bf6ef40e9 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vvmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vvmx.h
@@ -22,10 +22,10 @@
#ifndef __ASM_X86_HVM_VVMX_H__
#define __ASM_X86_HVM_VVMX_H__
-struct vvmcs_list {
- unsigned long vvmcs_mfn;
- struct list_head node;
-};
+#include <xen/list.h>
+#include <xen/types.h>
+
+#include <asm/hvm/vmx/vmcs.h>
struct nestedvmx {
/*
@@ -42,11 +42,11 @@ struct nestedvmx {
/* deferred nested interrupt */
struct {
unsigned long intr_info;
- u32 error_code;
- u8 source;
+ uint32_t error_code;
+ uint8_t source;
} intr;
struct {
- bool_t enabled;
+ bool enabled;
uint32_t exit_reason;
uint32_t exit_qual;
} ept;
@@ -54,161 +54,10 @@ struct nestedvmx {
struct list_head launched_list;
};
-#define vcpu_2_nvmx(v) (vcpu_nestedhvm(v).u.nvmx)
-
-/* bit 1, 2, 4 must be 1 */
-#define VMX_PINBASED_CTLS_DEFAULT1 0x16
-/* bit 1, 4-6,8,13-16,26 must be 1 */
-#define VMX_PROCBASED_CTLS_DEFAULT1 0x401e172
-/* bit 0-8, 10,11,13,14,16,17 must be 1 */
-#define VMX_EXIT_CTLS_DEFAULT1 0x36dff
-/* bit 0-8, and 12 must be 1 */
-#define VMX_ENTRY_CTLS_DEFAULT1 0x11ff
-
-
-union vmx_inst_info {
- struct {
- unsigned int scaling :2; /* bit 0-1 */
- unsigned int __rsvd0 :1; /* bit 2 */
- unsigned int reg1 :4; /* bit 3-6 */
- unsigned int addr_size :3; /* bit 7-9 */
- unsigned int memreg :1; /* bit 10 */
- unsigned int __rsvd1 :4; /* bit 11-14 */
- unsigned int segment :3; /* bit 15-17 */
- unsigned int index_reg :4; /* bit 18-21 */
- unsigned int index_reg_invalid :1; /* bit 22 */
- unsigned int base_reg :4; /* bit 23-26 */
- unsigned int base_reg_invalid :1; /* bit 27 */
- unsigned int reg2 :4; /* bit 28-31 */
- } fields;
- u32 word;
-};
-
-int cf_check nvmx_vcpu_initialise(struct vcpu *v);
-void cf_check nvmx_vcpu_destroy(struct vcpu *v);
-int cf_check nvmx_vcpu_reset(struct vcpu *v);
-uint64_t cf_check nvmx_vcpu_eptp_base(struct vcpu *v);
-enum hvm_intblk cf_check nvmx_intr_blocked(struct vcpu *v);
-bool cf_check nvmx_intercepts_exception(
- struct vcpu *v, unsigned int vector, int error_code);
-void cf_check nvmx_domain_relinquish_resources(struct domain *d);
-
-bool cf_check nvmx_ept_enabled(struct vcpu *v);
-
#define EPT_TRANSLATE_SUCCEED 0
#define EPT_TRANSLATE_VIOLATION 1
#define EPT_TRANSLATE_MISCONFIG 2
#define EPT_TRANSLATE_RETRY 3
-int cf_check nvmx_hap_walk_L1_p2m(
- struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa, unsigned int *page_order,
- uint8_t *p2m_acc, struct npfec npfec);
-
-/*
- * Virtual VMCS layout
- *
- * Since physical VMCS layout is unknown, a custom layout is used
- * for virtual VMCS seen by guest. It occupies a 4k page, and the
- * field is offset by an 9-bit offset into u64[], The offset is as
- * follow, which means every <width, type> pair has a max of 32
- * fields available.
- *
- * 9 7 5 0
- * --------------------------------
- * offset: | width | type | index |
- * --------------------------------
- *
- * Also, since the lower range <width=0, type={0,1}> has only one
- * field: VPID, it is moved to a higher offset (63), and leaves the
- * lower range to non-indexed field like VMCS revision.
- *
- */
-
-struct vvmcs_header {
- u32 revision;
- u32 abort;
-};
-
-union vmcs_encoding {
- struct {
- u32 access_type : 1;
- u32 index : 9;
- u32 type : 2;
- u32 rsv1 : 1;
- u32 width : 2;
- u32 rsv2 : 17;
- };
- u32 word;
-};
-
-enum vvmcs_encoding_width {
- VVMCS_WIDTH_16 = 0,
- VVMCS_WIDTH_64,
- VVMCS_WIDTH_32,
- VVMCS_WIDTH_NATURAL,
-};
-
-enum vvmcs_encoding_type {
- VVMCS_TYPE_CONTROL = 0,
- VVMCS_TYPE_RO,
- VVMCS_TYPE_GSTATE,
- VVMCS_TYPE_HSTATE,
-};
-
-u64 get_vvmcs_virtual(void *vvmcs, u32 encoding);
-u64 get_vvmcs_real(const struct vcpu *, u32 encoding);
-void set_vvmcs_virtual(void *vvmcs, u32 encoding, u64 val);
-void set_vvmcs_real(const struct vcpu *, u32 encoding, u64 val);
-enum vmx_insn_errno get_vvmcs_virtual_safe(void *vvmcs, u32 encoding, u64 *val);
-enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *, u32 encoding,
- u64 *val);
-enum vmx_insn_errno set_vvmcs_virtual_safe(void *vvmcs, u32 encoding, u64 val);
-enum vmx_insn_errno set_vvmcs_real_safe(const struct vcpu *, u32 encoding,
- u64 val);
-
-#define get_vvmcs(vcpu, encoding) \
- (cpu_has_vmx_vmcs_shadowing ? \
- get_vvmcs_real(vcpu, encoding) : \
- get_vvmcs_virtual(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding))
-
-#define set_vvmcs(vcpu, encoding, val) \
- (cpu_has_vmx_vmcs_shadowing ? \
- set_vvmcs_real(vcpu, encoding, val) : \
- set_vvmcs_virtual(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding, val))
-
-#define get_vvmcs_safe(vcpu, encoding, val) \
- (cpu_has_vmx_vmcs_shadowing ? \
- get_vvmcs_real_safe(vcpu, encoding, val) : \
- get_vvmcs_virtual_safe(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding, val))
-
-#define set_vvmcs_safe(vcpu, encoding, val) \
- (cpu_has_vmx_vmcs_shadowing ? \
- set_vvmcs_real_safe(vcpu, encoding, val) : \
- set_vvmcs_virtual_safe(vcpu_nestedhvm(vcpu).nv_vvmcx, encoding, val))
-
-void nvmx_destroy_vmcs(struct vcpu *v);
-int nvmx_handle_vmx_insn(struct cpu_user_regs *regs, unsigned int exit_reason);
-int nvmx_msr_read_intercept(unsigned int msr,
- u64 *msr_content);
-
-void nvmx_update_exec_control(struct vcpu *v, u32 value);
-void nvmx_update_secondary_exec_control(struct vcpu *v,
- unsigned long value);
-void nvmx_update_exception_bitmap(struct vcpu *v, unsigned long value);
-void nvmx_switch_guest(void);
-void nvmx_idtv_handling(void);
-u64 nvmx_get_tsc_offset(struct vcpu *v);
-int nvmx_n2_vmexit_handler(struct cpu_user_regs *regs,
- unsigned int exit_reason);
-void nvmx_set_cr_read_shadow(struct vcpu *v, unsigned int cr);
-
-uint64_t nept_get_ept_vpid_cap(void);
-
-int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
- unsigned int *page_order, uint32_t rwx_acc,
- unsigned long *l1gfn, uint8_t *p2m_acc,
- uint64_t *exit_qual, uint32_t *exit_reason);
-int nvmx_cpu_up_prepare(unsigned int cpu);
-void nvmx_cpu_dead(unsigned int cpu);
#endif /* __ASM_X86_HVM_VVMX_H__ */
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* [PATCH v3 14/14] x86/vmx: move vmcs declarations used only by vmx code to private header
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (12 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 13/14] x86/vmx: move vvmx declarations used only by vmx code to private header Xenia Ragiadakou
@ 2023-02-24 18:50 ` Xenia Ragiadakou
2023-02-24 19:29 ` [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Andrew Cooper
14 siblings, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 18:50 UTC (permalink / raw)
To: xen-devel
Cc: Jun Nakajima, Kevin Tian, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu
Create a new private header in arch/x86/hvm/vmx called vmcs.h and move
there all definitions and declarations that are used only by vmx code and
don't need to reside in an external header.
Take the opportunity to replace u* with uint*_t, bool_t with bool and to
re-arrange the header as follows, all structures first, then all variable
declarations, all function delarations, and finally all inline functions.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- new patch
xen/arch/x86/hvm/vmx/intr.c | 1 +
xen/arch/x86/hvm/vmx/vmcs.c | 1 +
xen/arch/x86/hvm/vmx/vmcs.h | 100 ++++++++++++++++++++
xen/arch/x86/hvm/vmx/vmx.c | 1 +
xen/arch/x86/hvm/vmx/vvmx.c | 1 +
xen/arch/x86/include/asm/hvm/vmx/vmcs.h | 118 +++++-------------------
6 files changed, 128 insertions(+), 94 deletions(-)
create mode 100644 xen/arch/x86/hvm/vmx/vmcs.h
diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
index 8431937f42..d8387e7215 100644
--- a/xen/arch/x86/hvm/vmx/intr.c
+++ b/xen/arch/x86/hvm/vmx/intr.c
@@ -38,6 +38,7 @@
#include <asm/hvm/trace.h>
#include <asm/vm_event.h>
+#include "vmcs.h"
#include "vmx.h"
#include "vvmx.h"
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 3d0f6be5bb..57e19e8dad 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -42,6 +42,7 @@
#include <asm/tboot.h>
#include <asm/apic.h>
+#include "vmcs.h"
#include "vmx.h"
#include "vvmx.h"
diff --git a/xen/arch/x86/hvm/vmx/vmcs.h b/xen/arch/x86/hvm/vmx/vmcs.h
new file mode 100644
index 0000000000..c0cca0ce73
--- /dev/null
+++ b/xen/arch/x86/hvm/vmx/vmcs.h
@@ -0,0 +1,100 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * vmcs.h: VMCS related definitions
+ *
+ * Copyright (c) 2004, Intel Corporation.
+ */
+
+#ifndef __X86_HVM_VMX_VMCS_PRIV_H__
+#define __X86_HVM_VMX_VMCS_PRIV_H__
+
+#include <xen/sched.h>
+#include <xen/types.h>
+
+#include <asm/hvm/vmx/vmcs.h>
+
+struct vmcs_struct {
+ uint32_t vmcs_revision_id;
+ unsigned char data [0]; /* vmcs size is read from MSR */
+};
+
+#define _VMX_DOMAIN_PML_ENABLED 0
+#define VMX_DOMAIN_PML_ENABLED (1ul << _VMX_DOMAIN_PML_ENABLED)
+
+/*
+ * Layout of the MSR bitmap, as interpreted by hardware:
+ * - *_low covers MSRs 0 to 0x1fff
+ * - *_ligh covers MSRs 0xc0000000 to 0xc0001fff
+ */
+struct vmx_msr_bitmap {
+ unsigned long read_low [0x2000 / BITS_PER_LONG];
+ unsigned long read_high [0x2000 / BITS_PER_LONG];
+ unsigned long write_low [0x2000 / BITS_PER_LONG];
+ unsigned long write_high[0x2000 / BITS_PER_LONG];
+};
+
+#define NR_PML_ENTRIES 512
+
+void vmcs_dump_vcpu(struct vcpu *v);
+int vmx_vmcs_init(void);
+int cf_check vmx_cpu_up_prepare(unsigned int cpu);
+void cf_check vmx_cpu_dead(unsigned int cpu);
+int cf_check vmx_cpu_up(void);
+void cf_check vmx_cpu_down(void);
+
+int vmx_create_vmcs(struct vcpu *v);
+void vmx_destroy_vmcs(struct vcpu *v);
+bool __must_check vmx_vmcs_try_enter(struct vcpu *v);
+void vmx_vmcs_reload(struct vcpu *v);
+
+void vmx_vmcs_switch(paddr_t from, paddr_t to);
+void vmx_set_eoi_exit_bitmap(struct vcpu *v, uint8_t vector);
+void vmx_clear_eoi_exit_bitmap(struct vcpu *v, uint8_t vector);
+bool vmx_msr_is_intercepted(struct vmx_msr_bitmap *msr_bitmap,
+ unsigned int msr, bool is_write) __nonnull(1);
+void virtual_vmcs_enter(const struct vcpu *);
+void virtual_vmcs_exit(const struct vcpu *);
+u64 virtual_vmcs_vmread(const struct vcpu *, uint32_t encoding);
+enum vmx_insn_errno virtual_vmcs_vmread_safe(const struct vcpu *v,
+ uint32_t vmcs_encoding,
+ uint64_t *val);
+void virtual_vmcs_vmwrite(const struct vcpu *, uint32_t encoding, uint64_t val);
+enum vmx_insn_errno virtual_vmcs_vmwrite_safe(const struct vcpu *v,
+ uint32_t vmcs_encoding,
+ uint64_t val);
+
+DECLARE_PER_CPU(bool, vmxon);
+
+bool vmx_vcpu_pml_enabled(const struct vcpu *v);
+int vmx_vcpu_enable_pml(struct vcpu *v);
+void vmx_vcpu_disable_pml(struct vcpu *v);
+void vmx_vcpu_flush_pml_buffer(struct vcpu *v);
+
+static inline int vmx_read_guest_loadonly_msr(
+ const struct vcpu *v, uint32_t msr, uint64_t *val)
+{
+ const struct vmx_msr_entry *ent =
+ vmx_find_msr(v, msr, VMX_MSR_GUEST_LOADONLY);
+
+ if ( !ent )
+ {
+ *val = 0;
+ return -ESRCH;
+ }
+
+ *val = ent->data;
+
+ return 0;
+}
+
+#endif /* __X86_HVM_VMX_VMCS_PRIV_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 73ab4e9816..c5f6902206 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -57,6 +57,7 @@
#include <public/hvm/ioreq.h>
#include "pi.h"
+#include "vmcs.h"
#include "vmx.h"
#include "vvmx.h"
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 0af5411076..d0a6fa2d20 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -29,6 +29,7 @@
#include <asm/hvm/vmx/vvmx.h>
#include <asm/hvm/nestedhvm.h>
+#include "vmcs.h"
#include "vmx.h"
#include "vvmx.h"
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
index 0a84e74478..47206b1e9d 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -20,22 +20,10 @@
#include <xen/mm.h>
-extern void vmcs_dump_vcpu(struct vcpu *v);
-extern int vmx_vmcs_init(void);
-int cf_check vmx_cpu_up_prepare(unsigned int cpu);
-void cf_check vmx_cpu_dead(unsigned int cpu);
-int cf_check vmx_cpu_up(void);
-void cf_check vmx_cpu_down(void);
-
-struct vmcs_struct {
- u32 vmcs_revision_id;
- unsigned char data [0]; /* vmcs size is read from MSR */
-};
-
struct vmx_msr_entry {
- u32 index;
- u32 mbz;
- u64 data;
+ uint32_t index;
+ uint32_t mbz;
+ uint64_t data;
};
#define EPT_DEFAULT_MT X86_MT_WB
@@ -49,14 +37,12 @@ struct ept_data {
:5, /* rsvd. */
mfn:52;
};
- u64 eptp;
+ uint64_t eptp;
};
/* Set of PCPUs needing an INVEPT before a VMENTER. */
cpumask_var_t invalidate;
};
-#define _VMX_DOMAIN_PML_ENABLED 0
-#define VMX_DOMAIN_PML_ENABLED (1ul << _VMX_DOMAIN_PML_ENABLED)
struct vmx_domain {
mfn_t apic_access_mfn;
/* VMX_DOMAIN_* */
@@ -69,36 +55,22 @@ struct vmx_domain {
bool exec_sp;
};
-/*
- * Layout of the MSR bitmap, as interpreted by hardware:
- * - *_low covers MSRs 0 to 0x1fff
- * - *_ligh covers MSRs 0xc0000000 to 0xc0001fff
- */
-struct vmx_msr_bitmap {
- unsigned long read_low [0x2000 / BITS_PER_LONG];
- unsigned long read_high [0x2000 / BITS_PER_LONG];
- unsigned long write_low [0x2000 / BITS_PER_LONG];
- unsigned long write_high[0x2000 / BITS_PER_LONG];
-};
-
struct pi_desc {
DECLARE_BITMAP(pir, X86_NR_VECTORS);
union {
struct {
- u16 on : 1, /* bit 256 - Outstanding Notification */
- sn : 1, /* bit 257 - Suppress Notification */
- rsvd_1 : 14; /* bit 271:258 - Reserved */
- u8 nv; /* bit 279:272 - Notification Vector */
- u8 rsvd_2; /* bit 287:280 - Reserved */
- u32 ndst; /* bit 319:288 - Notification Destination */
+ uint16_t on : 1, /* bit 256 - Outstanding Notification */
+ sn : 1, /* bit 257 - Suppress Notification */
+ rsvd_1 : 14; /* bit 271:258 - Reserved */
+ uint8_t nv; /* bit 279:272 - Notification Vector */
+ uint8_t rsvd_2; /* bit 287:280 - Reserved */
+ uint32_t ndst; /* bit 319:288 - Notification Destination */
};
- u64 control;
+ uint64_t control;
};
- u32 rsvd[6];
+ uint32_t rsvd[6];
} __attribute__ ((aligned (64)));
-#define NR_PML_ENTRIES 512
-
struct pi_blocking_vcpu {
struct list_head list;
spinlock_t *lock;
@@ -123,9 +95,9 @@ struct vmx_vcpu {
int launched;
/* Cache of cpu execution control. */
- u32 exec_control;
- u32 secondary_exec_control;
- u32 exception_bitmap;
+ uint32_t exec_control;
+ uint32_t secondary_exec_control;
+ uint32_t exception_bitmap;
uint64_t shadow_gs;
uint64_t star;
@@ -154,7 +126,7 @@ struct vmx_vcpu {
unsigned long host_cr0;
/* Do we need to tolerate a spurious EPT_MISCONFIG VM exit? */
- bool_t ept_spurious_misconfig;
+ bool ept_spurious_misconfig;
/* Processor Trace configured and enabled for the vcpu. */
bool ipt_active;
@@ -191,12 +163,8 @@ struct vmx_vcpu {
struct pi_blocking_vcpu pi_blocking;
};
-int vmx_create_vmcs(struct vcpu *v);
-void vmx_destroy_vmcs(struct vcpu *v);
void vmx_vmcs_enter(struct vcpu *v);
-bool_t __must_check vmx_vmcs_try_enter(struct vcpu *v);
void vmx_vmcs_exit(struct vcpu *v);
-void vmx_vmcs_reload(struct vcpu *v);
#define CPU_BASED_VIRTUAL_INTR_PENDING 0x00000004
#define CPU_BASED_USE_TSC_OFFSETING 0x00000008
@@ -219,14 +187,14 @@ void vmx_vmcs_reload(struct vcpu *v);
#define CPU_BASED_MONITOR_EXITING 0x20000000
#define CPU_BASED_PAUSE_EXITING 0x40000000
#define CPU_BASED_ACTIVATE_SECONDARY_CONTROLS 0x80000000
-extern u32 vmx_cpu_based_exec_control;
+extern uint32_t vmx_cpu_based_exec_control;
#define PIN_BASED_EXT_INTR_MASK 0x00000001
#define PIN_BASED_NMI_EXITING 0x00000008
#define PIN_BASED_VIRTUAL_NMIS 0x00000020
#define PIN_BASED_PREEMPT_TIMER 0x00000040
#define PIN_BASED_POSTED_INTERRUPT 0x00000080
-extern u32 vmx_pin_based_exec_control;
+extern uint32_t vmx_pin_based_exec_control;
#define VM_EXIT_SAVE_DEBUG_CNTRLS 0x00000004
#define VM_EXIT_IA32E_MODE 0x00000200
@@ -238,7 +206,7 @@ extern u32 vmx_pin_based_exec_control;
#define VM_EXIT_LOAD_HOST_EFER 0x00200000
#define VM_EXIT_SAVE_PREEMPT_TIMER 0x00400000
#define VM_EXIT_CLEAR_BNDCFGS 0x00800000
-extern u32 vmx_vmexit_control;
+extern uint32_t vmx_vmexit_control;
#define VM_ENTRY_IA32E_MODE 0x00000200
#define VM_ENTRY_SMM 0x00000400
@@ -247,7 +215,7 @@ extern u32 vmx_vmexit_control;
#define VM_ENTRY_LOAD_GUEST_PAT 0x00004000
#define VM_ENTRY_LOAD_GUEST_EFER 0x00008000
#define VM_ENTRY_LOAD_BNDCFGS 0x00010000
-extern u32 vmx_vmentry_control;
+extern uint32_t vmx_vmentry_control;
#define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
#define SECONDARY_EXEC_ENABLE_EPT 0x00000002
@@ -269,7 +237,7 @@ extern u32 vmx_vmentry_control;
#define SECONDARY_EXEC_TSC_SCALING 0x02000000
#define SECONDARY_EXEC_BUS_LOCK_DETECTION 0x40000000
#define SECONDARY_EXEC_NOTIFY_VM_EXITING 0x80000000
-extern u32 vmx_secondary_exec_control;
+extern uint32_t vmx_secondary_exec_control;
#define VMX_EPT_EXEC_ONLY_SUPPORTED 0x00000001
#define VMX_EPT_WALK_LENGTH_4_SUPPORTED 0x00000040
@@ -286,7 +254,7 @@ extern u32 vmx_secondary_exec_control;
#define VMX_VPID_INVVPID_SINGLE_CONTEXT 0x20000000000ULL
#define VMX_VPID_INVVPID_ALL_CONTEXT 0x40000000000ULL
#define VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL 0x80000000000ULL
-extern u64 vmx_ept_vpid_cap;
+extern uint64_t vmx_ept_vpid_cap;
#define VMX_MISC_PROC_TRACE 0x00004000
#define VMX_MISC_CR3_TARGET 0x01ff0000
@@ -373,7 +341,7 @@ extern u64 vmx_ept_vpid_cap;
*/
#define VMX_BASIC_DEFAULT1_ZERO (1ULL << 55)
-extern u64 vmx_basic_msr;
+extern uint64_t vmx_basic_msr;
#define cpu_has_vmx_ins_outs_instr_info \
(!!(vmx_basic_msr & VMX_BASIC_INS_OUT_INFO))
@@ -614,23 +582,6 @@ static inline int vmx_read_guest_msr(const struct vcpu *v, uint32_t msr,
return 0;
}
-static inline int vmx_read_guest_loadonly_msr(
- const struct vcpu *v, uint32_t msr, uint64_t *val)
-{
- const struct vmx_msr_entry *ent =
- vmx_find_msr(v, msr, VMX_MSR_GUEST_LOADONLY);
-
- if ( !ent )
- {
- *val = 0;
- return -ESRCH;
- }
-
- *val = ent->data;
-
- return 0;
-}
-
static inline int vmx_write_guest_msr(struct vcpu *v, uint32_t msr,
uint64_t val)
{
@@ -644,7 +595,6 @@ static inline int vmx_write_guest_msr(struct vcpu *v, uint32_t msr,
return 0;
}
-
/* MSR intercept bitmap infrastructure. */
enum vmx_msr_intercept_type {
VMX_MSR_R = 1,
@@ -656,27 +606,7 @@ void vmx_clear_msr_intercept(struct vcpu *v, unsigned int msr,
enum vmx_msr_intercept_type type);
void vmx_set_msr_intercept(struct vcpu *v, unsigned int msr,
enum vmx_msr_intercept_type type);
-void vmx_vmcs_switch(paddr_t from, paddr_t to);
-void vmx_set_eoi_exit_bitmap(struct vcpu *v, u8 vector);
-void vmx_clear_eoi_exit_bitmap(struct vcpu *v, u8 vector);
-bool vmx_msr_is_intercepted(struct vmx_msr_bitmap *msr_bitmap,
- unsigned int msr, bool is_write) __nonnull(1);
-void virtual_vmcs_enter(const struct vcpu *);
-void virtual_vmcs_exit(const struct vcpu *);
-u64 virtual_vmcs_vmread(const struct vcpu *, u32 encoding);
-enum vmx_insn_errno virtual_vmcs_vmread_safe(const struct vcpu *v,
- u32 vmcs_encoding, u64 *val);
-void virtual_vmcs_vmwrite(const struct vcpu *, u32 encoding, u64 val);
-enum vmx_insn_errno virtual_vmcs_vmwrite_safe(const struct vcpu *v,
- u32 vmcs_encoding, u64 val);
-
-DECLARE_PER_CPU(bool_t, vmxon);
-
-bool_t vmx_vcpu_pml_enabled(const struct vcpu *v);
-int vmx_vcpu_enable_pml(struct vcpu *v);
-void vmx_vcpu_disable_pml(struct vcpu *v);
-void vmx_vcpu_flush_pml_buffer(struct vcpu *v);
-bool_t vmx_domain_pml_enabled(const struct domain *d);
+bool vmx_domain_pml_enabled(const struct domain *d);
int vmx_domain_enable_pml(struct domain *d);
void vmx_domain_disable_pml(struct domain *d);
void vmx_domain_flush_pml_buffers(struct domain *d);
--
2.37.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-24 18:49 [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Xenia Ragiadakou
` (13 preceding siblings ...)
2023-02-24 18:50 ` [PATCH v3 14/14] x86/vmx: move vmcs " Xenia Ragiadakou
@ 2023-02-24 19:29 ` Andrew Cooper
2023-02-24 20:08 ` Xenia Ragiadakou
14 siblings, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2023-02-24 19:29 UTC (permalink / raw)
To: Xenia Ragiadakou, xen-devel
Cc: Jan Beulich, Roger Pau Monné, Wei Liu, Jun Nakajima, Kevin Tian
On 24/02/2023 6:49 pm, Xenia Ragiadakou wrote:
> There are more detailed per-patch changesets.
>
> Xenia Ragiadakou (14):
> x86/svm: move declarations used only by svm code from svm.h to private
> header
> x86/svm: make asid.h private
> x86/svm: delete header asm/hvm/svm/intr.h
Thankyou for this work. I've acked and committed patches 1 and 3.
Note that you had one hunk in patch 5 (whitespace correction in
svm_invlpga) that obviously should have been in patch 1, so I've moved it.
Looking at asid.h, I still can't bare to keep it even in that state, so
I've constructed an alternative which I'll email out in a moment.
~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-24 19:29 ` [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup Andrew Cooper
@ 2023-02-24 20:08 ` Xenia Ragiadakou
2023-02-24 21:33 ` Andrew Cooper
0 siblings, 1 reply; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 20:08 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: Jan Beulich, Roger Pau Monné, Wei Liu, Jun Nakajima, Kevin Tian
On 2/24/23 21:29, Andrew Cooper wrote:
> On 24/02/2023 6:49 pm, Xenia Ragiadakou wrote:
>> There are more detailed per-patch changesets.
>>
>> Xenia Ragiadakou (14):
>> x86/svm: move declarations used only by svm code from svm.h to private
>> header
>> x86/svm: make asid.h private
>> x86/svm: delete header asm/hvm/svm/intr.h
>
> Thankyou for this work. I've acked and committed patches 1 and 3.
>
> Note that you had one hunk in patch 5 (whitespace correction in
> svm_invlpga) that obviously should have been in patch 1, so I've moved it.
Thanks, I missed that ...
>
> Looking at asid.h, I still can't bare to keep it even in that state, so
> I've constructed an alternative which I'll email out in a moment.
I 'm also in favor of less headers.
>
> ~Andrew
--
Xenia
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-24 20:08 ` Xenia Ragiadakou
@ 2023-02-24 21:33 ` Andrew Cooper
2023-02-24 23:39 ` Xenia Ragiadakou
2023-02-27 10:46 ` Jan Beulich
0 siblings, 2 replies; 45+ messages in thread
From: Andrew Cooper @ 2023-02-24 21:33 UTC (permalink / raw)
To: Xenia Ragiadakou, xen-devel
Cc: Jan Beulich, Roger Pau Monné, Wei Liu, Jun Nakajima, Kevin Tian
On 24/02/2023 8:08 pm, Xenia Ragiadakou wrote:
>
> On 2/24/23 21:29, Andrew Cooper wrote:
>> On 24/02/2023 6:49 pm, Xenia Ragiadakou wrote:
>>> There are more detailed per-patch changesets.
>>>
>>> Xenia Ragiadakou (14):
>>> x86/svm: move declarations used only by svm code from svm.h to
>>> private
>>> header
>>> x86/svm: make asid.h private
>>> x86/svm: delete header asm/hvm/svm/intr.h
>>
>> Thankyou for this work. I've acked and committed patches 1 and 3.
>>
>> Note that you had one hunk in patch 5 (whitespace correction in
>> svm_invlpga) that obviously should have been in patch 1, so I've
>> moved it.
>
> Thanks, I missed that ...
>
>>
>> Looking at asid.h, I still can't bare to keep it even in that state, so
>> I've constructed an alternative which I'll email out in a moment.
>
> I 'm also in favor of less headers.
I've committed as far as the nestedhvm move. At that point, I've sent
out a small patch to try and help decouple later changes.
But I think we want to change tact slightly at this point, so I'm not
going to do any further tweaking on commit.
Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
updating the non-SVM include paths as we go. Probably best to
chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
only got one tree-wide cleanup of the external include paths.
Quick tangent - I will be making all of that cpu_has_svm_*
infrastructure disappear by moving it into the normal CPUID handling,
but I've not had sufficient time to finish that yet.
Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
disappear (after my decoupling patch has gone in).
After that, hvm/svm/vmcb.h can be cleanly split in half. struct
svm_{domain,vcpu} can move sideways into hvm/svm.h (with a forward
declaration of vmcb_struct), as can the svm msr intercept declarations.
Everything else can move to being a private vmcb.h
Finally your svmdebug.h can come at the end, pretty much in the same
shape it is now. One thing to say is that right now, you've left enum
vmcb_sync_state public, but it's type is already decoupled by virtue of
struct svm_vcpu having a uint8_t field rather than an enum field.
And I think that cleanly gets rid of the entire asm/hvm/svm/* dir, which
is a great position to get to.
Beyond that, you will want to clean up the hvm msr intercept handling as
hvm_funcs, which I think will decouple the vpmu files from svm/vmx
specifically, but that will want to be a separate series.
How does this sound?
Thanks,
~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-24 21:33 ` Andrew Cooper
@ 2023-02-24 23:39 ` Xenia Ragiadakou
2023-02-27 10:46 ` Jan Beulich
1 sibling, 0 replies; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-24 23:39 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: Jan Beulich, Roger Pau Monné, Wei Liu, Jun Nakajima, Kevin Tian
On 2/24/23 23:33, Andrew Cooper wrote:
> On 24/02/2023 8:08 pm, Xenia Ragiadakou wrote:
>>
>> On 2/24/23 21:29, Andrew Cooper wrote:
>>> On 24/02/2023 6:49 pm, Xenia Ragiadakou wrote:
>>>> There are more detailed per-patch changesets.
>>>>
>>>> Xenia Ragiadakou (14):
>>>> x86/svm: move declarations used only by svm code from svm.h to
>>>> private
>>>> header
>>>> x86/svm: make asid.h private
>>>> x86/svm: delete header asm/hvm/svm/intr.h
>>>
>>> Thankyou for this work. I've acked and committed patches 1 and 3.
>>>
>>> Note that you had one hunk in patch 5 (whitespace correction in
>>> svm_invlpga) that obviously should have been in patch 1, so I've
>>> moved it.
>>
>> Thanks, I missed that ...
>>
>>>
>>> Looking at asid.h, I still can't bare to keep it even in that state, so
>>> I've constructed an alternative which I'll email out in a moment.
>>
>> I 'm also in favor of less headers.
>
> I've committed as far as the nestedhvm move. At that point, I've sent
> out a small patch to try and help decouple later changes.
>
> But I think we want to change tact slightly at this point, so I'm not
> going to do any further tweaking on commit.
>
> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
> updating the non-SVM include paths as we go. Probably best to
> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
> only got one tree-wide cleanup of the external include paths.
>
> Quick tangent - I will be making all of that cpu_has_svm_*
> infrastructure disappear by moving it into the normal CPUID handling,
> but I've not had sufficient time to finish that yet.
>
> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
> disappear (after my decoupling patch has gone in).
>
> After that, hvm/svm/vmcb.h can be cleanly split in half. struct
> svm_{domain,vcpu} can move sideways into hvm/svm.h (with a forward
> declaration of vmcb_struct), as can the svm msr intercept declarations.
> Everything else can move to being a private vmcb.h
>
> Finally your svmdebug.h can come at the end, pretty much in the same
> shape it is now. One thing to say is that right now, you've left enum
> vmcb_sync_state public, but it's type is already decoupled by virtue of
> struct svm_vcpu having a uint8_t field rather than an enum field.
>
> And I think that cleanly gets rid of the entire asm/hvm/svm/* dir, which
> is a great position to get to.
>
>
> Beyond that, you will want to clean up the hvm msr intercept handling as
> hvm_funcs, which I think will decouple the vpmu files from svm/vmx
> specifically, but that will want to be a separate series.
>
> How does this sound?
Thanks for the review. I think it sounds good.
The last part i.e the hvm msr intercept handling as hvm_funcs, I have it
already I just didn't have the chance to send it yet (because the
changes affect {svm,vmx}.h). I will rebase and send it on Monday for
review to verify that we are on the same page.
>
> Thanks,
>
> ~Andrew
--
Xenia
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-24 21:33 ` Andrew Cooper
2023-02-24 23:39 ` Xenia Ragiadakou
@ 2023-02-27 10:46 ` Jan Beulich
2023-02-27 11:15 ` Andrew Cooper
1 sibling, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2023-02-27 10:46 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné,
Wei Liu, Jun Nakajima, Kevin Tian, Xenia Ragiadakou, xen-devel
On 24.02.2023 22:33, Andrew Cooper wrote:
> But I think we want to change tact slightly at this point, so I'm not
> going to do any further tweaking on commit.
>
> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
> updating the non-SVM include paths as we go. Probably best to
> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
> only got one tree-wide cleanup of the external include paths.
>
> Quick tangent - I will be making all of that cpu_has_svm_*
> infrastructure disappear by moving it into the normal CPUID handling,
> but I've not had sufficient time to finish that yet.
>
> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
> disappear (after my decoupling patch has gone in).
Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h?
The latter doesn't use anything from the former, does it? And it
also doesn't include it right now. Since nested is still far from
being properly functional, and since most guests don't use it, I'd
rather see struct nestedvcpu's union to become a union of two
pointers, which get space allocated only for nested-enabled guests.
Yet it's only the use there which makes it necessary to globally
expose the struct right now.
Jan
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-27 10:46 ` Jan Beulich
@ 2023-02-27 11:15 ` Andrew Cooper
2023-02-27 11:33 ` Jan Beulich
0 siblings, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2023-02-27 11:15 UTC (permalink / raw)
To: Jan Beulich
Cc: Roger Pau Monné,
Wei Liu, Jun Nakajima, Kevin Tian, Xenia Ragiadakou, xen-devel
On 27/02/2023 10:46 am, Jan Beulich wrote:
> On 24.02.2023 22:33, Andrew Cooper wrote:
>> But I think we want to change tact slightly at this point, so I'm not
>> going to do any further tweaking on commit.
>>
>> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
>> updating the non-SVM include paths as we go. Probably best to
>> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
>> only got one tree-wide cleanup of the external include paths.
>>
>> Quick tangent - I will be making all of that cpu_has_svm_*
>> infrastructure disappear by moving it into the normal CPUID handling,
>> but I've not had sufficient time to finish that yet.
>>
>> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
>> disappear (after my decoupling patch has gone in).
> Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h?
> The latter doesn't use anything from the former, does it?
It's about what else uses them.
hvm_vcpu needs both svm_vcpu and nestedsvm, so both headers are always
included in tandem.
nestedsvm is literally just one struct now, and no subsystem ought to
have multiple headers when one will do.
The resulting "unified" svm.h will have very little in it, but
everything in it (including nestedsvm) should be there.
~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-27 11:15 ` Andrew Cooper
@ 2023-02-27 11:33 ` Jan Beulich
2023-02-27 12:06 ` Andrew Cooper
0 siblings, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2023-02-27 11:33 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné,
Wei Liu, Jun Nakajima, Kevin Tian, Xenia Ragiadakou, xen-devel
On 27.02.2023 12:15, Andrew Cooper wrote:
> On 27/02/2023 10:46 am, Jan Beulich wrote:
>> On 24.02.2023 22:33, Andrew Cooper wrote:
>>> But I think we want to change tact slightly at this point, so I'm not
>>> going to do any further tweaking on commit.
>>>
>>> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
>>> updating the non-SVM include paths as we go. Probably best to
>>> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
>>> only got one tree-wide cleanup of the external include paths.
>>>
>>> Quick tangent - I will be making all of that cpu_has_svm_*
>>> infrastructure disappear by moving it into the normal CPUID handling,
>>> but I've not had sufficient time to finish that yet.
>>>
>>> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
>>> disappear (after my decoupling patch has gone in).
>> Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h?
>> The latter doesn't use anything from the former, does it?
>
> It's about what else uses them.
>
> hvm_vcpu needs both svm_vcpu and nestedsvm, so both headers are always
> included in tandem.
Well, yes, that's how things are today. But can you explain to me why
hvm_vcpu has to know nestedsvm's layout? If the field was a pointer,
a forward decl of that struct would suffice, and any entity in the
rest of Xen not caring about struct nestedsvm would no longer see it
(and hence also no longer be re-built if a change is made there).
> nestedsvm is literally just one struct now, and no subsystem ought to
> have multiple headers when one will do.
When one will do, yes. Removing build dependencies is a good reason
to have separate headers, though.
Jan
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-27 11:33 ` Jan Beulich
@ 2023-02-27 12:06 ` Andrew Cooper
2023-02-27 12:17 ` Jan Beulich
0 siblings, 1 reply; 45+ messages in thread
From: Andrew Cooper @ 2023-02-27 12:06 UTC (permalink / raw)
To: Jan Beulich
Cc: Roger Pau Monné,
Wei Liu, Jun Nakajima, Kevin Tian, Xenia Ragiadakou, xen-devel
On 27/02/2023 11:33 am, Jan Beulich wrote:
> On 27.02.2023 12:15, Andrew Cooper wrote:
>> On 27/02/2023 10:46 am, Jan Beulich wrote:
>>> On 24.02.2023 22:33, Andrew Cooper wrote:
>>>> But I think we want to change tact slightly at this point, so I'm not
>>>> going to do any further tweaking on commit.
>>>>
>>>> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
>>>> updating the non-SVM include paths as we go. Probably best to
>>>> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
>>>> only got one tree-wide cleanup of the external include paths.
>>>>
>>>> Quick tangent - I will be making all of that cpu_has_svm_*
>>>> infrastructure disappear by moving it into the normal CPUID handling,
>>>> but I've not had sufficient time to finish that yet.
>>>>
>>>> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
>>>> disappear (after my decoupling patch has gone in).
>>> Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h?
>>> The latter doesn't use anything from the former, does it?
>> It's about what else uses them.
>>
>> hvm_vcpu needs both svm_vcpu and nestedsvm, so both headers are always
>> included in tandem.
> Well, yes, that's how things are today. But can you explain to me why
> hvm_vcpu has to know nestedsvm's layout?
Because it's part of the same single memory allocation.
> If the field was a pointer,
> a forward decl of that struct would suffice, and any entity in the
> rest of Xen not caring about struct nestedsvm would no longer see it
> (and hence also no longer be re-built if a change is made there).
Yes, you could hide it as a pointer. The cost of doing so is an
unnecessary extra memory allocation, and extra pointer handling on
create/destroy, not to mention extra pointer chasing in memory during use.
>> nestedsvm is literally just one struct now, and no subsystem ought to
>> have multiple headers when one will do.
> When one will do, yes. Removing build dependencies is a good reason
> to have separate headers, though.
Its not the only only option, and an #ifdef CONFIG_NESTED_VIRT inside
the struct would be an equally acceptable way of doing this which
wouldn't involve making an extra memory allocation.
Everything you've posed here is way out of scope for Xenia's series.
You are welcome to try and make the changes in the future if you want,
but you're going to have a uphill battle trying to justify it as a
sensible move.
~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-27 12:06 ` Andrew Cooper
@ 2023-02-27 12:17 ` Jan Beulich
2023-02-28 7:09 ` Xenia Ragiadakou
0 siblings, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2023-02-27 12:17 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné,
Wei Liu, Jun Nakajima, Kevin Tian, Xenia Ragiadakou, xen-devel
On 27.02.2023 13:06, Andrew Cooper wrote:
> On 27/02/2023 11:33 am, Jan Beulich wrote:
>> On 27.02.2023 12:15, Andrew Cooper wrote:
>>> On 27/02/2023 10:46 am, Jan Beulich wrote:
>>>> On 24.02.2023 22:33, Andrew Cooper wrote:
>>>>> But I think we want to change tact slightly at this point, so I'm not
>>>>> going to do any further tweaking on commit.
>>>>>
>>>>> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
>>>>> updating the non-SVM include paths as we go. Probably best to
>>>>> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
>>>>> only got one tree-wide cleanup of the external include paths.
>>>>>
>>>>> Quick tangent - I will be making all of that cpu_has_svm_*
>>>>> infrastructure disappear by moving it into the normal CPUID handling,
>>>>> but I've not had sufficient time to finish that yet.
>>>>>
>>>>> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
>>>>> disappear (after my decoupling patch has gone in).
>>>> Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h?
>>>> The latter doesn't use anything from the former, does it?
>>> It's about what else uses them.
>>>
>>> hvm_vcpu needs both svm_vcpu and nestedsvm, so both headers are always
>>> included in tandem.
>> Well, yes, that's how things are today. But can you explain to me why
>> hvm_vcpu has to know nestedsvm's layout?
>
> Because it's part of the same single memory allocation.
Which keeps growing, and sooner or later we'll need to find something
again to split off, so we won't exceed a page in size. The nested
structures would, to me, look to be prime candidates for such.
>> If the field was a pointer,
>> a forward decl of that struct would suffice, and any entity in the
>> rest of Xen not caring about struct nestedsvm would no longer see it
>> (and hence also no longer be re-built if a change is made there).
>
> Yes, you could hide it as a pointer. The cost of doing so is an
> unnecessary extra memory allocation, and extra pointer handling on
> create/destroy, not to mention extra pointer chasing in memory during use.
>
>>> nestedsvm is literally just one struct now, and no subsystem ought to
>>> have multiple headers when one will do.
>> When one will do, yes. Removing build dependencies is a good reason
>> to have separate headers, though.
>
> Its not the only only option, and an #ifdef CONFIG_NESTED_VIRT inside
> the struct would be an equally acceptable way of doing this which
> wouldn't involve making an extra memory allocation.
That would make it a build-time decision, but then on NESTED_VIRT=y
hypervisors there might still be guests not meaning to use that
functionality, and for quite some time that may actually be a majority.
> Everything you've posed here is way out of scope for Xenia's series.
There was never an intention to extend the scope of the work she's doing.
Instead I was trying to limit the scope by suggesting to avoid a piece
of rework which later may want undoing.
Jan
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-27 12:17 ` Jan Beulich
@ 2023-02-28 7:09 ` Xenia Ragiadakou
2023-02-28 7:11 ` Jan Beulich
0 siblings, 1 reply; 45+ messages in thread
From: Xenia Ragiadakou @ 2023-02-28 7:09 UTC (permalink / raw)
To: Jan Beulich, Andrew Cooper
Cc: Roger Pau Monné, Wei Liu, Jun Nakajima, Kevin Tian, xen-devel
On 2/27/23 14:17, Jan Beulich wrote:
> On 27.02.2023 13:06, Andrew Cooper wrote:
>> On 27/02/2023 11:33 am, Jan Beulich wrote:
>>> On 27.02.2023 12:15, Andrew Cooper wrote:
>>>> On 27/02/2023 10:46 am, Jan Beulich wrote:
>>>>> On 24.02.2023 22:33, Andrew Cooper wrote:
>>>>>> But I think we want to change tact slightly at this point, so I'm not
>>>>>> going to do any further tweaking on commit.
>>>>>>
>>>>>> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
>>>>>> updating the non-SVM include paths as we go. Probably best to
>>>>>> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
>>>>>> only got one tree-wide cleanup of the external include paths.
>>>>>>
>>>>>> Quick tangent - I will be making all of that cpu_has_svm_*
>>>>>> infrastructure disappear by moving it into the normal CPUID handling,
>>>>>> but I've not had sufficient time to finish that yet.
>>>>>>
>>>>>> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
>>>>>> disappear (after my decoupling patch has gone in).
>>>>> Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h?
>>>>> The latter doesn't use anything from the former, does it?
>>>> It's about what else uses them.
>>>>
>>>> hvm_vcpu needs both svm_vcpu and nestedsvm, so both headers are always
>>>> included in tandem.
>>> Well, yes, that's how things are today. But can you explain to me why
>>> hvm_vcpu has to know nestedsvm's layout?
>>
>> Because it's part of the same single memory allocation.
>
> Which keeps growing, and sooner or later we'll need to find something
> again to split off, so we won't exceed a page in size. The nested
> structures would, to me, look to be prime candidates for such.
>
>>> If the field was a pointer,
>>> a forward decl of that struct would suffice, and any entity in the
>>> rest of Xen not caring about struct nestedsvm would no longer see it
>>> (and hence also no longer be re-built if a change is made there).
>>
>> Yes, you could hide it as a pointer. The cost of doing so is an
>> unnecessary extra memory allocation, and extra pointer handling on
>> create/destroy, not to mention extra pointer chasing in memory during use.
>>
>>>> nestedsvm is literally just one struct now, and no subsystem ought to
>>>> have multiple headers when one will do.
>>> When one will do, yes. Removing build dependencies is a good reason
>>> to have separate headers, though.
>>
>> Its not the only only option, and an #ifdef CONFIG_NESTED_VIRT inside
>> the struct would be an equally acceptable way of doing this which
>> wouldn't involve making an extra memory allocation.
>
> That would make it a build-time decision, but then on NESTED_VIRT=y
> hypervisors there might still be guests not meaning to use that
> functionality, and for quite some time that may actually be a majority.
>
>> Everything you've posed here is way out of scope for Xenia's series.
>
> There was never an intention to extend the scope of the work she's doing.
> Instead I was trying to limit the scope by suggesting to avoid a piece
> of rework which later may want undoing.
Can I suggest to leave hvm/svm/svm.h and hvm/svm/nestedsvm.h separate
for now?
>
> Jan
--
Xenia
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-28 7:09 ` Xenia Ragiadakou
@ 2023-02-28 7:11 ` Jan Beulich
2023-03-01 13:05 ` Andrew Cooper
0 siblings, 1 reply; 45+ messages in thread
From: Jan Beulich @ 2023-02-28 7:11 UTC (permalink / raw)
To: Xenia Ragiadakou
Cc: Roger Pau Monné,
Wei Liu, Jun Nakajima, Kevin Tian, xen-devel, Andrew Cooper
On 28.02.2023 08:09, Xenia Ragiadakou wrote:
>
> On 2/27/23 14:17, Jan Beulich wrote:
>> On 27.02.2023 13:06, Andrew Cooper wrote:
>>> On 27/02/2023 11:33 am, Jan Beulich wrote:
>>>> On 27.02.2023 12:15, Andrew Cooper wrote:
>>>>> On 27/02/2023 10:46 am, Jan Beulich wrote:
>>>>>> On 24.02.2023 22:33, Andrew Cooper wrote:
>>>>>>> But I think we want to change tact slightly at this point, so I'm not
>>>>>>> going to do any further tweaking on commit.
>>>>>>>
>>>>>>> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
>>>>>>> updating the non-SVM include paths as we go. Probably best to
>>>>>>> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
>>>>>>> only got one tree-wide cleanup of the external include paths.
>>>>>>>
>>>>>>> Quick tangent - I will be making all of that cpu_has_svm_*
>>>>>>> infrastructure disappear by moving it into the normal CPUID handling,
>>>>>>> but I've not had sufficient time to finish that yet.
>>>>>>>
>>>>>>> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
>>>>>>> disappear (after my decoupling patch has gone in).
>>>>>> Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h?
>>>>>> The latter doesn't use anything from the former, does it?
>>>>> It's about what else uses them.
>>>>>
>>>>> hvm_vcpu needs both svm_vcpu and nestedsvm, so both headers are always
>>>>> included in tandem.
>>>> Well, yes, that's how things are today. But can you explain to me why
>>>> hvm_vcpu has to know nestedsvm's layout?
>>>
>>> Because it's part of the same single memory allocation.
>>
>> Which keeps growing, and sooner or later we'll need to find something
>> again to split off, so we won't exceed a page in size. The nested
>> structures would, to me, look to be prime candidates for such.
>>
>>>> If the field was a pointer,
>>>> a forward decl of that struct would suffice, and any entity in the
>>>> rest of Xen not caring about struct nestedsvm would no longer see it
>>>> (and hence also no longer be re-built if a change is made there).
>>>
>>> Yes, you could hide it as a pointer. The cost of doing so is an
>>> unnecessary extra memory allocation, and extra pointer handling on
>>> create/destroy, not to mention extra pointer chasing in memory during use.
>>>
>>>>> nestedsvm is literally just one struct now, and no subsystem ought to
>>>>> have multiple headers when one will do.
>>>> When one will do, yes. Removing build dependencies is a good reason
>>>> to have separate headers, though.
>>>
>>> Its not the only only option, and an #ifdef CONFIG_NESTED_VIRT inside
>>> the struct would be an equally acceptable way of doing this which
>>> wouldn't involve making an extra memory allocation.
>>
>> That would make it a build-time decision, but then on NESTED_VIRT=y
>> hypervisors there might still be guests not meaning to use that
>> functionality, and for quite some time that may actually be a majority.
>>
>>> Everything you've posed here is way out of scope for Xenia's series.
>>
>> There was never an intention to extend the scope of the work she's doing.
>> Instead I was trying to limit the scope by suggesting to avoid a piece
>> of rework which later may want undoing.
>
> Can I suggest to leave hvm/svm/svm.h and hvm/svm/nestedsvm.h separate
> for now?
As per before - that's my preference. It'll be Andrew who you would need
to convince, as he did suggest the folding.
Jan
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
2023-02-28 7:11 ` Jan Beulich
@ 2023-03-01 13:05 ` Andrew Cooper
0 siblings, 0 replies; 45+ messages in thread
From: Andrew Cooper @ 2023-03-01 13:05 UTC (permalink / raw)
To: Jan Beulich, Xenia Ragiadakou
Cc: Roger Pau Monné, Wei Liu, Jun Nakajima, Kevin Tian, xen-devel
On 28/02/2023 7:11 am, Jan Beulich wrote:
> On 28.02.2023 08:09, Xenia Ragiadakou wrote:
>> On 2/27/23 14:17, Jan Beulich wrote:
>>> On 27.02.2023 13:06, Andrew Cooper wrote:
>>>> On 27/02/2023 11:33 am, Jan Beulich wrote:
>>>>> On 27.02.2023 12:15, Andrew Cooper wrote:
>>>>>> On 27/02/2023 10:46 am, Jan Beulich wrote:
>>>>>>> On 24.02.2023 22:33, Andrew Cooper wrote:
>>>>>>>> But I think we want to change tact slightly at this point, so I'm not
>>>>>>>> going to do any further tweaking on commit.
>>>>>>>>
>>>>>>>> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
>>>>>>>> updating the non-SVM include paths as we go. Probably best to
>>>>>>>> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
>>>>>>>> only got one tree-wide cleanup of the external include paths.
>>>>>>>>
>>>>>>>> Quick tangent - I will be making all of that cpu_has_svm_*
>>>>>>>> infrastructure disappear by moving it into the normal CPUID handling,
>>>>>>>> but I've not had sufficient time to finish that yet.
>>>>>>>>
>>>>>>>> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
>>>>>>>> disappear (after my decoupling patch has gone in).
>>>>>>> Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h?
>>>>>>> The latter doesn't use anything from the former, does it?
>>>>>> It's about what else uses them.
>>>>>>
>>>>>> hvm_vcpu needs both svm_vcpu and nestedsvm, so both headers are always
>>>>>> included in tandem.
>>>>> Well, yes, that's how things are today. But can you explain to me why
>>>>> hvm_vcpu has to know nestedsvm's layout?
>>>> Because it's part of the same single memory allocation.
>>> Which keeps growing, and sooner or later we'll need to find something
>>> again to split off, so we won't exceed a page in size. The nested
>>> structures would, to me, look to be prime candidates for such.
>>>
>>>>> If the field was a pointer,
>>>>> a forward decl of that struct would suffice, and any entity in the
>>>>> rest of Xen not caring about struct nestedsvm would no longer see it
>>>>> (and hence also no longer be re-built if a change is made there).
>>>> Yes, you could hide it as a pointer. The cost of doing so is an
>>>> unnecessary extra memory allocation, and extra pointer handling on
>>>> create/destroy, not to mention extra pointer chasing in memory during use.
>>>>
>>>>>> nestedsvm is literally just one struct now, and no subsystem ought to
>>>>>> have multiple headers when one will do.
>>>>> When one will do, yes. Removing build dependencies is a good reason
>>>>> to have separate headers, though.
>>>> Its not the only only option, and an #ifdef CONFIG_NESTED_VIRT inside
>>>> the struct would be an equally acceptable way of doing this which
>>>> wouldn't involve making an extra memory allocation.
>>> That would make it a build-time decision, but then on NESTED_VIRT=y
>>> hypervisors there might still be guests not meaning to use that
>>> functionality, and for quite some time that may actually be a majority.
>>>
>>>> Everything you've posed here is way out of scope for Xenia's series.
>>> There was never an intention to extend the scope of the work she's doing.
>>> Instead I was trying to limit the scope by suggesting to avoid a piece
>>> of rework which later may want undoing.
>> Can I suggest to leave hvm/svm/svm.h and hvm/svm/nestedsvm.h separate
>> for now?
> As per before - that's my preference. It'll be Andrew who you would need
> to convince, as he did suggest the folding.
Please fold them.
I have strong doubts that they would actually be unfolded, even if we
did want to make nested virt a build time choice.
(I'm not actually convinced that the existing nestedvcpu structure will
survive first-pass design of a working nested virt solution.)
~Andrew
^ permalink raw reply [flat|nested] 45+ messages in thread