RE: [PATCH v2] x86/asm: fix assembly constraints in bitops

From: David Laight <David.Laight@ACULAB.COM>
To: 'Alexander Potapenko' <glider@google.com>,
	"paulmck@linux.ibm.com" <paulmck@linux.ibm.com>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"peterz@infradead.org" <peterz@infradead.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dvyukov@google.com" <dvyukov@google.com>,
	"jyknight@google.com" <jyknight@google.com>,
	"x86@kernel.org" <x86@kernel.org>,
	"mingo@redhat.com" <mingo@redhat.com>
Subject: RE: [PATCH v2] x86/asm: fix assembly constraints in bitops
Date: Tue, 2 Apr 2019 11:45:20 +0000	[thread overview]
Message-ID: <4c6a1e592fd345618ef7b7d5bee592da@AcuMS.aculab.com> (raw)
In-Reply-To: <20190402112813.193378-1-glider@google.com>

From: Alexander Potapenko
> Sent: 02 April 2019 12:28
> 
> 1. Use memory clobber in bitops that touch arbitrary memory
> 
> Certain bit operations that read/write bits take a base pointer and an
> arbitrarily large offset to address the bit relative to that base.

Although x86_64 can use a signed 64bit bit number, looking at arm and arm64
they use 'int nr' throughout as do the generic functions.
Maybe x86 ought to be consistent here.
I doubt negative bit numbers are expected to work?

Did you try telling gcc that a big buffer (250MB is the limit for 32bit)
from the pointer might be changed?
That ought to be softer than a full 'memory' clobber as it should
only affect memory that could be accessed through the pointer.

....
> -#define BITOP_ADDR(x) "+m" (*(volatile long *) (x))
> +#define RLONG_ADDR(x) "m" (*(volatile long *) (x))
> +#define WBYTE_ADDR(x) "+m" (*(volatile char *) (x))
> 
> -#define ADDR				BITOP_ADDR(addr)
> +#define ADDR				RLONG_ADDR(addr)

Is it worth just killing ADDR ?
(as a different patch)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)