On 23/02/2023 10:47 am, Jan Beulich wrote: > On 22.02.2023 13:00, Xenia Ragiadakou wrote: >> --- /dev/null >> +++ b/xen/arch/x86/hvm/vmx/vmx.h >> @@ -0,0 +1,459 @@ >> +#include <xen/sched.h>
Like svm.h, SPDX tag and guards please. > >> +extern int8_t opt_ept_exec_sp; >> + >> +#define PI_xAPIC_NDST_MASK 0xFF00 >> + >> +void vmx_asm_vmexit_handler(struct cpu_user_regs); > Even if it was like this originally, this is bogus. This not-really-a- > function doesn't take any parameters. It's declaration also looks to be > missing cf_check - Andrew, was this deliberate? Yes, it's deliberate. As you identified, it's never called, and therefore doesn't undergo any CFI typechecking. It's an address written into the VMCS's HOST_RIP field, which also explains why it has a bogus type and no-one's noticed in 18 years. (Yup really - c/s 9c3118a825, December 2004) But it does bring us back to a general question which has come up a couple of times recently - how to represent asm symbols in C. This ought to be void nocall vmx_asm_vmexit_handler(void); to identify that it is a function-like thing without a normal calling convention. IMO i could live in vmcs.c rather than being declared publicly. I see nothing in MISRA that objects to this. Rule 8.5 states that an external object or function shall be declared once, and only in one file. There is no mandate that this single file is a header file. The point of moving this to a .c file is to indicate that the asm symbol isn't legitimate to reference anywhere else. Such a change probably wants backporting. Let me do a specific fix, to separate it from the cleanup. ~Andrew
