Re: [PATCH v3] x86/vlapic: call vmx_vlapic_msr_changed through an hvm_function callback

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

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

> 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.

> Thanks,
>
> ~Andrew
--
Xenia