Dear Miod-san, Hello! Thank you for your reply.
>I believe some parts of this diff are incorrect. >> @@ -90,16 +90,19 @@ >> NOP >> DMFC0 k1, COP_0_UBASE # PCB_SEGTAB(CI_CURPROCPADDR(curcpu)) >> MFC0_HAZARD >> + PTR_SRL k0, SEGSHIFT >Why did you add this shift ? It is done a few lines before, before the >bounds check, and k0 is not modified after. This instruction should not >be necessary. >> PTR_ADDU k1, k0 >> lwu k0, 0(k1) # get pte >> + dsll k0, k0, 24 >> + dsrl k0, k0, 24 >These dsll/dsrl instructions have no effect. By using `lwu' to load the >pte, the upper 32 bits of k0 are set to 0; using these dsll/dsrl >instructions clear the upper 24 bits again. >> DMTC0 k0, COP_0_TLB_LO >> MTC0_HAZARD >> TLBW >> @@ -163,6 +166,8 @@ >> MFC0_HAZARD >> PTR_ADDU k1, k0 >> lwu k0, 0(k1) # get pte >> + dsll k0, k0, 24 >> + dsrl k0, k0, 24 >> DMTC0 k0, COP_0_TLB_LO >> MTC0_HAZARD >> TLBW >> @@ -221,12 +226,14 @@ >> MFC0_HAZARD >> PTR_ADDU k1, k0 >> lwu k0, 0(k1) # get pte >> - andi k1, k0, PG_V >> - beqz k1, k_general # if not valid >> - NOP >> + dsll k0, k0, 24 >> + dsrl k0, k0, 24 >> + DMTC0 k0, COP_0_TLB_LO >> + MTC0_HAZARD >> + and k0, k0, PG_V >> + beqz k0, k_general # if not valid >> + NOP >This causes a TLB entry to be updated even if it is not valid. But if we >are running this code, the TLB entry was already invalid! Did you see >any change in behaviour with this change? I am sorry for the confusion. I imitate mips64/mips64/tlbhandler.S. (http://bxr.su/OpenBSD/sys/arch/mips64/mips64/tlbhandler.S#157 etc...) Just because. Please ignore. >> PTR_SLL k0, LOGREGSZ >> PTR_ADDU k1, k0 >> PTR_L k1, 0(k1) # get pointer to page table >> DMFC0 k0, COP_0_WORK0 # saved COP_0_VADDR >> MFC0_HAZARD >> beqz k1, _inv_seg >> - PTR_SRL k0, PAGE_SHIFT - 2 >> - andi k0, ((NPTEPG / 2) - 1) << 2 >> + PTR_SRL k0, PAGE_SHIFT >> + andi k0, NPTEPG - 1 >The 2 in the original code is log2(pte size); k0 >> PAGE_SHIFT will be >the pte number. But in the page table, it is stored as an array of >32-bit words, so we need to shift it to the left by 2. The original >instructions: > PTR_SRL k0, PAGE_SHIFT - 2 > andi k0, ((NPTEPG / 2) - 1) << 2 >are equivalent to: > PTR_SRL k0, PAGE_SHIFT > andi k0, (NPTEPG / 2) - 1 > PTR_SLL k0, 2 >and guarantees the address is correctly aligned for the `lwu' >instruction later. >However, I think you are right changing NPTEPG / 2 into NPTEPG - this >looks like a real bug. >so we need to shift it to the left by 2. Thank you for your easy to understand answer. I cannot understand this code. However, I think bug lurk here. I refer to "#define uvtopte(va) (((va) >> PAGE_SHIFT) & (NPTEPG -1))" ! >With the NPTEPG / 2 fix only, my IP26 runs multiuser with a kernel built >from unmodified CVS HEAD sources. SCSI and Ethernet appear to work >correctly. Thank you so much. "ARIGATO!" >I have enabled IP26 builds in the OpenBSD/sgi release logic; the next >snapshot should contain everything needed to install on IP26 (including >the correct /usr/mdec/boot-IP26 link so that the boot loader can be >installed by bsd.rd.IP26). and what is more,Japanese win against South Africa at RWC2015. What a wonderful day! --- Naruaki Etomi nullnil...@gmail.com
