Avi Kivity wrote:
> On 06/26/2010 02:24 AM, Alexander Graf wrote:
>> For transparent variable sharing between the hypervisor and guest, I
>> introduce
>> a shared page. This shared page will contain all the registers the
>> guest can
>> read and write safely without exiting guest context.
>>
>> This patch only implements the stubs required for the basic structure
>> of the
>> shared page. The actual register moving follows.
>>
>>
>> @@ -123,8 +123,14 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct
>> kvm *kvm, unsigned int id)
>> if (err)
>> goto free_vcpu;
>>
>> + vcpu->arch.shared = (void*)__get_free_page(GFP_KERNEL|__GFP_ZERO);
>> + if (!vcpu->arch.shared)
>> + goto uninit_vcpu;
>> +
>> return vcpu;
>>
>> +uninit_vcpu:
>> + kvm_vcpu_uninit(vcpu);
>> free_vcpu:
>> kmem_cache_free(kvm_vcpu_cache, vcpu_44x);
>> out:
>> @@ -135,6 +141,7 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
>> {
>> struct kvmppc_vcpu_44x *vcpu_44x = to_44x(vcpu);
>>
>> + free_page((unsigned long)vcpu->arch.shared);
>> kvm_vcpu_uninit(vcpu);
>> kmem_cache_free(kvm_vcpu_cache, vcpu_44x);
>> }
>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>> index 884d4a5..ba79b35 100644
>> --- a/arch/powerpc/kvm/book3s.c
>> +++ b/arch/powerpc/kvm/book3s.c
>> @@ -1247,6 +1247,10 @@ struct kvm_vcpu
>> *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
>> if (err)
>> goto free_shadow_vcpu;
>>
>> + vcpu->arch.shared = (void*)__get_free_page(GFP_KERNEL|__GFP_ZERO);
>> + if (!vcpu->arch.shared)
>> + goto uninit_vcpu;
>> +
>> vcpu->arch.host_retip = kvm_return_point;
>> vcpu->arch.host_msr = mfmsr();
>> #ifdef CONFIG_PPC_BOOK3S_64
>> @@ -1277,6 +1281,8 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct
>> kvm *kvm, unsigned int id)
>>
>> return vcpu;
>>
>> +uninit_vcpu:
>> + kvm_vcpu_uninit(vcpu);
>> free_shadow_vcpu:
>> kfree(vcpu_book3s->shadow_vcpu);
>> free_vcpu:
>> @@ -1289,6 +1295,7 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
>> {
>> struct kvmppc_vcpu_book3s *vcpu_book3s = to_book3s(vcpu);
>>
>> + free_page((unsigned long)vcpu->arch.shared);
>> kvm_vcpu_uninit(vcpu);
>> kfree(vcpu_book3s->shadow_vcpu);
>> vfree(vcpu_book3s);
>> diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c
>> index e8a00b0..71750f2 100644
>> --- a/arch/powerpc/kvm/e500.c
>> +++ b/arch/powerpc/kvm/e500.c
>> @@ -117,8 +117,14 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct
>> kvm *kvm, unsigned int id)
>> if (err)
>> goto uninit_vcpu;
>>
>> + vcpu->arch.shared = (void*)__get_free_page(GFP_KERNEL|__GFP_ZERO);
>> + if (!vcpu->arch.shared)
>> + goto uninit_tlb;
>> +
>> return vcpu;
>>
>> +uninit_tlb:
>> + kvmppc_e500_tlb_uninit(vcpu_e500);
>> uninit_vcpu:
>> kvm_vcpu_uninit(vcpu);
>> free_vcpu:
>> @@ -131,6 +137,7 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
>> {
>> struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
>>
>> + free_page((unsigned long)vcpu->arch.shared);
>> kvmppc_e500_tlb_uninit(vcpu_e500);
>> kvm_vcpu_uninit(vcpu);
>> kmem_cache_free(kvm_vcpu_cache, vcpu_e500);
>>
>
> Code repeats 3x. Share please.
Looking at this again, I could combine the 3 lines of init code into 3
lines of code that do a generic function call and then error checking.
And I could convert the one free_page line with one function call that
would free the page. Is there a real gain behind this?
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
