On 21.07.14 11:59, mihai.caraman@xxxxxxxxxxxxx wrote:
-----Original Message-----
From: Linuxppc-dev [mailto:linuxppc-dev-
bounces+mihai.caraman=freescale.com@xxxxxxxxxxxxxxxx] On Behalf Of
mihai.caraman@xxxxxxxxxxxxx
Sent: Friday, July 18, 2014 12:06 PM
To: Alexander Graf; kvm-ppc@xxxxxxxxxxxxxxx
Cc: linuxppc-dev@xxxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx
Subject: RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
-----Original Message-----
From: Alexander Graf [mailto:agraf@xxxxxxx]
Sent: Thursday, July 17, 2014 5:21 PM
To: Caraman Mihai Claudiu-B02008; kvm-ppc@xxxxxxxxxxxxxxx
Cc: kvm@xxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx
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.caraman@xxxxxxxxxxxxx>
---
...
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. last_inst stays unmodified.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
