From patchwork Tue Apr 22 20:56:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Murzin X-Patchwork-Id: 4044861 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 4337CBFF02 for ; Wed, 23 Apr 2014 21:00:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 45A382025A for ; Wed, 23 Apr 2014 21:00:11 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1967220219 for ; Wed, 23 Apr 2014 21:00:10 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Wd4F0-0000mu-0r; Wed, 23 Apr 2014 20:58:02 +0000 Received: from mail-we0-x232.google.com ([2a00:1450:400c:c03::232]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Wd4Eu-0000dM-Ps for linux-arm-kernel@lists.infradead.org; Wed, 23 Apr 2014 20:58:00 +0000 Received: by mail-we0-f178.google.com with SMTP id u56so1369225wes.23 for ; Wed, 23 Apr 2014 13:57:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=5rbJONgVMOQNn3e7VSJbxipDvtuRp+5j9hWzePzt2yw=; b=Nsh89PKp5m9hTo05GQd6pJMJZ8nGgEsf/Fiy4jegyqL48PXgxDNideNiHLQz/W8+iJ L9Nv7K3ov0m+YN8Ph7C6MN6Uim1nlAXBclVPyzwZ7OxkF4TDtzrPx9yUeh9NSkj1pSX0 SrBHJ7j1ZsEyCYV20F3XJDfdXLYrIztsV9+l3lpv43D0U/dX8+coInnWAaLSYcGpRym3 17mgny/oSWcBa0CSyLQqMoaLrLYdELTI5zIPmvZk+vjmjTy3L/xvpXwSpUw5IaJgFSUh 9iO/Riy8EuBxWUYS8vTuFe6DO6Kms7ggyzZEm78XzsBIz3UhAhHAsSmcBUP33Qt1In8L +Vww== X-Received: by 10.194.5.5 with SMTP id o5mr40147368wjo.16.1398286653238; Wed, 23 Apr 2014 13:57:33 -0700 (PDT) Received: from hp530 (cpc4-cmbg17-2-0-cust445.5-4.cable.virginm.net. [86.14.225.190]) by mx.google.com with ESMTPSA id gr2sm3065175wjc.12.2014.04.23.13.57.31 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 23 Apr 2014 13:57:32 -0700 (PDT) Date: Tue, 22 Apr 2014 21:56:57 +0100 From: Vladimir Murzin To: Ian Campbell Subject: Re: [Xen-devel] [PATCH RFC] arm64: 32-bit tolerant sync bitops Message-ID: <20140422205656.GA4467@hp530> References: <1397723881-31648-1-git-send-email-murzin.v@gmail.com> <534FB009.5030904@citrix.com> <1398241889.4880.96.camel@kazak.uk.xensource.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1398241889.4880.96.camel@kazak.uk.xensource.com> User-Agent: Mutt/1.5.22 (2013-10-16) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140423_135756_991855_B623A9C6 X-CRM114-Status: GOOD ( 29.23 ) X-Spam-Score: 0.4 (/) Cc: catalin.marinas@arm.com, will.deacon@arm.com, David Vrabel , JBeulich@suse.com, xen-devel@lists.xenproject.org, boris.ostrovsky@oracle.com, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00, DATE_IN_PAST_24_48, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Apr 23, 2014 at 09:31:29AM +0100, Ian Campbell wrote: > On Thu, 2014-04-17 at 11:42 +0100, David Vrabel wrote: > > On 17/04/14 09:38, Vladimir Murzin wrote: > > > Xen assumes that bit operations are able to operate on 32-bit size and > > > alignment [1]. For arm64 bitops are based on atomic exclusive load/store > > > instructions to guarantee that changes are made atomically. However, these > > > instructions require that address to be aligned to the data size. Because, by > > > default, bitops operates on 64-bit size it implies that address should be > > > aligned appropriately. All these lead to breakage of Xen assumption for bitops > > > properties. > > > > > > With this patch 32-bit sized/aligned bitops is implemented. > > > > > > [1] http://www.gossamer-threads.com/lists/xen/devel/325613 > > > > > > Signed-off-by: Vladimir Murzin > > > --- > > > Apart this patch other approaches were implemented: > > > 1. turn bitops to be 32-bit size/align tolerant. > > > the changes are minimal, but I'm not sure how broad side effect might be > > > 2. separate 32-bit size/aligned operations. > > > it exports new API, which might not be good > > > > I've never been particularly happy with the way the events_fifo.c uses > > casts for the sync_*_bit() calls and I think we should do option 2. > > > > A generic implementation could be something like: > > It seems like there isn't currently much call for/interest in a generic > 32-bit set of bitops. Since this is a regression on arm64 right now how > about we simply fix it up in the Xen layer now and if in the future a > common need is found we can rework to use it. > > We already have evtchn_fifo_{clear,set,is}_pending (although > evtchn_fifo_unmask and consume_one_event need fixing to use is_pending) > and evtchn_fifo_{test_and_set_,}mask, plus we would need > evtchn_fifo_set_mask for consume_one_event to use instead of open > coding. > > With that then those helpers can become something like: > #define BM(w) (unsigned long *)(w & ~0x7) > #define EVTCHN_FIFO_BIT(b, w) (BUG_ON(w&0x3), w & 0x4 ? EVTCHN_FIFO_#b + 32 : EVTCHN_FIFO_#b) > static void evtchn_fifo_clear_pending(unsigned port) > { > event_word_t *word = event_word_from_port(port); > sync_clear_bit(EVTCHN_FIFO_BIT(PENDING, word), BM(word)); > } > similarly for the others. > > If that is undesirable in the common code then wrap each existing helper > in #ifndef evtchn_fifo_clean_pending and put something like the above in > arch/arm64/include/asm/xen/events.h with a #define (and/or make the > helpers themselves macros, or #define HAVE_XEN_FIFO_EVTCHN_ACCESSORS etc > etc) > > We still need your patch from > http://article.gmane.org/gmane.comp.emulators.xen.devel/196067 for the > other unaligned bitmap case. Note that this also removes the use of BM > for non-PENDING/MASKED bits, which is why it was safe for me to change > it above (also it would be safe to & ~0x7 after that change anyway). > > Ian. > Might be dealing with it on Xen side is only way unless arm64 folks say what they think... because reports related to unaligned access still appear I'll put the minimally tested patch for making arm64 bitops 32-bits friendly ;) --- arch/arm64/lib/bitops.S | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/arch/arm64/lib/bitops.S b/arch/arm64/lib/bitops.S index 7dac371..7c3b058 100644 --- a/arch/arm64/lib/bitops.S +++ b/arch/arm64/lib/bitops.S @@ -26,14 +26,14 @@ */ .macro bitop, name, instr ENTRY( \name ) - and w3, w0, #63 // Get bit offset + and w3, w0, #31 // Get bit offset eor w0, w0, w3 // Clear low bits mov x2, #1 - add x1, x1, x0, lsr #3 // Get word offset + add x1, x1, x0, lsr #2 // Get word offset lsl x3, x2, x3 // Create mask -1: ldxr x2, [x1] - \instr x2, x2, x3 - stxr w0, x2, [x1] +1: ldxr w2, [x1] + \instr w2, w2, w3 + stxr w0, w2, [x1] cbnz w0, 1b ret ENDPROC(\name ) @@ -41,15 +41,15 @@ ENDPROC(\name ) .macro testop, name, instr ENTRY( \name ) - and w3, w0, #63 // Get bit offset + and w3, w0, #31 // Get bit offset eor w0, w0, w3 // Clear low bits mov x2, #1 - add x1, x1, x0, lsr #3 // Get word offset + add x1, x1, x0, lsr #2 // Get word offset lsl x4, x2, x3 // Create mask -1: ldxr x2, [x1] +1: ldxr w2, [x1] lsr x0, x2, x3 // Save old value of bit - \instr x2, x2, x4 // toggle bit - stlxr w5, x2, [x1] + \instr w2, w2, w4 // toggle bit + stlxr w5, w2, [x1] cbnz w5, 1b dmb ish and x0, x0, #1