| Message ID | 1387813240-13499-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 23.12.2013, at 16:40, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote: > From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> > > Targets like ppc64 support different typed of KVM, one which use > hypervisor mode and the other which doesn't. Add a new machine > property kvm-type that helps in selecting the respective ones > We also add a new QEMUMachine callback get_vm_type that helps > in mapping the string representation of kvm type specified. > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> Thanks, applied to ppc-next. Alex
Am 23.12.2013 16:40, schrieb Aneesh Kumar K.V: > From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> > > Targets like ppc64 support different typed of KVM, one which use "types" - Alex, please fix. :) > hypervisor mode and the other which doesn't. Add a new machine > property kvm-type that helps in selecting the respective ones There is no property being added in this patch, it's an "option". Please edit, also in subject. > We also add a new QEMUMachine callback get_vm_type that helps > in mapping the string representation of kvm type specified. > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > --- > Changes from V4: > * Fix build failure for ppc{,64}-linux-user > > hw/ppc/spapr.c | 19 +++++++++++++++++++ > include/hw/boards.h | 3 +++ > include/hw/xen/xen.h | 3 ++- > include/sysemu/kvm.h | 4 ++-- > include/sysemu/qtest.h | 3 ++- > kvm-all.c | 16 +++++++++++++--- > kvm-stub.c | 3 ++- > qtest.c | 2 +- > vl.c | 14 +++++++++----- > xen-all.c | 2 +- > xen-stub.c | 2 +- > 11 files changed, 55 insertions(+), 16 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 7e53a5f97781..267a47d6cc4d 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1357,6 +1357,24 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) > assert(spapr->fdt_skel != NULL); > } > > +static int spapr_kvm_type(const char *vm_type) > +{ > + if (!vm_type) { > + return 0; > + } > + > + if (!strcmp(vm_type, "HV")) { > + return 1; > + } > + > + if (!strcmp(vm_type, "PR")) { > + return 2; > + } > + > + hw_error("Unknown kvm-type specified '%s'", vm_type); error_report() - hw_error() would need \n IIRC and dumps CPU state, which seems useless at that point. > + exit(1); > +} > + > static QEMUMachine spapr_machine = { > .name = "pseries", > .desc = "pSeries Logical Partition (PAPR compliant)", > @@ -1367,6 +1385,7 @@ static QEMUMachine spapr_machine = { > .max_cpus = MAX_CPUS, > .no_parallel = 1, > .default_boot_order = NULL, > + .kvm_type = spapr_kvm_type, > }; > > static void spapr_machine_init(void) [...] > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 3b25f27a7cc5..400a2682923e 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -151,8 +151,8 @@ typedef struct KVMState KVMState; > extern KVMState *kvm_state; > > /* external API */ > - Line dropped accidentally? I guess external API doesn't just apply to the two lines added... > -int kvm_init(void); > +typedef struct QEMUMachine QEMUMachine; > +int kvm_init(QEMUMachine *machine); > > int kvm_has_sync_mmu(void); > int kvm_has_vcpu_events(void); > diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h > index 112a661ac4b0..7185174a39b2 100644 > --- a/include/sysemu/qtest.h > +++ b/include/sysemu/qtest.h > @@ -23,7 +23,8 @@ static inline bool qtest_enabled(void) > return qtest_allowed; > } > > -int qtest_init_accel(void); > +typedef struct QEMUMachine QEMUMachine; > +int qtest_init_accel(QEMUMachine *machine); > void qtest_init(const char *qtest_chrdev, const char *qtest_log); > > static inline int qtest_available(void) > diff --git a/kvm-all.c b/kvm-all.c > index 393775459d9f..57472804fe44 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -35,6 +35,8 @@ > #include "qemu/event_notifier.h" > #include "trace.h" > > +#include "hw/boards.h" > + > /* This check must be after config-host.h is included */ > #ifdef CONFIG_EVENTFD > #include <sys/eventfd.h> > @@ -1352,7 +1354,7 @@ static int kvm_max_vcpus(KVMState *s) > return (ret) ? ret : kvm_recommended_vcpus(s); > } > > -int kvm_init(void) > +int kvm_init(QEMUMachine *machine) > { > static const char upgrade_note[] = > "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n" > @@ -1369,7 +1371,8 @@ int kvm_init(void) > KVMState *s; > const KVMCapabilityInfo *missing_cap; > int ret; > - int i; > + int i, type = 0; > + const char *kvm_type; > > s = g_malloc0(sizeof(KVMState)); > > @@ -1442,7 +1445,14 @@ int kvm_init(void) > nc++; > } > > - s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0); > + kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type"); > + if (machine->kvm_type) { > + type = machine->kvm_type(kvm_type); > + } else if (kvm_type) { > + fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type); error_report()? (without \n then) > + goto err; > + } > + s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, type); > if (s->vmfd < 0) { > #ifdef TARGET_S390X > fprintf(stderr, "Please add the 'switch_amode' kernel parameter to " [snip] Almost all remarks could be edited into the patch file on the same line to avoid yet another respin. Andreas
On 23.12.2013, at 19:03, Andreas Färber <afaerber@suse.de> wrote: > Am 23.12.2013 16:40, schrieb Aneesh Kumar K.V: >> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> >> >> Targets like ppc64 support different typed of KVM, one which use > > "types" - Alex, please fix. :) > >> hypervisor mode and the other which doesn't. Add a new machine >> property kvm-type that helps in selecting the respective ones > > There is no property being added in this patch, it's an "option". Please > edit, also in subject. > >> We also add a new QEMUMachine callback get_vm_type that helps >> in mapping the string representation of kvm type specified. >> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> >> --- >> Changes from V4: >> * Fix build failure for ppc{,64}-linux-user >> >> hw/ppc/spapr.c | 19 +++++++++++++++++++ >> include/hw/boards.h | 3 +++ >> include/hw/xen/xen.h | 3 ++- >> include/sysemu/kvm.h | 4 ++-- >> include/sysemu/qtest.h | 3 ++- >> kvm-all.c | 16 +++++++++++++--- >> kvm-stub.c | 3 ++- >> qtest.c | 2 +- >> vl.c | 14 +++++++++----- >> xen-all.c | 2 +- >> xen-stub.c | 2 +- >> 11 files changed, 55 insertions(+), 16 deletions(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 7e53a5f97781..267a47d6cc4d 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -1357,6 +1357,24 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) >> assert(spapr->fdt_skel != NULL); >> } >> >> +static int spapr_kvm_type(const char *vm_type) >> +{ >> + if (!vm_type) { >> + return 0; >> + } >> + >> + if (!strcmp(vm_type, "HV")) { >> + return 1; >> + } >> + >> + if (!strcmp(vm_type, "PR")) { >> + return 2; >> + } >> + >> + hw_error("Unknown kvm-type specified '%s'", vm_type); > > error_report() - hw_error() would need \n IIRC and dumps CPU state, > which seems useless at that point. > >> + exit(1); >> +} >> + >> static QEMUMachine spapr_machine = { >> .name = "pseries", >> .desc = "pSeries Logical Partition (PAPR compliant)", >> @@ -1367,6 +1385,7 @@ static QEMUMachine spapr_machine = { >> .max_cpus = MAX_CPUS, >> .no_parallel = 1, >> .default_boot_order = NULL, >> + .kvm_type = spapr_kvm_type, >> }; >> >> static void spapr_machine_init(void) > [...] >> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h >> index 3b25f27a7cc5..400a2682923e 100644 >> --- a/include/sysemu/kvm.h >> +++ b/include/sysemu/kvm.h >> @@ -151,8 +151,8 @@ typedef struct KVMState KVMState; >> extern KVMState *kvm_state; >> >> /* external API */ >> - > > Line dropped accidentally? I guess external API doesn't just apply to > the two lines added... > >> -int kvm_init(void); >> +typedef struct QEMUMachine QEMUMachine; >> +int kvm_init(QEMUMachine *machine); >> >> int kvm_has_sync_mmu(void); >> int kvm_has_vcpu_events(void); >> diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h >> index 112a661ac4b0..7185174a39b2 100644 >> --- a/include/sysemu/qtest.h >> +++ b/include/sysemu/qtest.h >> @@ -23,7 +23,8 @@ static inline bool qtest_enabled(void) >> return qtest_allowed; >> } >> >> -int qtest_init_accel(void); >> +typedef struct QEMUMachine QEMUMachine; >> +int qtest_init_accel(QEMUMachine *machine); >> void qtest_init(const char *qtest_chrdev, const char *qtest_log); >> >> static inline int qtest_available(void) >> diff --git a/kvm-all.c b/kvm-all.c >> index 393775459d9f..57472804fe44 100644 >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -35,6 +35,8 @@ >> #include "qemu/event_notifier.h" >> #include "trace.h" >> >> +#include "hw/boards.h" >> + >> /* This check must be after config-host.h is included */ >> #ifdef CONFIG_EVENTFD >> #include <sys/eventfd.h> >> @@ -1352,7 +1354,7 @@ static int kvm_max_vcpus(KVMState *s) >> return (ret) ? ret : kvm_recommended_vcpus(s); >> } >> >> -int kvm_init(void) >> +int kvm_init(QEMUMachine *machine) >> { >> static const char upgrade_note[] = >> "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n" >> @@ -1369,7 +1371,8 @@ int kvm_init(void) >> KVMState *s; >> const KVMCapabilityInfo *missing_cap; >> int ret; >> - int i; >> + int i, type = 0; >> + const char *kvm_type; >> >> s = g_malloc0(sizeof(KVMState)); >> >> @@ -1442,7 +1445,14 @@ int kvm_init(void) >> nc++; >> } >> >> - s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0); >> + kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type"); >> + if (machine->kvm_type) { >> + type = machine->kvm_type(kvm_type); >> + } else if (kvm_type) { >> + fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type); > > error_report()? (without \n then) I agree to the others, but that particular one is very consistent with the code around it. I don't want two different error reporting paths in the same function. > >> + goto err; >> + } >> + s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, type); >> if (s->vmfd < 0) { >> #ifdef TARGET_S390X >> fprintf(stderr, "Please add the 'switch_amode' kernel parameter to " > [snip] > > Almost all remarks could be edited into the patch file on the same line > to avoid yet another respin. Fixed them inline. Alex
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 7e53a5f97781..267a47d6cc4d 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1357,6 +1357,24 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) assert(spapr->fdt_skel != NULL); } +static int spapr_kvm_type(const char *vm_type) +{ + if (!vm_type) { + return 0; + } + + if (!strcmp(vm_type, "HV")) { + return 1; + } + + if (!strcmp(vm_type, "PR")) { + return 2; + } + + hw_error("Unknown kvm-type specified '%s'", vm_type); + exit(1); +} + static QEMUMachine spapr_machine = { .name = "pseries", .desc = "pSeries Logical Partition (PAPR compliant)", @@ -1367,6 +1385,7 @@ static QEMUMachine spapr_machine = { .max_cpus = MAX_CPUS, .no_parallel = 1, .default_boot_order = NULL, + .kvm_type = spapr_kvm_type, }; static void spapr_machine_init(void) diff --git a/include/hw/boards.h b/include/hw/boards.h index 2151460f9ec4..89eb6651e2c7 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -24,6 +24,8 @@ typedef void QEMUMachineResetFunc(void); typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp); +typedef int QEMUMachineGetKvmtypeFunc(const char *arg); + struct QEMUMachine { const char *name; const char *alias; @@ -31,6 +33,7 @@ struct QEMUMachine { QEMUMachineInitFunc *init; QEMUMachineResetFunc *reset; QEMUMachineHotAddCPUFunc *hot_add_cpu; + QEMUMachineGetKvmtypeFunc *kvm_type; BlockInterfaceType block_default_type; int max_cpus; unsigned int no_serial:1, diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h index e1f88bf9cf8f..acc3d74c0911 100644 --- a/include/hw/xen/xen.h +++ b/include/hw/xen/xen.h @@ -36,7 +36,8 @@ void xen_cmos_set_s3_resume(void *opaque, int irq, int level); qemu_irq *xen_interrupt_controller_init(void); -int xen_init(void); +typedef struct QEMUMachine QEMUMachine; +int xen_init(QEMUMachine *machine); int xen_hvm_init(MemoryRegion **ram_memory); void xenstore_store_pv_console_info(int i, struct CharDriverState *chr); diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 3b25f27a7cc5..400a2682923e 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -151,8 +151,8 @@ typedef struct KVMState KVMState; extern KVMState *kvm_state; /* external API */ - -int kvm_init(void); +typedef struct QEMUMachine QEMUMachine; +int kvm_init(QEMUMachine *machine); int kvm_has_sync_mmu(void); int kvm_has_vcpu_events(void); diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h index 112a661ac4b0..7185174a39b2 100644 --- a/include/sysemu/qtest.h +++ b/include/sysemu/qtest.h @@ -23,7 +23,8 @@ static inline bool qtest_enabled(void) return qtest_allowed; } -int qtest_init_accel(void); +typedef struct QEMUMachine QEMUMachine; +int qtest_init_accel(QEMUMachine *machine); void qtest_init(const char *qtest_chrdev, const char *qtest_log); static inline int qtest_available(void) diff --git a/kvm-all.c b/kvm-all.c index 393775459d9f..57472804fe44 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -35,6 +35,8 @@ #include "qemu/event_notifier.h" #include "trace.h" +#include "hw/boards.h" + /* This check must be after config-host.h is included */ #ifdef CONFIG_EVENTFD #include <sys/eventfd.h> @@ -1352,7 +1354,7 @@ static int kvm_max_vcpus(KVMState *s) return (ret) ? ret : kvm_recommended_vcpus(s); } -int kvm_init(void) +int kvm_init(QEMUMachine *machine) { static const char upgrade_note[] = "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n" @@ -1369,7 +1371,8 @@ int kvm_init(void) KVMState *s; const KVMCapabilityInfo *missing_cap; int ret; - int i; + int i, type = 0; + const char *kvm_type; s = g_malloc0(sizeof(KVMState)); @@ -1442,7 +1445,14 @@ int kvm_init(void) nc++; } - s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0); + kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type"); + if (machine->kvm_type) { + type = machine->kvm_type(kvm_type); + } else if (kvm_type) { + fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type); + goto err; + } + s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, type); if (s->vmfd < 0) { #ifdef TARGET_S390X fprintf(stderr, "Please add the 'switch_amode' kernel parameter to " diff --git a/kvm-stub.c b/kvm-stub.c index e979f76d0782..6dd4454a640c 100644 --- a/kvm-stub.c +++ b/kvm-stub.c @@ -28,13 +28,14 @@ bool kvm_gsi_routing_allowed; bool kvm_gsi_direct_mapping; bool kvm_allowed; bool kvm_readonly_mem_allowed; +typedef struct QEMUMachine QEMUMachine; int kvm_init_vcpu(CPUState *cpu) { return -ENOSYS; } -int kvm_init(void) +int kvm_init(QEMUMachine *machine) { return -ENOSYS; } diff --git a/qtest.c b/qtest.c index dcf130122961..b156225e7278 100644 --- a/qtest.c +++ b/qtest.c @@ -500,7 +500,7 @@ static void qtest_event(void *opaque, int event) } } -int qtest_init_accel(void) +int qtest_init_accel(QEMUMachine *machine) { configure_icount("0"); diff --git a/vl.c b/vl.c index 7511e7036c1f..c2689494596e 100644 --- a/vl.c +++ b/vl.c @@ -430,6 +430,10 @@ static QemuOptsList qemu_machine_opts = { .name = "firmware", .type = QEMU_OPT_STRING, .help = "firmware image", + },{ + .name = "kvm-type", + .type = QEMU_OPT_STRING, + .help = "Specifies the KVM virtualization mode (HV, PR)", }, { /* End of list */ } }, @@ -2608,7 +2612,7 @@ static QEMUMachine *machine_parse(const char *name) exit(!name || !is_help_option(name)); } -static int tcg_init(void) +static int tcg_init(QEMUMachine *machine) { tcg_exec_init(tcg_tb_size * 1024 * 1024); return 0; @@ -2618,7 +2622,7 @@ static struct { const char *opt_name; const char *name; int (*available)(void); - int (*init)(void); + int (*init)(QEMUMachine *); bool *allowed; } accel_list[] = { { "tcg", "tcg", tcg_available, tcg_init, &tcg_allowed }, @@ -2627,7 +2631,7 @@ static struct { { "qtest", "QTest", qtest_available, qtest_init_accel, &qtest_allowed }, }; -static int configure_accelerator(void) +static int configure_accelerator(QEMUMachine *machine) { const char *p; char buf[10]; @@ -2654,7 +2658,7 @@ static int configure_accelerator(void) continue; } *(accel_list[i].allowed) = true; - ret = accel_list[i].init(); + ret = accel_list[i].init(machine); if (ret < 0) { init_failed = true; fprintf(stderr, "failed to initialize %s: %s\n", @@ -4048,7 +4052,7 @@ int main(int argc, char **argv, char **envp) exit(0); } - configure_accelerator(); + configure_accelerator(machine); if (qtest_chrdev) { qtest_init(qtest_chrdev, qtest_log); diff --git a/xen-all.c b/xen-all.c index 4a594bdd9b56..ba3473901e6a 100644 --- a/xen-all.c +++ b/xen-all.c @@ -1001,7 +1001,7 @@ static void xen_exit_notifier(Notifier *n, void *data) xs_daemon_close(state->xenstore); } -int xen_init(void) +int xen_init(QEMUMachine *machine) { xen_xc = xen_xc_interface_open(0, 0, 0); if (xen_xc == XC_HANDLER_INITIAL_VALUE) { diff --git a/xen-stub.c b/xen-stub.c index ad189a6df8c2..59927cb5d635 100644 --- a/xen-stub.c +++ b/xen-stub.c @@ -47,7 +47,7 @@ qemu_irq *xen_interrupt_controller_init(void) return NULL; } -int xen_init(void) +int xen_init(QEMUMachine *machine) { return -ENOSYS; }