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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/02/2023 12:58 pm, Xenia Ragiadakou wrote:
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 270bc98195..6138dc0885 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3011,6 +3011,10 @@ const struct hvm_function_table * __init 
> start_vmx(void)
>          setup_ept_dump();
>      }
>  
> +    if ( cpu_has_vmx_virtualize_apic_accesses ||
> +         cpu_has_vmx_virtualize_x2apic_mode )

x2apic_mode is definitely wrong here, but I think apic_accesses is too. 
The top of vmx_vlapic_msr_changed() is buggy too.

Right now, the hook is called unconditionally.  Given no adjustment in
vmx_vlapic_msr_changed(), the new form (using an alternative) needs
calling unconditionally too.

Naming wise, Linux is fairly bogus too.  This should be
hvm_update_vlapic_mode(), but I suspect the hook will disappear in due
course.

> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
> b/xen/arch/x86/include/asm/hvm/hvm.h
> index 80e4565bd2..b690e2924c 100644
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -786,6 +787,11 @@ static inline int hvm_pi_update_irte(const struct vcpu 
> *v,
>      return alternative_call(hvm_funcs.pi_update_irte, v, pirq, gvec);
>  }
>  
> +static inline void hvm_set_virtual_apic_mode(struct vcpu *v)
> +{
> +    alternative_vcall(hvm_funcs.set_virtual_apic_mode, v);

This has to be something like:

if ( hvm_funcs.set_virtual_apic_mode )
    alternative_vcall(...)

Otherwise, Xen will BUG() every time an SVM guest modifies MSR_APIC_BASE.

~Andrew