Add userpace tlb access

This version of the userspace tlb access has some changes out of discussions with Zhang Wei responsible for the e500 work and Hollis for ppc440. It now transports only a single tlb entry per get/set and uses an index specifier that every architecture can interpret as needed e.g. e500 could map tlb0/tlb1 in the high bits and the array index in them in the low bits while ppc440 only needs a linear index. The right vcpu is already selected since this is a vcpu ioctl.
Internally the selction of the right tlb layout in kernel e.g. for set/get_tlb or in userspace for kvm_dump_tlb is now based on the pvr that selects the defines the guest type.

Subject: [PATCH] [3/4] Add userpace tlb access
From: Christian Ehrhardt <[email protected]>

This adds userspace tlb access transporting a single tlb entry to/from userspace. It has an index variable passed with the call to specify the tlb entry to get/set. For the guest tlb get&set is supported while the shadow tlb can only be read from userspace.

include/asm-powerpc/kvm.h +1 -0

@@ -21,6 +21,7 @@
 #define __POWERPC_KVM_H__
 
 #include <asm/types.h>
+#include <asm/mmu-44x.h>
 
 struct kvm_regs {
 	__u32 pc;

Quoted 14 lines · On Wed, 2008-01-30 at 13:31 +0100, [email protected] wrote:

This version of the userspace tlb access has some changes out of discussions with Zhang Wei responsible for the e500 work and Hollis for ppc440. It now transports only a single tlb entry per get/set and uses an index specifier that every architecture can interpret as needed e.g. e500 could map tlb0/tlb1 in the high bits and the array index in them in the low bits while ppc440 only needs a linear index. The right vcpu is already selected since this is a vcpu ioctl.
Internally the selction of the right tlb layout in kernel e.g. for set/get_tlb or in userspace for kvm_dump_tlb is now based on the pvr that selects the defines the guest type.

Subject: [PATCH] [3/4] Add userpace tlb access
From: Christian Ehrhardt <[email protected]>

Quoted 4 lines

This adds userspace tlb access transporting a single tlb entry to/from userspace. It has an index variable passed with the call to specify the tlb entry to get/set. For the guest tlb get&set is supported while the shadow tlb can only be read from userspace.

Didn't we agree not to export the shadow TLB?

...

Quoted 15 lines

include/asm-powerpc/kvm.h +1 -0

@@ -21,6 +21,7 @@
 #define __POWERPC_KVM_H__
 
 #include <asm/types.h>
+#include <asm/mmu-44x.h>
 
 struct kvm_regs {
 	__u32 pc;

@@ -53,6 +54,22 @@ struct kvm_fpu {
 struct kvm_fpu {
 };
 

include/asm-powerpc/kvm.h struct kvm_fpu { · line 62

Quoted 6 lines

include/asm-powerpc/kvm.h +6 -0

+struct tlbe {
+	__u32 tid; /* Only the low 8 bits are used. */
+	__u32 word0;
+	__u32 word1;
+	__u32 word2;
+};

This definitely must be renamed.

include/asm-powerpc/kvm.h struct kvm_fpu { · line 71

Quoted 8 lines

include/asm-powerpc/kvm.h +8 -0

+struct kvm_tlbe {
+	__u32 index;
+	union {
+		struct tlbe tlbe_ppc440;
+		/* Fix the size of the union (hopefully) */
+		char padding[32];
+	};
+};

Are you sure 32 bytes will be large enough? I think Freescale uses a ton of MAS registers for TLB insertion.

Quoted 2 lines · Hollis Blanchard

Quoted 1 line · On Wed, 2008-01-30 at 13:31 +0100, [email protected] wrote:

Quoted 7 lines

include/asm-powerpc/kvm.h struct kvm_fpu { · line 62

Quoted 6 lines

include/asm-powerpc/kvm.h +6 -0

+struct tlbe {
+	__u32 tid; /* Only the low 8 bits are used. */
+	__u32 word0;
+	__u32 word1;
+	__u32 word2;
+};

This definitely must be renamed.

I'll add a renaming patch on top when I resend the patches next time.

Quoted 2 lines · Hollis Blanchard

Didn't we agree not to export the shadow TLB?

The last agreement I found was to export shadow TLB read only which is what I have here. But it's easy to remove if needed ;-)
So do you think it is really completely useless e.g. on a tlb dump ? If yes we can remove it, but if it can give us even a small benefit we should keep the read only access.

Quoted 11 lines

include/asm-powerpc/kvm.h struct kvm_fpu { · line 71

Quoted 8 lines

include/asm-powerpc/kvm.h +8 -0

+struct kvm_tlbe {
+	__u32 index;
+	union {
+		struct tlbe tlbe_ppc440;
+		/* Fix the size of the union (hopefully) */
+		char padding[32];
+	};
+};

Are you sure 32 bytes will be large enough? I think Freescale uses a ton of MAS registers for TLB insertion.

Well I thought it should be enough but we can use 64bit just to be safer.

-----Original Message-----
From: Christian Ehrhardt [mailto:[email protected]]

Quoted 2 lines · Christian Ehrhardt

Quoted 1 line · Hollis Blanchard

On Wed, 2008-01-30 at 13:31 +0100,

Quoted 1 line · [email protected] wrote:

Quoted 1 line

Quoted 1 line

[...]

Quoted 9 lines

Quoted 7 lines

include/asm-powerpc/kvm.h struct kvm_fpu { · line 62

Quoted 6 lines

include/asm-powerpc/kvm.h +6 -0

+struct tlbe {
+	__u32 tid; /* Only the low 8 bits are used. */
+	__u32 word0;
+	__u32 word1;
+	__u32 word2;
+};

This definitely must be renamed.

I'll add a renaming patch on top when I resend the patches

next time.

Well I skipped that to have time for virtio, we can do it anytime between now and the merge of e500.
The main question before just renaming these structures is how do we want to integrate multiple sub-architectures here - ifdefs, including separate headers, union in tlbe struct, ... - is this already decided ?

Maybe it is ok to let it as is atm and wait for Zhang Wei's merge which will bring in his tlbe style anyway.

-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Thursday, January 31, 2008 10:57 PM
To: kvm-ppc-devel
Cc: Zhang Wei; Hollis Blanchard; [email protected]; [email protected]
Subject: [PATCH] [3/4] Add userpace tlb access

Subject: [PATCH] [3/4] Add userpace tlb access
From: Christian Ehrhardt <[email protected]>

This adds userspace tlb access transporting a single tlb entry to/from userspace. It has an index variable passed with the call to specify the tlb
entry to get/set. For the guest tlb get&set is supported while the shadow
tlb can only be read from userspace.

Signed-off-by Christian Ehrhardt <[email protected]>

---

 arch/powerpc/kvm/powerpc.c     |  101 
++++++++++++++++++++++++++++++++++-------
 include/asm-powerpc/kvm.h      |   17 ++++++
 include/asm-powerpc/kvm_host.h |    7 --
 include/linux/kvm.h            |    3 +
 4 files changed, 105 insertions(+), 23 deletions(-)

arch/powerpc/kvm/powerpc.c +0 -0

@@ -512,24 +512,9 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *
 /* Initial guest state: 16MB mapping 0 -> 0, PC = 0, MSR = 

0, R1 = 16MB */
int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
{

arch/powerpc/kvm/powerpc.c +0 -15

-	struct tlbe *tlbe = &vcpu->arch.guest_tlb[0];
-
-	tlbe->tid = 0;
-	tlbe->word0 = PPC44x_TLB_16M | PPC44x_TLB_VALID;
-	tlbe->word1 = 0;
-	tlbe->word2 = PPC44x_TLB_SX | PPC44x_TLB_SW | PPC44x_TLB_SR;
-
-	tlbe++;
-	tlbe->tid = 0;
-	tlbe->word0 = 0xef600000 | PPC44x_TLB_4K | PPC44x_TLB_VALID;
-	tlbe->word1 = 0xef600000;
-	tlbe->word2 = PPC44x_TLB_SX | PPC44x_TLB_SW | PPC44x_TLB_SR
-	              | PPC44x_TLB_I | PPC44x_TLB_G;
-
 	vcpu->arch.pc = 0;
 	vcpu->arch.guest_msr = 0;
 	vcpu->arch.shadow_msr = MSR_PR|MSR_EE|MSR_IS|MSR_DS;
-	vcpu->arch.gpr[1] = (16<<20) - 8; /* -8 for the 

callee-save LR slot */

/* Eye-catching number so we know if the guest takes an
interrupt

arch/powerpc/kvm/powerpc.c +37 -1

 	 * before it's programmed its own IVPR. */

@@ -764,7 +749,7 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_
 }
 
 long kvm_arch_vcpu_ioctl(struct file *filp,
-                        unsigned int ioctl, unsigned long arg)
+			unsigned int ioctl, unsigned long arg)
 {
 	struct kvm_vcpu *vcpu = filp->private_data;
 	void __user *argp = (void __user *)arg;

@@ -777,6 +762,90 @@ long kvm_arch_vcpu_ioctl(struct file *fi
 		if (copy_from_user(&irq, argp, sizeof(irq)))
 			goto out;
 		r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
+		break;
+	}
+	case KVM_GET_GUEST_TLB: {
+		struct kvm_vcpu *vcpu = filp->private_data;
+		struct kvm_tlbe __user *utlbe = (struct 
 kvm_tlbe __user *)arg;
+		__u64 index;
+
+		r = copy_from_user(&index, &(utlbe->index), 
 sizeof(__u64));
+		if (r)
+			return r;
+
+		switch (vcpu->arch.pvr) {
+		case KVM_PPC_PVR_PPC440EP: {
+			if (unlikely(index > (PPC44x_TLB_SIZE-1))) {
+				r = -EINVAL;
+				break;
+			}
+			r = copy_to_user(&(utlbe->tlbe_ppc440),
+					 &(vcpu->arch.guest_tlb[index]),
+					 sizeof(struct tlbe));
+			if (r)
+				return r;
+			break;
+		}
+		default: {
+			r = -ENOTSUPP;
+		}
+		}
+		break;
+	}
+	case KVM_SET_GUEST_TLB: {
+		struct kvm_vcpu *vcpu = filp->private_data;
+		struct kvm_tlbe __user *utlbe = (struct 
 kvm_tlbe __user *)arg;
+		__u64 index;
+
+		r = copy_from_user(&index, &(utlbe->index), 

sizeof(__u64));

arch/powerpc/kvm/powerpc.c +10 -0

+		if (r)
+			return r;
+
+		switch (vcpu->arch.pvr) {
+		case KVM_PPC_PVR_PPC440EP: {
+			if (unlikely(index > (PPC44x_TLB_SIZE-1))) {
+				r = -EINVAL;
+				break;
+			}
+			r = 

copy_from_user(&(vcpu->arch.guest_tlb[index]),

arch/powerpc/kvm/powerpc.c +15 -0

+					   &(utlbe->tlbe_ppc440),
+					   sizeof(struct tlbe));
+			if (r)
+				return r;
+			break;
+		}
+		default: {
+			r = -ENOTSUPP;
+		}
+		}
+		break;
+	}
+	case KVM_GET_SHADOW_TLB: {
+		struct kvm_vcpu *vcpu = filp->private_data;
+		struct kvm_tlbe __user *utlbe = (struct 

kvm_tlbe __user *)arg;

arch/powerpc/kvm/powerpc.c +3 -0

+		__u64 index;
+
+		r = copy_from_user(&index, &(utlbe->index), 

sizeof(__u64));

arch/powerpc/kvm/powerpc.c +11 -0

+		if (r)
+			return r;
+
+		switch (vcpu->arch.pvr) {
+		case KVM_PPC_PVR_PPC440EP: {
+			if (unlikely(index > (PPC44x_TLB_SIZE-1))) {
+				r = -EINVAL;
+				break;
+			}
+			r = copy_to_user(&(utlbe->tlbe_ppc440),
+					 

&(vcpu->arch.shadow_tlb[index]),

arch/powerpc/kvm/powerpc.c +9 -0

+					 sizeof(struct tlbe));
+			if (r)
+				return r;
+			break;
+		}
+		default: {
+			r = -ENOTSUPP;
+		}
+		}
 		break;
 	}
 	case KVM_SET_GUEST_PVR: {

include/asm-powerpc/kvm.h +17 -0

@@ -21,6 +21,7 @@
 #define __POWERPC_KVM_H__
 
 #include <asm/types.h>
+#include <asm/mmu-44x.h>
 
 struct kvm_regs {
 	__u32 pc;

@@ -53,6 +54,22 @@ struct kvm_fpu {
 struct kvm_fpu {
 };
 
+struct tlbe {
+	__u32 tid; /* Only the low 8 bits are used. */
+	__u32 word0;
+	__u32 word1;
+	__u32 word2;
+};
+
+struct kvm_tlbe {
+	__u64 index;
+	union {
+		struct tlbe tlbe_ppc440;
+		/* Fix the size of the union (hopefully) */
+		char padding[32];
+	};
+};
+
 /*
  * guest ident based on virtual pvr's to differ between cpu types
  *  this may later on be extended to appear and behave as real pvr's

include/asm-powerpc/kvm_host.h +0 -7

@@ -39,13 +39,6 @@ struct kvm_vcpu_stat {
 	u32 mmio_exits;
 	u32 signal_exits;
 	u32 light_exits;
-};
-
-struct tlbe {
-	u32 tid; /* Only the low 8 bits are used. */
-	u32 word0;
-	u32 word1;
-	u32 word2;
 };
 
 struct kvm_arch {

include/linux/kvm.h +3 -0

@@ -291,5 +291,8 @@ struct kvm_vapic_addr {
 #define KVM_SET_VAPIC_ADDR        _IOW(KVMIO,  0x93, struct 
 kvm_vapic_addr)
 #define KVM_SET_GUEST_PVR         _IOW(KVMIO,  0x94, struct kvm_pvr)
 #define KVM_GET_GUEST_PVR         _IOR(KVMIO,  0x95, struct kvm_pvr)
+#define KVM_GET_GUEST_TLB         _IOR(KVMIO,  0x96, struct kvm_tlbe)
+#define KVM_SET_GUEST_TLB         _IOW(KVMIO,  0x97, struct kvm_tlbe)
+#define KVM_GET_SHADOW_TLB        _IOR(KVMIO,  0x98, struct kvm_tlbe)
 

#endif