Date: Fri, 2 Jul 2010 01:34:35 +0530 From: "Jayachandran C." <c.jayachandran@gmail.com> To: Luiz Otavio O Souza <lists.br@gmail.com> Cc: freebsd-mips@freebsd.org Subject: Re: Merging 64 bit changes to -HEAD Message-ID: <AANLkTinY_RL3PGZ7UaLi16vitt_iag4MoWOImAe8JB06@mail.gmail.com> In-Reply-To: <AANLkTinGkedtaE0EDUWaK-Csjiy1r5pIK5U9uKWTD_PS@mail.gmail.com> References: <AANLkTik8jFkB7FTIIhyjalkfv1c0yXqse57Jzz527uf_@mail.gmail.com> <897604F6-95C4-49A8-B11F-277A74C8DBAE@gmail.com> <AANLkTilfW_zOFKuIa0gJ3ahTo-vGC1VNk99a1H24uFRq@mail.gmail.com> <AANLkTil78NFxH016C7MntD8L3d4rFlCudJ0Lv22L0KCb@mail.gmail.com> <3C0AEF9B-AE0C-4459-A4E1-2C8C30C10FD6@gmail.com> <AANLkTint7Hyf79EH29OLsIfreQRd7dQMdvX9wRq4v_yG@mail.gmail.com> <C6D73C96-3640-4502-A9D7-B3597E37E80A@gmail.com> <AANLkTilQIqF4FCfgLdVcKdcsAUVjCmr89Lu0TEXUFdYN@mail.gmail.com> <25B9A19D-0A6B-4731-8FB1-A2C6722F0E9C@gmail.com> <AANLkTim_9-G6ZuA0vkpLeG4n4GVBpBlxhGBy_7eQoIM4@mail.gmail.com> <C1BD2512-8A48-4AB2-B9CA-C46DDCBE5256@gmail.com> <AANLkTimgyYggdDYchEfb3yskmA0PsttjVzkOSMaWFsjH@mail.gmail.com> <7886D15B-79BF-4BC6-8467-26A8D0FE3D00@gmail.com> <AANLkTinGkedtaE0EDUWaK-Csjiy1r5pIK5U9uKWTD_PS@mail.gmail.com>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] On Fri, Jul 2, 2010 at 12:17 AM, Jayachandran C. <c.jayachandran@gmail.com> wrote: > On Thu, Jul 1, 2010 at 10:25 PM, Luiz Otavio O Souza <lists.br@gmail.com> wrote: >> On Jun 30, 2010, at 7:40 PM, Jayachandran C. wrote: >>> On Wed, Jun 30, 2010 at 10:38 PM, Luiz Otavio O Souza >>> <lists.br@gmail.com> wrote: >>>> On Jun 30, 2010, at 9:57 AM, Jayachandran C. wrote: >>>> >>>>> On Tue, Jun 29, 2010 at 10:32 PM, Luiz Otavio O Souza >>>>> <lists.br@gmail.com> wrote: >>>>>> >>>>>> On Jun 29, 2010, at 8:02 AM, Jayachandran C. wrote: >>>>>> >>>>>>> On Tue, Jun 29, 2010 at 2:28 AM, Luiz Otavio O Souza <lists.br@gmail.com> wrote: >>>>>>>>> Thanks for the the update. Looks like pmap_map for kernel is failing, >>>>>>>>> may be the new tlb_update code causes this. Can you apply the >>>>>>>>> attached patch and see if the problem still persists, it replaces the >>>>>>>>> new tlb_update code with the older version. >>>>>>>>> >>>>>>>>> Obviously not a fix, but if we can narrow it down to this function, >>>>>>>>> fixing will be easier. >>>>>>>>> >>>>>>>>> JC. >>>>>>>>> <try.diff> >>>>>>>> >>>>>>>> JC, >>>>>>>> >>>>>>>> This fix the problem ! Thanks ! Now, at least, you know where to look :) >>>>>>> >>>>>>> The new tlb_update does not seem to update the tlb entry if the tlbp >>>>>>> fails. Here's a patch that should make the new function behave like >>>>>>> the older one. The patch is in attached file 'tlb-update.diff'. >>>>>>> >>>>>>> If that does not work, I'm not sure what the issue is. You could also >>>>>>> try try the nop-change.diff attached. It tries to switch the ssnop >>>>>>> used for delay in the new code with 'nop' which was used by the old >>>>>>> code. >>>>>>> >>>>>>> Thanks, >>>>>>> JC. >>>>>>> <tlb-update.diff><nop-change.diff> >>>>>> >>>>>> JC, >>>>>> >>>>>> The nop-change seems to have no effect at all and with the tlb-update patch the kernel apparently crash at bzero(), here is the dmesg with TRAP_DEBUG enabled: >>>>>> >>>>>> http://mips.pastebin.com/jydPvJ20 >>>>>> >>>>>> So hopefully you are on the right track and this may be something obvious to you. >>>>> >>>>> Not yet :) I really hoped the earlier change would fix it. The number >>>>> of nop does not seem to be the issue as it is higher in the C code >>>>> than the assembly. >>>>> >>>>> Can you try the attached patch (try.diff) - this re-implements the >>>>> assembly code functionality almost in the same way in C. This really >>>>> should work, given that the patch which made it assembly worked... >>>>> >>>>> If that works can you see if the second attached patch works, this >>>>> fixes a potential problem (ie, we should be masking 13bits for TLBHI). >>>>> >>>>> Both patches should apply directly to SVN (not dependent on each >>>>> other, or on previous patches) >>>>> >>>>> Thanks again, >>>>> JC. >>>>> <try.diff><pte.h-fix.diff> >>>> >>>> >>>> JC, >>>> >>>> The try.diff works with or without the pte.h change (at least for a simple boot) and the pte.h change does nothing without the try.diff. >>> >>> I've attached the final(final.diff) version I want to check-in, can >>> you please quickly test it? >>> >>> If that does not work, can you tell me if the attached alt1.diff or >>> alt2.diff works? The try.diff had three changes: handle case of >>> index>0, remove pagemask operation, restore full entryhi instead of >>> asid. So if the first does not work, this will help narrow down the >>> rest of the cases. >>> >>> Hopefully this is the last iteration :) >>> >>> JC. >>> <final.diff><alt1.diff><alt2.diff> >> >> >> JC, >> >> The final.diff and alt1.diff crash (into debugger) like the first try.diff (here is the dmesg: http://pastebin.com/r2uunZPZ). >> >> The alt2.diff works, i'll try it a little more (buildworld). >> >> Feel free to send more tests if needed. > > Looks like I was on the wrong track, it now looks like the pagemask > maybe the cause. My suspicion is that the bootloader setups a > pagemask and we never clear it because all the operations save and > restore the mask. As far as I can see the TLB exception handler will > not update pagemask. > > Juli - any comments on this? Do you need to save/restore pagemask for > some reason, otherwise I will take out the part from tlb.c. Will send > out a patch a bit later. Okay - here's hopefully the final patch. Let me know if it boots up. Thanks, JC. [-- Attachment #2 --] Index: sys/mips/include/pte.h =================================================================== --- sys/mips/include/pte.h (revision 209635) +++ sys/mips/include/pte.h (working copy) @@ -73,7 +73,8 @@ * Note that in FreeBSD, we map 2 TLB pages is equal to 1 VM page. */ #define TLBHI_ASID_MASK (0xff) -#define TLBHI_ENTRY(va, asid) (((va) & ~PAGE_MASK) | ((asid) & TLBHI_ASID_MASK)) +#define TLBHI_PAGE_MASK (2 * PAGE_SIZE - 1) +#define TLBHI_ENTRY(va, asid) (((va) & ~TLBHI_PAGE_MASK) | ((asid) & TLBHI_ASID_MASK)) #ifndef _LOCORE typedef uint32_t pt_entry_t; Index: sys/mips/mips/tlb.c =================================================================== --- sys/mips/mips/tlb.c (revision 209635) +++ sys/mips/mips/tlb.c (working copy) @@ -91,13 +91,12 @@ void tlb_insert_wired(unsigned i, vm_offset_t va, pt_entry_t pte0, pt_entry_t pte1) { - register_t mask, asid; + register_t asid; register_t s; va &= ~PAGE_MASK; s = intr_disable(); - mask = mips_rd_pagemask(); asid = mips_rd_entryhi() & TLBHI_ASID_MASK; mips_wr_index(i); @@ -108,21 +107,19 @@ tlb_write_indexed(); mips_wr_entryhi(asid); - mips_wr_pagemask(mask); intr_restore(s); } void tlb_invalidate_address(struct pmap *pmap, vm_offset_t va) { - register_t mask, asid; + register_t asid; register_t s; int i; va &= ~PAGE_MASK; s = intr_disable(); - mask = mips_rd_pagemask(); asid = mips_rd_entryhi() & TLBHI_ASID_MASK; mips_wr_pagemask(0); @@ -133,38 +130,34 @@ tlb_invalidate_one(i); mips_wr_entryhi(asid); - mips_wr_pagemask(mask); intr_restore(s); } void tlb_invalidate_all(void) { - register_t mask, asid; + register_t asid; register_t s; unsigned i; s = intr_disable(); - mask = mips_rd_pagemask(); asid = mips_rd_entryhi() & TLBHI_ASID_MASK; for (i = mips_rd_wired(); i < num_tlbentries; i++) tlb_invalidate_one(i); mips_wr_entryhi(asid); - mips_wr_pagemask(mask); intr_restore(s); } void tlb_invalidate_all_user(struct pmap *pmap) { - register_t mask, asid; + register_t asid; register_t s; unsigned i; s = intr_disable(); - mask = mips_rd_pagemask(); asid = mips_rd_entryhi() & TLBHI_ASID_MASK; for (i = mips_rd_wired(); i < num_tlbentries; i++) { @@ -191,7 +184,6 @@ } mips_wr_entryhi(asid); - mips_wr_pagemask(mask); intr_restore(s); } @@ -217,7 +209,7 @@ void tlb_update(struct pmap *pmap, vm_offset_t va, pt_entry_t pte) { - register_t mask, asid; + register_t asid; register_t s; int i; @@ -225,7 +217,6 @@ pte &= ~TLBLO_SWBITS_MASK; s = intr_disable(); - mask = mips_rd_pagemask(); asid = mips_rd_entryhi() & TLBHI_ASID_MASK; mips_wr_pagemask(0); @@ -244,7 +235,6 @@ } mips_wr_entryhi(asid); - mips_wr_pagemask(mask); intr_restore(s); }home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinY_RL3PGZ7UaLi16vitt_iag4MoWOImAe8JB06>