On 04.07.14 01:00, Scott Wood wrote:
On Fri, 2014-07-04 at 00:35 +0200, Alexander Graf wrote:
On 04.07.14 00:31, Scott Wood wrote:
On Thu, 2014-07-03 at 17:15 -0500, Scott Wood wrote:
On Thu, 2014-07-03 at 10:25 -0500, Caraman Mihai Claudiu-B02008 wrote:
-----Original Message-----
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, July 03, 2014 3:21 PM
To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
Cc: k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for
SPE/FP/AltiVec int numbers
On 30.06.14 17:34, Mihai Caraman wrote:
Use common BOOKE_IRQPRIO and BOOKE_INTERRUPT defines for SPE/FP/AltiVec
which share the same interrupt numbers.
Signed-off-by: Mihai Caraman <mihai.cara...@freescale.com>
---
v2:
- remove outdated definitions
arch/powerpc/include/asm/kvm_asm.h | 8 --------
arch/powerpc/kvm/booke.c | 17 +++++++++--------
arch/powerpc/kvm/booke.h | 4 ++--
arch/powerpc/kvm/booke_interrupts.S | 9 +++++----
arch/powerpc/kvm/bookehv_interrupts.S | 4 ++--
arch/powerpc/kvm/e500.c | 10 ++++++----
arch/powerpc/kvm/e500_emulate.c | 10 ++++++----
7 files changed, 30 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_asm.h
b/arch/powerpc/include/asm/kvm_asm.h
index 9601741..c94fd33 100644
--- a/arch/powerpc/include/asm/kvm_asm.h
+++ b/arch/powerpc/include/asm/kvm_asm.h
@@ -56,14 +56,6 @@
/* E500 */
#define BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL 32
#define BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST 33
-/*
- * TODO: Unify 32-bit and 64-bit kernel exception handlers to use same
defines
- */
-#define BOOKE_INTERRUPT_SPE_UNAVAIL
BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL
-#define BOOKE_INTERRUPT_SPE_FP_DATA
BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
-#define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL
BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL
-#define BOOKE_INTERRUPT_ALTIVEC_ASSIST \
- BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
I think I'd prefer to keep them separate.
What is the reason from changing your mind from ver 1? Do you want to have
different defines with same values (we specifically mapped them to the
hardware interrupt numbers). We already upstreamed the necessary changes
in the kernel. Scott, please share your opinion here.
I don't like hiding the fact that they're the same number, which could
lead to wrong code in the absence of ifdefs that strictly mutually
exclude SPE and Altivec code -- there was an instance of this with
MSR_VEC versus MSR_SPE in a previous patchset.
That said, if you want to enforce that mutual exclusion in a way that is
clear, I won't object too loudly -- but the code does look pretty
similar between the two (as well as between the two IVORs).
Yes, I want to make sure we have 2 separate code paths for SPE and
Altivec. No code sharing at all unless it's very generically possible.
Also, which code does look pretty similar? The fact that we deflect
interrupts back into the guest? That's mostly boilerplate.
There's also the injection of a program check (or exiting to userspace)
when CONFIG_SPE/ALTIVEC is missing. Not a big deal, but maybe it could
be factored into a helper function. I like minimizing boilerplate.
Yes, me too - but I also like to be explicit. If there's enough code to
share, factoring those into helpers certainly works well for me.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
