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.2On 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
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
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
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.2On 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
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
© 2016 - 2026 Red Hat, Inc.