RE: [patch 0/4] add e500 platform support for KVM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Hollis Blanchard [mailto:hollisb@xxxxxxxxxx] 
> Sent: Saturday, August 30, 2008 1:09 AM
> To: Liu Yu-B13201
> Cc: kvm-ppc@xxxxxxxxxxxxxxx
> Subject: RE: [patch 0/4] add e500 platform support for KVM
> 
> On Fri, 2008-08-22 at 19:00 +0800, Liu Yu wrote: 
> > > 
> > > Could you talk a little about how it differs from the 440
> > > implementation? For example, how are you using TLB0 and TLB1?
> > 
> > Sorry that I havent provided the detailed description.
> > 
> > The case of TLB1 is the same as 44x.
> > Since TLB0 is 4K-fixed map, it has different implementation.
> > 
> > Guest always has TLB0 and TLB1.
> > Host can chosen how to map them.
> > 
> > Currently if not define KVMPPC_E500_TLB0_ENABLE,
> > both guest TLB1 and guest TLB0 map to host TLB1.
> > Otherwise, guest TLB1 map to host TLB1 and guest TLB0 map 
> to host TLB0.
> > KVMPPC_E500_TLB0_ENABLE is an intergradation,
> > as I first only use host TLB1, after TLB1 is ok then I 
> debug host TLB0.
> 
> OK, I think you're saying that KVMPPC_E500_TLB0_ENABLE is a temporary
> hack, since Wei's original code used only TLB1, and you're 
> now extending
> that to also use TLB0.

Yes.

> 
> > Unlike TLB1(one-to-many mapping, the same as 44x), guest 
> TLB0 and shadow
> > TLB0 is one-one mapping. 
> > That is to say, you can use the entry select or index to find the
> > guest/shadow pair.
> > 
> > Moreover, TLB1 has fixed one-one mapping between shadow 
> TLB1 and host
> > TLB1.
> > You can find the host TLB1 entry according to the index of 
> shadow TLB1.
> > For TLB0, it's a dynamic one-one mapping, you can not find 
> which host
> > entry the shadow ultimately be mapped.
> > This is due to the auto-entry-select mechanism of E500. And 
> the only way
> > to destroy an entry is tibivax.
> 
> You lost me a little bit here.
> 
> It looks like your kvm_vcpu_arch structure holds a full copy of both
> TLB0 and TLB1, guest and shadow. However, I can't see where you write
> into shadow_tlb[0]; instead it looks like you directly insert those
> entries into the hardware without saving a copy in the vcpu.

All the shadow tlb update exist in shadow_map()
TLB1 will call it from kvm_mmu_map(), and TLB0 will call it from
kvm_tlbe_map().

> 
> > Unlike 44x use TID=0 map userspace and TID=1 map kernel space,
> > I plan to use host TLB1 to map kernel, and host TLB0 to map 
> userspace.
> > This category can make it convient to handle privilege 
> switch: that is
> > before enterring guest just need tlbivax TLB1.
> > Userspace tlb enties with differnet TID can exist in host 
> TLB0 till host
> > kvm process switch or guest meet explicit tlbiax command.
> > 
> > I just have this idea and have not thought all the detail through.
> > What do you think of it?
> 
> You might run into problems if you ever get large guest userspace
> mappings. I know hugetlbfs doesn't exist for e500 Linux right now, but
> it could in the future, and plus there are other kernels to consider.

Can you give me more details? 
I'm not sure how hugetlbfs could be in the future, but TLB0 has a fixed
mapping size 4KB.

> 
> > > Some of the refactoring you've done, like creating 
> completely separate
> > > kvmppc_handle_tlb_miss() functions, surprises me. For example, I'd
> > > expect the DTLB miss code to be refactored like this:
> > 
> > In fact, I don't have much experience on refactor,
> > I just thought we could merge as more common code as we can.
> > 
> > Do we need to separate dtlb and itlb?
> > I tried to merge them so that the code became what it looks 
> like. :-|
> 
> Your code does something similar to mine, just without the helper
> functions:
> 
>         kvmppc_handle_tlb_miss(vcpu, eaddr, MSR_IS);
> vs
>         kvmppc_44x_itlb_search(...) {
>                 kvmppc_44x_tlb_index(..., !!(msr & MSR_IS))
>         }

OK.
I will change them back.

> 
> > > booke_fsl_interrupts.S looks like a lot of code copied and 
> > > pasted, with
> > > the obvious exception of the TLB handlers. Can't we work out 
> > > some better
> > > way to share the rest?
> > 
> > That would be great.
> > I just considered it step-by-step, and I am not very 
> familiar with gnu
> > assemble macro, define things,
> > and I nocited head_fsl_booke.S and head_44x.S...
> > 
> > I will think about it.
> 
> OK, so there are two basic differences in booke_interrupts.S, and
> they're both in the common "lightweight exit" path: the additional PID
> register switching and the modifications to the TLB save/restore.
> 
> The PID stuff should be easy enough to hide: just create a
> "KVMPPC_SAVE_PID" macro.

Do we need a new header file to define this?

> 
> For now we can probably do the same with the TLB manipulations.

How do you think the way I define struct tlb_array and tlbe?
If you think it's fine, I think 44x can adopt it.
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM Development]     [KVM ARM]     [KVM ia64]     [Linux Virtualization]     [Linux USB Devel]     [Linux Video]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux