Re: [RFCv2 PATCH 09/12] KVM: SVM: Refresh AVIC settings when changing APIC mode

[Maxim Levitsky <mlevitsk@redhat.com>]

On Tue, 2022-03-08 at 10:39 -0600, Suravee Suthikulpanit wrote:
> When APIC mode is updated (e.g. from xAPIC to x2APIC),
> KVM needs to update AVIC settings accordingly, whic is
> handled by svm_refresh_apicv_exec_ctrl().
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/kvm/svm/avic.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 7e5a39a8e698..53559b8dfa52 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -625,7 +625,24 @@ void avic_post_state_restore(struct kvm_vcpu *vcpu)
>  
>  void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
>  {
> -	return;
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	if (!lapic_in_kernel(vcpu) || (avic_mode == AVIC_MODE_NONE))
> +		return;
> +
> +	if (kvm_get_apic_mode(vcpu) == LAPIC_MODE_INVALID)
> +		WARN_ONCE(true, "Invalid local APIC state");
> +
> +	svm->vmcb->control.avic_vapic_bar = svm->vcpu.arch.apic_base &
> +					    VMCB_AVIC_APIC_BAR_MASK;

No need for that - APIC base relocation doesn't work when AVIC is enabled,
since the page which contains it has to be marked R/W in NPT, which we
only do for the default APIC base.

I recently removed the code from AVIC which still tried to set the
'avic_vapic_bar' like this.



> +	kvm_vcpu_update_apicv(&svm->vcpu);
> +
> +	/*
> +	 * The VM could be running w/ AVIC activated switching from APIC
> +	 * to x2APIC mode. We need to all refresh to make sure that all
> +	 * x2AVIC configuration are being done.

Why? When AVIC is un-inhibited later then the svm_refresh_apicv_exec_ctrl will be called
again and switch to x2avic mode I think.

When AVIC is inhibited, then regardless of x2apic mode, VMCB must not have
any avic bits set, and all x2apic msrs should be read/write intercepted.,
thus I don't think that svm_refresh_apicv_exec_ctrl should be force called.


> +	 */
> +	svm_refresh_apicv_exec_ctrl(&svm->vcpu);
>  }
>  
>  void svm_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)

Best regards,
	Maxim Levitsky