Date: Tue, 4 Oct 2011 03:23:11 +0530 From: "Jayachandran C." <jchandra@freebsd.org> To: Adrian Chadd <adrian@freebsd.org> Cc: Kostik Belousov <kostikbel@gmail.com>, Alexander Motin <mav@freebsd.org>, freebsd-mips@freebsd.org Subject: Re: svn commit: r225892 - head/sys/mips/mips Message-ID: <CA%2B7sy7Cin5-cHcP-8_qYGhpEnAN9gw6S5ekXYK6Q3X9FREQggA@mail.gmail.com> In-Reply-To: <CA%2B7sy7Ax9SXSK1CyxuBNboktJxuQTMiu3D4NFmZSoq7-ipoQgA@mail.gmail.com> References: <CA%2B7sy7BiRvTB79H9=y%2BS4jQ=%2BboW1bcDJn%2BBULMmJU9KLLVJ5A@mail.gmail.com> <CAJ-VmokAsDpjJLt%2BVJ2gDGX%2BiMAwZvL2TPaaAD_LRm-Yyquxig@mail.gmail.com> <CA%2B7sy7D6h5a08Q6yNfX6xSqwabDLzE5GLu5aV3fCMYQKn_4AoQ@mail.gmail.com> <CAJ-Vmon32cVEVvC=3WJVmDkCUdyLWyec3sqU-ifzspVSPxedfg@mail.gmail.com> <CAJ-Vmomsq5PQzbCBmWob5juB9EqdcEoYV%2B9vwYjnJQYTo_%2B4kw@mail.gmail.com> <CAJ-Vmon_a_zLZmEGqwFaYaobjYFE2i1u2Viq3QD5dw4wpNNURA@mail.gmail.com> <CA%2B7sy7DFCMxo-2bJwBJcSEJf7ewG7Y=XwdgKXkhpRyDXQpvsYA@mail.gmail.com> <CAJ-VmokPFqS2oNWZ_mFSxy=0MXfgqtOcBHSQe%2BdYXvsLHAyGjQ@mail.gmail.com> <CAJ-VmomqmKPRHBCbt46_xXD0VoU47Q-vYWbAqCFaM635ZnOHWA@mail.gmail.com> <CAJ-VmomLbueaG3bmnT0WfeKaMSyXSNo80BWXqEe39z6x%2Bx8QoA@mail.gmail.com> <20111002110331.GF1511@deviant.kiev.zoral.com.ua> <CA%2B7sy7A%2Bq_N6Hr%2B3-tD=BJxmqtDgBeWF9HJCtopLF0RUz6hVyw@mail.gmail.com> <CA%2B7sy7Ax9SXSK1CyxuBNboktJxuQTMiu3D4NFmZSoq7-ipoQgA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Tue, Oct 4, 2011 at 12:34 AM, Jayachandran C. <jchandra@freebsd.org> wrote:
> On Mon, Oct 3, 2011 at 10:19 PM, Jayachandran C. <jchandra@freebsd.org> wrote:
>> Hi Adrain,
>>
>> On Sun, Oct 2, 2011 at 4:33 PM, Kostik Belousov <kostikbel@gmail.com> wrote:
>>> On Sun, Oct 02, 2011 at 05:28:25PM +0800, Adrian Chadd wrote:
>>>> Hi,
>>>>
>>>> It doesn't look like openbsd or netbsd have tried addressing this.
>>>>
>>>> I took a shot at trying to port over the relevant bits.
>>>>
>>>> Linux seems to store the "can reschedule" flag in a bit of memory,
>>>> rather than calling a function to check each time.
>>>> This means that I can't simply port r4k_wait() verbose; there's no
>>>> guarantee EPC would be pointing to inside r4k_wait if it had to call
>>>> sched_runnable().
>>>>
>>>> Also, since we are calling 'wait' inside a critical section, any EPC
>>>> unwinding would have to also unwind the critical section (and maybe
>>>> reprogram the timer) before restarting things. But since that now
>>>> makes the "rollback" section even larger and unwieldy.
>>>>
>>>> I really don't have the time or brain power at the moment to try and
>>>> port this solution over from Linux.
>>>>
>>>> I would really appreciate it if someone would help out here.
>>>
>>> I probably need to describe some details of the mentioned "kib' idea".
>>>
>>> Looking at the x86 sti; hlt sequence, I noted that, in fact, we do
>>> not strictly need the special CPU behaviour of delaying enabling the
>>> interrupts till next instruction after sti is executed. The race there
>>> is the interrupt happen right after sti but before hlt, causing CPU
>>> to enter halted state while potentially having runnable thread. On x86
>>> it is closed by sti not enabling interrupts till hlt started execution
>>> (it is more subtle, but let ignore the detail for the discussion).
>>>
>>> Now, if sti would not offer the useful postponing behaviour, we can
>>> easily emulate it. In the hardware interrupt handlers return path, we
>>> can check for $pc being equal to the address of the hlt instruction. If
>>> it is, we can advance $pc over the hlt, avoiding the halt if potentially
>>> we have a runnable thread.
>>>
>>> Briefly looking over the MIPS64 specifications, I do not see why we cannot
>>> implement the spirit of the trick for the ei; wait instruction sequence.
>>> ray talked about possibility of $pc living in the shadow register bank,
>>> which I think is not important. Another (minor) issue seems to be
>>> that our code does not use ei, directly manipulating the bit.
>>>
>>> My belief is that the trick can be done if only we have exact
>>> interrupts. It seems, from "run mips run" text, that possible inexact
>>> interrupts are either for much older platforms then modern MIPS SoC, or
>>> are irrelant there, because inexactness is only related to mul/div unit.
>>>
>>> [I am not on mips@]
>>
>> I have implemented a variant of this, can you try out the attached
>> patch and see how it goes? It should apply on the version before your
>> changes to machdep.c
>>
>> Also, if anybody on mips@ can review the code, it would be helpful...
>
> Actually there are two issues with this, the first is a simple bug, it
> should be :
> mtc0 t1, MIPS_COP_0_STATUS
>
> The second one is that a move to the status register should followed
> by a COP0 write hazard on some mips platforms (although not on
> XLR/XLP). Adding this hazard would make the calculations more
> complex....
This version should be better(thanks to Juli for suggestions). Can
you see if this helps?
Thanks,
JC
[-- Attachment #2 --]
diff --git a/sys/mips/include/md_var.h b/sys/mips/include/md_var.h
index c2a6155..6f65a0f 100644
--- a/sys/mips/include/md_var.h
+++ b/sys/mips/include/md_var.h
@@ -56,6 +56,7 @@ void MipsSwitchFPState(struct thread *, struct trapframe *);
u_long kvtop(void *addr);
int is_cacheable_mem(vm_paddr_t addr);
void mips_generic_reset(void);
+void mips_wait(void);
#define MIPS_DEBUG 0
diff --git a/sys/mips/mips/exception.S b/sys/mips/mips/exception.S
index 729391e..aad0c7a 100644
--- a/sys/mips/mips/exception.S
+++ b/sys/mips/mips/exception.S
@@ -557,6 +557,33 @@ NNON_LEAF(MipsUserGenException, CALLFRAME_SIZ, ra)
.set at
END(MipsUserGenException)
+ .set push
+ .set noat
+NON_LEAF(mips_wait, CALLFRAME_SIZ, ra)
+ PTR_SUBU sp, sp, CALLFRAME_SIZ
+ .mask 0x80000000, (CALLFRAME_RA - CALLFRAME_SIZ)
+ REG_S ra, CALLFRAME_RA(sp) # save RA
+ mfc0 t0, MIPS_COP_0_STATUS
+ xori t1, t0, MIPS_SR_INT_IE
+ mtc0 t1, MIPS_COP_0_STATUS
+ COP0_SYNC
+ jal sched_runnable
+ nop
+ REG_L ra, CALLFRAME_RA(sp)
+ mfc0 t0, MIPS_COP_0_STATUS
+ ori t1, t0, MIPS_SR_INT_IE
+ .align 4
+StartWaitSkip: # this is 16 byte aligned
+ mtc0 t1, MIPS_COP_0_STATUS
+ bnez v0, EndWaitSkip
+ nop
+ wait
+EndWaitSkip: # StartWaitSkip + 16
+ jr ra
+ PTR_ADDU sp, sp, CALLFRAME_SIZ
+END(mips_wait)
+ .set pop
+
/*----------------------------------------------------------------------------
*
* MipsKernIntr --
@@ -578,6 +605,19 @@ NNON_LEAF(MipsKernIntr, KERN_EXC_FRAME_SIZE, ra)
.set noat
PTR_SUBU sp, sp, KERN_EXC_FRAME_SIZE
.mask 0x80000000, (CALLFRAME_RA - KERN_EXC_FRAME_SIZE)
+
+/*
+ * Check for getting interrupts just before wait
+ */
+ MFC0 k0, MIPS_COP_0_EXC_PC
+ ori k0, 0xf
+ xori k0, 0xf # 16 byte align
+ PTR_LA k1, StartWaitSkip
+ bne k0, k1, 1f
+ nop
+ PTR_ADDU k1, 16 # skip over wait
+ MTC0 k1, MIPS_COP_0_EXC_PC
+1:
/*
* Save CPU state, building 'frame'.
*/
diff --git a/sys/mips/mips/machdep.c b/sys/mips/mips/machdep.c
index f7e5248..a0725a9 100644
--- a/sys/mips/mips/machdep.c
+++ b/sys/mips/mips/machdep.c
@@ -497,7 +497,7 @@ cpu_idle(int busy)
critical_enter();
cpu_idleclock();
}
- __asm __volatile ("wait");
+ mips_wait();
if (!busy) {
cpu_activeclock();
critical_exit();
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2B7sy7Cin5-cHcP-8_qYGhpEnAN9gw6S5ekXYK6Q3X9FREQggA>