[PATCH RFC] powerpc: Implements MMIO emulation for lvx/stvx instructions

[PATCH RFC] powerpc: Implements MMIO emulation for lvx/stvx instructions

[Jose Ricardo Ziviani]

Hello!

This patch implements MMIO emulation for two instructions: lvx and stvx. I started to implement other instructions but I'd like to have this reviewed beforehand because this is my first patch here and I'll certainly have some rework/fixes :-).

Note: stvx is only storing 8 bytes, for some reason the code "vcpu->arch.paddr_accessed += run->mmio.len;", which adds the 8-byte offset after the first write is not making any difference (interesting that it works for load operations). I'm still investigating it but any idea about it will be appreciated.

Thank you very much,

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  |  32 +++++++
 arch/powerpc/kvm/powerpc.c            | 162 ++++++++++++++++++++++++++++++----
 5 files changed, 189 insertions(+), 17 deletions(-)

-- 
2.7.4

[PATCH RFC] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions

[Jose Ricardo Ziviani]

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

Instructions implemented: lvx, stvx

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  |  32 +++++++
 arch/powerpc/kvm/powerpc.c            | 162 ++++++++++++++++++++++++++++++----
 5 files changed, 189 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 8b3f123..5835163 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -697,6 +697,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;
@@ -807,6 +808,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	0x00a0
 
 #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 ba5fadd..c444d16 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 fa9ebae..ea9bf37 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 af83353..40fbc14 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,25 @@ 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.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.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 1a75c0b..dc3611b 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -828,23 +828,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
-static inline int kvmppc_get_vsr_dword_offset(int index)
-{
-	int offset;
-
-	if ((index != 0) && (index != 1))
-		return -1;
-
-#ifdef __BIG_ENDIAN
-	offset =  index;
-#else
-	offset = 1 - index;
-#endif
-
-	return offset;
-}
-
+#ifdef CONFIG_ALTIVEC
 static inline int kvmppc_get_vsr_word_offset(int index)
 {
 	int offset;
@@ -860,6 +844,40 @@ static inline int kvmppc_get_vsr_word_offset(int index)
 	return offset;
 }
 
+static inline void kvmppc_set_vmx_dword(struct kvm_vcpu *vcpu,
+		u64 gpr)
+{
+	int index = vcpu->arch.io_gpr & KVM_MMIO_REG_MASK;
+	u64 hi = gpr >> 32;
+	u64 lo = gpr & 0xffffffff;
+
+	if (vcpu->arch.mmio_vmx_copy_nums == 1) {
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(2)] = hi;
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(3)] = lo;
+	} else if (vcpu->arch.mmio_vmx_copy_nums == 2) {
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(0)] = hi;
+		VCPU_VSX_VR(vcpu, index).u[kvmppc_get_vsr_word_offset(1)] = lo;
+	}
+}
+#endif /* CONFIG_ALTIVEC */
+
+#ifdef CONFIG_VSX
+static inline int kvmppc_get_vsr_dword_offset(int index)
+{
+	int offset;
+
+	if ((index != 0) && (index != 1))
+		return -1;
+
+#ifdef __BIG_ENDIAN
+	offset =  index;
+#else
+	offset = 1 - index;
+#endif
+
+	return offset;
+}
+
 static inline void kvmppc_set_vsr_dword(struct kvm_vcpu *vcpu,
 	u64 gpr)
 {
@@ -1024,6 +1042,11 @@ static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu,
 			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();
 	}
@@ -1303,6 +1326,99 @@ 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 = EMULATE_DONE;
+
+	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.mmio_vmx_copy_nums--;
+	}
+	return emulated;
+}
+
+static inline int kvmppc_get_vmx_data(struct kvm_vcpu *vcpu, int rs, u64 *val)
+{
+
+	if (vcpu->arch.mmio_vmx_copy_nums == 1) {
+		*val = VCPU_VSX_VR(vcpu, rs).u[kvmppc_get_vsr_word_offset(2)];
+		*val = (*val << 32) | VCPU_VSX_VR(vcpu, rs).
+			u[kvmppc_get_vsr_word_offset(3)];
+		return 0;
+	} else if (vcpu->arch.mmio_vmx_copy_nums == 2) {
+		*val = VCPU_VSX_VR(vcpu, rs).u[kvmppc_get_vsr_word_offset(0)];
+		*val = (*val << 32) | VCPU_VSX_VR(vcpu, rs).
+			u[kvmppc_get_vsr_word_offset(1)];
+		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.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;
@@ -1423,6 +1539,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 			}
 		}
 #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;
 		int i;
-- 
2.7.4

Re: [PATCH RFC] KVM: PPC: Book3S: Add MMIO emulation for VMX instructions

[Paul Mackerras]

On Tue, Aug 29, 2017 at 07:18:02PM -0300, Jose Ricardo Ziviani wrote:
> This patch provides the MMIO load/store vector indexed
> X-Form emulation.
> 
> Instructions implemented: lvx, stvx
> 
> Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>

Thanks for the patch.  The basic outline of it looks fine.  The parts
where you are accessing the VMX register image in memory are a bit
hard to follow, and I'll have to check them in detail.

Longer term, I want to switch over to using the instruction emulation
infrastructure in arch/powerpc/lib/sstep.c that I have recently
expanded to handle almost all load and store instructions.  That will
reduce duplication and should simplify all of this code enormously.

Regards,
Paul.

Re: [PATCH RFC] powerpc: Implements MMIO emulation for lvx/stvx instructions

[Paul Mackerras]

On Tue, Aug 29, 2017 at 07:18:01PM -0300, Jose Ricardo Ziviani wrote:
> Hello!
> 
> This patch implements MMIO emulation for two instructions: lvx and stvx. I started to implement other instructions but I'd like to have this reviewed beforehand because this is my first patch here and I'll certainly have some rework/fixes :-).
> 
> Note: stvx is only storing 8 bytes, for some reason the code "vcpu->arch.paddr_accessed += run->mmio.len;", which adds the 8-byte offset after the first write is not making any difference (interesting that it works for load operations). I'm still investigating it but any idea about it will be appreciated.

The run structure is mmapped by userspace (i.e. QEMU) and can be
written by userspace between the first and the second exits to
userspace (you have to do two exits to userspace because you can only
transfer 8 bytes on each exit).  It's possible that userspace might be
clearing run->mmio.len.  In general it's better not to rely on
anything in *run (except of course the mmio_data for a MMIO read) when
we come in from userspace to the kernel.

Paul.

Re: [PATCH RFC] powerpc: Implements MMIO emulation for lvx/stvx instructions

[joserz]

On Wed, Aug 30, 2017 at 07:45:17PM +1000, Paul Mackerras wrote:
> On Tue, Aug 29, 2017 at 07:18:01PM -0300, Jose Ricardo Ziviani wrote:
> > Hello!
> > 
> > This patch implements MMIO emulation for two instructions: lvx and stvx. I started to implement other instructions but I'd like to have this reviewed beforehand because this is my first patch here and I'll certainly have some rework/fixes :-).
> > 
> > Note: stvx is only storing 8 bytes, for some reason the code "vcpu->arch.paddr_accessed += run->mmio.len;", which adds the 8-byte offset after the first write is not making any difference (interesting that it works for load operations). I'm still investigating it but any idea about it will be appreciated.
> 
> The run structure is mmapped by userspace (i.e. QEMU) and can be
> written by userspace between the first and the second exits to
> userspace (you have to do two exits to userspace because you can only
> transfer 8 bytes on each exit).  It's possible that userspace might be
> clearing run->mmio.len.  In general it's better not to rely on
> anything in *run (except of course the mmio_data for a MMIO read) when
> we come in from userspace to the kernel.
> 
> Paul.
> 

Hello Paul,

My bad, actually it works. I was mmap'ping an address that doesn't allow 16-byte writing access. After mmap'ping a higher address (of the same device) I was able to perform 16-byte read/write.

== before stvx ==

  (gdb) info registers vr0
  vr0  {uint128 = 0x000000001234567800000000abcdef09, ...}

  (gdb) info registers r9
  r9 0x3fffb7c90010

  (gdb) x /4wx 0x3fffb7c90010
  0x3fffb7c90010: 0x00000000 0x00000000 0x00000000 0x00000000

  (gdb) info registers r28
  r28 0x0 

stvx    v0,r28,r9

== after stvx ==

  (gdb) x /4wx 0x3fffb7c90010
  0x3fffb7c90010: 0x12345678 0x00000000 0xabcdef09 0x00000000

== before lvx ==

  (gdb) info registers vr10
  vr10 {uint128 = 0x00000000000000000000000000000000,...}

lvx     v10,r28,r9

== after lvx ==

  (gdb) info registers vr10
  vr10 {uint128 = 0x000000001234567800000000abcdef09,...}

If you think it's ok I'll submit this patch without the RFC.

Thank you very much!

Ziviani