On 07.02.2023 10:43, Xenia Ragiadakou wrote: > APIC virtualization support is currently implemented only for Intel VT-x. > To aid future work on separating AMD-V from Intel VT-x code, instead of > calling directly vmx_vlapic_msr_changed() from common hvm code, add a stub > to the hvm_function_table, named update_vlapic_mode, and create a wrapper > function, called hvm_update_vlapic_mode(), to be used by common hvm code. > > After the change above, do not include header asm/hvm/vmx/vmx.h as it is > not required anymore and resolve subsequent build errors for implicit > declaration of functions ‘TRACE_2_LONG_3D’ and ‘TRC_PAR_LONG’ by including > missing asm/hvm/trace.h header. > > No functional change intended. > > Signed-off-by: Xenia Ragiadakou <burzalod...@gmail.com> > --- > > Changes in v2: > - rename set_virtual_apic_mode to update_vlapic_mode, suggested by Andrew > - in hvm_update_vlapic_mode(), call the stub only if available, otherwise > a BUG() will be triggered every time an svm guest writes the APIC_BASE > MSR, > bug reported by Andrew > - initialize the stub for vmx unconditionally to maintain current behavior > since no functional change is intended, bug reported by Andrew (here, I > decided to place the initialization in start_vmx to be closer to the > initializations of the other stubs that are relevant to apic > virtualization)
I disagree with this - unconditional hooks are better put in place right in vmx_function_table's initializer. Also now that you use the function as a callback, vmx_vlapic_msr_changed() will need to have cf_check added (on both declaration and definition, albeit I keep forgetting why putting it on just the declaration isn't sufficient; I guess a short comment to that effect next to cf_check's definition in compiler.h would help). Jan
