On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote:
> Introduce avic_update_vapic_bar(), which checks the x2APIC enable bit
> of the APIC Base register and update the new struct vcpu_svm.x2apic_enabled
> to keep track of current APIC mode of each vCPU,
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> ---
> arch/x86/kvm/svm/avic.c | 13 +++++++++++++
> arch/x86/kvm/svm/svm.c | 4 ++++
> arch/x86/kvm/svm/svm.h | 2 ++
> 3 files changed, 19 insertions(+)
>
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 1999076966fd..60f30e48d816 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -609,6 +609,19 @@ void avic_post_state_restore(struct kvm_vcpu *vcpu)
> avic_handle_ldr_update(vcpu);
> }
>
> +void avic_update_vapic_bar(struct vcpu_svm *svm, u64 data)
> +{
> + svm->vmcb->control.avic_vapic_bar = data & VMCB_AVIC_APIC_BAR_MASK;
> +
> + /* Set x2APIC mode bit if guest enable x2apic mode. */
> + svm->x2apic_enabled = (avic_mode == AVIC_MODE_X2 &&
> + kvm_apic_mode(data) == LAPIC_MODE_X2APIC);
> + pr_debug("vcpu_id:%d switch to %s\n", svm->vcpu.vcpu_id,
> + svm->x2apic_enabled ? "x2APIC" : "xAPIC");
> + vmcb_mark_dirty(svm->vmcb, VMCB_AVIC);
> + kvm_vcpu_update_apicv(&svm->vcpu);
> +}
> +
> void svm_set_virtual_apic_mode(struct kvm_vcpu *vcpu)
> {
> return;
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 3687026f2859..4e6dc1feeac7 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2867,6 +2867,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
> svm->msr_decfg = data;
> break;
> }
> + case MSR_IA32_APICBASE:
> + if (kvm_vcpu_apicv_active(vcpu))
> + avic_update_vapic_bar(to_svm(vcpu), data);
> + fallthrough;
> default:
> return kvm_set_msr_common(vcpu, msr);
> }
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 1a0bf6b853df..bfbebb933da2 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -225,6 +225,7 @@ struct vcpu_svm {
> u32 dfr_reg;
> struct page *avic_backing_page;
> u64 *avic_physical_id_cache;
> + bool x2apic_enabled;
>
> /*
> * Per-vcpu list of struct amd_svm_iommu_ir:
> @@ -566,6 +567,7 @@ void avic_init_vmcb(struct vcpu_svm *svm);
> int avic_incomplete_ipi_interception(struct kvm_vcpu *vcpu);
> int avic_unaccelerated_access_interception(struct kvm_vcpu *vcpu);
> int avic_init_vcpu(struct vcpu_svm *svm);
> +void avic_update_vapic_bar(struct vcpu_svm *svm, u64 data);
> void avic_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
> void avic_vcpu_put(struct kvm_vcpu *vcpu);
> void avic_post_state_restore(struct kvm_vcpu *vcpu);
Have you looked at how this is done on Intel's APICv side?
You need to implement .set_virtual_apic_mode instead.
(look at vmx_set_virtual_apic_mode)
I also don't think you need x2apic_enabled boolean.
You can already know if a vCPU uses apic or x2apic via
kvm_get_apic_mode(vcpu);
in fact I don't think avic code should have any bookeeping in regard to x2apic/x2avic mode,
but rather kvm's apic mode (which is read directly from apic base msr (vcpu->arch.apic_base),
should enable avic, or x2avic if possible, or inhibit avic if not possible.
e.g it should drive the bits in vmcb and such.
Best regards,
Maxim Levitsky
