linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions
@ 2018-02-01 18:15 Jose Ricardo Ziviani
  2018-02-01 18:15 ` [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions Jose Ricardo Ziviani
  2018-02-02  0:30 ` [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Paul Mackerras
  0 siblings, 2 replies; 5+ messages in thread
From: Jose Ricardo Ziviani @ 2018-02-01 18:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: kvm-ppc, paulus, lvivier

v5:
 - Fixed the mask off of the effective address

v4:
  - Changed KVM_MMIO_REG_VMX to 0xc0 because there are 64 VSX registers

v3:
  - Added Reported-by in the commit message

v2:
  - kvmppc_get_vsr_word_offset() moved back to its original place
  - EA AND ~0xF, following ISA.
  - fixed BE/LE cases

TESTS:

For testing purposes I wrote a small program that performs stvx/lvx using the
program's virtual memory and using MMIO. Load/Store into virtual memory is the
model I use to check if MMIO results are correct (because only MMIO is emulated
by KVM).

Results:

HOST LE - GUEST BE
address: 0x10034850010
0x21436587bbbbaaaa4444555578563412
io_address: 0x3fff89a20000
0x21436587bbbbaaaa4444555578563412

HOST LE - GUEST LE
address: 0x10033a20010
0x1234567855554444aaaabbbb87654321
io_address: 0x3fffb5380000
0x1234567855554444aaaabbbb87654321

HOST BE - GUEST BE
address: 0x1002c4a0010
0x21436587bbbbaaaa4444555578563412
io_address: 0x3ffface40000
0x21436587bbbbaaaa4444555578563412

HOST BR - GUEST LE
address: 0x100225e0010
0x1234567855554444aaaabbbb87654321
io_address: 0x3fff7fcb0000
0x1234567855554444aaaabbbb87654321

This patch implements MMIO emulation for two instructions: lvx and stvx.

Jose Ricardo Ziviani (1):
  KVM: PPC: Book3S: Add MMIO emulation for VMX instructions

 arch/powerpc/include/asm/kvm_host.h   |   2 +
 arch/powerpc/include/asm/kvm_ppc.h    |   4 +
 arch/powerpc/include/asm/ppc-opcode.h |   6 ++
 arch/powerpc/kvm/emulate_loadstore.c  |  34 ++++++++
 arch/powerpc/kvm/powerpc.c            | 153 +++++++++++++++++++++++++++++++++-
 5 files changed, 198 insertions(+), 1 deletion(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions
  2018-02-01 18:15 [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Jose Ricardo Ziviani
@ 2018-02-01 18:15 ` Jose Ricardo Ziviani
  2018-02-02  2:55   ` Paul Mackerras
  2018-02-02  0:30 ` [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Paul Mackerras
  1 sibling, 1 reply; 5+ messages in thread
From: Jose Ricardo Ziviani @ 2018-02-01 18:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: kvm-ppc, paulus, lvivier

This patch provides the MMIO load/store vector indexed
X-Form emulation.

Instructions implemented:
lvx: the quadword in storage addressed by the result of EA &
0xffff_ffff_ffff_fff0 is loaded into VRT.

stvx: the contents of VRS are stored into the quadword in storage
addressed by the result of EA & 0xffff_ffff_ffff_fff0.

Reported-by: Gopesh Kumar Chaudhary <gopchaud@in.ibm.com>
Reported-by: Balamuruhan S <bala24@linux.vnet.ibm.com>
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/kvm_host.h   |   2 +
 arch/powerpc/include/asm/kvm_ppc.h    |   4 +
 arch/powerpc/include/asm/ppc-opcode.h |   6 ++
 arch/powerpc/kvm/emulate_loadstore.c  |  34 ++++++++
 arch/powerpc/kvm/powerpc.c            | 153 +++++++++++++++++++++++++++++++++-
 5 files changed, 198 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 3aa5b577cd60..045acc843e98 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -690,6 +690,7 @@ struct kvm_vcpu_arch {
 	u8 mmio_vsx_offset;
 	u8 mmio_vsx_copy_type;
 	u8 mmio_vsx_tx_sx_enabled;
+	u8 mmio_vmx_copy_nums;
 	u8 osi_needed;
 	u8 osi_enabled;
 	u8 papr_enabled;
@@ -800,6 +801,7 @@ struct kvm_vcpu_arch {
 #define KVM_MMIO_REG_QPR	0x0040
 #define KVM_MMIO_REG_FQPR	0x0060
 #define KVM_MMIO_REG_VSX	0x0080
+#define KVM_MMIO_REG_VMX	0x00c0
 
 #define __KVM_HAVE_ARCH_WQP
 #define __KVM_HAVE_CREATE_DEVICE
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 9db18287b5f4..7765a800ddae 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -81,6 +81,10 @@ extern int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu,
 extern int kvmppc_handle_vsx_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
 				unsigned int rt, unsigned int bytes,
 			int is_default_endian, int mmio_sign_extend);
+extern int kvmppc_handle_load128_by2x64(struct kvm_run *run,
+		struct kvm_vcpu *vcpu, unsigned int rt, int is_default_endian);
+extern int kvmppc_handle_store128_by2x64(struct kvm_run *run,
+		struct kvm_vcpu *vcpu, unsigned int rs, int is_default_endian);
 extern int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu,
 			       u64 val, unsigned int bytes,
 			       int is_default_endian);
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index ab5c1588b487..f1083bcf449c 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -156,6 +156,12 @@
 #define OP_31_XOP_LFDX          599
 #define OP_31_XOP_LFDUX		631
 
+/* VMX Vector Load Instructions */
+#define OP_31_XOP_LVX           103
+
+/* VMX Vector Store Instructions */
+#define OP_31_XOP_STVX          231
+
 #define OP_LWZ  32
 #define OP_STFS 52
 #define OP_STFSU 53
diff --git a/arch/powerpc/kvm/emulate_loadstore.c b/arch/powerpc/kvm/emulate_loadstore.c
index af833531af31..332b82eafd48 100644
--- a/arch/powerpc/kvm/emulate_loadstore.c
+++ b/arch/powerpc/kvm/emulate_loadstore.c
@@ -58,6 +58,18 @@ static bool kvmppc_check_vsx_disabled(struct kvm_vcpu *vcpu)
 }
 #endif /* CONFIG_VSX */
 
+#ifdef CONFIG_ALTIVEC
+static bool kvmppc_check_altivec_disabled(struct kvm_vcpu *vcpu)
+{
+	if (!(kvmppc_get_msr(vcpu) & MSR_VEC)) {
+		kvmppc_core_queue_vec_unavail(vcpu);
+		return true;
+	}
+
+	return false;
+}
+#endif /* CONFIG_ALTIVEC */
+
 /*
  * XXX to do:
  * lfiwax, lfiwzx
@@ -98,6 +110,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
 	vcpu->arch.mmio_vsx_copy_type = KVMPPC_VSX_COPY_NONE;
 	vcpu->arch.mmio_sp64_extend = 0;
 	vcpu->arch.mmio_sign_extend = 0;
+	vcpu->arch.mmio_vmx_copy_nums = 0;
 
 	switch (get_op(inst)) {
 	case 31:
@@ -459,6 +472,27 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
 							 rs, 4, 1);
 			break;
 #endif /* CONFIG_VSX */
+
+#ifdef CONFIG_ALTIVEC
+		case OP_31_XOP_LVX:
+			if (kvmppc_check_altivec_disabled(vcpu))
+				return EMULATE_DONE;
+			vcpu->arch.vaddr_accessed &= ~0xFULL;
+			vcpu->arch.mmio_vmx_copy_nums = 2;
+			emulated = kvmppc_handle_load128_by2x64(run, vcpu,
+					KVM_MMIO_REG_VMX|rt, 1);
+			break;
+
+		case OP_31_XOP_STVX:
+			if (kvmppc_check_altivec_disabled(vcpu))
+				return EMULATE_DONE;
+			vcpu->arch.vaddr_accessed &= ~0xFULL;
+			vcpu->arch.mmio_vmx_copy_nums = 2;
+			emulated = kvmppc_handle_store128_by2x64(run, vcpu,
+					rs, 1);
+			break;
+#endif /* CONFIG_ALTIVEC */
+
 		default:
 			emulated = EMULATE_FAIL;
 			break;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 1915e86cef6f..a19f42120b38 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -832,7 +832,7 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
 		kvm->arch.kvm_ops->irq_bypass_del_producer(cons, prod);
 }
 
-#ifdef CONFIG_VSX
+#ifdef CONFIG_ALTIVEC
 static inline int kvmppc_get_vsr_dword_offset(int index)
 {
 	int offset;
@@ -848,7 +848,9 @@ static inline int kvmppc_get_vsr_dword_offset(int index)
 
 	return offset;
 }
+#endif /* CONFIG_ALTIVEC */
 
+#ifdef CONFIG_VSX
 static inline int kvmppc_get_vsr_word_offset(int index)
 {
 	int offset;
@@ -925,6 +927,31 @@ static inline void kvmppc_set_vsr_word(struct kvm_vcpu *vcpu,
 }
 #endif /* CONFIG_VSX */
 
+#ifdef CONFIG_ALTIVEC
+static inline void kvmppc_set_vmx_dword(struct kvm_vcpu *vcpu,
+		u64 gpr)
+{
+	int index = vcpu->arch.io_gpr & KVM_MMIO_REG_MASK;
+	u32 hi, lo;
+
+#ifdef __BIG_ENDIAN
+	hi = gpr >> 32;
+	lo = gpr & 0xffffffff;
+#else
+	lo = gpr >> 32;
+	hi = gpr & 0xffffffff;
+#endif
+
+	if (vcpu->arch.mmio_vmx_copy_nums == 1) {
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(2)] = lo;
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(3)] = hi;
+	} else if (vcpu->arch.mmio_vmx_copy_nums == 2) {
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(0)] = lo;
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(1)] = hi;
+	}
+}
+#endif /* CONFIG_ALTIVEC */
+
 #ifdef CONFIG_PPC_FPU
 static inline u64 sp_to_dp(u32 fprs)
 {
@@ -1027,6 +1054,11 @@ static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu,
 				KVMPPC_VSX_COPY_DWORD_LOAD_DUMP)
 			kvmppc_set_vsr_dword_dump(vcpu, gpr);
 		break;
+#endif
+#ifdef CONFIG_ALTIVEC
+	case KVM_MMIO_REG_VMX:
+		kvmppc_set_vmx_dword(vcpu, gpr);
+		break;
 #endif
 	default:
 		BUG();
@@ -1307,6 +1339,113 @@ static int kvmppc_emulate_mmio_vsx_loadstore(struct kvm_vcpu *vcpu,
 }
 #endif /* CONFIG_VSX */
 
+#ifdef CONFIG_ALTIVEC
+/* handle quadword load access in two halves */
+int kvmppc_handle_load128_by2x64(struct kvm_run *run, struct kvm_vcpu *vcpu,
+		unsigned int rt, int is_default_endian)
+{
+	enum emulation_result emulated;
+
+	while (vcpu->arch.mmio_vmx_copy_nums) {
+		emulated = __kvmppc_handle_load(run, vcpu, rt, 8,
+				is_default_endian, 0);
+
+		if (emulated != EMULATE_DONE)
+			break;
+
+		vcpu->arch.paddr_accessed += run->mmio.len;
+		vcpu->arch.mmio_vmx_copy_nums--;
+	}
+
+	return emulated;
+}
+
+static inline int kvmppc_get_vmx_data(struct kvm_vcpu *vcpu, int rs, u64 *val)
+{
+	vector128 vrs = VCPU_VSX_VR(vcpu, rs);
+
+	if (vcpu->arch.mmio_vmx_copy_nums == 1) {
+#ifdef __BIG_ENDIAN
+		*val = vrs.u[kvmppc_get_vsr_word_offset(3)];
+		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(2)];
+#else
+		*val = vrs.u[kvmppc_get_vsr_word_offset(2)];
+		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(3)];
+#endif
+		return 0;
+	} else if (vcpu->arch.mmio_vmx_copy_nums == 2) {
+#ifdef __BIG_ENDIAN
+		*val = vrs.u[kvmppc_get_vsr_word_offset(1)];
+		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(0)];
+#else
+		*val = vrs.u[kvmppc_get_vsr_word_offset(0)];
+		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(1)];
+#endif
+		return 0;
+	}
+	return -1;
+}
+
+/* handle quadword store in two halves */
+int kvmppc_handle_store128_by2x64(struct kvm_run *run, struct kvm_vcpu *vcpu,
+		unsigned int rs, int is_default_endian)
+{
+	u64 val = 0;
+	enum emulation_result emulated = EMULATE_DONE;
+
+	vcpu->arch.io_gpr = rs;
+
+	while (vcpu->arch.mmio_vmx_copy_nums) {
+		if (kvmppc_get_vmx_data(vcpu, rs, &val) == -1)
+			return EMULATE_FAIL;
+
+		emulated = kvmppc_handle_store(run, vcpu, val, 8,
+				is_default_endian);
+		if (emulated != EMULATE_DONE)
+			break;
+
+		vcpu->arch.paddr_accessed += run->mmio.len;
+		vcpu->arch.mmio_vmx_copy_nums--;
+	}
+
+	return emulated;
+}
+
+static int kvmppc_emulate_mmio_vmx_loadstore(struct kvm_vcpu *vcpu,
+		struct kvm_run *run)
+{
+	enum emulation_result emulated = EMULATE_FAIL;
+	int r;
+
+	vcpu->arch.paddr_accessed += run->mmio.len;
+
+	if (!vcpu->mmio_is_write) {
+		emulated = kvmppc_handle_load128_by2x64(run, vcpu,
+				vcpu->arch.io_gpr, 1);
+	} else {
+		emulated = kvmppc_handle_store128_by2x64(run, vcpu,
+				vcpu->arch.io_gpr, 1);
+	}
+
+	switch (emulated) {
+	case EMULATE_DO_MMIO:
+		run->exit_reason = KVM_EXIT_MMIO;
+		r = RESUME_HOST;
+		break;
+	case EMULATE_FAIL:
+		pr_info("KVM: MMIO emulation failed (VMX repeat)\n");
+		run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
+		run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
+		r = RESUME_HOST;
+		break;
+	default:
+		r = RESUME_GUEST;
+		break;
+	}
+	return r;
+}
+#endif /* CONFIG_ALTIVEC */
+
 int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 {
 	int r = 0;
@@ -1425,6 +1564,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 				return r;
 			}
 		}
+#endif
+#ifdef CONFIG_ALTIVEC
+		if (vcpu->arch.mmio_vmx_copy_nums > 0)
+			vcpu->arch.mmio_vmx_copy_nums--;
+
+		if (vcpu->arch.mmio_vmx_copy_nums > 0) {
+			r = kvmppc_emulate_mmio_vmx_loadstore(vcpu, run);
+			if (r == RESUME_HOST) {
+				vcpu->mmio_needed = 1;
+				return r;
+			}
+		}
 #endif
 	} else if (vcpu->arch.osi_needed) {
 		u64 *gprs = run->osi.gprs;
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions
  2018-02-01 18:15 [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Jose Ricardo Ziviani
  2018-02-01 18:15 ` [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions Jose Ricardo Ziviani
@ 2018-02-02  0:30 ` Paul Mackerras
  2018-02-03  0:02   ` joserz
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Mackerras @ 2018-02-02  0:30 UTC (permalink / raw)
  To: Jose Ricardo Ziviani; +Cc: linuxppc-dev, kvm-ppc, lvivier

On Thu, Feb 01, 2018 at 04:15:38PM -0200, Jose Ricardo Ziviani wrote:
> v5:
>  - Fixed the mask off of the effective address
> 
> v4:
>   - Changed KVM_MMIO_REG_VMX to 0xc0 because there are 64 VSX registers
> 
> v3:
>   - Added Reported-by in the commit message
> 
> v2:
>   - kvmppc_get_vsr_word_offset() moved back to its original place
>   - EA AND ~0xF, following ISA.
>   - fixed BE/LE cases
> 
> TESTS:
> 
> For testing purposes I wrote a small program that performs stvx/lvx using the
> program's virtual memory and using MMIO. Load/Store into virtual memory is the
> model I use to check if MMIO results are correct (because only MMIO is emulated
> by KVM).

I'd be interested to see your test program because in my testing it's
still not right, unfortunately.  Interestingly, it is right for the BE
guest on LE host case.  However, with a LE guest on a LE host the two
halves are swapped, both for lvx and stvx:

error in lvx at byte 0
was: -> 62 69 70 77 7e 85 8c 93 2a 31 38 3f 46 4d 54 5b
ref: -> 2a 31 38 3f 46 4d 54 5b 62 69 70 77 7e 85 8c 93
error in stvx at byte 0
was: -> 49 50 57 5e 65 6c 73 7a 11 18 1f 26 2d 34 3b 42
ref: -> 11 18 1f 26 2d 34 3b 42 49 50 57 5e 65 6c 73 7a

The byte order within each 8-byte half is correct but the two halves
are swapped.  ("was" is what was in memory and "ref" is the correct
value.  For lvx it does lvx from emulated MMIO and stvx to ordinary
memory, and for stvx it does lvx from ordinary memory and stvx to
emulated MMIO.  In both cases the checking is done with a byte by byte
comparison.)

Paul.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions
  2018-02-01 18:15 ` [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions Jose Ricardo Ziviani
@ 2018-02-02  2:55   ` Paul Mackerras
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Mackerras @ 2018-02-02  2:55 UTC (permalink / raw)
  To: Jose Ricardo Ziviani; +Cc: linuxppc-dev, kvm-ppc, lvivier

On Thu, Feb 01, 2018 at 04:15:39PM -0200, Jose Ricardo Ziviani wrote:
> This patch provides the MMIO load/store vector indexed
> X-Form emulation.
> 
> Instructions implemented:
> lvx: the quadword in storage addressed by the result of EA &
> 0xffff_ffff_ffff_fff0 is loaded into VRT.
> 
> stvx: the contents of VRS are stored into the quadword in storage
> addressed by the result of EA & 0xffff_ffff_ffff_fff0.
> 
> Reported-by: Gopesh Kumar Chaudhary <gopchaud@in.ibm.com>
> Reported-by: Balamuruhan S <bala24@linux.vnet.ibm.com>
> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/kvm_host.h   |   2 +
>  arch/powerpc/include/asm/kvm_ppc.h    |   4 +
>  arch/powerpc/include/asm/ppc-opcode.h |   6 ++
>  arch/powerpc/kvm/emulate_loadstore.c  |  34 ++++++++
>  arch/powerpc/kvm/powerpc.c            | 153 +++++++++++++++++++++++++++++++++-
>  5 files changed, 198 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 3aa5b577cd60..045acc843e98 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -690,6 +690,7 @@ struct kvm_vcpu_arch {
>  	u8 mmio_vsx_offset;
>  	u8 mmio_vsx_copy_type;
>  	u8 mmio_vsx_tx_sx_enabled;
> +	u8 mmio_vmx_copy_nums;
>  	u8 osi_needed;
>  	u8 osi_enabled;
>  	u8 papr_enabled;
> @@ -800,6 +801,7 @@ struct kvm_vcpu_arch {
>  #define KVM_MMIO_REG_QPR	0x0040
>  #define KVM_MMIO_REG_FQPR	0x0060
>  #define KVM_MMIO_REG_VSX	0x0080
> +#define KVM_MMIO_REG_VMX	0x00c0
>  
>  #define __KVM_HAVE_ARCH_WQP
>  #define __KVM_HAVE_CREATE_DEVICE
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index 9db18287b5f4..7765a800ddae 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -81,6 +81,10 @@ extern int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu,
>  extern int kvmppc_handle_vsx_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
>  				unsigned int rt, unsigned int bytes,
>  			int is_default_endian, int mmio_sign_extend);
> +extern int kvmppc_handle_load128_by2x64(struct kvm_run *run,
> +		struct kvm_vcpu *vcpu, unsigned int rt, int is_default_endian);
> +extern int kvmppc_handle_store128_by2x64(struct kvm_run *run,
> +		struct kvm_vcpu *vcpu, unsigned int rs, int is_default_endian);
>  extern int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu,
>  			       u64 val, unsigned int bytes,
>  			       int is_default_endian);
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index ab5c1588b487..f1083bcf449c 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -156,6 +156,12 @@
>  #define OP_31_XOP_LFDX          599
>  #define OP_31_XOP_LFDUX		631
>  
> +/* VMX Vector Load Instructions */
> +#define OP_31_XOP_LVX           103
> +
> +/* VMX Vector Store Instructions */
> +#define OP_31_XOP_STVX          231
> +
>  #define OP_LWZ  32
>  #define OP_STFS 52
>  #define OP_STFSU 53
> diff --git a/arch/powerpc/kvm/emulate_loadstore.c b/arch/powerpc/kvm/emulate_loadstore.c
> index af833531af31..332b82eafd48 100644
> --- a/arch/powerpc/kvm/emulate_loadstore.c
> +++ b/arch/powerpc/kvm/emulate_loadstore.c
> @@ -58,6 +58,18 @@ static bool kvmppc_check_vsx_disabled(struct kvm_vcpu *vcpu)
>  }
>  #endif /* CONFIG_VSX */
>  
> +#ifdef CONFIG_ALTIVEC
> +static bool kvmppc_check_altivec_disabled(struct kvm_vcpu *vcpu)
> +{
> +	if (!(kvmppc_get_msr(vcpu) & MSR_VEC)) {
> +		kvmppc_core_queue_vec_unavail(vcpu);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +#endif /* CONFIG_ALTIVEC */
> +
>  /*
>   * XXX to do:
>   * lfiwax, lfiwzx
> @@ -98,6 +110,7 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
>  	vcpu->arch.mmio_vsx_copy_type = KVMPPC_VSX_COPY_NONE;
>  	vcpu->arch.mmio_sp64_extend = 0;
>  	vcpu->arch.mmio_sign_extend = 0;
> +	vcpu->arch.mmio_vmx_copy_nums = 0;
>  
>  	switch (get_op(inst)) {
>  	case 31:
> @@ -459,6 +472,27 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
>  							 rs, 4, 1);
>  			break;
>  #endif /* CONFIG_VSX */
> +
> +#ifdef CONFIG_ALTIVEC
> +		case OP_31_XOP_LVX:
> +			if (kvmppc_check_altivec_disabled(vcpu))
> +				return EMULATE_DONE;
> +			vcpu->arch.vaddr_accessed &= ~0xFULL;
> +			vcpu->arch.mmio_vmx_copy_nums = 2;
> +			emulated = kvmppc_handle_load128_by2x64(run, vcpu,
> +					KVM_MMIO_REG_VMX|rt, 1);
> +			break;
> +
> +		case OP_31_XOP_STVX:
> +			if (kvmppc_check_altivec_disabled(vcpu))
> +				return EMULATE_DONE;
> +			vcpu->arch.vaddr_accessed &= ~0xFULL;
> +			vcpu->arch.mmio_vmx_copy_nums = 2;
> +			emulated = kvmppc_handle_store128_by2x64(run, vcpu,
> +					rs, 1);
> +			break;
> +#endif /* CONFIG_ALTIVEC */
> +
>  		default:
>  			emulated = EMULATE_FAIL;
>  			break;
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 1915e86cef6f..a19f42120b38 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -832,7 +832,7 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
>  		kvm->arch.kvm_ops->irq_bypass_del_producer(cons, prod);
>  }
>  
> -#ifdef CONFIG_VSX
> +#ifdef CONFIG_ALTIVEC
>  static inline int kvmppc_get_vsr_dword_offset(int index)
>  {
>  	int offset;
> @@ -848,7 +848,9 @@ static inline int kvmppc_get_vsr_dword_offset(int index)
>  
>  	return offset;
>  }
> +#endif /* CONFIG_ALTIVEC */
>  
> +#ifdef CONFIG_VSX
>  static inline int kvmppc_get_vsr_word_offset(int index)

You make the dword version available with ALTIVEC && ~VSX, but in fact
it's the word version that you use below.  However, I don't think we
actually want either of them (see below).

>  {
>  	int offset;
> @@ -925,6 +927,31 @@ static inline void kvmppc_set_vsr_word(struct kvm_vcpu *vcpu,
>  }
>  #endif /* CONFIG_VSX */
>  
> +#ifdef CONFIG_ALTIVEC
> +static inline void kvmppc_set_vmx_dword(struct kvm_vcpu *vcpu,
> +		u64 gpr)
> +{
> +	int index = vcpu->arch.io_gpr & KVM_MMIO_REG_MASK;
> +	u32 hi, lo;
> +
> +#ifdef __BIG_ENDIAN
> +	hi = gpr >> 32;
> +	lo = gpr & 0xffffffff;
> +#else
> +	lo = gpr >> 32;
> +	hi = gpr & 0xffffffff;
> +#endif
> +
> +	if (vcpu->arch.mmio_vmx_copy_nums == 1) {
> +		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(2)] = lo;
> +		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(3)] = hi;
> +	} else if (vcpu->arch.mmio_vmx_copy_nums == 2) {
> +		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(0)] = lo;
> +		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(1)] = hi;
> +	}

Since what we're doing is a 16-byte load, the main thing we have to do
here in handling a cross-endian situation is to swap the two 8-byte
halves.  The byte-swapping within each 8-byte half has already been
handled more generically.

I suggest the following code.  It is simpler and passes my test case.

static inline void kvmppc_set_vmx_dword(struct kvm_vcpu *vcpu,
		u64 gpr)
{
	int index = vcpu->arch.io_gpr & KVM_MMIO_REG_MASK;
	u32 hi, lo;
	u32 di;

#ifdef __BIG_ENDIAN
	hi = gpr >> 32;
	lo = gpr & 0xffffffff;
#else
	lo = gpr >> 32;
	hi = gpr & 0xffffffff;
#endif

	di = 2 - vcpu->arch.mmio_vmx_copy_nums;		/* doubleword index */
	if (di > 1)
		return;
	if (vcpu->arch.mmio_host_swabbed)
		di = 1 - di;

	VCPU_VSX_VR(vcpu, index).u[di * 2] = hi;
	VCPU_VSX_VR(vcpu, index).u[di * 2 + 1] = lo;
}

> +static inline int kvmppc_get_vmx_data(struct kvm_vcpu *vcpu, int rs, u64 *val)
> +{
> +	vector128 vrs = VCPU_VSX_VR(vcpu, rs);
> +
> +	if (vcpu->arch.mmio_vmx_copy_nums == 1) {
> +#ifdef __BIG_ENDIAN
> +		*val = vrs.u[kvmppc_get_vsr_word_offset(3)];
> +		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(2)];
> +#else
> +		*val = vrs.u[kvmppc_get_vsr_word_offset(2)];
> +		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(3)];
> +#endif
> +		return 0;
> +	} else if (vcpu->arch.mmio_vmx_copy_nums == 2) {
> +#ifdef __BIG_ENDIAN
> +		*val = vrs.u[kvmppc_get_vsr_word_offset(1)];
> +		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(0)];
> +#else
> +		*val = vrs.u[kvmppc_get_vsr_word_offset(0)];
> +		*val = (*val << 32) | vrs.u[kvmppc_get_vsr_word_offset(1)];
> +#endif
> +		return 0;
> +	}
> +	return -1;

Once again the main thing is to swap the two halves.  My suggested
code is:

static inline int kvmppc_get_vmx_data(struct kvm_vcpu *vcpu, int rs, u64 *val)
{
	vector128 vrs = VCPU_VSX_VR(vcpu, rs);
	u32 di;
	u64 w0, w1;

	di = 2 - vcpu->arch.mmio_vmx_copy_nums;		/* doubleword index */
	if (di > 1)
		return -1;
	if (vcpu->arch.mmio_host_swabbed)
		di = 1 - di;

	w0 = vrs.u[di * 2];
	w1 = vrs.u[di * 2 + 1];

#ifdef __BIG_ENDIAN
	*val = (w0 << 32) | w1;
#else
	*val = (w1 << 32) | w0;
#endif
	return 0;
}

Paul.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions
  2018-02-02  0:30 ` [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Paul Mackerras
@ 2018-02-03  0:02   ` joserz
  0 siblings, 0 replies; 5+ messages in thread
From: joserz @ 2018-02-03  0:02 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: lvivier, linuxppc-dev, kvm-ppc

On Fri, Feb 02, 2018 at 11:30:18AM +1100, Paul Mackerras wrote:
> On Thu, Feb 01, 2018 at 04:15:38PM -0200, Jose Ricardo Ziviani wrote:
> > v5:
> >  - Fixed the mask off of the effective address
> > 
> > v4:
> >   - Changed KVM_MMIO_REG_VMX to 0xc0 because there are 64 VSX registers
> > 
> > v3:
> >   - Added Reported-by in the commit message
> > 
> > v2:
> >   - kvmppc_get_vsr_word_offset() moved back to its original place
> >   - EA AND ~0xF, following ISA.
> >   - fixed BE/LE cases
> > 
> > TESTS:
> > 
> > For testing purposes I wrote a small program that performs stvx/lvx using the
> > program's virtual memory and using MMIO. Load/Store into virtual memory is the
> > model I use to check if MMIO results are correct (because only MMIO is emulated
> > by KVM).
> 
> I'd be interested to see your test program because in my testing it's
> still not right, unfortunately.  Interestingly, it is right for the BE
> guest on LE host case.  However, with a LE guest on a LE host the two
> halves are swapped, both for lvx and stvx:

Absolutely, here it's: https://gist.github.com/jrziviani/a65e71c5d661bffa8afcd6710fedd520
It basically maps an IO region and also allocates some memory from the
program's address space. Then I store to/load from both addresses and
compare the results. Because only the mmio load/store are emulated, I
use the regular load/store as a model.

> 
> error in lvx at byte 0
> was: -> 62 69 70 77 7e 85 8c 93 2a 31 38 3f 46 4d 54 5b
> ref: -> 2a 31 38 3f 46 4d 54 5b 62 69 70 77 7e 85 8c 93
> error in stvx at byte 0
> was: -> 49 50 57 5e 65 6c 73 7a 11 18 1f 26 2d 34 3b 42
> ref: -> 11 18 1f 26 2d 34 3b 42 49 50 57 5e 65 6c 73 7a
> 
> The byte order within each 8-byte half is correct but the two halves
> are swapped.  ("was" is what was in memory and "ref" is the correct
> value.  For lvx it does lvx from emulated MMIO and stvx to ordinary
> memory, and for stvx it does lvx from ordinary memory and stvx to
> emulated MMIO.  In both cases the checking is done with a byte by byte
> comparison.)

The funny thing is that I still see it right in both cases, so I believe
that my test case is incorrect. Example (host LE, guest LE):

====> VR0 after lvx
(gdb) p $vr0
{uint128 = 0x1234567855554444aaaabbbb87654321, v4_float = {
    -1.72477726e-34, -3.03283305e-13, 1.46555735e+13, 5.69045661e-28},
  v4_int32 = {-2023406815, -1431651397, 1431651396, 305419896}, v8_int16 = {
    17185, -30875, -17477, -21846, 17476, 21845, 22136, 4660}, v16_int8 = {33,
    67, 101, -121, -69, -69, -86, -86, 68, 68, 85, 85, 120, 86, 52, 18}}


address: 0x10030010
0x1234567855554444aaaabbbb87654321

====> VR0 after lvx from MMIO
(gdb) p $vr0
$3 = {uint128 = 0x1234567855554444aaaabbbb87654321, v4_float = {
    -1.72477726e-34, -3.03283305e-13, 1.46555735e+13, 5.69045661e-28},
  v4_int32 = {-2023406815, -1431651397, 1431651396, 305419896}, v8_int16 = {
    17185, -30875, -17477, -21846, 17476, 21845, 22136, 4660}, v16_int8 = {33,
    67, 101, -121, -69, -69, -86, -86, 68, 68, 85, 85, 120, 86, 52, 18}}

io_address: 0x3fffb7f70000
0x1234567855554444aaaabbbb87654321

I only see it wrong when I mess with copy order:

	if (vcpu->arch.mmio_vmx_copy_nums == /*from 1 to */ 2) {
		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(2)] = lo;
		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(3)] = hi;
	} else if (vcpu->arch.mmio_vmx_copy_nums == /*from 2 to */ 1) {
		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(0)] = lo;
		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(1)] = hi;
	}

then I get:

address: 0x1003b530010
0x1234567855554444aaaabbbb87654321
io_address: 0x3fff811a0000
0xaaaabbbb876543211234567855554444

Anyway, your suggestion works great and it's a way more elegant/easy to
understand. I'll send the next version with it.

Thank you very much for your patience and hints that helped me to
understand KVM better. :-)

> 
> Paul.
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-02-03  0:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-01 18:15 [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Jose Ricardo Ziviani
2018-02-01 18:15 ` [PATCH v5 1/1] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions Jose Ricardo Ziviani
2018-02-02  2:55   ` Paul Mackerras
2018-02-02  0:30 ` [PATCH v5 0/1] Implements MMIO emulation for lvx/stvx instructions Paul Mackerras
2018-02-03  0:02   ` joserz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).