On 16/02/2023 1:36 pm, Xenia Ragiadakou wrote: > Hi Andrew, > > On 2/16/23 12:28, Andrew Cooper wrote: >> On 13/02/2023 11:50 am, Xenia Ragiadakou wrote: >>> diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmx.h >>> b/xen/arch/x86/include/asm/hvm/vmx/vmx.h >>> index 234da4a7f4..97d6b810ec 100644 >>> --- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h >>> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h >>> @@ -85,7 +85,7 @@ typedef enum { >>> void vmx_asm_vmexit_handler(struct cpu_user_regs); >>> void vmx_intr_assist(void); >>> void noreturn cf_check vmx_do_resume(void); >>> -void vmx_vlapic_msr_changed(struct vcpu *v); >>> +void cf_check vmx_vlapic_msr_changed(struct vcpu *v); >> >> Hi, >> >> I see this patch has been committed, but this public declaration should >> deleted, and vmx_vlapic_msr_changed() made static now that it's only >> referenced in vmx.c. > > It is also used in vmcs.c
Oh, so it is. It just doesn't show up on the patch diff. That use likely won't survive fixing the Intel APIC-V logic, but I guess we're stuck with it for now. Sorry for the noise. > >> >> It needs a forward declaration in vmx.c because of its position relative >> to the vmx_function_table, but that's fine - we've got plenty of other >> examples like this. >> >> Could I talk you into doing an incremental fix? >> >> Alternatively, we could get better cleanup by forward declaring just >> {vmx,svm}_function_table, then moving the tables to the very bottom of >> {vmx,svm}.c at which point we can drop all the forward declarations. >> >> Oh top of that, I suspect we have other public function definitions >> which can turn static, if you happen to spot any while doing this. > > Sure I could try to cleanup {svm,vmx}.c and the corresponding headers. Thankyou. ~Andrew