* [PATCH v3 1/2] MIPS: io: prevent compiler reordering on the default writeX() implementation @ 2018-04-03 12:55 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-03 12:55 UTC (permalink / raw) To: linux-mips, arnd, timur, sulrich Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Ralf Baechle, James Hogan, Paul Burton, linux-kernel writeX() has a strong ordering semantics with respect to memory updates. In the abscence of a write barrier or a compiler barrier, commpiler can reorder register and memory update instructions. This breaks the writeX() API. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- arch/mips/include/asm/io.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h index 0cbf3af..fd00ddaf 100644 --- a/arch/mips/include/asm/io.h +++ b/arch/mips/include/asm/io.h @@ -307,7 +307,7 @@ static inline void iounmap(const volatile void __iomem *addr) #if defined(CONFIG_CPU_CAVIUM_OCTEON) || defined(CONFIG_LOONGSON3_ENHANCEMENT) #define war_io_reorder_wmb() wmb() #else -#define war_io_reorder_wmb() do { } while (0) +#define war_io_reorder_wmb() barrier() #endif #define __BUILD_MEMORY_SINGLE(pfx, bwlq, type, irq) \ -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 1/2] MIPS: io: prevent compiler reordering on the default writeX() implementation @ 2018-04-03 12:55 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-03 12:55 UTC (permalink / raw) To: linux-arm-kernel writeX() has a strong ordering semantics with respect to memory updates. In the abscence of a write barrier or a compiler barrier, commpiler can reorder register and memory update instructions. This breaks the writeX() API. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- arch/mips/include/asm/io.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h index 0cbf3af..fd00ddaf 100644 --- a/arch/mips/include/asm/io.h +++ b/arch/mips/include/asm/io.h @@ -307,7 +307,7 @@ static inline void iounmap(const volatile void __iomem *addr) #if defined(CONFIG_CPU_CAVIUM_OCTEON) || defined(CONFIG_LOONGSON3_ENHANCEMENT) #define war_io_reorder_wmb() wmb() #else -#define war_io_reorder_wmb() do { } while (0) +#define war_io_reorder_wmb() barrier() #endif #define __BUILD_MEMORY_SINGLE(pfx, bwlq, type, irq) \ -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-03 12:55 ` Sinan Kaya @ 2018-04-03 12:55 ` Sinan Kaya -1 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-03 12:55 UTC (permalink / raw) To: linux-mips, arnd, timur, sulrich Cc: linux-arm-msm, linux-arm-kernel, Sinan Kaya, Ralf Baechle, James Hogan, Paul Burton, linux-kernel While a barrier is present in writeX() function before the register write, a similar barrier is missing in the readX() function after the register read. This could allow memory accesses following readX() to observe stale data. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> Reported-by: Arnd Bergmann <arnd@arndb.de> --- arch/mips/include/asm/io.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h index fd00ddaf..6ac502f 100644 --- a/arch/mips/include/asm/io.h +++ b/arch/mips/include/asm/io.h @@ -377,6 +377,7 @@ static inline type pfx##read##bwlq(const volatile void __iomem *mem) \ BUG(); \ } \ \ + rmb(); \ return pfx##ioswab##bwlq(__mem, __val); \ } -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-03 12:55 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-03 12:55 UTC (permalink / raw) To: linux-arm-kernel While a barrier is present in writeX() function before the register write, a similar barrier is missing in the readX() function after the register read. This could allow memory accesses following readX() to observe stale data. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> Reported-by: Arnd Bergmann <arnd@arndb.de> --- arch/mips/include/asm/io.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h index fd00ddaf..6ac502f 100644 --- a/arch/mips/include/asm/io.h +++ b/arch/mips/include/asm/io.h @@ -377,6 +377,7 @@ static inline type pfx##read##bwlq(const volatile void __iomem *mem) \ BUG(); \ } \ \ + rmb(); \ return pfx##ioswab##bwlq(__mem, __val); \ } -- 2.7.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-03 12:55 ` Sinan Kaya @ 2018-04-06 1:34 ` Sinan Kaya -1 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-06 1:34 UTC (permalink / raw) To: linux-mips, arnd, timur, sulrich Cc: linux-arm-msm, linux-arm-kernel, Ralf Baechle, James Hogan, Paul Burton, linux-kernel On 4/3/2018 8:55 AM, Sinan Kaya wrote: > While a barrier is present in writeX() function before the register write, > a similar barrier is missing in the readX() function after the register > read. This could allow memory accesses following readX() to observe > stale data. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Reported-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/mips/include/asm/io.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h > index fd00ddaf..6ac502f 100644 > --- a/arch/mips/include/asm/io.h > +++ b/arch/mips/include/asm/io.h > @@ -377,6 +377,7 @@ static inline type pfx##read##bwlq(const volatile void __iomem *mem) \ > BUG(); \ > } \ > \ > + rmb(); \ > return pfx##ioswab##bwlq(__mem, __val); \ > } > > Can we get these merged to 4.17? There was a consensus to fix the architectures having API violation issues. https://www.mail-archive.com/netdev@vger.kernel.org/msg225971.html -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-06 1:34 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-06 1:34 UTC (permalink / raw) To: linux-arm-kernel On 4/3/2018 8:55 AM, Sinan Kaya wrote: > While a barrier is present in writeX() function before the register write, > a similar barrier is missing in the readX() function after the register > read. This could allow memory accesses following readX() to observe > stale data. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Reported-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/mips/include/asm/io.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h > index fd00ddaf..6ac502f 100644 > --- a/arch/mips/include/asm/io.h > +++ b/arch/mips/include/asm/io.h > @@ -377,6 +377,7 @@ static inline type pfx##read##bwlq(const volatile void __iomem *mem) \ > BUG(); \ > } \ > \ > + rmb(); \ > return pfx##ioswab##bwlq(__mem, __val); \ > } > > Can we get these merged to 4.17? There was a consensus to fix the architectures having API violation issues. https://www.mail-archive.com/netdev at vger.kernel.org/msg225971.html -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-06 1:34 ` Sinan Kaya @ 2018-04-06 18:15 ` Sinan Kaya -1 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-06 18:15 UTC (permalink / raw) To: linux-mips, arnd, timur, sulrich Cc: linux-arm-msm, linux-arm-kernel, Ralf Baechle, James Hogan, Paul Burton, linux-kernel On 4/5/2018 9:34 PM, Sinan Kaya wrote: > On 4/3/2018 8:55 AM, Sinan Kaya wrote: >> While a barrier is present in writeX() function before the register write, >> a similar barrier is missing in the readX() function after the register >> read. This could allow memory accesses following readX() to observe >> stale data. >> >> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >> Reported-by: Arnd Bergmann <arnd@arndb.de> >> --- >> arch/mips/include/asm/io.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h >> index fd00ddaf..6ac502f 100644 >> --- a/arch/mips/include/asm/io.h >> +++ b/arch/mips/include/asm/io.h >> @@ -377,6 +377,7 @@ static inline type pfx##read##bwlq(const volatile void __iomem *mem) \ >> BUG(); \ >> } \ >> \ >> + rmb(); \ >> return pfx##ioswab##bwlq(__mem, __val); \ >> } >> >> > > Can we get these merged to 4.17? > > There was a consensus to fix the architectures having API violation issues. > https://www.mail-archive.com/netdev@vger.kernel.org/msg225971.html > > Any news on the MIPS front? Is this something that Arnd can merge? or does it have to go through the MIPS tree. It feels like the MIPS is dead since nobody replied to me in the last few weeks on a very important topic. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-06 18:15 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-06 18:15 UTC (permalink / raw) To: linux-arm-kernel On 4/5/2018 9:34 PM, Sinan Kaya wrote: > On 4/3/2018 8:55 AM, Sinan Kaya wrote: >> While a barrier is present in writeX() function before the register write, >> a similar barrier is missing in the readX() function after the register >> read. This could allow memory accesses following readX() to observe >> stale data. >> >> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >> Reported-by: Arnd Bergmann <arnd@arndb.de> >> --- >> arch/mips/include/asm/io.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h >> index fd00ddaf..6ac502f 100644 >> --- a/arch/mips/include/asm/io.h >> +++ b/arch/mips/include/asm/io.h >> @@ -377,6 +377,7 @@ static inline type pfx##read##bwlq(const volatile void __iomem *mem) \ >> BUG(); \ >> } \ >> \ >> + rmb(); \ >> return pfx##ioswab##bwlq(__mem, __val); \ >> } >> >> > > Can we get these merged to 4.17? > > There was a consensus to fix the architectures having API violation issues. > https://www.mail-archive.com/netdev at vger.kernel.org/msg225971.html > > Any news on the MIPS front? Is this something that Arnd can merge? or does it have to go through the MIPS tree. It feels like the MIPS is dead since nobody replied to me in the last few weeks on a very important topic. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-06 18:15 ` Sinan Kaya @ 2018-04-06 21:26 ` James Hogan -1 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-06 21:26 UTC (permalink / raw) To: Sinan Kaya Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel [-- Attachment #1: Type: text/plain, Size: 854 bytes --] On Fri, Apr 06, 2018 at 02:15:57PM -0400, Sinan Kaya wrote: > On 4/5/2018 9:34 PM, Sinan Kaya wrote: > > Can we get these merged to 4.17? > > > > There was a consensus to fix the architectures having API violation issues. > > https://www.mail-archive.com/netdev@vger.kernel.org/msg225971.html > > > > > > Any news on the MIPS front? Is this something that Arnd can merge? or does it have > to go through the MIPS tree. It needs some MIPS input really. I'll try and take a look soon. Thanks for the nudge. > It feels like the MIPS is dead since nobody replied to me in the last few weeks on > a very important topic. Not dead, just both maintainers heavily distracted by real life right now (which sadly, for me at least, trumps this very important topic) and doing the best they can given the circumstances. Cheers James [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-06 21:26 ` James Hogan 0 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-06 21:26 UTC (permalink / raw) To: linux-arm-kernel On Fri, Apr 06, 2018 at 02:15:57PM -0400, Sinan Kaya wrote: > On 4/5/2018 9:34 PM, Sinan Kaya wrote: > > Can we get these merged to 4.17? > > > > There was a consensus to fix the architectures having API violation issues. > > https://www.mail-archive.com/netdev at vger.kernel.org/msg225971.html > > > > > > Any news on the MIPS front? Is this something that Arnd can merge? or does it have > to go through the MIPS tree. It needs some MIPS input really. I'll try and take a look soon. Thanks for the nudge. > It feels like the MIPS is dead since nobody replied to me in the last few weeks on > a very important topic. Not dead, just both maintainers heavily distracted by real life right now (which sadly, for me at least, trumps this very important topic) and doing the best they can given the circumstances. Cheers James -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180406/50aa7ed9/attachment.sig> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-06 21:26 ` James Hogan @ 2018-04-07 21:43 ` Sinan Kaya -1 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-07 21:43 UTC (permalink / raw) To: James Hogan Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel On 4/6/2018 5:26 PM, James Hogan wrote: > On Fri, Apr 06, 2018 at 02:15:57PM -0400, Sinan Kaya wrote: >> On 4/5/2018 9:34 PM, Sinan Kaya wrote: >>> Can we get these merged to 4.17? >>> >>> There was a consensus to fix the architectures having API violation issues. >>> https://www.mail-archive.com/netdev@vger.kernel.org/msg225971.html >>> >>> >> >> Any news on the MIPS front? Is this something that Arnd can merge? or does it have >> to go through the MIPS tree. > > It needs some MIPS input really. I'll try and take a look soon. Thanks > for the nudge. > >> It feels like the MIPS is dead since nobody replied to me in the last few weeks on >> a very important topic. > > Not dead, just both maintainers heavily distracted by real life right > now (which sadly, for me at least, trumps this very important topic) and > doing the best they can given the circumstances. Thanks for the reply. Glad to hear somebody cares. > > Cheers > James > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-07 21:43 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-07 21:43 UTC (permalink / raw) To: linux-arm-kernel On 4/6/2018 5:26 PM, James Hogan wrote: > On Fri, Apr 06, 2018 at 02:15:57PM -0400, Sinan Kaya wrote: >> On 4/5/2018 9:34 PM, Sinan Kaya wrote: >>> Can we get these merged to 4.17? >>> >>> There was a consensus to fix the architectures having API violation issues. >>> https://www.mail-archive.com/netdev at vger.kernel.org/msg225971.html >>> >>> >> >> Any news on the MIPS front? Is this something that Arnd can merge? or does it have >> to go through the MIPS tree. > > It needs some MIPS input really. I'll try and take a look soon. Thanks > for the nudge. > >> It feels like the MIPS is dead since nobody replied to me in the last few weeks on >> a very important topic. > > Not dead, just both maintainers heavily distracted by real life right > now (which sadly, for me at least, trumps this very important topic) and > doing the best they can given the circumstances. Thanks for the reply. Glad to hear somebody cares. > > Cheers > James > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-07 21:43 ` Sinan Kaya @ 2018-04-11 17:10 ` Sinan Kaya -1 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-11 17:10 UTC (permalink / raw) To: James Hogan Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel On 4/7/2018 5:43 PM, Sinan Kaya wrote: > On 4/6/2018 5:26 PM, James Hogan wrote: >> On Fri, Apr 06, 2018 at 02:15:57PM -0400, Sinan Kaya wrote: >>> On 4/5/2018 9:34 PM, Sinan Kaya wrote: >>>> Can we get these merged to 4.17? >>>> >>>> There was a consensus to fix the architectures having API violation issues. >>>> https://www.mail-archive.com/netdev@vger.kernel.org/msg225971.html >>>> >>>> >>> >>> Any news on the MIPS front? Is this something that Arnd can merge? or does it have >>> to go through the MIPS tree. >> >> It needs some MIPS input really. I'll try and take a look soon. Thanks >> for the nudge. >> >>> It feels like the MIPS is dead since nobody replied to me in the last few weeks on >>> a very important topic. >> >> Not dead, just both maintainers heavily distracted by real life right >> now (which sadly, for me at least, trumps this very important topic) and >> doing the best they can given the circumstances. > > Thanks for the reply. Glad to hear somebody cares. How is the likelihood of getting this fixed on 4.17 kernel? There was an agreement to fix all architectures. MIPS is the only one left. > >> >> Cheers >> James >> > > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-11 17:10 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-11 17:10 UTC (permalink / raw) To: linux-arm-kernel On 4/7/2018 5:43 PM, Sinan Kaya wrote: > On 4/6/2018 5:26 PM, James Hogan wrote: >> On Fri, Apr 06, 2018 at 02:15:57PM -0400, Sinan Kaya wrote: >>> On 4/5/2018 9:34 PM, Sinan Kaya wrote: >>>> Can we get these merged to 4.17? >>>> >>>> There was a consensus to fix the architectures having API violation issues. >>>> https://www.mail-archive.com/netdev at vger.kernel.org/msg225971.html >>>> >>>> >>> >>> Any news on the MIPS front? Is this something that Arnd can merge? or does it have >>> to go through the MIPS tree. >> >> It needs some MIPS input really. I'll try and take a look soon. Thanks >> for the nudge. >> >>> It feels like the MIPS is dead since nobody replied to me in the last few weeks on >>> a very important topic. >> >> Not dead, just both maintainers heavily distracted by real life right >> now (which sadly, for me at least, trumps this very important topic) and >> doing the best they can given the circumstances. > > Thanks for the reply. Glad to hear somebody cares. How is the likelihood of getting this fixed on 4.17 kernel? There was an agreement to fix all architectures. MIPS is the only one left. > >> >> Cheers >> James >> > > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-11 17:10 ` Sinan Kaya (?) (?) @ 2018-04-11 20:26 ` James Hogan -1 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-11 20:26 UTC (permalink / raw) To: Sinan Kaya Cc: linux-mips, sulrich, arnd, linux-arm-msm, timur, linux-kernel, Ralf Baechle, Paul Burton, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 143 bytes --] On Wed, Apr 11, 2018 at 01:10:41PM -0400, Sinan Kaya wrote: > How is the likelihood of getting this fixed on 4.17 kernel? High. Thanks James [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-11 20:26 ` James Hogan 0 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-11 20:26 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 11, 2018 at 01:10:41PM -0400, Sinan Kaya wrote: > How is the likelihood of getting this fixed on 4.17 kernel? High. Thanks James -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180411/a68a6239/attachment.sig> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-11 20:26 ` James Hogan 0 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-11 20:26 UTC (permalink / raw) To: Sinan Kaya Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel [-- Attachment #1: Type: text/plain, Size: 143 bytes --] On Wed, Apr 11, 2018 at 01:10:41PM -0400, Sinan Kaya wrote: > How is the likelihood of getting this fixed on 4.17 kernel? High. Thanks James [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-11 20:26 ` James Hogan 0 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-11 20:26 UTC (permalink / raw) To: Sinan Kaya Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel [-- Attachment #1: Type: text/plain, Size: 143 bytes --] On Wed, Apr 11, 2018 at 01:10:41PM -0400, Sinan Kaya wrote: > How is the likelihood of getting this fixed on 4.17 kernel? High. Thanks James [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-11 20:26 ` James Hogan @ 2018-04-11 20:48 ` Sinan Kaya -1 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-11 20:48 UTC (permalink / raw) To: James Hogan Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel On 4/11/2018 4:26 PM, James Hogan wrote: > On Wed, Apr 11, 2018 at 01:10:41PM -0400, Sinan Kaya wrote: >> How is the likelihood of getting this fixed on 4.17 kernel? > > High. > Thanks for the confirmation. > Thanks > James > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-11 20:48 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-11 20:48 UTC (permalink / raw) To: linux-arm-kernel On 4/11/2018 4:26 PM, James Hogan wrote: > On Wed, Apr 11, 2018 at 01:10:41PM -0400, Sinan Kaya wrote: >> How is the likelihood of getting this fixed on 4.17 kernel? > > High. > Thanks for the confirmation. > Thanks > James > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-06 21:26 ` James Hogan (?) (?) @ 2018-04-11 17:04 ` Maciej W. Rozycki -1 siblings, 0 replies; 40+ messages in thread From: Maciej W. Rozycki @ 2018-04-11 17:04 UTC (permalink / raw) To: James Hogan Cc: linux-mips, sulrich, arnd, linux-arm-msm, timur, linux-kernel, Ralf Baechle, Sinan Kaya, Paul Burton, linux-arm-kernel Hi James, > > Any news on the MIPS front? Is this something that Arnd can merge? or does it have > > to go through the MIPS tree. > > It needs some MIPS input really. I'll try and take a look soon. Thanks > for the nudge. > > > It feels like the MIPS is dead since nobody replied to me in the last few weeks on > > a very important topic. > > Not dead, just both maintainers heavily distracted by real life right > now (which sadly, for me at least, trumps this very important topic) and > doing the best they can given the circumstances. Can I help move this change forward anyhow? Maciej ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-11 17:04 ` Maciej W. Rozycki 0 siblings, 0 replies; 40+ messages in thread From: Maciej W. Rozycki @ 2018-04-11 17:04 UTC (permalink / raw) To: linux-arm-kernel Hi James, > > Any news on the MIPS front? Is this something that Arnd can merge? or does it have > > to go through the MIPS tree. > > It needs some MIPS input really. I'll try and take a look soon. Thanks > for the nudge. > > > It feels like the MIPS is dead since nobody replied to me in the last few weeks on > > a very important topic. > > Not dead, just both maintainers heavily distracted by real life right > now (which sadly, for me at least, trumps this very important topic) and > doing the best they can given the circumstances. Can I help move this change forward anyhow? Maciej ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-11 17:04 ` Maciej W. Rozycki 0 siblings, 0 replies; 40+ messages in thread From: Maciej W. Rozycki @ 2018-04-11 17:04 UTC (permalink / raw) To: James Hogan Cc: Sinan Kaya, linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel Hi James, > > Any news on the MIPS front? Is this something that Arnd can merge? or does it have > > to go through the MIPS tree. > > It needs some MIPS input really. I'll try and take a look soon. Thanks > for the nudge. > > > It feels like the MIPS is dead since nobody replied to me in the last few weeks on > > a very important topic. > > Not dead, just both maintainers heavily distracted by real life right > now (which sadly, for me at least, trumps this very important topic) and > doing the best they can given the circumstances. Can I help move this change forward anyhow? Maciej ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-11 17:04 ` Maciej W. Rozycki 0 siblings, 0 replies; 40+ messages in thread From: Maciej W. Rozycki @ 2018-04-11 17:04 UTC (permalink / raw) To: James Hogan Cc: Sinan Kaya, linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel Hi James, > > Any news on the MIPS front? Is this something that Arnd can merge? or does it have > > to go through the MIPS tree. > > It needs some MIPS input really. I'll try and take a look soon. Thanks > for the nudge. > > > It feels like the MIPS is dead since nobody replied to me in the last few weeks on > > a very important topic. > > Not dead, just both maintainers heavily distracted by real life right > now (which sadly, for me at least, trumps this very important topic) and > doing the best they can given the circumstances. Can I help move this change forward anyhow? Maciej ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-03 12:55 ` Sinan Kaya (?) @ 2018-04-12 21:51 ` James Hogan -1 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-12 21:51 UTC (permalink / raw) To: Sinan Kaya Cc: linux-mips, sulrich, arnd, linux-arm-msm, timur, linux-kernel, Ralf Baechle, Paul Burton, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 1041 bytes --] On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: > While a barrier is present in writeX() function before the register write, > a similar barrier is missing in the readX() function after the register > read. This could allow memory accesses following readX() to observe > stale data. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Reported-by: Arnd Bergmann <arnd@arndb.de> Both patches look like obvious improvements to me, so I'm happy to apply to my fixes branch. I'm guessing the case of a write to DMA buffer (i.e. reusing it) after a MMIO readX() (checking DMA complete) being visible to DMA reads prior to the readX() is precluded by a control dependency (you shouldn't reuse buffer until you've checked DMA is complete). But why don't we always use wmb() in the writeX() case? Might not the cached write to DMA buffer be reordered with the uncached write to MMIO register from the coherent DMA point of view? I'm waiting on feedback from MIPS hardware folk on this topic. Cheers James [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-12 21:51 ` James Hogan 0 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-12 21:51 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: > While a barrier is present in writeX() function before the register write, > a similar barrier is missing in the readX() function after the register > read. This could allow memory accesses following readX() to observe > stale data. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Reported-by: Arnd Bergmann <arnd@arndb.de> Both patches look like obvious improvements to me, so I'm happy to apply to my fixes branch. I'm guessing the case of a write to DMA buffer (i.e. reusing it) after a MMIO readX() (checking DMA complete) being visible to DMA reads prior to the readX() is precluded by a control dependency (you shouldn't reuse buffer until you've checked DMA is complete). But why don't we always use wmb() in the writeX() case? Might not the cached write to DMA buffer be reordered with the uncached write to MMIO register from the coherent DMA point of view? I'm waiting on feedback from MIPS hardware folk on this topic. Cheers James -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180412/086384f7/attachment.sig> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-12 21:51 ` James Hogan 0 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-12 21:51 UTC (permalink / raw) To: Sinan Kaya Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1041 bytes --] On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: > While a barrier is present in writeX() function before the register write, > a similar barrier is missing in the readX() function after the register > read. This could allow memory accesses following readX() to observe > stale data. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > Reported-by: Arnd Bergmann <arnd@arndb.de> Both patches look like obvious improvements to me, so I'm happy to apply to my fixes branch. I'm guessing the case of a write to DMA buffer (i.e. reusing it) after a MMIO readX() (checking DMA complete) being visible to DMA reads prior to the readX() is precluded by a control dependency (you shouldn't reuse buffer until you've checked DMA is complete). But why don't we always use wmb() in the writeX() case? Might not the cached write to DMA buffer be reordered with the uncached write to MMIO register from the coherent DMA point of view? I'm waiting on feedback from MIPS hardware folk on this topic. Cheers James [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-12 21:51 ` James Hogan @ 2018-04-12 21:58 ` James Hogan -1 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-12 21:58 UTC (permalink / raw) To: Sinan Kaya Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel [-- Attachment #1: Type: text/plain, Size: 759 bytes --] On Thu, Apr 12, 2018 at 10:51:49PM +0100, James Hogan wrote: > On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: > > While a barrier is present in writeX() function before the register write, > > a similar barrier is missing in the readX() function after the register > > read. This could allow memory accesses following readX() to observe > > stale data. > > > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > Reported-by: Arnd Bergmann <arnd@arndb.de> > > Both patches look like obvious improvements to me, so I'm happy to apply > to my fixes branch. Though having said that, a comment to go with the rmb() (as suggested by checkpatch) to detail the situation we're concerned about would be nice to have. Cheers James [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-12 21:58 ` James Hogan 0 siblings, 0 replies; 40+ messages in thread From: James Hogan @ 2018-04-12 21:58 UTC (permalink / raw) To: linux-arm-kernel On Thu, Apr 12, 2018 at 10:51:49PM +0100, James Hogan wrote: > On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: > > While a barrier is present in writeX() function before the register write, > > a similar barrier is missing in the readX() function after the register > > read. This could allow memory accesses following readX() to observe > > stale data. > > > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > Reported-by: Arnd Bergmann <arnd@arndb.de> > > Both patches look like obvious improvements to me, so I'm happy to apply > to my fixes branch. Though having said that, a comment to go with the rmb() (as suggested by checkpatch) to detail the situation we're concerned about would be nice to have. Cheers James -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180412/f9f87e90/attachment.sig> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-12 21:58 ` James Hogan @ 2018-04-12 22:38 ` Sinan Kaya -1 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-12 22:38 UTC (permalink / raw) To: James Hogan Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel On 4/12/2018 5:58 PM, James Hogan wrote: > On Thu, Apr 12, 2018 at 10:51:49PM +0100, James Hogan wrote: >> On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: >>> While a barrier is present in writeX() function before the register write, >>> a similar barrier is missing in the readX() function after the register >>> read. This could allow memory accesses following readX() to observe >>> stale data. >>> >>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >>> Reported-by: Arnd Bergmann <arnd@arndb.de> >> >> Both patches look like obvious improvements to me, so I'm happy to apply >> to my fixes branch. > > Though having said that, a comment to go with the rmb() (as suggested by > checkpatch) to detail the situation we're concerned about would be nice > to have. I can send a new version. No worries. Functional correctness is more important at this moment. I can accommodate any nice to haves. > > Cheers > James > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-12 22:38 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-12 22:38 UTC (permalink / raw) To: linux-arm-kernel On 4/12/2018 5:58 PM, James Hogan wrote: > On Thu, Apr 12, 2018 at 10:51:49PM +0100, James Hogan wrote: >> On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: >>> While a barrier is present in writeX() function before the register write, >>> a similar barrier is missing in the readX() function after the register >>> read. This could allow memory accesses following readX() to observe >>> stale data. >>> >>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >>> Reported-by: Arnd Bergmann <arnd@arndb.de> >> >> Both patches look like obvious improvements to me, so I'm happy to apply >> to my fixes branch. > > Though having said that, a comment to go with the rmb() (as suggested by > checkpatch) to detail the situation we're concerned about would be nice > to have. I can send a new version. No worries. Functional correctness is more important at this moment. I can accommodate any nice to haves. > > Cheers > James > -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-12 21:51 ` James Hogan @ 2018-04-12 22:20 ` Sinan Kaya -1 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-12 22:20 UTC (permalink / raw) To: James Hogan Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel On 4/12/2018 5:51 PM, James Hogan wrote: > But why don't we always use wmb() in the writeX() case? Might not the > cached write to DMA buffer be reordered with the uncached write to MMIO > register from the coherent DMA point of view? I'm waiting on feedback > from MIPS hardware folk on this topic. Are you asking about this? #if defined(CONFIG_CPU_CAVIUM_OCTEON) || defined(CONFIG_LOONGSON3_ENHANCEMENT) #define war_io_reorder_wmb() wmb() #else -#define war_io_reorder_wmb() do { } while (0) +#define war_io_reorder_wmb() barrier() #endif There is a write barrier in writeX() but seem to be different from platform to platform. I'm not familiar with the MIPS architecture. We can always use a wmb() but it could hurt performance where it is not needed. This is the kind of input we need from the MIPS folks if compiler barrier is enough or we need a wmb() for all cases. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-12 22:20 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-12 22:20 UTC (permalink / raw) To: linux-arm-kernel On 4/12/2018 5:51 PM, James Hogan wrote: > But why don't we always use wmb() in the writeX() case? Might not the > cached write to DMA buffer be reordered with the uncached write to MMIO > register from the coherent DMA point of view? I'm waiting on feedback > from MIPS hardware folk on this topic. Are you asking about this? #if defined(CONFIG_CPU_CAVIUM_OCTEON) || defined(CONFIG_LOONGSON3_ENHANCEMENT) #define war_io_reorder_wmb() wmb() #else -#define war_io_reorder_wmb() do { } while (0) +#define war_io_reorder_wmb() barrier() #endif There is a write barrier in writeX() but seem to be different from platform to platform. I'm not familiar with the MIPS architecture. We can always use a wmb() but it could hurt performance where it is not needed. This is the kind of input we need from the MIPS folks if compiler barrier is enough or we need a wmb() for all cases. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-12 21:51 ` James Hogan (?) @ 2018-04-13 15:41 ` David Laight -1 siblings, 0 replies; 40+ messages in thread From: David Laight @ 2018-04-13 15:41 UTC (permalink / raw) To: 'James Hogan', Sinan Kaya Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel From: James Hogan > Sent: 12 April 2018 22:52 > On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: > > While a barrier is present in writeX() function before the register write, > > a similar barrier is missing in the readX() function after the register > > read. This could allow memory accesses following readX() to observe > > stale data. > > > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > Reported-by: Arnd Bergmann <arnd@arndb.de> > > Both patches look like obvious improvements to me, so I'm happy to apply > to my fixes branch. Don't you also need at least barrier() between the register write in writeX() and the register read in readX()? On ppc you probably need eieio. Or are drivers expected to insert that one? If they need to insert that one then why not all the others?? David ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-13 15:41 ` David Laight 0 siblings, 0 replies; 40+ messages in thread From: David Laight @ 2018-04-13 15:41 UTC (permalink / raw) To: linux-arm-kernel From: James Hogan > Sent: 12 April 2018 22:52 > On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: > > While a barrier is present in writeX() function before the register write, > > a similar barrier is missing in the readX() function after the register > > read. This could allow memory accesses following readX() to observe > > stale data. > > > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > Reported-by: Arnd Bergmann <arnd@arndb.de> > > Both patches look like obvious improvements to me, so I'm happy to apply > to my fixes branch. Don't you also need at least barrier() between the register write in writeX() and the register read in readX()? On ppc you probably need eieio. Or are drivers expected to insert that one? If they need to insert that one then why not all the others?? David ^ permalink raw reply [flat|nested] 40+ messages in thread
* RE: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-13 15:41 ` David Laight 0 siblings, 0 replies; 40+ messages in thread From: David Laight @ 2018-04-13 15:41 UTC (permalink / raw) To: 'James Hogan', Sinan Kaya Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel From: James Hogan > Sent: 12 April 2018 22:52 > On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: > > While a barrier is present in writeX() function before the register write, > > a similar barrier is missing in the readX() function after the register > > read. This could allow memory accesses following readX() to observe > > stale data. > > > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > Reported-by: Arnd Bergmann <arnd@arndb.de> > > Both patches look like obvious improvements to me, so I'm happy to apply > to my fixes branch. Don't you also need at least barrier() between the register write in writeX() and the register read in readX()? On ppc you probably need eieio. Or are drivers expected to insert that one? If they need to insert that one then why not all the others?? David ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() 2018-04-13 15:41 ` David Laight (?) (?) @ 2018-04-13 16:36 ` Sinan Kaya -1 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-13 16:36 UTC (permalink / raw) To: David Laight, 'James Hogan' Cc: linux-mips, sulrich, arnd, linux-arm-msm, timur, linux-kernel, Ralf Baechle, Paul Burton, linux-arm-kernel On 4/13/2018 11:41 AM, David Laight wrote: > From: James Hogan >> Sent: 12 April 2018 22:52 >> On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: >>> While a barrier is present in writeX() function before the register write, >>> a similar barrier is missing in the readX() function after the register >>> read. This could allow memory accesses following readX() to observe >>> stale data. >>> >>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >>> Reported-by: Arnd Bergmann <arnd@arndb.de> >> >> Both patches look like obvious improvements to me, so I'm happy to apply >> to my fixes branch. > > Don't you also need at least barrier() between the register write in writeX() > and the register read in readX()? > On ppc you probably need eieio. > Or are drivers expected to insert that one? > If they need to insert that one then why not all the others?? > Good question. The volatile in here should prevent compiler from reordering the register read or write instructions. static inline type pfx##read##bwlq(const volatile void __iomem *mem) This is the solution all other architectures rely on especially via __raw_readX() and __raw_writeX() API. Now, things can get reordered when it leaves the CPU. This is guaranteed by embedding wmb() and rmb() into the writeX() and readX() functions in other architectures. This ordering guarantee has been agreed to be the responsibility of the architecture not drivers. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-13 16:36 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-13 16:36 UTC (permalink / raw) To: linux-arm-kernel On 4/13/2018 11:41 AM, David Laight wrote: > From: James Hogan >> Sent: 12 April 2018 22:52 >> On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: >>> While a barrier is present in writeX() function before the register write, >>> a similar barrier is missing in the readX() function after the register >>> read. This could allow memory accesses following readX() to observe >>> stale data. >>> >>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >>> Reported-by: Arnd Bergmann <arnd@arndb.de> >> >> Both patches look like obvious improvements to me, so I'm happy to apply >> to my fixes branch. > > Don't you also need at least barrier() between the register write in writeX() > and the register read in readX()? > On ppc you probably need eieio. > Or are drivers expected to insert that one? > If they need to insert that one then why not all the others?? > Good question. The volatile in here should prevent compiler from reordering the register read or write instructions. static inline type pfx##read##bwlq(const volatile void __iomem *mem) This is the solution all other architectures rely on especially via __raw_readX() and __raw_writeX() API. Now, things can get reordered when it leaves the CPU. This is guaranteed by embedding wmb() and rmb() into the writeX() and readX() functions in other architectures. This ordering guarantee has been agreed to be the responsibility of the architecture not drivers. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-13 16:36 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-13 16:36 UTC (permalink / raw) To: David Laight, 'James Hogan' Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel On 4/13/2018 11:41 AM, David Laight wrote: > From: James Hogan >> Sent: 12 April 2018 22:52 >> On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: >>> While a barrier is present in writeX() function before the register write, >>> a similar barrier is missing in the readX() function after the register >>> read. This could allow memory accesses following readX() to observe >>> stale data. >>> >>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >>> Reported-by: Arnd Bergmann <arnd@arndb.de> >> >> Both patches look like obvious improvements to me, so I'm happy to apply >> to my fixes branch. > > Don't you also need at least barrier() between the register write in writeX() > and the register read in readX()? > On ppc you probably need eieio. > Or are drivers expected to insert that one? > If they need to insert that one then why not all the others?? > Good question. The volatile in here should prevent compiler from reordering the register read or write instructions. static inline type pfx##read##bwlq(const volatile void __iomem *mem) This is the solution all other architectures rely on especially via __raw_readX() and __raw_writeX() API. Now, things can get reordered when it leaves the CPU. This is guaranteed by embedding wmb() and rmb() into the writeX() and readX() functions in other architectures. This ordering guarantee has been agreed to be the responsibility of the architecture not drivers. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() @ 2018-04-13 16:36 ` Sinan Kaya 0 siblings, 0 replies; 40+ messages in thread From: Sinan Kaya @ 2018-04-13 16:36 UTC (permalink / raw) To: David Laight, 'James Hogan' Cc: linux-mips, arnd, timur, sulrich, linux-arm-msm, linux-arm-kernel, Ralf Baechle, Paul Burton, linux-kernel On 4/13/2018 11:41 AM, David Laight wrote: > From: James Hogan >> Sent: 12 April 2018 22:52 >> On Tue, Apr 03, 2018 at 08:55:04AM -0400, Sinan Kaya wrote: >>> While a barrier is present in writeX() function before the register write, >>> a similar barrier is missing in the readX() function after the register >>> read. This could allow memory accesses following readX() to observe >>> stale data. >>> >>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> >>> Reported-by: Arnd Bergmann <arnd@arndb.de> >> >> Both patches look like obvious improvements to me, so I'm happy to apply >> to my fixes branch. > > Don't you also need at least barrier() between the register write in writeX() > and the register read in readX()? > On ppc you probably need eieio. > Or are drivers expected to insert that one? > If they need to insert that one then why not all the others?? > Good question. The volatile in here should prevent compiler from reordering the register read or write instructions. static inline type pfx##read##bwlq(const volatile void __iomem *mem) This is the solution all other architectures rely on especially via __raw_readX() and __raw_writeX() API. Now, things can get reordered when it leaves the CPU. This is guaranteed by embedding wmb() and rmb() into the writeX() and readX() functions in other architectures. This ordering guarantee has been agreed to be the responsibility of the architecture not drivers. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2018-04-13 16:36 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-03 12:55 [PATCH v3 1/2] MIPS: io: prevent compiler reordering on the default writeX() implementation Sinan Kaya 2018-04-03 12:55 ` Sinan Kaya 2018-04-03 12:55 ` [PATCH v3 2/2] MIPS: io: add a barrier after register read in readX() Sinan Kaya 2018-04-03 12:55 ` Sinan Kaya 2018-04-06 1:34 ` Sinan Kaya 2018-04-06 1:34 ` Sinan Kaya 2018-04-06 18:15 ` Sinan Kaya 2018-04-06 18:15 ` Sinan Kaya 2018-04-06 21:26 ` James Hogan 2018-04-06 21:26 ` James Hogan 2018-04-07 21:43 ` Sinan Kaya 2018-04-07 21:43 ` Sinan Kaya 2018-04-11 17:10 ` Sinan Kaya 2018-04-11 17:10 ` Sinan Kaya 2018-04-11 20:26 ` James Hogan 2018-04-11 20:26 ` James Hogan 2018-04-11 20:26 ` James Hogan 2018-04-11 20:26 ` James Hogan 2018-04-11 20:48 ` Sinan Kaya 2018-04-11 20:48 ` Sinan Kaya 2018-04-11 17:04 ` Maciej W. Rozycki 2018-04-11 17:04 ` Maciej W. Rozycki 2018-04-11 17:04 ` Maciej W. Rozycki 2018-04-11 17:04 ` Maciej W. Rozycki 2018-04-12 21:51 ` James Hogan 2018-04-12 21:51 ` James Hogan 2018-04-12 21:51 ` James Hogan 2018-04-12 21:58 ` James Hogan 2018-04-12 21:58 ` James Hogan 2018-04-12 22:38 ` Sinan Kaya 2018-04-12 22:38 ` Sinan Kaya 2018-04-12 22:20 ` Sinan Kaya 2018-04-12 22:20 ` Sinan Kaya 2018-04-13 15:41 ` David Laight 2018-04-13 15:41 ` David Laight 2018-04-13 15:41 ` David Laight 2018-04-13 16:36 ` Sinan Kaya 2018-04-13 16:36 ` Sinan Kaya 2018-04-13 16:36 ` Sinan Kaya 2018-04-13 16:36 ` Sinan Kaya
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.