答复: [PATCH] KVM: fix error handling in svm_hardware_setup

["Li,Rongqing" <lirongqing@xxxxxxxxx>]

> -----邮件原件-----
> 发件人: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] 代
> 表 Singh, Brijesh
> 发送时间: 2019年3月13日 23:50
> 收件人: Li,Rongqing <lirongqing@xxxxxxxxx>; x86@xxxxxxxxxx;
> kvm@xxxxxxxxxxxxxxx
> 抄送: Singh, Brijesh <brijesh.singh@xxxxxxx>
> 主题: Re: [PATCH] KVM: fix error handling in svm_hardware_setup
> 
...
> > +	if (svm_sev_enabled())
> > +		bitmap_free(sev_asid_bitmap);
> > +
> > +	for_each_possible_cpu(cpu)
> > +		svm_cpu_uninit(cpu);
> > +
> >   	__free_pages(iopm_pages, IOPM_ALLOC_ORDER);
> >   	iopm_base = 0;
> >   	return r;
> >
> 
> 
> Does it make sense to call the svm_hardware_unsetup() instead of duplicating
> the logic in error code path ?
> 

Thanks for your review;

Two thing needs to do if call svm_hardware_unsetup

1. move svm_hardware_unsetup before svm_hardware_setup, to avoid declaration
2. remove __exit attribute for svm_hardware_unsetup , otherwise these is the below warning:

              WARNING: arch/x86/kvm/kvm-amd.o(.init.text+0x4aa): Section mismatch in reference from the function svm_hardware_setup() to         the function .exit.text:svm_hardware_unsetup()
              The function __init svm_hardware_setup() references
              a function __exit svm_hardware_unsetup().
              This is often seen when error handling in the init function
              uses functionality in the exit path.
              The fix is often to remove the __exit annotation of
              svm_hardware_unsetup() so it may be used outside an exit section.


The RFC is below:

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;


-RongQing




> thanks