| Message ID | 20151009112418.GN32532@n2100.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 09, 2015 at 12:24:18PM +0100, Russell King - ARM Linux wrote: > On Fri, Oct 09, 2015 at 11:53:09AM +0100, Will Deacon wrote: > > It looks like we're faulting on the TLBI instruction, because it's > > targetting a userspace address (r0 == 0xb6f01002) and the DAC prohibits > > access to userspace. > > That's kind'a weird - I'd have expected _cache_ operations to fault, > but not TLB operations. From a quick search of the ARM ARM, I can't > find anything that says that TLB operations are allowed to fault. Yes, you're right. I was concentrating on the cross-call scenario and forgot this was TLB not cache maintenance, so faulting is indeed bizarre. > > It's weird that this only seems to happen on 11MPCore > > though; if this core was one of the guys getting cross-called, then I > > could understand the bug, but the lr suggests that CPU 2 is initiating > > the flush, so I'd expect the same problem to appear on any ARMv6 part. > > It sounds to me like a CPU bug, but one which we need to work around. > ipi_flush_tlb_range() will be the function concerned, we need to > save-and-enable, and then restore the user access state around that > call. > > > Russell, have you tried the s/w PAN stuff on any v6 CPUs? > > No. I have considered having the Realview EB board as part of the test > farm, but as that board is hassle to get going, I deem the hardware to > be too unreliable for that. (I reported the problem at the time.) > > Linus, can you try the patch below to see if it resolves the problem > you're seeing please? If this fixes things, I'll dig up the errata document to see if this is a known hardware issue. Will
On Fri, Oct 9, 2015 at 1:24 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > Linus, can you try the patch below to see if it resolves the problem > you're seeing please? > > arch/arm/kernel/smp_tlb.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c > index 2e72be4f623e..7cb079e74010 100644 > --- a/arch/arm/kernel/smp_tlb.c > +++ b/arch/arm/kernel/smp_tlb.c > @@ -9,6 +9,7 @@ > */ > #include <linux/preempt.h> > #include <linux/smp.h> > +#include <linux/uaccess.h> > > #include <asm/smp_plat.h> > #include <asm/tlbflush.h> > @@ -40,8 +41,11 @@ static inline void ipi_flush_tlb_mm(void *arg) > static inline void ipi_flush_tlb_page(void *arg) > { > struct tlb_args *ta = (struct tlb_args *)arg; > + unsigned int __ua_flags = uaccess_save_and_enable(); > > local_flush_tlb_page(ta->ta_vma, ta->ta_start); > + > + uaccess_restore(__ua_flags); > } > > static inline void ipi_flush_tlb_kernel_page(void *arg) > @@ -54,8 +58,11 @@ static inline void ipi_flush_tlb_kernel_page(void *arg) > static inline void ipi_flush_tlb_range(void *arg) > { > struct tlb_args *ta = (struct tlb_args *)arg; > + unsigned int __ua_flags = uaccess_save_and_enable(); > > local_flush_tlb_range(ta->ta_vma, ta->ta_start, ta->ta_end); > + > + uaccess_restore(__ua_flags); > } Yes this makes it rock solid again. Tested-by: Linus Walleij <linus.walleij@linaro.org> I guess it will only affect ARMv6 SMP (11MPCore) machines and there are not so many of them AFAICT, so I guess it's a bit annoying to have that in the ipi_flush_tlb_kernel_page() for all of the ARM CPUs :( but it does work. Yours, Linus Walleij
On Fri, Oct 9, 2015 at 1:24 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > [Will] >> It's weird that this only seems to happen on 11MPCore >> though; if this core was one of the guys getting cross-called, then I >> could understand the bug, but the lr suggests that CPU 2 is initiating >> the flush, so I'd expect the same problem to appear on any ARMv6 part. > > It sounds to me like a CPU bug, but one which we need to work around. > ipi_flush_tlb_range() will be the function concerned, we need to > save-and-enable, and then restore the user access state around that > call. > >> Russell, have you tried the s/w PAN stuff on any v6 CPUs? > > No. I have considered having the Realview EB board as part of the test > farm, but as that board is hassle to get going, I deem the hardware to > be too unreliable for that. (I reported the problem at the time.) > > Linus, can you try the patch below to see if it resolves the problem > you're seeing please? > > arch/arm/kernel/smp_tlb.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c > index 2e72be4f623e..7cb079e74010 100644 > --- a/arch/arm/kernel/smp_tlb.c > +++ b/arch/arm/kernel/smp_tlb.c > @@ -9,6 +9,7 @@ > */ > #include <linux/preempt.h> > #include <linux/smp.h> > +#include <linux/uaccess.h> > > #include <asm/smp_plat.h> > #include <asm/tlbflush.h> > @@ -40,8 +41,11 @@ static inline void ipi_flush_tlb_mm(void *arg) > static inline void ipi_flush_tlb_page(void *arg) > { > struct tlb_args *ta = (struct tlb_args *)arg; > + unsigned int __ua_flags = uaccess_save_and_enable(); > > local_flush_tlb_page(ta->ta_vma, ta->ta_start); > + > + uaccess_restore(__ua_flags); > } > > static inline void ipi_flush_tlb_kernel_page(void *arg) > @@ -54,8 +58,11 @@ static inline void ipi_flush_tlb_kernel_page(void *arg) > static inline void ipi_flush_tlb_range(void *arg) > { > struct tlb_args *ta = (struct tlb_args *)arg; > + unsigned int __ua_flags = uaccess_save_and_enable(); > > local_flush_tlb_range(ta->ta_vma, ta->ta_start, ta->ta_end); > + > + uaccess_restore(__ua_flags); > } > > static inline void ipi_flush_tlb_kernel_range(void *arg) Do we have a solution for this? I'm carrying the patch and v4.3-rc6 is broken on upstream RealView PB11MPCore, at least for me. :( Yours, Linus Walleij
On Fri, Oct 23, 2015 at 10:05:06AM +0200, Linus Walleij wrote: > Do we have a solution for this? > > I'm carrying the patch and v4.3-rc6 is broken on upstream > RealView PB11MPCore, at least for me. :( Will was going to do some research and get back to me, but what he was going to look at has been archived off-site. Will said he wanted to reproduce it on the latest silicon, but I haven't heard anything further and he hasn't been around this week.
On Fri, Oct 23, 2015 at 09:46:40AM +0100, Russell King - ARM Linux wrote: > On Fri, Oct 23, 2015 at 10:05:06AM +0200, Linus Walleij wrote: > > Do we have a solution for this? > > > > I'm carrying the patch and v4.3-rc6 is broken on upstream > > RealView PB11MPCore, at least for me. :( > > Will was going to do some research and get back to me, but what he was > going to look at has been archived off-site. Will said he wanted to > reproduce it on the latest silicon, but I haven't heard anything > further and he hasn't been around this week. I'm afraid my digging didn't prove to be very enlightening. One thing I'd like to try, is to see if we can reproduce the problem on an 11MPCore platform with a CPU > r0p0. However, I don't have access to such a platform that can run mainline, so without that I think we have to go with Russell's workaround predicated on a check for 11MPCore. Will
diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c index 2e72be4f623e..7cb079e74010 100644 --- a/arch/arm/kernel/smp_tlb.c +++ b/arch/arm/kernel/smp_tlb.c @@ -9,6 +9,7 @@ */ #include <linux/preempt.h> #include <linux/smp.h> +#include <linux/uaccess.h> #include <asm/smp_plat.h> #include <asm/tlbflush.h> @@ -40,8 +41,11 @@ static inline void ipi_flush_tlb_mm(void *arg) static inline void ipi_flush_tlb_page(void *arg) { struct tlb_args *ta = (struct tlb_args *)arg; + unsigned int __ua_flags = uaccess_save_and_enable(); local_flush_tlb_page(ta->ta_vma, ta->ta_start); + + uaccess_restore(__ua_flags); } static inline void ipi_flush_tlb_kernel_page(void *arg) @@ -54,8 +58,11 @@ static inline void ipi_flush_tlb_kernel_page(void *arg) static inline void ipi_flush_tlb_range(void *arg) { struct tlb_args *ta = (struct tlb_args *)arg; + unsigned int __ua_flags = uaccess_save_and_enable(); local_flush_tlb_range(ta->ta_vma, ta->ta_start, ta->ta_end); + + uaccess_restore(__ua_flags); } static inline void ipi_flush_tlb_kernel_range(void *arg)