Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest
On 02.07.14 19:28, bharat.bhus...@freescale.com wrote: -Original Message- From: Bhushan Bharat-R65777 Sent: Wednesday, July 02, 2014 5:07 PM To: Wood Scott-B07421; Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest -Original Message- From: Wood Scott-B07421 Sent: Tuesday, July 01, 2014 10:11 PM To: Alexander Graf Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest On Tue, 2014-07-01 at 18:22 +0200, Alexander Graf wrote: On 01.07.14 17:35, Scott Wood wrote: On Tue, 2014-07-01 at 17:04 +0200, Alexander Graf wrote: On 01.07.14 16:58, Scott Wood wrote: On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote: I don't think QEMU should be aware of these limitations. OK, but we should at least have some idea of how the whole thing is supposed to work, in order to determine if this is the correct behavior for QEMU. I thought the model was that debug resources are either owned by QEMU or by the guest, and in the latter case, QEMU would never see the debug exception to begin with. That's bad for a number of reasons. For starters it's different from how x86 handles debug registers - and I hate to be different just for the sake of being different. How does it work on x86? It overwrites more-or-less random breakpoints with its own ones, but leaves the others intact ;). Are you talking about software breakpoints or management of hardware debug registers? So if we do want to declare that debug registers are owned by either QEMU or the guest, we should change the semantics for all architectures. If we want to say that ownership of the registers is shared, we need a plan for how that would actually work. I think you're overengineering here :). When do people actually use gdbstub? Usually when they want to debug a broken guest. We can either * overengineer heavily and reduce the number of registers available to the guest to always have spares * overengineer a bit and turn off guest debugging completely when we use gdbstub * just leave as much alive as we can, hoping that it helps with the debugging Option 3 is what x86 does - and I think it's a reasonable approach. This is not an interface that needs to be 100% consistent and bullet proof, it's a best effort to enable you to debug as much as possible. I'm not insisting on 100% -- just hoping for some explanation/discussion about how it's intended to work for the cases where it can. How will MSR[DE] and MSRP[DEP] be handled? How would I go about telling QEMU/KVM that I don't want this shared mode, because I don't want guest to interfere with the debugging I'm trying to do from QEMU? Will guest accesses to debug registers cause a userspace exit when guest_debug is enabled? I think we're in a path that is slow enough already to not worry about performance. It's not just about performance, but simplicity of use, and consistency of API. Oh, and it looks like there already exist one reg definitions and implementations for most of the debug registers. For BookE? Where? arch/powerpc/kvm/booke.c: KVM_REG_PPC_IACn, KVM_REG_PPC_DACn I tried to quickly prototype what I think we want to do (this is not tested) Hi Scott, There is one problem which is stopping us to share debug resource between qemu and guest, looking for suggestions: - As qemu is also using debug resource, We have to set MSR_DE and set MSRP_DEP (guest will not be able to clear MSR_DE). So qemu set debug events will always cause the debug interrupts. - Now guest is also using debug resources and for some reason if guest wants to clear MSR_DE (disable debug interrupt) But it will not be able to disable as MSRP_DEP is set and KVM will not come to know guest willingness to disable MSR_DE. - If the debug interrupts occurs then we will exit to QEMU and this may not a QEMU set event so it will inject interrupt to guest (using one-reg or set-sregs) - Now KVM, when handling one-reg/sregs request to inject debug interrupt, do not know whether guest can handle the debug interrupt or not (as guest might have tried to set/clear MSR_DE). Yeah, and with this everything falls apart. I guess we can't share hardware debug resources after all then. So yes, let's declare all hardware debug facilities QEMU owned as soon as QEMU starts using gdbstub. 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
Re: [PATCH v2] powerpc/kvm: support to handle sw breakpoint
On 01.07.14 10:41, Madhavan Srinivasan wrote:
This patch adds kernel side support for software breakpoint.
Design is that, by using an illegal instruction, we trap to hypervisor
via Emulation Assistance interrupt, where we check for the illegal instruction
and accordingly we return to Host or Guest. Patch also adds support for
software breakpoint in PR KVM.
Patch mandates use of abs instruction as sw breakpoint instruction
(primary opcode 31 and extended opcode 360). Based on PowerISA v2.01,
ABS instruction has been dropped from the architecture and treated an
illegal instruction.
Changes v1-v2:
Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also
share it.
Added code to use KVM get one reg infrastructure to get debug opcode.
Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
Made changes to commit message.
Signed-off-by: Madhavan Srinivasan ma...@linux.vnet.ibm.com
---
arch/powerpc/include/asm/kvm_book3s.h |8
arch/powerpc/include/asm/ppc-opcode.h |5 +
arch/powerpc/kvm/book3s.c |3 ++-
arch/powerpc/kvm/book3s_hv.c |9 +
arch/powerpc/kvm/book3s_pr.c |3 +++
arch/powerpc/kvm/emulate.c| 10 ++
6 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index f52f656..180d549 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -24,6 +24,14 @@
#include linux/kvm_host.h
#include asm/kvm_book3s_asm.h
+/*
+ * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software
Breakpoint.
+ * Instruction mnemonic is ABS, primary opcode is 31 and extended opcode is
360.
+ * Based on PowerISA v2.01, ABS instruction has been dropped from the
architecture
+ * and treated an illegal instruction.
+ */
+#define KVMPPC_INST_BOOK3S_DEBUG 0x7c0002d0
This will still break with LE guests.
+
struct kvmppc_bat {
u64 raw;
u32 bepi;
diff --git a/arch/powerpc/include/asm/ppc-opcode.h
b/arch/powerpc/include/asm/ppc-opcode.h
index 3132bb9..3fbb4c1 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -111,6 +111,11 @@
#define OP_31_XOP_LHBRX 790
#define OP_31_XOP_STHBRX918
+/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction
+ * Instruction mnemonic is ABS, primary opcode is 31 and extended opcode is
360.
+ */
+#define OP_31_XOP_ABS 360
+
#define OP_LWZ 32
#define OP_LD 58
#define OP_LWZU 33
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index c254c27..b40fe5d 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
- return -EINVAL;
+ vcpu-guest_debug = dbg-control;
+ return 0;
}
void kvmppc_decrementer_func(unsigned long data)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 7a12edb..402c1ec 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -725,8 +725,14 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run,
struct kvm_vcpu *vcpu,
* we don't emulate any guest instructions at this stage.
*/
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
+ if (kvmppc_get_last_inst(vcpu) == KVMPPC_INST_BOOK3S_DEBUG ) {
+ run-exit_reason = KVM_EXIT_DEBUG;
+ run-debug.arch.address = kvmppc_get_pc(vcpu);
+ r = RESUME_HOST;
Phew - why can't we just go into the normal instruction emulator for
EMUL_ASSIST?
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
Re: [PATCH] KVM: PPC: e500: Fix default tlb for victim hint
On 30.06.14 20:18, Scott Wood wrote:
On Mon, 2014-06-30 at 15:54 +0300, Mihai Caraman wrote:
Tlb search operation used for victim hint relies on the default tlb set by the
host. When hardware tablewalk support is enabled in the host, the default tlb is
TLB1 which leads KVM to evict the bolted entry. Set and restore the default tlb
when searching for victim hint.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
arch/powerpc/include/asm/mmu-book3e.h | 5 -
arch/powerpc/kvm/e500_mmu_host.c | 4
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/mmu-book3e.h
b/arch/powerpc/include/asm/mmu-book3e.h
index 901dac6..5dad378 100644
--- a/arch/powerpc/include/asm/mmu-book3e.h
+++ b/arch/powerpc/include/asm/mmu-book3e.h
@@ -40,7 +40,9 @@
/* MAS registers bit definitions */
-#define MAS0_TLBSEL(x) (((x) 28) 0x3000)
+#define MAS0_TLBSEL_MASK0x3000
+#define MAS0_TLBSEL_SHIFT 28
+#define MAS0_TLBSEL(x) (((x) MAS0_TLBSEL_SHIFT) MAS0_TLBSEL_MASK)
#define MAS0_ESEL_MASK0x0FFF
#define MAS0_ESEL_SHIFT 16
#define MAS0_ESEL(x) (((x) MAS0_ESEL_SHIFT) MAS0_ESEL_MASK)
@@ -86,6 +88,7 @@
#define MAS3_SPSIZE 0x003e
#define MAS3_SPSIZE_SHIFT 1
+#define MAS4_TLBSEL_MASK MAS0_TLBSEL_MASK
#define MAS4_TLBSELD(x) MAS0_TLBSEL(x)
#define MAS4_INDD 0x8000 /* Default IND */
#define MAS4_TSIZED(x)MAS1_TSIZE(x)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index dd2cc03..79677d7 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -107,11 +107,15 @@ static u32 get_host_mas0(unsigned long eaddr)
{
unsigned long flags;
u32 mas0;
+ u32 mas4;
local_irq_save(flags);
mtspr(SPRN_MAS6, 0);
+ mas4 = mfspr(SPRN_MAS4);
+ mtspr(SPRN_MAS4, mas4 ~MAS4_TLBSEL_MASK);
asm volatile(tlbsx 0, %0 : : b (eaddr ~CONFIG_PAGE_OFFSET));
mas0 = mfspr(SPRN_MAS0);
+ mtspr(SPRN_MAS4, mas4);
local_irq_restore(flags);
return mas0;
Reviewed-by: Scott Wood scottw...@freescale.com
Thanks, applied to kvm-ppc-queue.
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
Re: [PATCH 2/6 v2] KVM: PPC: Book3E: Refactor SPE/FP exit handling
On 30.06.14 17:34, Mihai Caraman wrote: SPE/FP/AltiVec interrupts share the same numbers. Refactor SPE/FP exit handling to accommodate AltiVec later on the same flow. Add kvmppc_supports_spe() to detect suport for the unit at runtime since it can be configured in the kernel but not featured on hardware and vice versa. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com Why not keep them 100% separate? 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
Re: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness
On 30.06.14 17:34, Mihai Caraman wrote: Increase FPU laziness by calling kvmppc_load_guest_fp() just before returning to guest instead of each sched in. Without this improvement an interrupt may also claim floting point corrupting guest state. How do you handle context switching with this patch applied? During most of the guest's lifetime we never exit kvmppc_vcpu_run(), so when the guest gets switched out all FPU state gets lost? 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
Re: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support
On 30.06.14 17:34, Mihai Caraman wrote: Add KVM Book3E AltiVec support. KVM Book3E FPU support gracefully reuse host infrastructure so follow the same approach for AltiVec. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com Same comment here - I fail to see how we refetch Altivec state after a context switch. 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
Re: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst() function
On 28.06.14 00:49, Mihai Caraman wrote:
In the context of replacing kvmppc_ld() function calls with a version of
kvmppc_get_last_inst() which allow to fail, Alex Graf suggested this:
If we get EMULATE_AGAIN, we just have to make sure we go back into the guest.
No need to inject an ISI into the guest - it'll do that all by itself.
With an error returning kvmppc_get_last_inst we can just use completely
get rid of kvmppc_read_inst() and only use kvmppc_get_last_inst() instead.
As a intermediate step get rid of kvmppc_read_inst() and only use kvmppc_ld()
instead.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
v4:
- new patch
arch/powerpc/kvm/book3s_pr.c | 85 ++--
1 file changed, 35 insertions(+), 50 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 15fd6c2..d247d88 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -665,42 +665,6 @@ static void kvmppc_giveup_fac(struct kvm_vcpu *vcpu, ulong
fac)
#endif
}
-static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
-{
- ulong srr0 = kvmppc_get_pc(vcpu);
- u32 last_inst = kvmppc_get_last_inst(vcpu);
- int ret;
-
- ret = kvmppc_ld(vcpu, srr0, sizeof(u32), last_inst, false);
- if (ret == -ENOENT) {
- ulong msr = kvmppc_get_msr(vcpu);
-
- msr = kvmppc_set_field(msr, 33, 33, 1);
- msr = kvmppc_set_field(msr, 34, 36, 0);
- msr = kvmppc_set_field(msr, 42, 47, 0);
- kvmppc_set_msr_fast(vcpu, msr);
- kvmppc_book3s_queue_irqprio(vcpu,
BOOK3S_INTERRUPT_INST_STORAGE);
- return EMULATE_AGAIN;
- }
-
- return EMULATE_DONE;
-}
-
-static int kvmppc_check_ext(struct kvm_vcpu *vcpu, unsigned int exit_nr)
-{
-
- /* Need to do paired single emulation? */
- if (!(vcpu-arch.hflags BOOK3S_HFLAG_PAIRED_SINGLE))
- return EMULATE_DONE;
-
- /* Read out the instruction */
- if (kvmppc_read_inst(vcpu) == EMULATE_DONE)
- /* Need to emulate */
- return EMULATE_FAIL;
-
- return EMULATE_AGAIN;
-}
-
/* Handle external providers (FPU, Altivec, VSX) */
static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int exit_nr,
ulong msr)
@@ -1101,31 +1065,51 @@ program_interrupt:
case BOOK3S_INTERRUPT_VSX:
{
int ext_msr = 0;
+ int emul;
+ ulong pc;
+ u32 last_inst;
- switch (exit_nr) {
- case BOOK3S_INTERRUPT_FP_UNAVAIL: ext_msr = MSR_FP; break;
- case BOOK3S_INTERRUPT_ALTIVEC:ext_msr = MSR_VEC; break;
- case BOOK3S_INTERRUPT_VSX:ext_msr = MSR_VSX; break;
- }
+ if (!(vcpu-arch.hflags BOOK3S_HFLAG_PAIRED_SINGLE)) {
Please make paired single emulation the unusual, if()'ed case, not the
normal exit path :).
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
Re: [PATCH 4/5 v4] KVM: PPC: Alow kvmppc_get_last_inst() to fail
On 28.06.14 00:49, Mihai Caraman wrote:
On book3e, guest last instruction is read on the exit path using load
external pid (lwepx) dedicated instruction. This load operation may fail
due to TLB eviction and execute-but-not-read entries.
This patch lay down the path for an alternative solution to read the guest
last instruction, by allowing kvmppc_get_lat_inst() function to fail.
Architecture specific implmentations of kvmppc_load_last_inst() may read
last guest instruction and instruct the emulation layer to re-execute the
guest in case of failure.
Make kvmppc_get_last_inst() definition common between architectures.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
v4:
- these changes compile on book3s, please validate the functionality and
do the necessary adaptations!
- common declaration and enum for kvmppc_load_last_inst()
- remove kvmppc_read_inst() in a preceding patch
v3:
- rework patch description
- add common definition for kvmppc_get_last_inst()
- check return values in book3s code
v2:
- integrated kvmppc_get_last_inst() in book3s code and checked build
- addressed cosmetic feedback
arch/powerpc/include/asm/kvm_book3s.h| 26 --
arch/powerpc/include/asm/kvm_booke.h | 5
arch/powerpc/include/asm/kvm_ppc.h | 24 +
arch/powerpc/kvm/book3s.c| 11
arch/powerpc/kvm/book3s_64_mmu_hv.c | 17
arch/powerpc/kvm/book3s_paired_singles.c | 38 +--
arch/powerpc/kvm/book3s_pr.c | 45
arch/powerpc/kvm/booke.c | 3 +++
arch/powerpc/kvm/e500_mmu_host.c | 6 +
arch/powerpc/kvm/emulate.c | 18 -
arch/powerpc/kvm/powerpc.c | 11 ++--
11 files changed, 128 insertions(+), 76 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index ceb70aa..1300cd9 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -276,32 +276,6 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu
*vcpu)
return (kvmppc_get_msr(vcpu) MSR_LE) != (MSR_KERNEL MSR_LE);
}
-static inline u32 kvmppc_get_last_inst_internal(struct kvm_vcpu *vcpu, ulong pc)
-{
- /* Load the instruction manually if it failed to do so in the
-* exit path */
- if (vcpu-arch.last_inst == KVM_INST_FETCH_FAILED)
- kvmppc_ld(vcpu, pc, sizeof(u32), vcpu-arch.last_inst, false);
-
- return kvmppc_need_byteswap(vcpu) ? swab32(vcpu-arch.last_inst) :
- vcpu-arch.last_inst;
-}
-
-static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)
-{
- return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu));
-}
-
-/*
- * Like kvmppc_get_last_inst(), but for fetching a sc instruction.
- * Because the sc instruction sets SRR0 to point to the following
- * instruction, we have to fetch from pc - 4.
- */
-static inline u32 kvmppc_get_last_sc(struct kvm_vcpu *vcpu)
-{
- return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu) - 4);
-}
-
static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu)
{
return vcpu-arch.fault_dar;
diff --git a/arch/powerpc/include/asm/kvm_booke.h
b/arch/powerpc/include/asm/kvm_booke.h
index c7aed61..cbb1990 100644
--- a/arch/powerpc/include/asm/kvm_booke.h
+++ b/arch/powerpc/include/asm/kvm_booke.h
@@ -69,11 +69,6 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu
*vcpu)
return false;
}
-static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)
-{
- return vcpu-arch.last_inst;
-}
-
static inline void kvmppc_set_ctr(struct kvm_vcpu *vcpu, ulong val)
{
vcpu-arch.ctr = val;
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index e2fd5a1..ec326c8 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -47,6 +47,11 @@ enum emulation_result {
EMULATE_EXIT_USER,/* emulation requires exit to user-space */
};
+enum instruction_type {
+ INST_GENERIC,
+ INST_SC,/* system call */
+};
+
extern int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu);
extern int __kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu);
extern void kvmppc_handler_highmem(void);
@@ -62,6 +67,9 @@ extern int kvmppc_handle_store(struct kvm_run *run, struct
kvm_vcpu *vcpu,
u64 val, unsigned int bytes,
int is_default_endian);
+extern int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
+enum instruction_type type, u32 *inst);
+
extern int kvmppc_emulate_instruction(struct kvm_run *run,
struct kvm_vcpu *vcpu);
extern int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu);
@@
Re: [PATCH 5/5 v4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
On 28.06.14 00:49, Mihai Caraman wrote:
On book3e, KVM uses load external pid (lwepx) dedicated instruction to read
guest last instruction on the exit path. lwepx exceptions (DTLB_MISS, DSI
and LRAT), generated by loading a guest address, needs to be handled by KVM.
These exceptions are generated in a substituted guest translation context
(EPLC[EGS] = 1) from host context (MSR[GS] = 0).
Currently, KVM hooks only interrupts generated from guest context (MSR[GS] = 1),
doing minimal checks on the fast path to avoid host performance degradation.
lwepx exceptions originate from host state (MSR[GS] = 0) which implies
additional checks in DO_KVM macro (beside the current MSR[GS] = 1) by looking
at the Exception Syndrome Register (ESR[EPID]) and the External PID Load Context
Register (EPLC[EGS]). Doing this on each Data TLB miss exception is obvious
too intrusive for the host.
Read guest last instruction from kvmppc_load_last_inst() by searching for the
physical address and kmap it. This address the TODO for TLB eviction and
execute-but-not-read entries, and allow us to get rid of lwepx until we are
able to handle failures.
A simple stress benchmark shows a 1% sys performance degradation compared with
previous approach (lwepx without failure handling):
time for i in `seq 1 1`; do /bin/echo /dev/null; done
real0m 8.85s
user0m 4.34s
sys 0m 4.48s
vs
real0m 8.84s
user0m 4.36s
sys 0m 4.44s
An alternative solution, to handle lwepx exceptions in KVM, is to temporary
highjack the interrupt vector from host. Some cores share host IVOR registers
between hardware threads, which is the case of FSL e6500, which impose
additional
synchronization logic for this solution to work. The optimization can be
addressed
later on top of this patch.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
v4:
- add switch and new function when getting last inst earlier
- use enum instead of prev semnatic
- get rid of mas0, optimize mas7_mas3
- give more context in visible messages
- check storage attributes mismatch on MMUv2
- get rid of pfn_valid check
v3:
- reworked patch description
- use unaltered kmap addr for kunmap
- get last instruction before beeing preempted
v2:
- reworked patch description
- used pr_* functions
- addressed cosmetic feedback
arch/powerpc/kvm/booke.c | 44 +
arch/powerpc/kvm/bookehv_interrupts.S | 37 --
arch/powerpc/kvm/e500_mmu_host.c | 91 +++
3 files changed, 144 insertions(+), 28 deletions(-)
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 34a42b9..843077b 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -869,6 +869,28 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
}
}
+static int kvmppc_resume_inst_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
+ enum emulation_result emulated, u32 last_inst)
+{
+ switch (emulated) {
+ case EMULATE_AGAIN:
+ return RESUME_GUEST;
+
+ case EMULATE_FAIL:
+ pr_debug(%s: load instruction from guest address %lx failed\n,
+ __func__, vcpu-arch.pc);
+ /* For debugging, encode the failing instruction and
+* report it to userspace. */
+ run-hw.hardware_exit_reason = ~0ULL 32;
+ run-hw.hardware_exit_reason |= last_inst;
+ kvmppc_core_queue_program(vcpu, ESR_PIL);
+ return RESUME_HOST;
+
+ default:
+ BUG();
+ }
+}
+
/**
* kvmppc_handle_exit
*
@@ -880,6 +902,8 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu
*vcpu,
int r = RESUME_HOST;
int s;
int idx;
+ u32 last_inst = KVM_INST_FETCH_FAILED;
+ enum emulation_result emulated = EMULATE_DONE;
/* update before a new last_exit_type is rewritten */
kvmppc_update_timing_stats(vcpu);
@@ -887,6 +911,20 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
kvm_vcpu *vcpu,
/* restart interrupts if they were meant for the host */
kvmppc_restart_interrupt(vcpu, exit_nr);
+ /*
+* get last instruction before beeing preempted
+* TODO: for e6500 check also BOOKE_INTERRUPT_LRAT_ERROR ESR_DATA
+*/
+ switch (exit_nr) {
+ case BOOKE_INTERRUPT_DATA_STORAGE:
+ case BOOKE_INTERRUPT_DTLB_MISS:
+ case BOOKE_INTERRUPT_HV_PRIV:
+ emulated = kvmppc_get_last_inst(vcpu, false, last_inst);
+ break;
+ default:
+ break;
+ }
+
local_irq_enable();
trace_kvm_exit(exit_nr, vcpu);
@@ -895,6 +933,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
kvm_vcpu *vcpu,
run-exit_reason = KVM_EXIT_UNKNOWN;
run-ready_for_interrupt_injection = 1;
+ if (emulated != EMULATE_DONE) {
+ r =
[RFC PATCH 0/4] KVM Book3E support for HTW guests
KVM Book3E support for Hardware Page Tablewalk enabled guests. Mihai Caraman (4): powerpc/booke64: Add LRAT next and max entries to tlb_core_data structure KVM: PPC: Book3E: Handle LRAT error exception KVM: PPC: e500: TLB emulation for IND entries KVM: PPC: e500mc: Advertise E.PT to support HTW guests arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/include/asm/kvm_ppc.h| 2 + arch/powerpc/include/asm/mmu-book3e.h | 12 +++ arch/powerpc/include/asm/reg_booke.h | 14 +++ arch/powerpc/kernel/asm-offsets.c | 1 + arch/powerpc/kvm/booke.c | 40 + arch/powerpc/kvm/bookehv_interrupts.S | 9 +- arch/powerpc/kvm/e500.h | 81 ++ arch/powerpc/kvm/e500_mmu.c | 84 ++ arch/powerpc/kvm/e500_mmu_host.c | 156 +- arch/powerpc/kvm/e500mc.c | 55 +++- arch/powerpc/mm/fsl_booke_mmu.c | 8 ++ 12 files changed, 423 insertions(+), 40 deletions(-) -- 1.7.11.7 -- 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
[RFC PATCH 1/4] powerpc/booke64: Add LRAT next and max entries to tlb_core_data structure
LRAT (Logical to Real Address Translation) is shared between hw threads.
Add LRAT next and max entries to tlb_core_data structure and initialize them.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
arch/powerpc/include/asm/mmu-book3e.h | 7 +++
arch/powerpc/include/asm/reg_booke.h | 1 +
arch/powerpc/mm/fsl_booke_mmu.c | 8
3 files changed, 16 insertions(+)
diff --git a/arch/powerpc/include/asm/mmu-book3e.h
b/arch/powerpc/include/asm/mmu-book3e.h
index 8d24f78..088fd9f 100644
--- a/arch/powerpc/include/asm/mmu-book3e.h
+++ b/arch/powerpc/include/asm/mmu-book3e.h
@@ -217,6 +217,12 @@
#define TLBILX_T_CLASS26
#define TLBILX_T_CLASS37
+/* LRATCFG bits */
+#define LRATCFG_ASSOC 0xFF00
+#define LRATCFG_LASIZE 0x00FE
+#define LRATCFG_LPID 0x2000
+#define LRATCFG_NENTRY 0x0FFF
+
#ifndef __ASSEMBLY__
#include asm/bug.h
@@ -294,6 +300,7 @@ struct tlb_core_data {
/* For software way selection, as on Freescale TLB1 */
u8 esel_next, esel_max, esel_first;
+ u8 lrat_next, lrat_max;
};
#ifdef CONFIG_PPC64
diff --git a/arch/powerpc/include/asm/reg_booke.h
b/arch/powerpc/include/asm/reg_booke.h
index 464f108..75bda23 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -64,6 +64,7 @@
#define SPRN_DVC2 0x13F /* Data Value Compare Register 2 */
#define SPRN_LPID 0x152 /* Logical Partition ID */
#define SPRN_MAS8 0x155 /* MMU Assist Register 8 */
+#define SPRN_LRATCFG 0x156 /* LRAT Configuration Register */
#define SPRN_TLB0PS0x158 /* TLB 0 Page Size Register */
#define SPRN_TLB1PS0x159 /* TLB 1 Page Size Register */
#define SPRN_MAS5_MAS6 0x15c /* MMU Assist Register 5 || 6 */
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index 94cd728..6492708 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -196,6 +196,14 @@ static unsigned long map_mem_in_cams_addr(phys_addr_t
phys, unsigned long virt,
get_paca()-tcd.esel_next = i;
get_paca()-tcd.esel_max = mfspr(SPRN_TLB1CFG) TLBnCFG_N_ENTRY;
get_paca()-tcd.esel_first = i;
+
+ get_paca()-tcd.lrat_next = 0;
+ if (((mfspr(SPRN_MMUCFG) MMUCFG_MAVN) == MMUCFG_MAVN_V2)
+ (mfspr(SPRN_MMUCFG) MMUCFG_LRAT)) {
+ get_paca()-tcd.lrat_max = mfspr(SPRN_LRATCFG) LRATCFG_NENTRY;
+ } else {
+ get_paca()-tcd.lrat_max = 0;
+ }
#endif
return amount_mapped;
--
1.7.11.7
--
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
[RFC PATCH 3/4] KVM: PPC: e500: TLB emulation for IND entries
Handle indirect entries (IND) in TLB emulation code. Translation size of IND
entries differ from the size of referred Page Tables (Linux guests now use IND
of 2MB for 4KB PTs) and this require careful tweak of the existing logic.
TLB search emulation requires additional search in HW TLB0 (since these entries
are directly added by HTW) and found entries shoud be presented to the guest
with
RPN changed from PFN to GFN. There might be more GFNs pointing to the same PFN
so
the only way to get the corresponding GFN is to search it in guest's PTE. If IND
entry for the corresponding PT is not available just invalidate guest's ea and
report a tlbsx miss. This patch only implements the invalidation and let a TODO
note for searching HW TLB0.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
arch/powerpc/include/asm/mmu-book3e.h | 2 +
arch/powerpc/kvm/e500.h | 81 ---
arch/powerpc/kvm/e500_mmu.c | 78 +++--
arch/powerpc/kvm/e500_mmu_host.c | 31 --
arch/powerpc/kvm/e500mc.c | 53 +--
5 files changed, 211 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu-book3e.h
b/arch/powerpc/include/asm/mmu-book3e.h
index ac6acf7..e482ad8 100644
--- a/arch/powerpc/include/asm/mmu-book3e.h
+++ b/arch/powerpc/include/asm/mmu-book3e.h
@@ -59,6 +59,7 @@
#define MAS1_IPROT 0x4000
#define MAS1_TID(x)(((x) 16) 0x3FFF)
#define MAS1_IND 0x2000
+#define MAS1_IND_SHIFT 13
#define MAS1_TS0x1000
#define MAS1_TSIZE_MASK0x0f80
#define MAS1_TSIZE_SHIFT 7
@@ -94,6 +95,7 @@
#define MAS4_TLBSEL_MASK MAS0_TLBSEL_MASK
#define MAS4_TLBSELD(x)MAS0_TLBSEL(x)
#define MAS4_INDD 0x8000 /* Default IND */
+#define MAS4_INDD_SHIFT15
#define MAS4_TSIZED(x) MAS1_TSIZE(x)
#define MAS4_X0D 0x0040
#define MAS4_X1D 0x0020
diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
index a326178..70a556d 100644
--- a/arch/powerpc/kvm/e500.h
+++ b/arch/powerpc/kvm/e500.h
@@ -148,6 +148,22 @@ unsigned int kvmppc_e500_get_sid(struct kvmppc_vcpu_e500
*vcpu_e500,
unsigned int pr, int avoid_recursion);
#endif
+static inline bool has_feature(const struct kvm_vcpu *vcpu,
+ enum vcpu_ftr ftr)
+{
+ bool has_ftr;
+
+ switch (ftr) {
+ case VCPU_FTR_MMU_V2:
+ has_ftr = ((vcpu-arch.mmucfg MMUCFG_MAVN) == MMUCFG_MAVN_V2);
+ break;
+
+ default:
+ return false;
+ }
+ return has_ftr;
+}
+
/* TLB helper functions */
static inline unsigned int
get_tlb_size(const struct kvm_book3e_206_tlb_entry *tlbe)
@@ -207,6 +223,16 @@ get_tlb_tsize(const struct kvm_book3e_206_tlb_entry *tlbe)
return (tlbe-mas1 MAS1_TSIZE_MASK) MAS1_TSIZE_SHIFT;
}
+static inline unsigned int
+get_tlb_ind(const struct kvm_vcpu *vcpu,
+ const struct kvm_book3e_206_tlb_entry *tlbe)
+{
+ if (has_feature(vcpu, VCPU_FTR_MMU_V2))
+ return (tlbe-mas1 MAS1_IND) MAS1_IND_SHIFT;
+
+ return 0;
+}
+
static inline unsigned int get_cur_pid(struct kvm_vcpu *vcpu)
{
return vcpu-arch.pid 0xff;
@@ -232,6 +258,30 @@ static inline unsigned int get_cur_sas(const struct
kvm_vcpu *vcpu)
return vcpu-arch.shared-mas6 0x1;
}
+static inline unsigned int get_cur_ind(const struct kvm_vcpu *vcpu)
+{
+ if (has_feature(vcpu, VCPU_FTR_MMU_V2))
+ return (vcpu-arch.shared-mas1 MAS1_IND) MAS1_IND_SHIFT;
+
+ return 0;
+}
+
+static inline unsigned int get_cur_indd(const struct kvm_vcpu *vcpu)
+{
+ if (has_feature(vcpu, VCPU_FTR_MMU_V2))
+ return (vcpu-arch.shared-mas4 MAS4_INDD) MAS4_INDD_SHIFT;
+
+ return 0;
+}
+
+static inline unsigned int get_cur_sind(const struct kvm_vcpu *vcpu)
+{
+ if (has_feature(vcpu, VCPU_FTR_MMU_V2))
+ return (vcpu-arch.shared-mas6 MAS6_SIND) MAS6_SIND_SHIFT;
+
+ return 0;
+}
+
static inline unsigned int get_tlb_tlbsel(const struct kvm_vcpu *vcpu)
{
/*
@@ -286,6 +336,22 @@ void kvmppc_e500_tlbil_one(struct kvmppc_vcpu_e500
*vcpu_e500,
void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500 *vcpu_e500);
#ifdef CONFIG_KVM_BOOKE_HV
+void inval_tlb_on_host(struct kvm_vcpu *vcpu, int type, int pid);
+
+void inval_ea_on_host(struct kvm_vcpu *vcpu, gva_t ea, int pid, int sas,
+ int sind);
+#else
+/* TLB is fully virtualized */
+static inline void inval_tlb_on_host(struct kvm_vcpu *vcpu,
+int type, int pid)
+{}
+
+static inline void inval_ea_on_host(struct kvm_vcpu *vcpu, gva_t ea, int pid,
+ int sas, int sind)
+{}
+#endif
+
+#ifdef
[RFC PATCH 2/4] KVM: PPC: Book3E: Handle LRAT error exception
Handle LRAT error exception with support for lrat mapping and invalidation.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
arch/powerpc/include/asm/kvm_host.h | 1 +
arch/powerpc/include/asm/kvm_ppc.h| 2 +
arch/powerpc/include/asm/mmu-book3e.h | 3 +
arch/powerpc/include/asm/reg_booke.h | 13
arch/powerpc/kernel/asm-offsets.c | 1 +
arch/powerpc/kvm/booke.c | 40 +++
arch/powerpc/kvm/bookehv_interrupts.S | 9 ++-
arch/powerpc/kvm/e500_mmu_host.c | 125 ++
arch/powerpc/kvm/e500mc.c | 2 +
9 files changed, 195 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index bb66d8b..7b6b2ec 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -433,6 +433,7 @@ struct kvm_vcpu_arch {
u32 eplc;
u32 epsc;
u32 oldpir;
+ u64 fault_lper;
#endif
#if defined(CONFIG_BOOKE)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index 9c89cdd..2730a29 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -86,6 +86,8 @@ extern gpa_t kvmppc_mmu_xlate(struct kvm_vcpu *vcpu, unsigned
int gtlb_index,
gva_t eaddr);
extern void kvmppc_mmu_dtlb_miss(struct kvm_vcpu *vcpu);
extern void kvmppc_mmu_itlb_miss(struct kvm_vcpu *vcpu);
+extern void kvmppc_lrat_map(struct kvm_vcpu *vcpu, gfn_t gfn);
+extern void kvmppc_lrat_invalidate(struct kvm_vcpu *vcpu);
extern struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm,
unsigned int id);
diff --git a/arch/powerpc/include/asm/mmu-book3e.h
b/arch/powerpc/include/asm/mmu-book3e.h
index 088fd9f..ac6acf7 100644
--- a/arch/powerpc/include/asm/mmu-book3e.h
+++ b/arch/powerpc/include/asm/mmu-book3e.h
@@ -40,6 +40,8 @@
/* MAS registers bit definitions */
+#define MAS0_ATSEL 0x8000
+#define MAS0_ATSEL_SHIFT 31
#define MAS0_TLBSEL_MASK0x3000
#define MAS0_TLBSEL_SHIFT 28
#define MAS0_TLBSEL(x) (((x) MAS0_TLBSEL_SHIFT) MAS0_TLBSEL_MASK)
@@ -53,6 +55,7 @@
#define MAS0_WQ_CLR_RSRV 0x2000
#define MAS1_VALID 0x8000
+#define MAS1_VALID_SHIFT 31
#define MAS1_IPROT 0x4000
#define MAS1_TID(x)(((x) 16) 0x3FFF)
#define MAS1_IND 0x2000
diff --git a/arch/powerpc/include/asm/reg_booke.h
b/arch/powerpc/include/asm/reg_booke.h
index 75bda23..783d617 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -43,6 +43,8 @@
/* Special Purpose Registers (SPRNs)*/
#define SPRN_DECAR 0x036 /* Decrementer Auto Reload Register */
+#define SPRN_LPER 0x038 /* Logical Page Exception Register */
+#define SPRN_LPERU 0x039 /* Logical Page Exception Register Upper */
#define SPRN_IVPR 0x03F /* Interrupt Vector Prefix Register */
#define SPRN_USPRG00x100 /* User Special Purpose Register General 0 */
#define SPRN_SPRG3R0x103 /* Special Purpose Register General 3 Read */
@@ -358,6 +360,9 @@
#define ESR_ILK0x0010 /* Instr. Cache Locking */
#define ESR_PUO0x0004 /* Unimplemented Operation
exception */
#define ESR_BO 0x0002 /* Byte Ordering */
+#define ESR_DATA 0x0400 /* Page Table Data Access */
+#define ESR_TLBI 0x0200 /* Page Table TLB Ineligible */
+#define ESR_PT 0x0100 /* Page Table Translation */
#define ESR_SPV0x0080 /* Signal Processing operation
*/
/* Bit definitions related to the DBCR0. */
@@ -649,6 +654,14 @@
#define EPC_EPID 0x3fff
#define EPC_EPID_SHIFT 0
+/* Bit definitions for LPER */
+#define LPER_ALPN 0x000FF000ULL
+#define LPER_ALPN_SHIFT12
+#define LPER_WIMGE 0x0F80
+#define LPER_WIMGE_SHIFT 7
+#define LPER_LPS 0x000F
+#define LPER_LPS_SHIFT 0
+
/*
* The IBM-403 is an even more odd special case, as it is much
* older than the IBM-405 series. We put these down here incase someone
diff --git a/arch/powerpc/kernel/asm-offsets.c
b/arch/powerpc/kernel/asm-offsets.c
index f5995a9..be6e329 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -713,6 +713,7 @@ int main(void)
DEFINE(VCPU_HOST_MAS4, offsetof(struct kvm_vcpu, arch.host_mas4));
DEFINE(VCPU_HOST_MAS6, offsetof(struct kvm_vcpu, arch.host_mas6));
DEFINE(VCPU_EPLC, offsetof(struct kvm_vcpu, arch.eplc));
+ DEFINE(VCPU_FAULT_LPER, offsetof(struct kvm_vcpu, arch.fault_lper));
#endif
#ifdef CONFIG_KVM_EXIT_TIMING
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index a192975..ab1077f 100644
[RFC PATCH 4/4] KVM: PPC: e500mc: Advertise E.PT to support HTW guests
Enable E.PT for vcpus with MMU MAV 2.0 to support Hardware Page Tablewalk (HTW) in guests. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/e500_mmu.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c index b775e6a..1de0cd6 100644 --- a/arch/powerpc/kvm/e500_mmu.c +++ b/arch/powerpc/kvm/e500_mmu.c @@ -945,11 +945,7 @@ static int vcpu_mmu_init(struct kvm_vcpu *vcpu, vcpu-arch.tlbps[1] = mfspr(SPRN_TLB1PS); vcpu-arch.mmucfg = ~MMUCFG_LRAT; - - /* Guest mmu emulation currently doesn't handle E.PT */ - vcpu-arch.eptcfg = 0; - vcpu-arch.tlbcfg[0] = ~TLBnCFG_PT; - vcpu-arch.tlbcfg[1] = ~TLBnCFG_IND; + vcpu-arch.eptcfg = mfspr(SPRN_EPTCFG); } return 0; -- 1.7.11.7 -- 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
RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
-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. #define BOOKE_INTERRUPT_SPE_FP_ROUND 34 #define BOOKE_INTERRUPT_PERFORMANCE_MONITOR 35 #define BOOKE_INTERRUPT_DOORBELL 36 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index ab62109..3c86d9b 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -388,8 +388,8 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, case BOOKE_IRQPRIO_ITLB_MISS: case BOOKE_IRQPRIO_SYSCALL: case BOOKE_IRQPRIO_FP_UNAVAIL: - case BOOKE_IRQPRIO_SPE_UNAVAIL: - case BOOKE_IRQPRIO_SPE_FP_DATA: + case BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL: + case BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST: #ifdef CONFIG_KVM_E500V2 case ...SPE: #else case ..ALTIVEC: #endif -Mike -- 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
Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
On 03.07.14 17:25, mihai.cara...@freescale.com 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 Uh, mind to point me to an email where I said I like the approach? :) different defines with same values (we specifically mapped them to the hardware interrupt numbers). We already upstreamed the necessary changes Yes, I think that'd end up the most readable flow of things. in the kernel. Scott, please share your opinion here. I'm not going to be religious about it, but names like BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST are 1) too long 2) too ambiguous It just means the code gets harder to read. Any way we can take to simplify the code flow is a win IMHO. And if I don't even remotely have to consider SPE when reading an Altivec path, I think that's a good thing :). 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
RE: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 03, 2014 3:29 PM To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org Subject: Re: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness On 30.06.14 17:34, Mihai Caraman wrote: Increase FPU laziness by calling kvmppc_load_guest_fp() just before returning to guest instead of each sched in. Without this improvement an interrupt may also claim floting point corrupting guest state. How do you handle context switching with this patch applied? During most of the guest's lifetime we never exit kvmppc_vcpu_run(), so when the guest gets switched out all FPU state gets lost? No, we had this discussion in ver 1. The FP/VMX/VSX is implemented lazy in the kernel i.e. the unit state is not saved/restored until another thread that once claimed the unit is sched in. Since FP/VMX/VSX can be activated by the guest independent of the host, the vcpu thread is always using the unit (even if it did not claimed it once). Now, this patch optimize the sched in flow. Instead of checking on each vcpu sched in if the kernel unloaded unit's guest state for another competing host process we do this when we enter the guest. -Mike -- 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
RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 03, 2014 6:31 PM To: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm- p...@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 03.07.14 17:25, mihai.cara...@freescale.com 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 Uh, mind to point me to an email where I said I like the approach? :) You tacitly approved it in this thread ... I did not say you like it :) https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-July/108501.html -Mike -- 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
RE: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 03, 2014 3:32 PM To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org Subject: Re: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support On 30.06.14 17:34, Mihai Caraman wrote: Add KVM Book3E AltiVec support. KVM Book3E FPU support gracefully reuse host infrastructure so follow the same approach for AltiVec. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com Same comment here - I fail to see how we refetch Altivec state after a context switch. See previous comment. I also run my usual Altivec stress test consisting in a guest and host process running affine to a physical core an competing for the same unit's resources using different data sets. -Mike -- 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
RE: [PATCH 5/6 v2] KVM: PPC: Book3E: Add ONE_REG AltiVec support
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 03, 2014 3:34 PM To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org Cc: k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org Subject: Re: [PATCH 5/6 v2] KVM: PPC: Book3E: Add ONE_REG AltiVec support On 30.06.14 17:34, Mihai Caraman wrote: Add ONE_REG support for AltiVec on Book3E. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com Any chance we can handle these in generic code? I expected this request :) Can we let this for a second phase to have e6500 enabled first? Can you share with us a Book3S setup so I can validate the requested changes? I already fell anxious touching strange hardware specific Book3S code without running it. -Mike -- 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
RE: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst() function
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, July 03, 2014 4:37 PM
To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
Cc: k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst()
function
On 28.06.14 00:49, Mihai Caraman wrote:
In the context of replacing kvmppc_ld() function calls with a version
of
kvmppc_get_last_inst() which allow to fail, Alex Graf suggested this:
If we get EMULATE_AGAIN, we just have to make sure we go back into the
guest.
No need to inject an ISI into the guest - it'll do that all by itself.
With an error returning kvmppc_get_last_inst we can just use completely
get rid of kvmppc_read_inst() and only use kvmppc_get_last_inst()
instead.
As a intermediate step get rid of kvmppc_read_inst() and only use
kvmppc_ld()
instead.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
v4:
- new patch
arch/powerpc/kvm/book3s_pr.c | 85 ++-
-
1 file changed, 35 insertions(+), 50 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_pr.c
b/arch/powerpc/kvm/book3s_pr.c
index 15fd6c2..d247d88 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -665,42 +665,6 @@ static void kvmppc_giveup_fac(struct kvm_vcpu
*vcpu, ulong fac)
#endif
}
-static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
-{
- ulong srr0 = kvmppc_get_pc(vcpu);
- u32 last_inst = kvmppc_get_last_inst(vcpu);
- int ret;
-
- ret = kvmppc_ld(vcpu, srr0, sizeof(u32), last_inst, false);
- if (ret == -ENOENT) {
- ulong msr = kvmppc_get_msr(vcpu);
-
- msr = kvmppc_set_field(msr, 33, 33, 1);
- msr = kvmppc_set_field(msr, 34, 36, 0);
- msr = kvmppc_set_field(msr, 42, 47, 0);
- kvmppc_set_msr_fast(vcpu, msr);
- kvmppc_book3s_queue_irqprio(vcpu,
BOOK3S_INTERRUPT_INST_STORAGE);
- return EMULATE_AGAIN;
- }
-
- return EMULATE_DONE;
-}
-
-static int kvmppc_check_ext(struct kvm_vcpu *vcpu, unsigned int
exit_nr)
-{
-
- /* Need to do paired single emulation? */
- if (!(vcpu-arch.hflags BOOK3S_HFLAG_PAIRED_SINGLE))
- return EMULATE_DONE;
-
- /* Read out the instruction */
- if (kvmppc_read_inst(vcpu) == EMULATE_DONE)
- /* Need to emulate */
- return EMULATE_FAIL;
-
- return EMULATE_AGAIN;
-}
-
/* Handle external providers (FPU, Altivec, VSX) */
static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int
exit_nr,
ulong msr)
@@ -1101,31 +1065,51 @@ program_interrupt:
case BOOK3S_INTERRUPT_VSX:
{
int ext_msr = 0;
+ int emul;
+ ulong pc;
+ u32 last_inst;
- switch (exit_nr) {
- case BOOK3S_INTERRUPT_FP_UNAVAIL: ext_msr = MSR_FP; break;
- case BOOK3S_INTERRUPT_ALTIVEC:ext_msr = MSR_VEC; break;
- case BOOK3S_INTERRUPT_VSX:ext_msr = MSR_VSX; break;
- }
+ if (!(vcpu-arch.hflags BOOK3S_HFLAG_PAIRED_SINGLE)) {
Please make paired single emulation the unusual, if()'ed case, not the
normal exit path :).
Huh ... do you have more Book3s specific requests, it will be strange if
it will still work after all this blind changes :)
-Mike
--
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
Re: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst() function
Am 03.07.2014 um 18:18 schrieb mihai.cara...@freescale.com
mihai.cara...@freescale.com:
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, July 03, 2014 4:37 PM
To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
Cc: k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst()
function
On 28.06.14 00:49, Mihai Caraman wrote:
In the context of replacing kvmppc_ld() function calls with a version
of
kvmppc_get_last_inst() which allow to fail, Alex Graf suggested this:
If we get EMULATE_AGAIN, we just have to make sure we go back into the
guest.
No need to inject an ISI into the guest - it'll do that all by itself.
With an error returning kvmppc_get_last_inst we can just use completely
get rid of kvmppc_read_inst() and only use kvmppc_get_last_inst()
instead.
As a intermediate step get rid of kvmppc_read_inst() and only use
kvmppc_ld()
instead.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
v4:
- new patch
arch/powerpc/kvm/book3s_pr.c | 85 ++-
-
1 file changed, 35 insertions(+), 50 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_pr.c
b/arch/powerpc/kvm/book3s_pr.c
index 15fd6c2..d247d88 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -665,42 +665,6 @@ static void kvmppc_giveup_fac(struct kvm_vcpu
*vcpu, ulong fac)
#endif
}
-static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
-{
-ulong srr0 = kvmppc_get_pc(vcpu);
-u32 last_inst = kvmppc_get_last_inst(vcpu);
-int ret;
-
-ret = kvmppc_ld(vcpu, srr0, sizeof(u32), last_inst, false);
-if (ret == -ENOENT) {
-ulong msr = kvmppc_get_msr(vcpu);
-
-msr = kvmppc_set_field(msr, 33, 33, 1);
-msr = kvmppc_set_field(msr, 34, 36, 0);
-msr = kvmppc_set_field(msr, 42, 47, 0);
-kvmppc_set_msr_fast(vcpu, msr);
-kvmppc_book3s_queue_irqprio(vcpu,
BOOK3S_INTERRUPT_INST_STORAGE);
-return EMULATE_AGAIN;
-}
-
-return EMULATE_DONE;
-}
-
-static int kvmppc_check_ext(struct kvm_vcpu *vcpu, unsigned int
exit_nr)
-{
-
-/* Need to do paired single emulation? */
-if (!(vcpu-arch.hflags BOOK3S_HFLAG_PAIRED_SINGLE))
-return EMULATE_DONE;
-
-/* Read out the instruction */
-if (kvmppc_read_inst(vcpu) == EMULATE_DONE)
-/* Need to emulate */
-return EMULATE_FAIL;
-
-return EMULATE_AGAIN;
-}
-
/* Handle external providers (FPU, Altivec, VSX) */
static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int
exit_nr,
ulong msr)
@@ -1101,31 +1065,51 @@ program_interrupt:
case BOOK3S_INTERRUPT_VSX:
{
int ext_msr = 0;
+int emul;
+ulong pc;
+u32 last_inst;
-switch (exit_nr) {
-case BOOK3S_INTERRUPT_FP_UNAVAIL: ext_msr = MSR_FP; break;
-case BOOK3S_INTERRUPT_ALTIVEC: ext_msr = MSR_VEC; break;
-case BOOK3S_INTERRUPT_VSX:ext_msr = MSR_VSX; break;
-}
+if (!(vcpu-arch.hflags BOOK3S_HFLAG_PAIRED_SINGLE)) {
Please make paired single emulation the unusual, if()'ed case, not the
normal exit path :).
Huh ... do you have more Book3s specific requests, it will be strange if
it will still work after all this blind changes :)
Heh :).
All I'm saying is that rather than
if (no emulation) {
foo();
break;
)
ps_emulation();
break;
We should do
if (ps emulation) {
ps_emulation();
break;
}
foo();
break;
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
Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
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. #define BOOKE_INTERRUPT_SPE_FP_ROUND 34 #define BOOKE_INTERRUPT_PERFORMANCE_MONITOR 35 #define BOOKE_INTERRUPT_DOORBELL 36 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index ab62109..3c86d9b 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -388,8 +388,8 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, case BOOKE_IRQPRIO_ITLB_MISS: case BOOKE_IRQPRIO_SYSCALL: case BOOKE_IRQPRIO_FP_UNAVAIL: - case BOOKE_IRQPRIO_SPE_UNAVAIL: - case BOOKE_IRQPRIO_SPE_FP_DATA: + case BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL: + case BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST: #ifdef CONFIG_KVM_E500V2 case ...SPE: #else case ..ALTIVEC: #endif I don't think that's an improvement. -Scott -- 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
Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
On 04.07.14 00:15, 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. The nice thing here is that we use almost all of these numbers in switch() statements which give us automated duplicate checking - so we don't accidentally go into the wrong code path without knowing it. 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
Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
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). -Scott -- 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
Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
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. 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
Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
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. -Scott -- 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
Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
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
Re: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support
On Mon, 2014-06-30 at 18:34 +0300, Mihai Caraman wrote:
Add KVM Book3E AltiVec support. KVM Book3E FPU support gracefully reuse host
infrastructure so follow the same approach for AltiVec.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
v2:
- integrate Paul's FP/VMX/VSX changes
arch/powerpc/kvm/booke.c | 67
++--
1 file changed, 65 insertions(+), 2 deletions(-)
I had to apply the whole patchset to get proper context for reviewing
this, and found some nits:
case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
if (kvmppc_supports_spe() || kvmppc_supports_altivec()) {
kvmppc_booke_queue_irqprio(vcpu,
BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST);
r = RESUME_GUEST;
} else {
/*
* These really should never happen without
CONFIG_SPE,
* as we should never enable the real MSR[SPE] in the
* guest.
*/
Besides the comment not being updated for Altivec, it's not true on HV,
where the guest can enable MSR[VEC] all by itself. For HV, the reason
we shouldn't be able to get here is that we disable KVM on e6500 if
CONFIG_ALTIVEC is not enabled, and no other HV core supports either SPE
or Altivec.
pr_crit(%s: unexpected SPE interrupt %u at %08lx\n,
__func__, exit_nr, vcpu-arch.pc);
Error string will say SPE regardless of what sort of chip you're on.
Given that this is explicitly on the no support for Altivec or SPE
path, SPE/Altivec phrasing seems appropriate. Of course we have
bigger problems than that if we ever reach this code. :-)
-Scott
--
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
