Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754345AbbLVOqY (ORCPT ); Tue, 22 Dec 2015 09:46:24 -0500 Received: from pandora.arm.linux.org.uk ([78.32.30.218]:42741 "EHLO pandora.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753958AbbLVOqX (ORCPT ); Tue, 22 Dec 2015 09:46:23 -0500 Date: Tue, 22 Dec 2015 14:46:06 +0000 From: Russell King - ARM Linux To: Stefano Stabellini Cc: jaccon.bastiaansen@gmail.com, arnd@arndb.de, catalin.marinas@arm.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org, will.deacon@arm.com Subject: Re: [PATCH RESEND v4] arm: remove !CPU_V6 and !GENERIC_ATOMIC64 build dependencies for XEN Message-ID: <20151222144605.GJ8644@n2100.arm.linux.org.uk> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2435 Lines: 73 On Tue, Dec 22, 2015 at 02:17:17PM +0000, Stefano Stabellini wrote: > Hello Russell, > > Would you please consider this patch for the next merge window? It > is a very old patch (March 2014) which has been left over. This patch has some obvious problems... > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 34e1569..e09ed94 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1807,8 +1807,7 @@ config XEN_DOM0 > config XEN > bool "Xen guest support on ARM" > depends on ARM && AEABI && OF > - depends on CPU_V7 && !CPU_V6 > - depends on !GENERIC_ATOMIC64 > + depends on CPU_V7 How sure are we that this won't cause regressions? How much testing has been done with these changed dependencies? > diff --git a/arch/arm/include/asm/sync_bitops.h b/arch/arm/include/asm/sync_bitops.h > index 9732b8e..4103319 100644 > --- a/arch/arm/include/asm/sync_bitops.h > +++ b/arch/arm/include/asm/sync_bitops.h > @@ -20,7 +20,29 @@ > #define sync_test_and_clear_bit(nr, p) _test_and_clear_bit(nr, p) > #define sync_test_and_change_bit(nr, p) _test_and_change_bit(nr, p) > #define sync_test_bit(nr, addr) test_bit(nr, addr) > -#define sync_cmpxchg cmpxchg > > +static inline unsigned long sync_cmpxchg(volatile void *ptr, > + unsigned long old, > + unsigned long new) > +{ > + unsigned long oldval; > + int size = sizeof(*(ptr)); This is buggy. You're doing sizeof(void) here, which on GCC will always be 1: A consequence of this is that `sizeof' is also allowed on `void' and on function types, and returns 1. > + > + smp_mb(); > + switch (size) { > + case 1: > + oldval = __cmpxchg8(ptr, old, new); > + break; > + case 2: > + oldval = __cmpxchg16(ptr, old, new); > + break; The ldrexb/ldrexh instructions are not available on ARMv6 CPUs. __xchg() and friends avoided these for ARMv6 CPUs, but this does not. I'd expect anything that uses sync_cmpxchg() will fail to build when a kernel including ARMv6 is attempted. So... I don't think this patch is ready. -- RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/