Re: [RFC PATCH 02/17] KVM: PPC64: booke: Add EPCR support in mtspr/mfspr emulation

On 04.07.2012, at 16:14, Caraman Mihai Claudiu-B02008 wrote:

quoted

-----Original Message-----
From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
owner@vger.kernel.org] On Behalf Of Alexander Graf
Sent: Wednesday, July 04, 2012 4:22 PM
To: Caraman Mihai Claudiu-B02008
Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc-
dev@lists.ozlabs.org; qemu-ppc@nongnu.org
Subject: Re: [RFC PATCH 02/17] KVM: PPC64: booke: Add EPCR support in
mtspr/mfspr emulation
=20
=20
On 25.06.2012, at 14:26, Mihai Caraman wrote:
=20

quoted

Add EPCR support in booke mtspr/mfspr emulation. EPCR register is

defined

quoted

only for 64-bit and HV categories, so it shoud be available only on =

64-

quoted

bit

quoted

virtual processors. Undefine the support for 32-bit builds.
Define a reusable setter function for vcpu's EPCR.
=20
Signed-off-by: Mihai Caraman <redacted>
---
arch/powerpc/kvm/booke.c         |   12 +++++++++++-
arch/powerpc/kvm/booke.h         |    6 ++++++
arch/powerpc/kvm/booke_emulate.c |   13 ++++++++++++-
3 files changed, 29 insertions(+), 2 deletions(-)
=20

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 72f13f4..f9fa260 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c

@@ -13,7 +13,7 @@

* Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  =

02110-1301,

quoted

USA.

quoted

*
* Copyright IBM Corp. 2007
- * Copyright 2010-2011 Freescale Semiconductor, Inc.
+ * Copyright 2010-2012 Freescale Semiconductor, Inc.
*
* Authors: Hollis Blanchard [off-list ref]
*          Christian Ehrhardt [off-list ref]

@@ -1243,6 +1243,16 @@ void kvmppc_core_commit_memory_region(struct =

kvm

quoted

*kvm,

quoted

{
}
=20
+#ifdef CONFIG_64BIT
+void kvmppc_set_epcr(struct kvm_vcpu *vcpu, u32 new_epcr)
+{
+	vcpu->arch.epcr =3D new_epcr;
+	vcpu->arch.shadow_epcr &=3D ~SPRN_EPCR_GICM;
+	if (vcpu->arch.epcr  & SPRN_EPCR_ICM)
+		vcpu->arch.shadow_epcr |=3D SPRN_EPCR_GICM;

=20
Why would the setter be #ifdef CONFIG_64BIT? EPCR exists on e500mc =

too,

quoted

no? Please only #ifdef the GICM bits out.

=20
kvmppc_set_epcr deals with guest EPCR and EPCR does not exist on a =

virtual e500mc

as detailed in patch's comment. All callers are also guarded by #ifdef =

CONFIG_64BIT,

my assumption was that we will not support a virtual core with 64-bit =

category

on a 32-bit host.

My main concern is that every #ifdef potentially breaks things without =
us knowing. So the less #ifdef's we have, the better off we are. The =
spec only says that we don't _have_ to implement EPCR for non-hv =
non-64bit systems. It doesn't forbid to do so, right?


Alex