[PATCH v5 08/11] target/i386/kvm: Clean up return values of MSR filter related functions

Zhao Liu
2024-11-06T02:50:14Z

Before commit 0cc42e63bb54 ("kvm/i386: refactor kvm_arch_init and split
it into smaller functions"), error_report() attempts to print the error
code from kvm_filter_msr(). However, printing error code does not work
due to kvm_filter_msr() returns bool instead int.

0cc42e63bb54 fixed the error by removing error code printing, but this
lost useful error messages. Bring it back by making kvm_filter_msr()
return int.

This also makes the function call chain processing clearer, allowing for
better handling of error result propagation from kvm_filter_msr() to
kvm_arch_init(), preparing for the subsequent cleanup work of error
handling in kvm_arch_init().

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Zide Chen <zide.chen@intel.com>
---
v5: Rebased and cleaned up kvm_vm_enable_energy_msrs() as well.
v4: Returned kvm_vm_ioctl() directly. (Zide)
v3: new commit.
---
target/i386/kvm/kvm.c | 87 ++++++++++++++++++++++---------------------
1 file changed, 44 insertions(+), 43 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 1fb4bd19fcf7..9993382fb40e 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -108,8 +108,8 @@ typedef struct {
} KVMMSRHandlers;

static void kvm_init_msrs(X86CPU *cpu);
-static bool kvm_filter_msr(KVMState *s, uint32_t msr, QEMURDMSRHandler *rdmsr,
- QEMUWRMSRHandler *wrmsr);
+static int kvm_filter_msr(KVMState *s, uint32_t msr, QEMURDMSRHandler *rdmsr,
+ QEMUWRMSRHandler *wrmsr);

const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
KVM_CAP_INFO(SET_TSS_ADDR),
@@ -3135,17 +3135,21 @@ static int kvm_vm_enable_notify_vmexit(KVMState *s)

static int kvm_vm_enable_userspace_msr(KVMState *s)
{
- int ret = kvm_vm_enable_cap(s, KVM_CAP_X86_USER_SPACE_MSR, 0,
- KVM_MSR_EXIT_REASON_FILTER);
+ int ret;
+
+ ret = kvm_vm_enable_cap(s, KVM_CAP_X86_USER_SPACE_MSR, 0,
+ KVM_MSR_EXIT_REASON_FILTER);
if (ret < 0) {
error_report("Could not enable user space MSRs: %s",
strerror(-ret));
exit(1);
}

- if (!kvm_filter_msr(s, MSR_CORE_THREAD_COUNT,
- kvm_rdmsr_core_thread_count, NULL)) {
- error_report("Could not install MSR_CORE_THREAD_COUNT handler!");
+ ret = kvm_filter_msr(s, MSR_CORE_THREAD_COUNT,
+ kvm_rdmsr_core_thread_count, NULL);
+ if (ret < 0) {
+ error_report("Could not install MSR_CORE_THREAD_COUNT handler: %s",
+ strerror(-ret));
exit(1);
}

@@ -3154,36 +3158,37 @@ static int kvm_vm_enable_userspace_msr(KVMState *s)

static void kvm_vm_enable_energy_msrs(KVMState *s)
{
- bool r;
+ int ret;
+
if (s->msr_energy.enable == true) {
- r = kvm_filter_msr(s, MSR_RAPL_POWER_UNIT,
- kvm_rdmsr_rapl_power_unit, NULL);
- if (!r) {
- error_report("Could not install MSR_RAPL_POWER_UNIT \
- handler");
+ ret = kvm_filter_msr(s, MSR_RAPL_POWER_UNIT,
+ kvm_rdmsr_rapl_power_unit, NULL);
+ if (ret < 0) {
+ error_report("Could not install MSR_RAPL_POWER_UNIT handler: %s",
+ strerror(-ret));
exit(1);
}

- r = kvm_filter_msr(s, MSR_PKG_POWER_LIMIT,
- kvm_rdmsr_pkg_power_limit, NULL);
- if (!r) {
- error_report("Could not install MSR_PKG_POWER_LIMIT \
- handler");
+ ret = kvm_filter_msr(s, MSR_PKG_POWER_LIMIT,
+ kvm_rdmsr_pkg_power_limit, NULL);
+ if (ret < 0) {
+ error_report("Could not install MSR_PKG_POWER_LIMIT handler: %s",
+ strerror(-ret));
exit(1);
}

- r = kvm_filter_msr(s, MSR_PKG_POWER_INFO,
- kvm_rdmsr_pkg_power_info, NULL);
- if (!r) {
- error_report("Could not install MSR_PKG_POWER_INFO \
- handler");
+ ret = kvm_filter_msr(s, MSR_PKG_POWER_INFO,
+ kvm_rdmsr_pkg_power_info, NULL);
+ if (ret < 0) {
+ error_report("Could not install MSR_PKG_POWER_INFO handler: %s",
+ strerror(-ret));
exit(1);
}
- r = kvm_filter_msr(s, MSR_PKG_ENERGY_STATUS,
- kvm_rdmsr_pkg_energy_status, NULL);
- if (!r) {
- error_report("Could not install MSR_PKG_ENERGY_STATUS \
- handler");
+ 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);
}
}
@@ -5842,13 +5847,13 @@ void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
}
}

-static bool kvm_install_msr_filters(KVMState *s)
+static int kvm_install_msr_filters(KVMState *s)
{
uint64_t zero = 0;
struct kvm_msr_filter filter = {
.flags = KVM_MSR_FILTER_DEFAULT_ALLOW,
};
- int r, i, j = 0;
+ int i, j = 0;

for (i = 0; i < KVM_MSR_FILTER_MAX_RANGES; i++) {
KVMMSRHandlers *handler = &msr_handlers[i];
@@ -5872,18 +5877,13 @@ static bool kvm_install_msr_filters(KVMState *s)
}
}

- r = kvm_vm_ioctl(s, KVM_X86_SET_MSR_FILTER, &filter);
- if (r) {
- return false;
- }
-
- return true;
+ return kvm_vm_ioctl(s, KVM_X86_SET_MSR_FILTER, &filter);
}

-static bool kvm_filter_msr(KVMState *s, uint32_t msr, QEMURDMSRHandler *rdmsr,
- QEMUWRMSRHandler *wrmsr)
+static int kvm_filter_msr(KVMState *s, uint32_t msr, QEMURDMSRHandler *rdmsr,
+ QEMUWRMSRHandler *wrmsr)
{
- int i;
+ int i, ret;

for (i = 0; i < ARRAY_SIZE(msr_handlers); i++) {
if (!msr_handlers[i].msr) {
@@ -5893,16 +5893,17 @@ static bool kvm_filter_msr(KVMState *s, uint32_t msr, QEMURDMSRHandler *rdmsr,
.wrmsr = wrmsr,
};

- if (!kvm_install_msr_filters(s)) {
+ ret = kvm_install_msr_filters(s);
+ if (ret) {
msr_handlers[i] = (KVMMSRHandlers) { };
- return false;
+ return ret;
}

- return true;
+ return 0;
}
}

- return false;
+ return -EINVAL;
}

static int kvm_handle_rdmsr(X86CPU *cpu, struct kvm_run *run)
--
2.34.1