Re: [RFC PATCH 05/11] kvm: powerpc: book3s: Add kvmppc_ops callback for HV and PR specific operations
Alexander Graf writes:
> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
>
>> From: "Aneesh Kumar K.V"
>>
>> This moves HV and PR specific functions to kvmppc_ops callback.
>> This is needed so that we can enable HV and PR together in the
>> same kernel. Actual changes to enable both come in the later
>> patch.This also renames almost all of the symbols that exist in both PR and
>> HV
>> KVM for clarity. Symbols in the PR KVM implementation get "_pr"
>> appended, and those in the HV KVM implementation get "_hv". Then,
>> in book3s.c, we add a function with the name without the suffix and
>> arrange for it to call the appropriate kvmppc_ops callback depending on
>> which kvm type we selected during VM creation.
>>
>> NOTE: we still don't enable selecting both the HV and PR together
>> in this commit that will be done by a later commit.
>>
>> Signed-off-by: Aneesh Kumar K.V
>> ---
>> arch/powerpc/include/asm/kvm_book3s.h | 5 +-
>> arch/powerpc/include/asm/kvm_ppc.h| 63 --
>> arch/powerpc/kvm/Kconfig | 15 ++-
>> arch/powerpc/kvm/Makefile | 9 +-
>> arch/powerpc/kvm/book3s.c | 145 +-
>> arch/powerpc/kvm/book3s_32_mmu_host.c | 2 +-
>> arch/powerpc/kvm/book3s_64_mmu_host.c | 2 +-
>> arch/powerpc/kvm/book3s_64_mmu_hv.c | 17 ++-
>> arch/powerpc/kvm/book3s_emulate.c | 8 +-
>> arch/powerpc/kvm/book3s_hv.c | 226
>> +-
>> arch/powerpc/kvm/book3s_interrupts.S | 2 +-
>> arch/powerpc/kvm/book3s_pr.c | 196 ++---
>> arch/powerpc/kvm/emulate.c| 6 +-
>> arch/powerpc/kvm/powerpc.c| 58 +++--
>> 14 files changed, 539 insertions(+), 215 deletions(-)
>>
>>
>
> [...]
>
>> @@ -888,14 +890,8 @@ int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id,
>> union kvmppc_one_reg *val)
>> return r;
>> }
>>
>> -int kvmppc_core_check_processor_compat(void)
>> -{
>> -if (cpu_has_feature(CPU_FTR_HVMODE))
>> -return 0;
>> -return -EIO;
>> -}
>> -
>> -struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
>> +static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
>> + unsigned int id)
>> {
>> struct kvm_vcpu *vcpu;
>> int err = -EINVAL;
>> @@ -920,7 +916,6 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm
>> *kvm, unsigned int id)
>> vcpu->arch.ctrl = CTRL_RUNLATCH;
>> /* default to host PVR, since we can't spoof it */
>> vcpu->arch.pvr = mfspr(SPRN_PVR);
>> -kvmppc_set_pvr(vcpu, vcpu->arch.pvr);
>
> Where is this one going?
That is same as the line above.
void kvmppc_set_pvr_hv(struct kvm_vcpu *vcpu, u32 pvr)
{
vcpu->arch.pvr = pvr;
}
>
>> spin_lock_init(&vcpu->arch.vpa_update_lock);
>> spin_lock_init(&vcpu->arch.tbacct_lock);
>> vcpu->arch.busy_preempt = TB_NIL;
>> @@ -972,7 +967,7 @@ static void unpin_vpa(struct kvm *kvm, struct kvmppc_vpa
>> *vpa)
>> vpa->dirty);
>> }
>>
>> -void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
>> +static void kvmppc_core_vcpu_free_hv(struct kvm_vcpu *vcpu)
>> {
>> spin_lock(&vcpu->arch.vpa_update_lock);
>> unpin_vpa(vcpu->kvm, &vcpu->arch.dtl);
>> @@ -983,6 +978,12 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
>> kmem_cache_free(kvm_vcpu_cache, vcpu);
>> }
>>
>> +static int kvmppc_core_check_requests_hv(struct kvm_vcpu *vcpu)
>> +{
>> +/* Indicate we want to get back into the guest */
>> +return 1;
>> +}
>> +
>>
>
> [...]
>
>> +case KVM_PPC_GET_HTAB_FD: {
>> +struct kvm_get_htab_fd ghf;
>> +
>> +r = -EFAULT;
>> +if (copy_from_user(&ghf, argp, sizeof(ghf)))
>> +break;
>> +r = kvm_vm_ioctl_get_htab_fd(kvm, &ghf);
>> +break;
>> +}
>> +
>> +default:
>> +r = -ENOTTY;
>> +}
>> +
>> +return r;
>> }
>>
>> -static int kvmppc_book3s_hv_init(void)
>> +/* FIXME!! move to header */
>
> Hrm :)
yes, want to get something out for review. Will fix if we agree on the
approach.
>
>> +extern void kvmppc_core_flush_memslot_hv(struct kvm *kvm,
>> + struct kvm_memory_slot *memslot);
>> +extern int kvm_unmap_hva_hv(struct kvm *kvm, unsigned long hva);
>> +extern int kvm_unmap_hva_range_hv(struct kvm *kvm, unsigned long start,
>> + unsigned long end);
>> +extern int kvm_age_hva_hv(struct kvm *kvm, unsigned long hva);
>> +extern int kvm_test_age_hva_hv(struct kvm *kvm, unsigned long hva);
>> +extern void kvm_set_spte_hva_hv(struct kvm *kvm, unsigned long hva, pte_t
>> pte);
>> +
>> +static struct kvmppc_ops kvmppc_hv_ops = {
>> +.get_sregs = kvm_arch_vcpu_ioctl_get_sregs_hv,
>> +.set_sregs = kvm_arch_vcpu_ioctl_set_sregs_hv,
>> +.get_one_reg = kvmppc_get_one_reg_hv,
>> +.set_one_reg = kvmppc_set_one_reg_hv
Re: [RFC PATCH 05/11] kvm: powerpc: book3s: Add kvmppc_ops callback for HV and PR specific operations
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V"
>
> This moves HV and PR specific functions to kvmppc_ops callback.
> This is needed so that we can enable HV and PR together in the
> same kernel. Actual changes to enable both come in the later
> patch.This also renames almost all of the symbols that exist in both PR and HV
> KVM for clarity. Symbols in the PR KVM implementation get "_pr"
> appended, and those in the HV KVM implementation get "_hv". Then,
> in book3s.c, we add a function with the name without the suffix and
> arrange for it to call the appropriate kvmppc_ops callback depending on
> which kvm type we selected during VM creation.
>
> NOTE: we still don't enable selecting both the HV and PR together
> in this commit that will be done by a later commit.
>
> Signed-off-by: Aneesh Kumar K.V
> ---
> arch/powerpc/include/asm/kvm_book3s.h | 5 +-
> arch/powerpc/include/asm/kvm_ppc.h| 63 --
> arch/powerpc/kvm/Kconfig | 15 ++-
> arch/powerpc/kvm/Makefile | 9 +-
> arch/powerpc/kvm/book3s.c | 145 +-
> arch/powerpc/kvm/book3s_32_mmu_host.c | 2 +-
> arch/powerpc/kvm/book3s_64_mmu_host.c | 2 +-
> arch/powerpc/kvm/book3s_64_mmu_hv.c | 17 ++-
> arch/powerpc/kvm/book3s_emulate.c | 8 +-
> arch/powerpc/kvm/book3s_hv.c | 226 +-
> arch/powerpc/kvm/book3s_interrupts.S | 2 +-
> arch/powerpc/kvm/book3s_pr.c | 196 ++---
> arch/powerpc/kvm/emulate.c| 6 +-
> arch/powerpc/kvm/powerpc.c| 58 +++--
> 14 files changed, 539 insertions(+), 215 deletions(-)
>
>
[...]
> @@ -888,14 +890,8 @@ int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id,
> union kvmppc_one_reg *val)
> return r;
> }
>
> -int kvmppc_core_check_processor_compat(void)
> -{
> - if (cpu_has_feature(CPU_FTR_HVMODE))
> - return 0;
> - return -EIO;
> -}
> -
> -struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
> +static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
> +unsigned int id)
> {
> struct kvm_vcpu *vcpu;
> int err = -EINVAL;
> @@ -920,7 +916,6 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm,
> unsigned int id)
> vcpu->arch.ctrl = CTRL_RUNLATCH;
> /* default to host PVR, since we can't spoof it */
> vcpu->arch.pvr = mfspr(SPRN_PVR);
> - kvmppc_set_pvr(vcpu, vcpu->arch.pvr);
Where is this one going?
> spin_lock_init(&vcpu->arch.vpa_update_lock);
> spin_lock_init(&vcpu->arch.tbacct_lock);
> vcpu->arch.busy_preempt = TB_NIL;
> @@ -972,7 +967,7 @@ static void unpin_vpa(struct kvm *kvm, struct kvmppc_vpa
> *vpa)
> vpa->dirty);
> }
>
> -void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
> +static void kvmppc_core_vcpu_free_hv(struct kvm_vcpu *vcpu)
> {
> spin_lock(&vcpu->arch.vpa_update_lock);
> unpin_vpa(vcpu->kvm, &vcpu->arch.dtl);
> @@ -983,6 +978,12 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
> kmem_cache_free(kvm_vcpu_cache, vcpu);
> }
>
> +static int kvmppc_core_check_requests_hv(struct kvm_vcpu *vcpu)
> +{
> + /* Indicate we want to get back into the guest */
> + return 1;
> +}
> +
>
[...]
> + case KVM_PPC_GET_HTAB_FD: {
> + struct kvm_get_htab_fd ghf;
> +
> + r = -EFAULT;
> + if (copy_from_user(&ghf, argp, sizeof(ghf)))
> + break;
> + r = kvm_vm_ioctl_get_htab_fd(kvm, &ghf);
> + break;
> + }
> +
> + default:
> + r = -ENOTTY;
> + }
> +
> + return r;
> }
>
> -static int kvmppc_book3s_hv_init(void)
> +/* FIXME!! move to header */
Hrm :)
> +extern void kvmppc_core_flush_memslot_hv(struct kvm *kvm,
> + struct kvm_memory_slot *memslot);
> +extern int kvm_unmap_hva_hv(struct kvm *kvm, unsigned long hva);
> +extern int kvm_unmap_hva_range_hv(struct kvm *kvm, unsigned long start,
> + unsigned long end);
> +extern int kvm_age_hva_hv(struct kvm *kvm, unsigned long hva);
> +extern int kvm_test_age_hva_hv(struct kvm *kvm, unsigned long hva);
> +extern void kvm_set_spte_hva_hv(struct kvm *kvm, unsigned long hva, pte_t
> pte);
> +
> +static struct kvmppc_ops kvmppc_hv_ops = {
> + .get_sregs = kvm_arch_vcpu_ioctl_get_sregs_hv,
> + .set_sregs = kvm_arch_vcpu_ioctl_set_sregs_hv,
> + .get_one_reg = kvmppc_get_one_reg_hv,
> + .set_one_reg = kvmppc_set_one_reg_hv,
> + .vcpu_load = kvmppc_core_vcpu_load_hv,
> + .vcpu_put= kvmppc_core_vcpu_put_hv,
> + .set_msr = kvmppc_set_msr_hv,
> + .vcpu_run= kvmppc_vcpu_run_hv,
> + .vcpu_create = kvmppc_core_vcpu_create_hv,
> + .vcpu_free = kvmppc_core_vcpu_free_hv,
> + .check_requests = kvmppc_core_check_requests_h
