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

[Liu Yu-B13201]

> -----Original Message-----
> From: Alexander Graf [mailto:address@hidden 
> Sent: Friday, August 21, 2009 8:39 PM
> To: Liu Yu-B13201
> Cc: Hollis Blanchard; <address@hidden>; 
> <address@hidden>; <address@hidden>; 
> <address@hidden>
> Subject: Re: [PATCH 4/5] kvmppc: Translate eaddr for fsl_booke mmu
> 
> 
> Am 20.08.2009 um 12:21 schrieb "Liu Yu-B13201" <address@hidden>:
> 
> >
> >
> >> -----Original Message-----
> >> From: Hollis Blanchard [mailto:address@hidden
> >> Sent: Thursday, August 20, 2009 6:51 AM
> >> To: Liu Yu-B13201
> >> Cc: address@hidden; address@hidden;
> >> address@hidden; address@hidden; 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 <address@hidden>
> >>> ---
> >>> 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.