[kvm-ppc-devel] [PATCH] [3/4] Add userpace tlb access

[kvm-ppc-devel] [PATCH] [3/4] Add userpace tlb access

[ehrhardt]

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 <ehrhardt@linux.vnet.ibm.com>

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 <ehrhardt@linux.vnet.ibm.com>
---

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

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -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)
 {
-	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
 	 * 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;
+		__u32 index;
+
+		r = copy_from_user(&index, &(utlbe->index), sizeof(__u32));
+		if (r)
+			return r;
+
+		switch (vcpu->kvm->arch.pvr) {
+		case KVM_PPC_PVR_PPC44X: {
+			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;
+		__u32 index;
+
+		r = copy_from_user(&index, &(utlbe->index), sizeof(__u32));
+		if (r)
+			return r;
+
+		switch (vcpu->kvm->arch.pvr) {
+		case KVM_PPC_PVR_PPC44X: {
+			if (unlikely(index > (PPC44x_TLB_SIZE-1))) {
+				r = -EINVAL;
+				break;
+			}
+			r = copy_from_user(&(vcpu->arch.guest_tlb[index]),
+					   &(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;
+		__u32 index;
+
+		r = copy_from_user(&index, &(utlbe->index), sizeof(__u32));
+		if (r)
+			return r;
+
+		switch (vcpu->kvm->arch.pvr) {
+		case KVM_PPC_PVR_PPC44X: {
+			if (unlikely(index > (PPC44x_TLB_SIZE-1))) {
+				r = -EINVAL;
+				break;
+			}
+			r = copy_to_user(&(utlbe->tlbe_ppc440),
+					 &(vcpu->arch.shadow_tlb[index]),
+					 sizeof(struct tlbe));
+			if (r)
+				return r;
+			break;
+		}
+		default: {
+			r = -ENOTSUPP;
+		}
+		}
 		break;
 	}
 	default:
diff --git a/include/asm-powerpc/kvm.h b/include/asm-powerpc/kvm.h
--- a/include/asm-powerpc/kvm.h
+++ b/include/asm-powerpc/kvm.h
@@ -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 {
+	__u32 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
diff --git a/include/asm-powerpc/kvm_host.h b/include/asm-powerpc/kvm_host.h
--- a/include/asm-powerpc/kvm_host.h
+++ b/include/asm-powerpc/kvm_host.h
@@ -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 {
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -292,4 +292,8 @@ struct kvm_vapic_addr {
 /* Available with KVM_CAP_VAPIC */
 #define KVM_SET_VAPIC_ADDR        _IOW(KVMIO,  0x93, struct kvm_vapic_addr)
 
+#define KVM_GET_GUEST_TLB         _IOR(KVMIO,  0x94, struct kvm_tlbe)
+#define KVM_SET_GUEST_TLB         _IOW(KVMIO,  0x95, struct kvm_tlbe)
+#define KVM_GET_SHADOW_TLB        _IOR(KVMIO,  0x96, struct kvm_tlbe)
+
 #endif

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [PATCH] [3/4] Add userpace tlb access

[Hollis Blanchard]

On Wed, 2008-01-30 at 13:31 +0100, ehrhardt@linux.vnet.ibm.com 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 <ehrhardt@linux.vnet.ibm.com>
> 
> 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?

...
> diff --git a/include/asm-powerpc/kvm.h b/include/asm-powerpc/kvm.h
> --- a/include/asm-powerpc/kvm.h
> +++ b/include/asm-powerpc/kvm.h
> @@ -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;
> +};

This definitely must be renamed.

> +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.

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [PATCH] [3/4] Add userpace tlb access

[Christian Ehrhardt]

Hollis Blanchard wrote:
> On Wed, 2008-01-30 at 13:31 +0100, ehrhardt@linux.vnet.ibm.com 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 <ehrhardt@linux.vnet.ibm.com>
>>
>> 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?

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.
 
> ...
>> diff --git a/include/asm-powerpc/kvm.h b/include/asm-powerpc/kvm.h
>> --- a/include/asm-powerpc/kvm.h
>> +++ b/include/asm-powerpc/kvm.h
>> @@ -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;
>> +};
> 
> This definitely must be renamed.

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

>> +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.

-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

[kvm-ppc-devel] [PATCH] [3/4] Add userpace tlb access

[ehrhardt]

Subject: [PATCH] [3/4] Add userpace tlb access
From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>

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 <ehrhardt@linux.vnet.ibm.com>
---

 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(-)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -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)
 {
-	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
 	 * 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));
+		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]),
+					   &(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;
+		__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.shadow_tlb[index]),
+					 sizeof(struct tlbe));
+			if (r)
+				return r;
+			break;
+		}
+		default: {
+			r = -ENOTSUPP;
+		}
+		}
 		break;
 	}
 	case KVM_SET_GUEST_PVR: {
diff --git a/include/asm-powerpc/kvm.h b/include/asm-powerpc/kvm.h
--- a/include/asm-powerpc/kvm.h
+++ b/include/asm-powerpc/kvm.h
@@ -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
diff --git a/include/asm-powerpc/kvm_host.h b/include/asm-powerpc/kvm_host.h
--- a/include/asm-powerpc/kvm_host.h
+++ b/include/asm-powerpc/kvm_host.h
@@ -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 {
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [PATCH] [3/4] Add userpace tlb access

[Christian Ehrhardt]

Christian Ehrhardt wrote:
> Hollis Blanchard wrote:
>> On Wed, 2008-01-30 at 13:31 +0100, ehrhardt@linux.vnet.ibm.com wrote:
>>>
[...]
>>> +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.

-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [PATCH] [3/4] Add userpace tlb access

[Hollis Blanchard]

On Thu, 2008-01-31 at 09:32 +0100, Christian Ehrhardt wrote:
> Hollis Blanchard wrote:
> > 
> > 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.

OK, we can keep it.

We will need custom debug tool code to understand any TLB dump anyways.

> >> +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.

I guess you mean 64 bytes. Wei, how many bytes do you think are needed
to save current and future TLB entries?

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [PATCH] [3/4] Add userpace tlb access

[Zhang Wei]

 

> -----Original Message-----
> From: Christian Ehrhardt [mailto:ehrhardt@linux.vnet.ibm.com] 
> 
> Christian Ehrhardt wrote:
> > Hollis Blanchard wrote:
> >> On Wed, 2008-01-30 at 13:31 +0100, 
> ehrhardt@linux.vnet.ibm.com wrote:
> >>>
> [...]
> >>> +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.
> 

Yep, it's different than E500 tlbe. :) I need to align them.

Cheers!
Wei.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

Re: [kvm-ppc-devel] [PATCH] [3/4] Add userpace tlb access

[Zhang Wei]

The same as [PATCH 1/4]. Could you make a rebase? :)

Cheers!
Wei

> -----Original Message-----
> From: ehrhardt@linux.vnet.ibm.com 
> [mailto:ehrhardt@linux.vnet.ibm.com] 
> Sent: Thursday, January 31, 2008 10:57 PM
> To: kvm-ppc-devel
> Cc: Zhang Wei; Hollis Blanchard; jyoung5@us.ibm.com; 
> ehrhardt@linux.vnet.ibm.com
> Subject: [PATCH] [3/4] Add userpace tlb access
> 
> Subject: [PATCH] [3/4] Add userpace tlb access
> From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
> 
> 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 <ehrhardt@linux.vnet.ibm.com>
> ---
> 
>  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(-)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -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)
>  {
> -	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
>  	 * 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));
> +		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]),
> +					   &(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;
> +		__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.shadow_tlb[index]),
> +					 sizeof(struct tlbe));
> +			if (r)
> +				return r;
> +			break;
> +		}
> +		default: {
> +			r = -ENOTSUPP;
> +		}
> +		}
>  		break;
>  	}
>  	case KVM_SET_GUEST_PVR: {
> diff --git a/include/asm-powerpc/kvm.h b/include/asm-powerpc/kvm.h
> --- a/include/asm-powerpc/kvm.h
> +++ b/include/asm-powerpc/kvm.h
> @@ -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
> diff --git a/include/asm-powerpc/kvm_host.h 
> b/include/asm-powerpc/kvm_host.h
> --- a/include/asm-powerpc/kvm_host.h
> +++ b/include/asm-powerpc/kvm_host.h
> @@ -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 {
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -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
> 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel