Re: [PATCH 07/28] kvm tools: Move 'kvm__recommended_cpus' to arch-specific code

Alexander Graf
2011-12-20T15:23:28Z

On 07.12.2011, at 09:29, Sasha Levin wrote:

> On Wed, 2011-12-07 at 18:28 +1100, Matt Evans wrote:
>> On 07/12/11 18:24, Alexander Graf wrote:
>>>
>>> On 07.12.2011, at 08:19, Matt Evans <matt@ozlabs.org> wrote:
>>>
>>>> On 07/12/11 17:34, Sasha Levin wrote:
>>>>> On Wed, 2011-12-07 at 17:17 +1100, Matt Evans wrote:
>>>>>> On 06/12/11 19:20, Sasha Levin wrote:
>>>>>>> Why is it getting moved out of generic code?
>>>>>>>
>>>>>>> This is used to determine the maximum amount of vcpus supported by the
>>>>>>> host for a single guest, and as far as I know KVM_CAP_NR_VCPUS and
>>>>>>> KVM_CAP_MAX_VCPUS are not arch specific.
>>>>>>
>>>>>> I checked api.txt and you're right, it isn't arch-specific. I assumed it was,
>>>>>> because PPC KVM doesn't support it ;-) I've dropped this patch and in its place
>>>>>> implemented the api.txt suggestion of "if KVM_CAP_NR_VCPUS fails, use 4" instead
>>>>>> of die(); you'll see that when I repost.
>>>>>>
>>>>>> This will have the effect of PPC being limited to 4 CPUs until the kernel
>>>>>> supports that CAP. (I'll see about this part too.)
>>>>>
>>>>> I went to look at which limitation PPC places on amount of vcpus in
>>>>> guest, and saw this in kvmppc_core_vcpu_create() in the book3s code:
>>>>>
>>>>> vcpu = kvmppc_core_vcpu_create(kvm, id);
>>>>> vcpu->arch.wqp = &vcpu->wq;
>>>>> if (!IS_ERR(vcpu))
>>>>> kvmppc_create_vcpu_debugfs(vcpu, id);
>>>>>
>>>>> This is wrong, right? The VCPU is dereferenced before actually checking
>>>>> that it's not an error.
>>>>
>>>> Yeah, that's b0rk. Alex, a patch below. :)
>>>
>>> Thanks :). Will apply asap but don't have a real keyboard today :).
>>
>> Ha! Voice control on your phone, what could go wrong?
>>
>>> I suppose this is stable material?
>>
>> Good idea, (and if we're formal,
>> Signed-off-by: Matt Evans <matt@ozlabs.org>
>> ). I suppose no one's seen a vcpu fail to be created, yet.
>
> I also got another one, but it's **completely untested** (not even
> compiled). Alex, Matt, any chance one of you can loan a temporary ppc
> shell for the upcoming tests of KVM tool/ppc KVM?

The problem with giving you a shell on a PPC box is really that the hardware Matt's work is focusing on is not available to the public yet. I could maybe try and get you access on a G5 box, but I'm not sure how useful that is to you really, as you'll only be able to run PR KVM, not HV KVM.

>
> ---
>
> From: Sasha Levin <levinsasha928@gmail.com>
> Date: Wed, 7 Dec 2011 10:24:56 +0200
> Subject: [PATCH] KVM: PPC: Use the vcpu kmem_cache when allocating new VCPUs
>
> Currently the code kzalloc()s new VCPUs instead of using the kmem_cache
> which is created when KVM is initialized.
>
> Modify it to allocate VCPUs from that kmem_cache.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 0cb137a..e309099 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -411,7 +411,7 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
> goto out;
>
> err = -ENOMEM;
> - vcpu = kzalloc(sizeof(struct kvm_vcpu), GFP_KERNEL);
> + vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);

Paul, is there any rationale on why not to use the kmem cache? Are we bound by real mode magic again?

Alex

> if (!vcpu)
> goto out;
>
> @@ -463,7 +463,7 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
> return vcpu;
>
> free_vcpu:
> - kfree(vcpu);
> + kmem_cache_free(kvm_vcpu_cache, vcpu);
> out:
> return ERR_PTR(err);
> }
> @@ -471,7 +471,7 @@ out:
> void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
> {
> kvm_vcpu_uninit(vcpu);
> - kfree(vcpu);
> + kmem_cache_free(kvm_vcpu_cache, vcpu);
> }
>
> static void kvmppc_set_timer(struct kvm_vcpu *vcpu)
>
> --
>
> Sasha.
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html