Do not include the headers:
xen/irq.h
asm/hvm/svm/intr.h
asm/io.h
asm/mem_sharing.h
asm/regs.h
because none of the declarations and macro definitions in them is used.
Sort alphabetically the rest of the headers.
Remove the forward declaration of svm_function_table and place start_svm()
after the svm_function_table's definition.
Replace double new lines with one.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
xen/arch/x86/hvm/svm/svm.c | 159 +++++++++++++++++--------------------
1 file changed, 72 insertions(+), 87 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index fa73257203..f0bc72c313 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -16,60 +16,47 @@
* 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/lib.h>
-#include <xen/trace.h>
#include <xen/sched.h>
-#include <xen/irq.h>
-#include <xen/softirq.h>
-#include <xen/hypercall.h>
-#include <xen/domain_page.h>
+#include <xen/trace.h>
#include <xen/xenoprof.h>
-#include <asm/current.h>
-#include <asm/io.h>
-#include <asm/paging.h>
-#include <asm/p2m.h>
-#include <asm/mem_sharing.h>
-#include <asm/regs.h>
-#include <asm/cpufeature.h>
-#include <asm/processor.h>
#include <asm/amd.h>
+#include <asm/apic.h>
+#include <asm/cpufeature.h>
+#include <asm/current.h>
#include <asm/debugreg.h>
-#include <asm/msr.h>
-#include <asm/i387.h>
-#include <asm/iocap.h>
+#include <asm/gdbsx.h>
#include <asm/hvm/emulate.h>
#include <asm/hvm/hvm.h>
-#include <asm/hvm/support.h>
#include <asm/hvm/io.h>
-#include <asm/hvm/emulate.h>
+#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/svm.h>
-#include <asm/hvm/svm/vmcb.h>
#include <asm/hvm/svm/emulate.h>
-#include <asm/hvm/svm/intr.h>
-#include <asm/hvm/svm/svmdebug.h>
#include <asm/hvm/svm/nestedsvm.h>
-#include <asm/hvm/nestedhvm.h>
-#include <asm/spec_ctrl.h>
-#include <asm/x86_emulate.h>
-#include <public/sched.h>
-#include <asm/hvm/vpt.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/hap.h>
-#include <asm/apic.h>
-#include <asm/gdbsx.h>
-#include <asm/hvm/monitor.h>
+#include <asm/iocap.h>
+#include <asm/i387.h>
#include <asm/monitor.h>
-#include <asm/xstate.h>
+#include <asm/msr.h>
+#include <asm/paging.h>
+#include <asm/processor.h>
+#include <asm/p2m.h>
+#include <asm/x86_emulate.h>
+#include <public/sched.h>
void noreturn svm_asm_do_resume(void);
u32 svm_feature_flags;
-static struct hvm_function_table svm_function_table;
-
/*
* Physical addresses of the Host State Area (for hardware) and vmcb (for Xen)
* which contains Xen's fs/gs/tr/ldtr and GSBASE/STAR/SYSENTER state when in
@@ -505,7 +492,6 @@ static int svm_vmcb_restore(struct vcpu *v, struct hvm_hw_cpu *c)
return 0;
}
-
static void svm_save_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
{
struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
@@ -517,7 +503,6 @@ static void svm_save_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
data->msr_syscall_mask = vmcb->sfmask;
}
-
static void svm_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
{
struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
@@ -1649,57 +1634,6 @@ static int cf_check svm_cpu_up(void)
return _svm_cpu_up(false);
}
-const struct hvm_function_table * __init start_svm(void)
-{
- bool_t printed = 0;
-
- svm_host_osvw_reset();
-
- if ( _svm_cpu_up(true) )
- {
- printk("SVM: failed to initialise.\n");
- return NULL;
- }
-
- setup_vmcb_dump();
-
- if ( boot_cpu_data.extended_cpuid_level >= 0x8000000a )
- svm_feature_flags = cpuid_edx(0x8000000a);
-
- printk("SVM: Supported advanced features:\n");
-
- /* DecodeAssists fast paths assume nextrip is valid for fast rIP update. */
- if ( !cpu_has_svm_nrips )
- __clear_bit(SVM_FEATURE_DECODEASSISTS, &svm_feature_flags);
-
- if ( cpu_has_tsc_ratio )
- svm_function_table.tsc_scaling.ratio_frac_bits = 32;
-
-#define P(p,s) if ( p ) { printk(" - %s\n", s); printed = 1; }
- P(cpu_has_svm_npt, "Nested Page Tables (NPT)");
- P(cpu_has_svm_lbrv, "Last Branch Record (LBR) Virtualisation");
- P(cpu_has_svm_nrips, "Next-RIP Saved on #VMEXIT");
- P(cpu_has_svm_cleanbits, "VMCB Clean Bits");
- P(cpu_has_svm_decode, "DecodeAssists");
- P(cpu_has_svm_vloadsave, "Virtual VMLOAD/VMSAVE");
- P(cpu_has_svm_vgif, "Virtual GIF");
- P(cpu_has_pause_filter, "Pause-Intercept Filter");
- P(cpu_has_pause_thresh, "Pause-Intercept Filter Threshold");
- P(cpu_has_tsc_ratio, "TSC Rate MSR");
- P(cpu_has_svm_sss, "NPT Supervisor Shadow Stack");
- P(cpu_has_svm_spec_ctrl, "MSR_SPEC_CTRL virtualisation");
-#undef P
-
- if ( !printed )
- printk(" - none\n");
-
- svm_function_table.hap_supported = !!cpu_has_svm_npt;
- svm_function_table.hap_capabilities = HVM_HAP_SUPERPAGE_2MB |
- (cpu_has_page1gb ? HVM_HAP_SUPERPAGE_1GB : 0);
-
- return &svm_function_table;
-}
-
static void svm_do_nested_pgfault(struct vcpu *v,
struct cpu_user_regs *regs, uint64_t pfec, paddr_t gpa)
{
@@ -2598,6 +2532,57 @@ static struct hvm_function_table __initdata_cf_clobber svm_function_table = {
},
};
+const struct hvm_function_table * __init start_svm(void)
+{
+ bool_t printed = 0;
+
+ svm_host_osvw_reset();
+
+ if ( _svm_cpu_up(true) )
+ {
+ printk("SVM: failed to initialise.\n");
+ return NULL;
+ }
+
+ setup_vmcb_dump();
+
+ if ( boot_cpu_data.extended_cpuid_level >= 0x8000000a )
+ svm_feature_flags = cpuid_edx(0x8000000a);
+
+ printk("SVM: Supported advanced features:\n");
+
+ /* DecodeAssists fast paths assume nextrip is valid for fast rIP update. */
+ if ( !cpu_has_svm_nrips )
+ __clear_bit(SVM_FEATURE_DECODEASSISTS, &svm_feature_flags);
+
+ if ( cpu_has_tsc_ratio )
+ svm_function_table.tsc_scaling.ratio_frac_bits = 32;
+
+#define P(p,s) if ( p ) { printk(" - %s\n", s); printed = 1; }
+ P(cpu_has_svm_npt, "Nested Page Tables (NPT)");
+ P(cpu_has_svm_lbrv, "Last Branch Record (LBR) Virtualisation");
+ P(cpu_has_svm_nrips, "Next-RIP Saved on #VMEXIT");
+ P(cpu_has_svm_cleanbits, "VMCB Clean Bits");
+ P(cpu_has_svm_decode, "DecodeAssists");
+ P(cpu_has_svm_vloadsave, "Virtual VMLOAD/VMSAVE");
+ P(cpu_has_svm_vgif, "Virtual GIF");
+ P(cpu_has_pause_filter, "Pause-Intercept Filter");
+ P(cpu_has_pause_thresh, "Pause-Intercept Filter Threshold");
+ P(cpu_has_tsc_ratio, "TSC Rate MSR");
+ P(cpu_has_svm_sss, "NPT Supervisor Shadow Stack");
+ P(cpu_has_svm_spec_ctrl, "MSR_SPEC_CTRL virtualisation");
+#undef P
+
+ if ( !printed )
+ printk(" - none\n");
+
+ svm_function_table.hap_supported = !!cpu_has_svm_npt;
+ svm_function_table.hap_capabilities = HVM_HAP_SUPERPAGE_2MB |
+ (cpu_has_page1gb ? HVM_HAP_SUPERPAGE_1GB : 0);
+
+ return &svm_function_table;
+}
+
void svm_vmexit_handler(struct cpu_user_regs *regs)
{
uint64_t exit_reason;
--
2.37.2On 17/02/2023 6:48 pm, Xenia Ragiadakou wrote: > Do not include the headers: > xen/irq.h > asm/hvm/svm/intr.h > asm/io.h > asm/mem_sharing.h > asm/regs.h Out of interest, how are you calculating these? They're accurate as far as I can tell. Looking at asm/hvm/svm/*, intr.h itself can be straight deleted, svmdebug.h can be merged into vmcb.h, and all the others can move into xen/arch/x86/hvm/svm/ as local headers. None of them have any business being included elsewhere in Xen. > because none of the declarations and macro definitions in them is used. > Sort alphabetically the rest of the headers. Minor grammar point. "Sort the rest of the headers alphabetically" would be a more normal way of phrasing this. > > Remove the forward declaration of svm_function_table and place start_svm() > after the svm_function_table's definition. > > Replace double new lines with one. > > No functional change intended. > > Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Hi Andrew, On 2/21/23 00:12, Andrew Cooper wrote: > On 17/02/2023 6:48 pm, Xenia Ragiadakou wrote: >> Do not include the headers: >> xen/irq.h >> asm/hvm/svm/intr.h >> asm/io.h >> asm/mem_sharing.h >> asm/regs.h > > Out of interest, how are you calculating these? They're accurate as far > as I can tell. I do not use a script (at least not a decent one), if that 's the question :) . I verify that none of the symbols defined or declared in the header is used in the file including the header. > > Looking at asm/hvm/svm/*, intr.h itself can be straight deleted, > svmdebug.h can be merged into vmcb.h, and all the others can move into > xen/arch/x86/hvm/svm/ as local headers. None of them have any business > being included elsewhere in Xen. I can send another patch for that. > >> because none of the declarations and macro definitions in them is used. >> Sort alphabetically the rest of the headers. > > Minor grammar point. "Sort the rest of the headers alphabetically" would > be a more normal way of phrasing this. I will fix it in v2. > >> >> Remove the forward declaration of svm_function_table and place start_svm() >> after the svm_function_table's definition. >> >> Replace double new lines with one. >> >> No functional change intended. >> >> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> -- Xenia
On 21.02.2023 08:45, Xenia Ragiadakou wrote: > Hi Andrew, > > On 2/21/23 00:12, Andrew Cooper wrote: >> On 17/02/2023 6:48 pm, Xenia Ragiadakou wrote: >>> Do not include the headers: >>> xen/irq.h >>> asm/hvm/svm/intr.h >>> asm/io.h >>> asm/mem_sharing.h >>> asm/regs.h >> >> Out of interest, how are you calculating these? They're accurate as far >> as I can tell. > > I do not use a script (at least not a decent one), if that 's the > question :) . I verify that none of the symbols defined or declared in > the header is used in the file including the header. > >> >> Looking at asm/hvm/svm/*, intr.h itself can be straight deleted, >> svmdebug.h can be merged into vmcb.h, and all the others can move into >> xen/arch/x86/hvm/svm/ as local headers. None of them have any business >> being included elsewhere in Xen. > > I can send another patch for that. > >> >>> because none of the declarations and macro definitions in them is used. >>> Sort alphabetically the rest of the headers. >> >> Minor grammar point. "Sort the rest of the headers alphabetically" would >> be a more normal way of phrasing this. > > I will fix it in v2. I guess this can be adjusted while committing, seeing that ... >>> Remove the forward declaration of svm_function_table and place start_svm() >>> after the svm_function_table's definition. >>> >>> Replace double new lines with one. >>> >>> No functional change intended. >>> >>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> >> >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> ... it's otherwise ready to be committed. Jan
On 2/21/23 13:12, Jan Beulich wrote: > On 21.02.2023 08:45, Xenia Ragiadakou wrote: >> Hi Andrew, >> >> On 2/21/23 00:12, Andrew Cooper wrote: >>> On 17/02/2023 6:48 pm, Xenia Ragiadakou wrote: >>>> Do not include the headers: >>>> xen/irq.h >>>> asm/hvm/svm/intr.h >>>> asm/io.h >>>> asm/mem_sharing.h >>>> asm/regs.h >>> >>> Out of interest, how are you calculating these? They're accurate as far >>> as I can tell. >> >> I do not use a script (at least not a decent one), if that 's the >> question :) . I verify that none of the symbols defined or declared in >> the header is used in the file including the header. >> >>> >>> Looking at asm/hvm/svm/*, intr.h itself can be straight deleted, >>> svmdebug.h can be merged into vmcb.h, and all the others can move into >>> xen/arch/x86/hvm/svm/ as local headers. None of them have any business >>> being included elsewhere in Xen. >> >> I can send another patch for that. >> >>> >>>> because none of the declarations and macro definitions in them is used. >>>> Sort alphabetically the rest of the headers. >>> >>> Minor grammar point. "Sort the rest of the headers alphabetically" would >>> be a more normal way of phrasing this. >> >> I will fix it in v2. > > I guess this can be adjusted while committing, seeing that ... > >>>> Remove the forward declaration of svm_function_table and place start_svm() >>>> after the svm_function_table's definition. >>>> >>>> Replace double new lines with one. >>>> >>>> No functional change intended. >>>> >>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> >>> >>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > > ... it's otherwise ready to be committed. Great, thx. > > Jan -- Xenia
On 21/02/2023 11:42 am, Xenia Ragiadakou wrote: > > On 2/21/23 13:12, Jan Beulich wrote: >> On 21.02.2023 08:45, Xenia Ragiadakou wrote: >>> Hi Andrew, >>> >>> On 2/21/23 00:12, Andrew Cooper wrote: >>>> On 17/02/2023 6:48 pm, Xenia Ragiadakou wrote: >>>>> Do not include the headers: >>>>> xen/irq.h >>>>> asm/hvm/svm/intr.h >>>>> asm/io.h >>>>> asm/mem_sharing.h >>>>> asm/regs.h >>>> >>>> Out of interest, how are you calculating these? They're accurate >>>> as far >>>> as I can tell. >>> >>> I do not use a script (at least not a decent one), if that 's the >>> question :) . I verify that none of the symbols defined or declared in >>> the header is used in the file including the header. >>> >>>> >>>> Looking at asm/hvm/svm/*, intr.h itself can be straight deleted, >>>> svmdebug.h can be merged into vmcb.h, and all the others can move into >>>> xen/arch/x86/hvm/svm/ as local headers. None of them have any >>>> business >>>> being included elsewhere in Xen. >>> >>> I can send another patch for that. >>> >>>> >>>>> because none of the declarations and macro definitions in them is >>>>> used. >>>>> Sort alphabetically the rest of the headers. >>>> >>>> Minor grammar point. "Sort the rest of the headers alphabetically" >>>> would >>>> be a more normal way of phrasing this. >>> >>> I will fix it in v2. >> >> I guess this can be adjusted while committing, seeing that ... >> >>>>> Remove the forward declaration of svm_function_table and place >>>>> start_svm() >>>>> after the svm_function_table's definition. >>>>> >>>>> Replace double new lines with one. >>>>> >>>>> No functional change intended. >>>>> >>>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> >>>> >>>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> >> >> ... it's otherwise ready to be committed. > > Great, thx. I already committed this patch, with it fixed up, and one other tweak that we commonly do which is to leave a blank line between different groups of headers. It greatly helps people trying to figure out where to put a new header. ~Andrew
© 2016 - 2026 Red Hat, Inc.