On 11/6/24 04:07, Zhao Liu wrote:
Currently, there're following incorrect error handling cases in kvm_arch_init(): * Missed to handle failure of kvm_get_supported_feature_msrs(). * Missed to return when kvm_vm_enable_disable_exits() fails.
At least in these two cases I think it was intentional to avoid hard failures. It's probably not a very likely case and I think your patch is overall a good idea.
Paolo
* MSR filter related cases called exit() directly instead of returning to kvm_init(). (The caller of kvm_arch_init() - kvm_init() - needs to know if kvm_arch_init() fails in order to perform cleanup). Fix the above cases. Signed-off-by: Zhao Liu <zhao1.liu@xxxxxxxxx> Reviewed-by: Zide Chen <zide.chen@xxxxxxxxx> --- v5: cleaned up kvm_vm_enable_energy_msrs(). v3: new commit. --- target/i386/kvm/kvm.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 69825b53b6da..013c0359acbe 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -3147,7 +3147,7 @@ static int kvm_vm_enable_userspace_msr(KVMState *s) return 0; }-static void kvm_vm_enable_energy_msrs(KVMState *s)+static int kvm_vm_enable_energy_msrs(KVMState *s) { int ret;@@ -3157,7 +3157,7 @@ static void kvm_vm_enable_energy_msrs(KVMState *s)if (ret < 0) { error_report("Could not install MSR_RAPL_POWER_UNIT handler: %s", strerror(-ret)); - exit(1); + return ret; }ret = kvm_filter_msr(s, MSR_PKG_POWER_LIMIT,@@ -3165,7 +3165,7 @@ static void kvm_vm_enable_energy_msrs(KVMState *s) if (ret < 0) { error_report("Could not install MSR_PKG_POWER_LIMIT handler: %s", strerror(-ret)); - exit(1); + return ret; }ret = kvm_filter_msr(s, MSR_PKG_POWER_INFO,@@ -3173,17 +3173,17 @@ static void kvm_vm_enable_energy_msrs(KVMState *s) if (ret < 0) { error_report("Could not install MSR_PKG_POWER_INFO handler: %s", strerror(-ret)); - exit(1); + return ret; } ret = kvm_filter_msr(s, MSR_PKG_ENERGY_STATUS, kvm_rdmsr_pkg_energy_status, NULL); if (ret < 0) { error_report("Could not install MSR_PKG_ENERGY_STATUS handler: %s", strerror(-ret)); - exit(1); + return ret; } } - return; + return 0; }int kvm_arch_init(MachineState *ms, KVMState *s)@@ -3250,7 +3250,10 @@ int kvm_arch_init(MachineState *ms, KVMState *s) return ret; }- kvm_get_supported_feature_msrs(s);+ ret = kvm_get_supported_feature_msrs(s); + if (ret < 0) { + return ret; + }uname(&utsname);lm_capable_kernel = strcmp(utsname.machine, "x86_64") == 0; @@ -3286,6 +3289,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) if (ret < 0) { error_report("kvm: guest stopping CPU not supported: %s", strerror(-ret)); + return ret; } }@@ -3317,12 +3321,15 @@ int kvm_arch_init(MachineState *ms, KVMState *s)}if (s->msr_energy.enable == true) {- kvm_vm_enable_energy_msrs(s); + ret = kvm_vm_enable_energy_msrs(s); + if (ret < 0) { + return ret; + }ret = kvm_msr_energy_thread_init(s, ms);if (ret < 0) { error_report("kvm : error RAPL feature requirement not met"); - exit(1); + return ret; } } }
