| Message ID | 20230815213533.548732-6-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VM: SVM: Honor KVM_MAX_VCPUS when AVIC is enabled | expand |
У вт, 2023-08-15 у 14:35 -0700, Sean Christopherson пише: > Drop vcpu_svm's avic_backing_page pointer and instead grab the physical > address of KVM's vAPIC page directly from the source. Getting a physical > address from a kernel virtual address is not an expensive operation, and > getting the physical address from a struct page is *more* expensive for > CONFIG_SPARSEMEM=y kernels. Regardless, none of the paths that consume > the address are hot paths, i.e. shaving cycles is not a priority. > > Eliminating the "cache" means KVM doesn't have to worry about the cache > being invalid, which will simplify a future fix when dealing with vCPU IDs > that are too big. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/svm/avic.c | 4 +--- > arch/x86/kvm/svm/svm.h | 1 - > 2 files changed, 1 insertion(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index 954bdb45033b..e49b682c8469 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -243,7 +243,7 @@ int avic_vm_init(struct kvm *kvm) > > static phys_addr_t avic_get_backing_page_address(struct vcpu_svm *svm) > { > - return __sme_set(page_to_phys(svm->avic_backing_page)); > + return __sme_set(__pa(svm->vcpu.arch.apic->regs)); I overall agree with this patch however the old code was safer: svm->avic_backing_page is set to physical address of the apic registers only in the avic_init_backing_page() and after checking the vcpu->arch.apic->regs != NULL and now in theory NULL vcpu->arch.apic->regs are not checked. I know that you later add a patch which adds a similar warning, but I prefer that you fold it with this patch. > } > > void avic_init_vmcb(struct vcpu_svm *svm, struct vmcb *vmcb) > @@ -305,8 +305,6 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu) > return ret; > } > > - svm->avic_backing_page = virt_to_page(vcpu->arch.apic->regs); > - > /* Setting AVIC backing page address in the phy APIC ID table */ > entry = avic_get_physical_id_entry(vcpu, id); > if (!entry) > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h > index 2237230aad98..a9fde1bb85ee 100644 > --- a/arch/x86/kvm/svm/svm.h > +++ b/arch/x86/kvm/svm/svm.h > @@ -261,7 +261,6 @@ struct vcpu_svm { > > u32 ldr_reg; > u32 dfr_reg; > - struct page *avic_backing_page; > u64 *avic_physical_id_cache; > > /* Best regards, Maxim Levitsky
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index 954bdb45033b..e49b682c8469 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c @@ -243,7 +243,7 @@ int avic_vm_init(struct kvm *kvm) static phys_addr_t avic_get_backing_page_address(struct vcpu_svm *svm) { - return __sme_set(page_to_phys(svm->avic_backing_page)); + return __sme_set(__pa(svm->vcpu.arch.apic->regs)); } void avic_init_vmcb(struct vcpu_svm *svm, struct vmcb *vmcb) @@ -305,8 +305,6 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu) return ret; } - svm->avic_backing_page = virt_to_page(vcpu->arch.apic->regs); - /* Setting AVIC backing page address in the phy APIC ID table */ entry = avic_get_physical_id_entry(vcpu, id); if (!entry) diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index 2237230aad98..a9fde1bb85ee 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -261,7 +261,6 @@ struct vcpu_svm { u32 ldr_reg; u32 dfr_reg; - struct page *avic_backing_page; u64 *avic_physical_id_cache; /*
Drop vcpu_svm's avic_backing_page pointer and instead grab the physical address of KVM's vAPIC page directly from the source. Getting a physical address from a kernel virtual address is not an expensive operation, and getting the physical address from a struct page is *more* expensive for CONFIG_SPARSEMEM=y kernels. Regardless, none of the paths that consume the address are hot paths, i.e. shaving cycles is not a priority. Eliminating the "cache" means KVM doesn't have to worry about the cache being invalid, which will simplify a future fix when dealing with vCPU IDs that are too big. Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/svm/avic.c | 4 +--- arch/x86/kvm/svm/svm.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-)