On Thu, Oct 17, 2024 at 08:14:03AM -0700, Steve Sistare wrote:
Extract the first part of the AccelState init_machine function into
a new function preinit, which can be called without knowing any
machine properties. For now call preinit and init_machine at the
same place, so no functional change.
Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
---
accel/accel-system.c | 6 +++++
accel/kvm/kvm-all.c | 58 +++++++++++++++++++++++++++++----------------
accel/xen/xen-all.c | 11 ++++++---
include/qemu/accel.h | 1 +
target/i386/nvmm/nvmm-all.c | 10 +++++++-
target/i386/whpx/whpx-all.c | 14 +++++++----
6 files changed, 70 insertions(+), 30 deletions(-)
diff --git a/accel/accel-system.c b/accel/accel-system.c
index f6c947d..fef6625 100644
--- a/accel/accel-system.c
+++ b/accel/accel-system.c
@@ -36,8 +36,14 @@ int accel_init_machine(AccelState *accel, MachineState *ms)
int ret;
ms->accelerator = accel;
*(acc->allowed) = true;
+ ret = acc->preinit(accel);
+ if (ret < 0) {
+ goto fail;
+ }
+
ret = acc->init_machine(ms);
if (ret < 0) {
+fail:
Jump into another failure path's if clause might be error prone in the
future.
Maybe move the error handling out while at it, e.g.:
ret = acc->init_machine();
if (ret < 0) {
goto fail;
}
object_set_accelerator_compat_props(acc->compat_props);
return 0;
fail:
ms->accelerator = NULL;
*(acc->allowed) = false;
object_unref(OBJECT(accel));
return ret;
ms->accelerator = NULL;
*(acc->allowed) = false;
object_unref(OBJECT(accel));
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 905fb84..c7c6001 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2484,6 +2484,42 @@ static int kvm_setup_dirty_ring(KVMState *s)
return 0;
}
+static int kvm_preinit(AccelState *accel)
+{
+ int ret;
+ KVMState *s = KVM_STATE(accel);
+
+ s->fd = qemu_open_old(s->device ?: "/dev/kvm", O_RDWR);
+ if (s->fd == -1) {
+ error_report("Could not access KVM kernel module: %m");
+ ret = -errno;
+ goto err;
+ }
+
+ ret = kvm_ioctl(s, KVM_GET_API_VERSION, 0);
+ if (ret < KVM_API_VERSION) {
+ if (ret >= 0) {
+ ret = -EINVAL;
+ }
+ fprintf(stderr, "kvm version too old\n");
+ goto err;
+ }
+
+ if (ret > KVM_API_VERSION) {
+ ret = -EINVAL;
+ error_report("kvm version not supported");
+ goto err;
+ }
+ return 0;
+
+err:
+ assert(ret < 0);
+ if (s->fd != -1) {
+ close(s->fd);
+ }
+ return ret;
+}
+
static int kvm_init(MachineState *ms)
{
MachineClass *mc = MACHINE_GET_CLASS(ms);
@@ -2523,27 +2559,6 @@ static int kvm_init(MachineState *ms)
QTAILQ_INIT(&s->kvm_sw_breakpoints);
#endif
QLIST_INIT(&s->kvm_parked_vcpus);
- s->fd = qemu_open_old(s->device ?: "/dev/kvm", O_RDWR);
- if (s->fd == -1) {
- error_report("Could not access KVM kernel module: %m");
- ret = -errno;
- goto err;
- }
-
- ret = kvm_ioctl(s, KVM_GET_API_VERSION, 0);
- if (ret < KVM_API_VERSION) {
- if (ret >= 0) {
- ret = -EINVAL;
- }
- error_report("kvm version too old");
- goto err;
- }
-
- if (ret > KVM_API_VERSION) {
- ret = -EINVAL;
- error_report("kvm version not supported");
- goto err;
- }
kvm_supported_memory_attributes = kvm_check_extension(s, KVM_CAP_MEMORY_ATTRIBUTES);
kvm_guest_memfd_supported =
@@ -3891,6 +3906,7 @@ static void kvm_accel_class_init(ObjectClass *oc, void
*data)
{
AccelClass *ac = ACCEL_CLASS(oc);
ac->name = "KVM";
+ ac->preinit = kvm_preinit;
ac->init_machine = kvm_init;
ac->has_memory = kvm_accel_has_memory;
ac->allowed = &kvm_allowed;
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 0bdefce..dfcb90c 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -75,10 +75,8 @@ static void xen_setup_post(MachineState *ms, AccelState
*accel)
}
}
-static int xen_init(MachineState *ms)
+static int xen_preinit(AccelState *accel)
{
- MachineClass *mc = MACHINE_GET_CLASS(ms);
-
xen_xc = xc_interface_open(0, 0, 0);
if (xen_xc == NULL) {
xen_pv_printf(NULL, 0, "can't open xen interface\n");
@@ -97,6 +95,12 @@ static int xen_init(MachineState *ms)
xc_interface_close(xen_xc);
return -1;
}
+ return 0;
+}
+
+static int xen_init(MachineState *ms)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
/*
* The XenStore write would fail when running restricted so don't attempt
@@ -125,6 +129,7 @@ static void xen_accel_class_init(ObjectClass *oc, void
*data)
};
ac->name = "Xen";
+ ac->preinit = xen_preinit;
ac->init_machine = xen_init;
ac->setup_post = xen_setup_post;
ac->allowed = &xen_allowed;
diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index 972a849..6b3b1cf 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -37,6 +37,7 @@ typedef struct AccelClass {
/*< public >*/
const char *name;
+ int (*preinit)(AccelState *accel);
int (*init_machine)(MachineState *ms);
Might be nice to add some comment on what should be part of preinit() and
what should be part of init_machine().
AFAIU the preinit() was about probing whether an accel is supported. Maybe
rename it to probe()? Then it's also clear why some accel (e.g. tcg)
doesn't need that, because it is always supported and no probe is needed.
#ifndef CONFIG_USER_ONLY
void (*setup_post)(MachineState *ms, AccelState *accel);
diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
index 65768ac..ce858a0 100644
--- a/target/i386/nvmm/nvmm-all.c
+++ b/target/i386/nvmm/nvmm-all.c
@@ -1153,7 +1153,7 @@ static struct RAMBlockNotifier nvmm_ram_notifier = {
/* --------------------------------------------------------------------------
*/
static int
-nvmm_accel_init(MachineState *ms)
+nvmm_accel_preinit(MachineState *ms)
{
int ret, err;
@@ -1178,6 +1178,13 @@ nvmm_accel_init(MachineState *ms)
error_report("NVMM: Wrong state size %u", qemu_mach.cap.state_size);
return -EPROGMISMATCH;
}
+ return 0;
+}
+
+static int
+nvmm_accel_init(MachineState *ms)
+{
+ int ret, err;
ret = nvmm_machine_create(&qemu_mach.mach);
if (ret == -1) {
@@ -1204,6 +1211,7 @@ nvmm_accel_class_init(ObjectClass *oc, void *data)
{
AccelClass *ac = ACCEL_CLASS(oc);
ac->name = "NVMM";
+ ac->preinit = nvmm_accel_preinit;
ac->init_machine = nvmm_accel_init;
ac->allowed = &nvmm_allowed;
}
diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
index a6674a8..50bfc19 100644
--- a/target/i386/whpx/whpx-all.c
+++ b/target/i386/whpx/whpx-all.c
@@ -2516,6 +2516,14 @@ static void whpx_set_kernel_irqchip(Object *obj, Visitor
*v,
* Partition support
*/
+static int whpx_accel_preinit(AccelState *accel)
+{
+ if (!init_whp_dispatch()) {
+ return -ENOSYS;
+ }
+ return 0;
+}
+
static int whpx_accel_init(MachineState *ms)
{
struct whpx_state *whpx;
@@ -2529,11 +2537,6 @@ static int whpx_accel_init(MachineState *ms)
whpx = &whpx_global;
- if (!init_whp_dispatch()) {
- ret = -ENOSYS;
- goto error;
- }
-
whpx->mem_quota = ms->ram_size;
hr = whp_dispatch.WHvGetCapability(
@@ -2713,6 +2716,7 @@ static void whpx_accel_class_init(ObjectClass *oc, void
*data)
{
AccelClass *ac = ACCEL_CLASS(oc);
ac->name = "WHPX";
+ ac->preinit = whpx_accel_preinit;
ac->init_machine = whpx_accel_init;
ac->allowed = &whpx_allowed;
--
1.8.3.1
