RE: [PATCH 1/2] KVM: PPC: e500mc: Add support for single threaded vcpus on e6500 core
I abandon this patch, I will send a v2 with a minor fix for 85xx.
Mike
-Original Message-
From: Mihai Caraman [mailto:mihai.cara...@freescale.com]
Sent: Friday, August 29, 2014 8:04 PM
To: kvm-ppc@vger.kernel.org
Cc: k...@vger.kernel.org; Caraman Mihai Claudiu-B02008
Subject: [PATCH 1/2] KVM: PPC: e500mc: Add support for single threaded
vcpus on e6500 core
ePAPR represents hardware threads as cpu node properties in device tree.
So with existing QEMU, hardware threads are simply exposed as vcpus with
one hardware thread.
The e6500 core shares TLBs between hardware threads. Without tlb write
conditional instruction, the Linux kernel uses per core mechanisms to
protect against duplicate TLB entries.
The guest is unable to detect real siblings threads, so it can't use a
TLB protection mechanism. An alternative solution is to use the
hypervisor
to allocate different lpids to guest's vcpus running simultaneous on real
siblings threads. On systems with two threads per core this patch halves
the size of the lpid pool that the allocator sees and use two lpids per
VM.
Use even numbers to speedup vcpu lpid computation with consecutive lpids
per VM: vm1 will use lpids 2 and 3, vm2 lpids 4 and 5, and so on.
Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
arch/powerpc/include/asm/kvm_booke.h | 5 +++-
arch/powerpc/kvm/e500.h | 20
arch/powerpc/kvm/e500_mmu_host.c | 16 ++---
arch/powerpc/kvm/e500mc.c| 46 ++
--
4 files changed, 64 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_booke.h
b/arch/powerpc/include/asm/kvm_booke.h
index f7aa5cc..630134d 100644
--- a/arch/powerpc/include/asm/kvm_booke.h
+++ b/arch/powerpc/include/asm/kvm_booke.h
@@ -23,7 +23,10 @@
#include linux/types.h
#include linux/kvm_host.h
-/* LPIDs we support with this build -- runtime limit may be lower */
+/*
+ * Number of available lpids. Only the low-order 6 bits of LPID rgister
are
+ * implemented on e500mc+ cores.
+ */
#define KVMPPC_NR_LPIDS64
#define KVMPPC_INST_EHPRIV 0x7c00021c
diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
index a326178..7b74453 100644
--- a/arch/powerpc/kvm/e500.h
+++ b/arch/powerpc/kvm/e500.h
@@ -22,6 +22,7 @@
#include linux/kvm_host.h
#include asm/mmu-book3e.h
#include asm/tlb.h
+#include asm/cputhreads.h
enum vcpu_ftr {
VCPU_FTR_MMU_V2
@@ -289,6 +290,25 @@ void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500
*vcpu_e500);
#define kvmppc_e500_get_tlb_stid(vcpu, gtlbe) get_tlb_tid(gtlbe)
#define get_tlbmiss_tid(vcpu) get_cur_pid(vcpu)
#define get_tlb_sts(gtlbe) (gtlbe-mas1 MAS1_TS)
+
+/*
+ * This functios should be called with preemtion disabled
+ * and the returned value is valid only in that context
+ */
+static inline int get_thread_specific_lpid(int vm_lpid)
+{
+ int vcpu_lpid = vm_lpid;
+
+ if (threads_per_core == 2)
+ vcpu_lpid |= smp_processor_id() 1;
+
+ return vcpu_lpid;
+}
+
+static inline int get_lpid(struct kvm_vcpu *vcpu)
+{
+ return get_thread_specific_lpid(vcpu-kvm-arch.lpid);
+}
#else
unsigned int kvmppc_e500_get_tlb_stid(struct kvm_vcpu *vcpu,
struct kvm_book3e_206_tlb_entry *gtlbe);
diff --git a/arch/powerpc/kvm/e500_mmu_host.c
b/arch/powerpc/kvm/e500_mmu_host.c
index 08f14bb..5759608 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -69,7 +69,8 @@ static inline u32 e500_shadow_mas3_attrib(u32 mas3, int
usermode)
* writing shadow tlb entry to host TLB
*/
static inline void __write_host_tlbe(struct kvm_book3e_206_tlb_entry
*stlbe,
- uint32_t mas0)
+ uint32_t mas0,
+ uint32_t lpid)
{
unsigned long flags;
@@ -80,7 +81,7 @@ static inline void __write_host_tlbe(struct
kvm_book3e_206_tlb_entry *stlbe,
mtspr(SPRN_MAS3, (u32)stlbe-mas7_3);
mtspr(SPRN_MAS7, (u32)(stlbe-mas7_3 32));
#ifdef CONFIG_KVM_BOOKE_HV
- mtspr(SPRN_MAS8, stlbe-mas8);
+ mtspr(SPRN_MAS8, MAS8_TGS | get_thread_specific_lpid(lpid));
#endif
asm volatile(isync; tlbwe : : : memory);
@@ -129,11 +130,12 @@ static inline void write_host_tlbe(struct
kvmppc_vcpu_e500 *vcpu_e500,
if (tlbsel == 0) {
mas0 = get_host_mas0(stlbe-mas2);
- __write_host_tlbe(stlbe, mas0);
+ __write_host_tlbe(stlbe, mas0, vcpu_e500-vcpu.kvm-
arch.lpid);
} else {
__write_host_tlbe(stlbe,
MAS0_TLBSEL(1) |
- MAS0_ESEL(to_htlb1_esel(sesel)));
+ MAS0_ESEL(to_htlb1_esel(sesel)),
+ vcpu_e500
RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
-Original Message-
From: Wood Scott-B07421
Sent: Saturday, July 26, 2014 3:11 AM
To: Caraman Mihai Claudiu-B02008
Cc: Alexander Graf; kvm-ppc@vger.kernel.org; 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 Thu, 2014-07-24 at 04:16 -0500, Caraman Mihai Claudiu-B02008 wrote:
Scott, Alex's request to define SPE handlers only for e500v2 implies
changes
in 32-bit FSL kernel to have exclusive configurations for e200/e500v2
and
e500mc/e5500. We would probably need something like this, what's your
take on it?
That is already a compile-time decision.
Yes, but is not fully implemented.
diff --git a/arch/powerpc/kernel/head_fsl_booke.S
b/arch/powerpc/kernel/head_fsl_booke.S
index b497188..9d41015 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -613,6 +613,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
mfspr r10, SPRN_SPRG_RSCRATCH0
b InstructionStorage
+/* Define SPE handlers only for e500v2 and e200 */
+#ifndef CONFIG_PPC_E500MC
#ifdef CONFIG_SPE
/* SPE Unavailable */
START_EXCEPTION(SPEUnavailable)
@@ -626,7 +628,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
EXCEPTION(0x2020, SPE_ALTIVEC_UNAVAIL, SPEUnavailable, \
unknown_exception, EXC_XFER_EE)
#endif /* CONFIG_SPE */
+#endif
This is redundant:
config SPE
bool SPE Support
depends on E200 || (E500 !PPC_E500MC)
default y
I see what you mean, but it's not redundant. Alex was looking to remove SPE
handlers for e500mc+ and the proposal handled !SPE case. In the new
light I find this variant more readable:
+/* Define SPE handlers for e200 and e500v2 */
#ifdef CONFIG_SPE
/* SPE Unavailable */
START_EXCEPTION(SPEUnavailable)
@@ -622,11 +623,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
b fast_exception_return
1: addir3,r1,STACK_FRAME_OVERHEAD
EXC_XFER_EE_LITE(0x2010, KernelSPE)
-#else
+#elif defined(CONFIG_E200) || \
+ (defined(CONFIG_E500) !defined(CONFIG_PPC_E500MC))
EXCEPTION(0x2020, SPE_ALTIVEC_UNAVAIL, SPEUnavailable, \
unknown_exception, EXC_XFER_EE)
#endif /* CONFIG_SPE */
diff --git a/arch/powerpc/kernel/cputable.c
b/arch/powerpc/kernel/cputable.c
index c1faade..3ab65c2 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -2030,6 +2030,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
#endif /* CONFIG_PPC32 */
#ifdef CONFIG_E500
#ifdef CONFIG_PPC32
+#ifndef CONFIG_PPC_E500MC
{ /* e500 */
.pvr_mask = 0x,
.pvr_value = 0x8020,
@@ -2069,6 +2070,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check = machine_check_e500,
.platform = ppc8548,
},
+#endif /* !CONFIG_PPC_E500MC */
{ /* e500mc */
.pvr_mask = 0x,
.pvr_value = 0x8023,
This looks a bit strange -- e500v2 gets excluded if PPC_E500MC, but
e500mc gets included in !PPC_E500MC?
Right, my main purpose was to get rid of __setup_e500_ivors on PPC_E500MC
which refers SPEUnavailable. I will add an #else to exclude e500mc.
The generic E500 PPC default cpu advertises PPC_FEATURE_HAS_SPE_COMP.
Do we want to excluded it if PPC_E500MC? Is this cpu useful without cpu_setup()
and if so why don't we also have a default for 64-bit?
-Mike
RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-
ow...@vger.kernel.org] On Behalf Of mihai.cara...@freescale.com
Sent: Monday, July 21, 2014 4:23 PM
To: Alexander Graf; Wood Scott-B07421
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-
d...@lists.ozlabs.org
Subject: 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.
#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
case BOOKE_IRQPRIO_SPE_FP_ROUND:
case BOOKE_IRQPRIO_AP_UNAVAIL:
allowed = 1;
@@ -977,18 +977,19 @@ int kvmppc_handle_exit(struct kvm_run *run,
struct kvm_vcpu *vcpu,
break;
#ifdef CONFIG_SPE
- case BOOKE_INTERRUPT_SPE_UNAVAIL: {
+ case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL: {
if (vcpu-arch.shared-msr MSR_SPE)
kvmppc_vcpu_enable_spe(vcpu);
else
kvmppc_booke_queue_irqprio(vcpu,
-BOOKE_IRQPRIO_SPE_UNAVAIL);
+ BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL);
r = RESUME_GUEST;
break;
}
- case BOOKE_INTERRUPT_SPE_FP_DATA:
- kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_DATA);
+ case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
+ kvmppc_booke_queue_irqprio(vcpu,
+ BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST);
r = RESUME_GUEST;
break;
@@ -997,7 +998,7 @@ int kvmppc_handle_exit(struct kvm_run *run,
struct
kvm_vcpu *vcpu,
r = RESUME_GUEST;
break;
#else
- case BOOKE_INTERRUPT_SPE_UNAVAIL:
+ case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL:
/*
* Guest wants SPE, but host kernel doesn't support it.
Send
* an unimplemented operation program check to the
guest.
@@ -1010,7 +1011,7 @@ int kvmppc_handle_exit(struct kvm_run *run,
struct kvm_vcpu *vcpu,
* These really should never happen without CONFIG_SPE,
* as we should never enable the real MSR[SPE] in the guest.
*/
- case
RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-
ow...@vger.kernel.org] On Behalf Of Alexander Graf
Sent: Wednesday, July 23, 2014 12:21 AM
To: Caraman Mihai Claudiu-B02008
Cc: kvm-ppc@vger.kernel.org; linuxppc-...@lists.ozlabs.org;
k...@vger.kernel.org
Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
On 21.07.14 11:59, mihai.cara...@freescale.com wrote:
-Original Message-
From: Linuxppc-dev [mailto:linuxppc-dev-
bounces+mihai.caraman=freescale@lists.ozlabs.org] On Behalf Of
mihai.cara...@freescale.com
Sent: Friday, July 18, 2014 12:06 PM
To: Alexander Graf; kvm-ppc@vger.kernel.org
Cc: linuxppc-...@lists.ozlabs.org; k...@vger.kernel.org
Subject: RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to
fail
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, July 17, 2014 5:21 PM
To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
Cc: k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to
fail
On 17.07.14 13:22, 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
---
...
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index e2fd5a1..7f9c634 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);
@@ -234,6 +242,23 @@ struct kvmppc_ops {
extern struct kvmppc_ops *kvmppc_hv_ops;
extern struct kvmppc_ops *kvmppc_pr_ops;
+static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
+enum instruction_type type, u32
*inst)
+{
+int ret = EMULATE_DONE;
+
+/* Load the instruction manually if it failed to do so in the
+ * exit path */
+if (vcpu-arch.last_inst == KVM_INST_FETCH_FAILED)
+ret = kvmppc_load_last_inst(vcpu, type, vcpu-
arch.last_inst);
+
+
+*inst = (ret == EMULATE_DONE kvmppc_need_byteswap(vcpu)) ?
+swab32(vcpu-arch.last_inst) : vcpu-arch.last_inst;
This makes even less sense than the previous version. Either you
treat
inst as definitely overwritten or as preserves previous data on
failure.
Both v4 and v5 versions treat inst as definitely overwritten.
So either you unconditionally swap like you did before
If we make abstraction of its symmetry, KVM_INST_FETCH_FAILED is
operated
in host endianness, so it doesn't need byte swap.
I agree with your reasoning if last_inst is initialized and compared
with
data in guest endianess, which is not the case yet for
KVM_INST_FETCH_FAILED.
Alex, are you relying on the fact that KVM_INST_FETCH_FAILED value is
symmetrical?
With a non symmetrical value like 0xDEADBEEF, and considering a little-
endian guest
on a big-endian host, we need to fix kvm logic to initialize and
compare last_inst
with 0xEFBEADDE swaped value.
Your suggestion to unconditionally swap makes sense only with the above
fix, otherwise
inst may end up with 0xEFBEADDE swaped value with is wrong.
Only for *inst which we would treat as undefined after the function
returned EMULATE_AGAIN.
Right. With this do you acknowledge that v5
RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
Right. With this do you acknowledge that v5 (definitely overwritten approach) is ok? I think I'm starting to understand your logic of v5. You write fetch_failed into *inst unswapped if the fetch failed. v5 - don't swap when load fails :) I think that's ok, but I definitely do not like the code flow - it's too hard to understand at a glimpse. Just rewrite it to swab at local variable level, preferably with if()s and comments what this is about and have a single unconditional *inst = fetched_inst; at the end of the function. I will incorporate these change requests into v6. Thanks, -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 v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
-Original Message-
From: Linuxppc-dev [mailto:linuxppc-dev-
bounces+mihai.caraman=freescale@lists.ozlabs.org] On Behalf Of
mihai.cara...@freescale.com
Sent: Friday, July 18, 2014 12:06 PM
To: Alexander Graf; kvm-ppc@vger.kernel.org
Cc: linuxppc-...@lists.ozlabs.org; k...@vger.kernel.org
Subject: RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, July 17, 2014 5:21 PM
To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
Cc: k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to
fail
On 17.07.14 13:22, 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
---
...
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index e2fd5a1..7f9c634 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);
@@ -234,6 +242,23 @@ struct kvmppc_ops {
extern struct kvmppc_ops *kvmppc_hv_ops;
extern struct kvmppc_ops *kvmppc_pr_ops;
+static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
+ enum instruction_type type, u32 *inst)
+{
+ int ret = EMULATE_DONE;
+
+ /* Load the instruction manually if it failed to do so in the
+ * exit path */
+ if (vcpu-arch.last_inst == KVM_INST_FETCH_FAILED)
+ ret = kvmppc_load_last_inst(vcpu, type, vcpu-
arch.last_inst);
+
+
+ *inst = (ret == EMULATE_DONE kvmppc_need_byteswap(vcpu)) ?
+ swab32(vcpu-arch.last_inst) : vcpu-arch.last_inst;
This makes even less sense than the previous version. Either you treat
inst as definitely overwritten or as preserves previous data on
failure.
Both v4 and v5 versions treat inst as definitely overwritten.
So either you unconditionally swap like you did before
If we make abstraction of its symmetry, KVM_INST_FETCH_FAILED is operated
in host endianness, so it doesn't need byte swap.
I agree with your reasoning if last_inst is initialized and compared with
data in guest endianess, which is not the case yet for
KVM_INST_FETCH_FAILED.
Alex, are you relying on the fact that KVM_INST_FETCH_FAILED value is
symmetrical?
With a non symmetrical value like 0xDEADBEEF, and considering a little-endian
guest
on a big-endian host, we need to fix kvm logic to initialize and compare
last_inst
with 0xEFBEADDE swaped value.
Your suggestion to unconditionally swap makes sense only with the above fix,
otherwise
inst may end up with 0xEFBEADDE swaped value with is wrong.
-Mike
N�r��yb�X��ǧv�^�){.n�+jir)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
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.
#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
case BOOKE_IRQPRIO_SPE_FP_ROUND:
case BOOKE_IRQPRIO_AP_UNAVAIL:
allowed = 1;
@@ -977,18 +977,19 @@ int kvmppc_handle_exit(struct kvm_run *run,
struct kvm_vcpu *vcpu,
break;
#ifdef CONFIG_SPE
- case BOOKE_INTERRUPT_SPE_UNAVAIL: {
+ case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL: {
if (vcpu-arch.shared-msr MSR_SPE)
kvmppc_vcpu_enable_spe(vcpu);
else
kvmppc_booke_queue_irqprio(vcpu,
- BOOKE_IRQPRIO_SPE_UNAVAIL);
+ BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL);
r = RESUME_GUEST;
break;
}
- case BOOKE_INTERRUPT_SPE_FP_DATA:
- kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_DATA);
+ case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
+ kvmppc_booke_queue_irqprio(vcpu,
+ BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST);
r = RESUME_GUEST;
break;
@@ -997,7 +998,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
kvm_vcpu *vcpu,
r = RESUME_GUEST;
break;
#else
- case BOOKE_INTERRUPT_SPE_UNAVAIL:
+ case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL:
/*
* Guest wants SPE, but host kernel doesn't support it. Send
* an unimplemented operation program check to the guest.
@@ -1010,7 +1011,7 @@ int kvmppc_handle_exit(struct kvm_run *run,
struct kvm_vcpu *vcpu,
* These really should never happen without CONFIG_SPE,
* as we should never enable the real MSR[SPE] in the guest.
*/
- case BOOKE_INTERRUPT_SPE_FP_DATA:
+ case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
case BOOKE_INTERRUPT_SPE_FP_ROUND:
printk(KERN_CRIT %s: unexpected SPE interrupt %u at
%08lx\n,
__func__, exit_nr, vcpu-arch.pc);
diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h
index b632cd3..f182b32 100644
--- a/arch/powerpc/kvm/booke.h
+++ b/arch/powerpc/kvm/booke.h
@@ -32,8 +32,8 @@
#define BOOKE_IRQPRIO_ALIGNMENT 2
#define BOOKE_IRQPRIO_PROGRAM 3
#define BOOKE_IRQPRIO_FP_UNAVAIL 4
-#define BOOKE_IRQPRIO_SPE_UNAVAIL 5
-#define BOOKE_IRQPRIO_SPE_FP_DATA 6
+#define
RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Thursday, July 17, 2014 5:21 PM
To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
Cc: k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org
Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
On 17.07.14 13:22, 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
---
...
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index e2fd5a1..7f9c634 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);
@@ -234,6 +242,23 @@ struct kvmppc_ops {
extern struct kvmppc_ops *kvmppc_hv_ops;
extern struct kvmppc_ops *kvmppc_pr_ops;
+static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
+ enum instruction_type type, u32 *inst)
+{
+ int ret = EMULATE_DONE;
+
+ /* Load the instruction manually if it failed to do so in the
+* exit path */
+ if (vcpu-arch.last_inst == KVM_INST_FETCH_FAILED)
+ ret = kvmppc_load_last_inst(vcpu, type, vcpu-
arch.last_inst);
+
+
+ *inst = (ret == EMULATE_DONE kvmppc_need_byteswap(vcpu)) ?
+ swab32(vcpu-arch.last_inst) : vcpu-arch.last_inst;
This makes even less sense than the previous version. Either you treat
inst as definitely overwritten or as preserves previous data on
failure.
Both v4 and v5 versions treat inst as definitely overwritten.
So either you unconditionally swap like you did before
If we make abstraction of its symmetry, KVM_INST_FETCH_FAILED is operated
in host endianness, so it doesn't need byte swap.
I agree with your reasoning if last_inst is initialized and compared with
data in guest endianess, which is not the case yet for KVM_INST_FETCH_FAILED.
or you do
if (ret == EMULATE_DONE)
*inst = kvmppc_need_byteswap(vcpu) ? swab32(vcpu-arch.last_inst) :
vcpu-arch.last_inst;
-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 6/6 v2] kvm: ppc: Add SPRN_EPR get helper function
-Original Message-
From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-
ow...@vger.kernel.org] On Behalf Of Bharat Bhushan
Sent: Thursday, July 17, 2014 2:32 PM
To: ag...@suse.de; kvm-ppc@vger.kernel.org
Cc: k...@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248; Bhushan
Bharat-R65777
Subject: [PATCH 6/6 v2] kvm: ppc: Add SPRN_EPR get helper function
kvmppc_set_epr() is already defined in asm/kvm_ppc.h, So
rename and move get_epr helper function to same file.
Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v1-v2
- vcpu-arch.epr under CONFIG_BOOKE
arch/powerpc/include/asm/kvm_ppc.h | 10 ++
arch/powerpc/kvm/booke.c | 11 +--
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index 58a5202..14e2d87 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -395,6 +395,16 @@ static inline int kvmppc_xics_hcall(struct kvm_vcpu
*vcpu, u32 cmd)
{ return 0; }
#endif
+static inline unsigned long kvmppc_get_epr(struct kvm_vcpu *vcpu)
+{
+#ifdef CONFIG_KVM_BOOKE_HV
+ return mfspr(SPRN_GEPR);
+#elif defined(CONFIG_BOOKE)
+ return vcpu-arch.epr;
+#endif
+ return 0;
+}
+
static inline void kvmppc_set_epr(struct kvm_vcpu *vcpu, u32 epr)
{
#ifdef CONFIG_KVM_BOOKE_HV
EPR is a BookE resource, why don't we move the helpers to kvm_booke.h?
-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 6/6] kvm: ppc: Add SPRN_SPR get helper function
kvmppc_set_epr() is already defined in asm/kvm_ppc.h, So rename and move get_epr helper function to same file. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/include/asm/kvm_ppc.h | 9 + arch/powerpc/kvm/booke.c | 11 +-- 2 files changed, 10 insertions(+), 10 deletions(-) This patch which is already applied on kvm-ppc-queue breaks book3s. I get these errors with g5_defconfig: In file included from arch/powerpc/kernel/asm-offsets.c:57:0: ./arch/powerpc/include/asm/kvm_ppc.h: In function 'kvmppc_get_epr': ./arch/powerpc/include/asm/kvm_ppc.h:400:19: error: 'struct kvm_vcpu_arch' has no member named 'epr' ./arch/powerpc/include/asm/kvm_ppc.h: In function 'kvmppc_get_sprg0': ./arch/powerpc/include/asm/kvm_ppc.h:522:1: error: 'SPRN_GSPRG0' undeclared (first use in this function) ./arch/powerpc/include/asm/kvm_ppc.h:522:1: note: each undeclared identifier is reported only once for each function it appears in ... -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 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 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] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Tuesday, June 17, 2014 12:09 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org Subject: Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule On 13.06.14 21:42, Scott Wood wrote: On Fri, 2014-06-13 at 16:55 +0200, Alexander Graf wrote: On 13.06.14 16:43, mihai.cara...@freescale.com wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, June 12, 2014 8:05 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; Wood Scott-B07421 Subject: Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule On 06/12/2014 04:00 PM, Mihai Caraman wrote: On vcpu schedule, the condition checked for tlb pollution is too tight. The tlb entries of one vcpu are polluted when a different vcpu from the same partition runs in-between. Relax the current tlb invalidation condition taking into account the lpid. Can you quantify the performance improvement from this? We've had bugs in this area before, so let's make sure it's worth it before making this more complicated. Signed-off-by: Mihai Caraman mihai.caraman at freescale.com Your mailer is broken? :) This really should be an @. I think this should work. Scott, please ack. Alex, you were right. I screwed up the patch description by inverting relax and tight terms :) It should have been more like this: KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule On vcpu schedule, the condition checked for tlb pollution is too loose. The tlb entries of a vcpu are polluted (vs stale) only when a different vcpu within the same logical partition runs in-between. Optimize the tlb invalidation condition taking into account the lpid. Can't we give every vcpu its own lpid? Or don't we trap on global invalidates? That would significantly increase the odds of exhausting LPIDs, especially on large chips like t4240 with similarly large VMs. If we were to do that, the LPIDs would need to be dynamically assigned (like PIDs), and should probably be a separate numberspace per physical core. True, I didn't realize we only have so few of them. It would however save us from most flushing as long as we have spare LPIDs available :). Yes, we had this proposal on the table for e6500 multithreaded core. This core lacks tlb write conditional instruction, so an OS needs to use locks to protect itself against concurrent tlb writes executed from sibling threads. When we expose hw treads as single-threaded vcpus (useful when the user opt not to pin vcpus), the guest can't no longer protect itself optimally (it can protect tlb writes across all threads but this is not acceptable). So instead, we found a solution at hypervisor level by assigning different logical partition ids to guest's vcpus running simultaneous on sibling hw threads. Currently in FSL SDK we allocate two lpids to each guest. I am also a proponent for using all LPID space (63 values) per (multi-threaded) physical core, which will lead to fewer invalidates on vcpu schedule and will accommodate the solution described above. -Mike
RE: [PATCH v2] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, June 17, 2014 6:36 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH v2] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule On Tue, 2014-06-17 at 12:02 +0300, Mihai Caraman wrote: On vcpu schedule, the condition checked for tlb pollution is too loose. The tlb entries of a vcpu become polluted (vs stale) only when a different vcpu within the same logical partition runs in-between. Optimize the tlb invalidation condition taking into account the logical partition id. With the new invalidation condition, a guest shows 4% performance improvement on P5020DS while running a memory stress application with the cpu oversubscribed, the other guest running a cpu intensive workload. See https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-June/118547.html Thanks. The original code needs just a simple adjustment to benefit from this optimization, please review v3. - Mike
RE: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
-static DEFINE_PER_CPU(struct kvm_vcpu *, last_vcpu_on_cpu); +static DEFINE_PER_CPU(struct kvm_vcpu * [KVMPPC_NR_LPIDS], last_vcpu_on_cpu); Hmm, I didn't know you could express types like that. Is this special syntax that only works for typeof? Yes, AFAIK. No space after * Checkpatch complains about the missing space ;) Name should be adjusted to match, something like last_vcpu_of_lpid (with the _on_cpu being implied by the fact that it's PER_CPU). I was thinking to the long name but it was not appealing, I will change it to last_vcpu_of_lpid. -Mike
RE: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
-Original Message-
From: Wood Scott-B07421
Sent: Tuesday, June 17, 2014 10:48 PM
To: Caraman Mihai Claudiu-B02008
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-
d...@lists.ozlabs.org
Subject: Re: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation
condition on vcpu schedule
On Tue, 2014-06-17 at 14:42 -0500, Caraman Mihai Claudiu-B02008 wrote:
-static DEFINE_PER_CPU(struct kvm_vcpu *, last_vcpu_on_cpu);
+static DEFINE_PER_CPU(struct kvm_vcpu * [KVMPPC_NR_LPIDS],
last_vcpu_on_cpu);
Hmm, I didn't know you could express types like that. Is this
special
syntax that only works for typeof?
Yes, AFAIK.
No space after *
Checkpatch complains about the missing space ;)
Checkpatch is wrong, which isn't surprising given that this is unusual
syntax. We don't normally put a space after * when used to represent a
pointer.
This is not something new. See [PATCH 04/10] percpu: cleanup percpu array
definitions:
https://lkml.org/lkml/2009/6/24/26
-Mike
N�r��yb�X��ǧv�^�){.n�+jir)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
RE: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, June 17, 2014 11:05 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule On Tue, 2014-06-17 at 15:02 -0500, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, June 17, 2014 10:48 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule On Tue, 2014-06-17 at 14:42 -0500, Caraman Mihai Claudiu-B02008 wrote: -static DEFINE_PER_CPU(struct kvm_vcpu *, last_vcpu_on_cpu); +static DEFINE_PER_CPU(struct kvm_vcpu * [KVMPPC_NR_LPIDS], last_vcpu_on_cpu); Hmm, I didn't know you could express types like that. Is this special syntax that only works for typeof? Yes, AFAIK. No space after * Checkpatch complains about the missing space ;) Checkpatch is wrong, which isn't surprising given that this is unusual syntax. We don't normally put a space after * when used to represent a pointer. This is not something new. See [PATCH 04/10] percpu: cleanup percpu array definitions: https://lkml.org/lkml/2009/6/24/26 I didn't say it was new, just unusual, and checkpatch doesn't recognize it. Checkpatch shouldn't be blindly and silently obeyed when it says something strange. I agree with you about the syntax and I know other cases where checkpatch is a moron. For similar corner cases checkpatch maintainers did not wanted (or found it difficult) to make an exception. I would also like to see Alex opinion on this. -Mike
RE: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, June 12, 2014 8:05 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; Wood Scott-B07421 Subject: Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule On 06/12/2014 04:00 PM, Mihai Caraman wrote: On vcpu schedule, the condition checked for tlb pollution is too tight. The tlb entries of one vcpu are polluted when a different vcpu from the same partition runs in-between. Relax the current tlb invalidation condition taking into account the lpid. Signed-off-by: Mihai Caraman mihai.caraman at freescale.com Your mailer is broken? :) This really should be an @. I think this should work. Scott, please ack. Alex, you were right. I screwed up the patch description by inverting relax and tight terms :) It should have been more like this: KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule On vcpu schedule, the condition checked for tlb pollution is too loose. The tlb entries of a vcpu are polluted (vs stale) only when a different vcpu within the same logical partition runs in-between. Optimize the tlb invalidation condition taking into account the lpid. -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 v2 1/4] KVM: PPC: e500mc: Revert add load inst fixup
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Sunday, May 04, 2014 1:14 AM To: Caraman Mihai Claudiu-B02008 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH v2 1/4] KVM: PPC: e500mc: Revert add load inst fixup Am 03.05.2014 um 01:14 schrieb mihai.cara...@freescale.com mihai.cara...@freescale.com: From: Alexander Graf ag...@suse.de Sent: Friday, May 2, 2014 12:24 PM This was the first idea that sprang to my mind inspired from how DO_KVM is hooked on PR. I actually did a simple POC for e500mc/e5500, but this will not work on e6500 which has shared IVORs between HW threads. What if we combine the ideas? On read we flip the IVOR to a separate handler that checks for a field in the PACA. Only if that field is set, we treat the fault as kvm fault, otherwise we jump into the normal handler. I suppose we'd have to also take a lock to make sure we don't race with the other thread when it wants to also read a guest instruction, but you get the idea. This might be a solution for TLB eviction but not for execute-but-not-read entries which requires access from host context. I have no idea whether this would be any faster, it's more of a brainstorming thing really. But regardless this patch set would be a move into the right direction. Btw, do we have any guarantees that we don't get scheduled away before we run kvmppc_get_last_inst()? If we run on a different core we can't read the inst anymore. Hrm. It was your suggestion to move the logic from kvmppc_handle_exit() irq disabled area to kvmppc_get_last_inst(): http://git.freescale.com/git/cgit.cgi/ppc/sdk/linux.git/tree/arch/powerpc/kvm/booke.c Still, what is wrong if we get scheduled on another core? We will emulate again and the guest will populate the TLB on the new core. -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 v2 3/4] KVM: PPC: Alow kvmppc_get_last_inst() to fail
-Original Message-
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Friday, May 02, 2014 12:55 PM
To: Caraman Mihai Claudiu-B02008
Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-
d...@lists.ozlabs.org
Subject: Re: [PATCH v2 3/4] KVM: PPC: Alow kvmppc_get_last_inst() to fail
On 05/01/2014 02:45 AM, Mihai Caraman wrote:
...
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index 4096f16..6e7c358 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -72,6 +72,8 @@ extern int kvmppc_sanity_check(struct kvm_vcpu
*vcpu);
extern int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu);
extern void kvmppc_subarch_vcpu_uninit(struct kvm_vcpu *vcpu);
+extern int kvmppc_get_last_inst(struct kvm_vcpu *vcpu, u32 *inst);
Phew. Moving this into a separate function sure has some performance
implications. Was there no way to keep it in a header?
You could just move it into its own .h file which we include after
kvm_ppc.h. That way everything's available. That would also help me a
lot with the little endian port where I'm also struggling with header
file inclusion order and kvmppc_need_byteswap().
Great, I will do this.
diff --git a/arch/powerpc/kvm/book3s_pr.c
b/arch/powerpc/kvm/book3s_pr.c
index c5c052a..b7fffd1 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -608,12 +608,9 @@ void kvmppc_giveup_ext(struct kvm_vcpu *vcpu,
ulong msr)
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;
+ u32 last_inst;
- ret = kvmppc_ld(vcpu, srr0, sizeof(u32), last_inst, false);
- if (ret == -ENOENT) {
+ if (kvmppc_get_last_inst(vcpu, last_inst) == -ENOENT) {
ENOENT?
You have to tell us :) Why does kvmppc_ld() mix emulation_result
enumeration with generic errors? Do you want to change that and
use EMULATE_FAIL instead?
ulong msr = vcpu-arch.shared-msr;
msr = kvmppc_set_field(msr, 33, 33, 1);
@@ -867,15 +864,18 @@ int kvmppc_handle_exit_pr(struct kvm_run *run,
struct kvm_vcpu *vcpu,
{
enum emulation_result er;
ulong flags;
+ u32 last_inst;
program_interrupt:
flags = vcpu-arch.shadow_srr1 0x1full;
+ kvmppc_get_last_inst(vcpu, last_inst);
No check for the return value?
Should we queue a program exception and resume guest?
if (vcpu-arch.shared-msr MSR_PR) {
#ifdef EXIT_DEBUG
- printk(KERN_INFO Userspace triggered 0x700 exception
at 0x%lx (0x%x)\n, kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
+ pr_info(Userspace triggered 0x700 exception at\n
+ 0x%lx (0x%x)\n, kvmppc_get_pc(vcpu), last_inst);
#endif
- if ((kvmppc_get_last_inst(vcpu) 0xff0007ff) !=
+ if ((last_inst 0xff0007ff) !=
(INS_DCBZ 0xfff7)) {
kvmppc_core_queue_program(vcpu, flags);
r = RESUME_GUEST;
@@ -894,7 +894,7 @@ program_interrupt:
break;
case EMULATE_FAIL:
printk(KERN_CRIT %s: emulation at %lx failed
(%08x)\n,
- __func__, kvmppc_get_pc(vcpu),
kvmppc_get_last_inst(vcpu));
+ __func__, kvmppc_get_pc(vcpu), last_inst);
kvmppc_core_queue_program(vcpu, flags);
r = RESUME_GUEST;
break;
@@ -911,8 +911,12 @@ program_interrupt:
break;
}
case BOOK3S_INTERRUPT_SYSCALL:
+ {
+ u32 last_sc;
+
+ kvmppc_get_last_sc(vcpu, last_sc);
No check for the return value?
The existing code does not handle KVM_INST_FETCH_FAILED.
How should we continue if papr is enabled and last_sc fails?
if (vcpu-arch.papr_enabled
- (kvmppc_get_last_sc(vcpu) == 0x4422)
+ (last_sc == 0x4422)
!(vcpu-arch.shared-msr MSR_PR)) {
/* SC 1 papr hypercalls */
ulong cmd = kvmppc_get_gpr(vcpu, 3);
@@ -957,6 +961,7 @@ program_interrupt:
r = RESUME_GUEST;
}
break;
+ }
case BOOK3S_INTERRUPT_FP_UNAVAIL:
case BOOK3S_INTERRUPT_ALTIVEC:
case BOOK3S_INTERRUPT_VSX:
@@ -985,15 +990,20 @@ program_interrupt:
break;
}
case BOOK3S_INTERRUPT_ALIGNMENT:
+ {
+ u32 last_inst;
+
if (kvmppc_read_inst(vcpu) == EMULATE_DONE) {
- vcpu-arch.shared-dsisr = kvmppc_alignment_dsisr(vcpu,
- kvmppc_get_last_inst(vcpu));
- vcpu-arch.shared-dar =
RE: [PATCH v2 1/4] KVM: PPC: e500mc: Revert add load inst fixup
From: Alexander Graf ag...@suse.de Sent: Friday, May 2, 2014 12:24 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org Subject: Re: [PATCH v2 1/4] KVM: PPC: e500mc: Revert add load inst fixup On 05/01/2014 02:45 AM, Mihai Caraman wrote: The commit 1d628af7 add load inst fixup made an attempt to handle failures generated by reading the guest current instruction. The fixup code that was added works by chance hiding the real issue. Load external pid (lwepx) instruction, used by KVM to read guest instructions, is executed in a subsituted guest translation context (EPLC[EGS] = 1). In consequence lwepx's TLB error and data storage interrupts need to be handled by KVM, even though these interrupts are generated 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. As a result, the host kernel handles lwepx faults searching the faulting guest data address (loaded in DEAR) in its own Logical Partition ID (LPID) 0 context. In case a host translation is found the execution returns to the lwepx instruction instead of the fixup, the host ending up in an infinite loop. Revert the commit add load inst fixup. lwepx issue will be addressed in a subsequent patch without needing fixup code. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com Just a random idea: Could we just switch IVOR2 during the critical lwepx phase? In fact, we could even do that later when we're already in C code and try to recover the last instruction. The code IVOR2 would point to would simply set the register we're trying to read to as LAST_INST_FAIL and rfi. This was the first idea that sprang to my mind inspired from how DO_KVM is hooked on PR. I actually did a simple POC for e500mc/e5500, but this will not work on e6500 which has shared IVORs between HW threads. -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] KVM: PPC: Add devname:kvm aliases for modules
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Monday, December 09, 2013 5:02 PM To: ; kvm-ppc@vger.kernel.org@suse.de Cc: k...@vger.kernel.org mailing list Subject: [PATCH] KVM: PPC: Add devname:kvm aliases for modules Systems that support automatic loading of kernel modules through device aliases should try and automatically load kvm when /dev/kvm gets opened. Add code to support that magic for all PPC kvm targets, even the ones that don't support modules yet. Signed-off-by: Alexander Graf ag...@suse.de ... --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -391,3 +391,6 @@ static void __exit kvmppc_e500mc_exit(void) module_init(kvmppc_e500mc_init); module_exit(kvmppc_e500mc_exit); +#include linux/miscdevice.h +MODULE_ALIAS_MISCDEV(KVM_MINOR); +MODULE_ALIAS(devname:kvm); -- This patch breaks the build on KMV Book3E, you need to include linux/module.h too. -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] KVM: PPC: Add devname:kvm aliases for modules
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Monday, December 09, 2013 5:02 PM To: ; kvm-ppc@vger.kernel.org@suse.de Cc: k...@vger.kernel.org mailing list Subject: [PATCH] KVM: PPC: Add devname:kvm aliases for modules Systems that support automatic loading of kernel modules through device aliases should try and automatically load kvm when /dev/kvm gets opened. Add code to support that magic for all PPC kvm targets, even the ones that don't support modules yet. Signed-off-by: Alexander Graf ag...@suse.de ... --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -391,3 +391,6 @@ static void __exit kvmppc_e500mc_exit(void) module_init(kvmppc_e500mc_init); module_exit(kvmppc_e500mc_exit); +#include linux/miscdevice.h +MODULE_ALIAS_MISCDEV(KVM_MINOR); +MODULE_ALIAS(devname:kvm); -- This patch breaks the build on KMV Book3E, you need to include linux/module.h too. -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
