linux-mips
[Top] [All Lists]

Re: [PATCH] MIPS: make local_irq_disable macro safe for non-mipsr2

To: Jim Quinlan <jim2101024@gmail.com>
Subject: Re: [PATCH] MIPS: make local_irq_disable macro safe for non-mipsr2
From: David Daney <ddaney.cavm@gmail.com>
Date: Wed, 27 Nov 2013 12:58:26 -0800
Cc: ralf@linux-mips.org, linux-mips@linux-mips.org, cernekee@gmail.com
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=i8XvftrUHieCN6j9pTEdSMWn9YHWTse9fx3Wx5DkSEc=; b=auClh7JYIOWEzIX+JzfyZi07CQd1BNqKf4osuE9rDJaY7p2vpL73dkkoRgXoVb0k0g JOFYyAy07iNYwutTLN9xwcl7FCxZzcrBa/V+rfljB1psLqtakVJayxaoMwFus+Om1Vzf lEj4pvA7vLrayVVM2gRtyJCq3hSSSgNiVBVpB2jnPJxPUSGE+S8/4LMzdFt85WW4ZZ61 PMnLXh/AT+YSt+8Rw3FjNYLNGnW4Csio8HBdRMYTg0W1wYZ5Cqjf+8zz5rQxdlZsXrIC udEDaVFQ2AWX/OpJ3OqWhQGfjNxxhcS65uLXGBp960hgtZ6vGtkcekcydy4b8F/qEc70 eudw==
In-reply-to: <1385584490-20589-1-git-send-email-jim2101024@gmail.com>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <1385584490-20589-1-git-send-email-jim2101024@gmail.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7
On 11/27/2013 12:34 PM, Jim Quinlan wrote:
For non-mipsr2 processors, the local_irq_disable contains an mfc0-mtc0
pair with instructions inbetween.  With preemption enabled, this sequence
may get preempted and effect a stale value of CP0_STATUS when executing
the mtc0 instruction.  This commit avoids this scenario by incrementing
the preempt count before the mfc0 and decrementing it after the mtc9.

Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
---
  arch/mips/include/asm/asmmacro.h |   11 +++++++++++
  1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/mips/include/asm/asmmacro.h b/arch/mips/include/asm/asmmacro.h
index 6c8342a..3f809a4 100644
--- a/arch/mips/include/asm/asmmacro.h
+++ b/arch/mips/include/asm/asmmacro.h
@@ -9,6 +9,7 @@
  #define _ASM_ASMMACRO_H

  #include <asm/hazards.h>
+//#include <asm/asm-offsets.h>

I'm guessing it didn't pass checkpatch.pl


  #ifdef CONFIG_32BIT
  #include <asm/asmmacro-32.h>
@@ -54,11 +55,21 @@
        .endm

        .macro  local_irq_disable reg=t0
+#ifdef CONFIG_PREEMPT
+       lw      \reg, TI_PRE_COUNT($28)
+       addi    \reg, \reg, 1
+       sw      \reg, TI_PRE_COUNT($28)
+#endif

Does this need to be atomic?

More importantly, how does that prevent the problem you describe?

An interrupt can still occur between the mfc0 and mtc0 causing Status bits to be changed. What bits do we care about? is it IM*, I doubt you would see CU* changing.

Which bits are getting clobbered that shouldn't be?


        mfc0    \reg, CP0_STATUS
        ori     \reg, \reg, 1
        xori    \reg, \reg, 1
        mtc0    \reg, CP0_STATUS
        irq_disable_hazard
+#ifdef CONFIG_PREEMPT
+       lw      \reg, TI_PRE_COUNT($28)
+       addi    \reg, \reg, -1
+       sw      \reg, TI_PRE_COUNT($28)
+#endif
        .endm
  #endif /* CONFIG_MIPS_MT_SMTC */




<Prev in Thread] Current Thread [Next in Thread>