* [PATCH] ARM: SWP emulation: Restore original *data when failed @ 2015-10-14 2:51 Shengjiu Wang 2015-10-15 8:24 ` Russell King - ARM Linux 0 siblings, 1 reply; 11+ messages in thread From: Shengjiu Wang @ 2015-10-14 2:51 UTC (permalink / raw) To: linux-arm-kernel __user_swpX_asm maybe failed in first STREX operation, emulate_swpX will try again, but the *data has been changed in first time. which cause the result is wrong. So need to recover the *data when failed. Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com> --- arch/arm/kernel/swp_emulate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/kernel/swp_emulate.c b/arch/arm/kernel/swp_emulate.c index 5b26e7e..c61fbf92 100644 --- a/arch/arm/kernel/swp_emulate.c +++ b/arch/arm/kernel/swp_emulate.c @@ -41,6 +41,7 @@ "1: strex"B" %0, %2, [%3]\n" \ " cmp %0, #0\n" \ " movne %0, %4\n" \ + " movne %1, %2\n" \ "2:\n" \ " .section .text.fixup,\"ax\"\n" \ " .align 2\n" \ -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-14 2:51 [PATCH] ARM: SWP emulation: Restore original *data when failed Shengjiu Wang @ 2015-10-15 8:24 ` Russell King - ARM Linux 2015-10-15 8:36 ` Shengjiu Wang 0 siblings, 1 reply; 11+ messages in thread From: Russell King - ARM Linux @ 2015-10-15 8:24 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 14, 2015 at 10:51:17AM +0800, Shengjiu Wang wrote: > __user_swpX_asm maybe failed in first STREX operation, emulate_swpX > will try again, but the *data has been changed in first time. which > cause the result is wrong. So need to recover the *data when failed. > > Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com> > --- > arch/arm/kernel/swp_emulate.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/kernel/swp_emulate.c b/arch/arm/kernel/swp_emulate.c > index 5b26e7e..c61fbf92 100644 > --- a/arch/arm/kernel/swp_emulate.c > +++ b/arch/arm/kernel/swp_emulate.c > @@ -41,6 +41,7 @@ > "1: strex"B" %0, %2, [%3]\n" \ > " cmp %0, #0\n" \ > " movne %0, %4\n" \ > + " movne %1, %2\n" \ > "2:\n" \ > " .section .text.fixup,\"ax\"\n" \ > " .align 2\n" \ I think I'd prefer this to be: __asm__ __volatile__( \ "0: ldrex"B" %2, [%3]\n" \ "1: strex"B" %0, %1, [%3]\n" \ " cmp %0, #0\n" \ " moveq %1, %2\n" \ " movne %0, %4\n" \ so that we're not loading into %1 (an in-out non-temporary) but rather loading it into a temporary - and only overwriting the saved register value if the swap succeeds. Thanks. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-15 8:24 ` Russell King - ARM Linux @ 2015-10-15 8:36 ` Shengjiu Wang 2015-10-15 8:57 ` Russell King - ARM Linux 0 siblings, 1 reply; 11+ messages in thread From: Shengjiu Wang @ 2015-10-15 8:36 UTC (permalink / raw) To: linux-arm-kernel On Thu, Oct 15, 2015 at 09:24:17AM +0100, Russell King - ARM Linux wrote: > On Wed, Oct 14, 2015 at 10:51:17AM +0800, Shengjiu Wang wrote: > > __user_swpX_asm maybe failed in first STREX operation, emulate_swpX > > will try again, but the *data has been changed in first time. which > > cause the result is wrong. So need to recover the *data when failed. > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com> > > --- > > arch/arm/kernel/swp_emulate.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm/kernel/swp_emulate.c b/arch/arm/kernel/swp_emulate.c > > index 5b26e7e..c61fbf92 100644 > > --- a/arch/arm/kernel/swp_emulate.c > > +++ b/arch/arm/kernel/swp_emulate.c > > @@ -41,6 +41,7 @@ > > "1: strex"B" %0, %2, [%3]\n" \ > > " cmp %0, #0\n" \ > > " movne %0, %4\n" \ > > + " movne %1, %2\n" \ > > "2:\n" \ > > " .section .text.fixup,\"ax\"\n" \ > > " .align 2\n" \ > > I think I'd prefer this to be: > > __asm__ __volatile__( \ > "0: ldrex"B" %2, [%3]\n" \ > "1: strex"B" %0, %1, [%3]\n" \ > " cmp %0, #0\n" \ > " moveq %1, %2\n" \ > " movne %0, %4\n" \ > > so that we're not loading into %1 (an in-out non-temporary) but rather > loading it into a temporary - and only overwriting the saved register > value if the swap succeeds. > > Thanks. I am ok with your change. Need I send another patch for this change? or will you send it by yourself? best regards wang shengjiu > > -- > FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up > according to speedtest.net. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-15 8:36 ` Shengjiu Wang @ 2015-10-15 8:57 ` Russell King - ARM Linux 2015-10-15 9:17 ` Vladimir Murzin 0 siblings, 1 reply; 11+ messages in thread From: Russell King - ARM Linux @ 2015-10-15 8:57 UTC (permalink / raw) To: linux-arm-kernel On Thu, Oct 15, 2015 at 04:36:31PM +0800, Shengjiu Wang wrote: > On Thu, Oct 15, 2015 at 09:24:17AM +0100, Russell King - ARM Linux wrote: > > On Wed, Oct 14, 2015 at 10:51:17AM +0800, Shengjiu Wang wrote: > > > __user_swpX_asm maybe failed in first STREX operation, emulate_swpX > > > will try again, but the *data has been changed in first time. which > > > cause the result is wrong. So need to recover the *data when failed. > > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com> > > > --- > > > arch/arm/kernel/swp_emulate.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/arm/kernel/swp_emulate.c b/arch/arm/kernel/swp_emulate.c > > > index 5b26e7e..c61fbf92 100644 > > > --- a/arch/arm/kernel/swp_emulate.c > > > +++ b/arch/arm/kernel/swp_emulate.c > > > @@ -41,6 +41,7 @@ > > > "1: strex"B" %0, %2, [%3]\n" \ > > > " cmp %0, #0\n" \ > > > " movne %0, %4\n" \ > > > + " movne %1, %2\n" \ > > > "2:\n" \ > > > " .section .text.fixup,\"ax\"\n" \ > > > " .align 2\n" \ > > > > I think I'd prefer this to be: > > > > __asm__ __volatile__( \ > > "0: ldrex"B" %2, [%3]\n" \ > > "1: strex"B" %0, %1, [%3]\n" \ > > " cmp %0, #0\n" \ > > " moveq %1, %2\n" \ > > " movne %0, %4\n" \ > > > > so that we're not loading into %1 (an in-out non-temporary) but rather > > loading it into a temporary - and only overwriting the saved register > > value if the swap succeeds. > > > > Thanks. > > I am ok with your change. Need I send another patch for this change? or will > you send it by yourself? Please send a new patch, thanks. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-15 8:57 ` Russell King - ARM Linux @ 2015-10-15 9:17 ` Vladimir Murzin 2015-10-15 13:02 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Vladimir Murzin @ 2015-10-15 9:17 UTC (permalink / raw) To: linux-arm-kernel On 15/10/15 09:57, Russell King - ARM Linux wrote: > On Thu, Oct 15, 2015 at 04:36:31PM +0800, Shengjiu Wang wrote: >> On Thu, Oct 15, 2015 at 09:24:17AM +0100, Russell King - ARM Linux wrote: >>> On Wed, Oct 14, 2015 at 10:51:17AM +0800, Shengjiu Wang wrote: >>>> __user_swpX_asm maybe failed in first STREX operation, emulate_swpX >>>> will try again, but the *data has been changed in first time. which >>>> cause the result is wrong. So need to recover the *data when failed. >>>> >>>> Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com> >>>> --- >>>> arch/arm/kernel/swp_emulate.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/arch/arm/kernel/swp_emulate.c b/arch/arm/kernel/swp_emulate.c >>>> index 5b26e7e..c61fbf92 100644 >>>> --- a/arch/arm/kernel/swp_emulate.c >>>> +++ b/arch/arm/kernel/swp_emulate.c >>>> @@ -41,6 +41,7 @@ >>>> "1: strex"B" %0, %2, [%3]\n" \ >>>> " cmp %0, #0\n" \ >>>> " movne %0, %4\n" \ >>>> + " movne %1, %2\n" \ >>>> "2:\n" \ >>>> " .section .text.fixup,\"ax\"\n" \ >>>> " .align 2\n" \ >>> >>> I think I'd prefer this to be: >>> >>> __asm__ __volatile__( \ >>> "0: ldrex"B" %2, [%3]\n" \ >>> "1: strex"B" %0, %1, [%3]\n" \ >>> " cmp %0, #0\n" \ >>> " moveq %1, %2\n" \ >>> " movne %0, %4\n" \ >>> >>> so that we're not loading into %1 (an in-out non-temporary) but rather >>> loading it into a temporary - and only overwriting the saved register >>> value if the swap succeeds. >>> >>> Thanks. >> >> I am ok with your change. Need I send another patch for this change? or will >> you send it by yourself? > > Please send a new patch, thanks. > We might need the same change for arm64 counterpart (see arch/arm64/kernel/armv8_deprecated.c). Vladimir ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-15 9:17 ` Vladimir Murzin @ 2015-10-15 13:02 ` Will Deacon 2015-10-15 13:25 ` Vladimir Murzin 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2015-10-15 13:02 UTC (permalink / raw) To: linux-arm-kernel On Thu, Oct 15, 2015 at 10:17:47AM +0100, Vladimir Murzin wrote: > We might need the same change for arm64 counterpart (see > arch/arm64/kernel/armv8_deprecated.c). Something like below? Will >From 63c3e83073cfac2e011adf0ed6f335275cc977a7 Mon Sep 17 00:00:00 2001 From: Will Deacon <will.deacon@arm.com> Date: Thu, 15 Oct 2015 13:55:53 +0100 Subject: [PATCH] arm64: compat: fix stxr failure case in SWP emulation If the STXR instruction fails in the SWP emulation code, we leave *data overwritten with the loaded value, therefore corrupting the data written by a subsequent, successful attempt. This patch re-jigs the code so that we only write back to *data once we know that the update has happened. Reported-by: Shengjiu Wang <shengjiu.wang@freescale.com> Reported-by: Vladimir Murzin <vladimir.murzin@arm.com> Signed-off-by: Will Deacon <will.deacon@arm.com> --- arch/arm64/kernel/armv8_deprecated.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c index bcee7abac68e..6039d1eb5912 100644 --- a/arch/arm64/kernel/armv8_deprecated.c +++ b/arch/arm64/kernel/armv8_deprecated.c @@ -284,12 +284,12 @@ static void register_insn_emulation_sysctl(struct ctl_table *table) __asm__ __volatile__( \ ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \ CONFIG_ARM64_PAN) \ - " mov %w2, %w1\n" \ - "0: ldxr"B" %w1, [%3]\n" \ - "1: stxr"B" %w0, %w2, [%3]\n" \ + "0: ldxr"B" %w2, [%3]\n" \ + "1: stxr"B" %w0, %w1, [%3]\n" \ " cbz %w0, 2f\n" \ " mov %w0, %w4\n" \ "2:\n" \ + " mov %w1, %w2\n" \ " .pushsection .fixup,\"ax\"\n" \ " .align 2\n" \ "3: mov %w0, %w5\n" \ -- 2.1.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-15 13:02 ` Will Deacon @ 2015-10-15 13:25 ` Vladimir Murzin 2015-10-16 7:52 ` Vladimir Murzin 0 siblings, 1 reply; 11+ messages in thread From: Vladimir Murzin @ 2015-10-15 13:25 UTC (permalink / raw) To: linux-arm-kernel On 15/10/15 14:02, Will Deacon wrote: > On Thu, Oct 15, 2015 at 10:17:47AM +0100, Vladimir Murzin wrote: >> We might need the same change for arm64 counterpart (see >> arch/arm64/kernel/armv8_deprecated.c). > > Something like below? Looks good. Should these two go to stable? Vladimir > > Will > >>From 63c3e83073cfac2e011adf0ed6f335275cc977a7 Mon Sep 17 00:00:00 2001 > From: Will Deacon <will.deacon@arm.com> > Date: Thu, 15 Oct 2015 13:55:53 +0100 > Subject: [PATCH] arm64: compat: fix stxr failure case in SWP emulation > > If the STXR instruction fails in the SWP emulation code, we leave *data > overwritten with the loaded value, therefore corrupting the data written > by a subsequent, successful attempt. > > This patch re-jigs the code so that we only write back to *data once we > know that the update has happened. > > Reported-by: Shengjiu Wang <shengjiu.wang@freescale.com> > Reported-by: Vladimir Murzin <vladimir.murzin@arm.com> > Signed-off-by: Will Deacon <will.deacon@arm.com> > --- > arch/arm64/kernel/armv8_deprecated.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c > index bcee7abac68e..6039d1eb5912 100644 > --- a/arch/arm64/kernel/armv8_deprecated.c > +++ b/arch/arm64/kernel/armv8_deprecated.c > @@ -284,12 +284,12 @@ static void register_insn_emulation_sysctl(struct ctl_table *table) > __asm__ __volatile__( \ > ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \ > CONFIG_ARM64_PAN) \ > - " mov %w2, %w1\n" \ > - "0: ldxr"B" %w1, [%3]\n" \ > - "1: stxr"B" %w0, %w2, [%3]\n" \ > + "0: ldxr"B" %w2, [%3]\n" \ > + "1: stxr"B" %w0, %w1, [%3]\n" \ > " cbz %w0, 2f\n" \ > " mov %w0, %w4\n" \ > "2:\n" \ > + " mov %w1, %w2\n" \ > " .pushsection .fixup,\"ax\"\n" \ > " .align 2\n" \ > "3: mov %w0, %w5\n" \ > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-15 13:25 ` Vladimir Murzin @ 2015-10-16 7:52 ` Vladimir Murzin 2015-10-16 10:37 ` Catalin Marinas 0 siblings, 1 reply; 11+ messages in thread From: Vladimir Murzin @ 2015-10-16 7:52 UTC (permalink / raw) To: linux-arm-kernel On 15/10/15 14:25, Vladimir Murzin wrote: > On 15/10/15 14:02, Will Deacon wrote: >> On Thu, Oct 15, 2015 at 10:17:47AM +0100, Vladimir Murzin wrote: >>> We might need the same change for arm64 counterpart (see >>> arch/arm64/kernel/armv8_deprecated.c). >> >> Something like below? > > Looks good. Should these two go to stable? On the second thought looks like we still update *data in case stxr fails (or I need more coffee). Vladimir > > Vladimir > >> >> Will >> >> >From 63c3e83073cfac2e011adf0ed6f335275cc977a7 Mon Sep 17 00:00:00 2001 >> From: Will Deacon <will.deacon@arm.com> >> Date: Thu, 15 Oct 2015 13:55:53 +0100 >> Subject: [PATCH] arm64: compat: fix stxr failure case in SWP emulation >> >> If the STXR instruction fails in the SWP emulation code, we leave *data >> overwritten with the loaded value, therefore corrupting the data written >> by a subsequent, successful attempt. >> >> This patch re-jigs the code so that we only write back to *data once we >> know that the update has happened. >> >> Reported-by: Shengjiu Wang <shengjiu.wang@freescale.com> >> Reported-by: Vladimir Murzin <vladimir.murzin@arm.com> >> Signed-off-by: Will Deacon <will.deacon@arm.com> >> --- >> arch/arm64/kernel/armv8_deprecated.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c >> index bcee7abac68e..6039d1eb5912 100644 >> --- a/arch/arm64/kernel/armv8_deprecated.c >> +++ b/arch/arm64/kernel/armv8_deprecated.c >> @@ -284,12 +284,12 @@ static void register_insn_emulation_sysctl(struct ctl_table *table) >> __asm__ __volatile__( \ >> ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \ >> CONFIG_ARM64_PAN) \ >> - " mov %w2, %w1\n" \ >> - "0: ldxr"B" %w1, [%3]\n" \ >> - "1: stxr"B" %w0, %w2, [%3]\n" \ >> + "0: ldxr"B" %w2, [%3]\n" \ >> + "1: stxr"B" %w0, %w1, [%3]\n" \ >> " cbz %w0, 2f\n" \ >> " mov %w0, %w4\n" \ >> "2:\n" \ >> + " mov %w1, %w2\n" \ >> " .pushsection .fixup,\"ax\"\n" \ >> " .align 2\n" \ >> "3: mov %w0, %w5\n" \ >> > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-16 7:52 ` Vladimir Murzin @ 2015-10-16 10:37 ` Catalin Marinas 2015-10-27 15:44 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Catalin Marinas @ 2015-10-16 10:37 UTC (permalink / raw) To: linux-arm-kernel On Fri, Oct 16, 2015 at 08:52:24AM +0100, Vladimir Murzin wrote: > > On 15/10/15 14:02, Will Deacon wrote: > >> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c > >> index bcee7abac68e..6039d1eb5912 100644 > >> --- a/arch/arm64/kernel/armv8_deprecated.c > >> +++ b/arch/arm64/kernel/armv8_deprecated.c > >> @@ -284,12 +284,12 @@ static void register_insn_emulation_sysctl(struct ctl_table *table) > >> __asm__ __volatile__( \ > >> ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \ > >> CONFIG_ARM64_PAN) \ > >> - " mov %w2, %w1\n" \ > >> - "0: ldxr"B" %w1, [%3]\n" \ > >> - "1: stxr"B" %w0, %w2, [%3]\n" \ > >> + "0: ldxr"B" %w2, [%3]\n" \ > >> + "1: stxr"B" %w0, %w1, [%3]\n" \ > >> " cbz %w0, 2f\n" \ > >> " mov %w0, %w4\n" \ > >> "2:\n" \ > >> + " mov %w1, %w2\n" \ > >> " .pushsection .fixup,\"ax\"\n" \ > >> " .align 2\n" \ > >> "3: mov %w0, %w5\n" \ > > On the second thought looks like we still update *data in case stxr > fails (or I need more coffee). I'm on the second cup and I see the same problem. Even if stxr fails, we fall back through "mov %w1, %w2", so *data is always updated with the loaded value. Maybe something like below on top of Will's patch: diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c index 6039d1eb5912..3fab37b3bc95 100644 --- a/arch/arm64/kernel/armv8_deprecated.c +++ b/arch/arm64/kernel/armv8_deprecated.c @@ -286,10 +286,10 @@ static void register_insn_emulation_sysctl(struct ctl_table *table) CONFIG_ARM64_PAN) \ "0: ldxr"B" %w2, [%3]\n" \ "1: stxr"B" %w0, %w1, [%3]\n" \ - " cbz %w0, 2f\n" \ - " mov %w0, %w4\n" \ + " cmp %w0, #0\n" \ + " csel %w0, %w4, wzr, ne\n" \ + " csel %w1, %w2, %w1, eq\n" \ "2:\n" \ - " mov %w1, %w2\n" \ " .pushsection .fixup,\"ax\"\n" \ " .align 2\n" \ "3: mov %w0, %w5\n" \ @@ -303,7 +303,7 @@ static void register_insn_emulation_sysctl(struct ctl_table *table) ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN, \ CONFIG_ARM64_PAN) \ : "=&r" (res), "+r" (data), "=&r" (temp) \ - : "r" (addr), "i" (-EAGAIN), "i" (-EFAULT) \ + : "r" (addr), "r" (-EAGAIN), "i" (-EFAULT) \ : "memory") #define __user_swp_asm(data, addr, res, temp) \ @@ -342,7 +342,7 @@ static void set_segfault(struct pt_regs *regs, unsigned long addr) static int emulate_swpX(unsigned int address, unsigned int *data, unsigned int type) { - unsigned int res = 0; + unsigned int res; if ((type != TYPE_SWPB) && (address & 0x3)) { /* SWP to unaligned address not permitted */ Since Will is away for a week, I'll wait until he's back to push this patch. -- Catalin ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-16 10:37 ` Catalin Marinas @ 2015-10-27 15:44 ` Will Deacon 2015-10-28 16:16 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2015-10-27 15:44 UTC (permalink / raw) To: linux-arm-kernel On Fri, Oct 16, 2015 at 11:37:29AM +0100, Catalin Marinas wrote: > On Fri, Oct 16, 2015 at 08:52:24AM +0100, Vladimir Murzin wrote: > > > On 15/10/15 14:02, Will Deacon wrote: > > >> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c > > >> index bcee7abac68e..6039d1eb5912 100644 > > >> --- a/arch/arm64/kernel/armv8_deprecated.c > > >> +++ b/arch/arm64/kernel/armv8_deprecated.c > > >> @@ -284,12 +284,12 @@ static void register_insn_emulation_sysctl(struct ctl_table *table) > > >> __asm__ __volatile__( \ > > >> ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \ > > >> CONFIG_ARM64_PAN) \ > > >> - " mov %w2, %w1\n" \ > > >> - "0: ldxr"B" %w1, [%3]\n" \ > > >> - "1: stxr"B" %w0, %w2, [%3]\n" \ > > >> + "0: ldxr"B" %w2, [%3]\n" \ > > >> + "1: stxr"B" %w0, %w1, [%3]\n" \ > > >> " cbz %w0, 2f\n" \ > > >> " mov %w0, %w4\n" \ > > >> "2:\n" \ > > >> + " mov %w1, %w2\n" \ > > >> " .pushsection .fixup,\"ax\"\n" \ > > >> " .align 2\n" \ > > >> "3: mov %w0, %w5\n" \ > > > > On the second thought looks like we still update *data in case stxr > > fails (or I need more coffee). > > I'm on the second cup and I see the same problem. Even if stxr fails, we > fall back through "mov %w1, %w2", so *data is always updated with the > loaded value. Maybe something like below on top of Will's patch: Yeah, sorry, my original patch was an untested mess. I think we can avoid the "cc" clobber by adding a branch to the slow-path, as below. Still needs testing, mind. Will --->8 diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c index 6039d1eb5912..937f5e58a4d3 100644 --- a/arch/arm64/kernel/armv8_deprecated.c +++ b/arch/arm64/kernel/armv8_deprecated.c @@ -288,17 +288,19 @@ static void register_insn_emulation_sysctl(struct ctl_table *table) "1: stxr"B" %w0, %w1, [%3]\n" \ " cbz %w0, 2f\n" \ " mov %w0, %w4\n" \ + " b 3f\n" \ "2:\n" \ " mov %w1, %w2\n" \ + "3:\n" \ " .pushsection .fixup,\"ax\"\n" \ " .align 2\n" \ - "3: mov %w0, %w5\n" \ - " b 2b\n" \ + "4: mov %w0, %w5\n" \ + " b 3b\n" \ " .popsection" \ " .pushsection __ex_table,\"a\"\n" \ " .align 3\n" \ - " .quad 0b, 3b\n" \ - " .quad 1b, 3b\n" \ + " .quad 0b, 4b\n" \ + " .quad 1b, 4b\n" \ " .popsection\n" \ ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN, \ CONFIG_ARM64_PAN) \ ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] ARM: SWP emulation: Restore original *data when failed 2015-10-27 15:44 ` Will Deacon @ 2015-10-28 16:16 ` Will Deacon 0 siblings, 0 replies; 11+ messages in thread From: Will Deacon @ 2015-10-28 16:16 UTC (permalink / raw) To: linux-arm-kernel On Tue, Oct 27, 2015 at 03:44:24PM +0000, Will Deacon wrote: > On Fri, Oct 16, 2015 at 11:37:29AM +0100, Catalin Marinas wrote: > > On Fri, Oct 16, 2015 at 08:52:24AM +0100, Vladimir Murzin wrote: > > > > On 15/10/15 14:02, Will Deacon wrote: > > > >> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c > > > >> index bcee7abac68e..6039d1eb5912 100644 > > > >> --- a/arch/arm64/kernel/armv8_deprecated.c > > > >> +++ b/arch/arm64/kernel/armv8_deprecated.c > > > >> @@ -284,12 +284,12 @@ static void register_insn_emulation_sysctl(struct ctl_table *table) > > > >> __asm__ __volatile__( \ > > > >> ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \ > > > >> CONFIG_ARM64_PAN) \ > > > >> - " mov %w2, %w1\n" \ > > > >> - "0: ldxr"B" %w1, [%3]\n" \ > > > >> - "1: stxr"B" %w0, %w2, [%3]\n" \ > > > >> + "0: ldxr"B" %w2, [%3]\n" \ > > > >> + "1: stxr"B" %w0, %w1, [%3]\n" \ > > > >> " cbz %w0, 2f\n" \ > > > >> " mov %w0, %w4\n" \ > > > >> "2:\n" \ > > > >> + " mov %w1, %w2\n" \ > > > >> " .pushsection .fixup,\"ax\"\n" \ > > > >> " .align 2\n" \ > > > >> "3: mov %w0, %w5\n" \ > > > > > > On the second thought looks like we still update *data in case stxr > > > fails (or I need more coffee). > > > > I'm on the second cup and I see the same problem. Even if stxr fails, we > > fall back through "mov %w1, %w2", so *data is always updated with the > > loaded value. Maybe something like below on top of Will's patch: > > Yeah, sorry, my original patch was an untested mess. I think we can avoid > the "cc" clobber by adding a branch to the slow-path, as below. > > Still needs testing, mind. Right, it passes the simple test case I wrote (which fails under mainline). Will ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-10-28 16:16 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-14 2:51 [PATCH] ARM: SWP emulation: Restore original *data when failed Shengjiu Wang 2015-10-15 8:24 ` Russell King - ARM Linux 2015-10-15 8:36 ` Shengjiu Wang 2015-10-15 8:57 ` Russell King - ARM Linux 2015-10-15 9:17 ` Vladimir Murzin 2015-10-15 13:02 ` Will Deacon 2015-10-15 13:25 ` Vladimir Murzin 2015-10-16 7:52 ` Vladimir Murzin 2015-10-16 10:37 ` Catalin Marinas 2015-10-27 15:44 ` Will Deacon 2015-10-28 16:16 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).