Re: [PATCH 4/5] kvmppc: Translate eaddr for fsl_booke mmu

2009-08-29 Thread Alexander Graf


On 24.08.2009, at 19:33, Hollis Blanchard wrote:


On Mon, 2009-08-24 at 10:44 +0800, Liu Yu-B13201 wrote:



IMHO userspace should do the translation and do an ioctl to
fetch the
required information (soft TLB cache / SLB / SDR1) so we can
reuse the
existing qemu infrastructure.



BOOK3S has mmu implement in qemu, but BOOKE doesn't.


Even if it did, I'd be skeptical. But you're right that that's a
critical point: as things stand today, only KVM (not qemu) emulates  
the

Book E MMU.


Well, that's the whole point I was trying to make.

If we had the booke mmu implemented in qemu debugging would be a lot  
easier I guess. Also it'd benefit people who for whatever reason want  
to emulate a booke cpu instead of virtualizing one, maybe because  
their development machines are x86 ;-).


If I remember the x86 KVM architecture correctly, CR3 (the register  
holding a phys addr to the current pagetable) gets pulled by userspace  
and then qemu does the complete translation based on that information.


So yes, we do have two separate MMU implementations for x86 here, but  
that's a good thing IMHO, because it makes it easier to spot bugs and  
find out where things went wrong.


So my suggestion is: Implement the BOOKE MMU in Qemu, make an IOCTL to  
pull the TLB to userspace and thus make life easier for everyone.


Alex

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 4/5] kvmppc: Translate eaddr for fsl_booke mmu

2009-08-23 Thread Liu Yu-B13201
 

 -Original Message-
 From: Alexander Graf [mailto:a...@csgraf.de] 
 Sent: Friday, August 21, 2009 8:39 PM
 To: Liu Yu-B13201
 Cc: Hollis Blanchard; qemu-de...@nongnu.org; 
 kvm-ppc@vger.kernel.org; jan.kis...@siemens.com; 
 froy...@codesourcery.com
 Subject: Re: [PATCH 4/5] kvmppc: Translate eaddr for fsl_booke mmu
 
 
 Am 20.08.2009 um 12:21 schrieb Liu Yu-B13201 yu@freescale.com:
 
 
 
  -Original Message-
  From: Hollis Blanchard [mailto:holl...@us.ibm.com]
  Sent: Thursday, August 20, 2009 6:51 AM
  To: Liu Yu-B13201
  Cc: qemu-de...@nongnu.org; kvm-ppc@vger.kernel.org;
  jan.kis...@siemens.com; froy...@codesourcery.com; Alexander Graf
  Subject: Re: [PATCH 4/5] kvmppc: Translate eaddr for fsl_booke mmu
 
  On Tue, 2009-08-04 at 17:36 +0800, Liu Yu wrote:
  Signed-off-by: Liu Yu yu@freescale.com
  ---
  target-ppc/helper.c |   17 +++--
  1 files changed, 15 insertions(+), 2 deletions(-)
 
  diff --git a/target-ppc/helper.c b/target-ppc/helper.c
  index 6eca2e5..07e56a4 100644
  --- a/target-ppc/helper.c
  +++ b/target-ppc/helper.c
  @@ -22,6 +22,7 @@
  #include string.h
  #include inttypes.h
  #include signal.h
  +#include linux/kvm.h
 
  #include cpu.h
  #include exec-all.h
  @@ -1325,8 +1326,20 @@ static always_inline int
  check_physical (CPUState *env, mmu_ctx_t *ctx,
  cpu_abort(env, MPC8xx MMU model is not implemented\n);
  break;
  case POWERPC_MMU_BOOKE_FSL:
  -/* XXX: TODO */
  -cpu_abort(env, BookE FSL MMU model not implemented\n);
  +if (kvm_enabled()) {
  +struct kvm_translation tr;
  +
  +/* For now we only debug guest kernel */
  +tr.linear_address = eaddr;
  +ret = kvm_vcpu_ioctl(env, KVM_TRANSLATE, tr);
  +if (ret  0)
  +return ret;
  +
  +ctx-raddr = tr.physical_address;
  +} else {
  +/* XXX: TODO */
  +cpu_abort(env, BookE FSL MMU model not
  implemented\n);
  +}
  break;
  default:
  cpu_abort(env, Unknown or invalid MMU model\n);
 
  One objection: the comment is a little obscure. I think what you're
  really saying is in Linux guests, kernel addresses should 
 always be
  covered by TLB1, which means for those addresses we can expect this
  ioctl to succeed. However, since you need to handle failures
  anyways, I
  think you should remove the comment entirely.
 
  As BOOKE mmu translation needs AS + PID + address,
  The infomations we pass to kvmppc here only count in address and set
  AS=0, PID=0.
  Which indicates that it's a kernel address.
 
  If want to translate user space address, one way is read registers  
  from
  kvmppc at first
  and then pass the correct AS and PID to translator.
  As we don't need to debug guest userspace, for simplicity, 
 I didn't do
  that.
 
 
  Second, (and this isn't an objection but rather a question)
  do you have
  any better ideas for struct kvm_translation? It only really
  makes sense
  for x86. We don't need to stick with it.
 
 
  Hrr.. We need to combine AS, PID and 32-bit addr into 64-bit linear
  address. it's not that convenient.
  But except that, I am not sure if there is strong requirement to  
  change
  it...
 
  BOOK3S KVM has more work in qemu (openbios, vga etc.),
  Maybe Alex has some suggestion?
 
 
 What does that do again? Enable userspace to do EA to PA translation?
 
 IMHO userspace should do the translation and do an ioctl to 
 fetch the  
 required information (soft TLB cache / SLB / SDR1) so we can 
 reuse the  
 existing qemu infrastructure.
 

BOOK3S has mmu implement in qemu, but BOOKE doesn't.

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] kvmppc: Translate eaddr for fsl_booke mmu

2009-08-21 Thread Alexander Graf


Am 20.08.2009 um 12:21 schrieb Liu Yu-B13201 yu@freescale.com:





-Original Message-
From: Hollis Blanchard [mailto:holl...@us.ibm.com]
Sent: Thursday, August 20, 2009 6:51 AM
To: Liu Yu-B13201
Cc: qemu-de...@nongnu.org; kvm-ppc@vger.kernel.org;
jan.kis...@siemens.com; froy...@codesourcery.com; Alexander Graf
Subject: Re: [PATCH 4/5] kvmppc: Translate eaddr for fsl_booke mmu

On Tue, 2009-08-04 at 17:36 +0800, Liu Yu wrote:

Signed-off-by: Liu Yu yu@freescale.com
---
target-ppc/helper.c |   17 +++--
1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 6eca2e5..07e56a4 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -22,6 +22,7 @@
#include string.h
#include inttypes.h
#include signal.h
+#include linux/kvm.h

#include cpu.h
#include exec-all.h
@@ -1325,8 +1326,20 @@ static always_inline int

check_physical (CPUState *env, mmu_ctx_t *ctx,

cpu_abort(env, MPC8xx MMU model is not implemented\n);
break;
case POWERPC_MMU_BOOKE_FSL:
-/* XXX: TODO */
-cpu_abort(env, BookE FSL MMU model not implemented\n);
+if (kvm_enabled()) {
+struct kvm_translation tr;
+
+/* For now we only debug guest kernel */
+tr.linear_address = eaddr;
+ret = kvm_vcpu_ioctl(env, KVM_TRANSLATE, tr);
+if (ret  0)
+return ret;
+
+ctx-raddr = tr.physical_address;
+} else {
+/* XXX: TODO */
+cpu_abort(env, BookE FSL MMU model not

implemented\n);

+}
break;
default:
cpu_abort(env, Unknown or invalid MMU model\n);


One objection: the comment is a little obscure. I think what you're
really saying is in Linux guests, kernel addresses should always be
covered by TLB1, which means for those addresses we can expect this
ioctl to succeed. However, since you need to handle failures
anyways, I
think you should remove the comment entirely.


As BOOKE mmu translation needs AS + PID + address,
The infomations we pass to kvmppc here only count in address and set
AS=0, PID=0.
Which indicates that it's a kernel address.

If want to translate user space address, one way is read registers  
from

kvmppc at first
and then pass the correct AS and PID to translator.
As we don't need to debug guest userspace, for simplicity, I didn't do
that.



Second, (and this isn't an objection but rather a question)
do you have
any better ideas for struct kvm_translation? It only really
makes sense
for x86. We don't need to stick with it.



Hrr.. We need to combine AS, PID and 32-bit addr into 64-bit linear
address. it's not that convenient.
But except that, I am not sure if there is strong requirement to  
change

it...

BOOK3S KVM has more work in qemu (openbios, vga etc.),
Maybe Alex has some suggestion?



What does that do again? Enable userspace to do EA to PA translation?

IMHO userspace should do the translation and do an ioctl to fetch the  
required information (soft TLB cache / SLB / SDR1) so we can reuse the  
existing qemu infrastructure.


Alex
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] kvmppc: Translate eaddr for fsl_booke mmu

2009-08-19 Thread Hollis Blanchard
On Tue, 2009-08-04 at 17:36 +0800, Liu Yu wrote:
 Signed-off-by: Liu Yu yu@freescale.com
 ---
  target-ppc/helper.c |   17 +++--
  1 files changed, 15 insertions(+), 2 deletions(-)
 
 diff --git a/target-ppc/helper.c b/target-ppc/helper.c
 index 6eca2e5..07e56a4 100644
 --- a/target-ppc/helper.c
 +++ b/target-ppc/helper.c
 @@ -22,6 +22,7 @@
  #include string.h
  #include inttypes.h
  #include signal.h
 +#include linux/kvm.h
 
  #include cpu.h
  #include exec-all.h
 @@ -1325,8 +1326,20 @@ static always_inline int check_physical (CPUState 
 *env, mmu_ctx_t *ctx,
  cpu_abort(env, MPC8xx MMU model is not implemented\n);
  break;
  case POWERPC_MMU_BOOKE_FSL:
 -/* XXX: TODO */
 -cpu_abort(env, BookE FSL MMU model not implemented\n);
 +if (kvm_enabled()) {
 +struct kvm_translation tr;
 +
 +/* For now we only debug guest kernel */
 +tr.linear_address = eaddr;
 +ret = kvm_vcpu_ioctl(env, KVM_TRANSLATE, tr);
 +if (ret  0)
 +return ret;
 +
 +ctx-raddr = tr.physical_address;
 +} else {
 +/* XXX: TODO */
 +cpu_abort(env, BookE FSL MMU model not implemented\n);
 +}
  break;
  default:
  cpu_abort(env, Unknown or invalid MMU model\n);

One objection: the comment is a little obscure. I think what you're
really saying is in Linux guests, kernel addresses should always be
covered by TLB1, which means for those addresses we can expect this
ioctl to succeed. However, since you need to handle failures anyways, I
think you should remove the comment entirely.

Second, (and this isn't an objection but rather a question) do you have
any better ideas for struct kvm_translation? It only really makes sense
for x86. We don't need to stick with it.

-- 
Hollis Blanchard
IBM Linux Technology Center


--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/5] kvmppc: Translate eaddr for fsl_booke mmu

2009-08-04 Thread Liu Yu
Signed-off-by: Liu Yu yu@freescale.com
---
 target-ppc/helper.c |   17 +++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 6eca2e5..07e56a4 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -22,6 +22,7 @@
 #include string.h
 #include inttypes.h
 #include signal.h
+#include linux/kvm.h
 
 #include cpu.h
 #include exec-all.h
@@ -1325,8 +1326,20 @@ static always_inline int check_physical (CPUState *env, 
mmu_ctx_t *ctx,
 cpu_abort(env, MPC8xx MMU model is not implemented\n);
 break;
 case POWERPC_MMU_BOOKE_FSL:
-/* XXX: TODO */
-cpu_abort(env, BookE FSL MMU model not implemented\n);
+if (kvm_enabled()) {
+struct kvm_translation tr;
+
+/* For now we only debug guest kernel */
+tr.linear_address = eaddr;
+ret = kvm_vcpu_ioctl(env, KVM_TRANSLATE, tr);
+if (ret  0)
+return ret;
+
+ctx-raddr = tr.physical_address;
+} else {
+/* XXX: TODO */
+cpu_abort(env, BookE FSL MMU model not implemented\n);
+}
 break;
 default:
 cpu_abort(env, Unknown or invalid MMU model\n);
-- 
1.5.4

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html