| Message ID | 1552529524-14668-1-git-send-email-lirongqing@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] KVM: fix error handling in svm_hardware_setup | expand |
Li RongQing <lirongqing@baidu.com> writes: > From: Li Rongqing <lirongqing@baidu.com> > > move svm_hardware_unsetup before svm_hardware_setup, and call > it to free all memory if fail to setup in svm_hardware_setup, > otherwise memory will be leaked > > remove __exit attribute for svm_hardware_unsetup since it is > called in __init function > > Signed-off-by: Li Rongqing <lirongqing@baidu.com> > Signed-off-by: Zhang Yu <zhangyu31@baidu.com> > --- > arch/x86/kvm/svm.c | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 1787a484d21c..276ab8ab6c95 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1290,6 +1290,20 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu) > control->pause_filter_count, old); > } > > +static void svm_hardware_unsetup(void) > +{ Unrelated to your patch but "unsetup" makes me feel sad. Why can't we use e.g. "teardown" instead? (KVM is among just four users of 'unsetup' word in kernel). > + int cpu; > + > + if (svm_sev_enabled()) > + bitmap_free(sev_asid_bitmap); > + > + for_each_possible_cpu(cpu) > + svm_cpu_uninit(cpu); > + > + __free_pages(pfn_to_page(iopm_base >> PAGE_SHIFT), IOPM_ALLOC_ORDER); > + iopm_base = 0; > +} > + > static __init int svm_hardware_setup(void) > { > int cpu; > @@ -1396,25 +1410,10 @@ static __init int svm_hardware_setup(void) > return 0; > > err: > - __free_pages(iopm_pages, IOPM_ALLOC_ORDER); > - iopm_base = 0; > + svm_hardware_unsetup(); > return r; > } > > -static __exit void svm_hardware_unsetup(void) > -{ > - int cpu; > - > - if (svm_sev_enabled()) > - bitmap_free(sev_asid_bitmap); > - > - for_each_possible_cpu(cpu) > - svm_cpu_uninit(cpu); > - > - __free_pages(pfn_to_page(iopm_base >> PAGE_SHIFT), IOPM_ALLOC_ORDER); > - iopm_base = 0; > -} > - > static void init_seg(struct vmcb_seg *seg) > { > seg->selector = 0; Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > From: Li Rongqing <lirongqing@baidu.com> > > > > move svm_hardware_unsetup before svm_hardware_setup, and call it to > > free all memory if fail to setup in svm_hardware_setup, otherwise > > memory will be leaked > > > > remove __exit attribute for svm_hardware_unsetup since it is called in > > __init function > > > > Signed-off-by: Li Rongqing <lirongqing@baidu.com> > > Signed-off-by: Zhang Yu <zhangyu31@baidu.com> > > --- > > arch/x86/kvm/svm.c | 31 +++++++++++++++---------------- > > 1 file changed, 15 insertions(+), 16 deletions(-) > > > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index > > 1787a484d21c..276ab8ab6c95 100644 > > --- a/arch/x86/kvm/svm.c > > +++ b/arch/x86/kvm/svm.c > > @@ -1290,6 +1290,20 @@ static void shrink_ple_window(struct kvm_vcpu > *vcpu) > > control->pause_filter_count, old); } > > > > +static void svm_hardware_unsetup(void) { > > Unrelated to your patch but "unsetup" makes me feel sad. Why can't we use > e.g. "teardown" instead? (KVM is among just four users of 'unsetup' > word in kernel). > > > + int cpu; > > + > > + if (svm_sev_enabled()) > > + bitmap_free(sev_asid_bitmap); > > + > > + for_each_possible_cpu(cpu) > > + svm_cpu_uninit(cpu); > > + > > + __free_pages(pfn_to_page(iopm_base >> PAGE_SHIFT), > IOPM_ALLOC_ORDER); > > + iopm_base = 0; > > +} > > + > > static __init int svm_hardware_setup(void) { > > int cpu; > > @@ -1396,25 +1410,10 @@ static __init int svm_hardware_setup(void) > > return 0; > > > > err: > > - __free_pages(iopm_pages, IOPM_ALLOC_ORDER); > > - iopm_base = 0; > > + svm_hardware_unsetup(); > > return r; > > } > > > > -static __exit void svm_hardware_unsetup(void) -{ > > - int cpu; > > - > > - if (svm_sev_enabled()) > > - bitmap_free(sev_asid_bitmap); > > - > > - for_each_possible_cpu(cpu) > > - svm_cpu_uninit(cpu); > > - > > - __free_pages(pfn_to_page(iopm_base >> PAGE_SHIFT), > IOPM_ALLOC_ORDER); > > - iopm_base = 0; > > -} > > - > > static void init_seg(struct vmcb_seg *seg) { > > seg->selector = 0; > > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > Ping -RongQing > -- > Vitaly
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 1787a484d21c..276ab8ab6c95 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1290,6 +1290,20 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu) control->pause_filter_count, old); } +static void svm_hardware_unsetup(void) +{ + int cpu; + + if (svm_sev_enabled()) + bitmap_free(sev_asid_bitmap); + + for_each_possible_cpu(cpu) + svm_cpu_uninit(cpu); + + __free_pages(pfn_to_page(iopm_base >> PAGE_SHIFT), IOPM_ALLOC_ORDER); + iopm_base = 0; +} + static __init int svm_hardware_setup(void) { int cpu; @@ -1396,25 +1410,10 @@ static __init int svm_hardware_setup(void) return 0; err: - __free_pages(iopm_pages, IOPM_ALLOC_ORDER); - iopm_base = 0; + svm_hardware_unsetup(); return r; } -static __exit void svm_hardware_unsetup(void) -{ - int cpu; - - if (svm_sev_enabled()) - bitmap_free(sev_asid_bitmap); - - for_each_possible_cpu(cpu) - svm_cpu_uninit(cpu); - - __free_pages(pfn_to_page(iopm_base >> PAGE_SHIFT), IOPM_ALLOC_ORDER); - iopm_base = 0; -} - static void init_seg(struct vmcb_seg *seg) { seg->selector = 0;