| Message ID | 1729178055-207271-3-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | precreate phase | expand |
cc Xen, whpx, and nvmm accelerator maintainers for this accelerator-specific patch. The cover letter for this series is here: https://lore.kernel.org/qemu-devel/1729178055-207271-1-git-send-email-steven.sistare@oracle.com - Steve On 10/17/2024 11:14 AM, 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.sistare@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: > 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); > #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; >
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.sistare@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 >
On 10/17/24 17:14, 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. For KVM I would also include missing_cap = kvm_check_extension_list(s, kvm_required_capabilites); if (!missing_cap) { missing_cap = kvm_check_extension_list(s, kvm_arch_required_capabilities); } With this change, patches 1-3 are okay as a separate submission. Paolo > Signed-off-by: Steve Sistare <steven.sistare@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: > 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); > #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; >
On 10/21/2024 10:54 AM, Peter Xu wrote: > 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.sistare@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. I agree this is ugly, but the diff is small and easy to understand, and patch 3 makes it pretty again: @@ -36,14 +36,8 @@ 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: ms->accelerator = NULL; - Steve > 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 >> >
On 10/23/2024 11:53 AM, Paolo Bonzini wrote: > On 10/17/24 17:14, 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. > > For KVM I would also include > > missing_cap = kvm_check_extension_list(s, kvm_required_capabilites); > if (!missing_cap) { > missing_cap = > kvm_check_extension_list(s, kvm_arch_required_capabilities); > } > > With this change, patches 1-3 are okay as a separate submission. Thanks, will do and will re-submit - steve >> Signed-off-by: Steve Sistare <steven.sistare@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: >> 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); >> #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; >
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: 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); #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;
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.sistare@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(-)