> -----Original Message----- > From: Hollis Blanchard [mailto:hollisb@xxxxxxxxxx] > Sent: Thursday, April 09, 2009 12:43 AM > To: Liu Yu-B13201 > Cc: kvm-ppc@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] Map guest (ts,tid) to individual shadow tid > > On Tuesday 07 April 2009 21:11:11 Liu Yu-B13201 wrote: > > > > > -----Original Message----- > > > From: Hollis Blanchard [mailto:hollisb@xxxxxxxxxx] > > > Sent: Tuesday, April 07, 2009 11:41 PM > > > To: Liu Yu-B13201 > > > Cc: kvm-ppc@xxxxxxxxxxxxxxx > > > Subject: Re: [PATCH] Map guest (ts,tid) to individual shadow tid > > > > > > On Tue, 2009-04-07 at 17:51 +0800, Liu Yu wrote: > > > > Hi guys, > > > > > > > > Thanks to Hollis's idea. > > > > This patch help to make KVMPPC be more friendly to OSes > > > other than Linux. > > > > > > Wow, that was quick. :) > > > > > > Have you tested this code? Doesn't this break the assumption in > > > kvmppc_e500_tlb1_invalidate() (and probably elsewhere) > that stid == > > > gtid? > > > > Yes, have taken a simple test. > > Once we can reduce the number of TLB flushes (see below) it will be > interesting to see if there's a performance impact. > > >Good catch, it needs to handle it here in > kvmppc_e500_tlb1_invalidate(). > >Thanks. > >But it's ok for now, because TLB1 only contains tid=0 > entries, and tid=0 > >always be mapped to stid=0. > >That's why the test is fine... > > OK. Still, it makes me nervous to break such a simple > assumption. We should > introduce nice accessors to make it difficult to code it > wrong in the future. > I'm honestly surprised that kvmppc_e500_tlb1_invalidate() is > the only affected > site. It is the only site. We don't pay much attention on shadow tid. For 500, shadow tid is inherit from guest tid. As guest tlb1 mapping is broken into 4K shadow mappings, the kvmppc_e500_tlb1_invalidate() need to check tid to find all 4k shadow mappings related to an guest mapping. Actually, I have been thinking about remove all shadow tlb like 440 did. This maybe helpful to succedent work such as huge tlb mapping. After doing that, kvmppc_e500_tlb1_invalidate() won't has this assumption. Anyway, the patch is an RFC, not aimed at getting applied. Just make something to discuss. :) > > > > > Any (ts,tid) combination will be mapped to an individual > > > shadow tid(stid). > > > > Whenever the stid is exhausted, it needs to reset of stid > > > and flush host TLB. > > > > > > What I was thinking was that KVM would participate in the > > > host's normal > > > PID allocation policy. Specifically, guest activity could > > > cause the host > > > to invoke steal_context_up/smp() (see switch_mmu_context() for > > > reference). > > > > Host only use AS=0 space and the context only have 255 PIDs. > > But we use AS=1 space then we have another 255 PIDs. > > If we participate in host, how could we utilize the PIDs > under condition > > AS=1 without consume host PIDs? > > I'm suggesting we not use AS=0. We can't use AS=0 and AS=1 PIDs > interchangeably anyways. > > > > When you do that, there is no need for KVM to explicitly > flush the TLB > > > ever -- it's done as needed by steal_context*(). That's > > > actually better > > > than what we do today, because today we must flush all > guest mappings > > > every time we de-schedule a guest. In the new scheme, when > > > (guest PIDs + > > > host PIDs) < 255, we would never need to flush the TLB due to PID > > > pressure. > > > > No. we only need to flush TLB when additional 255 PIDs are > exhausted. > > And it's nothing to do with host PIDs. > > For Linux running in guest, PIDs won't be exhausted so no > TLB flush is > > needed. > > I think this isn't true when using normal (AS=0) host PIDs. > > That said, I guess throwing away the AS trick on Linux hosts > wouldn't be a > great idea. Since it's available (on Book E), we might as > well take advantage > of it. > > So the only question then is about supporting *guest* use of > AS, which I see > now is what your patch is about. The one big change I would > make is to have > the AS=1 PID allocator be host-global rather than per-vcpu. > That way we can > avoid the unnecessary TLB flushes on every guest deschedule I > mentioned > before. (Imagine guest A using two host AS=1 PIDs, and guest > B using five: we > never need to flush the TLB due to PID pressure.) Yes, I remember you once mentioned it. Then we don't need to flush tlb on every vcpu_get()/put(). I've thought of it, and it should be simple to make make the allocator host-global. And I'd like to put it into a separate performance improving patch. But there is one thing I haven't thought through. That is how should we handle guest kernel mapping (tid=0) after apply this trick? As it's supposed to share by all PIDs. Any suggestion? > > That would also put the new PID management code into booke > code instead of > e500 code. Certainly. > > > > (Also, rather than an array, I would have created a hash > table using > > > include/linux/hash.h and include/linux/list.h. I haven't > thought it > > > through all the way though... I mean the tradeoffs of an > array vs hash > > > table here.) > > > > > > > Hmm. What do you think we can benefit from hash table? > > Lower memory consumption. You know it's going to be sparse, > since not every > guest AS|PID will have a corresponding host PID. However, 512 > bytes isn't bad, > especially if it's shared by all vcpus. Don't worry about it. Ah I see. For the design in this patch, we can put the allocate count global to reduce the tlb flush on guest schedule, but we can't put the mappings global. If we do, we need to identify different vcpu from each mapping entry. -- 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
