[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/6] PPC: Implement e500 (FSL) MMU
|
From: |
Alexander Graf |
|
Subject: |
Re: [Qemu-devel] [PATCH 5/6] PPC: Implement e500 (FSL) MMU |
|
Date: |
Fri, 6 May 2011 12:01:11 +0200 |
On 03.05.2011, at 21:25, Scott Wood wrote:
> On Mon, 2 May 2011 17:03:21 +0200
> Alexander Graf <address@hidden> wrote:
>
>> Most of the code to support e500 style MMUs is already in place, but
>> we're missing on some of the special TLB0-TLB1 handling code and slightly
>> different TLB modification.
>>
>> This patch adds support for the FSL style MMU.
>>
>> Signed-off-by: Alexander Graf <address@hidden>
>
> You beat me to (part of) it... :-)
>
> How is this going to interact with the KVM MMU API, where it was suggested
> that qemu use that representation as its native structure? Should we just
> change the representation at that point, for both types of booke mmu (so 4xx
> would be represented with MAS registers internally, even though a
> different interface is exposed to the guest)?
Ugh - I completely forgot about that one. I guess we'll just have to redo the
internal TLB resolution code at that point, yes. I somehow prefer to work my
way from working solution to working solution though, so I don't think I'll
move it over to a MAS based approach just yet.
>
>> @@ -678,8 +887,9 @@ struct CPUPPCState {
>> int nb_BATs;
>> target_ulong DBAT[2][8];
>> target_ulong IBAT[2][8];
>> - /* PowerPC TLB registers (for 4xx and 60x software driven TLBs) */
>> + /* PowerPC TLB registers (for 4xx, e500 and 60x software driven TLBs) */
>> int nb_tlb; /* Total number of TLB
>> */
>> + int nb_tlbs[4]; /* Number of respective TLB entries (e500)
>> */
>
> Looks like "number of tlbs", not "number of tlb entries"... Was less
> confusing when there was only one tlb array.
I certainly agree, but this way I don't have to change non-e500 code :). I
actually started off creating a new TLB struct with 4 entries (the 4 TLBs
allowed by the 2.06 spec), but quickly gave up as it's a _lot_ of code using
these O_o.
Either way, it's all different now anyways :).
>> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
>> index 5e4030b..261c1a9 100644
>> --- a/target-ppc/helper.c
>> +++ b/target-ppc/helper.c
>> @@ -1153,48 +1153,144 @@ void store_40x_sler (CPUPPCState *env, uint32_t val)
>> env->spr[SPR_405_SLER] = val;
>> }
>>
>> +static inline int mmubooke_check_tlb (CPUState *env, ppcemb_tlb_t *tlb,
>> + target_phys_addr_t *raddr, int *prot,
>> + target_ulong address, int rw,
>> + int access_type, int i)
>> +{
>> + int ret, _prot;
>> +
>> + if (ppcemb_tlb_check(env, tlb, raddr, address,
>> + env->spr[SPR_BOOKE_PID],
>> + !env->nb_pids, i) >= 0) {
>> + goto found_tlb;
>> + }
>> +
>> + if ((env->nb_pids > 1) && env->spr[SPR_BOOKE_PID1] &&
>> + ppcemb_tlb_check(env, tlb, raddr, address,
>> + env->spr[SPR_BOOKE_PID1], 0, i) >= 0) {
>> + goto found_tlb;
>> + }
>> +
>> + if ((env->nb_pids > 2) && env->spr[SPR_BOOKE_PID2] &&
>> + ppcemb_tlb_check(env, tlb, raddr, address,
>> + env->spr[SPR_BOOKE_PID2], 0, i) >= 0) {
>> + goto found_tlb;
>> + }
>
> Maybe PID1/PID2 should be moved into ppcemb_tlb_check, and eliminate the
> nb_pids checks by putting zero in PID1/PID2 on chips that don't have it?
> I'm assuming this is performance sensitive for non-KVM.
PID1/PID2 are already 0 on chips that don't have them, so we could eliminate
the check on nb_pids, yup. The small branch here really isn't too bad on
performance though, so it's more of a readability thing than performance. We're
currently killing performance anyways by flushing qemu's internal TLB on every
MSR_IR/DR switch.
>
>> +static int mmue500_get_physical_address (CPUState *env, mmu_ctx_t *ctx,
>> + target_ulong address, int rw,
>> + int access_type)
>> +{
>> + ppcemb_tlb_t *tlb;
>> + target_phys_addr_t raddr;
>> + int i, ret;
>> + uint32_t ea = (address >> MAS2_EPN_SHIFT) & 0x7f;
>> +
>> + ret = -1;
>> + raddr = (target_phys_addr_t)-1ULL;
>> +
>> + /* check TLB1 - hits often because the kernel is here */
>
> I'd optimize for userspace instead -- that's where the real work is more
> likely to happen. Plus, compare what's likely to be 5-6 iterations before
> finding the entry for a kernel large-page entry if we check TLB0 first,
> versus 17+ iterations (65+ on e500mc) for finding a TLB0 entry if TLB1 is
> checked first -- based on Linux's TLB1 usage patterns.
Good point!
>
>> + for (i = env->nb_tlbs[0]; i < env->nb_tlb; i++) {
>> + tlb = &env->tlb[i].tlbe;
>> + ret = mmubooke_check_tlb(env, tlb, &raddr, &ctx->prot, address, rw,
>> + access_type, i);
>> + if (!ret) {
>> + goto found_tlb;
>> }
>> }
>> - if (ret >= 0)
>> +
>> + /* check possible TLB0 entries */
>> + for (i = 0; i < env->nb_ways; i++) {
>> + tlb = &env->tlb[ea | (i << 7)].tlbe;
>
> Do we have to hardcode 7 (and 0x7f) here?
I'm not sure - the spec isn't exactly obvious on this part. How do I find out
the mask from TLBnCFG? But then again I could just take (nb_tlbs[n] - (1 <<
nb_ways[n])) - 1. I'll give that a try :).
> And if possible I'd rearrange the tlb so that ways within a set are
> contiguous. Better for cache utilization, and is what the KVM MMU API will
> want.
Ok.
>
>> @@ -1348,6 +1446,44 @@ target_phys_addr_t cpu_get_phys_page_debug (CPUState
>> *env, target_ulong addr)
>> return ctx.raddr & TARGET_PAGE_MASK;
>> }
>>
>> +static void e500_update_mas_tlb_miss(CPUState *env, target_ulong address,
>> + int rw)
>> +{
>> + env->spr[SPR_BOOKE_MAS0] = env->spr[SPR_BOOKE_MAS4] & MAS4_TLBSELD_MASK;
>> + env->spr[SPR_BOOKE_MAS1] = env->spr[SPR_BOOKE_MAS4] & MAS4_TSIZED_MASK;
>> + env->spr[SPR_BOOKE_MAS2] = env->spr[SPR_BOOKE_MAS4] & MAS4_WIMGED_MASK;
>> + env->spr[SPR_BOOKE_MAS6] = 0;
>> +
>> + /* AS */
>> + if (((rw == 2) && msr_ir) || ((rw != 2) && msr_dr)) {
>> + env->spr[SPR_BOOKE_MAS1] |= MAS1_TS;
>> + env->spr[SPR_BOOKE_MAS6] |= MAS6_SAS;
>> + }
>> +
>> + env->spr[SPR_BOOKE_MAS1] |= MAS1_VALID;
>> + env->spr[SPR_BOOKE_MAS2] |= address & MAS2_EPN_MASK;
>> +
>> + switch (env->spr[SPR_BOOKE_MAS4] & MAS4_TIDSELD_PIDZ) {
>> + case MAS4_TIDSELD_PID0:
>> + env->spr[SPR_BOOKE_MAS1] |= env->spr[SPR_BOOKE_PID] <<
>> MAS1_TID_SHIFT;
>> + break;
>> + case MAS4_TIDSELD_PID1:
>> + env->spr[SPR_BOOKE_MAS1] |= env->spr[SPR_BOOKE_PID1] <<
>> MAS1_TID_SHIFT;
>> + break;
>> + case MAS4_TIDSELD_PID2:
>> + env->spr[SPR_BOOKE_MAS1] |= env->spr[SPR_BOOKE_PID2] <<
>> MAS1_TID_SHIFT;
>> + break;
>> + }
>> +
>> + env->spr[SPR_BOOKE_MAS6] |= env->spr[SPR_BOOKE_PID] << 16;
>> +
>> + /* next victim logic */
>> + env->spr[SPR_BOOKE_MAS0] |= env->last_way << MAS0_ESEL_SHIFT;
>> + env->last_way++;
>> + env->last_way &= 3;
>> + env->spr[SPR_BOOKE_MAS0] |= env->last_way << MAS0_NV_SHIFT;
>> +}
>
> MAS3/7 should be zeroed according to the architecture.
Ah, missed that one :)
> Better victim selection would check for empty ways, and perhaps keep
> last_way on a per-set basis.
Does the hardware do this?
>
>> @@ -1820,12 +1956,8 @@ void ppc_tlb_invalidate_all (CPUPPCState *env)
>> cpu_abort(env, "MPC8xx MMU model is not implemented\n");
>> break;
>> case POWERPC_MMU_BOOKE:
>> - tlb_flush(env, 1);
>> - break;
>> case POWERPC_MMU_BOOKE_FSL:
>> - /* XXX: TODO */
>> - if (!kvm_enabled())
>> - cpu_abort(env, "BookE MMU model is not implemented\n");
>> + tlb_flush(env, 1);
>> break;
>> case POWERPC_MMU_32B:
>> case POWERPC_MMU_601:
>
> This flushes env->tlb_table, but don't we still need to clear out env->tlb?
I guess so, yes. That would break my PID -> tlb_flush patch though. Hrm. This
way we don't reset the TLB on reset. I'll fix it up :).
>
>> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
>> index d5db484..3c0b061 100644
>> --- a/target-ppc/op_helper.c
>> +++ b/target-ppc/op_helper.c
>> @@ -4206,4 +4206,422 @@ target_ulong helper_440_tlbsx (target_ulong address)
>> return ppcemb_tlb_search(env, address, env->spr[SPR_440_MMUCR] & 0xFF);
>> }
>>
>> +/* PowerPC e500 TLB management */
>> +
>> +static inline int e500_tlb_num(CPUState *env, ppcemb_tlb_t *tlb)
>> +{
>> + ulong tlbel = (ulong)tlb;
>> + ulong tlbl = (ulong)env->tlb;
>> +
>> + return (tlbel - tlbl) / sizeof(env->tlb[0]);
>> +}
>> +
>> +static inline ppcemb_tlb_t *e500_cur_tlb(CPUState *env)
>
> This is a bit big to be inlined - let the compiler decide such things.
*shrug* ok :).
>
>> +{
>> + uint32_t tlbncfg = 0;
>> + int esel = (env->spr[SPR_BOOKE_MAS0] & MAS0_ESEL_MASK) >>
>> MAS0_ESEL_SHIFT;
>> + int ea = (env->spr[SPR_BOOKE_MAS2] & MAS2_EPN_MASK) >> MAS2_EPN_SHIFT;
>> + int tlb;
>> + int r;
>> +
>> + tlb = (env->spr[SPR_BOOKE_MAS0] & MAS0_TLBSEL_MASK) >>
>> MAS0_TLBSEL_SHIFT;
>> + tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlb];
>> +
>> + if ((tlbncfg & TLBnCFG_HES) && (env->spr[SPR_BOOKE_MAS0] & MAS0_HES)) {
>> + cpu_abort(env, "we don't support HES yet\n");
>> + }
>> +
>> + if (tlb == 1) {
>> + r = env->nb_tlbs[0] + (esel % env->nb_tlbs[1]);
>
> Ouch, division by non-constant.
>
> There's no need for it anyway; esel >= env_nb_tlbs[1] is undefined
> behavior. Just crop the value arbitrarily (or error out, if possible) to
> avoid dereferencing a bad array index.
I just rewrote all of this anyways, getting rid of the division along the way.
>
>> + } else {
>> + esel &= env->nb_ways - 1;
>> + esel <<= 7;
>> + ea &= 0x7f;
>> + r = (esel | ea) % env->nb_tlbs[0];
>
> More unnecessary division -- use "& (env->nb_tlbs[0] - 1)", or variable-ize
> the 7 and 0x7f so that we know that we won't produce an out-of-bounds value.
>
>> +static const target_phys_addr_t e500_tlb_sizes[] = {
>> + 0,
>> + 4 * 1024,
>> + 16 * 1024,
>> + 64 * 1024,
>> + 256 * 1024,
>> + 1024 * 1024,
>> + 4 * 1024 * 1024,
>> + 16 * 1024 * 1024,
>> + 64 * 1024 * 1024,
>> + 256 * 1024 * 1024,
>> + 1024 * 1024 * 1024,
>> + (target_phys_addr_t)(4ULL * 1024ULL * 1024ULL * 1024ULL),
>> +};
>
> FWIW, power-of-2 page sizes are architected, and may appear in future
> e500-family chips. The TSIZE field is extended by one bit on the right
> (previously reserved and should-be-zero).
Yeah, I've seen that mentioned as MAV 2.0 or so change. This is the exact list
of supported page sizes for an e500v2 though. Should we just support all of the
possible one and ignore the chip's deficiencies?
>
>> +static inline target_phys_addr_t e500_tlb_to_page_size(int size)
>
> unsigned int
Why?
>
>> +{
>> + target_phys_addr_t r;
>> +
>> + if (size > 11) {
>
> if (size >= ARRAY_SIZE(e500_tlb_sizes)) {
>
>> + /* should not happen */
>> + r = 1024 * 1024 * 1024;
>
> Error message?
>
> + switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) {
> + case MAS0_WQ_ALWAYS:
> + /* good to go, write that entry */
> + break;
> + case MAS0_WQ_COND:
> + /* XXX check if reserved */
> + if (0) {
> + return;
> + }
> + break;
> + case MAS0_WQ_CLR_RSRV:
> + /* XXX clear entry */
> + return;
> + default:
> + /* no idea what to do */
> + return;
> + }
>
> If the plan is to support such things, might want a more generic name than
> "e500" -- we're really talking about isa 2.06 book3e. The first chip that
> implements this stuff will probably not be from Freescale, much less
> called "e500". :-)
Good point :). I'll rename it booke206.
>
>> + if (msr_gs) {
>> + cpu_abort(env, "missing HV implementation\n");
>
> What's the policy on aborting versus error-message versus no-op if the
> guest invokes undefined/unimplemented behavior?
The less likely a failure is, the more I find aborting/error-message useful.
For KVM, we want to keep the abort cases very very very low. For TCG, I prefer
to see when something goes wrong.
So general rule of thumb:
Guest user space should never be able to kill the VM
Guest kernel can kill the VM anyways, so it can also do so by issuing
unimplemented things
If the VM would work just fine without the unimplemented thing, don't abort
>
>> + } else {
>> + rpn = ((uint64_t)env->spr[SPR_BOOKE_MAS7] << 32) |
>> + (env->spr[SPR_BOOKE_MAS3] & 0xfffff000);
>> + }
>
> Not sure why this is in an else-branch versus msr_gs.
Because that's what the spec says:
if category E.HV.LRAT supported & (MSRGS=1) & (MAS1V=1) then
rpn <- translate_logical_to_real(MAS7RPNU || MAS3RPNL, MAS8TLPID)
else if MAS7 implemented then
rpn <- MAS7RPNU || MAS3RPNL
else rpn <- 320 || MAS3RPNL
>
>> + tlb->RPN = rpn;
>> +
>> + tlb->PID = (env->spr[SPR_BOOKE_MAS1] & MAS1_TID_MASK) >> MAS1_TID_SHIFT;
>> + tlb->size = e500_tlb_to_page_size((env->spr[SPR_BOOKE_MAS1]
>> + & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT);
>
> e500 manuals document that tsize is ignored in tlb0.
Yup, thanks :).
>
>> + tlb->EPN = (uint32_t)(env->spr[SPR_BOOKE_MAS2] & MAS2_EPN_MASK);
>> + tlb->attr = env->spr[SPR_BOOKE_MAS2] & (MAS2_ACM | MAS2_VLE | MAS2_W |
>> + MAS2_I | MAS2_M | MAS2_G |
>> MAS2_E)
>> + << 1;
>> + tlb->attr |= env->spr[SPR_BOOKE_MAS1] & MAS1_IPROT;
>> + tlb->attr |= (env->spr[SPR_BOOKE_MAS3] &
>> + ((MAS3_U0 | MAS3_U1 | MAS3_U2 | MAS3_U3)) << 8);
>
> Might be nice to #define the encoding of these bits into attr/etc, versus
> magic shifts (though it's moot if we're going to switch to a MAS-based
> representation).
Yeah, this will be moot once we switch. No need to worry about it now.
>
>> + if (tlb->size == TARGET_PAGE_SIZE) {
>> + tlb_flush_page(env, tlb->EPN);
>> + } else {
>> + tlb_flush(env, 1);
>> + }
>
> Need to check whether the previous entry was a large page needing a full
> flush.
Right, not the current one.
>
>> +void helper_e500_tlbre(void)
>> +{
>> + ppcemb_tlb_t *tlb = NULL;
>> +
>> + tlb = e500_cur_tlb(env);
>> + e500_tlb_to_mas(env, tlb, e500_tlb_num(env, tlb));
>> +}
>> +
>> +/* Generic TLB check function for embedded PowerPC implementations */
>> +static inline int ppcemb_tlb_check(CPUState *env, ppcemb_tlb_t *tlb,
>> + target_phys_addr_t *raddrp,
>> + target_ulong address, uint32_t pid, int
>> ext,
>> + int i)
>
> Why is this both here and in helper.c?
Removed this version and exported the helper.c one.
>
>> +void helper_e500_tlbsx(target_ulong address_hi, target_ulong address_lo)
>> +{
>> + ppcemb_tlb_t *tlb = NULL;
>> + int i;
>> + target_phys_addr_t raddr;
>> + uint32_t spid, sas;
>> + uint32_t ea = (address_lo >> MAS2_EPN_SHIFT) & 0x7f;
>> +
>> + spid = (env->spr[SPR_BOOKE_MAS6] & MAS6_SPID_MASK) >> MAS6_SPID_SHIFT;
>> + sas = env->spr[SPR_BOOKE_MAS6] & MAS6_SAS;
>> +
>> + /* check possible TLB0 entries */
>> + for (i = 0; i < env->nb_ways; i++) {
>> + tlb = &env->tlb[ea | (i << 7)].tlbe;
>> +
>> + if (ppcemb_tlb_check(env, tlb, &raddr, address_lo, spid, 0, i) < 0)
>> {
>> + continue;
>> + }
>> +
>> + if (sas != (tlb->attr & MAS6_SAS)) {
>> + continue;
>> + }
>> +
>> + e500_tlb_to_mas(env, tlb, ea | (i << 7));
>> + return;
>> + }
>> +
>> + /* check TLB1 */
>> + for (i = env->nb_tlbs[0]; i < (env->nb_tlbs[0] + env->nb_tlbs[1]); i++)
>> {
>> + tlb = &env->tlb[i].tlbe;
>> +
>> + if (ppcemb_tlb_check(env, tlb, &raddr, address_lo, spid, 0, i) < 0)
>> {
>> + continue;
>> + }
>> +
>> + if (sas != (tlb->attr & MAS6_SAS)) {
>> + continue;
>> + }
>> +
>> + e500_tlb_to_mas(env, tlb, i);
>> + return;
>> + }
>
> This has a lot of overlap with mmue500_get_physical_address()...
It doesn't look as bad now :).
>
>> +
>> + /* no entry found, fill with defaults */
>> + env->spr[SPR_BOOKE_MAS0] = env->spr[SPR_BOOKE_MAS4] & MAS4_TLBSELD_MASK;
>> + env->spr[SPR_BOOKE_MAS1] = env->spr[SPR_BOOKE_MAS4] & MAS4_TSIZED_MASK;
>> + env->spr[SPR_BOOKE_MAS2] = env->spr[SPR_BOOKE_MAS4] & MAS4_WIMGED_MASK;
>> +
>> + if (env->spr[SPR_BOOKE_MAS6] & MAS6_SAS) {
>> + env->spr[SPR_BOOKE_MAS1] |= MAS1_TS;
>> + }
>> +
>> + env->spr[SPR_BOOKE_MAS1] |= (env->spr[SPR_BOOKE_MAS6] >> 16)
>> + << MAS1_TID_SHIFT;
>> +
>> + /* next victim logic */
>> + env->spr[SPR_BOOKE_MAS0] |= env->last_way << MAS0_ESEL_SHIFT;
>> + env->last_way++;
>> + env->last_way &= 3;
>> + env->spr[SPR_BOOKE_MAS0] |= env->last_way << MAS0_NV_SHIFT;
>
> ...and this with e500_update_mas_tlb_miss().
Yes, I need to think of a good way to generalize NV still. It isn't defined
that there's only a single NV available in the architecture either...
>
>> @@ -8433,7 +8505,7 @@ GEN_HANDLER2(icbt_40x, "icbt", 0x1F, 0x06, 0x08,
>> 0x03E00001, PPC_40x_ICBT),
>> GEN_HANDLER(iccci, 0x1F, 0x06, 0x1E, 0x00000001, PPC_4xx_COMMON),
>> GEN_HANDLER(icread, 0x1F, 0x06, 0x1F, 0x03E00001, PPC_4xx_COMMON),
>> GEN_HANDLER2(rfci_40x, "rfci", 0x13, 0x13, 0x01, 0x03FF8001, PPC_40x_EXCP),
>> -GEN_HANDLER(rfci, 0x13, 0x13, 0x01, 0x03FF8001, PPC_BOOKE),
>> +GEN_HANDLER_E(rfci, 0x13, 0x13, 0x01, 0x03FF8001, PPC_BOOKE,
>> PPC2_BOOKE_FSL),
>
> What is PPC2_BOOKE_FSL supposed to indicate?
>
> rfci is basic booke. It's in the 440.
It's also in e500, right? The flags there are a mask. Basically it means "this
opcode is available on PPC_BOOKE and PPC2_BOOKE_FSL capable CPUs".
>
>> +GEN_HANDLER2_E(icbt_440, "icbt", 0x1F, 0x16, 0x00, 0x03E00001,
>> + PPC_BOOKE, PPC2_BOOKE_FSL),
>
> "icbt_440" is FSL-specific? :-)
No, it's available on both :).
>
>> +static void spr_write_booke_pid (void *opaque, int sprn, int gprn)
>> +{
>> + gen_store_spr(sprn, cpu_gpr[gprn]);
>> + /* switching context, so need to flush tlb */
>> + gen_helper_tlbia();
>> +}
>
> Well, we want to flush the non-guest-visible TLB that doesn't understand
> PIDs -- but I'd expect "tlbia" to flush the architected TLB. 8xx, at
> least, has both tlbia and an architected TLB.
>
> Plus, we need ppc_tlb_invalidate_all() to clear the architected TLB, unless
> we call something different on reset.
Yep, changed all that into its own op_helper call that simply does tlb_flush().
If this should ever be more than a toy we should probably investigate on how to
add tags to qemu's tlb, so we don't constantly flush it.
>
>> @@ -1578,45 +1614,38 @@ static void gen_spr_BookE_FSL (CPUPPCState *env,
>> uint32_t mas_mask)
>> SPR_NOACCESS, SPR_NOACCESS,
>> &spr_read_generic, SPR_NOACCESS,
>> 0x00000000); /* TOFIX */
>> - /* XXX : not implemented */
>> - spr_register(env, SPR_MMUCSR0, "MMUCSR0",
>> - SPR_NOACCESS, SPR_NOACCESS,
>> - &spr_read_generic, &spr_write_generic,
>> - 0x00000000); /* TOFIX */
>> switch (env->nb_ways) {
>> case 4:
>> - /* XXX : not implemented */
>> spr_register(env, SPR_BOOKE_TLB3CFG, "TLB3CFG",
>> SPR_NOACCESS, SPR_NOACCESS,
>> &spr_read_generic, SPR_NOACCESS,
>> - 0x00000000); /* TOFIX */
>> + tlbncfg[3]);
>> /* Fallthru */
>> case 3:
>> - /* XXX : not implemented */
>> spr_register(env, SPR_BOOKE_TLB2CFG, "TLB2CFG",
>> SPR_NOACCESS, SPR_NOACCESS,
>> &spr_read_generic, SPR_NOACCESS,
>> - 0x00000000); /* TOFIX */
>> + tlbncfg[2]);
>> /* Fallthru */
>
> In some places you use nb_ways as the associativity of TLB0. But here it
> seems to be the number of TLB arrays?
It's always the number of TLB arrays. It's gone in v4 though :).
>
>> @@ -4334,8 +4373,23 @@ static void init_proc_e500 (CPUPPCState *env)
>> /* Memory management */
>> #if !defined(CONFIG_USER_ONLY)
>> env->nb_pids = 3;
>> + env->nb_ways = 2;
>> + env->id_tlbs = 0;
>> + if ((env->spr[SPR_PVR] & 0x00010000)) {
>> + /* e500v2 */
>> + env->nb_tlbs[0] = 512;
>> + tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, env->nb_tlbs[0]);
>> + tlbncfg[1] = gen_tlbncfg(16, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT,
>> 16);
>> + } else {
>> + /* e500v1 */
>> + env->nb_tlbs[0] = 256;
>> + tlbncfg[0] = gen_tlbncfg(2, 1, 1, 0, env->nb_tlbs[0]);
>> + tlbncfg[1] = gen_tlbncfg(16, 1, 9, TLBnCFG_AVAIL | TLBnCFG_IPROT,
>> 16);
>> + }
>
> It would be nice to be more precise with PVR testing, and complain if an
> unrecognized PVR is used (e.g. e500mc).
Alright, done.
Alex
- [Qemu-devel] [PATCH 0/6] PPC: Add FSL (e500) MMU emulation v3, Alexander Graf, 2011/05/02
- [Qemu-devel] [PATCH 2/6] PPC: Make MPC8544DS emulation work w/o KVM, Alexander Graf, 2011/05/02
- [Qemu-devel] [PATCH 1/6] PPC: Make MPC8544DS obey -cpu switch, Alexander Graf, 2011/05/02
- [Qemu-devel] [PATCH 3/6] PPC: Add GS MSR definition, Alexander Graf, 2011/05/02
- [Qemu-devel] [PATCH 6/6] PPC: Qdev'ify e500 pci, Alexander Graf, 2011/05/02
- [Qemu-devel] [PATCH 5/6] PPC: Implement e500 (FSL) MMU, Alexander Graf, 2011/05/02
[Qemu-devel] [PATCH 4/6] PPC: Add another 64 bits to instruction feature mask, Alexander Graf, 2011/05/02
Re: [Qemu-devel] [PATCH 0/6] PPC: Add FSL (e500) MMU emulation v3, Blue Swirl, 2011/05/04