| Message ID | 20220221021922.733373-7-suravee.suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introducing AMD x2APIC Virtualization (x2AVIC) support. | expand |
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@amd.com> > --- > 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
Hi Maxim, On 2/25/2022 12:29 AM, Maxim Levitsky wrote: >> 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) Actually, that would be better. I'll update this part to use svm_set_virtual_apic_mode, which is doing nothing at the moment. Regards, Suravee
Hi Maxim On 2/25/22 12:29 AM, Maxim Levitsky wrote: >> 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); > > .... > > 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. I'll also clean this up in V2. Regards, Suravee
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);
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@amd.com> --- 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(+)