Re: [PATCH] sparc64: Add support for Application Data Integrity (ADI)

Hi Khalid,

On Fri, Mar 4, 2016 at 4:42 AM, Khalid Aziz [off-list ref] wrote:

On 03/02/2016 06:33 PM, Julian Calaby wrote:

quoted

Hi Khalid,

A couple of other comments:

On Thu, Mar 3, 2016 at 5:54 AM, Khalid Aziz [off-list ref]
wrote:

quoted

Enable Application Data Integrity (ADI) support in the sparc
kernel for applications to use ADI in userspace. ADI is a new
feature supported on sparc M7 and newer processors. ADI is supported
for data fetches only and not instruction fetches. This patch adds
prctl commands to enable and disable ADI (TSTATE.mcde), return ADI
parameters to userspace, enable/disable MCD (Memory Corruption
Detection) on selected memory ranges and enable TTE.mcd in PTEs. It
also adds handlers for all traps related to MCD. ADI is not enabled
by default for any task and a task must explicitly enable ADI
(TSTATE.mcde), turn MCD on on a memory range and set version tag
for ADI to be effective for the task. This patch adds support for
ADI for hugepages only. Addresses passed into system calls must be
non-ADI tagged addresses.

Signed-off-by: Khalid Aziz <redacted>
---
NOTES: ADI is a new feature added to M7 processor to allow hardware
         to catch rogue accesses to memory. An app can enable ADI on
         its data pages, set version tags on them and use versioned
         addresses (bits 63-60 of the address contain a version tag)
         to access the data pages. If a rogue app attempts to access
         ADI enabled data pages, its access is blocked and processor
         generates an exception. Enabling this functionality for all
         data pages of an app requires adding infrastructure to save
         version tags for any data pages that get swapped out and
         restoring those tags when pages are swapped back in. In this
         first implementation I am enabling ADI for hugepages only
         since these pages are locked in memory and hence avoid the
         issue of saving and restoring tags. Once this core functionality
         is stable, ADI for other memory pages can be enabled more
         easily.

  Documentation/prctl/sparc_adi.txt     |  62 ++++++++++
  Documentation/sparc/adi.txt           | 206
+++++++++++++++++++++++++++++++
  arch/sparc/Kconfig                    |  12 ++
  arch/sparc/include/asm/hugetlb.h      |  14 +++
  arch/sparc/include/asm/hypervisor.h   |   2 +
  arch/sparc/include/asm/mmu_64.h       |   1 +
  arch/sparc/include/asm/pgtable_64.h   |  15 +++
  arch/sparc/include/asm/processor_64.h |  19 +++
  arch/sparc/include/asm/ttable.h       |  10 ++
  arch/sparc/include/uapi/asm/asi.h     |   3 +
  arch/sparc/include/uapi/asm/pstate.h  |  10 ++
  arch/sparc/kernel/entry.h             |   3 +
  arch/sparc/kernel/head_64.S           |   1 +
  arch/sparc/kernel/mdesc.c             |  81 +++++++++++++
  arch/sparc/kernel/process_64.c        | 221
++++++++++++++++++++++++++++++++++
  arch/sparc/kernel/sun4v_mcd.S         |  16 +++
  arch/sparc/kernel/traps_64.c          |  96 ++++++++++++++-
  arch/sparc/kernel/ttable_64.S         |   6 +-
  include/linux/mm.h                    |   2 +
  include/uapi/asm-generic/siginfo.h    |   5 +-
  include/uapi/linux/prctl.h            |  16 +++
  kernel/sys.c                          |  30 +++++
  22 files changed, 825 insertions(+), 6 deletions(-)
  create mode 100644 Documentation/prctl/sparc_adi.txt
  create mode 100644 Documentation/sparc/adi.txt
  create mode 100644 arch/sparc/kernel/sun4v_mcd.S

diff --git a/arch/sparc/include/asm/pgtable_64.h

b/arch/sparc/include/asm/pgtable_64.h
index 131d36f..cddea30 100644

--- a/arch/sparc/include/asm/pgtable_64.h
+++ b/arch/sparc/include/asm/pgtable_64.h

@@ -162,6 +162,9 @@ bool kern_addr_valid(unsigned long addr);
  #define _PAGE_E_4V       _AC(0x0000000000000800,UL) /* side-Effect

*/
  #define _PAGE_CP_4V      _AC(0x0000000000000400,UL) /* Cacheable in
P-Cache */
  #define _PAGE_CV_4V      _AC(0x0000000000000200,UL) /* Cacheable in
V-Cache */
+/* Bit 9 is used to enable MCD corruption detection instead on M7
+ */
+#define _PAGE_MCD_4V     _AC(0x0000000000000200,UL) /* Memory Corruption
*/

I'm not sure that everywhere _PAGE_CV_4V is used is guarded against
setting it on M7, could someone who knows the code better than I do
please check that? It looks like the tests around it's use are
essentially "is it sun4v".

I'm probably being paranoid, but reused values like this make me worry.

I took care of this issue in an earlier patch (commit
494e5b6faeda1d1e830a13e10b3c7bc323f35d97 - "sparc: Resolve conflict between
sparc v9 and M7 on usage of bit 9 of TTE"), so I think we are ok here.

Ah, I remember those changes, however I didn't recall which processor
they were for, so that's awesome.

quoted

quoted

  #define _PAGE_P_4V       _AC(0x0000000000000100,UL) /* Privileged Page
*/
  #define _PAGE_EXEC_4V    _AC(0x0000000000000080,UL) /* Executable Page
*/
  #define _PAGE_W_4V       _AC(0x0000000000000040,UL) /* Writable
*/

diff --git a/arch/sparc/include/uapi/asm/pstate.h

b/arch/sparc/include/uapi/asm/pstate.h
index cf832e1..d0521db 100644

--- a/arch/sparc/include/uapi/asm/pstate.h
+++ b/arch/sparc/include/uapi/asm/pstate.h

@@ -10,7 +10,12 @@
   *
-----------------------------------------------------------------------
   *  63  12  11   10    9     8    7   6   5     4     3     2     1

0
   */
+/* IG on V9 conflicts with MCDE on M7. PSTATE_MCDE will only be used on
+ * processors that support ADI which do not use IG, hence there is no
+ * functional conflict
+ */
  #define PSTATE_IG   _AC(0x0000000000000800,UL) /* Interrupt Globals.
*/
+#define PSTATE_MCDE _AC(0x0000000000000800,UL) /* MCD Enable
*/

Again, I can't tell if the code that uses PSTATE_IG is guarded against
use on M7. Could someone else please check? It's used in cherrs.S
which appears to be Cheetah specific, so that's not a problem, however
it's also used in ultra.S in xcall_sync_tick which might get patched
out however I don't know the code well enough to be certain. I'm also
guessing that as this file is in include/uapi, userspace could use it
for something.

My understanding of the code in ultra.S is xcall_sync_tick doe snot get
called on sun4v, so PSTATE_IG will not get set unintentionally on M7.
include/uapi is an interesting thought. PSTATE is a privileged register, so
userspace can not write to it directly without using a system call. I don't
think that is an issue here.

Is it something userspace would interpret then? I guess any software
making use of the PSTATE or TSTATE registers would have to handle any
processor differences itself.

Thanks for explaining this!

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>