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.
The function svm_invlpga() stays in arch/x86/hvm/svm/svm.h because it is used
by arch/x86/hvm/svm/asid.h.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v2:
- new patch, the creation of a private header was suggested by Andrew and Jan
I have not added #ifndef guards as it is a private and it should not be
included by other headers. However, this is considered a MISRA-C violation
... I 'm not sure what to do.
xen/arch/x86/hvm/svm/nestedsvm.c | 1 +
xen/arch/x86/hvm/svm/svm.c | 2 ++
xen/arch/x86/hvm/svm/svm.h | 40 ++++++++++++++++++++++++++
xen/arch/x86/include/asm/hvm/svm/svm.h | 29 -------------------
4 files changed, 43 insertions(+), 29 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..b2ec293078
--- /dev/null
+++ b/xen/arch/x86/hvm/svm/svm.h
@@ -0,0 +1,40 @@
+#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" );
+}
+
+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);
+
+/* 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)
+
+/*
+ * 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 c62d0caa32..254de55ee9 100644
--- a/xen/arch/x86/include/asm/hvm/svm/svm.h
+++ b/xen/arch/x86/include/asm/hvm/svm/svm.h
@@ -22,20 +22,6 @@
#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 (
@@ -45,11 +31,6 @@ static inline void svm_invlpga(unsigned long linear, uint32_t asid)
"a" (linear), "c" (asid));
}
-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);
-
/*
* PV context switch helpers. Prefetching the VMCB area itself has been shown
* to be useful for performance.
@@ -95,14 +76,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.2On 22/02/2023 12:00 pm, Xenia Ragiadakou wrote:
> 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.
>
> The function svm_invlpga() stays in arch/x86/hvm/svm/svm.h because it is used
> by arch/x86/hvm/svm/asid.h.
I'm reasonably sure that all headers in arch/x86/hvm/svm/ other than
svm.h can move to being private easily.
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> ---
>
> Changes in v2:
> - new patch, the creation of a private header was suggested by Andrew and Jan
>
> I have not added #ifndef guards as it is a private and it should not be
> included by other headers. However, this is considered a MISRA-C violation
> ... I 'm not sure what to do.
Always have guards. Firstly because that is the decision taken by the
MISRA working group.
But more importantly, because life is too short to deal with the
shooting yourself in the foot which will occur from trying to take
shortcuts like these.
> diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
> new file mode 100644
> index 0000000000..b2ec293078
> --- /dev/null
> +++ b/xen/arch/x86/hvm/svm/svm.h
> @@ -0,0 +1,40 @@
> +#include <xen/types.h>
Elsewhere, we're retrofitting SPDX tags to all source files, but we're
already putting tags in new source files.
This one needs to be /* SPDX-License-Identifier: GPL-2.0 */ I think.
> +
> +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" );
> +}
> +
> +struct cpu_user_regs;
Looking at this, you could fold patch 1 into this patch and reduce the
net churn. It would be fine to say "delete used forward declaration" in
the commit message, seeing as you're deleting that block of code anyway
from svm.h
If you want to do this, then I'll skip committing patch 1.
~Andrew
On 2/23/23 13:16, Andrew Cooper wrote:
> On 22/02/2023 12:00 pm, Xenia Ragiadakou wrote:
>> 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.
>>
>> The function svm_invlpga() stays in arch/x86/hvm/svm/svm.h because it is used
>> by arch/x86/hvm/svm/asid.h.
>
> I'm reasonably sure that all headers in arch/x86/hvm/svm/ other than
> svm.h can move to being private easily.
>
>>
>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>> ---
>>
>> Changes in v2:
>> - new patch, the creation of a private header was suggested by Andrew and Jan
>>
>> I have not added #ifndef guards as it is a private and it should not be
>> included by other headers. However, this is considered a MISRA-C violation
>> ... I 'm not sure what to do.
>
> Always have guards. Firstly because that is the decision taken by the
> MISRA working group.
>
> But more importantly, because life is too short to deal with the
> shooting yourself in the foot which will occur from trying to take
> shortcuts like these.
>
>
>> diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
>> new file mode 100644
>> index 0000000000..b2ec293078
>> --- /dev/null
>> +++ b/xen/arch/x86/hvm/svm/svm.h
>> @@ -0,0 +1,40 @@
>> +#include <xen/types.h>
>
> Elsewhere, we're retrofitting SPDX tags to all source files, but we're
> already putting tags in new source files.
>
> This one needs to be /* SPDX-License-Identifier: GPL-2.0 */ I think.
>
>> +
>> +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" );
>> +}
>> +
>> +struct cpu_user_regs;
>
> Looking at this, you could fold patch 1 into this patch and reduce the
> net churn. It would be fine to say "delete used forward declaration" in
> the commit message, seeing as you're deleting that block of code anyway
> from svm.h
>
> If you want to do this, then I'll skip committing patch 1.
Yes I will do it.
>
> ~Andrew
--
Xenia
On 22.02.2023 13:00, Xenia Ragiadakou wrote: > 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. > > The function svm_invlpga() stays in arch/x86/hvm/svm/svm.h because it is used > by arch/x86/hvm/svm/asid.h. > > Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> Acked-by: Jan Beulich <jbeulich@suse.com> > Changes in v2: > - new patch, the creation of a private header was suggested by Andrew and Jan Same remark here as on patch 2. > I have not added #ifndef guards as it is a private and it should not be > included by other headers. However, this is considered a MISRA-C violation > ... I 'm not sure what to do. Personally I prefer the guard-less form, but since Misra insists, we may better add them from the start. Besides being a little bit of clutter they're harmless, after all. My ack stands with or without the addition, but I wouldn't want to add them "while committing". So I guess we want to wait a little for further views, and then either commit as is or wait for a v3 with the guards added. Jan
© 2016 - 2026 Red Hat, Inc.