| Message ID | 20211213113110.12143-2-suravee.suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | svm: avic: Allow AVIC support on system w/ physical APIC ID > 255 | expand |
On Mon, Dec 13, 2021, Suravee Suthikulpanit wrote: > To prepare for upcoming AVIC changes. There is no functional change. > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > --- > arch/x86/kvm/svm/avic.c | 10 ++++++++++ > arch/x86/kvm/svm/svm.c | 8 +------- > arch/x86/kvm/svm/svm.h | 1 + > 3 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index 8052d92069e0..63c3801d1829 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -1011,3 +1011,13 @@ void svm_vcpu_unblocking(struct kvm_vcpu *vcpu) > kvm_vcpu_update_apicv(vcpu); > avic_set_running(vcpu, true); > } > + > +bool avic_hardware_setup(bool avic) > +{ > + if (!avic || !npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) > + return false; > + > + pr_info("AVIC enabled\n"); > + amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); > + return true; > +} > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 989685098b3e..e59f663ab8cb 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -1031,13 +1031,7 @@ static __init int svm_hardware_setup(void) > nrips = false; > } > > - enable_apicv = avic = avic && npt_enabled && boot_cpu_has(X86_FEATURE_AVIC); > - > - if (enable_apicv) { > - pr_info("AVIC enabled\n"); > - > - amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); > - } > + enable_apicv = avic = avic_hardware_setup(avic); Rather than pass in "avic", just do enable_apicv = avic == avic && avic_hardware_setup(); This also conflicts with changes sitting in kvm/queue to nullify vcpu_(un)blocking when AVIC is disabled. But moving AVIC setup to avic.c provides an opportunity for further cleanup, as it means vcpu_(un)blocking can be NULL by default and set to the AVIC helpers if and only if AVIC is enable. That will allow making the helpers static in avic.c. E.g. --- arch/x86/kvm/svm/avic.c | 17 +++++++++++++++-- arch/x86/kvm/svm/svm.c | 13 +------------ arch/x86/kvm/svm/svm.h | 3 +-- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index 90364d02f22a..f5c6cab42d74 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c @@ -1027,7 +1027,7 @@ void avic_vcpu_put(struct kvm_vcpu *vcpu) WRITE_ONCE(*(svm->avic_physical_id_cache), entry); } -void avic_vcpu_blocking(struct kvm_vcpu *vcpu) +static void avic_vcpu_blocking(struct kvm_vcpu *vcpu) { if (!kvm_vcpu_apicv_active(vcpu)) return; @@ -1052,7 +1052,7 @@ void avic_vcpu_blocking(struct kvm_vcpu *vcpu) preempt_enable(); } -void avic_vcpu_unblocking(struct kvm_vcpu *vcpu) +static void avic_vcpu_unblocking(struct kvm_vcpu *vcpu) { int cpu; @@ -1066,3 +1066,16 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu) put_cpu(); } + +bool avic_hardware_setup(struct kvm_x86_ops *x86_ops) +{ + if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) + return false; + + x86_ops->vcpu_blocking = avic_vcpu_blocking, + x86_ops->vcpu_unblocking = avic_vcpu_unblocking, + + pr_info("AVIC enabled\n"); + amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); + return true; +} diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 6cb38044a860..6cb0f58238cd 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -4390,8 +4390,6 @@ static struct kvm_x86_ops svm_x86_ops __initdata = { .prepare_guest_switch = svm_prepare_guest_switch, .vcpu_load = svm_vcpu_load, .vcpu_put = svm_vcpu_put, - .vcpu_blocking = avic_vcpu_blocking, - .vcpu_unblocking = avic_vcpu_unblocking, .update_exception_bitmap = svm_update_exception_bitmap, .get_msr_feature = svm_get_msr_feature, @@ -4674,16 +4672,7 @@ static __init int svm_hardware_setup(void) nrips = false; } - enable_apicv = avic = avic && npt_enabled && boot_cpu_has(X86_FEATURE_AVIC); - - if (enable_apicv) { - pr_info("AVIC enabled\n"); - - amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); - } else { - svm_x86_ops.vcpu_blocking = NULL; - svm_x86_ops.vcpu_unblocking = NULL; - } + enable_apicv = avic = avic && avic_hardware_setup(&svm_x86_ops); if (vls) { if (!npt_enabled || diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index daa8ca84afcc..59d91b969bd7 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -573,6 +573,7 @@ extern struct kvm_x86_nested_ops svm_nested_ops; #define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL +bool avic_hardware_setup(struct kvm_x86_ops *ops); int avic_ga_log_notifier(u32 ga_tag); void avic_vm_destroy(struct kvm *kvm); int avic_vm_init(struct kvm *kvm); @@ -593,8 +594,6 @@ int svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec); bool svm_dy_apicv_has_pending_interrupt(struct kvm_vcpu *vcpu); int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq, uint32_t guest_irq, bool set); -void avic_vcpu_blocking(struct kvm_vcpu *vcpu); -void avic_vcpu_unblocking(struct kvm_vcpu *vcpu); /* sev.c */ --
Hi Sean, Thanks for the suggestion. I'll update this in v4. Regards, Suravee On 12/31/2021 12:26 AM, Sean Christopherson wrote: > On Mon, Dec 13, 2021, Suravee Suthikulpanit wrote: >> To prepare for upcoming AVIC changes. There is no functional change. >> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >> --- >> arch/x86/kvm/svm/avic.c | 10 ++++++++++ >> arch/x86/kvm/svm/svm.c | 8 +------- >> arch/x86/kvm/svm/svm.h | 1 + >> 3 files changed, 12 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c >> index 8052d92069e0..63c3801d1829 100644 >> --- a/arch/x86/kvm/svm/avic.c >> +++ b/arch/x86/kvm/svm/avic.c >> @@ -1011,3 +1011,13 @@ void svm_vcpu_unblocking(struct kvm_vcpu *vcpu) >> kvm_vcpu_update_apicv(vcpu); >> avic_set_running(vcpu, true); >> } >> + >> +bool avic_hardware_setup(bool avic) >> +{ >> + if (!avic || !npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) >> + return false; >> + >> + pr_info("AVIC enabled\n"); >> + amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); >> + return true; >> +} >> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c >> index 989685098b3e..e59f663ab8cb 100644 >> --- a/arch/x86/kvm/svm/svm.c >> +++ b/arch/x86/kvm/svm/svm.c >> @@ -1031,13 +1031,7 @@ static __init int svm_hardware_setup(void) >> nrips = false; >> } >> >> - enable_apicv = avic = avic && npt_enabled && boot_cpu_has(X86_FEATURE_AVIC); >> - >> - if (enable_apicv) { >> - pr_info("AVIC enabled\n"); >> - >> - amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); >> - } >> + enable_apicv = avic = avic_hardware_setup(avic); > > Rather than pass in "avic", just do > > enable_apicv = avic == avic && avic_hardware_setup(); > > This also conflicts with changes sitting in kvm/queue to nullify vcpu_(un)blocking > when AVIC is disabled. But moving AVIC setup to avic.c provides an opportunity for > further cleanup, as it means vcpu_(un)blocking can be NULL by default and set to > the AVIC helpers if and only if AVIC is enable. That will allow making the helpers > static in avic.c. E.g. > > --- > arch/x86/kvm/svm/avic.c | 17 +++++++++++++++-- > arch/x86/kvm/svm/svm.c | 13 +------------ > arch/x86/kvm/svm/svm.h | 3 +-- > 3 files changed, 17 insertions(+), 16 deletions(-) > > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index 90364d02f22a..f5c6cab42d74 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -1027,7 +1027,7 @@ void avic_vcpu_put(struct kvm_vcpu *vcpu) > WRITE_ONCE(*(svm->avic_physical_id_cache), entry); > } > > -void avic_vcpu_blocking(struct kvm_vcpu *vcpu) > +static void avic_vcpu_blocking(struct kvm_vcpu *vcpu) > { > if (!kvm_vcpu_apicv_active(vcpu)) > return; > @@ -1052,7 +1052,7 @@ void avic_vcpu_blocking(struct kvm_vcpu *vcpu) > preempt_enable(); > } > > -void avic_vcpu_unblocking(struct kvm_vcpu *vcpu) > +static void avic_vcpu_unblocking(struct kvm_vcpu *vcpu) > { > int cpu; > > @@ -1066,3 +1066,16 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu) > > put_cpu(); > } > + > +bool avic_hardware_setup(struct kvm_x86_ops *x86_ops) > +{ > + if (!npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) > + return false; > + > + x86_ops->vcpu_blocking = avic_vcpu_blocking, > + x86_ops->vcpu_unblocking = avic_vcpu_unblocking, > + > + pr_info("AVIC enabled\n"); > + amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); > + return true; > +} > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 6cb38044a860..6cb0f58238cd 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -4390,8 +4390,6 @@ static struct kvm_x86_ops svm_x86_ops __initdata = { > .prepare_guest_switch = svm_prepare_guest_switch, > .vcpu_load = svm_vcpu_load, > .vcpu_put = svm_vcpu_put, > - .vcpu_blocking = avic_vcpu_blocking, > - .vcpu_unblocking = avic_vcpu_unblocking, > > .update_exception_bitmap = svm_update_exception_bitmap, > .get_msr_feature = svm_get_msr_feature, > @@ -4674,16 +4672,7 @@ static __init int svm_hardware_setup(void) > nrips = false; > } > > - enable_apicv = avic = avic && npt_enabled && boot_cpu_has(X86_FEATURE_AVIC); > - > - if (enable_apicv) { > - pr_info("AVIC enabled\n"); > - > - amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); > - } else { > - svm_x86_ops.vcpu_blocking = NULL; > - svm_x86_ops.vcpu_unblocking = NULL; > - } > + enable_apicv = avic = avic && avic_hardware_setup(&svm_x86_ops); > > if (vls) { > if (!npt_enabled || > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h > index daa8ca84afcc..59d91b969bd7 100644 > --- a/arch/x86/kvm/svm/svm.h > +++ b/arch/x86/kvm/svm/svm.h > @@ -573,6 +573,7 @@ extern struct kvm_x86_nested_ops svm_nested_ops; > > #define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL > > +bool avic_hardware_setup(struct kvm_x86_ops *ops); > int avic_ga_log_notifier(u32 ga_tag); > void avic_vm_destroy(struct kvm *kvm); > int avic_vm_init(struct kvm *kvm); > @@ -593,8 +594,6 @@ int svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec); > bool svm_dy_apicv_has_pending_interrupt(struct kvm_vcpu *vcpu); > int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq, > uint32_t guest_irq, bool set); > -void avic_vcpu_blocking(struct kvm_vcpu *vcpu); > -void avic_vcpu_unblocking(struct kvm_vcpu *vcpu); > > /* sev.c */ > > -- > >
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index 8052d92069e0..63c3801d1829 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c @@ -1011,3 +1011,13 @@ void svm_vcpu_unblocking(struct kvm_vcpu *vcpu) kvm_vcpu_update_apicv(vcpu); avic_set_running(vcpu, true); } + +bool avic_hardware_setup(bool avic) +{ + if (!avic || !npt_enabled || !boot_cpu_has(X86_FEATURE_AVIC)) + return false; + + pr_info("AVIC enabled\n"); + amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); + return true; +} diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 989685098b3e..e59f663ab8cb 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -1031,13 +1031,7 @@ static __init int svm_hardware_setup(void) nrips = false; } - enable_apicv = avic = avic && npt_enabled && boot_cpu_has(X86_FEATURE_AVIC); - - if (enable_apicv) { - pr_info("AVIC enabled\n"); - - amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier); - } + enable_apicv = avic = avic_hardware_setup(avic); if (vls) { if (!npt_enabled || diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h index 5d30db599e10..3fa975031dc9 100644 --- a/arch/x86/kvm/svm/svm.h +++ b/arch/x86/kvm/svm/svm.h @@ -515,6 +515,7 @@ static inline bool avic_vcpu_is_running(struct kvm_vcpu *vcpu) return (READ_ONCE(*entry) & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK); } +bool avic_hardware_setup(bool avic); int avic_ga_log_notifier(u32 ga_tag); void avic_vm_destroy(struct kvm *kvm); int avic_vm_init(struct kvm *kvm);
To prepare for upcoming AVIC changes. There is no functional change. Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> --- arch/x86/kvm/svm/avic.c | 10 ++++++++++ arch/x86/kvm/svm/svm.c | 8 +------- arch/x86/kvm/svm/svm.h | 1 + 3 files changed, 12 insertions(+), 7 deletions(-)