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: Friday, August 22, 2008 4:54 AM
> To: Liu Yu
> Cc: kvm-ppc@xxxxxxxxxxxxxxx
> Subject: Re: [patch 0/4] add e500 platform support for KVM
> 
> On Fri, 2008-08-15 at 16:50 +0800, Liu Yu wrote:
> > These patches add the support of e500 platform for KVM.
> 
> Hi Yu, sorry it's taken me this long to take a look. I've 
> been sick and
> catching up from vacation...

That's all right. ;-)

> 
> > The code is just in the primary stage,
> > so this time just for discussion.
> 
> 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.

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.

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?

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

> 
>         case BOOKE_INTERRUPT_DTLB_MISS:	
>         	gtlbe = kvmppc_dtlb_search(vcpu, eaddr); <- CORE HOOK
>         	if (!gtlbe) {
>         		/* The guest didn't have a mapping for it. */
>         		kvmppc_queue_exception(vcpu, exit_nr);
>         		vcpu->arch.dear = vcpu->arch.fault_dear;
>         		vcpu->arch.esr = vcpu->arch.fault_esr;
>         		kvmppc_deliver_dtlb_miss(vcpu); <- CORE HOOK
>         		vcpu->stat.dtlb_real_miss_exits++;
>         		r = RESUME_GUEST;
>         		break;
>         	}
>         
>         	vcpu->arch.paddr_accessed = tlb_xlate(gtlbe, eaddr);
>         	gfn = vcpu->arch.paddr_accessed >> PAGE_SHIFT;
>         
>         	if (kvm_is_visible_gfn(vcpu->kvm, gfn)) {
>         		kvmppc_mmu_map(vcpu, eaddr, gfn, gtlbe->tid,
>         		               gtlbe->word2); <- CORE HOOK
>         		vcpu->stat.dtlb_virt_miss_exits++;
>         		r = RESUME_GUEST;
>         	} else
>         		r = kvmppc_emulate_mmio(run, vcpu);
>         
>         	break;
> 
> 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.

> 
> Christian makes a great point about interrupts in
> host_tlb_write_entry(), but I have an even bigger question: by not
> tracking the state of the shadow TLB, you're implementing lazy
> save/restore. In contrast, 440 is doing a full TLB reload, since I
> assume that a) the TLB is so small that is almost certainly full of
> useful entries, and b) TLB misses are much more expensive 
> with KVM than
> on bare metal.

Do you mean TLB0?
I explain it above.
When I found a TLB0 entry not mapped, I write it to hardware
immediately.
I just don't care about where this entry located and when it will be
replaced,
this is due to the auto-entry-select mechanism.
I hope the way I deal with host TLB0 could be consistent with the host
kernel's.

> 
> > I have tested them under branch 2.6.26, but not under the 
> lastest code,
> > for my board can not boot up via current kvm tree.
> 
> Hmm, that's too bad.

At lease I can still test the core code. :-)
--
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