diff mbox

[v3,3/3] KVM: PPC: Book3S: MMIO emulation support for little endian guests

Message ID 1381246272-29284-1-git-send-email-clg@fr.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Cédric Le Goater Oct. 8, 2013, 3:31 p.m. UTC
MMIO emulation reads the last instruction executed by the guest
and then emulates. If the guest is running in Little Endian mode,
the instruction needs to be byte-swapped before being emulated.

This patch stores the last instruction in the endian order of the
host, primarily doing a byte-swap if needed. The common code
which fetches 'last_inst' uses a helper routine kvmppc_need_byteswap().
and the exit paths for the Book3S PV and HR guests use their own
version in assembly.

Finally, the meaning of the 'is_bigendian' argument of the
routines kvmppc_handle_load() of kvmppc_handle_store() is
slightly changed to represent an eventual reverse operation. This
is used in conjunction with kvmppc_is_bigendian() to determine if
the instruction being emulated should be byte-swapped.

Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---

Changes in v3:

 - moved kvmppc_need_byteswap() in kvmppc_ld32. It previously was in
   kvmppc_ld_inst(). (Alexander Graf)

Changes in v2:

 - replaced rldicl. by andi. to test the MSR_LE bit in the guest
   exit paths. (Paul Mackerras)

 - moved the byte swapping logic to kvmppc_handle_load() and 
   kvmppc_handle_load() by changing the is_bigendian parameter
   meaning. (Paul Mackerras)

 arch/powerpc/include/asm/kvm_book3s.h   |    9 ++++++++-
 arch/powerpc/include/asm/kvm_ppc.h      |   10 +++++-----
 arch/powerpc/kvm/book3s_hv_rmhandlers.S |   11 +++++++++++
 arch/powerpc/kvm/book3s_segment.S       |   10 ++++++++++
 arch/powerpc/kvm/emulate.c              |    1 -
 arch/powerpc/kvm/powerpc.c              |   16 ++++++++++++----
 6 files changed, 46 insertions(+), 11 deletions(-)

Comments

Alexander Graf Oct. 8, 2013, 3:36 p.m. UTC | #1
On 08.10.2013, at 17:31, Cédric Le Goater <clg@fr.ibm.com> wrote:

> MMIO emulation reads the last instruction executed by the guest
> and then emulates. If the guest is running in Little Endian mode,
> the instruction needs to be byte-swapped before being emulated.
> 
> This patch stores the last instruction in the endian order of the
> host, primarily doing a byte-swap if needed. The common code
> which fetches 'last_inst' uses a helper routine kvmppc_need_byteswap().
> and the exit paths for the Book3S PV and HR guests use their own
> version in assembly.
> 
> Finally, the meaning of the 'is_bigendian' argument of the
> routines kvmppc_handle_load() of kvmppc_handle_store() is
> slightly changed to represent an eventual reverse operation. This
> is used in conjunction with kvmppc_is_bigendian() to determine if
> the instruction being emulated should be byte-swapped.
> 
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> ---
> 
> Changes in v3:
> 
> - moved kvmppc_need_byteswap() in kvmppc_ld32. It previously was in
>   kvmppc_ld_inst(). (Alexander Graf)
> 
> Changes in v2:
> 
> - replaced rldicl. by andi. to test the MSR_LE bit in the guest
>   exit paths. (Paul Mackerras)
> 
> - moved the byte swapping logic to kvmppc_handle_load() and 
>   kvmppc_handle_load() by changing the is_bigendian parameter
>   meaning. (Paul Mackerras)
> 
> arch/powerpc/include/asm/kvm_book3s.h   |    9 ++++++++-
> arch/powerpc/include/asm/kvm_ppc.h      |   10 +++++-----
> arch/powerpc/kvm/book3s_hv_rmhandlers.S |   11 +++++++++++
> arch/powerpc/kvm/book3s_segment.S       |   10 ++++++++++
> arch/powerpc/kvm/emulate.c              |    1 -
> arch/powerpc/kvm/powerpc.c              |   16 ++++++++++++----
> 6 files changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index 4ee6c66..9403042 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -283,7 +283,14 @@ static inline bool kvmppc_is_bigendian(struct kvm_vcpu *vcpu)
> static inline int kvmppc_ld32(struct kvm_vcpu *vcpu, ulong *eaddr,
> 			      u32 *ptr, bool data)
> {
> -	return kvmppc_ld(vcpu, eaddr, sizeof(u32), ptr, data);
> +	int ret;
> +
> +	ret = kvmppc_ld(vcpu, eaddr, sizeof(u32), ptr, data);
> +
> +	if (kvmppc_need_byteswap(vcpu))
> +		*ptr = swab32(*ptr);
> +
> +	return ret;
> }
> 
> static inline int kvmppc_ld_inst(struct kvm_vcpu *vcpu, ulong *eaddr,

... at which point this helper is pretty useless ;).


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cédric Le Goater Oct. 8, 2013, 4:10 p.m. UTC | #2
On 10/08/2013 05:36 PM, Alexander Graf wrote:
> 
> On 08.10.2013, at 17:31, Cédric Le Goater <clg@fr.ibm.com> wrote:
> 
>> MMIO emulation reads the last instruction executed by the guest
>> and then emulates. If the guest is running in Little Endian mode,
>> the instruction needs to be byte-swapped before being emulated.
>>
>> This patch stores the last instruction in the endian order of the
>> host, primarily doing a byte-swap if needed. The common code
>> which fetches 'last_inst' uses a helper routine kvmppc_need_byteswap().
>> and the exit paths for the Book3S PV and HR guests use their own
>> version in assembly.
>>
>> Finally, the meaning of the 'is_bigendian' argument of the
>> routines kvmppc_handle_load() of kvmppc_handle_store() is
>> slightly changed to represent an eventual reverse operation. This
>> is used in conjunction with kvmppc_is_bigendian() to determine if
>> the instruction being emulated should be byte-swapped.
>>
>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
>> ---
>>
>> Changes in v3:
>>
>> - moved kvmppc_need_byteswap() in kvmppc_ld32. It previously was in
>>   kvmppc_ld_inst(). (Alexander Graf)
>>
>> Changes in v2:
>>
>> - replaced rldicl. by andi. to test the MSR_LE bit in the guest
>>   exit paths. (Paul Mackerras)
>>
>> - moved the byte swapping logic to kvmppc_handle_load() and 
>>   kvmppc_handle_load() by changing the is_bigendian parameter
>>   meaning. (Paul Mackerras)
>>
>> arch/powerpc/include/asm/kvm_book3s.h   |    9 ++++++++-
>> arch/powerpc/include/asm/kvm_ppc.h      |   10 +++++-----
>> arch/powerpc/kvm/book3s_hv_rmhandlers.S |   11 +++++++++++
>> arch/powerpc/kvm/book3s_segment.S       |   10 ++++++++++
>> arch/powerpc/kvm/emulate.c              |    1 -
>> arch/powerpc/kvm/powerpc.c              |   16 ++++++++++++----
>> 6 files changed, 46 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
>> index 4ee6c66..9403042 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>> @@ -283,7 +283,14 @@ static inline bool kvmppc_is_bigendian(struct kvm_vcpu *vcpu)
>> static inline int kvmppc_ld32(struct kvm_vcpu *vcpu, ulong *eaddr,
>> 			      u32 *ptr, bool data)
>> {
>> -	return kvmppc_ld(vcpu, eaddr, sizeof(u32), ptr, data);
>> +	int ret;
>> +
>> +	ret = kvmppc_ld(vcpu, eaddr, sizeof(u32), ptr, data);
>> +
>> +	if (kvmppc_need_byteswap(vcpu))
>> +		*ptr = swab32(*ptr);
>> +
>> +	return ret;
>> }
>>
>> static inline int kvmppc_ld_inst(struct kvm_vcpu *vcpu, ulong *eaddr,
> 
> ... at which point this helper is pretty useless ;).

ok. This sounds like a request for a v4 ... :)

C.



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cédric Le Goater Oct. 8, 2013, 4:43 p.m. UTC | #3
MIO emulation reads the last instruction executed by the guest
and then emulates. If the guest is running in Little Endian mode,
the instruction needs to be byte-swapped before being emulated.

The first patches add simple helper routines to load instructions from 
the guest. It prepares ground for the byte-swapping of instructions 
when reading memory from Little Endian guests. 

The last patch enables the MMIO support by byte-swapping the last 
instruction if the guest is Little Endian.

This patchset is based on Alex Graf's kvm-ppc-queue branch. It has been 
tested with anton's patchset for Big Endian and Little Endian HV guests 
and Big Endian PR guests. 

Changes in v4:

 - got rid of useless helper routine kvmppc_ld_inst(). (Alexander Graf)

Changes in v3:

 - moved kvmppc_need_byteswap() in kvmppc_ld32. It previously was in
   kvmppc_ld_inst(). (Alexander Graf)

Changes in v2:

 - replaced rldicl. by andi. to test the MSR_LE bit in the guest
   exit paths. (Paul Mackerras)

 - moved the byte swapping logic to kvmppc_handle_load() and 
   kvmppc_handle_load() by changing the is_bigendian parameter
   meaning. (Paul Mackerras)

Thanks,

C.	

Cédric Le Goater (3):
  KVM: PPC: Book3S: add helper routine to load guest instructions
  KVM: PPC: Book3S: add helper routines to detect endian
  KVM: PPC: Book3S: MMIO emulation support for little endian guests

 arch/powerpc/include/asm/kvm_book3s.h   |   27 +++++++++++++++++++++++++--
 arch/powerpc/include/asm/kvm_ppc.h      |   10 +++++-----
 arch/powerpc/kvm/book3s_64_mmu_hv.c     |    2 +-
 arch/powerpc/kvm/book3s_hv_rmhandlers.S |   11 +++++++++++
 arch/powerpc/kvm/book3s_pr.c            |    2 +-
 arch/powerpc/kvm/book3s_segment.S       |   10 ++++++++++
 arch/powerpc/kvm/emulate.c              |    1 -
 arch/powerpc/kvm/powerpc.c              |   16 ++++++++++++----
 8 files changed, 65 insertions(+), 14 deletions(-)
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index 4ee6c66..9403042 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -283,7 +283,14 @@  static inline bool kvmppc_is_bigendian(struct kvm_vcpu *vcpu)
 static inline int kvmppc_ld32(struct kvm_vcpu *vcpu, ulong *eaddr,
 			      u32 *ptr, bool data)
 {
-	return kvmppc_ld(vcpu, eaddr, sizeof(u32), ptr, data);
+	int ret;
+
+	ret = kvmppc_ld(vcpu, eaddr, sizeof(u32), ptr, data);
+
+	if (kvmppc_need_byteswap(vcpu))
+		*ptr = swab32(*ptr);
+
+	return ret;
 }
 
 static inline int kvmppc_ld_inst(struct kvm_vcpu *vcpu, ulong *eaddr,
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index b15554a..3769a13 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -53,13 +53,13 @@  extern void kvmppc_handler_highmem(void);
 
 extern void kvmppc_dump_vcpu(struct kvm_vcpu *vcpu);
 extern int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
-                              unsigned int rt, unsigned int bytes,
-                              int is_bigendian);
+			      unsigned int rt, unsigned int bytes,
+			      int not_reverse);
 extern int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu,
-                               unsigned int rt, unsigned int bytes,
-                               int is_bigendian);
+			       unsigned int rt, unsigned int bytes,
+			       int not_reverse);
 extern int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu,
-                               u64 val, unsigned int bytes, int is_bigendian);
+			       u64 val, unsigned int bytes, int not_reverse);
 
 extern int kvmppc_emulate_instruction(struct kvm_run *run,
                                       struct kvm_vcpu *vcpu);
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 77f1baa..ff7da8b 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1404,10 +1404,21 @@  fast_interrupt_c_return:
 	lwz	r8, 0(r10)
 	mtmsrd	r3
 
+	ld	r0, VCPU_MSR(r9)
+
+	andi.	r10, r0, MSR_LE
+
 	/* Store the result */
 	stw	r8, VCPU_LAST_INST(r9)
 
+	beq	after_inst_store
+
+	/* Swap and store the result */
+	addi	r11, r9, VCPU_LAST_INST
+	stwbrx	r8, 0, r11
+
 	/* Unset guest mode. */
+after_inst_store:
 	li	r0, KVM_GUEST_MODE_HOST_HV
 	stb	r0, HSTATE_IN_GUEST(r13)
 	b	guest_exit_cont
diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S
index 1abe478..677ef7a 100644
--- a/arch/powerpc/kvm/book3s_segment.S
+++ b/arch/powerpc/kvm/book3s_segment.S
@@ -287,8 +287,18 @@  ld_last_inst:
 	sync
 
 #endif
+	ld	r8, SVCPU_SHADOW_SRR1(r13)
+
+	andi.	r10, r8, MSR_LE
+
 	stw	r0, SVCPU_LAST_INST(r13)
 
+	beq	no_ld_last_inst
+
+	/* swap and store the result */
+	addi	r11, r13, SVCPU_LAST_INST
+	stwbrx	r0, 0, r11
+
 no_ld_last_inst:
 
 	/* Unset guest mode */
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index 751cd45..5e38004 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -219,7 +219,6 @@  static int kvmppc_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
  * lmw
  * stmw
  *
- * XXX is_bigendian should depend on MMU mapping or MSR[LE]
  */
 /* XXX Should probably auto-generate instruction decoding for a particular core
  * from opcode tables in the future. */
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 07c0106..6950f2b 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -625,9 +625,13 @@  static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu,
 }
 
 int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
-                       unsigned int rt, unsigned int bytes, int is_bigendian)
+			unsigned int rt, unsigned int bytes, int not_reverse)
 {
 	int idx, ret;
+	int is_bigendian = not_reverse;
+
+	if (!kvmppc_is_bigendian(vcpu))
+		is_bigendian = !not_reverse;
 
 	if (bytes > sizeof(run->mmio.data)) {
 		printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__,
@@ -662,21 +666,25 @@  int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
 
 /* Same as above, but sign extends */
 int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu,
-                        unsigned int rt, unsigned int bytes, int is_bigendian)
+			unsigned int rt, unsigned int bytes, int not_reverse)
 {
 	int r;
 
 	vcpu->arch.mmio_sign_extend = 1;
-	r = kvmppc_handle_load(run, vcpu, rt, bytes, is_bigendian);
+	r = kvmppc_handle_load(run, vcpu, rt, bytes, not_reverse);
 
 	return r;
 }
 
 int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu,
-                        u64 val, unsigned int bytes, int is_bigendian)
+			u64 val, unsigned int bytes, int not_reverse)
 {
 	void *data = run->mmio.data;
 	int idx, ret;
+	int is_bigendian = not_reverse;
+
+	if (!kvmppc_is_bigendian(vcpu))
+		is_bigendian = !not_reverse;
 
 	if (bytes > sizeof(run->mmio.data)) {
 		printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__,