Re: [RFC v5 15/38] powerpc: helper function to read,write AMR,IAMR,UAMOR registers

On Thu, Jul 13, 2017 at 07:49:05PM +1000, Balbir Singh wrote:

On Thu, Jul 13, 2017 at 5:55 PM, Ram Pai [off-list ref] wrote:

quoted

On Wed, Jul 12, 2017 at 03:26:01PM +1000, Balbir Singh wrote:

quoted

On Wed,  5 Jul 2017 14:21:52 -0700
Ram Pai [off-list ref] wrote:

quoted

Implements helper functions to read and write the key related
registers; AMR, IAMR, UAMOR.

AMR register tracks the read,write permission of a key
IAMR register tracks the execute permission of a key
UAMOR register enables and disables a key

Signed-off-by: Ram Pai <redacted>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h |   60 ++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 85bc987..435d6a7 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h

@@ -428,6 +428,66 @@ static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
            pte_update(mm, addr, ptep, 0, _PAGE_PRIVILEGED, 1);
 }

+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
+
+#include <asm/reg.h>
+static inline u64 read_amr(void)
+{
+   return mfspr(SPRN_AMR);
+}
+static inline void write_amr(u64 value)
+{
+   mtspr(SPRN_AMR, value);
+}
+static inline u64 read_iamr(void)
+{
+   return mfspr(SPRN_IAMR);
+}
+static inline void write_iamr(u64 value)
+{
+   mtspr(SPRN_IAMR, value);
+}
+static inline u64 read_uamor(void)
+{
+   return mfspr(SPRN_UAMOR);
+}
+static inline void write_uamor(u64 value)
+{
+   mtspr(SPRN_UAMOR, value);
+}
+
+#else /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
+
+static inline u64 read_amr(void)
+{
+   WARN(1, "%s called with MEMORY PROTECTION KEYS disabled\n", __func__);
+   return -1;
+}

Why do we need to have a version here if we are going to WARN(), why not
let the compilation fail if called from outside of CONFIG_PPC64_MEMORY_PROTECTION_KEYS?
Is that the intention?

I did not want to stop someone; kernel module for example, from calling
these interfaces from outside the pkey domain.

Either way can be argued to be correct, I suppose.

Nope, build failures are better than run time failures, otherwise the
kernel will split its guts warning and warning here.

Well these are helper functions that can be called by anyone under
any situation. I will rather have them defined unconditionally; under
no ifdefs.  No spewing of warnings anymore. The registers will
be read or written as told. It just makes sense that way.

RP

-- 
Ram Pai