Re: [RFC PATCH 00/11 Allow PR and HV KVM to coexist in one kernel

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

Alexander Graf <agraf@suse.de> writes:

> On 27.09.2013, at 12:52, Aneesh Kumar K.V wrote:
>
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
>> 
>>> Hi All,
>>> 
>>> This patch series support enabling HV and PR KVM together in the same kernel. We
>>> extend machine property with new property "kvm_type". A value of 1 will force HV
>>> KVM and 2 PR KVM. The default value is 0 which will select the fastest KVM mode.
>>> ie, HV if that is supported otherwise PR.
>>> 
>>> With Qemu command line having
>>> 
>>> -machine pseries,accel=kvm,kvm_type=1
>>> 
>>> [root@llmp24l02 qemu]# bash ../qemu
>>> failed to initialize KVM: Invalid argument
>>> [root@llmp24l02 qemu]# modprobe kvm-pr
>>> [root@llmp24l02 qemu]# bash ../qemu
>>> failed to initialize KVM: Invalid argument
>>> [root@llmp24l02 qemu]# modprobe  kvm-hv
>>> [root@llmp24l02 qemu]# bash ../qemu
>>> 
>>> now with
>>> 
>>> -machine pseries,accel=kvm,kvm_type=2
>>> 
>>> [root@llmp24l02 qemu]# rmmod kvm-pr
>>> [root@llmp24l02 qemu]# bash ../qemu
>>> failed to initialize KVM: Invalid argument
>>> [root@llmp24l02 qemu]#
>>> [root@llmp24l02 qemu]# modprobe kvm-pr
>>> [root@llmp24l02 qemu]# bash ../qemu
>>> 
>>> if don't specify kvm_type machine property, it will take a default value 0,
>>> which means fastest supported.
>> 
>> Related qemu patch
>> 
>> commit 8d139053177d48a70cb710b211ea4c2843eccdfb
>> Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> Date:   Mon Sep 23 12:28:37 2013 +0530
>> 
>>    kvm: Add a new machine property kvm_type
>> 
>>    Targets like ppc64 support different type 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
>> 
>>    Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
> This really is too early, as we can't possibly run in HV mode for
> non-pseries machines, so the interpretation (or at least sanity
> checking) of what values are reasonable should occur in the
> machine. That's why it's a variable in the "machine opts".

With the current code CREATE_VM will fail, because we won't have
kvm-hv.ko loaded and trying to create a vm with type 1 will fail. 
Now the challenge related to moving that to machine_init or later is, we
depend on HV or PR callback early in CREATE_VM. With the changes we have

int kvmppc_core_init_vm(struct kvm *kvm)
{

#ifdef CONFIG_PPC64
	INIT_LIST_HEAD(&kvm->arch.spapr_tce_tables);
	INIT_LIST_HEAD(&kvm->arch.rtas_tokens);
#endif

	return kvm->arch.kvm_ops->init_vm(kvm);
}

Also the mmu notifier callback do end up calling kvm_unmap_hva etc which
are all HV/PR dependent. 



>
> Also, users don't want to say type=0. They want to say type=PR or
> type=HV or type=HV,PR. In fact, can't you make this a property of
> -accel? Then it's truly accel specific and everything should be well.

If we are doing this as machine property, we can't specify string,
because "HV"/"PR" are all powerpc dependent, so parsing that is not
possible in kvm_init in qemu. But, yes ideally it would be nice to be
able to speicy the type using string. I thought accel is a machine
property, hence was not sure whether I can have additional properties
against that. I was using it as below.

 -machine pseries,accel=kvm,kvm_type=1

will look into more details to check whether this can be accel property.

-aneesh