I haven't gone through this all (since it won't apply/build now), but just a couple quick comments: On Wed, 2008-12-03 at 17:28 +0800, Liu Yu wrote: > diff --git a/arch/powerpc/kvm/e500_tlb.h b/arch/powerpc/kvm/e500_tlb.h > new file mode 100644 > index 0000000..6adda39 > --- /dev/null > +++ b/arch/powerpc/kvm/e500_tlb.h > @@ -0,0 +1,290 @@ > +/* > + * Copyright (C) 2008 Freescale Semiconductor, Inc. All rights reserved. > + * > + * Author: Yu Liu, yu.liu@xxxxxxxxxxxxx, Aug, 2008 > + * > + * Description: > + * This file is based on arch/powerpc/kvm/44x_tlb.h, > + * by Hollis Blanchard <hollisb@xxxxxxxxxx>. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License, version 2, as > + * published by the Free Software Foundation. > + */ > + > +#ifndef __KVM_E500_TLB_H__ > +#define __KVM_E500_TLB_H__ > + > +#include <linux/kvm_host.h> > +#include <asm/mmu-fsl-booke.h> > +#include <asm/tlb.h> > +#include <asm/kvm_e500.h> > + > +#define KVM_E500_TLB0_ENABLE /* Enable it to utilize TLB0 */ Just remove this ifdef completely. > +#define KVM_E500_TLB0_WAY_SIZE_BIT 7 /* Fixed */ > +#define KVM_E500_TLB0_WAY_SIZE (1UL << KVM_E500_TLB0_WAY_SIZE_BIT) > +#define KVM_E500_TLB0_WAY_SIZE_MASK (KVM_E500_TLB0_WAY_SIZE - 1) > + > +#define KVM_E500_TLB0_WAY_NUM_BIT 1 /* No greater than 7 */ > +#define KVM_E500_TLB0_WAY_NUM (1UL << KVM_E500_TLB0_WAY_NUM_BIT) > +#define KVM_E500_TLB0_WAY_NUM_MASK (KVM_E500_TLB0_WAY_NUM - 1) Are these really KVM constants, or e500 in general? If general e500, you should put them in a general e500 header. > +#define KVM_E500_TLB0_SIZE (KVM_E500_TLB0_WAY_SIZE * KVM_E500_TLB0_WAY_NUM) > +#define KVM_E500_TLB1_SIZE 16 You probably want to distinguish between hardware TLB size and guest TLB size, since TLBnCFG allows you to trivially expose a larger virtual TLB to the guest than the hardware really supports. > +#define index_of(tlbsel, esel) (((tlbsel) << 16) | ((esel) & 0xFFFF)) > +#define tlbsel_of(index) ((index) >> 16) > +#define esel_of(index) ((index) & 0xFFFF) > +#define to_htlb1_esel(esel) (num_tlbcam_entries - (esel) - 1) I don't really like these names, but they're completely local so it's not too big a deal. > +extern unsigned int tlbcam_index; > +extern unsigned int num_tlbcam_entries; You should put these externs somewhere in a generic e500 header, since they are generic e500 globals. They should probably also be renamed to reflect that... Either way it's something you should submit, as a small separate patch, to the e500 maintainer to get their ack. -- Hollis Blanchard IBM Linux Technology Center -- 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
