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

[Hollis Blanchard <hollisb@us.ibm.com>]

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

> 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?

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:

        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?

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.

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

-- 
Hollis Blanchard
IBM Linux Technology Center