Re: [PATCH] arm64: clear_page() shouldn't use DC ZVA when DCZID_EL0.DZP == 1

On Thu, Oct 28, 2021 at 08:46:10AM +0100, Will Deacon wrote:

On Wed, Oct 27, 2021 at 12:09:47PM +0100, Mark Rutland wrote:

quoted

On Tue, Oct 26, 2021 at 11:44:51PM -0700, Reiji Watanabe wrote:

quoted

On Tue, Oct 26, 2021 at 5:23 AM Mark Rutland [off-list ref] wrote:

quoted

On Tue, Oct 26, 2021 at 12:22:20PM +0100, Robin Murphy wrote:

quoted

On 2021-10-26 04:48, Reiji Watanabe wrote:

quoted

Currently, clear_page() uses DC ZVA instruction unconditionally.  But it
should make sure that DCZID_EL0.DZP, which indicates whether or not use
of DC ZVA instruction is prohibited, is zero when using the instruction.
Use stp as memset does instead when DCZID_EL0.DZP == 1.

Signed-off-by: Reiji Watanabe <redacted>
---
  arch/arm64/lib/clear_page.S | 11 +++++++++++
  1 file changed, 11 insertions(+)

diff --git a/arch/arm64/lib/clear_page.S b/arch/arm64/lib/clear_page.S
index b84b179edba3..7ce1bfa4081c 100644
--- a/arch/arm64/lib/clear_page.S
+++ b/arch/arm64/lib/clear_page.S

@@ -16,6 +16,7 @@
   */
  SYM_FUNC_START_PI(clear_page)
    mrs     x1, dczid_el0
+   tbnz    x1, #4, 2f      /* Branch if DC GVA is prohibited */

DCZID_EL0.DZP (AKA DCZID_EL0[4]) says whether all of DC {ZVA,GVA,GZVA}
are prohibited. This loop uses DZ ZVA, not GC GVA, so it'd be nice to
s/GVA/ZVA/ here.

Thank you for catching it ! I will fix that.

quoted

Howver, `DC GVA` and `DC GZVA` are both used in mte_set_mem_tag_range(),
which'll need a similar update...

Yes, I'm aware of that and mte_zero_clear_page_tags() needs to get
updated as well.  But, Since I'm not familiar with MTE (and I don't
have any plans to use MTE yet), I didn't work on them (I'm not sure
how I can test them).
I might try to fix them separately later as well when I have time
(not so soon most likely though).

My view is that we should either:

* Document that we require DCZID_EL0.DZP==0, as is implicitly the case
  today.

I disagree with that. There's nothing wrong in trapping this stuff, as long
as you go ahead and emulate it, which is exactly why we aren't checking it at
the moment as EL2 should be prepared to handle the trap. The Arm ARM talks
about the instructions being "prohibited" but that doesn't mean anything --
the reality is that they trap to EL2.

TBH, I think trapping DC ZVA rather than forcing it to be UNDEF was an
architectural mistake.

I think the intent of the "prohibited" wording (and exposure of
DCZID_EL0.DZP to EL0 and EL1) is clearly that SW should *avoid* DC ZVA and
friends when DCZID_EL0.DZP is set (and libc and the Arm optimized
routines have *always* checked that), and performance would be abysmal
with emulated DC ZVA, so at minimum we want to avoid hitting emulation
in the common case.

We could document *that* though?

I guess; though it makes me uneasy since the architecture clearly pushes
people to read CTR_EL0.DZP, and heavily implies that there's no need to
emulate, so I don't think we have much of a leg to stand on from an
architecture PoV.

If we need to support this, my preference would be to support
DCZID_EL0.DZP==1 by avoiding the trapped instructions entirely.

I realise the problem as ever is "what does userspace do?".

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel