Re: [PATCH 13/13] KVM: PPC: e500: MMU API

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

 



On Thu, 19 May 2011 19:37:05 +0200
Alexander Graf <agraf@xxxxxxx> wrote:

> 
> On 19.05.2011, at 18:28, Scott Wood wrote:
> 
> > On Thu, 19 May 2011 16:10:48 +0200
> > Alexander Graf <agraf@xxxxxxx> wrote:
> > 
> >> On 05/18/2011 01:42 AM, Scott Wood wrote:
> >>> +For mmu types KVM_MMU_FSL_BOOKE_NOHV and KVM_MMU_FSL_BOOKE_HV:
> >> 
> >> Can we rename those KVM_MMU_BOOKE_206_NOHV and KVM_MMU_BOOKE_206_HV? 
> >> Same for the function names. It's just easier to read and more accurate. 
> >> Also I don't like how it says KVM_MMU_FSL :). It's really a booke interface.
> > 
> > That's more or less how I had it initially.  In our earlier discussion I
> > changed it to FSL because of assumptions about TLB layout, such as the set
> > hash:
> > 
> > http://www.spinics.net/lists/kvm-ppc/msg02641.html
> > http://www.spinics.net/lists/kvm-ppc/msg02645.html
> 
> Ah, that discussion was about calling it book3e, and that name is too generic. If the interface could satisfy any book3e 2.06 MMU, then calling it BOOKE_206 would be just fine.

OK, I was considering 2.06+ implicit in "book3e" versus "booke", as that
seems to be the naming scheme that other parts of the kernel are using, so
I misinterpreted the objection.

> If it can only handle e500, call it FSL or E500 - whichever fits you
> better. I'd prefer if the interface was clever enough to also be used by
> non-FSL 2.06 implementations, in case any of those follow, if it's not too
> much additional overhead.

I think the main difficulty in making it arbitrary book3e 2.06 is the set
hash.

Note that most of this can be reused with new MMU IDs that specify
different hash (or other) behavior.

> >>> + - The "array" field points to an array of type "struct
> >>> +   kvm_fsl_booke_tlb_entry".
> >>> + - The array consists of all entries in the first TLB, followed by all
> >>> +   entries in the second TLB.
> >>> + - Within TLB0 entries are ordered first by increasing set number.  Within a
> >>> +   set, entries are ordered by way (increasing ESEL).
> >>> +   The hash for determining set number is: (MAS2>>  12)&  (num_sets - 1)
> >>> +   where "num_sets" is the tlb_sizes[] value divided by the tlb_ways[] value.
> >>> + - Within TLB1, entries are ordered by increasing ESEL.
> >> 
> >> Don't special-case it. TLB1 basically contains of only a single set. The 
> >> MMU interface here shouldn't be too e500 specific :).
> > 
> > How do I define the hash for determining set number without being
> > e500-specific?
> 
> Hm. Isn't the hash always the way you put it above? Maybe I misread the spec :). The only obvious thing it states there are the corner cases O_o.

The ISA never defines the hash, it just says that ESEL selects from the
set of possible entries for a given MAS2[EPN].  MAS1{TID] and
MAS1[TSIZE] can also be used as hash inputs if HES is supported.  Using the
low bits of the page number is the obvious thing to do, but not the only
thing allowed.

> >>> +struct kvm_fsl_booke_tlb_params {
> >>> +	/*
> >>> +	 * book3e defines 4 TLBs, but current FSL Book E chips implement
> >>> +	 * only the first two.  The second two entries in tlb_sizes[]
> >>> +	 * and tlb_ways[] are reserved and must be zero.
> >> 
> >> I don't think that part is necessary. It's really an implementation detail.
> > 
> > It is not an implementation detail.  KVM on FSL Book E does not support
> > TLB2 or TLB3.  I had more generic wording when it was not FSL specific,
> > where the supported TLBs were determined by reading the TLB config
> > registers.
> 
> Either the struct is 100% e500 specific, then you only need 2 TLBs in there, or it's generic, then the actual implementation of TLB2 and TLB3 are open and as of yet unimplemented, but not reserved for eternity :).

Ah.  Well, perhaps this struct (and tlb_entry) should be book3e 2.06
even if the MMU ID is FSL-specific.

> >>> +	 * KVM will adjust TLBnCFG based on the sizes configured here,
> >>> +	 * though arrays greater than 2048 entries will have TLBnCFG[NENTRY]
> >>> +	 * set to zero.
> >>> +	 *
> >>> +	 * TLB0 supports 4 KiB pages.
> >>> +	 * The page sizes supported by TLB1 are as indicated by
> >>> +	 * TLB1CFG (if MMUCFG[MAVN] = 0) or TLB1PS (if MMUCFG[MAVN] = 1).
> >> 
> >> This is not part of the interface, is it?
> > 
> > It provides a way for qemu to discover what page sizes KVM will deal with.
> 
> Hrm. So where does it get MMUCFG from?

KVM_GET_SREGS

-Scott

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