On 17.02.2023 19:48, Xenia Ragiadakou wrote: > 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 alphabetically the rest of the headers. > Fix build by including asm/i387.h in vmx.c, needed for > vcpu_restore_fpu_lazy(). > > Move the definition of GAS_VMX_OP just above the functions that use it and > undefine it after its usage. > > Move in vmcs.c the definitions of: > ept_sync_all() > __vmxoff() > __vmxon() > because they are used only in this file. Take the opportunity to remove a > trailing white space.
While the latter two are probably fine to be moved, I think the first one wants to remain where it is, as new uses might appear. > Move in vmx.c the definitions of: > pi_test_and_set_pir() > pi_test_pir() > pi_test_and_set_on() > pi_set_on() > pi_test_and_clear_on() > pi_test_on() > pi_get_pir() > pi_test_sn() > pi_set_sn() > pi_clear_sn() > vpid_sync_vcpu_gva() > because they are used only in this file. I'm not fully convinced of such movement. As a general remark: We typically avoid "inline" for functions in .c files. Yet most if not all of the above are pretty good candidates for being explicitly marked "inline". > Move in vmx.c the declarations of: > ve_info_t > ept_qual_t > idt_or_gdt_instr_info_t > ldt_or_tr_instr_info_t > because they are used only in this file. I disagree with the movement of such types. Sooner or later they may want using by vvmx.c as well. If you want to move them, then to a private header under xen/arch/x86/hvm/vmx/. Finally I think at least the individual groups of what is being moved or adjusted want splitting into separate patches. Jan
