On 23.12.2013, at 19:03, Andreas Färber <afaer...@suse.de> wrote:
> Am 23.12.2013 16:40, schrieb Aneesh Kumar K.V:
>> From: "Aneesh Kumar K.V" <aneesh.ku...@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.ku...@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
