From: Alexander Graf <agraf@suse.de>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: paulus@samba.org, linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org
Subject: Re: [RFC PATCH 06/11] kvm: powerpc: book3s: Add is_hv_enabled to kvmppc_ops
Date: Fri, 27 Sep 2013 14:18:27 +0200 [thread overview]
Message-ID: <9BAA779D-FE0C-4971-992B-57D80F945906@suse.de> (raw)
In-Reply-To: <1380276233-17095-7-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> 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(-)
>=20
> 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;
> }
>=20
> -#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 =3D 1;
> - else if (old_pending)
> - vcpu->arch.shared->int_pending =3D 0;
> -}
> -
> -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu)
> -{
> - ulong crit_raw =3D vcpu->arch.shared->critical;
> - ulong crit_r1 =3D kvmppc_get_gpr(vcpu, 1);
> - bool crit;
> -
> - /* Truncate crit indicators in 32 bit mode */
> - if (!(vcpu->arch.shared->msr & MSR_SF)) {
> - crit_raw &=3D 0xffffffff;
> - crit_r1 &=3D 0xffffffff;
> - }
> -
> - /* Critical section when crit =3D=3D r1 */
> - crit =3D (crit_raw =3D=3D crit_r1);
> - /* ... and we're in supervisor mode */
> - crit =3D 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);
>=20
> struct kvmppc_ops {
> + bool is_hv_enabled;
This doesn't really belong into an ops struct. Either you compare
if (kvmppc_ops =3D=3D 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)
>=20
> static inline u32 kvmppc_get_xics_latch(void)
> {
> - u32 xirr =3D get_paca()->kvm_hstate.saved_xirr;
> + u32 xirr;
>=20
> + xirr =3D get_paca()->kvm_hstate.saved_xirr;
> get_paca()->kvm_hstate.saved_xirr =3D 0;
> -
I don't see any functionality change here?
> return xirr;
> }
>=20
> 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) +=3D \
> book3s_rmhandlers.o
> endif
>=20
> -kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) :=3D \
> +kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) +=3D \
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)
> {
> }
>=20
> +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 =3D 1;
> + else if (old_pending)
> + vcpu->arch.shared->int_pending =3D 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 =3D vcpu->arch.shared->critical;
> + crit_r1 =3D kvmppc_get_gpr(vcpu, 1);
> +
> + /* Truncate crit indicators in 32 bit mode */
> + if (!(vcpu->arch.shared->msr & MSR_SF)) {
> + crit_raw &=3D 0xffffffff;
> + crit_r1 &=3D 0xffffffff;
> + }
> +
> + /* Critical section when crit =3D=3D r1 */
> + crit =3D (crit_raw =3D=3D crit_r1);
> + /* ... and we're in supervisor mode */
> + crit =3D 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 =3D 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);
>=20
> static struct kvmppc_ops kvmppc_hv_ops =3D {
> + .is_hv_enabled =3D true,
> .get_sregs =3D kvm_arch_vcpu_ioctl_get_sregs_hv,
> .set_sregs =3D kvm_arch_vcpu_ioctl_set_sregs_hv,
> .get_one_reg =3D 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);
>=20
> static struct kvmppc_ops kvmppc_pr_ops =3D {
> + .is_hv_enabled =3D false,
> .get_sregs =3D kvm_arch_vcpu_ioctl_get_sregs_pr,
> .set_sregs =3D kvm_arch_vcpu_ioctl_set_sregs_pr,
> .get_one_reg =3D 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:
>=20
> +#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 =3D 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)
> }
>=20
> /* 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);
>=20
> 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;
> }
>=20
> -#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)
>=20
> return r;
> }
> -#endif /* CONFIG_KVM_BOOK3S_64_HV */
>=20
> 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 !=3D KVM_CPU_3S_64) && =
vcpu->arch.papr_enabled)
> goto out;
>=20
> -#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
>=20
> #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 =3D 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 =3D 1;
> + /* We support this only for PR */
> + r =3D !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 =3D KVM_COALESCED_MMIO_PAGE_OFFSET;
> break;
> #endif
> +#ifdef CONFIG_KVM_MPIC
> + case KVM_CAP_IRQ_MPIC:
> + r =3D 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 =3D 1;
> break;
> #endif /* CONFIG_PPC_BOOK3S_64 */
> -#ifdef CONFIG_KVM_BOOK3S_64_HV
> +#ifdef CONFIG_KVM_BOOK3S_HV
> case KVM_CAP_PPC_SMT:
> - r =3D threads_per_core;
> + if (kvmppc_ops->is_hv_enabled)
> + r =3D threads_per_core;
> + else
> + r =3D 0;
0? Or 1?
Alex
next prev parent reply other threads:[~2013-09-27 12:18 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-27 10:03 [RFC PATCH 00/11 Allow PR and HV KVM to coexist in one kernel Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 01/11] kvm: powerpc: book3s hv: Fix vcore leak Aneesh Kumar K.V
2013-09-27 11:39 ` Alexander Graf
2013-09-27 10:03 ` [RFC PATCH 02/11] kvm: powerpc: book3s: remove kvmppc_handler_highmem label Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 03/11] kvm: powerpc: book3s: move book3s_64_vio_hv.c into the main kernel binary Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 04/11] kvm: powerpc: book3s: Add a new config variable CONFIG_KVM_BOOK3S_HV Aneesh Kumar K.V
2013-09-27 11:43 ` Alexander Graf
2013-09-27 12:45 ` Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 05/11] kvm: powerpc: book3s: Add kvmppc_ops callback for HV and PR specific operations Aneesh Kumar K.V
2013-09-27 12:04 ` Alexander Graf
2013-09-27 12:52 ` Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 06/11] kvm: powerpc: book3s: Add is_hv_enabled to kvmppc_ops Aneesh Kumar K.V
2013-09-27 12:18 ` Alexander Graf [this message]
2013-09-27 13:03 ` Aneesh Kumar K.V
2013-09-30 10:09 ` Alexander Graf
2013-09-30 12:56 ` Aneesh Kumar K.V
2013-09-30 14:51 ` Alexander Graf
2013-09-30 16:20 ` Aneesh Kumar K.V
2013-09-30 16:36 ` Alexander Graf
2013-09-27 10:03 ` [RFC PATCH 07/11] kvm: powerpc: book3s: pr: move PR related tracepoints to a separate header Aneesh Kumar K.V
2013-09-27 12:22 ` Alexander Graf
2013-09-27 13:06 ` Aneesh Kumar K.V
2013-09-30 10:02 ` Alexander Graf
2013-09-30 12:57 ` Aneesh Kumar K.V
2013-09-30 14:51 ` Alexander Graf
2013-09-30 15:53 ` Aneesh Kumar K.V
2013-09-30 15:55 ` Alexander Graf
2013-09-27 10:03 ` [RFC PATCH 08/11] kvm: powerpc: book3s: Support building HV and PR KVM as module Aneesh Kumar K.V
2013-09-27 12:25 ` Alexander Graf
2013-09-27 13:08 ` Aneesh Kumar K.V
2013-09-30 10:04 ` Alexander Graf
2013-09-30 12:57 ` Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 09/11] kvm: simplify processor compat check Aneesh Kumar K.V
2013-09-27 12:31 ` Alexander Graf
2013-09-27 13:13 ` Aneesh Kumar K.V
2013-09-27 15:14 ` Paolo Bonzini
2013-09-28 15:36 ` Aneesh Kumar K.V
2013-09-29 8:58 ` Gleb Natapov
2013-09-29 15:05 ` Aneesh Kumar K.V
2013-09-29 15:11 ` Gleb Natapov
2013-09-27 10:03 ` [RFC PATCH 10/11] kvm: powerpc: book3s: Allow the HV and PR selection per virtual machine Aneesh Kumar K.V
2013-09-27 10:03 ` [RFC PATCH 11/11] kvm: powerpc: book3s: Fix module ownership Aneesh Kumar K.V
2013-09-27 10:52 ` [RFC PATCH 00/11 Allow PR and HV KVM to coexist in one kernel Aneesh Kumar K.V
2013-09-30 10:16 ` Alexander Graf
2013-09-30 13:09 ` Aneesh Kumar K.V
2013-09-30 14:54 ` Alexander Graf
2013-10-01 11:26 ` Aneesh Kumar K.V
2013-10-01 11:36 ` Alexander Graf
2013-10-01 11:41 ` Paolo Bonzini
2013-10-01 11:43 ` Alexander Graf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9BAA779D-FE0C-4971-992B-57D80F945906@suse.de \
--to=agraf@suse.de \
--cc=aneesh.kumar@linux.vnet.ibm.com \
--cc=kvm-ppc@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).