Remove !GENERIC_ATOMIC64 build dependency:
- rename atomic64_xchg to armv7_atomic64_xchg and define it even ifdef
GENERIC_ATOMIC64;
- call armv7_atomic64_xchg directly from xen/events.h.
Remove !CPU_V6 build dependency:
- introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
CONFIG_CPU_V6;
- implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16.
Signed-off-by: Stefano Stabellini <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
Changes in v3:
- rebase.
Changes in v2:
- handle return value of __cmpxchg8 and __cmpxchg16 in __cmpxchg;
- remove sync_cmpxchg to sync_cmpxchg16 rename in grant-table.c,
implement a generic sync_cmpxchg instead.
---
arch/arm/Kconfig | 3 +-
arch/arm/include/asm/atomic.h | 47 +++++++++++++++-------------
arch/arm/include/asm/cmpxchg.h | 60 ++++++++++++++++++++++++------------
arch/arm/include/asm/sync_bitops.h | 24 ++++++++++++++-
arch/arm/include/asm/xen/events.h | 2 +-
5 files changed, 91 insertions(+), 45 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c1f1a7e..ae54ae0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1881,8 +1881,7 @@ config XEN_DOM0
config XEN
bool "Xen guest support on ARM (EXPERIMENTAL)"
depends on ARM && AEABI && OF
- depends on CPU_V7 && !CPU_V6
- depends on !GENERIC_ATOMIC64
+ depends on CPU_V7
select ARM_PSCI
select SWIOTLB_XEN
help
diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h
index 62d2cb5..dfe1cd6 100644
--- a/arch/arm/include/asm/atomic.h
+++ b/arch/arm/include/asm/atomic.h
@@ -382,26 +382,7 @@ static inline long long atomic64_cmpxchg(atomic64_t *ptr, long long old,
return oldval;
}
-static inline long long atomic64_xchg(atomic64_t *ptr, long long new)
-{
- long long result;
- unsigned long tmp;
-
- smp_mb();
-
- __asm__ __volatile__("@ atomic64_xchg\n"
-"1: ldrexd %0, %H0, [%3]\n"
-" strexd %1, %4, %H4, [%3]\n"
-" teq %1, #0\n"
-" bne 1b"
- : "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter)
- : "r" (&ptr->counter), "r" (new)
- : "cc");
-
- smp_mb();
-
- return result;
-}
+#define armv7_atomic64_xchg atomic64_xchg
static inline long long atomic64_dec_if_positive(atomic64_t *v)
{
@@ -469,6 +450,30 @@ static inline int atomic64_add_unless(atomic64_t *v, long long a, long long u)
#define atomic64_dec_and_test(v) (atomic64_dec_return((v)) == 0)
#define atomic64_inc_not_zero(v) atomic64_add_unless((v), 1LL, 0LL)
-#endif /* !CONFIG_GENERIC_ATOMIC64 */
+#else /* !CONFIG_GENERIC_ATOMIC64 */
+#include <asm-generic/atomic64.h>
+#endif
+
+static inline u64 armv7_atomic64_xchg(atomic64_t *ptr, u64 new)
+{
+ long long result;
+ unsigned long tmp;
+
+ smp_mb();
+
+ __asm__ __volatile__("@ armv7_atomic64_xchg\n"
+"1: ldrexd %0, %H0, [%3]\n"
+" strexd %1, %4, %H4, [%3]\n"
+" teq %1, #0\n"
+" bne 1b"
+ : "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter)
+ : "r" (&ptr->counter), "r" (new)
+ : "cc");
+
+ smp_mb();
+
+ return result;
+}
+
#endif
#endif
diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index df2fbba..cc8a4a2 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -133,6 +133,44 @@ extern void __bad_cmpxchg(volatile void *ptr, int size);
* cmpxchg only support 32-bits operands on ARMv6.
*/
+static inline unsigned long __cmpxchg8(volatile void *ptr, unsigned long old,
+ unsigned long new)
+{
+ unsigned long oldval, res;
+
+ do {
+ asm volatile("@ __cmpxchg1\n"
+ " ldrexb %1, [%2]\n"
+ " mov %0, #0\n"
+ " teq %1, %3\n"
+ " strexbeq %0, %4, [%2]\n"
+ : "=&r" (res), "=&r" (oldval)
+ : "r" (ptr), "Ir" (old), "r" (new)
+ : "memory", "cc");
+ } while (res);
+
+ return oldval;
+}
+
+static inline unsigned long __cmpxchg16(volatile void *ptr, unsigned long old,
+ unsigned long new)
+{
+ unsigned long oldval, res;
+
+ do {
+ asm volatile("@ __cmpxchg1\n"
+ " ldrexh %1, [%2]\n"
+ " mov %0, #0\n"
+ " teq %1, %3\n"
+ " strexheq %0, %4, [%2]\n"
+ : "=&r" (res), "=&r" (oldval)
+ : "r" (ptr), "Ir" (old), "r" (new)
+ : "memory", "cc");
+ } while (res);
+
+ return oldval;
+}
+
static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
unsigned long new, int size)
{
@@ -141,28 +179,10 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
switch (size) {
#ifndef CONFIG_CPU_V6 /* min ARCH >= ARMv6K */
case 1:
- do {
- asm volatile("@ __cmpxchg1\n"
- " ldrexb %1, [%2]\n"
- " mov %0, #0\n"
- " teq %1, %3\n"
- " strexbeq %0, %4, [%2]\n"
- : "=&r" (res), "=&r" (oldval)
- : "r" (ptr), "Ir" (old), "r" (new)
- : "memory", "cc");
- } while (res);
+ oldval = __cmpxchg8(ptr, old, new);
break;
case 2:
- do {
- asm volatile("@ __cmpxchg1\n"
- " ldrexh %1, [%2]\n"
- " mov %0, #0\n"
- " teq %1, %3\n"
- " strexheq %0, %4, [%2]\n"
- : "=&r" (res), "=&r" (oldval)
- : "r" (ptr), "Ir" (old), "r" (new)
- : "memory", "cc");
- } while (res);
+ oldval = __cmpxchg16(ptr, old, new);
break;
#endif
case 4:
diff --git a/arch/arm/include/asm/sync_bitops.h b/arch/arm/include/asm/sync_bitops.h
index 63479ee..942659a 100644
--- a/arch/arm/include/asm/sync_bitops.h
+++ b/arch/arm/include/asm/sync_bitops.h
@@ -21,7 +21,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));
+
+ smp_mb();
+ switch (size) {
+ case 1:
+ oldval = __cmpxchg8(ptr, old, new);
+ break;
+ case 2:
+ oldval = __cmpxchg16(ptr, old, new);
+ break;
+ default:
+ oldval = __cmpxchg(ptr, old, new, size);
+ break;
+ }
+ smp_mb();
+
+ return oldval;
+}
#endif
diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
index 8b1f37b..4269519 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -16,7 +16,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->ARM_cpsr);
}
-#define xchg_xen_ulong(ptr, val) atomic64_xchg(container_of((ptr), \
+#define xchg_xen_ulong(ptr, val) armv7_atomic64_xchg(container_of((ptr), \
atomic64_t, \
counter), (val))
--
1.7.10.4
Hi Stefano,
On Wed, Jan 08, 2014 at 06:00:23PM +0000, Stefano Stabellini wrote:
> Remove !GENERIC_ATOMIC64 build dependency:
> - rename atomic64_xchg to armv7_atomic64_xchg and define it even ifdef
> GENERIC_ATOMIC64;
> - call armv7_atomic64_xchg directly from xen/events.h.
>
> Remove !CPU_V6 build dependency:
> - introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
> CONFIG_CPU_V6;
> - implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16.
>
> Signed-off-by: Stefano Stabellini <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
>
I'm confused here. It looks like you want to call armv7 code in a v6 kernel.
What am I missing?
Will
On Thursday 09 January 2014, Will Deacon wrote:
> On Wed, Jan 08, 2014 at 06:00:23PM +0000, Stefano Stabellini wrote:
> > Remove !GENERIC_ATOMIC64 build dependency:
> > - rename atomic64_xchg to armv7_atomic64_xchg and define it even ifdef
> > GENERIC_ATOMIC64;
> > - call armv7_atomic64_xchg directly from xen/events.h.
> >
> > Remove !CPU_V6 build dependency:
> > - introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
> > CONFIG_CPU_V6;
> > - implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16.
> >
> > Signed-off-by: Stefano Stabellini <[email protected]>
> > CC: [email protected]
> > CC: [email protected]
> > CC: [email protected]
> > CC: [email protected]
> > CC: [email protected]
> > CC: [email protected]
> > CC: [email protected]
> > CC: [email protected]
> >
>
> I'm confused here. It looks like you want to call armv7 code in a v6 kernel.
> What am I missing?
This is about being able to build a kernel that runs on ARMv6 and ARMv7
and also includes Xen. Because of obvious hardware limitations, Xen
will only run on v7, but currently you cannot even build it once you
enable (pre-v6K) ARMv6 support, since the combined v6+v7 kernel can't
do atomic accesses in a generic way on non-32bit variables.
Arnd
On Thu, 9 Jan 2014, Arnd Bergmann wrote:
> On Thursday 09 January 2014, Will Deacon wrote:
> > On Wed, Jan 08, 2014 at 06:00:23PM +0000, Stefano Stabellini wrote:
> > > Remove !GENERIC_ATOMIC64 build dependency:
> > > - rename atomic64_xchg to armv7_atomic64_xchg and define it even ifdef
> > > GENERIC_ATOMIC64;
> > > - call armv7_atomic64_xchg directly from xen/events.h.
> > >
> > > Remove !CPU_V6 build dependency:
> > > - introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
> > > CONFIG_CPU_V6;
> > > - implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16.
> > >
> > > Signed-off-by: Stefano Stabellini <[email protected]>
> > > CC: [email protected]
> > > CC: [email protected]
> > > CC: [email protected]
> > > CC: [email protected]
> > > CC: [email protected]
> > > CC: [email protected]
> > > CC: [email protected]
> > > CC: [email protected]
> > >
> >
> > I'm confused here. It looks like you want to call armv7 code in a v6 kernel.
> > What am I missing?
>
> This is about being able to build a kernel that runs on ARMv6 and ARMv7
> and also includes Xen. Because of obvious hardware limitations, Xen
> will only run on v7, but currently you cannot even build it once you
> enable (pre-v6K) ARMv6 support, since the combined v6+v7 kernel can't
> do atomic accesses in a generic way on non-32bit variables.
Yep, that's right.
On Thu, Jan 09, 2014 at 12:47:24PM +0000, Stefano Stabellini wrote:
> On Thu, 9 Jan 2014, Arnd Bergmann wrote:
> > On Thursday 09 January 2014, Will Deacon wrote:
> > > On Wed, Jan 08, 2014 at 06:00:23PM +0000, Stefano Stabellini wrote:
> > > > Remove !GENERIC_ATOMIC64 build dependency:
> > > > - rename atomic64_xchg to armv7_atomic64_xchg and define it even ifdef
> > > > GENERIC_ATOMIC64;
> > > > - call armv7_atomic64_xchg directly from xen/events.h.
> > > >
> > > > Remove !CPU_V6 build dependency:
> > > > - introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
> > > > CONFIG_CPU_V6;
> > > > - implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16.
> > > >
> > > > Signed-off-by: Stefano Stabellini <[email protected]>
> > > > CC: [email protected]
> > > > CC: [email protected]
> > > > CC: [email protected]
> > > > CC: [email protected]
> > > > CC: [email protected]
> > > > CC: [email protected]
> > > > CC: [email protected]
> > > > CC: [email protected]
> > > >
> > >
> > > I'm confused here. It looks like you want to call armv7 code in a v6 kernel.
> > > What am I missing?
> >
> > This is about being able to build a kernel that runs on ARMv6 and ARMv7
> > and also includes Xen. Because of obvious hardware limitations, Xen
> > will only run on v7, but currently you cannot even build it once you
> > enable (pre-v6K) ARMv6 support, since the combined v6+v7 kernel can't
> > do atomic accesses in a generic way on non-32bit variables.
>
> Yep, that's right.
Ok, thanks for the explanation. Looking at the patch, I wonder whether it's
not cleaner just to implement xchg code separately for Xen? The Linux code
isn't always sufficient (due to the GENERIC_ATOMIC64 stuff) and most of the
churn coming out of this patch is an attempt to provide some small code
reuse at the cost of code readability.
What do others think?
Will
On 01/10/2014 02:42 AM, Will Deacon wrote:
> On Thu, Jan 09, 2014 at 12:47:24PM +0000, Stefano Stabellini wrote:
>> On Thu, 9 Jan 2014, Arnd Bergmann wrote:
>>> On Thursday 09 January 2014, Will Deacon wrote:
>>>> On Wed, Jan 08, 2014 at 06:00:23PM +0000, Stefano Stabellini wrote:
>>>>> Remove !GENERIC_ATOMIC64 build dependency:
>>>>> - rename atomic64_xchg to armv7_atomic64_xchg and define it even ifdef
>>>>> GENERIC_ATOMIC64;
>>>>> - call armv7_atomic64_xchg directly from xen/events.h.
>>>>>
>>>>> Remove !CPU_V6 build dependency:
>>>>> - introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
>>>>> CONFIG_CPU_V6;
>>>>> - implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16.
>>>>>
>>>>> Signed-off-by: Stefano Stabellini <[email protected]>
>>>>> CC: [email protected]
>>>>> CC: [email protected]
>>>>> CC: [email protected]
>>>>> CC: [email protected]
>>>>> CC: [email protected]
>>>>> CC: [email protected]
>>>>> CC: [email protected]
>>>>> CC: [email protected]
>>>>>
>>>>
>>>> I'm confused here. It looks like you want to call armv7 code in a v6 kernel.
>>>> What am I missing?
>>>
>>> This is about being able to build a kernel that runs on ARMv6 and ARMv7
>>> and also includes Xen. Because of obvious hardware limitations, Xen
>>> will only run on v7, but currently you cannot even build it once you
>>> enable (pre-v6K) ARMv6 support, since the combined v6+v7 kernel can't
>>> do atomic accesses in a generic way on non-32bit variables.
>>
>> Yep, that's right.
>
> Ok, thanks for the explanation. Looking at the patch, I wonder whether it's
> not cleaner just to implement xchg code separately for Xen? The Linux code
> isn't always sufficient (due to the GENERIC_ATOMIC64 stuff) and most of the
> churn coming out of this patch is an attempt to provide some small code
> reuse at the cost of code readability.
>
> What do others think?
>
What Will said sounds reasonable to me.
Thanks.
--
Chen Gang
2014/1/10 Chen Gang F T <[email protected]>:
> On 01/10/2014 02:42 AM, Will Deacon wrote:
>> On Thu, Jan 09, 2014 at 12:47:24PM +0000, Stefano Stabellini wrote:
>>> On Thu, 9 Jan 2014, Arnd Bergmann wrote:
>>>> On Thursday 09 January 2014, Will Deacon wrote:
>>>>> On Wed, Jan 08, 2014 at 06:00:23PM +0000, Stefano Stabellini wrote:
>>>>>> Remove !GENERIC_ATOMIC64 build dependency:
>>>>>> - rename atomic64_xchg to armv7_atomic64_xchg and define it even ifdef
>>>>>> GENERIC_ATOMIC64;
>>>>>> - call armv7_atomic64_xchg directly from xen/events.h.
>>>>>>
>>>>>> Remove !CPU_V6 build dependency:
>>>>>> - introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
>>>>>> CONFIG_CPU_V6;
>>>>>> - implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16.
>>>>>>
>>>>>> Signed-off-by: Stefano Stabellini <[email protected]>
>>>>>> CC: [email protected]
>>>>>> CC: [email protected]
>>>>>> CC: [email protected]
>>>>>> CC: [email protected]
>>>>>> CC: [email protected]
>>>>>> CC: [email protected]
>>>>>> CC: [email protected]
>>>>>> CC: [email protected]
>>>>>>
>>>>>
>>>>> I'm confused here. It looks like you want to call armv7 code in a v6 kernel.
>>>>> What am I missing?
>>>>
>>>> This is about being able to build a kernel that runs on ARMv6 and ARMv7
>>>> and also includes Xen. Because of obvious hardware limitations, Xen
>>>> will only run on v7, but currently you cannot even build it once you
>>>> enable (pre-v6K) ARMv6 support, since the combined v6+v7 kernel can't
>>>> do atomic accesses in a generic way on non-32bit variables.
>>>
>>> Yep, that's right.
>>
>> Ok, thanks for the explanation. Looking at the patch, I wonder whether it's
>> not cleaner just to implement xchg code separately for Xen? The Linux code
>> isn't always sufficient (due to the GENERIC_ATOMIC64 stuff) and most of the
>> churn coming out of this patch is an attempt to provide some small code
>> reuse at the cost of code readability.
>>
>> What do others think?
>>
>
> What Will said sounds reasonable to me.
>
>
> Thanks.
> --
> Chen Gang
I agree with Will,
Regards,
Jaccon
On Thu, 9 Jan 2014, Will Deacon wrote:
> On Thu, Jan 09, 2014 at 12:47:24PM +0000, Stefano Stabellini wrote:
> > On Thu, 9 Jan 2014, Arnd Bergmann wrote:
> > > On Thursday 09 January 2014, Will Deacon wrote:
> > > > On Wed, Jan 08, 2014 at 06:00:23PM +0000, Stefano Stabellini wrote:
> > > > > Remove !GENERIC_ATOMIC64 build dependency:
> > > > > - rename atomic64_xchg to armv7_atomic64_xchg and define it even ifdef
> > > > > GENERIC_ATOMIC64;
> > > > > - call armv7_atomic64_xchg directly from xen/events.h.
> > > > >
> > > > > Remove !CPU_V6 build dependency:
> > > > > - introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
> > > > > CONFIG_CPU_V6;
> > > > > - implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16.
> > > > >
> > > > > Signed-off-by: Stefano Stabellini <[email protected]>
> > > > > CC: [email protected]
> > > > > CC: [email protected]
> > > > > CC: [email protected]
> > > > > CC: [email protected]
> > > > > CC: [email protected]
> > > > > CC: [email protected]
> > > > > CC: [email protected]
> > > > > CC: [email protected]
> > > > >
> > > >
> > > > I'm confused here. It looks like you want to call armv7 code in a v6 kernel.
> > > > What am I missing?
> > >
> > > This is about being able to build a kernel that runs on ARMv6 and ARMv7
> > > and also includes Xen. Because of obvious hardware limitations, Xen
> > > will only run on v7, but currently you cannot even build it once you
> > > enable (pre-v6K) ARMv6 support, since the combined v6+v7 kernel can't
> > > do atomic accesses in a generic way on non-32bit variables.
> >
> > Yep, that's right.
>
> Ok, thanks for the explanation. Looking at the patch, I wonder whether it's
> not cleaner just to implement xchg code separately for Xen? The Linux code
> isn't always sufficient (due to the GENERIC_ATOMIC64 stuff) and most of the
> churn coming out of this patch is an attempt to provide some small code
> reuse at the cost of code readability.
>
> What do others think?
I am OK with that, in fact my first version of the patch did just that:
http://marc.info/?l=linux-arm-kernel&m=138436406724990&w=2
Is that what you had in mind?
Hi Stefano,
On Thu, Jan 16, 2014 at 04:27:55PM +0000, Stefano Stabellini wrote:
> On Thu, 9 Jan 2014, Will Deacon wrote:
> > Ok, thanks for the explanation. Looking at the patch, I wonder whether it's
> > not cleaner just to implement xchg code separately for Xen? The Linux code
> > isn't always sufficient (due to the GENERIC_ATOMIC64 stuff) and most of the
> > churn coming out of this patch is an attempt to provide some small code
> > reuse at the cost of code readability.
> >
> > What do others think?
>
> I am OK with that, in fact my first version of the patch did just that:
>
> http://marc.info/?l=linux-arm-kernel&m=138436406724990&w=2
>
> Is that what you had in mind?
For the xchg part, yes, that looks a lot better. I don't like the #undef
CONFIG_CPU_V6 though, can that be rewritten to use __LINUX_ARM_ARCH__?
Will
On Thu, 16 Jan 2014, Will Deacon wrote:
> Hi Stefano,
>
> On Thu, Jan 16, 2014 at 04:27:55PM +0000, Stefano Stabellini wrote:
> > On Thu, 9 Jan 2014, Will Deacon wrote:
> > > Ok, thanks for the explanation. Looking at the patch, I wonder whether it's
> > > not cleaner just to implement xchg code separately for Xen? The Linux code
> > > isn't always sufficient (due to the GENERIC_ATOMIC64 stuff) and most of the
> > > churn coming out of this patch is an attempt to provide some small code
> > > reuse at the cost of code readability.
> > >
> > > What do others think?
> >
> > I am OK with that, in fact my first version of the patch did just that:
> >
> > http://marc.info/?l=linux-arm-kernel&m=138436406724990&w=2
> >
> > Is that what you had in mind?
>
> For the xchg part, yes, that looks a lot better. I don't like the #undef
> CONFIG_CPU_V6 though, can that be rewritten to use __LINUX_ARM_ARCH__?
The problem is that the 1 and 2 byte parameter size cases in __cmpxchg
are ifdef'ed CONFIG_CPU_V6 but drivers/xen/grant-table.c needs them.
So we can either undef CONFIG_CPU_V6 in grant-table.c or call a
different function.
If I switch from ifdef CONFIG_CPU_V6 to if __LINUX_ARM_ARCH__ > 6 in
__cmpxchg, we still have the problem that if __LINUX_ARM_ARCH__ == 6,
grant-table.c doesn't compile.
Maybe the approach taken by the other patch for cmpxchg is better, see
below.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c1f1a7e..ae54ae0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1881,8 +1881,7 @@ config XEN_DOM0
config XEN
bool "Xen guest support on ARM (EXPERIMENTAL)"
depends on ARM && AEABI && OF
- depends on CPU_V7 && !CPU_V6
- depends on !GENERIC_ATOMIC64
+ depends on CPU_V7
select ARM_PSCI
select SWIOTLB_XEN
help
diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
index df2fbba..cc8a4a2 100644
--- a/arch/arm/include/asm/cmpxchg.h
+++ b/arch/arm/include/asm/cmpxchg.h
@@ -133,6 +133,44 @@ extern void __bad_cmpxchg(volatile void *ptr, int size);
* cmpxchg only support 32-bits operands on ARMv6.
*/
+static inline unsigned long __cmpxchg8(volatile void *ptr, unsigned long old,
+ unsigned long new)
+{
+ unsigned long oldval, res;
+
+ do {
+ asm volatile("@ __cmpxchg1\n"
+ " ldrexb %1, [%2]\n"
+ " mov %0, #0\n"
+ " teq %1, %3\n"
+ " strexbeq %0, %4, [%2]\n"
+ : "=&r" (res), "=&r" (oldval)
+ : "r" (ptr), "Ir" (old), "r" (new)
+ : "memory", "cc");
+ } while (res);
+
+ return oldval;
+}
+
+static inline unsigned long __cmpxchg16(volatile void *ptr, unsigned long old,
+ unsigned long new)
+{
+ unsigned long oldval, res;
+
+ do {
+ asm volatile("@ __cmpxchg1\n"
+ " ldrexh %1, [%2]\n"
+ " mov %0, #0\n"
+ " teq %1, %3\n"
+ " strexheq %0, %4, [%2]\n"
+ : "=&r" (res), "=&r" (oldval)
+ : "r" (ptr), "Ir" (old), "r" (new)
+ : "memory", "cc");
+ } while (res);
+
+ return oldval;
+}
+
static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
unsigned long new, int size)
{
@@ -141,28 +179,10 @@ static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
switch (size) {
#ifndef CONFIG_CPU_V6 /* min ARCH >= ARMv6K */
case 1:
- do {
- asm volatile("@ __cmpxchg1\n"
- " ldrexb %1, [%2]\n"
- " mov %0, #0\n"
- " teq %1, %3\n"
- " strexbeq %0, %4, [%2]\n"
- : "=&r" (res), "=&r" (oldval)
- : "r" (ptr), "Ir" (old), "r" (new)
- : "memory", "cc");
- } while (res);
+ oldval = __cmpxchg8(ptr, old, new);
break;
case 2:
- do {
- asm volatile("@ __cmpxchg1\n"
- " ldrexh %1, [%2]\n"
- " mov %0, #0\n"
- " teq %1, %3\n"
- " strexheq %0, %4, [%2]\n"
- : "=&r" (res), "=&r" (oldval)
- : "r" (ptr), "Ir" (old), "r" (new)
- : "memory", "cc");
- } while (res);
+ oldval = __cmpxchg16(ptr, old, new);
break;
#endif
case 4:
diff --git a/arch/arm/include/asm/sync_bitops.h b/arch/arm/include/asm/sync_bitops.h
index 63479ee..942659a 100644
--- a/arch/arm/include/asm/sync_bitops.h
+++ b/arch/arm/include/asm/sync_bitops.h
@@ -21,7 +21,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));
+
+ smp_mb();
+ switch (size) {
+ case 1:
+ oldval = __cmpxchg8(ptr, old, new);
+ break;
+ case 2:
+ oldval = __cmpxchg16(ptr, old, new);
+ break;
+ default:
+ oldval = __cmpxchg(ptr, old, new, size);
+ break;
+ }
+ smp_mb();
+
+ return oldval;
+}
#endif
diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
index 8b1f37b..2032ee6 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -16,7 +16,37 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->ARM_cpsr);
}
-#define xchg_xen_ulong(ptr, val) atomic64_xchg(container_of((ptr), \
+#ifdef CONFIG_GENERIC_ATOMIC64
+/* if CONFIG_GENERIC_ATOMIC64 is defined we cannot use the generic
+ * atomic64_xchg function because it is implemented using spin locks.
+ * Here we need proper atomic instructions to read and write memory
+ * shared with the hypervisor.
+ */
+static inline u64 xen_atomic64_xchg(atomic64_t *ptr, u64 new)
+{
+ u64 result;
+ unsigned long tmp;
+
+ smp_mb();
+
+ __asm__ __volatile__("@ xen_atomic64_xchg\n"
+"1: ldrexd %0, %H0, [%3]\n"
+" strexd %1, %4, %H4, [%3]\n"
+" teq %1, #0\n"
+" bne 1b"
+ : "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter)
+ : "r" (&ptr->counter), "r" (new)
+ : "cc");
+
+ smp_mb();
+
+ return result;
+}
+#else
+#define xen_atomic64_xchg atomic64_xchg
+#endif
+
+#define xchg_xen_ulong(ptr, val) xen_atomic64_xchg(container_of((ptr), \
atomic64_t, \
counter), (val))
On Mon, Jan 20, 2014 at 03:32:48PM +0000, Stefano Stabellini wrote:
> On Thu, 16 Jan 2014, Will Deacon wrote:
> > For the xchg part, yes, that looks a lot better. I don't like the #undef
> > CONFIG_CPU_V6 though, can that be rewritten to use __LINUX_ARM_ARCH__?
>
> The problem is that the 1 and 2 byte parameter size cases in __cmpxchg
> are ifdef'ed CONFIG_CPU_V6 but drivers/xen/grant-table.c needs them.
>
> So we can either undef CONFIG_CPU_V6 in grant-table.c or call a
> different function.
>
> If I switch from ifdef CONFIG_CPU_V6 to if __LINUX_ARM_ARCH__ > 6 in
> __cmpxchg, we still have the problem that if __LINUX_ARM_ARCH__ == 6,
> grant-table.c doesn't compile.
>
> Maybe the approach taken by the other patch for cmpxchg is better, see
> below.
Yes, I prefer this approach. Minor comment below.
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index c1f1a7e..ae54ae0 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1881,8 +1881,7 @@ config XEN_DOM0
> config XEN
> bool "Xen guest support on ARM (EXPERIMENTAL)"
> depends on ARM && AEABI && OF
> - depends on CPU_V7 && !CPU_V6
> - depends on !GENERIC_ATOMIC64
> + depends on CPU_V7
> select ARM_PSCI
> select SWIOTLB_XEN
> help
> diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
> index df2fbba..cc8a4a2 100644
> --- a/arch/arm/include/asm/cmpxchg.h
> +++ b/arch/arm/include/asm/cmpxchg.h
> @@ -133,6 +133,44 @@ extern void __bad_cmpxchg(volatile void *ptr, int size);
> * cmpxchg only support 32-bits operands on ARMv6.
> */
>
> +static inline unsigned long __cmpxchg8(volatile void *ptr, unsigned long old,
> + unsigned long new)
> +{
> + unsigned long oldval, res;
> +
> + do {
> + asm volatile("@ __cmpxchg1\n"
> + " ldrexb %1, [%2]\n"
> + " mov %0, #0\n"
> + " teq %1, %3\n"
> + " strexbeq %0, %4, [%2]\n"
> + : "=&r" (res), "=&r" (oldval)
> + : "r" (ptr), "Ir" (old), "r" (new)
> + : "memory", "cc");
> + } while (res);
> +
> + return oldval;
> +}
> +
> +static inline unsigned long __cmpxchg16(volatile void *ptr, unsigned long old,
> + unsigned long new)
> +{
> + unsigned long oldval, res;
> +
> + do {
> + asm volatile("@ __cmpxchg1\n"
Can you fix this comment while you're here please?
Will
On Tue, 21 Jan 2014, Will Deacon wrote:
> On Mon, Jan 20, 2014 at 03:32:48PM +0000, Stefano Stabellini wrote:
> > On Thu, 16 Jan 2014, Will Deacon wrote:
> > > For the xchg part, yes, that looks a lot better. I don't like the #undef
> > > CONFIG_CPU_V6 though, can that be rewritten to use __LINUX_ARM_ARCH__?
> >
> > The problem is that the 1 and 2 byte parameter size cases in __cmpxchg
> > are ifdef'ed CONFIG_CPU_V6 but drivers/xen/grant-table.c needs them.
> >
> > So we can either undef CONFIG_CPU_V6 in grant-table.c or call a
> > different function.
> >
> > If I switch from ifdef CONFIG_CPU_V6 to if __LINUX_ARM_ARCH__ > 6 in
> > __cmpxchg, we still have the problem that if __LINUX_ARM_ARCH__ == 6,
> > grant-table.c doesn't compile.
> >
> > Maybe the approach taken by the other patch for cmpxchg is better, see
> > below.
>
> Yes, I prefer this approach. Minor comment below.
>
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index c1f1a7e..ae54ae0 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1881,8 +1881,7 @@ config XEN_DOM0
> > config XEN
> > bool "Xen guest support on ARM (EXPERIMENTAL)"
> > depends on ARM && AEABI && OF
> > - depends on CPU_V7 && !CPU_V6
> > - depends on !GENERIC_ATOMIC64
> > + depends on CPU_V7
> > select ARM_PSCI
> > select SWIOTLB_XEN
> > help
> > diff --git a/arch/arm/include/asm/cmpxchg.h b/arch/arm/include/asm/cmpxchg.h
> > index df2fbba..cc8a4a2 100644
> > --- a/arch/arm/include/asm/cmpxchg.h
> > +++ b/arch/arm/include/asm/cmpxchg.h
> > @@ -133,6 +133,44 @@ extern void __bad_cmpxchg(volatile void *ptr, int size);
> > * cmpxchg only support 32-bits operands on ARMv6.
> > */
> >
> > +static inline unsigned long __cmpxchg8(volatile void *ptr, unsigned long old,
> > + unsigned long new)
> > +{
> > + unsigned long oldval, res;
> > +
> > + do {
> > + asm volatile("@ __cmpxchg1\n"
> > + " ldrexb %1, [%2]\n"
> > + " mov %0, #0\n"
> > + " teq %1, %3\n"
> > + " strexbeq %0, %4, [%2]\n"
> > + : "=&r" (res), "=&r" (oldval)
> > + : "r" (ptr), "Ir" (old), "r" (new)
> > + : "memory", "cc");
> > + } while (res);
> > +
> > + return oldval;
> > +}
> > +
> > +static inline unsigned long __cmpxchg16(volatile void *ptr, unsigned long old,
> > + unsigned long new)
> > +{
> > + unsigned long oldval, res;
> > +
> > + do {
> > + asm volatile("@ __cmpxchg1\n"
>
> Can you fix this comment while you're here please?
OK, I'll use @ __cmpxchg16 and @ __cmpxchg8.
I'll resend as a separate patch.