| Message ID | 1380276233-17095-7-git-send-email-aneesh.kumar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote: > From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> > > This help us to identify whether we are running with hypervisor mode KVM > enabled. The change is needed so that we can have both HV and PR kvm > enabled in the same kernel. > > If both HV and PR KVM are included, interrupts come in to the HV version > of the kvmppc_interrupt code, which then jumps to the PR handler, > renamed to kvmppc_interrupt_pr, if the guest is a PR guest. > > Allowing both PR and HV in the same kernel required some changes to > kvm_dev_ioctl_check_extension(), since the values returned now can't > be selected with #ifdefs as much as previously. We look at is_hv_enabled > to return the right value when checking for capabilities.For capabilities that > are only provided by HV KVM, we return the HV value only if > is_hv_enabled is true. For capabilities provided by PR KVM but not HV, > we return the PR value only if is_hv_enabled is false. > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > --- > arch/powerpc/include/asm/kvm_book3s.h | 53 -------------------------------- > arch/powerpc/include/asm/kvm_ppc.h | 5 +-- > arch/powerpc/kvm/Makefile | 2 +- > arch/powerpc/kvm/book3s.c | 44 +++++++++++++++++++++++++++ > arch/powerpc/kvm/book3s_hv.c | 1 + > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 +++ > arch/powerpc/kvm/book3s_pr.c | 1 + > arch/powerpc/kvm/book3s_segment.S | 7 ++++- > arch/powerpc/kvm/book3s_xics.c | 2 +- > arch/powerpc/kvm/powerpc.c | 54 ++++++++++++++++++--------------- > 10 files changed, 90 insertions(+), 83 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h > index 3efba3c..ba33c49 100644 > --- a/arch/powerpc/include/asm/kvm_book3s.h > +++ b/arch/powerpc/include/asm/kvm_book3s.h > @@ -297,59 +297,6 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu) > return vcpu->arch.fault_dar; > } > > -#ifdef CONFIG_KVM_BOOK3S_PR > - > -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) > -{ > - return to_book3s(vcpu)->hior; > -} > - > -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, > - unsigned long pending_now, unsigned long old_pending) > -{ > - if (pending_now) > - vcpu->arch.shared->int_pending = 1; > - else if (old_pending) > - vcpu->arch.shared->int_pending = 0; > -} > - > -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) > -{ > - ulong crit_raw = vcpu->arch.shared->critical; > - ulong crit_r1 = kvmppc_get_gpr(vcpu, 1); > - bool crit; > - > - /* Truncate crit indicators in 32 bit mode */ > - if (!(vcpu->arch.shared->msr & MSR_SF)) { > - crit_raw &= 0xffffffff; > - crit_r1 &= 0xffffffff; > - } > - > - /* Critical section when crit == r1 */ > - crit = (crit_raw == crit_r1); > - /* ... and we're in supervisor mode */ > - crit = crit && !(vcpu->arch.shared->msr & MSR_PR); > - > - return crit; > -} > -#else /* CONFIG_KVM_BOOK3S_PR */ > - > -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) > -{ > - return 0; > -} > - > -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, > - unsigned long pending_now, unsigned long old_pending) > -{ > -} > - > -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) > -{ > - return false; > -} > -#endif > - > /* Magic register values loaded into r3 and r4 before the 'sc' assembly > * instruction for the OSI hypercalls */ > #define OSI_SC_MAGIC_R3 0x113724FA > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index 4d9641c..58e732f 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -169,6 +169,7 @@ extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq); > extern int kvmppc_xics_int_off(struct kvm *kvm, u32 irq); > > struct kvmppc_ops { > + bool is_hv_enabled; This doesn't really belong into an ops struct. Either you compare if (kvmppc_ops == kvmppc_ops_pr) against a global known good ops struct or you put the hint somewhere into the kvm struct. > int (*get_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); > int (*set_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); > int (*get_one_reg)(struct kvm_vcpu *vcpu, u64 id, > @@ -309,10 +310,10 @@ static inline void kvmppc_set_xics_phys(int cpu, unsigned long addr) > > static inline u32 kvmppc_get_xics_latch(void) > { > - u32 xirr = get_paca()->kvm_hstate.saved_xirr; > + u32 xirr; > > + xirr = get_paca()->kvm_hstate.saved_xirr; > get_paca()->kvm_hstate.saved_xirr = 0; > - I don't see any functionality change here? > return xirr; > } > > diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile > index c343793..a514ecd 100644 > --- a/arch/powerpc/kvm/Makefile > +++ b/arch/powerpc/kvm/Makefile > @@ -77,7 +77,7 @@ kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \ > book3s_rmhandlers.o > endif > > -kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \ > +kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) += \ This change looks unrelated? > book3s_hv.o \ > book3s_hv_interrupts.o \ > book3s_64_mmu_hv.o > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c > index bdc3f95..12f94bf 100644 > --- a/arch/powerpc/kvm/book3s.c > +++ b/arch/powerpc/kvm/book3s.c > @@ -69,6 +69,50 @@ void kvmppc_core_load_guest_debugstate(struct kvm_vcpu *vcpu) > { > } > > +static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) Please drop the "inline" keyword on functions in .c files :). That way the compiler tells us about unused functions. > +{ > + if (!kvmppc_ops->is_hv_enabled) > + return to_book3s(vcpu)->hior; > + return 0; > +} > + > +static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, > + unsigned long pending_now, unsigned long old_pending) > +{ > + if (kvmppc_ops->is_hv_enabled) > + return; > + if (pending_now) > + vcpu->arch.shared->int_pending = 1; > + else if (old_pending) > + vcpu->arch.shared->int_pending = 0; > +} > + > +static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) > +{ > + ulong crit_raw; > + ulong crit_r1; > + bool crit; > + > + if (kvmppc_ops->is_hv_enabled) > + return false; > + > + crit_raw = vcpu->arch.shared->critical; > + crit_r1 = kvmppc_get_gpr(vcpu, 1); > + > + /* Truncate crit indicators in 32 bit mode */ > + if (!(vcpu->arch.shared->msr & MSR_SF)) { > + crit_raw &= 0xffffffff; > + crit_r1 &= 0xffffffff; > + } > + > + /* Critical section when crit == r1 */ > + crit = (crit_raw == crit_r1); > + /* ... and we're in supervisor mode */ > + crit = crit && !(vcpu->arch.shared->msr & MSR_PR); > + > + return crit; > +} > + > void kvmppc_inject_interrupt(struct kvm_vcpu *vcpu, int vec, u64 flags) > { > vcpu->arch.shared->srr0 = kvmppc_get_pc(vcpu); > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index 36d7920..eec353d 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -2048,6 +2048,7 @@ 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 = { > + .is_hv_enabled = true, > .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, > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > index 5ede7fc..4838fdb 100644 > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > @@ -577,6 +577,10 @@ kvmppc_interrupt: > lbz r9, HSTATE_IN_GUEST(r13) > cmpwi r9, KVM_GUEST_MODE_HOST_HV > beq kvmppc_bad_host_intr > +#ifdef CONFIG_KVM_BOOK3S_PR > + cmpwi r9, KVM_GUEST_MODE_GUEST > + beq kvmppc_interrupt_pr > +#endif > /* We're now back in the host but in guest MMU context */ > li r9, KVM_GUEST_MODE_HOST_HV > stb r9, HSTATE_IN_GUEST(r13) > diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c > index ff5bd24..2a97279 100644 > --- a/arch/powerpc/kvm/book3s_pr.c > +++ b/arch/powerpc/kvm/book3s_pr.c > @@ -1509,6 +1509,7 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, > int sprn, ulong *spr_val); > > static struct kvmppc_ops kvmppc_pr_ops = { > + .is_hv_enabled = false, > .get_sregs = kvm_arch_vcpu_ioctl_get_sregs_pr, > .set_sregs = kvm_arch_vcpu_ioctl_set_sregs_pr, > .get_one_reg = kvmppc_get_one_reg_pr, > diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S > index 1abe478..e0229dd 100644 > --- a/arch/powerpc/kvm/book3s_segment.S > +++ b/arch/powerpc/kvm/book3s_segment.S > @@ -161,9 +161,14 @@ kvmppc_handler_trampoline_enter_end: > .global kvmppc_handler_trampoline_exit > kvmppc_handler_trampoline_exit: > > +#if defined(CONFIG_KVM_BOOK3S_HV) > +.global kvmppc_interrupt_pr > +kvmppc_interrupt_pr: > + ld r9, HSTATE_SCRATCH2(r13) > +#else > .global kvmppc_interrupt > kvmppc_interrupt: Just always call it kvmppc_interrupt_pr and thus share at least that part of the code :). > - > +#endif > /* Register usage at this point: > * > * SPRG_SCRATCH0 = guest R13 > diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c > index f0c732e..fa7625d 100644 > --- a/arch/powerpc/kvm/book3s_xics.c > +++ b/arch/powerpc/kvm/book3s_xics.c > @@ -818,7 +818,7 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req) > } > > /* Check for real mode returning too hard */ > - if (xics->real_mode) > + if (xics->real_mode && kvmppc_ops->is_hv_enabled) > return kvmppc_xics_rm_complete(vcpu, req); > > switch (req) { > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 69b9305..00a96fc 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -52,7 +52,6 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu) > return 1; > } > > -#ifndef CONFIG_KVM_BOOK3S_64_HV > /* > * Common checks before entering the guest world. Call with interrupts > * disabled. > @@ -127,7 +126,6 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu) > > return r; > } > -#endif /* CONFIG_KVM_BOOK3S_64_HV */ > > int kvmppc_kvm_pv(struct kvm_vcpu *vcpu) > { > @@ -194,11 +192,9 @@ int kvmppc_sanity_check(struct kvm_vcpu *vcpu) > if ((vcpu->arch.cpu_type != KVM_CPU_3S_64) && vcpu->arch.papr_enabled) > goto out; > > -#ifdef CONFIG_KVM_BOOK3S_64_HV > /* HV KVM can only do PAPR mode for now */ > - if (!vcpu->arch.papr_enabled) > + if (!vcpu->arch.papr_enabled && kvmppc_ops->is_hv_enabled) > goto out; > -#endif > > #ifdef CONFIG_KVM_BOOKE_HV > if (!cpu_has_feature(CPU_FTR_EMB_HV)) > @@ -322,22 +318,26 @@ int kvm_dev_ioctl_check_extension(long ext) > case KVM_CAP_DEVICE_CTRL: > r = 1; > break; > -#ifndef CONFIG_KVM_BOOK3S_64_HV > case KVM_CAP_PPC_PAIRED_SINGLES: > case KVM_CAP_PPC_OSI: > case KVM_CAP_PPC_GET_PVINFO: > #if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC) > case KVM_CAP_SW_TLB: > #endif > -#ifdef CONFIG_KVM_MPIC > - case KVM_CAP_IRQ_MPIC: > -#endif > - r = 1; > + /* We support this only for PR */ > + r = !kvmppc_ops->is_hv_enabled; Reading this, have you test compiled your code against e500 configs? > break; > +#ifdef CONFIG_KVM_MMIO > case KVM_CAP_COALESCED_MMIO: > r = KVM_COALESCED_MMIO_PAGE_OFFSET; > break; > #endif > +#ifdef CONFIG_KVM_MPIC > + case KVM_CAP_IRQ_MPIC: > + r = 1; > + break; > +#endif > + > #ifdef CONFIG_PPC_BOOK3S_64 > case KVM_CAP_SPAPR_TCE: > case KVM_CAP_PPC_ALLOC_HTAB: > @@ -348,32 +348,37 @@ int kvm_dev_ioctl_check_extension(long ext) > r = 1; > break; > #endif /* CONFIG_PPC_BOOK3S_64 */ > -#ifdef CONFIG_KVM_BOOK3S_64_HV > +#ifdef CONFIG_KVM_BOOK3S_HV > case KVM_CAP_PPC_SMT: > - r = threads_per_core; > + if (kvmppc_ops->is_hv_enabled) > + r = threads_per_core; > + else > + r = 0; 0? Or 1? Alex
Alexander Graf <agraf@suse.de> writes: > On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote: > >> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> >> >> This help us to identify whether we are running with hypervisor mode KVM >> enabled. The change is needed so that we can have both HV and PR kvm >> enabled in the same kernel. >> >> If both HV and PR KVM are included, interrupts come in to the HV version >> of the kvmppc_interrupt code, which then jumps to the PR handler, >> renamed to kvmppc_interrupt_pr, if the guest is a PR guest. >> >> Allowing both PR and HV in the same kernel required some changes to >> kvm_dev_ioctl_check_extension(), since the values returned now can't >> be selected with #ifdefs as much as previously. We look at is_hv_enabled >> to return the right value when checking for capabilities.For capabilities that >> are only provided by HV KVM, we return the HV value only if >> is_hv_enabled is true. For capabilities provided by PR KVM but not HV, >> we return the PR value only if is_hv_enabled is false. >> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> >> --- >> arch/powerpc/include/asm/kvm_book3s.h | 53 -------------------------------- >> arch/powerpc/include/asm/kvm_ppc.h | 5 +-- >> arch/powerpc/kvm/Makefile | 2 +- >> arch/powerpc/kvm/book3s.c | 44 +++++++++++++++++++++++++++ >> arch/powerpc/kvm/book3s_hv.c | 1 + >> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 +++ >> arch/powerpc/kvm/book3s_pr.c | 1 + >> arch/powerpc/kvm/book3s_segment.S | 7 ++++- >> arch/powerpc/kvm/book3s_xics.c | 2 +- >> arch/powerpc/kvm/powerpc.c | 54 ++++++++++++++++++--------------- >> 10 files changed, 90 insertions(+), 83 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h >> index 3efba3c..ba33c49 100644 >> --- a/arch/powerpc/include/asm/kvm_book3s.h >> +++ b/arch/powerpc/include/asm/kvm_book3s.h >> @@ -297,59 +297,6 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu) >> return vcpu->arch.fault_dar; >> } >> >> -#ifdef CONFIG_KVM_BOOK3S_PR >> - >> -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) >> -{ >> - return to_book3s(vcpu)->hior; >> -} >> - >> -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, >> - unsigned long pending_now, unsigned long old_pending) >> -{ >> - if (pending_now) >> - vcpu->arch.shared->int_pending = 1; >> - else if (old_pending) >> - vcpu->arch.shared->int_pending = 0; >> -} >> - >> -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) >> -{ >> - ulong crit_raw = vcpu->arch.shared->critical; >> - ulong crit_r1 = kvmppc_get_gpr(vcpu, 1); >> - bool crit; >> - >> - /* Truncate crit indicators in 32 bit mode */ >> - if (!(vcpu->arch.shared->msr & MSR_SF)) { >> - crit_raw &= 0xffffffff; >> - crit_r1 &= 0xffffffff; >> - } >> - >> - /* Critical section when crit == r1 */ >> - crit = (crit_raw == crit_r1); >> - /* ... and we're in supervisor mode */ >> - crit = crit && !(vcpu->arch.shared->msr & MSR_PR); >> - >> - return crit; >> -} >> -#else /* CONFIG_KVM_BOOK3S_PR */ >> - >> -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) >> -{ >> - return 0; >> -} >> - >> -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, >> - unsigned long pending_now, unsigned long old_pending) >> -{ >> -} >> - >> -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) >> -{ >> - return false; >> -} >> -#endif >> - >> /* Magic register values loaded into r3 and r4 before the 'sc' assembly >> * instruction for the OSI hypercalls */ >> #define OSI_SC_MAGIC_R3 0x113724FA >> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h >> index 4d9641c..58e732f 100644 >> --- a/arch/powerpc/include/asm/kvm_ppc.h >> +++ b/arch/powerpc/include/asm/kvm_ppc.h >> @@ -169,6 +169,7 @@ extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq); >> extern int kvmppc_xics_int_off(struct kvm *kvm, u32 irq); >> >> struct kvmppc_ops { >> + bool is_hv_enabled; > > This doesn't really belong into an ops struct. Either you compare > > if (kvmppc_ops == kvmppc_ops_pr) will do that in the next update. > > against a global known good ops struct or you put the hint somewhere into the kvm struct. > >> int (*get_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); >> int (*set_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); >> int (*get_one_reg)(struct kvm_vcpu *vcpu, u64 id, >> @@ -309,10 +310,10 @@ static inline void kvmppc_set_xics_phys(int cpu, unsigned long addr) >> >> static inline u32 kvmppc_get_xics_latch(void) >> { >> - u32 xirr = get_paca()->kvm_hstate.saved_xirr; >> + u32 xirr; >> >> + xirr = get_paca()->kvm_hstate.saved_xirr; >> get_paca()->kvm_hstate.saved_xirr = 0; >> - > > I don't see any functionality change here? > >> return xirr; Mistake on my side, I had a if condition in there before, which i later removed. But forgot to move the assignment back. Will fix. >> } >> >> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile >> index c343793..a514ecd 100644 >> --- a/arch/powerpc/kvm/Makefile >> +++ b/arch/powerpc/kvm/Makefile >> @@ -77,7 +77,7 @@ kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \ >> book3s_rmhandlers.o >> endif >> >> -kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \ >> +kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) += \ > > This change looks unrelated? > yes. Will move it to the correct patch >> book3s_hv.o \ >> book3s_hv_interrupts.o \ >> book3s_64_mmu_hv.o >> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c >> index bdc3f95..12f94bf 100644 >> --- a/arch/powerpc/kvm/book3s.c >> +++ b/arch/powerpc/kvm/book3s.c >> @@ -69,6 +69,50 @@ void kvmppc_core_load_guest_debugstate(struct kvm_vcpu *vcpu) >> { >> } >> >> +static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) > > Please drop the "inline" keyword on functions in .c files :). That way > the compiler tells us about unused functions. ok . > >> +{ >> + if (!kvmppc_ops->is_hv_enabled) >> + return to_book3s(vcpu)->hior; >> + return 0; >> +} >> + >> +static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, >> + unsigned long pending_now, unsigned long old_pending) >> +{ >> + if (kvmppc_ops->is_hv_enabled) >> + return; >> + if (pending_now) >> + vcpu->arch.shared->int_pending = 1; >> + else if (old_pending) >> + vcpu->arch.shared->int_pending = 0; >> +} >> + .... >> diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S >> index 1abe478..e0229dd 100644 >> --- a/arch/powerpc/kvm/book3s_segment.S >> +++ b/arch/powerpc/kvm/book3s_segment.S >> @@ -161,9 +161,14 @@ kvmppc_handler_trampoline_enter_end: >> .global kvmppc_handler_trampoline_exit >> kvmppc_handler_trampoline_exit: >> >> +#if defined(CONFIG_KVM_BOOK3S_HV) >> +.global kvmppc_interrupt_pr >> +kvmppc_interrupt_pr: >> + ld r9, HSTATE_SCRATCH2(r13) >> +#else >> .global kvmppc_interrupt >> kvmppc_interrupt: > > Just always call it kvmppc_interrupt_pr and thus share at least that > part of the code :). But if i don't have HV enabled, we don't compile book3s_hv_rmhandlers.S Hence don't have the kvmppc_interrupt symbol defined. > >> - >> +#endif >> /* Register usage at this point: >> * >> * SPRG_SCRATCH0 = guest R13 >> diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c >> index f0c732e..fa7625d 100644 >> --- a/arch/powerpc/kvm/book3s_xics.c >> +++ b/arch/powerpc/kvm/book3s_xics.c >> @@ -818,7 +818,7 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req) >> } >> >> /* Check for real mode returning too hard */ >> - if (xics->real_mode) >> + if (xics->real_mode && kvmppc_ops->is_hv_enabled) >> return kvmppc_xics_rm_complete(vcpu, req); >> >> switch (req) { >> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c >> index 69b9305..00a96fc 100644 >> --- a/arch/powerpc/kvm/powerpc.c >> +++ b/arch/powerpc/kvm/powerpc.c >> @@ -52,7 +52,6 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu) >> return 1; >> } >> >> -#ifndef CONFIG_KVM_BOOK3S_64_HV >> /* >> * Common checks before entering the guest world. Call with interrupts >> * disabled. >> @@ -127,7 +126,6 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu) >> >> return r; >> } >> -#endif /* CONFIG_KVM_BOOK3S_64_HV */ >> >> int kvmppc_kvm_pv(struct kvm_vcpu *vcpu) >> { >> @@ -194,11 +192,9 @@ int kvmppc_sanity_check(struct kvm_vcpu *vcpu) >> if ((vcpu->arch.cpu_type != KVM_CPU_3S_64) && vcpu->arch.papr_enabled) >> goto out; >> >> -#ifdef CONFIG_KVM_BOOK3S_64_HV >> /* HV KVM can only do PAPR mode for now */ >> - if (!vcpu->arch.papr_enabled) >> + if (!vcpu->arch.papr_enabled && kvmppc_ops->is_hv_enabled) >> goto out; >> -#endif >> >> #ifdef CONFIG_KVM_BOOKE_HV >> if (!cpu_has_feature(CPU_FTR_EMB_HV)) >> @@ -322,22 +318,26 @@ int kvm_dev_ioctl_check_extension(long ext) >> case KVM_CAP_DEVICE_CTRL: >> r = 1; >> break; >> -#ifndef CONFIG_KVM_BOOK3S_64_HV >> case KVM_CAP_PPC_PAIRED_SINGLES: >> case KVM_CAP_PPC_OSI: >> case KVM_CAP_PPC_GET_PVINFO: >> #if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC) >> case KVM_CAP_SW_TLB: >> #endif >> -#ifdef CONFIG_KVM_MPIC >> - case KVM_CAP_IRQ_MPIC: >> -#endif >> - r = 1; >> + /* We support this only for PR */ >> + r = !kvmppc_ops->is_hv_enabled; > > Reading this, have you test compiled your code against e500 configs? Not yet. > >> break; >> +#ifdef CONFIG_KVM_MMIO >> case KVM_CAP_COALESCED_MMIO: >> r = KVM_COALESCED_MMIO_PAGE_OFFSET; >> break; >> #endif >> +#ifdef CONFIG_KVM_MPIC >> + case KVM_CAP_IRQ_MPIC: >> + r = 1; >> + break; >> +#endif >> + >> #ifdef CONFIG_PPC_BOOK3S_64 >> case KVM_CAP_SPAPR_TCE: >> case KVM_CAP_PPC_ALLOC_HTAB: >> @@ -348,32 +348,37 @@ int kvm_dev_ioctl_check_extension(long ext) >> r = 1; >> break; >> #endif /* CONFIG_PPC_BOOK3S_64 */ >> -#ifdef CONFIG_KVM_BOOK3S_64_HV >> +#ifdef CONFIG_KVM_BOOK3S_HV >> case KVM_CAP_PPC_SMT: >> - r = threads_per_core; >> + if (kvmppc_ops->is_hv_enabled) >> + r = threads_per_core; >> + else >> + r = 0; > > 0? Or 1? > That check extension was not supported before on PR. So not sure what the value should be. May be 1 is better indicating we have one thread. Will change. -aneesh
On 27.09.2013, at 15:03, Aneesh Kumar K.V wrote: > Alexander Graf <agraf@suse.de> writes: > >> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote: >> >>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> >>> >>> This help us to identify whether we are running with hypervisor mode KVM >>> enabled. The change is needed so that we can have both HV and PR kvm >>> enabled in the same kernel. >>> >>> If both HV and PR KVM are included, interrupts come in to the HV version >>> of the kvmppc_interrupt code, which then jumps to the PR handler, >>> renamed to kvmppc_interrupt_pr, if the guest is a PR guest. >>> >>> Allowing both PR and HV in the same kernel required some changes to >>> kvm_dev_ioctl_check_extension(), since the values returned now can't >>> be selected with #ifdefs as much as previously. We look at is_hv_enabled >>> to return the right value when checking for capabilities.For capabilities that >>> are only provided by HV KVM, we return the HV value only if >>> is_hv_enabled is true. For capabilities provided by PR KVM but not HV, >>> we return the PR value only if is_hv_enabled is false. >>> >>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> >>> --- >>> arch/powerpc/include/asm/kvm_book3s.h | 53 -------------------------------- >>> arch/powerpc/include/asm/kvm_ppc.h | 5 +-- >>> arch/powerpc/kvm/Makefile | 2 +- >>> arch/powerpc/kvm/book3s.c | 44 +++++++++++++++++++++++++++ >>> arch/powerpc/kvm/book3s_hv.c | 1 + >>> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 +++ >>> arch/powerpc/kvm/book3s_pr.c | 1 + >>> arch/powerpc/kvm/book3s_segment.S | 7 ++++- >>> arch/powerpc/kvm/book3s_xics.c | 2 +- >>> arch/powerpc/kvm/powerpc.c | 54 ++++++++++++++++++--------------- >>> 10 files changed, 90 insertions(+), 83 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h >>> index 3efba3c..ba33c49 100644 >>> --- a/arch/powerpc/include/asm/kvm_book3s.h >>> +++ b/arch/powerpc/include/asm/kvm_book3s.h >>> @@ -297,59 +297,6 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu) >>> return vcpu->arch.fault_dar; >>> } >>> >>> -#ifdef CONFIG_KVM_BOOK3S_PR >>> - >>> -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) >>> -{ >>> - return to_book3s(vcpu)->hior; >>> -} >>> - >>> -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, >>> - unsigned long pending_now, unsigned long old_pending) >>> -{ >>> - if (pending_now) >>> - vcpu->arch.shared->int_pending = 1; >>> - else if (old_pending) >>> - vcpu->arch.shared->int_pending = 0; >>> -} >>> - >>> -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) >>> -{ >>> - ulong crit_raw = vcpu->arch.shared->critical; >>> - ulong crit_r1 = kvmppc_get_gpr(vcpu, 1); >>> - bool crit; >>> - >>> - /* Truncate crit indicators in 32 bit mode */ >>> - if (!(vcpu->arch.shared->msr & MSR_SF)) { >>> - crit_raw &= 0xffffffff; >>> - crit_r1 &= 0xffffffff; >>> - } >>> - >>> - /* Critical section when crit == r1 */ >>> - crit = (crit_raw == crit_r1); >>> - /* ... and we're in supervisor mode */ >>> - crit = crit && !(vcpu->arch.shared->msr & MSR_PR); >>> - >>> - return crit; >>> -} >>> -#else /* CONFIG_KVM_BOOK3S_PR */ >>> - >>> -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) >>> -{ >>> - return 0; >>> -} >>> - >>> -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, >>> - unsigned long pending_now, unsigned long old_pending) >>> -{ >>> -} >>> - >>> -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) >>> -{ >>> - return false; >>> -} >>> -#endif >>> - >>> /* Magic register values loaded into r3 and r4 before the 'sc' assembly >>> * instruction for the OSI hypercalls */ >>> #define OSI_SC_MAGIC_R3 0x113724FA >>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h >>> index 4d9641c..58e732f 100644 >>> --- a/arch/powerpc/include/asm/kvm_ppc.h >>> +++ b/arch/powerpc/include/asm/kvm_ppc.h >>> @@ -169,6 +169,7 @@ extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq); >>> extern int kvmppc_xics_int_off(struct kvm *kvm, u32 irq); >>> >>> struct kvmppc_ops { >>> + bool is_hv_enabled; >> >> This doesn't really belong into an ops struct. Either you compare >> >> if (kvmppc_ops == kvmppc_ops_pr) > > will do that in the next update. > >> >> against a global known good ops struct or you put the hint somewhere into the kvm struct. >> >>> int (*get_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); >>> int (*set_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); >>> int (*get_one_reg)(struct kvm_vcpu *vcpu, u64 id, >>> @@ -309,10 +310,10 @@ static inline void kvmppc_set_xics_phys(int cpu, unsigned long addr) >>> >>> static inline u32 kvmppc_get_xics_latch(void) >>> { >>> - u32 xirr = get_paca()->kvm_hstate.saved_xirr; >>> + u32 xirr; >>> >>> + xirr = get_paca()->kvm_hstate.saved_xirr; >>> get_paca()->kvm_hstate.saved_xirr = 0; >>> - >> >> I don't see any functionality change here? >> >>> return xirr; > > Mistake on my side, I had a if condition in there before, which i later > removed. But forgot to move the assignment back. Will fix. > >>> } >>> >>> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile >>> index c343793..a514ecd 100644 >>> --- a/arch/powerpc/kvm/Makefile >>> +++ b/arch/powerpc/kvm/Makefile >>> @@ -77,7 +77,7 @@ kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \ >>> book3s_rmhandlers.o >>> endif >>> >>> -kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \ >>> +kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) += \ >> >> This change looks unrelated? >> > > yes. Will move it to the correct patch > > >>> book3s_hv.o \ >>> book3s_hv_interrupts.o \ >>> book3s_64_mmu_hv.o >>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c >>> index bdc3f95..12f94bf 100644 >>> --- a/arch/powerpc/kvm/book3s.c >>> +++ b/arch/powerpc/kvm/book3s.c >>> @@ -69,6 +69,50 @@ void kvmppc_core_load_guest_debugstate(struct kvm_vcpu *vcpu) >>> { >>> } >>> >>> +static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) >> >> Please drop the "inline" keyword on functions in .c files :). That way >> the compiler tells us about unused functions. > > ok . > >> >>> +{ >>> + if (!kvmppc_ops->is_hv_enabled) >>> + return to_book3s(vcpu)->hior; >>> + return 0; >>> +} >>> + >>> +static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, >>> + unsigned long pending_now, unsigned long old_pending) >>> +{ >>> + if (kvmppc_ops->is_hv_enabled) >>> + return; >>> + if (pending_now) >>> + vcpu->arch.shared->int_pending = 1; >>> + else if (old_pending) >>> + vcpu->arch.shared->int_pending = 0; >>> +} >>> + > > .... > >>> diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S >>> index 1abe478..e0229dd 100644 >>> --- a/arch/powerpc/kvm/book3s_segment.S >>> +++ b/arch/powerpc/kvm/book3s_segment.S >>> @@ -161,9 +161,14 @@ kvmppc_handler_trampoline_enter_end: >>> .global kvmppc_handler_trampoline_exit >>> kvmppc_handler_trampoline_exit: >>> >>> +#if defined(CONFIG_KVM_BOOK3S_HV) >>> +.global kvmppc_interrupt_pr >>> +kvmppc_interrupt_pr: >>> + ld r9, HSTATE_SCRATCH2(r13) >>> +#else >>> .global kvmppc_interrupt >>> kvmppc_interrupt: >> >> Just always call it kvmppc_interrupt_pr and thus share at least that >> part of the code :). > > But if i don't have HV enabled, we don't compile book3s_hv_rmhandlers.S > Hence don't have the kvmppc_interrupt symbol defined. Ah, because we're always jumping to kvmppc_interrupt. Can we make this slightly less magical? How about we always call kvmppc_interrupt_hv when CONFIG_KVM_BOOK3S_HV_POSSIBLE and always kvmppc_interrupt_pr when CONFIG_KVM_BOOK3S_PR_POSSIBLE and then branch to kvmppc_interrupt_pr from kvmppc_interrupt_hv? IMHO that would make the code flow more obvious. > >> >>> - >>> +#endif >>> /* Register usage at this point: >>> * >>> * SPRG_SCRATCH0 = guest R13 >>> diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c >>> index f0c732e..fa7625d 100644 >>> --- a/arch/powerpc/kvm/book3s_xics.c >>> +++ b/arch/powerpc/kvm/book3s_xics.c >>> @@ -818,7 +818,7 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req) >>> } >>> >>> /* Check for real mode returning too hard */ >>> - if (xics->real_mode) >>> + if (xics->real_mode && kvmppc_ops->is_hv_enabled) >>> return kvmppc_xics_rm_complete(vcpu, req); >>> >>> switch (req) { >>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c >>> index 69b9305..00a96fc 100644 >>> --- a/arch/powerpc/kvm/powerpc.c >>> +++ b/arch/powerpc/kvm/powerpc.c >>> @@ -52,7 +52,6 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu) >>> return 1; >>> } >>> >>> -#ifndef CONFIG_KVM_BOOK3S_64_HV >>> /* >>> * Common checks before entering the guest world. Call with interrupts >>> * disabled. >>> @@ -127,7 +126,6 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu) >>> >>> return r; >>> } >>> -#endif /* CONFIG_KVM_BOOK3S_64_HV */ >>> >>> int kvmppc_kvm_pv(struct kvm_vcpu *vcpu) >>> { >>> @@ -194,11 +192,9 @@ int kvmppc_sanity_check(struct kvm_vcpu *vcpu) >>> if ((vcpu->arch.cpu_type != KVM_CPU_3S_64) && vcpu->arch.papr_enabled) >>> goto out; >>> >>> -#ifdef CONFIG_KVM_BOOK3S_64_HV >>> /* HV KVM can only do PAPR mode for now */ >>> - if (!vcpu->arch.papr_enabled) >>> + if (!vcpu->arch.papr_enabled && kvmppc_ops->is_hv_enabled) >>> goto out; >>> -#endif >>> >>> #ifdef CONFIG_KVM_BOOKE_HV >>> if (!cpu_has_feature(CPU_FTR_EMB_HV)) >>> @@ -322,22 +318,26 @@ int kvm_dev_ioctl_check_extension(long ext) >>> case KVM_CAP_DEVICE_CTRL: >>> r = 1; >>> break; >>> -#ifndef CONFIG_KVM_BOOK3S_64_HV >>> case KVM_CAP_PPC_PAIRED_SINGLES: >>> case KVM_CAP_PPC_OSI: >>> case KVM_CAP_PPC_GET_PVINFO: >>> #if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC) >>> case KVM_CAP_SW_TLB: >>> #endif >>> -#ifdef CONFIG_KVM_MPIC >>> - case KVM_CAP_IRQ_MPIC: >>> -#endif >>> - r = 1; >>> + /* We support this only for PR */ >>> + r = !kvmppc_ops->is_hv_enabled; >> >> Reading this, have you test compiled your code against e500 configs? > > > Not yet. Please do so - for every patch in your series. If you like I can give you my example configs I usually use to test compile this. > > >> >>> break; >>> +#ifdef CONFIG_KVM_MMIO >>> case KVM_CAP_COALESCED_MMIO: >>> r = KVM_COALESCED_MMIO_PAGE_OFFSET; >>> break; >>> #endif >>> +#ifdef CONFIG_KVM_MPIC >>> + case KVM_CAP_IRQ_MPIC: >>> + r = 1; >>> + break; >>> +#endif >>> + >>> #ifdef CONFIG_PPC_BOOK3S_64 >>> case KVM_CAP_SPAPR_TCE: >>> case KVM_CAP_PPC_ALLOC_HTAB: >>> @@ -348,32 +348,37 @@ int kvm_dev_ioctl_check_extension(long ext) >>> r = 1; >>> break; >>> #endif /* CONFIG_PPC_BOOK3S_64 */ >>> -#ifdef CONFIG_KVM_BOOK3S_64_HV >>> +#ifdef CONFIG_KVM_BOOK3S_HV >>> case KVM_CAP_PPC_SMT: >>> - r = threads_per_core; >>> + if (kvmppc_ops->is_hv_enabled) >>> + r = threads_per_core; >>> + else >>> + r = 0; >> >> 0? Or 1? >> > > That check extension was not supported before on PR. So not sure what > the value should be. May be 1 is better indicating we have one thread. > Will change. Ah, the default case (cap unknown) returns 0, so this is fine. Alex
Alexander Graf <agraf@suse.de> writes: > On 27.09.2013, at 15:03, Aneesh Kumar K.V wrote: > >> Alexander Graf <agraf@suse.de> writes: >> >> >>>> diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S >>>> index 1abe478..e0229dd 100644 >>>> --- a/arch/powerpc/kvm/book3s_segment.S >>>> +++ b/arch/powerpc/kvm/book3s_segment.S >>>> @@ -161,9 +161,14 @@ kvmppc_handler_trampoline_enter_end: >>>> .global kvmppc_handler_trampoline_exit >>>> kvmppc_handler_trampoline_exit: >>>> >>>> +#if defined(CONFIG_KVM_BOOK3S_HV) >>>> +.global kvmppc_interrupt_pr >>>> +kvmppc_interrupt_pr: >>>> + ld r9, HSTATE_SCRATCH2(r13) >>>> +#else >>>> .global kvmppc_interrupt >>>> kvmppc_interrupt: >>> >>> Just always call it kvmppc_interrupt_pr and thus share at least that >>> part of the code :). >> >> But if i don't have HV enabled, we don't compile book3s_hv_rmhandlers.S >> Hence don't have the kvmppc_interrupt symbol defined. > > Ah, because we're always jumping to kvmppc_interrupt. Can we make this > slightly less magical? How about we always call kvmppc_interrupt_hv > when CONFIG_KVM_BOOK3S_HV_POSSIBLE and always kvmppc_interrupt_pr when > CONFIG_KVM_BOOK3S_PR_POSSIBLE and then branch to kvmppc_interrupt_pr > from kvmppc_interrupt_hv? > > IMHO that would make the code flow more obvious. To make sure I understand you correctly, what you are suggesting is to update __KVM_HANDLER to call kvmppc_interupt_pr when HV is not enabled ? -aneesh
On 09/30/2013 02:56 PM, Aneesh Kumar K.V wrote: > Alexander Graf<agraf@suse.de> writes: > >> On 27.09.2013, at 15:03, Aneesh Kumar K.V wrote: >> >>> Alexander Graf<agraf@suse.de> writes: >>> >>> >>>>> diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S >>>>> index 1abe478..e0229dd 100644 >>>>> --- a/arch/powerpc/kvm/book3s_segment.S >>>>> +++ b/arch/powerpc/kvm/book3s_segment.S >>>>> @@ -161,9 +161,14 @@ kvmppc_handler_trampoline_enter_end: >>>>> .global kvmppc_handler_trampoline_exit >>>>> kvmppc_handler_trampoline_exit: >>>>> >>>>> +#if defined(CONFIG_KVM_BOOK3S_HV) >>>>> +.global kvmppc_interrupt_pr >>>>> +kvmppc_interrupt_pr: >>>>> + ld r9, HSTATE_SCRATCH2(r13) >>>>> +#else >>>>> .global kvmppc_interrupt >>>>> kvmppc_interrupt: >>>> Just always call it kvmppc_interrupt_pr and thus share at least that >>>> part of the code :). >>> But if i don't have HV enabled, we don't compile book3s_hv_rmhandlers.S >>> Hence don't have the kvmppc_interrupt symbol defined. >> Ah, because we're always jumping to kvmppc_interrupt. Can we make this >> slightly less magical? How about we always call kvmppc_interrupt_hv >> when CONFIG_KVM_BOOK3S_HV_POSSIBLE and always kvmppc_interrupt_pr when >> CONFIG_KVM_BOOK3S_PR_POSSIBLE and then branch to kvmppc_interrupt_pr >> from kvmppc_interrupt_hv? >> >> IMHO that would make the code flow more obvious. > > To make sure I understand you correctly, what you are suggesting is > to update __KVM_HANDLER to call kvmppc_interupt_pr when HV is not > enabled ? Yes, I think that makes the code flow more obvious. Every function always has the same name regardless of config options then. Alex
diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h index 3efba3c..ba33c49 100644 --- a/arch/powerpc/include/asm/kvm_book3s.h +++ b/arch/powerpc/include/asm/kvm_book3s.h @@ -297,59 +297,6 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu) return vcpu->arch.fault_dar; } -#ifdef CONFIG_KVM_BOOK3S_PR - -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) -{ - return to_book3s(vcpu)->hior; -} - -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, - unsigned long pending_now, unsigned long old_pending) -{ - if (pending_now) - vcpu->arch.shared->int_pending = 1; - else if (old_pending) - vcpu->arch.shared->int_pending = 0; -} - -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) -{ - ulong crit_raw = vcpu->arch.shared->critical; - ulong crit_r1 = kvmppc_get_gpr(vcpu, 1); - bool crit; - - /* Truncate crit indicators in 32 bit mode */ - if (!(vcpu->arch.shared->msr & MSR_SF)) { - crit_raw &= 0xffffffff; - crit_r1 &= 0xffffffff; - } - - /* Critical section when crit == r1 */ - crit = (crit_raw == crit_r1); - /* ... and we're in supervisor mode */ - crit = crit && !(vcpu->arch.shared->msr & MSR_PR); - - return crit; -} -#else /* CONFIG_KVM_BOOK3S_PR */ - -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) -{ - return 0; -} - -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, - unsigned long pending_now, unsigned long old_pending) -{ -} - -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) -{ - return false; -} -#endif - /* Magic register values loaded into r3 and r4 before the 'sc' assembly * instruction for the OSI hypercalls */ #define OSI_SC_MAGIC_R3 0x113724FA diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index 4d9641c..58e732f 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -169,6 +169,7 @@ extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq); extern int kvmppc_xics_int_off(struct kvm *kvm, u32 irq); struct kvmppc_ops { + bool is_hv_enabled; int (*get_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); int (*set_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs); int (*get_one_reg)(struct kvm_vcpu *vcpu, u64 id, @@ -309,10 +310,10 @@ static inline void kvmppc_set_xics_phys(int cpu, unsigned long addr) static inline u32 kvmppc_get_xics_latch(void) { - u32 xirr = get_paca()->kvm_hstate.saved_xirr; + u32 xirr; + xirr = get_paca()->kvm_hstate.saved_xirr; get_paca()->kvm_hstate.saved_xirr = 0; - return xirr; } diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile index c343793..a514ecd 100644 --- a/arch/powerpc/kvm/Makefile +++ b/arch/powerpc/kvm/Makefile @@ -77,7 +77,7 @@ kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \ book3s_rmhandlers.o endif -kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \ +kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) += \ book3s_hv.o \ book3s_hv_interrupts.o \ book3s_64_mmu_hv.o diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c index bdc3f95..12f94bf 100644 --- a/arch/powerpc/kvm/book3s.c +++ b/arch/powerpc/kvm/book3s.c @@ -69,6 +69,50 @@ void kvmppc_core_load_guest_debugstate(struct kvm_vcpu *vcpu) { } +static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu) +{ + if (!kvmppc_ops->is_hv_enabled) + return to_book3s(vcpu)->hior; + return 0; +} + +static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu, + unsigned long pending_now, unsigned long old_pending) +{ + if (kvmppc_ops->is_hv_enabled) + return; + if (pending_now) + vcpu->arch.shared->int_pending = 1; + else if (old_pending) + vcpu->arch.shared->int_pending = 0; +} + +static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu) +{ + ulong crit_raw; + ulong crit_r1; + bool crit; + + if (kvmppc_ops->is_hv_enabled) + return false; + + crit_raw = vcpu->arch.shared->critical; + crit_r1 = kvmppc_get_gpr(vcpu, 1); + + /* Truncate crit indicators in 32 bit mode */ + if (!(vcpu->arch.shared->msr & MSR_SF)) { + crit_raw &= 0xffffffff; + crit_r1 &= 0xffffffff; + } + + /* Critical section when crit == r1 */ + crit = (crit_raw == crit_r1); + /* ... and we're in supervisor mode */ + crit = crit && !(vcpu->arch.shared->msr & MSR_PR); + + return crit; +} + void kvmppc_inject_interrupt(struct kvm_vcpu *vcpu, int vec, u64 flags) { vcpu->arch.shared->srr0 = kvmppc_get_pc(vcpu); diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 36d7920..eec353d 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -2048,6 +2048,7 @@ 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 = { + .is_hv_enabled = true, .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, diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S index 5ede7fc..4838fdb 100644 --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S @@ -577,6 +577,10 @@ kvmppc_interrupt: lbz r9, HSTATE_IN_GUEST(r13) cmpwi r9, KVM_GUEST_MODE_HOST_HV beq kvmppc_bad_host_intr +#ifdef CONFIG_KVM_BOOK3S_PR + cmpwi r9, KVM_GUEST_MODE_GUEST + beq kvmppc_interrupt_pr +#endif /* We're now back in the host but in guest MMU context */ li r9, KVM_GUEST_MODE_HOST_HV stb r9, HSTATE_IN_GUEST(r13) diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c index ff5bd24..2a97279 100644 --- a/arch/powerpc/kvm/book3s_pr.c +++ b/arch/powerpc/kvm/book3s_pr.c @@ -1509,6 +1509,7 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val); static struct kvmppc_ops kvmppc_pr_ops = { + .is_hv_enabled = false, .get_sregs = kvm_arch_vcpu_ioctl_get_sregs_pr, .set_sregs = kvm_arch_vcpu_ioctl_set_sregs_pr, .get_one_reg = kvmppc_get_one_reg_pr, diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S index 1abe478..e0229dd 100644 --- a/arch/powerpc/kvm/book3s_segment.S +++ b/arch/powerpc/kvm/book3s_segment.S @@ -161,9 +161,14 @@ kvmppc_handler_trampoline_enter_end: .global kvmppc_handler_trampoline_exit kvmppc_handler_trampoline_exit: +#if defined(CONFIG_KVM_BOOK3S_HV) +.global kvmppc_interrupt_pr +kvmppc_interrupt_pr: + ld r9, HSTATE_SCRATCH2(r13) +#else .global kvmppc_interrupt kvmppc_interrupt: - +#endif /* Register usage at this point: * * SPRG_SCRATCH0 = guest R13 diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c index f0c732e..fa7625d 100644 --- a/arch/powerpc/kvm/book3s_xics.c +++ b/arch/powerpc/kvm/book3s_xics.c @@ -818,7 +818,7 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req) } /* Check for real mode returning too hard */ - if (xics->real_mode) + if (xics->real_mode && kvmppc_ops->is_hv_enabled) return kvmppc_xics_rm_complete(vcpu, req); switch (req) { diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 69b9305..00a96fc 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -52,7 +52,6 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu) return 1; } -#ifndef CONFIG_KVM_BOOK3S_64_HV /* * Common checks before entering the guest world. Call with interrupts * disabled. @@ -127,7 +126,6 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu) return r; } -#endif /* CONFIG_KVM_BOOK3S_64_HV */ int kvmppc_kvm_pv(struct kvm_vcpu *vcpu) { @@ -194,11 +192,9 @@ int kvmppc_sanity_check(struct kvm_vcpu *vcpu) if ((vcpu->arch.cpu_type != KVM_CPU_3S_64) && vcpu->arch.papr_enabled) goto out; -#ifdef CONFIG_KVM_BOOK3S_64_HV /* HV KVM can only do PAPR mode for now */ - if (!vcpu->arch.papr_enabled) + if (!vcpu->arch.papr_enabled && kvmppc_ops->is_hv_enabled) goto out; -#endif #ifdef CONFIG_KVM_BOOKE_HV if (!cpu_has_feature(CPU_FTR_EMB_HV)) @@ -322,22 +318,26 @@ int kvm_dev_ioctl_check_extension(long ext) case KVM_CAP_DEVICE_CTRL: r = 1; break; -#ifndef CONFIG_KVM_BOOK3S_64_HV case KVM_CAP_PPC_PAIRED_SINGLES: case KVM_CAP_PPC_OSI: case KVM_CAP_PPC_GET_PVINFO: #if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC) case KVM_CAP_SW_TLB: #endif -#ifdef CONFIG_KVM_MPIC - case KVM_CAP_IRQ_MPIC: -#endif - r = 1; + /* We support this only for PR */ + r = !kvmppc_ops->is_hv_enabled; break; +#ifdef CONFIG_KVM_MMIO case KVM_CAP_COALESCED_MMIO: r = KVM_COALESCED_MMIO_PAGE_OFFSET; break; #endif +#ifdef CONFIG_KVM_MPIC + case KVM_CAP_IRQ_MPIC: + r = 1; + break; +#endif + #ifdef CONFIG_PPC_BOOK3S_64 case KVM_CAP_SPAPR_TCE: case KVM_CAP_PPC_ALLOC_HTAB: @@ -348,32 +348,37 @@ int kvm_dev_ioctl_check_extension(long ext) r = 1; break; #endif /* CONFIG_PPC_BOOK3S_64 */ -#ifdef CONFIG_KVM_BOOK3S_64_HV +#ifdef CONFIG_KVM_BOOK3S_HV case KVM_CAP_PPC_SMT: - r = threads_per_core; + if (kvmppc_ops->is_hv_enabled) + r = threads_per_core; + else + r = 0; break; case KVM_CAP_PPC_RMA: - r = 1; + r = kvmppc_ops->is_hv_enabled; /* PPC970 requires an RMA */ - if (cpu_has_feature(CPU_FTR_ARCH_201)) + if (r && cpu_has_feature(CPU_FTR_ARCH_201)) r = 2; break; #endif case KVM_CAP_SYNC_MMU: -#ifdef CONFIG_KVM_BOOK3S_64_HV - r = cpu_has_feature(CPU_FTR_ARCH_206) ? 1 : 0; +#ifdef CONFIG_KVM_BOOK3S_HV + if (kvmppc_ops->is_hv_enabled) + r = cpu_has_feature(CPU_FTR_ARCH_206) ? 1 : 0; + else + r = 0; #elif defined(KVM_ARCH_WANT_MMU_NOTIFIER) r = 1; #else r = 0; - break; #endif -#ifdef CONFIG_KVM_BOOK3S_64_HV + break; +#ifdef CONFIG_KVM_BOOK3S_HV case KVM_CAP_PPC_HTAB_FD: - r = 1; + r = kvmppc_ops->is_hv_enabled; break; #endif - break; case KVM_CAP_NR_VCPUS: /* * Recommending a number of CPUs is somewhat arbitrary; we @@ -381,11 +386,10 @@ int kvm_dev_ioctl_check_extension(long ext) * will have secondary threads "offline"), and for other KVM * implementations just count online CPUs. */ -#ifdef CONFIG_KVM_BOOK3S_64_HV - r = num_present_cpus(); -#else - r = num_online_cpus(); -#endif + if (kvmppc_ops->is_hv_enabled) + r = num_present_cpus(); + else + r = num_online_cpus(); break; case KVM_CAP_MAX_VCPUS: r = KVM_MAX_VCPUS;