* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
@ 2019-11-05 10:29 ` Will Deacon
0 siblings, 0 replies; 24+ messages in thread
From: Will Deacon @ 2019-11-05 10:29 UTC (permalink / raw)
To: John Stultz
Cc: Alistair Delva, Catalin Marinas, Sandeep Patil, stable, linux-arm-kernel
Hi John,
On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > and made dirty on a subsequent write either through the hardware DBM
> > (dirty bit management) mechanism or through a write page fault. A clean
> > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > clear.
> >
> > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > bit handling out of set_pte_at()"), it was the responsibility of
> > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > software PTE_DIRTY bit was not set. However, the above commit removed
> > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > unchanged. The result is that shared+writable mappings are now dirty by
> > default
> >
> > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > attributes.
> >
> > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > Cc: <stable@vger.kernel.org> # 4.14.x-
> > Cc: Will Deacon <will@kernel.org>
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>
> Hey,
> So I'm not yet sure why, but I've just validated that this patch is
> causing trouble with booting AOSP on HiKey960 with 5.4-rc6 (-rc5 works
> fine).
Hmm. Annoying this wasn't spotted by CI.
> Its odd, because the system does boot and is alive, but seems to stall
> out at the boot animation, and userland never finishes coming up to
> the home screen. It just sits there without a useful error message
> that I can find so far. Reverting just this patch seems to solve it
> and it boots all the way.
Given that I don't think the HiKey960 supports h/w DBM, my initial guess
is that the GPU is stuck on a page fault.
> I'll try to dig further to see what might be going on (the mali driver
> is a prime suspect here), but I wanted to raise the flag since we're
> at the end of the -rc cycle.
What exactly are you using for the mali driver?
As an experiment, can you try reverting just the part of the patch that
removes PTE_DIRTY from the PROT_* definitions? (see below)
Thanks,
Will
--->8
diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
index 8dc6c5cdabe6..17a8eb13f4ce 100644
--- a/arch/arm64/include/asm/pgtable-prot.h
+++ b/arch/arm64/include/asm/pgtable-prot.h
@@ -32,11 +32,11 @@
#define PROT_DEFAULT (_PROT_DEFAULT | PTE_MAYBE_NG)
#define PROT_SECT_DEFAULT (_PROT_SECT_DEFAULT | PMD_MAYBE_NG)
-#define PROT_DEVICE_nGnRnE (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_WRITE | PTE_ATTRINDX(MT_DEVICE_nGnRnE))
-#define PROT_DEVICE_nGnRE (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_WRITE | PTE_ATTRINDX(MT_DEVICE_nGnRE))
-#define PROT_NORMAL_NC (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_WRITE | PTE_ATTRINDX(MT_NORMAL_NC))
-#define PROT_NORMAL_WT (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_WRITE | PTE_ATTRINDX(MT_NORMAL_WT))
-#define PROT_NORMAL (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_WRITE | PTE_ATTRINDX(MT_NORMAL))
+#define PROT_DEVICE_nGnRnE (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE | PTE_ATTRINDX(MT_DEVICE_nGnRnE))
+#define PROT_DEVICE_nGnRE (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE | PTE_ATTRINDX(MT_DEVICE_nGnRE))
+#define PROT_NORMAL_NC (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE | PTE_ATTRINDX(MT_NORMAL_NC))
+#define PROT_NORMAL_WT (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE | PTE_ATTRINDX(MT_NORMAL_WT))
+#define PROT_NORMAL (PROT_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE | PTE_ATTRINDX(MT_NORMAL))
#define PROT_SECT_DEVICE_nGnRE (PROT_SECT_DEFAULT | PMD_SECT_PXN | PMD_SECT_UXN | PMD_ATTRINDX(MT_DEVICE_nGnRE))
#define PROT_SECT_NORMAL (PROT_SECT_DEFAULT | PMD_SECT_PXN | PMD_SECT_UXN | PMD_ATTRINDX(MT_NORMAL))
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
2019-11-05 10:29 ` Will Deacon
@ 2019-11-05 16:54 ` Catalin Marinas
-1 siblings, 0 replies; 24+ messages in thread
From: Catalin Marinas @ 2019-11-05 16:54 UTC (permalink / raw)
To: Will Deacon
Cc: John Stultz, Alistair Delva, Sandeep Patil, stable,
linux-arm-kernel, Steve Capper
On Tue, Nov 05, 2019 at 10:29:03AM +0000, Will Deacon wrote:
> On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >
> > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > and made dirty on a subsequent write either through the hardware DBM
> > > (dirty bit management) mechanism or through a write page fault. A clean
> > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > clear.
> > >
> > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > bit handling out of set_pte_at()"), it was the responsibility of
> > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > unchanged. The result is that shared+writable mappings are now dirty by
> > > default
> > >
> > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > attributes.
> > >
> > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > Cc: Will Deacon <will@kernel.org>
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
[...]
> As an experiment, can you try reverting just the part of the patch that
> removes PTE_DIRTY from the PROT_* definitions? (see below)
Another thing worth trying is reverting commit 747a70e60b72 ("arm64: Fix
copy-on-write referencing in HugeTLB") when this patch is applied. That
commit is not just about hugetlb but changes pte_same() to ignore
PTE_RDONLY on the assumption that this is set by set_pte_at(). We
subsequently changed set_pte_at() to drop PTE_RDONLY.
--
Catalin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
@ 2019-11-05 16:54 ` Catalin Marinas
0 siblings, 0 replies; 24+ messages in thread
From: Catalin Marinas @ 2019-11-05 16:54 UTC (permalink / raw)
To: Will Deacon
Cc: Alistair Delva, Sandeep Patil, Steve Capper, stable, John Stultz,
linux-arm-kernel
On Tue, Nov 05, 2019 at 10:29:03AM +0000, Will Deacon wrote:
> On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >
> > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > and made dirty on a subsequent write either through the hardware DBM
> > > (dirty bit management) mechanism or through a write page fault. A clean
> > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > clear.
> > >
> > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > bit handling out of set_pte_at()"), it was the responsibility of
> > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > unchanged. The result is that shared+writable mappings are now dirty by
> > > default
> > >
> > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > attributes.
> > >
> > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > Cc: Will Deacon <will@kernel.org>
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
[...]
> As an experiment, can you try reverting just the part of the patch that
> removes PTE_DIRTY from the PROT_* definitions? (see below)
Another thing worth trying is reverting commit 747a70e60b72 ("arm64: Fix
copy-on-write referencing in HugeTLB") when this patch is applied. That
commit is not just about hugetlb but changes pte_same() to ignore
PTE_RDONLY on the assumption that this is set by set_pte_at(). We
subsequently changed set_pte_at() to drop PTE_RDONLY.
--
Catalin
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
2019-11-05 16:54 ` Catalin Marinas
@ 2019-11-05 21:17 ` John Stultz
-1 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-05 21:17 UTC (permalink / raw)
To: Catalin Marinas
Cc: Will Deacon, Alistair Delva, Sandeep Patil, stable,
linux-arm-kernel, Steve Capper
On Tue, Nov 5, 2019 at 8:54 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tue, Nov 05, 2019 at 10:29:03AM +0000, Will Deacon wrote:
> > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > >
> > > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > > and made dirty on a subsequent write either through the hardware DBM
> > > > (dirty bit management) mechanism or through a write page fault. A clean
> > > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > > clear.
> > > >
> > > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > > bit handling out of set_pte_at()"), it was the responsibility of
> > > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > > unchanged. The result is that shared+writable mappings are now dirty by
> > > > default
> > > >
> > > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > > attributes.
> > > >
> > > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > > Cc: Will Deacon <will@kernel.org>
> > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> [...]
> > As an experiment, can you try reverting just the part of the patch that
> > removes PTE_DIRTY from the PROT_* definitions? (see below)
>
> Another thing worth trying is reverting commit 747a70e60b72 ("arm64: Fix
> copy-on-write referencing in HugeTLB") when this patch is applied. That
> commit is not just about hugetlb but changes pte_same() to ignore
> PTE_RDONLY on the assumption that this is set by set_pte_at(). We
> subsequently changed set_pte_at() to drop PTE_RDONLY.
Just to confirm, reverting 747a70e60b72 instead of aa57157be69f also
seems to avoid the issue I'm seeing.
I've not tried Will patch but I'll do that next. Though its not clear
if you wanted me to revert 747a70e60b72 on top of Will's test patch or
not?
thanks
-john
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
@ 2019-11-05 21:17 ` John Stultz
0 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-05 21:17 UTC (permalink / raw)
To: Catalin Marinas
Cc: Alistair Delva, Sandeep Patil, Steve Capper, stable, Will Deacon,
linux-arm-kernel
On Tue, Nov 5, 2019 at 8:54 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tue, Nov 05, 2019 at 10:29:03AM +0000, Will Deacon wrote:
> > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > >
> > > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > > and made dirty on a subsequent write either through the hardware DBM
> > > > (dirty bit management) mechanism or through a write page fault. A clean
> > > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > > clear.
> > > >
> > > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > > bit handling out of set_pte_at()"), it was the responsibility of
> > > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > > unchanged. The result is that shared+writable mappings are now dirty by
> > > > default
> > > >
> > > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > > attributes.
> > > >
> > > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > > Cc: Will Deacon <will@kernel.org>
> > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> [...]
> > As an experiment, can you try reverting just the part of the patch that
> > removes PTE_DIRTY from the PROT_* definitions? (see below)
>
> Another thing worth trying is reverting commit 747a70e60b72 ("arm64: Fix
> copy-on-write referencing in HugeTLB") when this patch is applied. That
> commit is not just about hugetlb but changes pte_same() to ignore
> PTE_RDONLY on the assumption that this is set by set_pte_at(). We
> subsequently changed set_pte_at() to drop PTE_RDONLY.
Just to confirm, reverting 747a70e60b72 instead of aa57157be69f also
seems to avoid the issue I'm seeing.
I've not tried Will patch but I'll do that next. Though its not clear
if you wanted me to revert 747a70e60b72 on top of Will's test patch or
not?
thanks
-john
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
2019-11-05 21:17 ` John Stultz
@ 2019-11-05 21:29 ` John Stultz
-1 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-05 21:29 UTC (permalink / raw)
To: Catalin Marinas
Cc: Will Deacon, Alistair Delva, Sandeep Patil, stable,
linux-arm-kernel, Steve Capper
On Tue, Nov 5, 2019 at 1:17 PM John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Nov 5, 2019 at 8:54 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Tue, Nov 05, 2019 at 10:29:03AM +0000, Will Deacon wrote:
> > > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > >
> > > > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > > > and made dirty on a subsequent write either through the hardware DBM
> > > > > (dirty bit management) mechanism or through a write page fault. A clean
> > > > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > > > clear.
> > > > >
> > > > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > > > bit handling out of set_pte_at()"), it was the responsibility of
> > > > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > > > unchanged. The result is that shared+writable mappings are now dirty by
> > > > > default
> > > > >
> > > > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > > > attributes.
> > > > >
> > > > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > > > Cc: Will Deacon <will@kernel.org>
> > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > [...]
> > > As an experiment, can you try reverting just the part of the patch that
> > > removes PTE_DIRTY from the PROT_* definitions? (see below)
> >
> > Another thing worth trying is reverting commit 747a70e60b72 ("arm64: Fix
> > copy-on-write referencing in HugeTLB") when this patch is applied. That
> > commit is not just about hugetlb but changes pte_same() to ignore
> > PTE_RDONLY on the assumption that this is set by set_pte_at(). We
> > subsequently changed set_pte_at() to drop PTE_RDONLY.
>
> Just to confirm, reverting 747a70e60b72 instead of aa57157be69f also
> seems to avoid the issue I'm seeing.
>
> I've not tried Will patch but I'll do that next. Though its not clear
> if you wanted me to revert 747a70e60b72 on top of Will's test patch or
> not?
Not sure if its useful data, but while Will's patch on its own didn't
change the behavior, it along with reverting 747a70e60b72 seems to
work the same as just reverting 747a70e60b72 alone.
thanks
-john
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
@ 2019-11-05 21:29 ` John Stultz
0 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-05 21:29 UTC (permalink / raw)
To: Catalin Marinas
Cc: Alistair Delva, Sandeep Patil, Steve Capper, stable, Will Deacon,
linux-arm-kernel
On Tue, Nov 5, 2019 at 1:17 PM John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Nov 5, 2019 at 8:54 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Tue, Nov 05, 2019 at 10:29:03AM +0000, Will Deacon wrote:
> > > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > >
> > > > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > > > and made dirty on a subsequent write either through the hardware DBM
> > > > > (dirty bit management) mechanism or through a write page fault. A clean
> > > > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > > > clear.
> > > > >
> > > > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > > > bit handling out of set_pte_at()"), it was the responsibility of
> > > > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > > > unchanged. The result is that shared+writable mappings are now dirty by
> > > > > default
> > > > >
> > > > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > > > attributes.
> > > > >
> > > > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > > > Cc: Will Deacon <will@kernel.org>
> > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > [...]
> > > As an experiment, can you try reverting just the part of the patch that
> > > removes PTE_DIRTY from the PROT_* definitions? (see below)
> >
> > Another thing worth trying is reverting commit 747a70e60b72 ("arm64: Fix
> > copy-on-write referencing in HugeTLB") when this patch is applied. That
> > commit is not just about hugetlb but changes pte_same() to ignore
> > PTE_RDONLY on the assumption that this is set by set_pte_at(). We
> > subsequently changed set_pte_at() to drop PTE_RDONLY.
>
> Just to confirm, reverting 747a70e60b72 instead of aa57157be69f also
> seems to avoid the issue I'm seeing.
>
> I've not tried Will patch but I'll do that next. Though its not clear
> if you wanted me to revert 747a70e60b72 on top of Will's test patch or
> not?
Not sure if its useful data, but while Will's patch on its own didn't
change the behavior, it along with reverting 747a70e60b72 seems to
work the same as just reverting 747a70e60b72 alone.
thanks
-john
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
2019-11-05 21:17 ` John Stultz
@ 2019-11-06 8:59 ` Catalin Marinas
-1 siblings, 0 replies; 24+ messages in thread
From: Catalin Marinas @ 2019-11-06 8:59 UTC (permalink / raw)
To: John Stultz
Cc: Will Deacon, Alistair Delva, Sandeep Patil, stable,
linux-arm-kernel, Steve Capper
On Tue, Nov 05, 2019 at 01:17:11PM -0800, John Stultz wrote:
> On Tue, Nov 5, 2019 at 8:54 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Tue, Nov 05, 2019 at 10:29:03AM +0000, Will Deacon wrote:
> > > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > >
> > > > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > > > and made dirty on a subsequent write either through the hardware DBM
> > > > > (dirty bit management) mechanism or through a write page fault. A clean
> > > > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > > > clear.
> > > > >
> > > > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > > > bit handling out of set_pte_at()"), it was the responsibility of
> > > > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > > > unchanged. The result is that shared+writable mappings are now dirty by
> > > > > default
> > > > >
> > > > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > > > attributes.
> > > > >
> > > > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > > > Cc: Will Deacon <will@kernel.org>
> > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > [...]
> > > As an experiment, can you try reverting just the part of the patch that
> > > removes PTE_DIRTY from the PROT_* definitions? (see below)
> >
> > Another thing worth trying is reverting commit 747a70e60b72 ("arm64: Fix
> > copy-on-write referencing in HugeTLB") when this patch is applied. That
> > commit is not just about hugetlb but changes pte_same() to ignore
> > PTE_RDONLY on the assumption that this is set by set_pte_at(). We
> > subsequently changed set_pte_at() to drop PTE_RDONLY.
>
> Just to confirm, reverting 747a70e60b72 instead of aa57157be69f also
> seems to avoid the issue I'm seeing.
Thanks for confirming. I'm not sure about all the interactions in your
kernel but just looking at commit 747a70e60b72 it likely needs to be
reverted anyway. I'll send a separate patch and hopefully Steve can
confirm that it doesn't break the original hugetlb use-case.
--
Catalin
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
@ 2019-11-06 8:59 ` Catalin Marinas
0 siblings, 0 replies; 24+ messages in thread
From: Catalin Marinas @ 2019-11-06 8:59 UTC (permalink / raw)
To: John Stultz
Cc: Alistair Delva, Sandeep Patil, Steve Capper, stable, Will Deacon,
linux-arm-kernel
On Tue, Nov 05, 2019 at 01:17:11PM -0800, John Stultz wrote:
> On Tue, Nov 5, 2019 at 8:54 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Tue, Nov 05, 2019 at 10:29:03AM +0000, Will Deacon wrote:
> > > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > >
> > > > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > > > and made dirty on a subsequent write either through the hardware DBM
> > > > > (dirty bit management) mechanism or through a write page fault. A clean
> > > > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > > > clear.
> > > > >
> > > > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > > > bit handling out of set_pte_at()"), it was the responsibility of
> > > > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > > > unchanged. The result is that shared+writable mappings are now dirty by
> > > > > default
> > > > >
> > > > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > > > attributes.
> > > > >
> > > > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > > > Cc: Will Deacon <will@kernel.org>
> > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > [...]
> > > As an experiment, can you try reverting just the part of the patch that
> > > removes PTE_DIRTY from the PROT_* definitions? (see below)
> >
> > Another thing worth trying is reverting commit 747a70e60b72 ("arm64: Fix
> > copy-on-write referencing in HugeTLB") when this patch is applied. That
> > commit is not just about hugetlb but changes pte_same() to ignore
> > PTE_RDONLY on the assumption that this is set by set_pte_at(). We
> > subsequently changed set_pte_at() to drop PTE_RDONLY.
>
> Just to confirm, reverting 747a70e60b72 instead of aa57157be69f also
> seems to avoid the issue I'm seeing.
Thanks for confirming. I'm not sure about all the interactions in your
kernel but just looking at commit 747a70e60b72 it likely needs to be
reverted anyway. I'll send a separate patch and hopefully Steve can
confirm that it doesn't break the original hugetlb use-case.
--
Catalin
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
2019-11-05 10:29 ` Will Deacon
@ 2019-11-05 17:06 ` John Stultz
-1 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-05 17:06 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, linux-arm-kernel, stable, Alistair Delva, Sandeep Patil
On Tue, Nov 5, 2019 at 2:29 AM Will Deacon <will@kernel.org> wrote:
>
> Hi John,
>
> On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >
> > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > and made dirty on a subsequent write either through the hardware DBM
> > > (dirty bit management) mechanism or through a write page fault. A clean
> > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > clear.
> > >
> > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > bit handling out of set_pte_at()"), it was the responsibility of
> > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > unchanged. The result is that shared+writable mappings are now dirty by
> > > default
> > >
> > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > attributes.
> > >
> > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > Cc: Will Deacon <will@kernel.org>
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> >
> > Hey,
> > So I'm not yet sure why, but I've just validated that this patch is
> > causing trouble with booting AOSP on HiKey960 with 5.4-rc6 (-rc5 works
> > fine).
>
> Hmm. Annoying this wasn't spotted by CI.
>
> > Its odd, because the system does boot and is alive, but seems to stall
> > out at the boot animation, and userland never finishes coming up to
> > the home screen. It just sits there without a useful error message
> > that I can find so far. Reverting just this patch seems to solve it
> > and it boots all the way.
>
> Given that I don't think the HiKey960 supports h/w DBM, my initial guess
> is that the GPU is stuck on a page fault.
>
> > I'll try to dig further to see what might be going on (the mali driver
> > is a prime suspect here), but I wanted to raise the flag since we're
> > at the end of the -rc cycle.
>
> What exactly are you using for the mali driver?
I've got an old r10p0 bifrost blob we were given and kernel patches
I've carried forward since then.
Again, I don't want to distract you too much for something that may be
related to a blob driver. I mostly just wanted to raise a flag in case
there was something off that might affect others.
> As an experiment, can you try reverting just the part of the patch that
> removes PTE_DIRTY from the PROT_* definitions? (see below)
I'll give this a try! Feel free to let me know if there's anything
else I should test.
thanks
-john
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
@ 2019-11-05 17:06 ` John Stultz
0 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-05 17:06 UTC (permalink / raw)
To: Will Deacon
Cc: Alistair Delva, Catalin Marinas, Sandeep Patil, stable, linux-arm-kernel
On Tue, Nov 5, 2019 at 2:29 AM Will Deacon <will@kernel.org> wrote:
>
> Hi John,
>
> On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >
> > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > and made dirty on a subsequent write either through the hardware DBM
> > > (dirty bit management) mechanism or through a write page fault. A clean
> > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > clear.
> > >
> > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > bit handling out of set_pte_at()"), it was the responsibility of
> > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > unchanged. The result is that shared+writable mappings are now dirty by
> > > default
> > >
> > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > attributes.
> > >
> > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > Cc: Will Deacon <will@kernel.org>
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> >
> > Hey,
> > So I'm not yet sure why, but I've just validated that this patch is
> > causing trouble with booting AOSP on HiKey960 with 5.4-rc6 (-rc5 works
> > fine).
>
> Hmm. Annoying this wasn't spotted by CI.
>
> > Its odd, because the system does boot and is alive, but seems to stall
> > out at the boot animation, and userland never finishes coming up to
> > the home screen. It just sits there without a useful error message
> > that I can find so far. Reverting just this patch seems to solve it
> > and it boots all the way.
>
> Given that I don't think the HiKey960 supports h/w DBM, my initial guess
> is that the GPU is stuck on a page fault.
>
> > I'll try to dig further to see what might be going on (the mali driver
> > is a prime suspect here), but I wanted to raise the flag since we're
> > at the end of the -rc cycle.
>
> What exactly are you using for the mali driver?
I've got an old r10p0 bifrost blob we were given and kernel patches
I've carried forward since then.
Again, I don't want to distract you too much for something that may be
related to a blob driver. I mostly just wanted to raise a flag in case
there was something off that might affect others.
> As an experiment, can you try reverting just the part of the patch that
> removes PTE_DIRTY from the PROT_* definitions? (see below)
I'll give this a try! Feel free to let me know if there's anything
else I should test.
thanks
-john
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
2019-11-05 17:06 ` John Stultz
@ 2019-11-05 18:22 ` Will Deacon
-1 siblings, 0 replies; 24+ messages in thread
From: Will Deacon @ 2019-11-05 18:22 UTC (permalink / raw)
To: John Stultz
Cc: Catalin Marinas, linux-arm-kernel, stable, Alistair Delva, Sandeep Patil
On Tue, Nov 05, 2019 at 09:06:43AM -0800, John Stultz wrote:
> On Tue, Nov 5, 2019 at 2:29 AM Will Deacon <will@kernel.org> wrote:
> > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > So I'm not yet sure why, but I've just validated that this patch is
> > > causing trouble with booting AOSP on HiKey960 with 5.4-rc6 (-rc5 works
> > > fine).
[...]
> > As an experiment, can you try reverting just the part of the patch that
> > removes PTE_DIRTY from the PROT_* definitions? (see below)
>
> I'll give this a try! Feel free to let me know if there's anything
> else I should test.
Thanks. Also worth trying to revert 747a70e60b72 instead of this patch, as
Catalin suggested.
Will
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
@ 2019-11-05 18:22 ` Will Deacon
0 siblings, 0 replies; 24+ messages in thread
From: Will Deacon @ 2019-11-05 18:22 UTC (permalink / raw)
To: John Stultz
Cc: Alistair Delva, Catalin Marinas, Sandeep Patil, stable, linux-arm-kernel
On Tue, Nov 05, 2019 at 09:06:43AM -0800, John Stultz wrote:
> On Tue, Nov 5, 2019 at 2:29 AM Will Deacon <will@kernel.org> wrote:
> > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > So I'm not yet sure why, but I've just validated that this patch is
> > > causing trouble with booting AOSP on HiKey960 with 5.4-rc6 (-rc5 works
> > > fine).
[...]
> > As an experiment, can you try reverting just the part of the patch that
> > removes PTE_DIRTY from the PROT_* definitions? (see below)
>
> I'll give this a try! Feel free to let me know if there's anything
> else I should test.
Thanks. Also worth trying to revert 747a70e60b72 instead of this patch, as
Catalin suggested.
Will
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
2019-11-05 17:06 ` John Stultz
@ 2019-11-06 4:56 ` John Stultz
-1 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-06 4:56 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, linux-arm-kernel, stable, Alistair Delva, Sandeep Patil
On Tue, Nov 5, 2019 at 9:06 AM John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Nov 5, 2019 at 2:29 AM Will Deacon <will@kernel.org> wrote:
> >
> > Hi John,
> >
> > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > >
> > > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > > and made dirty on a subsequent write either through the hardware DBM
> > > > (dirty bit management) mechanism or through a write page fault. A clean
> > > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > > clear.
> > > >
> > > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > > bit handling out of set_pte_at()"), it was the responsibility of
> > > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > > unchanged. The result is that shared+writable mappings are now dirty by
> > > > default
> > > >
> > > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > > attributes.
> > > >
> > > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > > Cc: Will Deacon <will@kernel.org>
> > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > >
> > > Hey,
> > > So I'm not yet sure why, but I've just validated that this patch is
> > > causing trouble with booting AOSP on HiKey960 with 5.4-rc6 (-rc5 works
> > > fine).
> >
> > Hmm. Annoying this wasn't spotted by CI.
> >
> > > Its odd, because the system does boot and is alive, but seems to stall
> > > out at the boot animation, and userland never finishes coming up to
> > > the home screen. It just sits there without a useful error message
> > > that I can find so far. Reverting just this patch seems to solve it
> > > and it boots all the way.
> >
> > Given that I don't think the HiKey960 supports h/w DBM, my initial guess
> > is that the GPU is stuck on a page fault.
> >
> > > I'll try to dig further to see what might be going on (the mali driver
> > > is a prime suspect here), but I wanted to raise the flag since we're
> > > at the end of the -rc cycle.
> >
> > What exactly are you using for the mali driver?
>
> I've got an old r10p0 bifrost blob we were given and kernel patches
> I've carried forward since then.
>
> Again, I don't want to distract you too much for something that may be
> related to a blob driver. I mostly just wanted to raise a flag in case
> there was something off that might affect others.
Just as a further detail (about to close up for the day), I'm also
seeing this issue on the HiKey board as well. Similarly reverting
747a70e60b72 resolves it.
Its a mali blob driver too, but a different one (utgard) which makes
me suspect this might be a real issue w/ something in AOSP.
I'll be testing on a db845c tomorrow morning to see if I can trigger
it there as well.
thanks
-john
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
@ 2019-11-06 4:56 ` John Stultz
0 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-06 4:56 UTC (permalink / raw)
To: Will Deacon
Cc: Alistair Delva, Catalin Marinas, Sandeep Patil, stable, linux-arm-kernel
On Tue, Nov 5, 2019 at 9:06 AM John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Nov 5, 2019 at 2:29 AM Will Deacon <will@kernel.org> wrote:
> >
> > Hi John,
> >
> > On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > >
> > > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > > and made dirty on a subsequent write either through the hardware DBM
> > > > (dirty bit management) mechanism or through a write page fault. A clean
> > > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > > clear.
> > > >
> > > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > > bit handling out of set_pte_at()"), it was the responsibility of
> > > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > > unchanged. The result is that shared+writable mappings are now dirty by
> > > > default
> > > >
> > > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > > attributes.
> > > >
> > > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > > Cc: Will Deacon <will@kernel.org>
> > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > >
> > > Hey,
> > > So I'm not yet sure why, but I've just validated that this patch is
> > > causing trouble with booting AOSP on HiKey960 with 5.4-rc6 (-rc5 works
> > > fine).
> >
> > Hmm. Annoying this wasn't spotted by CI.
> >
> > > Its odd, because the system does boot and is alive, but seems to stall
> > > out at the boot animation, and userland never finishes coming up to
> > > the home screen. It just sits there without a useful error message
> > > that I can find so far. Reverting just this patch seems to solve it
> > > and it boots all the way.
> >
> > Given that I don't think the HiKey960 supports h/w DBM, my initial guess
> > is that the GPU is stuck on a page fault.
> >
> > > I'll try to dig further to see what might be going on (the mali driver
> > > is a prime suspect here), but I wanted to raise the flag since we're
> > > at the end of the -rc cycle.
> >
> > What exactly are you using for the mali driver?
>
> I've got an old r10p0 bifrost blob we were given and kernel patches
> I've carried forward since then.
>
> Again, I don't want to distract you too much for something that may be
> related to a blob driver. I mostly just wanted to raise a flag in case
> there was something off that might affect others.
Just as a further detail (about to close up for the day), I'm also
seeing this issue on the HiKey board as well. Similarly reverting
747a70e60b72 resolves it.
Its a mali blob driver too, but a different one (utgard) which makes
me suspect this might be a real issue w/ something in AOSP.
I'll be testing on a db845c tomorrow morning to see if I can trigger
it there as well.
thanks
-john
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
2019-11-05 10:29 ` Will Deacon
@ 2019-11-05 21:24 ` John Stultz
-1 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-05 21:24 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, linux-arm-kernel, stable, Alistair Delva, Sandeep Patil
On Tue, Nov 5, 2019 at 2:29 AM Will Deacon <will@kernel.org> wrote:
> On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >
> > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > and made dirty on a subsequent write either through the hardware DBM
> > > (dirty bit management) mechanism or through a write page fault. A clean
> > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > clear.
> > >
> > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > bit handling out of set_pte_at()"), it was the responsibility of
> > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > unchanged. The result is that shared+writable mappings are now dirty by
> > > default
> > >
> > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > attributes.
> > >
> > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > Cc: Will Deacon <will@kernel.org>
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> >
> > Hey,
> > So I'm not yet sure why, but I've just validated that this patch is
> > causing trouble with booting AOSP on HiKey960 with 5.4-rc6 (-rc5 works
> > fine).
>
> Hmm. Annoying this wasn't spotted by CI.
>
> > Its odd, because the system does boot and is alive, but seems to stall
> > out at the boot animation, and userland never finishes coming up to
> > the home screen. It just sits there without a useful error message
> > that I can find so far. Reverting just this patch seems to solve it
> > and it boots all the way.
>
> Given that I don't think the HiKey960 supports h/w DBM, my initial guess
> is that the GPU is stuck on a page fault.
>
> > I'll try to dig further to see what might be going on (the mali driver
> > is a prime suspect here), but I wanted to raise the flag since we're
> > at the end of the -rc cycle.
>
> What exactly are you using for the mali driver?
>
> As an experiment, can you try reverting just the part of the patch that
> removes PTE_DIRTY from the PROT_* definitions? (see below)
So reverting just the bit you sent here re-adding the PTE_DIRTY bit
didn't seem to fix it. I still see things stalling at the boot
animation.
thanks
-john
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] arm64: Ensure VM_WRITE|VM_SHARED ptes are clean by default
@ 2019-11-05 21:24 ` John Stultz
0 siblings, 0 replies; 24+ messages in thread
From: John Stultz @ 2019-11-05 21:24 UTC (permalink / raw)
To: Will Deacon
Cc: Alistair Delva, Catalin Marinas, Sandeep Patil, stable, linux-arm-kernel
On Tue, Nov 5, 2019 at 2:29 AM Will Deacon <will@kernel.org> wrote:
> On Mon, Nov 04, 2019 at 05:16:42PM -0800, John Stultz wrote:
> > On Tue, Oct 29, 2019 at 8:31 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >
> > > Shared and writable mappings (__S.1.) should be clean (!dirty) initially
> > > and made dirty on a subsequent write either through the hardware DBM
> > > (dirty bit management) mechanism or through a write page fault. A clean
> > > pte for the arm64 kernel is one that has PTE_RDONLY set and PTE_DIRTY
> > > clear.
> > >
> > > The PAGE_SHARED{,_EXEC} attributes have PTE_WRITE set (PTE_DBM) and
> > > PTE_DIRTY clear. Prior to commit 73e86cb03cf2 ("arm64: Move PTE_RDONLY
> > > bit handling out of set_pte_at()"), it was the responsibility of
> > > set_pte_at() to set the PTE_RDONLY bit and mark the pte clean if the
> > > software PTE_DIRTY bit was not set. However, the above commit removed
> > > the pte_sw_dirty() check and the subsequent setting of PTE_RDONLY in
> > > set_pte_at() while leaving the PAGE_SHARED{,_EXEC} definitions
> > > unchanged. The result is that shared+writable mappings are now dirty by
> > > default
> > >
> > > Fix the above by explicitly setting PTE_RDONLY in PAGE_SHARED{,_EXEC}.
> > > In addition, remove the superfluous PTE_DIRTY bit from the kernel PROT_*
> > > attributes.
> > >
> > > Fixes: 73e86cb03cf2 ("arm64: Move PTE_RDONLY bit handling out of set_pte_at()")
> > > Cc: <stable@vger.kernel.org> # 4.14.x-
> > > Cc: Will Deacon <will@kernel.org>
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> >
> > Hey,
> > So I'm not yet sure why, but I've just validated that this patch is
> > causing trouble with booting AOSP on HiKey960 with 5.4-rc6 (-rc5 works
> > fine).
>
> Hmm. Annoying this wasn't spotted by CI.
>
> > Its odd, because the system does boot and is alive, but seems to stall
> > out at the boot animation, and userland never finishes coming up to
> > the home screen. It just sits there without a useful error message
> > that I can find so far. Reverting just this patch seems to solve it
> > and it boots all the way.
>
> Given that I don't think the HiKey960 supports h/w DBM, my initial guess
> is that the GPU is stuck on a page fault.
>
> > I'll try to dig further to see what might be going on (the mali driver
> > is a prime suspect here), but I wanted to raise the flag since we're
> > at the end of the -rc cycle.
>
> What exactly are you using for the mali driver?
>
> As an experiment, can you try reverting just the part of the patch that
> removes PTE_DIRTY from the PROT_* definitions? (see below)
So reverting just the bit you sent here re-adding the PTE_DIRTY bit
didn't seem to fix it. I still see things stalling at the boot
animation.
thanks
-john
_______________________________________________
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] 24+ messages in thread