[{"id":1781915,"web_url":"http://patchwork.ozlabs.org/comment/1781915/","msgid":"","list_archive_url":null,"date":"2017-10-06T22:12:09","subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","submitter":{"id":71125,"url":"http://patchwork.ozlabs.org/api/people/71125/","name":"Anthony Yznaga","email":"anthony.yznaga@oracle.com"},"content":"> On Sep 25, 2017, at 9:49 AM, Khalid Aziz wrote:\n> \n> ADI is a new feature supported on SPARC M7 and newer processors to allow\n> hardware to catch rogue accesses to memory. ADI is supported for data\n> fetches only and not instruction fetches. An app can enable ADI on its\n> data pages, set version tags on them and use versioned addresses to\n> access the data pages. Upper bits of the address contain the version\n> tag. On M7 processors, upper four bits (bits 63-60) contain the version\n> tag. If a rogue app attempts to access ADI enabled data pages, its\n> access is blocked and processor generates an exception. Please see\n> Documentation/sparc/adi.txt for further details.\n> \n> This patch extends mprotect to enable ADI (TSTATE.mcde), enable/disable\n> MCD (Memory Corruption Detection) on selected memory ranges, enable\n> TTE.mcd in PTEs, return ADI parameters to userspace and save/restore ADI\n> version tags on page swap out/in or migration. ADI is not enabled by\n\nI still don't believe migration is properly supported. Your\nimplementation is relying on a fault happening on a page while its\nmigration is in progress so that do_swap_page() will be called, but\nI don't see how do_swap_page() will be called if a fault does not\nhappen until after the migration has completed.\n\n\n> default for any task. A task must explicitly enable ADI on a memory\n> range and set version tag for ADI to be effective for the task.\n> \n> Signed-off-by: Khalid Aziz \n> Cc: Khalid Aziz \n> ---\n> v8: \n> \t- Added note to doc about non-faulting loads not triggering\n> \t ADI tag mismatch and more details on special tag values\n> \t of 0x0 and 0xf, as suggested by Anthony Yznaga)\n> \t- Added an IPI on mprotect(...PROT_ADI...) call to set\n> \t TSTATE.MCDE on threads running on other processors and\n> \t restore of TSTATE.MCDE on context switch (suggested by\n> \t David Miller)\n> \t- Removed restriction on enabling ADI on read-only memory\n> \t (suggested by Anthony Yznaga)\n> \t- Changed kzalloc() for tag storage to use GFP_NOWAIT\n> \t- Added code to handle overflow and underflow when allocating\n> \t tag storage, as suggested by Anthony Yznaga\n> \t- Replaced sun_m7_patch_1insn_range() with sun4v_patch_1insn_range()\n> \t which is functionally identical (suggested by Anthony Yznaga)\n> \t- Added membar after restoring ADI tags in copy_user_highpage(),\n> \t as suggested by David Miller\n> \n> v7:\n> \t- Enhanced arch_validate_prot() to enable ADI only on writable\n> \t addresses backed by physical RAM\n> \t- Added support for saving/restoring ADI tags for each ADI\n> \t block size address range on a page on swap in/out\n> \t- Added code to copy ADI tags on COW\n> \t- Updated values for auxiliary vectors to not conflict with\n> \t values on other architectures to avoid conflict in glibc. glibc\n> \t consolidates all auxiliary vectors into its headers and\n> \t duplicate values in consolidated header are problematic\n> \t- Disable same page merging on ADI enabled pages since ADI tags\n> \t may not match on pages with identical data\n> \t- Broke the patch up further into smaller patches\n> \n> v6:\n> \t- Eliminated instructions to read and write PSTATE as well as\n> \t MCDPER and PMCDPER on every access to userspace addresses\n> \t by setting PSTATE and PMCDPER correctly upon entry into\n> \t kernel. PSTATE.mcde and PMCDPER are set upon entry into\n> \t kernel when running on an M7 processor. PSTATE.mcde being\n> \t set only affects memory accesses that have TTE.mcd set.\n> \t PMCDPER being set only affects writes to memory addresses\n> \t that have TTE.mcd set. This ensures any faults caused by\n> \t ADI tag mismatch on a write are exposed before kernel returns\n> \t to userspace.\n> \n> v5:\n> \t- Fixed indentation issues and instrcuctions in assembly code\n> \t- Removed CONFIG_SPARC64 from mdesc.c\n> \t- Changed to maintain state of MCDPER register in thread info\n> \t flags as opposed to in mm context. MCDPER is a per-thread\n> \t state and belongs in thread info flag as opposed to mm context\n> \t which is shared across threads. Added comments to clarify this\n> \t is a lazily maintained state and must be updated on context\n> \t switch and copy_process()\n> \t- Updated code to use the new arch_do_swap_page() and\n> \t arch_unmap_one() functions\n> \n> v4:\n> \t- Broke patch up into smaller patches\n> \n> v3:\n> \t- Removed CONFIG_SPARC_ADI\n> \t- Replaced prctl commands with mprotect\n> \t- Added auxiliary vectors for ADI parameters\n> \t- Enabled ADI for swappable pages\n> \n> v2:\n> \t- Fixed a build error\n> \n> Documentation/sparc/adi.txt | 278 ++++++++++++++++++++++++++++++\n> arch/sparc/include/asm/mman.h | 84 +++++++++-\n> arch/sparc/include/asm/mmu_64.h | 17 ++\n> arch/sparc/include/asm/mmu_context_64.h | 50 ++++++\n> arch/sparc/include/asm/page_64.h | 4 +\n> arch/sparc/include/asm/pgtable_64.h | 46 +++++\n> arch/sparc/include/asm/thread_info_64.h | 2 +-\n> arch/sparc/include/asm/trap_block.h | 2 +\n> arch/sparc/include/uapi/asm/mman.h | 2 +\n> arch/sparc/kernel/adi_64.c | 289 ++++++++++++++++++++++++++++++++\n> arch/sparc/kernel/etrap_64.S | 28 +++-\n> arch/sparc/kernel/process_64.c | 25 +++\n> arch/sparc/kernel/setup_64.c | 2 +\n> arch/sparc/kernel/vmlinux.lds.S | 5 +\n> arch/sparc/mm/gup.c | 37 ++++\n> arch/sparc/mm/hugetlbpage.c | 14 +-\n> arch/sparc/mm/init_64.c | 34 ++++\n> arch/sparc/mm/tsb.c | 21 +++\n> include/linux/mm.h | 3 +\n> mm/ksm.c | 4 +\n> 20 files changed, 943 insertions(+), 4 deletions(-)\n> create mode 100644 Documentation/sparc/adi.txt\n> \n> diff --git a/Documentation/sparc/adi.txt b/Documentation/sparc/adi.txt\n> new file mode 100644\n> index 000000000000..e1aed155fb89\n> --- /dev/null\n> +++ b/Documentation/sparc/adi.txt\n> @@ -0,0 +1,278 @@\n> +Application Data Integrity (ADI)\n> +================================\n> +\n> +SPARC M7 processor adds the Application Data Integrity (ADI) feature.\n> +ADI allows a task to set version tags on any subset of its address\n> +space. Once ADI is enabled and version tags are set for ranges of\n> +address space of a task, the processor will compare the tag in pointers\n> +to memory in these ranges to the version set by the application\n> +previously. Access to memory is granted only if the tag in given pointer\n> +matches the tag set by the application. In case of mismatch, processor\n> +raises an exception.\n> +\n> +Following steps must be taken by a task to enable ADI fully:\n> +\n> +1. Set the user mode PSTATE.mcde bit. This acts as master switch for\n> + the task's entire address space to enable/disable ADI for the task.\n> +\n> +2. Set TTE.mcd bit on any TLB entries that correspond to the range of\n> + addresses ADI is being enabled on. MMU checks the version tag only\n> + on the pages that have TTE.mcd bit set.\n> +\n> +3. Set the version tag for virtual addresses using stxa instruction\n> + and one of the MCD specific ASIs. Each stxa instruction sets the\n> + given tag for one ADI block size number of bytes. This step must\n> + be repeated for entire page to set tags for entire page.\n> +\n> +ADI block size for the platform is provided by the hypervisor to kernel\n> +in machine description tables. Hypervisor also provides the number of\n> +top bits in the virtual address that specify the version tag. Once\n> +version tag has been set for a memory location, the tag is stored in the\n> +physical memory and the same tag must be present in the ADI version tag\n> +bits of the virtual address being presented to the MMU. For example on\n> +SPARC M7 processor, MMU uses bits 63-60 for version tags and ADI block\n> +size is same as cacheline size which is 64 bytes. A task that sets ADI\n> +version to, say 10, on a range of memory, must access that memory using\n> +virtual addresses that contain 0xa in bits 63-60.\n> +\n> +ADI is enabled on a set of pages using mprotect() with PROT_ADI flag.\n> +When ADI is enabled on a set of pages by a task for the first time,\n> +kernel sets the PSTATE.mcde bit fot the task. Version tags for memory\n> +addresses are set with an stxa instruction on the addresses using\n> +ASI_MCD_PRIMARY or ASI_MCD_ST_BLKINIT_PRIMARY. ADI block size is\n> +provided by the hypervisor to the kernel. Kernel returns the value of\n> +ADI block size to userspace using auxiliary vector along with other ADI\n> +info. Following auxiliary vectors are provided by the kernel:\n> +\n> +\tAT_ADI_BLKSZ\tADI block size. This is the granularity and\n> +\t\t\talignment, in bytes, of ADI versioning.\n> +\tAT_ADI_NBITS\tNumber of ADI version bits in the VA\n> +\n> +\n> +IMPORTANT NOTES:\n> +\n> +- Version tag values of 0x0 and 0xf are reserved. These values match any\n> + tag in virtual address and never generate a mismatch exception.\n> +\n> +- Version tags are set on virtual addresses from userspace even though\n> + tags are stored in physical memory. Tags are set on a physical page\n> + after it has been allocated to a task and a pte has been created for\n> + it.\n> +\n> +- When a task frees a memory page it had set version tags on, the page\n> + goes back to free page pool. When this page is re-allocated to a task,\n> + kernel clears the page using block initialization ASI which clears the\n> + version tags as well for the page. If a page allocated to a task is\n> + freed and allocated back to the same task, old version tags set by the\n> + task on that page will no longer be present.\n> +\n> +- ADI tag mismatches are not detected for non-faulting loads.\n> +\n> +- Kernel does not set any tags for user pages and it is entirely a\n> + task's responsibility to set any version tags. Kernel does ensure the\n> + version tags are preserved if a page is swapped out to the disk and\n> + swapped back in. It also preserves that version tags if a page is\n> + migrated.\n> +\n> +- ADI works for any size pages. A userspace task need not be aware of\n> + page size when using ADI. It can simply select a virtual address\n> + range, enable ADI on the range using mprotect() and set version tags\n> + for the entire range. mprotect() ensures range is aligned to page size\n> + and is a multiple of page size.\n> +\n> +- ADI tags can only be set on writable memory. For example, ADI tags can\n> + not be set on read-only mappings.\n> +\n> +\n> +\n> +ADI related traps\n> +-----------------\n> +\n> +With ADI enabled, following new traps may occur:\n> +\n> +Disrupting memory corruption\n> +\n> +\tWhen a store accesses a memory localtion that has TTE.mcd=1,\n> +\tthe task is running with ADI enabled (PSTATE.mcde=1), and the ADI\n> +\ttag in the address used (bits 63:60) does not match the tag set on\n> +\tthe corresponding cacheline, a memory corruption trap occurs. By\n> +\tdefault, it is a disrupting trap and is sent to the hypervisor\n> +\tfirst. Hypervisor creates a sun4v error report and sends a\n> +\tresumable error (TT=0x7e) trap to the kernel. The kernel sends\n> +\ta SIGSEGV to the task that resulted in this trap with the following\n> +\tinfo:\n> +\n> +\t\tsiginfo.si_signo = SIGSEGV;\n> +\t\tsiginfo.errno = 0;\n> +\t\tsiginfo.si_code = SEGV_ADIDERR;\n> +\t\tsiginfo.si_addr = addr; /* PC where first mismatch occurred */\n> +\t\tsiginfo.si_trapno = 0;\n> +\n> +\n> +Precise memory corruption\n> +\n> +\tWhen a store accesses a memory location that has TTE.mcd=1,\n> +\tthe task is running with ADI enabled (PSTATE.mcde=1), and the ADI\n> +\ttag in the address used (bits 63:60) does not match the tag set on\n> +\tthe corresponding cacheline, a memory corruption trap occurs. If\n> +\tMCD precise exception is enabled (MCDPERR=1), a precise\n> +\texception is sent to the kernel with TT=0x1a. The kernel sends\n> +\ta SIGSEGV to the task that resulted in this trap with the following\n> +\tinfo:\n> +\n> +\t\tsiginfo.si_signo = SIGSEGV;\n> +\t\tsiginfo.errno = 0;\n> +\t\tsiginfo.si_code = SEGV_ADIPERR;\n> +\t\tsiginfo.si_addr = addr;\t/* address that caused trap */\n> +\t\tsiginfo.si_trapno = 0;\n> +\n> +\tNOTE: ADI tag mismatch on a load always results in precise trap.\n> +\n> +\n> +MCD disabled\n> +\n> +\tWhen a task has not enabled ADI and attempts to set ADI version\n> +\ton a memory address, processor sends an MCD disabled trap. This\n> +\ttrap is handled by hypervisor first and the hypervisor vectors this\n> +\ttrap through to the kernel as Data Access Exception trap with\n> +\tfault type set to 0xa (invalid ASI). When this occurs, the kernel\n> +\tsends the task SIGSEGV signal with following info:\n> +\n> +\t\tsiginfo.si_signo = SIGSEGV;\n> +\t\tsiginfo.errno = 0;\n> +\t\tsiginfo.si_code = SEGV_ACCADI;\n> +\t\tsiginfo.si_addr = addr;\t/* address that caused trap */\n> +\t\tsiginfo.si_trapno = 0;\n> +\n> +\n> +Sample program to use ADI\n> +-------------------------\n> +\n> +Following sample program is meant to illustrate how to use the ADI\n> +functionality.\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#ifndef AT_ADI_BLKSZ\n> +#define AT_ADI_BLKSZ\t48\n> +#endif\n> +#ifndef AT_ADI_NBITS\n> +#define AT_ADI_NBITS\t49\n> +#endif\n> +\n> +#ifndef PROT_ADI\n> +#define PROT_ADI\t0x10\n> +#endif\n> +\n> +#define BUFFER_SIZE 32*1024*1024UL\n> +\n> +main(int argc, char* argv[], char* envp[])\n> +{\n> + unsigned long i, mcde, adi_blksz, adi_nbits;\n> + char *shmaddr, *tmp_addr, *end, *veraddr, *clraddr;\n> + int shmid, version;\n> +\tElf64_auxv_t *auxv;\n> +\n> +\tadi_blksz = 0;\n> +\n> +\twhile(*envp++ != NULL);\n> +\tfor (auxv = (Elf64_auxv_t *)envp; auxv->a_type != AT_NULL; auxv++) {\n> +\t\tswitch (auxv->a_type) {\n> +\t\tcase AT_ADI_BLKSZ:\n> +\t\t\tadi_blksz = auxv->a_un.a_val;\n> +\t\t\tbreak;\n> +\t\tcase AT_ADI_NBITS:\n> +\t\t\tadi_nbits = auxv->a_un.a_val;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\tif (adi_blksz == 0) {\n> +\t\tfprintf(stderr, \"Oops! ADI is not supported\\n\");\n> +\t\texit(1);\n> +\t}\n> +\n> +\tprintf(\"ADI capabilities:\\n\");\n> +\tprintf(\"\\tBlock size = %ld\\n\", adi_blksz);\n> +\tprintf(\"\\tNumber of bits = %ld\\n\", adi_nbits);\n> +\n> + if ((shmid = shmget(2, BUFFER_SIZE,\n> + IPC_CREAT | SHM_R | SHM_W)) < 0) {\n> + perror(\"shmget failed\");\n> + exit(1);\n> + }\n> +\n> + shmaddr = shmat(shmid, NULL, 0);\n> + if (shmaddr == (char *)-1) {\n> + perror(\"shm attach failed\");\n> + shmctl(shmid, IPC_RMID, NULL);\n> + exit(1);\n> + }\n> +\n> +\tif (mprotect(shmaddr, BUFFER_SIZE, PROT_READ|PROT_WRITE|PROT_ADI)) {\n> +\t\tperror(\"mprotect failed\");\n> +\t\tgoto err_out;\n> +\t}\n> +\n> + /* Set the ADI version tag on the shm segment\n> + */\n> + version = 10;\n> + tmp_addr = shmaddr;\n> + end = shmaddr + BUFFER_SIZE;\n> + while (tmp_addr < end) {\n> + asm volatile(\n> + \"stxa %1, [%0]0x90\\n\\t\"\n> + :\n> + : \"r\" (tmp_addr), \"r\" (version));\n> + tmp_addr += adi_blksz;\n> + }\n> +\tasm volatile(\"membar #Sync\\n\\t\");\n> +\n> + /* Create a versioned address from the normal address by placing\n> +\t * version tag in the upper adi_nbits bits\n> + */\n> + tmp_addr = (void *) ((unsigned long)shmaddr << adi_nbits);\n> + tmp_addr = (void *) ((unsigned long)tmp_addr >> adi_nbits);\n> + veraddr = (void *) (((unsigned long)version << (64-adi_nbits))\n> + | (unsigned long)tmp_addr);\n> +\n> + printf(\"Starting the writes:\\n\");\n> + for (i = 0; i < BUFFER_SIZE; i++) {\n> + veraddr[i] = (char)(i);\n> + if (!(i % (1024 * 1024)))\n> + printf(\".\");\n> + }\n> + printf(\"\\n\");\n> +\n> + printf(\"Verifying data...\");\n> +\tfflush(stdout);\n> + for (i = 0; i < BUFFER_SIZE; i++)\n> + if (veraddr[i] != (char)i)\n> + printf(\"\\nIndex %lu mismatched\\n\", i);\n> + printf(\"Done.\\n\");\n> +\n> + /* Disable ADI and clean up\n> + */\n> +\tif (mprotect(shmaddr, BUFFER_SIZE, PROT_READ|PROT_WRITE)) {\n> +\t\tperror(\"mprotect failed\");\n> +\t\tgoto err_out;\n> +\t}\n> +\n> + if (shmdt((const void *)shmaddr) != 0)\n> + perror(\"Detach failure\");\n> + shmctl(shmid, IPC_RMID, NULL);\n> +\n> + exit(0);\n> +\n> +err_out:\n> + if (shmdt((const void *)shmaddr) != 0)\n> + perror(\"Detach failure\");\n> + shmctl(shmid, IPC_RMID, NULL);\n> + exit(1);\n> +}\n> diff --git a/arch/sparc/include/asm/mman.h b/arch/sparc/include/asm/mman.h\n> index 59bb5938d852..65b752c31c2f 100644\n> --- a/arch/sparc/include/asm/mman.h\n> +++ b/arch/sparc/include/asm/mman.h\n> @@ -6,5 +6,87 @@\n> #ifndef __ASSEMBLY__\n> #define arch_mmap_check(addr,len,flags)\tsparc_mmap_check(addr,len)\n> int sparc_mmap_check(unsigned long addr, unsigned long len);\n> -#endif\n> +\n> +#ifdef CONFIG_SPARC64\n> +#include \n> +\n> +static inline void ipi_set_tstate_mcde(void *arg)\n> +{\n> +\tstruct mm_struct *mm = arg;\n> +\n> +\t/* Set TSTATE_MCDE for the task using address map that ADI has been\n> +\t * enabled on if the task is running. If not, it will be set\n> +\t * automatically at the next context switch\n> +\t */\n> +\tif (current->mm == mm) {\n> +\t\tstruct pt_regs *regs;\n> +\n> +\t\tregs = task_pt_regs(current);\n> +\t\tregs->tstate |= TSTATE_MCDE;\n> +\t}\n> +}\n> +\n> +#define arch_calc_vm_prot_bits(prot, pkey) sparc_calc_vm_prot_bits(prot)\n> +static inline unsigned long sparc_calc_vm_prot_bits(unsigned long prot)\n> +{\n> +\tif (adi_capable() && (prot & PROT_ADI)) {\n> +\t\tstruct pt_regs *regs;\n> +\n> +\t\tif (!current->mm->context.adi) {\n> +\t\t\tregs = task_pt_regs(current);\n> +\t\t\tregs->tstate |= TSTATE_MCDE;\n> +\t\t\tcurrent->mm->context.adi = true;\n> +\t\t\ton_each_cpu_mask(mm_cpumask(current->mm),\n> +\t\t\t\t\t ipi_set_tstate_mcde, current->mm, 0);\n> +\t\t}\n> +\t\treturn VM_SPARC_ADI;\n> +\t} else {\n> +\t\treturn 0;\n> +\t}\n> +}\n> +\n> +#define arch_vm_get_page_prot(vm_flags) sparc_vm_get_page_prot(vm_flags)\n> +static inline pgprot_t sparc_vm_get_page_prot(unsigned long vm_flags)\n> +{\n> +\treturn (vm_flags & VM_SPARC_ADI) ? __pgprot(_PAGE_MCD_4V) : __pgprot(0);\n> +}\n> +\n> +#define arch_validate_prot(prot, addr) sparc_validate_prot(prot, addr)\n> +static inline int sparc_validate_prot(unsigned long prot, unsigned long addr)\n> +{\n> +\tif (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_ADI))\n> +\t\treturn 0;\n> +\tif (prot & PROT_ADI) {\n> +\t\tif (!adi_capable())\n> +\t\t\treturn 0;\n> +\n> +\t\tif (addr) {\n> +\t\t\tstruct vm_area_struct *vma;\n> +\n> +\t\t\tvma = find_vma(current->mm, addr);\n> +\t\t\tif (vma) {\n> +\t\t\t\t/* ADI can not be enabled on PFN\n> +\t\t\t\t * mapped pages\n> +\t\t\t\t */\n> +\t\t\t\tif (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))\n> +\t\t\t\t\treturn 0;\n> +\n> +\t\t\t\t/* Mergeable pages can become unmergeable\n> +\t\t\t\t * if ADI is enabled on them even if they\n> +\t\t\t\t * have identical data on them. This can be\n> +\t\t\t\t * because ADI enabled pages with identical\n> +\t\t\t\t * data may still not have identical ADI\n> +\t\t\t\t * tags on them. Disallow ADI on mergeable\n> +\t\t\t\t * pages.\n> +\t\t\t\t */\n> +\t\t\t\tif (vma->vm_flags & VM_MERGEABLE)\n> +\t\t\t\t\treturn 0;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\treturn 1;\n> +}\n> +#endif /* CONFIG_SPARC64 */\n> +\n> +#endif /* __ASSEMBLY__ */\n> #endif /* __SPARC_MMAN_H__ */\n> diff --git a/arch/sparc/include/asm/mmu_64.h b/arch/sparc/include/asm/mmu_64.h\n> index 83b36a5371ff..a65d51ebe00b 100644\n> --- a/arch/sparc/include/asm/mmu_64.h\n> +++ b/arch/sparc/include/asm/mmu_64.h\n> @@ -89,6 +89,20 @@ struct tsb_config {\n> #define MM_NUM_TSBS\t1\n> #endif\n> \n> +/* ADI tags are stored when a page is swapped out and the storage for\n> + * tags is allocated dynamically. There is a tag storage descriptor\n> + * associated with each set of tag storage pages. Tag storage descriptors\n> + * are allocated dynamically. Since kernel will allocate a full page for\n> + * each tag storage descriptor, we can store up to\n> + * PAGE_SIZE/sizeof(tag storage descriptor) descriptors on that page.\n> + */\n> +typedef struct {\n> +\tunsigned long\tstart;\t\t/* Start address for this tag storage */\n> +\tunsigned long\tend;\t\t/* Last address for tag storage */\n> +\tunsigned char\t*tags;\t\t/* Where the tags are */\n> +\tunsigned long\ttag_users;\t/* number of references to descriptor */\n> +} tag_storage_desc_t;\n> +\n> typedef struct {\n> \tspinlock_t\t\tlock;\n> \tunsigned long\t\tsparc64_ctx_val;\n> @@ -96,6 +110,9 @@ typedef struct {\n> \tunsigned long\t\tthp_pte_count;\n> \tstruct tsb_config\ttsb_block[MM_NUM_TSBS];\n> \tstruct hv_tsb_descr\ttsb_descr[MM_NUM_TSBS];\n> +\tbool\t\t\tadi;\n> +\ttag_storage_desc_t\t*tag_store;\n> +\tspinlock_t\t\ttag_lock;\n> } mm_context_t;\n> \n> #endif /* !__ASSEMBLY__ */\n> diff --git a/arch/sparc/include/asm/mmu_context_64.h b/arch/sparc/include/asm/mmu_context_64.h\n> index 87841d687f8d..8689bcfd8595 100644\n> --- a/arch/sparc/include/asm/mmu_context_64.h\n> +++ b/arch/sparc/include/asm/mmu_context_64.h\n> @@ -9,6 +9,7 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> \n> static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)\n> @@ -133,6 +134,55 @@ static inline void switch_mm(struct mm_struct *old_mm, struct mm_struct *mm, str\n> \n> #define deactivate_mm(tsk,mm)\tdo { } while (0)\n> #define activate_mm(active_mm, mm) switch_mm(active_mm, mm, NULL)\n> +\n> +#define __HAVE_ARCH_START_CONTEXT_SWITCH\n> +static inline void arch_start_context_switch(struct task_struct *prev)\n> +{\n> +\t/* Save the current state of MCDPER register for the process\n> +\t * we are switching from\n> +\t */\n> +\tif (adi_capable()) {\n> +\t\tregister unsigned long tmp_mcdper;\n> +\n> +\t\t__asm__ __volatile__(\n> +\t\t\t\".word 0x83438000\\n\\t\"\t/* rd %mcdper, %g1 */\n> +\t\t\t\"mov %%g1, %0\\n\\t\"\n> +\t\t\t: \"=r\" (tmp_mcdper)\n> +\t\t\t:\n> +\t\t\t: \"g1\");\n> +\t\tif (tmp_mcdper)\n> +\t\t\tset_tsk_thread_flag(prev, TIF_MCDPER);\n> +\t\telse\n> +\t\t\tclear_tsk_thread_flag(prev, TIF_MCDPER);\n> +\t}\n> +}\n> +\n> +#define finish_arch_post_lock_switch\tfinish_arch_post_lock_switch\n> +static inline void finish_arch_post_lock_switch(void)\n> +{\n> +\t/* Restore the state of MCDPER register for the new process\n> +\t * just switched to.\n> +\t */\n> +\tif (adi_capable()) {\n> +\t\tregister unsigned long tmp_mcdper;\n> +\n> +\t\ttmp_mcdper = test_thread_flag(TIF_MCDPER);\n> +\t\t__asm__ __volatile__(\n> +\t\t\t\"mov %0, %%g1\\n\\t\"\n> +\t\t\t\".word 0x9d800001\\n\\t\"\t/* wr %g0, %g1, %mcdper\" */\n> +\t\t\t\".word 0xaf902001\\n\\t\"\t/* wrpr %g0, 1, %pmcdper */\n> +\t\t\t:\n> +\t\t\t: \"ir\" (tmp_mcdper)\n> +\t\t\t: \"g1\");\n> +\t\tif (current && current->mm && current->mm->context.adi) {\n> +\t\t\tstruct pt_regs *regs;\n> +\n> +\t\t\tregs = task_pt_regs(current);\n> +\t\t\tregs->tstate |= TSTATE_MCDE;\n\n\nThis works, but it costs additional cycles on every context switch to\nkeep setting TSTATE_MCDE. PSTATE.mcde=1 only affects loads and stores\nto memory mapped with TTE.mcd=1 so there is no impact if it is set and\nno memory is mapped with TTE.mcd=1. That is why I suggested just\nsetting TSTATE_MCDE once when a process thread is initialized.\n\n\n> +\t\t}\n> +\t}\n> +}\n> +\n> #endif /* !(__ASSEMBLY__) */\n> \n> #endif /* !(__SPARC64_MMU_CONTEXT_H) */\n> diff --git a/arch/sparc/include/asm/page_64.h b/arch/sparc/include/asm/page_64.h\n> index 5961b2d8398a..dc582c5611f8 100644\n> --- a/arch/sparc/include/asm/page_64.h\n> +++ b/arch/sparc/include/asm/page_64.h\n> @@ -46,6 +46,10 @@ struct page;\n> void clear_user_page(void *addr, unsigned long vaddr, struct page *page);\n> #define copy_page(X,Y)\tmemcpy((void *)(X), (void *)(Y), PAGE_SIZE)\n> void copy_user_page(void *to, void *from, unsigned long vaddr, struct page *topage);\n> +#define __HAVE_ARCH_COPY_USER_HIGHPAGE\n> +struct vm_area_struct;\n> +void copy_user_highpage(struct page *to, struct page *from,\n> +\t\t\tunsigned long vaddr, struct vm_area_struct *vma);\n> \n> /* Unlike sparc32, sparc64's parameter passing API is more\n> * sane in that structures which as small enough are passed\n> diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h\n> index af045061f41e..51da342c392d 100644\n> --- a/arch/sparc/include/asm/pgtable_64.h\n> +++ b/arch/sparc/include/asm/pgtable_64.h\n> @@ -18,6 +18,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -570,6 +571,18 @@ static inline pte_t pte_mkspecial(pte_t pte)\n> \treturn pte;\n> }\n> \n> +static inline pte_t pte_mkmcd(pte_t pte)\n> +{\n> +\tpte_val(pte) |= _PAGE_MCD_4V;\n> +\treturn pte;\n> +}\n> +\n> +static inline pte_t pte_mknotmcd(pte_t pte)\n> +{\n> +\tpte_val(pte) &= ~_PAGE_MCD_4V;\n> +\treturn pte;\n> +}\n> +\n> static inline unsigned long pte_young(pte_t pte)\n> {\n> \tunsigned long mask;\n> @@ -1001,6 +1014,39 @@ int page_in_phys_avail(unsigned long paddr);\n> int remap_pfn_range(struct vm_area_struct *, unsigned long, unsigned long,\n> \t\t unsigned long, pgprot_t);\n> \n> +void adi_restore_tags(struct mm_struct *mm, struct vm_area_struct *vma,\n> +\t\t unsigned long addr, pte_t pte);\n> +\n> +int adi_save_tags(struct mm_struct *mm, struct vm_area_struct *vma,\n> +\t\t unsigned long addr, pte_t oldpte);\n> +\n> +#define __HAVE_ARCH_DO_SWAP_PAGE\n> +static inline void arch_do_swap_page(struct mm_struct *mm,\n> +\t\t\t\t struct vm_area_struct *vma,\n> +\t\t\t\t unsigned long addr,\n> +\t\t\t\t pte_t pte, pte_t oldpte)\n> +{\n> +\t/* If this is a new page being mapped in, there can be no\n> +\t * ADI tags stored away for this page. Skip looking for\n> +\t * stored tags\n> +\t */\n> +\tif (pte_none(oldpte))\n> +\t\treturn;\n> +\n> +\tif (adi_state.enabled && (pte_val(pte) & _PAGE_MCD_4V))\n> +\t\tadi_restore_tags(mm, vma, addr, pte);\n> +}\n> +\n> +#define __HAVE_ARCH_UNMAP_ONE\n> +static inline int arch_unmap_one(struct mm_struct *mm,\n> +\t\t\t\t struct vm_area_struct *vma,\n> +\t\t\t\t unsigned long addr, pte_t oldpte)\n> +{\n> +\tif (adi_state.enabled && (pte_val(oldpte) & _PAGE_MCD_4V))\n> +\t\treturn adi_save_tags(mm, vma, addr, oldpte);\n> +\treturn 0;\n> +}\n> +\n> static inline int io_remap_pfn_range(struct vm_area_struct *vma,\n> \t\t\t\t unsigned long from, unsigned long pfn,\n> \t\t\t\t unsigned long size, pgprot_t prot)\n> diff --git a/arch/sparc/include/asm/thread_info_64.h b/arch/sparc/include/asm/thread_info_64.h\n> index 38a24f257b85..9c04acb1f9af 100644\n> --- a/arch/sparc/include/asm/thread_info_64.h\n> +++ b/arch/sparc/include/asm/thread_info_64.h\n> @@ -190,7 +190,7 @@ register struct thread_info *current_thread_info_reg asm(\"g6\");\n> * in using in assembly, else we can't use the mask as\n> * an immediate value in instructions such as andcc.\n> */\n> -/* flag bit 12 is available */\n> +#define TIF_MCDPER\t\t12\t/* Precise MCD exception */\n> #define TIF_MEMDIE\t\t13\t/* is terminating due to OOM killer */\n> #define TIF_POLLING_NRFLAG\t14\n> \n> diff --git a/arch/sparc/include/asm/trap_block.h b/arch/sparc/include/asm/trap_block.h\n> index ff05992dae7a..73bb05aecd52 100644\n> --- a/arch/sparc/include/asm/trap_block.h\n> +++ b/arch/sparc/include/asm/trap_block.h\n> @@ -73,6 +73,8 @@ struct sun4v_1insn_patch_entry {\n> };\n> extern struct sun4v_1insn_patch_entry __sun4v_1insn_patch,\n> \t__sun4v_1insn_patch_end;\n> +extern struct sun4v_1insn_patch_entry __sun_m7_1insn_patch,\n> +\t__sun_m7_1insn_patch_end;\n> \n> struct sun4v_2insn_patch_entry {\n> \tunsigned int\taddr;\n> diff --git a/arch/sparc/include/uapi/asm/mman.h b/arch/sparc/include/uapi/asm/mman.h\n> index 9765896ecb2c..a72c03397345 100644\n> --- a/arch/sparc/include/uapi/asm/mman.h\n> +++ b/arch/sparc/include/uapi/asm/mman.h\n> @@ -5,6 +5,8 @@\n> \n> /* SunOS'ified... */\n> \n> +#define PROT_ADI\t0x10\t\t/* ADI enabled */\n> +\n> #define MAP_RENAME MAP_ANONYMOUS /* In SunOS terminology */\n> #define MAP_NORESERVE 0x40 /* don't reserve swap pages */\n> #define MAP_INHERIT 0x80 /* SunOS doesn't do this, but... */\n> diff --git a/arch/sparc/kernel/adi_64.c b/arch/sparc/kernel/adi_64.c\n> index 9abf7ea815fd..9902b560af3e 100644\n> --- a/arch/sparc/kernel/adi_64.c\n> +++ b/arch/sparc/kernel/adi_64.c\n> @@ -7,10 +7,24 @@\n> * This work is licensed under the terms of the GNU GPL, version 2.\n> */\n> #include \n> +#include \n> +#include \n> #include \n> #include \n> +#include \n> +#include \n> +\n> +/* Each page of storage for ADI tags can accommodate tags for 128\n> + * pages. When ADI enabled pages are being swapped out, it would be\n> + * prudent to allocate at least enough tag storage space to accommodate\n> + * SWAPFILE_CLUSTER number of pages. Allocate enough tag storage to\n> + * store tags for four SWAPFILE_CLUSTER pages to reduce need for\n> + * further allocations for same vma.\n> + */\n> +#define TAG_STORAGE_PAGES\t8\n> \n> struct adi_config adi_state;\n> +EXPORT_SYMBOL(adi_state);\n> \n> /* mdesc_adi_init() : Parse machine description provided by the\n> *\thypervisor to detect ADI capabilities\n> @@ -83,6 +97,19 @@ void __init mdesc_adi_init(void)\n> \t\tgoto adi_not_found;\n> \tadi_state.caps.ue_on_adi = *val;\n> \n> +\t/* Some of the code to support swapping ADI tags is written\n> +\t * assumption that two ADI tags can fit inside one byte. If\n> +\t * this assumption is broken by a future architecture change,\n> +\t * that code will have to be revisited. If that were to happen,\n> +\t * disable ADI support so we do not get unpredictable results\n> +\t * with programs trying to use ADI and their pages getting\n> +\t * swapped out\n> +\t */\n> +\tif (adi_state.caps.nbits > 4) {\n> +\t\tpr_warn(\"WARNING: ADI tag size >4 on this platform. Disabling AADI support\\n\");\n> +\t\tadi_state.enabled = false;\n> +\t}\n> +\n> \tmdesc_release(hp);\n> \treturn;\n> \n> @@ -93,3 +120,265 @@ void __init mdesc_adi_init(void)\n> \tif (hp)\n> \t\tmdesc_release(hp);\n> }\n> +\n> +tag_storage_desc_t *find_tag_store(struct mm_struct *mm,\n> +\t\t\t\t struct vm_area_struct *vma,\n> +\t\t\t\t unsigned long addr)\n> +{\n> +\ttag_storage_desc_t *tag_desc = NULL;\n> +\tunsigned long i, max_desc, flags;\n> +\n> +\t/* Check if this vma already has tag storage descriptor\n> +\t * allocated for it.\n> +\t */\n> +\tmax_desc = PAGE_SIZE/sizeof(tag_storage_desc_t);\n> +\tif (mm->context.tag_store) {\n> +\t\ttag_desc = mm->context.tag_store;\n> +\t\tspin_lock_irqsave(&mm->context.tag_lock, flags);\n> +\t\tfor (i = 0; i < max_desc; i++) {\n> +\t\t\tif ((addr >= tag_desc->start) &&\n> +\t\t\t ((addr + PAGE_SIZE - 1) <= tag_desc->end))\n> +\t\t\t\tbreak;\n> +\t\t\ttag_desc++;\n> +\t\t}\n> +\t\tspin_unlock_irqrestore(&mm->context.tag_lock, flags);\n> +\n> +\t\t/* If no matching entries were found, this must be a\n> +\t\t * freshly allocated page\n> +\t\t */\n> +\t\tif (i >= max_desc)\n> +\t\t\ttag_desc = NULL;\n> +\t}\n> +\n> +\treturn tag_desc;\n> +}\n> +\n> +tag_storage_desc_t *alloc_tag_store(struct mm_struct *mm,\n> +\t\t\t\t struct vm_area_struct *vma,\n> +\t\t\t\t unsigned long addr)\n> +{\n> +\tunsigned char *tags;\n> +\tunsigned long i, size, max_desc, flags;\n> +\ttag_storage_desc_t *tag_desc, *open_desc;\n> +\tunsigned long end_addr, hole_start, hole_end;\n> +\n> +\tmax_desc = PAGE_SIZE/sizeof(tag_storage_desc_t);\n> +\topen_desc = NULL;\n> +\thole_start = 0;\n> +\thole_end = ULONG_MAX;\n> +\tend_addr = addr + PAGE_SIZE - 1;\n> +\n> +\t/* Check if this vma already has tag storage descriptor\n> +\t * allocated for it.\n> +\t */\n> +\tspin_lock_irqsave(&mm->context.tag_lock, flags);\n> +\tif (mm->context.tag_store) {\n> +\t\ttag_desc = mm->context.tag_store;\n> +\n> +\t\t/* Look for a matching entry for this address. While doing\n> +\t\t * that, look for the first open slot as well and find\n> +\t\t * the hole in already allocated range where this request\n> +\t\t * will fit in.\n> +\t\t */\n> +\t\tfor (i = 0; i < max_desc; i++) {\n> +\t\t\tif (tag_desc->tag_users == 0) {\n> +\t\t\t\tif (open_desc == NULL)\n> +\t\t\t\t\topen_desc = tag_desc;\n> +\t\t\t} else {\n> +\t\t\t\tif ((addr >= tag_desc->start) &&\n> +\t\t\t\t (tag_desc->end >= (addr + PAGE_SIZE - 1))) {\n> +\t\t\t\t\ttag_desc->tag_users++;\n> +\t\t\t\t\tgoto out;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t\tif ((tag_desc->start > end_addr) &&\n> +\t\t\t (tag_desc->start < hole_end))\n> +\t\t\t\thole_end = tag_desc->start;\n> +\t\t\tif ((tag_desc->end < addr) &&\n> +\t\t\t (tag_desc->end > hole_start))\n> +\t\t\t\thole_start = tag_desc->end;\n> +\t\t\ttag_desc++;\n> +\t\t}\n> +\n> +\t} else {\n> +\t\tsize = sizeof(tag_storage_desc_t)*max_desc;\n> +\t\tmm->context.tag_store = kzalloc(size, GFP_NOWAIT|__GFP_NOWARN);\n> +\t\tif (mm->context.tag_store == NULL) {\n> +\t\t\ttag_desc = NULL;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\t\ttag_desc = mm->context.tag_store;\n> +\t\tfor (i = 0; i < max_desc; i++, tag_desc++)\n> +\t\t\ttag_desc->tag_users = 0;\n> +\t\topen_desc = mm->context.tag_store;\n> +\t\ti = 0;\n> +\t}\n> +\n> +\t/* Check if we ran out of tag storage descriptors */\n> +\tif (open_desc == NULL) {\n> +\t\ttag_desc = NULL;\n> +\t\tgoto out;\n> +\t}\n> +\n> +\t/* Mark this tag descriptor slot in use and then initialize it */\n> +\ttag_desc = open_desc;\n> +\ttag_desc->tag_users = 1;\n> +\n> +\t/* Tag storage has not been allocated for this vma and space\n> +\t * is available in tag storage descriptor. Since this page is\n> +\t * being swapped out, there is high probability subsequent pages\n> +\t * in the VMA will be swapped out as well. Allocate pages to\n> +\t * store tags for as many pages in this vma as possible but not\n> +\t * more than TAG_STORAGE_PAGES. Each byte in tag space holds\n> +\t * two ADI tags since each ADI tag is 4 bits. Each ADI tag\n> +\t * covers adi_blksize() worth of addresses. Check if the hole is\n> +\t * big enough to accommodate full address range for using\n> +\t * TAG_STORAGE_PAGES number of tag pages.\n> +\t */\n> +\tsize = TAG_STORAGE_PAGES * PAGE_SIZE;\n> +\tend_addr = addr + (size*2*adi_blksize()) - 1;\n> +\t/* Check for overflow. If overflow occurs, allocate only one page */\n> +\tif (end_addr < addr) {\n> +\t\tsize = PAGE_SIZE;\n> +\t\tend_addr = addr + (size*2*adi_blksize()) - 1;\n\nend_addr could still overflow even with size = PAGE_SIZE.\nMaybe you could just set end_addr to (unsigned long)-1 and recalculate\nthe size based on that.\n\n\n> +\t}\n> +\tif (hole_end < end_addr) {\n> +\t\t/* Available hole is too small on the upper end of\n> +\t\t * address. Can we expand the range towards the lower\n> +\t\t * address and maximize use of this slot?\n> +\t\t */\n> +\t\tunsigned long tmp_addr;\n> +\n> +\t\tend_addr = hole_end - 1;\n> +\t\ttmp_addr = end_addr - (size*2*adi_blksize()) + 1;\n> +\t\t/* Check for underflow. If underflow occurs, allocate\n> +\t\t * only one page for storing ADI tags\n> +\t\t */\n> +\t\tif (tmp_addr > addr) {\n\nShould compare tmp_addr to end_addr rather than addr.\n\n\n> +\t\t\tsize = PAGE_SIZE;\n> +\t\t\ttmp_addr = addr + (size*2*adi_blksize()) - 1;\n\ncopy/paste error? tmp_addr should be recalculated from end_addr and a\nnew size. The new size needs to be adjusted based on end_addr to as\nlittle as PAGE_SIZE.\n\n\n> +\t\t}\n> +\t\tif (tmp_addr < hole_start) {\n> +\t\t\t/* Available hole is restricted on lower address\n> +\t\t\t * end as well\n> +\t\t\t */\n> +\t\t\ttmp_addr = hole_start + 1;\n> +\t\t}\n> +\t\taddr = tmp_addr;\n> +\t\tsize = (end_addr + 1 - addr)/(2*adi_blksize());\n> +\t\tsize = (size + (PAGE_SIZE-adi_blksize()))/PAGE_SIZE;\n> +\t\tsize = size * PAGE_SIZE;\n> +\t}\n> +\ttags = kzalloc(size, GFP_NOWAIT|__GFP_NOWARN);\n> +\tif (tags == NULL) {\n> +\t\ttag_desc->tag_users = 0;\n> +\t\ttag_desc = NULL;\n> +\t\tgoto out;\n> +\t}\n> +\ttag_desc->start = addr;\n> +\ttag_desc->tags = tags;\n> +\ttag_desc->end = end_addr;\n> +\n> +out:\n> +\tspin_unlock_irqrestore(&mm->context.tag_lock, flags);\n> +\treturn tag_desc;\n> +}\n> +\n> +void del_tag_store(tag_storage_desc_t *tag_desc, struct mm_struct *mm)\n> +{\n> +\tunsigned long flags;\n> +\tunsigned char *tags = NULL;\n> +\n> +\tspin_lock_irqsave(&mm->context.tag_lock, flags);\n> +\ttag_desc->tag_users--;\n> +\tif (tag_desc->tag_users == 0) {\n> +\t\ttag_desc->start = tag_desc->end = 0;\n> +\t\t/* Do not free up the tag storage space allocated\n> +\t\t * by the first descriptor. This is persistent\n> +\t\t * emergency tag storage space for the task.\n> +\t\t */\n> +\t\tif (tag_desc != mm->context.tag_store) {\n> +\t\t\ttags = tag_desc->tags;\n> +\t\t\ttag_desc->tags = NULL;\n> +\t\t}\n> +\t}\n> +\tspin_unlock_irqrestore(&mm->context.tag_lock, flags);\n> +\tkfree(tags);\n> +}\n> +\n> +#define tag_start(addr, tag_desc)\t\t\\\n> +\t((tag_desc)->tags + ((addr - (tag_desc)->start)/(2*adi_blksize())))\n> +\n> +/* Retrieve any saved ADI tags for the page being swapped back in and\n> + * restore these tags to the newly allocated physical page.\n> + */\n> +void adi_restore_tags(struct mm_struct *mm, struct vm_area_struct *vma,\n> +\t\t unsigned long addr, pte_t pte)\n> +{\n> +\tunsigned char *tag;\n> +\ttag_storage_desc_t *tag_desc;\n> +\tunsigned long paddr, tmp, version1, version2;\n> +\n> +\t/* Check if the swapped out page has an ADI version\n> +\t * saved. If yes, restore version tag to the newly\n> +\t * allocated page.\n> +\t */\n> +\ttag_desc = find_tag_store(mm, vma, addr);\n> +\tif (tag_desc == NULL)\n> +\t\treturn;\n> +\n> +\ttag = tag_start(addr, tag_desc);\n> +\tpaddr = pte_val(pte) & _PAGE_PADDR_4V;\n> +\tfor (tmp = paddr; tmp < (paddr+PAGE_SIZE); tmp += adi_blksize()) {\n> +\t\tversion1 = (*tag) >> 4;\n> +\t\tversion2 = (*tag) & 0x0f;\n> +\t\t*tag++ = 0;\n> +\t\tasm volatile(\"stxa %0, [%1] %2\\n\\t\"\n> +\t\t\t:\n> +\t\t\t: \"r\" (version1), \"r\" (tmp),\n> +\t\t\t \"i\" (ASI_MCD_REAL));\n> +\t\ttmp += adi_blksize();\n> +\t\tasm volatile(\"stxa %0, [%1] %2\\n\\t\"\n> +\t\t\t:\n> +\t\t\t: \"r\" (version2), \"r\" (tmp),\n> +\t\t\t \"i\" (ASI_MCD_REAL));\n> +\t}\n> +\tasm volatile(\"membar #Sync\\n\\t\");\n> +\n> +\t/* Check and mark this tag space for release later if\n> +\t * the swapped in page was the last user of tag space\n> +\t */\n> +\tdel_tag_store(tag_desc, mm);\n> +}\n> +\n> +/* A page is about to be swapped out. Save any ADI tags associated with\n> + * this physical page so they can be restored later when the page is swapped\n> + * back in.\n> + */\n> +int adi_save_tags(struct mm_struct *mm, struct vm_area_struct *vma,\n> +\t\t unsigned long addr, pte_t oldpte)\n> +{\n> +\tunsigned char *tag;\n> +\ttag_storage_desc_t *tag_desc;\n> +\tunsigned long version1, version2, paddr, tmp;\n> +\n> +\ttag_desc = alloc_tag_store(mm, vma, addr);\n> +\tif (tag_desc == NULL)\n> +\t\treturn -1;\n> +\n> +\ttag = tag_start(addr, tag_desc);\n> +\tpaddr = pte_val(oldpte) & _PAGE_PADDR_4V;\n> +\tfor (tmp = paddr; tmp < (paddr+PAGE_SIZE); tmp += adi_blksize()) {\n> +\t\tasm volatile(\"ldxa [%1] %2, %0\\n\\t\"\n> +\t\t\t\t: \"=r\" (version1)\n> +\t\t\t\t: \"r\" (tmp), \"i\" (ASI_MCD_REAL));\n> +\t\ttmp += adi_blksize();\n> +\t\tasm volatile(\"ldxa [%1] %2, %0\\n\\t\"\n> +\t\t\t\t: \"=r\" (version2)\n> +\t\t\t\t: \"r\" (tmp), \"i\" (ASI_MCD_REAL));\n> +\t\t*tag = (version1 << 4) | version2;\n> +\t\ttag++;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> diff --git a/arch/sparc/kernel/etrap_64.S b/arch/sparc/kernel/etrap_64.S\n> index 1276ca2567ba..7be33bf45cff 100644\n> --- a/arch/sparc/kernel/etrap_64.S\n> +++ b/arch/sparc/kernel/etrap_64.S\n> @@ -132,7 +132,33 @@ etrap_save:\tsave\t%g2, -STACK_BIAS, %sp\n> \t\tstx\t%g6, [%sp + PTREGS_OFF + PT_V9_G6]\n> \t\tstx\t%g7, [%sp + PTREGS_OFF + PT_V9_G7]\n> \t\tor\t%l7, %l0, %l7\n> -\t\tsethi\t%hi(TSTATE_TSO | TSTATE_PEF), %l0\n> +661:\t\tsethi\t%hi(TSTATE_TSO | TSTATE_PEF), %l0\n> +\t\t/*\n> +\t\t * If userspace is using ADI, it could potentially pass\n> +\t\t * a pointer with version tag embedded in it. To maintain\n> +\t\t * the ADI security, we must enable PSTATE.mcde. Userspace\n> +\t\t * would have already set TTE.mcd in an earlier call to\n> +\t\t * kernel and set the version tag for the address being\n> +\t\t * dereferenced. Setting PSTATE.mcde would ensure any\n> +\t\t * access to userspace data through a system call honors\n> +\t\t * ADI and does not allow a rogue app to bypass ADI by\n> +\t\t * using system calls. Setting PSTATE.mcde only affects\n> +\t\t * accesses to virtual addresses that have TTE.mcd set.\n> +\t\t * Set PMCDPER to ensure any exceptions caused by ADI\n> +\t\t * version tag mismatch are exposed before system call\n> +\t\t * returns to userspace. Setting PMCDPER affects only\n> +\t\t * writes to virtual addresses that have TTE.mcd set and\n> +\t\t * have a version tag set as well.\n> +\t\t */\n> +\t\t.section .sun_m7_1insn_patch, \"ax\"\n> +\t\t.word\t661b\n> +\t\tsethi\t%hi(TSTATE_TSO | TSTATE_PEF | TSTATE_MCDE), %l0\n\nrtrap is still missing patches to turn on TSTATE_MCDE when needed.\n\n> +\t\t.previous\n> +661:\t\tnop\n> +\t\t.section .sun_m7_1insn_patch, \"ax\"\n> +\t\t.word\t661b\n> +\t\t.word 0xaf902001\t/* wrpr %g0, 1, %pmcdper */\n\n\nI still disagree with setting %pmcdper=1 on every trap into the kernel,\nand now %pmcdper is also being set to 1 on every context switch. It\nshould be sufficient to set it once for each CPU but also setting it\non every context switch is at least less impactful than setting it on\nevery etrap.\n\n\n\n> +\t\t.previous\n> \t\tor\t%l7, %l0, %l7\n> \t\twrpr\t%l2, %tnpc\n> \t\twrpr\t%l7, (TSTATE_PRIV | TSTATE_IE), %tstate\n> diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c\n> index b96104da5bd6..630e7a07e94d 100644\n> --- a/arch/sparc/kernel/process_64.c\n> +++ b/arch/sparc/kernel/process_64.c\n> @@ -664,6 +664,31 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,\n> \treturn 0;\n> }\n> \n> +/* TIF_MCDPER in thread info flags for current task is updated lazily upon\n> + * a context switch. Update this flag in current task's thread flags\n> + * before dup so the dup'd task will inherit the current TIF_MCDPER flag.\n> + */\n> +int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)\n> +{\n> +\tif (adi_capable()) {\n> +\t\tregister unsigned long tmp_mcdper;\n> +\n> +\t\t__asm__ __volatile__(\n> +\t\t\t\".word 0x83438000\\n\\t\"\t/* rd %mcdper, %g1 */\n> +\t\t\t\"mov %%g1, %0\\n\\t\"\n> +\t\t\t: \"=r\" (tmp_mcdper)\n> +\t\t\t:\n> +\t\t\t: \"g1\");\n> +\t\tif (tmp_mcdper)\n> +\t\t\tset_thread_flag(TIF_MCDPER);\n> +\t\telse\n> +\t\t\tclear_thread_flag(TIF_MCDPER);\n> +\t}\n> +\n> +\t*dst = *src;\n> +\treturn 0;\n> +}\n> +\n> typedef struct {\n> \tunion {\n> \t\tunsigned int\tpr_regs[32];\n> diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c\n> index 150ee7d4b059..98a877715832 100644\n> --- a/arch/sparc/kernel/setup_64.c\n> +++ b/arch/sparc/kernel/setup_64.c\n> @@ -293,6 +293,8 @@ static void __init sun4v_patch(void)\n> \tcase SUN4V_CHIP_SPARC_M7:\n> \tcase SUN4V_CHIP_SPARC_M8:\n> \tcase SUN4V_CHIP_SPARC_SN:\n> +\t\tsun4v_patch_1insn_range(&__sun_m7_1insn_patch,\n> +\t\t\t\t\t&__sun_m7_1insn_patch_end);\n> \t\tsun_m7_patch_2insn_range(&__sun_m7_2insn_patch,\n> \t\t\t\t\t &__sun_m7_2insn_patch_end);\n\nWhy did you keep sun_m7_patch_2insn_range() and not replace it with\nsun4v_m7_patch_2insn_range()?\n\nAnthony\n\n> \t\tbreak;\n> diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S\n> index 03b3d65d1266..f19a2ab990b3 100644\n> --- a/arch/sparc/kernel/vmlinux.lds.S\n> +++ b/arch/sparc/kernel/vmlinux.lds.S\n> @@ -144,6 +144,11 @@ SECTIONS\n> \t\t*(.pause_3insn_patch)\n> \t\t__pause_3insn_patch_end = .;\n> \t}\n> +\t.sun_m7_1insn_patch : {\n> +\t\t__sun_m7_1insn_patch = .;\n> +\t\t*(.sun_m7_1insn_patch)\n> +\t\t__sun_m7_1insn_patch_end = .;\n> +\t}\n> \t.sun_m7_2insn_patch : {\n> \t\t__sun_m7_2insn_patch = .;\n> \t\t*(.sun_m7_2insn_patch)\n> diff --git a/arch/sparc/mm/gup.c b/arch/sparc/mm/gup.c\n> index f80cfc64c55b..dc7a3d35f2b3 100644\n> --- a/arch/sparc/mm/gup.c\n> +++ b/arch/sparc/mm/gup.c\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> /*\n> * The performance critical leaf functions are made noinline otherwise gcc\n> @@ -157,6 +158,24 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,\n> \tpgd_t *pgdp;\n> \tint nr = 0;\n> \n> +#ifdef CONFIG_SPARC64\n> +\tif (adi_capable()) {\n> +\t\tlong addr = start;\n> +\n> +\t\t/* If userspace has passed a versioned address, kernel\n> +\t\t * will not find it in the VMAs since it does not store\n> +\t\t * the version tags in the list of VMAs. Storing version\n> +\t\t * tags in list of VMAs is impractical since they can be\n> +\t\t * changed any time from userspace without dropping into\n> +\t\t * kernel. Any address search in VMAs will be done with\n> +\t\t * non-versioned addresses. Ensure the ADI version bits\n> +\t\t * are dropped here by sign extending the last bit before\n> +\t\t * ADI bits. IOMMU does not implement version tags.\n> +\t\t */\n> +\t\taddr = (addr << (long)adi_nbits()) >> (long)adi_nbits();\n> +\t\tstart = addr;\n> +\t}\n> +#endif\n> \tstart &= PAGE_MASK;\n> \taddr = start;\n> \tlen = (unsigned long) nr_pages << PAGE_SHIFT;\n> @@ -187,6 +206,24 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,\n> \tpgd_t *pgdp;\n> \tint nr = 0;\n> \n> +#ifdef CONFIG_SPARC64\n> +\tif (adi_capable()) {\n> +\t\tlong addr = start;\n> +\n> +\t\t/* If userspace has passed a versioned address, kernel\n> +\t\t * will not find it in the VMAs since it does not store\n> +\t\t * the version tags in the list of VMAs. Storing version\n> +\t\t * tags in list of VMAs is impractical since they can be\n> +\t\t * changed any time from userspace without dropping into\n> +\t\t * kernel. Any address search in VMAs will be done with\n> +\t\t * non-versioned addresses. Ensure the ADI version bits\n> +\t\t * are dropped here by sign extending the last bit before\n> +\t\t * ADI bits. IOMMU does not implements version tags,\n> +\t\t */\n> +\t\taddr = (addr << (long)adi_nbits()) >> (long)adi_nbits();\n> +\t\tstart = addr;\n> +\t}\n> +#endif\n> \tstart &= PAGE_MASK;\n> \taddr = start;\n> \tlen = (unsigned long) nr_pages << PAGE_SHIFT;\n> diff --git a/arch/sparc/mm/hugetlbpage.c b/arch/sparc/mm/hugetlbpage.c\n> index 28ee8d8ffa07..f847c8027c4e 100644\n> --- a/arch/sparc/mm/hugetlbpage.c\n> +++ b/arch/sparc/mm/hugetlbpage.c\n> @@ -177,8 +177,20 @@ pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,\n> \t\t\t struct page *page, int writeable)\n> {\n> \tunsigned int shift = huge_page_shift(hstate_vma(vma));\n> +\tpte_t pte;\n> \n> -\treturn hugepage_shift_to_tte(entry, shift);\n> +\tpte = hugepage_shift_to_tte(entry, shift);\n> +\n> +#ifdef CONFIG_SPARC64\n> +\t/* If this vma has ADI enabled on it, turn on TTE.mcd\n> +\t */\n> +\tif (vma->vm_flags & VM_SPARC_ADI)\n> +\t\treturn pte_mkmcd(pte);\n> +\telse\n> +\t\treturn pte_mknotmcd(pte);\n> +#else\n> +\treturn pte;\n> +#endif\n> }\n> \n> static unsigned int sun4v_huge_tte_to_shift(pte_t entry)\n> diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c\n> index afa0099f3748..cff63d35ba4f 100644\n> --- a/arch/sparc/mm/init_64.c\n> +++ b/arch/sparc/mm/init_64.c\n> @@ -3122,3 +3122,37 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end)\n> \t\tdo_flush_tlb_kernel_range(start, end);\n> \t}\n> }\n> +\n> +void copy_user_highpage(struct page *to, struct page *from,\n> +\tunsigned long vaddr, struct vm_area_struct *vma)\n> +{\n> +\tchar *vfrom, *vto;\n> +\n> +\tvfrom = kmap_atomic(from);\n> +\tvto = kmap_atomic(to);\n> +\tcopy_user_page(vto, vfrom, vaddr, to);\n> +\tkunmap_atomic(vto);\n> +\tkunmap_atomic(vfrom);\n> +\n> +\t/* If this page has ADI enabled, copy over any ADI tags\n> +\t * as well\n> +\t */\n> +\tif (vma->vm_flags & VM_SPARC_ADI) {\n> +\t\tunsigned long pfrom, pto, i, adi_tag;\n> +\n> +\t\tpfrom = page_to_phys(from);\n> +\t\tpto = page_to_phys(to);\n> +\n> +\t\tfor (i = pfrom; i < (pfrom + PAGE_SIZE); i += adi_blksize()) {\n> +\t\t\tasm volatile(\"ldxa [%1] %2, %0\\n\\t\"\n> +\t\t\t\t\t: \"=r\" (adi_tag)\n> +\t\t\t\t\t: \"r\" (i), \"i\" (ASI_MCD_REAL));\n> +\t\t\tasm volatile(\"stxa %0, [%1] %2\\n\\t\"\n> +\t\t\t\t\t:\n> +\t\t\t\t\t: \"r\" (adi_tag), \"r\" (pto),\n> +\t\t\t\t\t \"i\" (ASI_MCD_REAL));\n> +\t\t\tpto += adi_blksize();\n> +\t\t}\n> +\t\tasm volatile(\"membar #Sync\\n\\t\");\n> +\t}\n> +}\n> diff --git a/arch/sparc/mm/tsb.c b/arch/sparc/mm/tsb.c\n> index 0d4b998c7d7b..6518cc42056b 100644\n> --- a/arch/sparc/mm/tsb.c\n> +++ b/arch/sparc/mm/tsb.c\n> @@ -545,6 +545,9 @@ int init_new_context(struct task_struct *tsk, struct mm_struct *mm)\n> \n> \tmm->context.sparc64_ctx_val = 0UL;\n> \n> +\tmm->context.tag_store = NULL;\n> +\tspin_lock_init(&mm->context.tag_lock);\n> +\n> #if defined(CONFIG_HUGETLB_PAGE) || defined(CONFIG_TRANSPARENT_HUGEPAGE)\n> \t/* We reset them to zero because the fork() page copying\n> \t * will re-increment the counters as the parent PTEs are\n> @@ -610,4 +613,22 @@ void destroy_context(struct mm_struct *mm)\n> \t}\n> \n> \tspin_unlock_irqrestore(&ctx_alloc_lock, flags);\n> +\n> +\t/* If ADI tag storage was allocated for this task, free it */\n> +\tif (mm->context.tag_store) {\n> +\t\ttag_storage_desc_t *tag_desc;\n> +\t\tunsigned long max_desc;\n> +\t\tunsigned char *tags;\n> +\n> +\t\ttag_desc = mm->context.tag_store;\n> +\t\tmax_desc = PAGE_SIZE/sizeof(tag_storage_desc_t);\n> +\t\tfor (i = 0; i < max_desc; i++) {\n> +\t\t\ttags = tag_desc->tags;\n> +\t\t\ttag_desc->tags = NULL;\n> +\t\t\tkfree(tags);\n> +\t\t\ttag_desc++;\n> +\t\t}\n> +\t\tkfree(mm->context.tag_store);\n> +\t\tmm->context.tag_store = NULL;\n> +\t}\n> }\n> diff --git a/include/linux/mm.h b/include/linux/mm.h\n> index cbb21facce6b..115f68126e57 100644\n> --- a/include/linux/mm.h\n> +++ b/include/linux/mm.h\n> @@ -231,6 +231,9 @@ extern unsigned int kobjsize(const void *objp);\n> # define VM_GROWSUP\tVM_ARCH_1\n> #elif defined(CONFIG_IA64)\n> # define VM_GROWSUP\tVM_ARCH_1\n> +#elif defined(CONFIG_SPARC64)\n> +# define VM_SPARC_ADI\tVM_ARCH_1\t/* Uses ADI tag for access control */\n> +# define VM_ARCH_CLEAR\tVM_SPARC_ADI\n> #elif !defined(CONFIG_MMU)\n> # define VM_MAPPED_COPY\tVM_ARCH_1\t/* T if mapped copy of data (nommu mmap) */\n> #endif\n> diff --git a/mm/ksm.c b/mm/ksm.c\n> index db20f8436bc3..83994ae85eb1 100644\n> --- a/mm/ksm.c\n> +++ b/mm/ksm.c\n> @@ -2362,6 +2362,10 @@ int ksm_madvise(struct vm_area_struct *vma, unsigned long start,\n> \t\tif (*vm_flags & VM_SAO)\n> \t\t\treturn 0;\n> #endif\n> +#ifdef VM_SPARC_ADI\n> +\t\tif (*vm_flags & VM_SPARC_ADI)\n> +\t\t\treturn 0;\n> +#endif\n> \n> \t\tif (!test_bit(MMF_VM_MERGEABLE, &mm->flags)) {\n> \t\t\terr = __ksm_enter(mm);\n> -- \n> 2.11.0\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y83pt3Ksdz9t30\n\tfor ;\n\tSat, 7 Oct 2017 09:15:02 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752880AbdJFWOs convert rfc822-to-8bit (ORCPT\n\t);\n\tFri, 6 Oct 2017 18:14:48 -0400","from aserp1040.oracle.com ([141.146.126.69]:32153 \"EHLO\n\taserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752191AbdJFWOp (ORCPT\n\t); Fri, 6 Oct 2017 18:14:45 -0400","from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74])\n\tby aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with\n\tESMTP id v96MCJv8027535\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Fri, 6 Oct 2017 22:12:20 GMT","from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75])\n\tby userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id v96MCJvV020664\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Fri, 6 Oct 2017 22:12:19 GMT","from abhmp0003.oracle.com (abhmp0003.oracle.com [141.146.116.9])\n\tby userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id v96MCFIV031920; \n\tFri, 6 Oct 2017 22:12:15 GMT","from dhcp-whq-twvpn-1-vpnpool-10-159-155-4.vpn.oracle.com\n\t(/10.159.155.4) by default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Fri, 06 Oct 2017 15:12:14 -0700"],"Subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","Mime-Version":"1.0 (Mac OS X Mail 9.3 \\(3124\\))","Content-Type":"text/plain; charset=us-ascii","From":"Anthony Yznaga ","In-Reply-To":"<9e3a8c90ade57d94d1ab2100c6d9508fc2d0a212.1506089472.git.khalid.aziz@oracle.com>","Date":"Fri, 6 Oct 2017 15:12:09 -0700","Cc":"David Miller , dave.hansen@linux.intel.com,\n\tcorbet@lwn.net, Bob Picco ,\n\tsteven.sistare@oracle.com, pasha.tatashin@oracle.com,\n\tmike.kravetz@oracle.com, rob.gardner@oracle.com, mingo@kernel.org,\n\tnitin.m.gupta@oracle.com, kirill.shutemov@linux.intel.com,\n\ttom.hromatka@oracle.com, eric.saint.etienne@oracle.com,\n\tallen.pais@oracle.com, cmetcalf@mellanox.com,\n\takpm@linux-foundation.org, geert@linux-m68k.org, pmladek@suse.com,\n\ttklauser@distanz.ch, atish.patra@oracle.com,\n\tshannon.nelson@oracle.com, vijay.ac.kumar@oracle.com,\n\tpeterz@infradead.org, mhocko@suse.com, jack@suse.cz,\n\tlstoakes@gmail.com, punit.agrawal@arm.com, hughd@google.com,\n\tthomas.tai@oracle.com, paul.gortmaker@windriver.com,\n\tross.zwisler@linux.intel.com, dave.jiang@intel.com,\n\twilly@infradead.org, ying.huang@intel.com, zhongjiang@huawei.com,\n\tminchan@kernel.org, imbrenda@linux.vnet.ibm.com,\n\taneesh.kumar@linux.vnet.ibm.com, aarcange@redhat.com,\n\tlinux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,\n\tsparclinux@vger.kernel.org, linux-mm@kvack.org,\n\tKhalid Aziz ","Content-Transfer-Encoding":"8BIT","Message-Id":"","References":"\n\t\n\t<9e3a8c90ade57d94d1ab2100c6d9508fc2d0a212.1506089472.git.khalid.aziz@oracle.com>","To":"Khalid Aziz ","X-Mailer":"Apple Mail (2.3124)","X-Source-IP":"userv0022.oracle.com [156.151.31.74]","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1785535,"web_url":"http://patchwork.ozlabs.org/comment/1785535/","msgid":"<5edaf7dc-6bc7-c365-0b54-b78975c08894@oracle.com>","list_archive_url":null,"date":"2017-10-12T14:44:12","subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","submitter":{"id":42386,"url":"http://patchwork.ozlabs.org/api/people/42386/","name":"Khalid Aziz","email":"khalid.aziz@oracle.com"},"content":"Hi Anthony,\n\nPlease quote only the relevant parts of the patch with comments. That \nmakes it much easier to find the comments.\n\nOn 10/06/2017 04:12 PM, Anthony Yznaga wrote:\n> \n>> On Sep 25, 2017, at 9:49 AM, Khalid Aziz wrote:\n>>\n>> This patch extends mprotect to enable ADI (TSTATE.mcde), enable/disable\n>> MCD (Memory Corruption Detection) on selected memory ranges, enable\n>> TTE.mcd in PTEs, return ADI parameters to userspace and save/restore ADI\n>> version tags on page swap out/in or migration. ADI is not enabled by\n> \n> I still don't believe migration is properly supported. Your\n> implementation is relying on a fault happening on a page while its\n> migration is in progress so that do_swap_page() will be called, but\n> I don't see how do_swap_page() will be called if a fault does not\n> happen until after the migration has completed.\n\nUser pages are on LRU list and for the mapped pages on LRU list, \nmigrate_pages() ultimately calls try_to_unmap_one and makes a migration \nswap entry for the page being migrated. This forces a page fault upon \naccess on the destination node and the page is swapped back in from swap \ncache. The fault is forced by the migration swap entry, rather than \nfault being an accidental event. If page fault happens on the \ndestination node while migration is in progress, do_swap_page() waits \nuntil migration is done. Please take a look at the code in \n__unmap_and_move().\n\n\n>> +#define finish_arch_post_lock_switch\tfinish_arch_post_lock_switch\n>> +static inline void finish_arch_post_lock_switch(void)\n>> +{\n>> +\t/* Restore the state of MCDPER register for the new process\n>> +\t * just switched to.\n>> +\t */\n>> +\tif (adi_capable()) {\n>> +\t\tregister unsigned long tmp_mcdper;\n>> +\n>> +\t\ttmp_mcdper = test_thread_flag(TIF_MCDPER);\n>> +\t\t__asm__ __volatile__(\n>> +\t\t\t\"mov %0, %%g1\\n\\t\"\n>> +\t\t\t\".word 0x9d800001\\n\\t\"\t/* wr %g0, %g1, %mcdper\" */\n>> +\t\t\t\".word 0xaf902001\\n\\t\"\t/* wrpr %g0, 1, %pmcdper */\n>> +\t\t\t:\n>> +\t\t\t: \"ir\" (tmp_mcdper)\n>> +\t\t\t: \"g1\");\n>> +\t\tif (current && current->mm && current->mm->context.adi) {\n>> +\t\t\tstruct pt_regs *regs;\n>> +\n>> +\t\t\tregs = task_pt_regs(current);\n>> +\t\t\tregs->tstate |= TSTATE_MCDE;\n> \n> \n> This works, but it costs additional cycles on every context switch to\n> keep setting TSTATE_MCDE. PSTATE.mcde=1 only affects loads and stores\n> to memory mapped with TTE.mcd=1 so there is no impact if it is set and\n> no memory is mapped with TTE.mcd=1. That is why I suggested just\n> setting TSTATE_MCDE once when a process thread is initialized.\n\nThis change was suggested by David Miller. I believe there is merit to \nthat suggestion.\n\n>> +\t/* Tag storage has not been allocated for this vma and space\n>> +\t * is available in tag storage descriptor. Since this page is\n>> +\t * being swapped out, there is high probability subsequent pages\n>> +\t * in the VMA will be swapped out as well. Allocate pages to\n>> +\t * store tags for as many pages in this vma as possible but not\n>> +\t * more than TAG_STORAGE_PAGES. Each byte in tag space holds\n>> +\t * two ADI tags since each ADI tag is 4 bits. Each ADI tag\n>> +\t * covers adi_blksize() worth of addresses. Check if the hole is\n>> +\t * big enough to accommodate full address range for using\n>> +\t * TAG_STORAGE_PAGES number of tag pages.\n>> +\t */\n>> +\tsize = TAG_STORAGE_PAGES * PAGE_SIZE;\n>> +\tend_addr = addr + (size*2*adi_blksize()) - 1;\n>> +\t/* Check for overflow. If overflow occurs, allocate only one page */\n>> +\tif (end_addr < addr) {\n>> +\t\tsize = PAGE_SIZE;\n>> +\t\tend_addr = addr + (size*2*adi_blksize()) - 1;\n> \n> end_addr could still overflow even with size = PAGE_SIZE.\n> Maybe you could just set end_addr to (unsigned long)-1 and recalculate\n> the size based on that.\n\nI agree at theoretical level. The number of VA bits is already limited \nby the max implemented VA bit in hardware plus with ADI in use, top 4 \nbits are not available as well either, so there is lot of unused room at \nthe upper end of VA and end_addr is not going to roll over. \nNevertheless, I can fix this as well for completeness sake.\n\n> \n> \n>> +\t}\n>> +\tif (hole_end < end_addr) {\n>> +\t\t/* Available hole is too small on the upper end of\n>> +\t\t * address. Can we expand the range towards the lower\n>> +\t\t * address and maximize use of this slot?\n>> +\t\t */\n>> +\t\tunsigned long tmp_addr;\n>> +\n>> +\t\tend_addr = hole_end - 1;\n>> +\t\ttmp_addr = end_addr - (size*2*adi_blksize()) + 1;\n>> +\t\t/* Check for underflow. If underflow occurs, allocate\n>> +\t\t * only one page for storing ADI tags\n>> +\t\t */\n>> +\t\tif (tmp_addr > addr) {\n> \n> Should compare tmp_addr to end_addr rather than addr.\n\nNo, this is correct. If tmp_addr wraps around to the upper end, \ntheoretically it can be smaller than end_addr but still be bigger than \naddr since addr < end_addr. The way it is written, this is a safer test.\n\n> \n> \n>> +\t\t\tsize = PAGE_SIZE;\n>> +\t\t\ttmp_addr = addr + (size*2*adi_blksize()) - 1;\n> \n> copy/paste error? tmp_addr should be recalculated from end_addr and a\n> new size. The new size needs to be adjusted based on end_addr to as\n> little as PAGE_SIZE.\n\nGood catch.\n\n>> diff --git a/arch/sparc/kernel/etrap_64.S b/arch/sparc/kernel/etrap_64.S\n>> index 1276ca2567ba..7be33bf45cff 100644\n>> --- a/arch/sparc/kernel/etrap_64.S\n>> +++ b/arch/sparc/kernel/etrap_64.S\n>> @@ -132,7 +132,33 @@ etrap_save:\tsave\t%g2, -STACK_BIAS, %sp\n>> \t\tstx\t%g6, [%sp + PTREGS_OFF + PT_V9_G6]\n>> \t\tstx\t%g7, [%sp + PTREGS_OFF + PT_V9_G7]\n>> \t\tor\t%l7, %l0, %l7\n>> -\t\tsethi\t%hi(TSTATE_TSO | TSTATE_PEF), %l0\n>> +661:\t\tsethi\t%hi(TSTATE_TSO | TSTATE_PEF), %l0\n>> +\t\t/*\n>> +\t\t * If userspace is using ADI, it could potentially pass\n>> +\t\t * a pointer with version tag embedded in it. To maintain\n>> +\t\t * the ADI security, we must enable PSTATE.mcde. Userspace\n>> +\t\t * would have already set TTE.mcd in an earlier call to\n>> +\t\t * kernel and set the version tag for the address being\n>> +\t\t * dereferenced. Setting PSTATE.mcde would ensure any\n>> +\t\t * access to userspace data through a system call honors\n>> +\t\t * ADI and does not allow a rogue app to bypass ADI by\n>> +\t\t * using system calls. Setting PSTATE.mcde only affects\n>> +\t\t * accesses to virtual addresses that have TTE.mcd set.\n>> +\t\t * Set PMCDPER to ensure any exceptions caused by ADI\n>> +\t\t * version tag mismatch are exposed before system call\n>> +\t\t * returns to userspace. Setting PMCDPER affects only\n>> +\t\t * writes to virtual addresses that have TTE.mcd set and\n>> +\t\t * have a version tag set as well.\n>> +\t\t */\n>> +\t\t.section .sun_m7_1insn_patch, \"ax\"\n>> +\t\t.word\t661b\n>> +\t\tsethi\t%hi(TSTATE_TSO | TSTATE_PEF | TSTATE_MCDE), %l0\n> \n> rtrap is still missing patches to turn on TSTATE_MCDE when needed.\n> \n>> +\t\t.previous\n>> +661:\t\tnop\n>> +\t\t.section .sun_m7_1insn_patch, \"ax\"\n>> +\t\t.word\t661b\n>> +\t\t.word 0xaf902001\t/* wrpr %g0, 1, %pmcdper */\n> \n> \n> I still disagree with setting %pmcdper=1 on every trap into the kernel,\n> and now %pmcdper is also being set to 1 on every context switch. It\n> should be sufficient to set it once for each CPU but also setting it\n> on every context switch is at least less impactful than setting it on\n> every etrap.\n> \n\nWe discussed this before and I believe not setting %pmcdper on trap into \nkernel can expose kernel to the possibility of running system calls with \ndeferred MCD exceptions which in turn causes unreliable behavior from \nuserspace point of view when MCD exception happens (userspace might get \nSIGSEGV, or system call terminates with error depending upon when \nexception is delievered). I believe it is important for system calls to \nbehave consistently.\n\n\n>> diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c\n>> index 150ee7d4b059..98a877715832 100644\n>> --- a/arch/sparc/kernel/setup_64.c\n>> +++ b/arch/sparc/kernel/setup_64.c\n>> @@ -293,6 +293,8 @@ static void __init sun4v_patch(void)\n>> \tcase SUN4V_CHIP_SPARC_M7:\n>> \tcase SUN4V_CHIP_SPARC_M8:\n>> \tcase SUN4V_CHIP_SPARC_SN:\n>> +\t\tsun4v_patch_1insn_range(&__sun_m7_1insn_patch,\n>> +\t\t\t\t\t&__sun_m7_1insn_patch_end);\n>> \t\tsun_m7_patch_2insn_range(&__sun_m7_2insn_patch,\n>> \t\t\t\t\t &__sun_m7_2insn_patch_end);\n> \n> Why did you keep sun_m7_patch_2insn_range() and not replace it with\n> sun4v_m7_patch_2insn_range()?\n> \n\nsun_m7_patch_2insn_range() is already in the kernel and not part of this \npatch. It can be replaced but that should be a separate patch in my opinion.\n\nThanks,\nKhalid\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yCYYf58BNz9t2l\n\tfor ;\n\tFri, 13 Oct 2017 01:45:42 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753156AbdJLOpl (ORCPT );\n\tThu, 12 Oct 2017 10:45:41 -0400","from aserp1040.oracle.com ([141.146.126.69]:24314 \"EHLO\n\taserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753148AbdJLOpk (ORCPT\n\t); Thu, 12 Oct 2017 10:45:40 -0400","from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71])\n\tby aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with\n\tESMTP id v9CEiLda032348\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Thu, 12 Oct 2017 14:44:22 GMT","from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235])\n\tby userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id\n\tv9CEiKN6000342\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Thu, 12 Oct 2017 14:44:20 GMT","from abhmp0015.oracle.com (abhmp0015.oracle.com [141.146.116.21])\n\tby aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id\n\tv9CEiI9J008894; Thu, 12 Oct 2017 14:44:19 GMT","from [192.168.1.16] (/24.9.64.241)\n\tby default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Thu, 12 Oct 2017 07:44:16 -0700"],"Subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","To":"Anthony Yznaga ","Cc":"David Miller , dave.hansen@linux.intel.com,\n\tcorbet@lwn.net, Bob Picco ,\n\tsteven.sistare@oracle.com, pasha.tatashin@oracle.com,\n\tmike.kravetz@oracle.com, rob.gardner@oracle.com, mingo@kernel.org,\n\tnitin.m.gupta@oracle.com, kirill.shutemov@linux.intel.com,\n\ttom.hromatka@oracle.com, eric.saint.etienne@oracle.com,\n\tallen.pais@oracle.com, cmetcalf@mellanox.com,\n\takpm@linux-foundation.org, geert@linux-m68k.org, pmladek@suse.com,\n\ttklauser@distanz.ch, atish.patra@oracle.com,\n\tshannon.nelson@oracle.com, vijay.ac.kumar@oracle.com,\n\tpeterz@infradead.org, mhocko@suse.com, jack@suse.cz,\n\tlstoakes@gmail.com, punit.agrawal@arm.com, hughd@google.com,\n\tthomas.tai@oracle.com, paul.gortmaker@windriver.com,\n\tross.zwisler@linux.intel.com, dave.jiang@intel.com,\n\twilly@infradead.org, ying.huang@intel.com, zhongjiang@huawei.com,\n\tminchan@kernel.org, imbrenda@linux.vnet.ibm.com,\n\taneesh.kumar@linux.vnet.ibm.com, aarcange@redhat.com,\n\tlinux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,\n\tsparclinux@vger.kernel.org, linux-mm@kvack.org,\n\tKhalid Aziz ","References":"\n\t\n\t<9e3a8c90ade57d94d1ab2100c6d9508fc2d0a212.1506089472.git.khalid.aziz@oracle.com>\n\t","From":"Khalid Aziz ","Organization":"Oracle Corp","Message-ID":"<5edaf7dc-6bc7-c365-0b54-b78975c08894@oracle.com>","Date":"Thu, 12 Oct 2017 08:44:12 -0600","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Source-IP":"userv0021.oracle.com [156.151.31.71]","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1785804,"web_url":"http://patchwork.ozlabs.org/comment/1785804/","msgid":"<782BD060-74C5-4D9B-B013-731249A72F87@oracle.com>","list_archive_url":null,"date":"2017-10-12T20:27:25","subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","submitter":{"id":71125,"url":"http://patchwork.ozlabs.org/api/people/71125/","name":"Anthony Yznaga","email":"anthony.yznaga@oracle.com"},"content":"> On Oct 12, 2017, at 7:44 AM, Khalid Aziz wrote:\n> \n> Hi Anthony,\n> \n> Please quote only the relevant parts of the patch with comments. That makes it much easier to find the comments.\n\nOkay.\n\n> \n> On 10/06/2017 04:12 PM, Anthony Yznaga wrote:\n>>> On Sep 25, 2017, at 9:49 AM, Khalid Aziz wrote:\n>>> \n>>> This patch extends mprotect to enable ADI (TSTATE.mcde), enable/disable\n>>> MCD (Memory Corruption Detection) on selected memory ranges, enable\n>>> TTE.mcd in PTEs, return ADI parameters to userspace and save/restore ADI\n>>> version tags on page swap out/in or migration. ADI is not enabled by\n>> I still don't believe migration is properly supported. Your\n>> implementation is relying on a fault happening on a page while its\n>> migration is in progress so that do_swap_page() will be called, but\n>> I don't see how do_swap_page() will be called if a fault does not\n>> happen until after the migration has completed.\n> \n> User pages are on LRU list and for the mapped pages on LRU list, migrate_pages() ultimately calls try_to_unmap_one and makes a migration swap entry for the page being migrated. This forces a page fault upon access on the destination node and the page is swapped back in from swap cache. The fault is forced by the migration swap entry, rather than fault being an accidental event. If page fault happens on the destination node while migration is in progress, do_swap_page() waits until migration is done. Please take a look at the code in __unmap_and_move().\n\nI looked at the code again, and I now believe ADI tags are never restored for migrated pages. Here's why:\n\nA successful call to try_to_unmap() by __unmap_and_move() will have unmapped the page, replaced the pte with a migration pte, and saved the ADI tags.\n\nIf an access to the unmapped VA range is attempted while the migration pte is in place, handle_pte_fault() will call do_swap_page() because the page present flag is not set in the pte. do_swap_page() will see that the pte is a migration pte and call migration_entry_wait() where it will block until the migration pte is removed. do_swap_page() will then return so that the fault is retried.\n\nremove_migration_pte() replaces the migration pte with a regular pte. The regular pte will have the page present flag set. Whether due to a retry or not, the next fault on the VA range will therefore not call do_swap_page() and the tags will not be restored.\n\n> \n> \n>>> +#define finish_arch_post_lock_switch\tfinish_arch_post_lock_switch\n>>> +static inline void finish_arch_post_lock_switch(void)\n>>> +{\n>>> +\t/* Restore the state of MCDPER register for the new process\n>>> +\t * just switched to.\n>>> +\t */\n>>> +\tif (adi_capable()) {\n>>> +\t\tregister unsigned long tmp_mcdper;\n>>> +\n>>> +\t\ttmp_mcdper = test_thread_flag(TIF_MCDPER);\n>>> +\t\t__asm__ __volatile__(\n>>> +\t\t\t\"mov %0, %%g1\\n\\t\"\n>>> +\t\t\t\".word 0x9d800001\\n\\t\"\t/* wr %g0, %g1, %mcdper\" */\n>>> +\t\t\t\".word 0xaf902001\\n\\t\"\t/* wrpr %g0, 1, %pmcdper */\n>>> +\t\t\t:\n>>> +\t\t\t: \"ir\" (tmp_mcdper)\n>>> +\t\t\t: \"g1\");\n>>> +\t\tif (current && current->mm && current->mm->context.adi) {\n>>> +\t\t\tstruct pt_regs *regs;\n>>> +\n>>> +\t\t\tregs = task_pt_regs(current);\n>>> +\t\t\tregs->tstate |= TSTATE_MCDE;\n>> This works, but it costs additional cycles on every context switch to\n>> keep setting TSTATE_MCDE. PSTATE.mcde=1 only affects loads and stores\n>> to memory mapped with TTE.mcd=1 so there is no impact if it is set and\n>> no memory is mapped with TTE.mcd=1. That is why I suggested just\n>> setting TSTATE_MCDE once when a process thread is initialized.\n> \n> This change was suggested by David Miller. I believe there is merit to that suggestion.\n\nI'm not saying it's without merit. I just wanted to point out that the solution adds a bit of additional work to every context switch and that it's possible to avoid it. I'm fine if David still prefers his solution.\n\n\n> \n>>> +\t/* Tag storage has not been allocated for this vma and space\n>>> +\t * is available in tag storage descriptor. Since this page is\n>>> +\t * being swapped out, there is high probability subsequent pages\n>>> +\t * in the VMA will be swapped out as well. Allocate pages to\n>>> +\t * store tags for as many pages in this vma as possible but not\n>>> +\t * more than TAG_STORAGE_PAGES. Each byte in tag space holds\n>>> +\t * two ADI tags since each ADI tag is 4 bits. Each ADI tag\n>>> +\t * covers adi_blksize() worth of addresses. Check if the hole is\n>>> +\t * big enough to accommodate full address range for using\n>>> +\t * TAG_STORAGE_PAGES number of tag pages.\n>>> +\t */\n>>> +\tsize = TAG_STORAGE_PAGES * PAGE_SIZE;\n>>> +\tend_addr = addr + (size*2*adi_blksize()) - 1;\n>>> +\t/* Check for overflow. If overflow occurs, allocate only one page */\n>>> +\tif (end_addr < addr) {\n>>> +\t\tsize = PAGE_SIZE;\n>>> +\t\tend_addr = addr + (size*2*adi_blksize()) - 1;\n>> end_addr could still overflow even with size = PAGE_SIZE.\n>> Maybe you could just set end_addr to (unsigned long)-1 and recalculate\n>> the size based on that.\n> \n> I agree at theoretical level. The number of VA bits is already limited by the max implemented VA bit in hardware plus with ADI in use, top 4 bits are not available as well either, so there is lot of unused room at the upper end of VA and end_addr is not going to roll over. Nevertheless, I can fix this as well for completeness sake.\n\n\nThe MMU ignores the ADI tag bits and sign extends from the actual most significant VA bit to get the actual VA so the hardware is capable of mapping a user page at 0xffffffffffffe000. However, Linux imposes an upper limit on the maximum user VA. There are theses comments in arch/sparc/include/asm/processor_64.h:\n\n/*\n * User lives in his very own context, and cannot reference us. Note\n * that TASK_SIZE is a misnomer, it really gives maximum user virtual\n * address that the kernel will allocate out.\n *\n * XXX No longer using virtual page tables, kill this upper limit...\n */\n\nMaybe this limit will remain forever, maybe not.\n\n\n> \n>>> +\t}\n>>> +\tif (hole_end < end_addr) {\n>>> +\t\t/* Available hole is too small on the upper end of\n>>> +\t\t * address. Can we expand the range towards the lower\n>>> +\t\t * address and maximize use of this slot?\n>>> +\t\t */\n>>> +\t\tunsigned long tmp_addr;\n>>> +\n>>> +\t\tend_addr = hole_end - 1;\n>>> +\t\ttmp_addr = end_addr - (size*2*adi_blksize()) + 1;\n>>> +\t\t/* Check for underflow. If underflow occurs, allocate\n>>> +\t\t * only one page for storing ADI tags\n>>> +\t\t */\n>>> +\t\tif (tmp_addr > addr) {\n>> Should compare tmp_addr to end_addr rather than addr.\n> \n> No, this is correct. If tmp_addr wraps around to the upper end, theoretically it can be smaller than end_addr but still be bigger than addr since addr < end_addr. The way it is written, this is a safer test.\n\nIf you subtract a large enough value from end_addr to cause underflow, the result will always be greater than end_addr.\n\n> \n>>> +\t\t\tsize = PAGE_SIZE;\n>>> +\t\t\ttmp_addr = addr + (size*2*adi_blksize()) - 1;\n>> copy/paste error? tmp_addr should be recalculated from end_addr and a\n>> new size. The new size needs to be adjusted based on end_addr to as\n>> little as PAGE_SIZE.\n> \n> Good catch.\n> \n>>> diff --git a/arch/sparc/kernel/etrap_64.S b/arch/sparc/kernel/etrap_64.S\n>>> index 1276ca2567ba..7be33bf45cff 100644\n>>> --- a/arch/sparc/kernel/etrap_64.S\n>>> +++ b/arch/sparc/kernel/etrap_64.S\n>>> @@ -132,7 +132,33 @@ etrap_save:\tsave\t%g2, -STACK_BIAS, %sp\n>>> \t\tstx\t%g6, [%sp + PTREGS_OFF + PT_V9_G6]\n>>> \t\tstx\t%g7, [%sp + PTREGS_OFF + PT_V9_G7]\n>>> \t\tor\t%l7, %l0, %l7\n>>> -\t\tsethi\t%hi(TSTATE_TSO | TSTATE_PEF), %l0\n>>> +661:\t\tsethi\t%hi(TSTATE_TSO | TSTATE_PEF), %l0\n>>> +\t\t/*\n>>> +\t\t * If userspace is using ADI, it could potentially pass\n>>> +\t\t * a pointer with version tag embedded in it. To maintain\n>>> +\t\t * the ADI security, we must enable PSTATE.mcde. Userspace\n>>> +\t\t * would have already set TTE.mcd in an earlier call to\n>>> +\t\t * kernel and set the version tag for the address being\n>>> +\t\t * dereferenced. Setting PSTATE.mcde would ensure any\n>>> +\t\t * access to userspace data through a system call honors\n>>> +\t\t * ADI and does not allow a rogue app to bypass ADI by\n>>> +\t\t * using system calls. Setting PSTATE.mcde only affects\n>>> +\t\t * accesses to virtual addresses that have TTE.mcd set.\n>>> +\t\t * Set PMCDPER to ensure any exceptions caused by ADI\n>>> +\t\t * version tag mismatch are exposed before system call\n>>> +\t\t * returns to userspace. Setting PMCDPER affects only\n>>> +\t\t * writes to virtual addresses that have TTE.mcd set and\n>>> +\t\t * have a version tag set as well.\n>>> +\t\t */\n>>> +\t\t.section .sun_m7_1insn_patch, \"ax\"\n>>> +\t\t.word\t661b\n>>> +\t\tsethi\t%hi(TSTATE_TSO | TSTATE_PEF | TSTATE_MCDE), %l0\n>> rtrap is still missing patches to turn on TSTATE_MCDE when needed.\n>>> +\t\t.previous\n>>> +661:\t\tnop\n>>> +\t\t.section .sun_m7_1insn_patch, \"ax\"\n>>> +\t\t.word\t661b\n>>> +\t\t.word 0xaf902001\t/* wrpr %g0, 1, %pmcdper */\n>> I still disagree with setting %pmcdper=1 on every trap into the kernel,\n>> and now %pmcdper is also being set to 1 on every context switch. It\n>> should be sufficient to set it once for each CPU but also setting it\n>> on every context switch is at least less impactful than setting it on\n>> every etrap.\n> \n> We discussed this before and I believe not setting %pmcdper on trap into kernel can expose kernel to the possibility of running system calls with deferred MCD exceptions which in turn causes unreliable behavior from userspace point of view when MCD exception happens (userspace might get SIGSEGV, or system call terminates with error depending upon when exception is delievered). I believe it is important for system calls to behave consistently.\n\nI'm not saying that %pmcdper should not be set to 1. I'm saying that it's expensive and unnecessary to keep setting it to 1 on every trap into the kernel and now every context switch (~50-70 cycles every time the register is updated). It should be sufficient to set it once when each CPU is configured. If it's really necessary to mitigate the possibility of something in the kernel clearing %pmcdper and reseting it then setting it on every context switch should be sufficient and less onerous than every etrap.\n\nAnthony\n\n> \n> \n>>> diff --git a/arch/sparc/kernel/setup_64.c b/arch/sparc/kernel/setup_64.c\n>>> index 150ee7d4b059..98a877715832 100644\n>>> --- a/arch/sparc/kernel/setup_64.c\n>>> +++ b/arch/sparc/kernel/setup_64.c\n>>> @@ -293,6 +293,8 @@ static void __init sun4v_patch(void)\n>>> \tcase SUN4V_CHIP_SPARC_M7:\n>>> \tcase SUN4V_CHIP_SPARC_M8:\n>>> \tcase SUN4V_CHIP_SPARC_SN:\n>>> +\t\tsun4v_patch_1insn_range(&__sun_m7_1insn_patch,\n>>> +\t\t\t\t\t&__sun_m7_1insn_patch_end);\n>>> \t\tsun_m7_patch_2insn_range(&__sun_m7_2insn_patch,\n>>> \t\t\t\t\t &__sun_m7_2insn_patch_end);\n>> Why did you keep sun_m7_patch_2insn_range() and not replace it with\n>> sun4v_m7_patch_2insn_range()?\n> \n> sun_m7_patch_2insn_range() is already in the kernel and not part of this patch. It can be replaced but that should be a separate patch in my opinion.\n> \n> Thanks,\n> Khalid\n> --\n> To unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yCjBv1tjFz9sBZ\n\tfor ;\n\tFri, 13 Oct 2017 07:29:59 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754188AbdJLU36 convert rfc822-to-8bit (ORCPT\n\t);\n\tThu, 12 Oct 2017 16:29:58 -0400","from userp1040.oracle.com ([156.151.31.81]:34727 \"EHLO\n\tuserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753574AbdJLU34 (ORCPT\n\t); Thu, 12 Oct 2017 16:29:56 -0400","from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233])\n\tby userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2)\n\twith ESMTP id v9CKRZUI017021\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Thu, 12 Oct 2017 20:27:35 GMT","from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236])\n\tby aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id\n\tv9CKRYj1032671\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Thu, 12 Oct 2017 20:27:34 GMT","from abhmp0012.oracle.com (abhmp0012.oracle.com [141.146.116.18])\n\tby aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id\n\tv9CKRUDL024903; Thu, 12 Oct 2017 20:27:30 GMT","from dhcp-whq-twvpn-3-vpnpool-10-159-248-60.vpn.oracle.com\n\t(/10.159.248.60) by default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Thu, 12 Oct 2017 13:27:30 -0700"],"Subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","Mime-Version":"1.0 (Mac OS X Mail 9.3 \\(3124\\))","Content-Type":"text/plain; charset=us-ascii","From":"Anthony Yznaga ","In-Reply-To":"<5edaf7dc-6bc7-c365-0b54-b78975c08894@oracle.com>","Date":"Thu, 12 Oct 2017 13:27:25 -0700","Cc":"David Miller , dave.hansen@linux.intel.com,\n\tcorbet@lwn.net, Bob Picco ,\n\tSTEVEN_SISTARE ,\n\tPasha Tatashin ,\n\tMike Kravetz ,\n\tRob Gardner , mingo@kernel.org,\n\tNitin Gupta , kirill.shutemov@linux.intel.com,\n\tTom Hromatka ,\n\tEric Saint Etienne ,\n\tAllen Pais , cmetcalf@mellanox.com,\n\takpm@linux-foundation.org, geert@linux-m68k.org, pmladek@suse.com,\n\ttklauser@distanz.ch, Atish Patra ,\n\tShannon Nelson ,\n\tVijay Kumar , peterz@infradead.org,\n\tmhocko@suse.com, jack@suse.cz, lstoakes@gmail.com,\n\tpunit.agrawal@arm.com, hughd@google.com, thomas.tai@oracle.com,\n\tpaul.gortmaker@windriver.com, ross.zwisler@linux.intel.com,\n\tdave.jiang@intel.com, willy@infradead.org, ying.huang@intel.com,\n\tzhongjiang@huawei.com, minchan@kernel.org,\n\timbrenda@linux.vnet.ibm.com, aneesh.kumar@linux.vnet.ibm.com,\n\taarcange@redhat.com, linux-doc@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,\n\tlinux-mm@kvack.org, Khalid Aziz ","Content-Transfer-Encoding":"8BIT","Message-Id":"<782BD060-74C5-4D9B-B013-731249A72F87@oracle.com>","References":"\n\t\n\t<9e3a8c90ade57d94d1ab2100c6d9508fc2d0a212.1506089472.git.khalid.aziz@oracle.com>\n\t\n\t<5edaf7dc-6bc7-c365-0b54-b78975c08894@oracle.com>","To":"Khalid Aziz ","X-Mailer":"Apple Mail (2.3124)","X-Source-IP":"aserv0021.oracle.com [141.146.126.233]","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1786321,"web_url":"http://patchwork.ozlabs.org/comment/1786321/","msgid":"<44c3473b-a8fb-197e-7fd3-03613569f339@oracle.com>","list_archive_url":null,"date":"2017-10-13T14:14:57","subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","submitter":{"id":42386,"url":"http://patchwork.ozlabs.org/api/people/42386/","name":"Khalid Aziz","email":"khalid.aziz@oracle.com"},"content":"On 10/12/2017 02:27 PM, Anthony Yznaga wrote:\n> \n>> On Oct 12, 2017, at 7:44 AM, Khalid Aziz wrote:\n>>\n>>\n>> On 10/06/2017 04:12 PM, Anthony Yznaga wrote:\n>>>> On Sep 25, 2017, at 9:49 AM, Khalid Aziz wrote:\n>>>>\n>>>> This patch extends mprotect to enable ADI (TSTATE.mcde), enable/disable\n>>>> MCD (Memory Corruption Detection) on selected memory ranges, enable\n>>>> TTE.mcd in PTEs, return ADI parameters to userspace and save/restore ADI\n>>>> version tags on page swap out/in or migration. ADI is not enabled by\n>>> I still don't believe migration is properly supported. Your\n>>> implementation is relying on a fault happening on a page while its\n>>> migration is in progress so that do_swap_page() will be called, but\n>>> I don't see how do_swap_page() will be called if a fault does not\n>>> happen until after the migration has completed.\n>>\n>> User pages are on LRU list and for the mapped pages on LRU list, migrate_pages() ultimately calls try_to_unmap_one and makes a migration swap entry for the page being migrated. This forces a page fault upon access on the destination node and the page is swapped back in from swap cache. The fault is forced by the migration swap entry, rather than fault being an accidental event. If page fault happens on the destination node while migration is in progress, do_swap_page() waits until migration is done. Please take a look at the code in __unmap_and_move().\n> \n> I looked at the code again, and I now believe ADI tags are never restored for migrated pages. Here's why:\n> \n\nI will take a look at it again. I have run extensive tests migrating \npages of a process across multiple NUMA nodes over and over again and \nADI tags were never lost, so this does work. I won't rule out the \npossibility of having missed a code path where tags are not restored and \nI will look for it.\n\nThanks,\nKhalid\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yD8s91WFZz9sNr\n\tfor ;\n\tSat, 14 Oct 2017 01:16:13 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753478AbdJMOQM (ORCPT );\n\tFri, 13 Oct 2017 10:16:12 -0400","from aserp1040.oracle.com ([141.146.126.69]:23042 \"EHLO\n\taserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753466AbdJMOQL (ORCPT\n\t); Fri, 13 Oct 2017 10:16:11 -0400","from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233])\n\tby aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2)\n\twith ESMTP id v9DEF4oH013967\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Fri, 13 Oct 2017 14:15:05 GMT","from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235])\n\tby aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id\n\tv9DEF4ru024442\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Fri, 13 Oct 2017 14:15:04 GMT","from abhmp0002.oracle.com (abhmp0002.oracle.com [141.146.116.8])\n\tby aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id v9DEF1a7022403; \n\tFri, 13 Oct 2017 14:15:01 GMT","from [192.168.1.16] (/24.9.64.241)\n\tby default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Fri, 13 Oct 2017 07:15:01 -0700"],"Subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","To":"Anthony Yznaga ","Cc":"David Miller , dave.hansen@linux.intel.com,\n\tcorbet@lwn.net, Bob Picco ,\n\tSTEVEN_SISTARE ,\n\tPasha Tatashin ,\n\tMike Kravetz ,\n\tRob Gardner , mingo@kernel.org,\n\tNitin Gupta , kirill.shutemov@linux.intel.com,\n\tTom Hromatka ,\n\tEric Saint Etienne ,\n\tAllen Pais , cmetcalf@mellanox.com,\n\takpm@linux-foundation.org, geert@linux-m68k.org, pmladek@suse.com,\n\ttklauser@distanz.ch, Atish Patra ,\n\tShannon Nelson ,\n\tVijay Kumar , peterz@infradead.org,\n\tmhocko@suse.com, jack@suse.cz, lstoakes@gmail.com,\n\tpunit.agrawal@arm.com, hughd@google.com, thomas.tai@oracle.com,\n\tpaul.gortmaker@windriver.com, ross.zwisler@linux.intel.com,\n\tdave.jiang@intel.com, willy@infradead.org, ying.huang@intel.com,\n\tzhongjiang@huawei.com, minchan@kernel.org,\n\timbrenda@linux.vnet.ibm.com, aneesh.kumar@linux.vnet.ibm.com,\n\taarcange@redhat.com, linux-doc@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,\n\tlinux-mm@kvack.org, Khalid Aziz ","References":"\n\t\n\t<9e3a8c90ade57d94d1ab2100c6d9508fc2d0a212.1506089472.git.khalid.aziz@oracle.com>\n\t\n\t<5edaf7dc-6bc7-c365-0b54-b78975c08894@oracle.com>\n\t<782BD060-74C5-4D9B-B013-731249A72F87@oracle.com>","From":"Khalid Aziz ","Organization":"Oracle Corp","Message-ID":"<44c3473b-a8fb-197e-7fd3-03613569f339@oracle.com>","Date":"Fri, 13 Oct 2017 08:14:57 -0600","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<782BD060-74C5-4D9B-B013-731249A72F87@oracle.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Source-IP":"aserv0021.oracle.com [141.146.126.233]","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1786432,"web_url":"http://patchwork.ozlabs.org/comment/1786432/","msgid":"","list_archive_url":null,"date":"2017-10-13T16:18:34","subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","submitter":{"id":42386,"url":"http://patchwork.ozlabs.org/api/people/42386/","name":"Khalid Aziz","email":"khalid.aziz@oracle.com"},"content":"On 10/13/2017 08:14 AM, Khalid Aziz wrote:\n> On 10/12/2017 02:27 PM, Anthony Yznaga wrote:\n>>\n>>> On Oct 12, 2017, at 7:44 AM, Khalid Aziz wrote:\n>>>\n>>>\n>>> On 10/06/2017 04:12 PM, Anthony Yznaga wrote:\n>>>>> On Sep 25, 2017, at 9:49 AM, Khalid Aziz \n>>>>> wrote:\n>>>>>\n>>>>> This patch extends mprotect to enable ADI (TSTATE.mcde), \n>>>>> enable/disable\n>>>>> MCD (Memory Corruption Detection) on selected memory ranges, enable\n>>>>> TTE.mcd in PTEs, return ADI parameters to userspace and \n>>>>> save/restore ADI\n>>>>> version tags on page swap out/in or migration. ADI is not enabled by\n>>>> I still don't believe migration is properly supported.  Your\n>>>> implementation is relying on a fault happening on a page while its\n>>>> migration is in progress so that do_swap_page() will be called, but\n>>>> I don't see how do_swap_page() will be called if a fault does not\n>>>> happen until after the migration has completed.\n>>>\n>>> User pages are on LRU list and for the mapped pages on LRU list, \n>>> migrate_pages() ultimately calls try_to_unmap_one and makes a \n>>> migration swap entry for the page being migrated. This forces a page \n>>> fault upon access on the destination node and the page is swapped \n>>> back in from swap cache. The fault is forced by the migration swap \n>>> entry, rather than fault being an accidental event. If page fault \n>>> happens on the destination node while migration is in progress, \n>>> do_swap_page() waits until migration is done. Please take a look at \n>>> the code in __unmap_and_move().\n>>\n>> I looked at the code again, and I now believe ADI tags are never \n>> restored for migrated pages.  Here's why:\n>>\n> \n> I will take a look at it again. I have run extensive tests migrating \n> pages of a process across multiple NUMA nodes over and over again and \n> ADI tags were never lost, so this does work. I won't rule out the \n> possibility of having missed a code path where tags are not restored and \n> I will look for it.\n\nAnthony,\n\nI just ran my migration test again which:\n\n- malloc's 16 GB of memory\n- Assigns a rotating ADI tag every 64 bytes to the malloc'd buffer\n- Writes a pattern to the entire buffer\n- Verifies the pattern it wrote using ADI tagged addresses.\n\nWhile this test was running, I had a script migrate test program pages \nacross two NUMA nodes every 30 seconds using migratepages command. I did \nnot see an ADI tag mismatch over multiple runs of this test. This test \nshows migration is working.\n\nCan you give me a test that shows the failure you think we should see \nand I will debug it.\n\nThanks,\nKhalid\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yDCcY59S1z9sxR\n\tfor ;\n\tSat, 14 Oct 2017 03:20:29 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751889AbdJMQUR (ORCPT );\n\tFri, 13 Oct 2017 12:20:17 -0400","from userp1040.oracle.com ([156.151.31.81]:38078 \"EHLO\n\tuserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751835AbdJMQUP (ORCPT\n\t); Fri, 13 Oct 2017 12:20:15 -0400","from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71])\n\tby userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with\n\tESMTP id v9DGIkIP003558\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Fri, 13 Oct 2017 16:18:46 GMT","from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236])\n\tby userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id\n\tv9DGIjhl024321\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Fri, 13 Oct 2017 16:18:45 GMT","from abhmp0006.oracle.com (abhmp0006.oracle.com [141.146.116.12])\n\tby aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id\n\tv9DGIcmK005201; Fri, 13 Oct 2017 16:18:39 GMT","from [192.168.1.16] (/24.9.64.241)\n\tby default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Fri, 13 Oct 2017 09:18:38 -0700"],"Subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","From":"Khalid Aziz ","To":"Anthony Yznaga ","Cc":"David Miller , dave.hansen@linux.intel.com,\n\tcorbet@lwn.net, Bob Picco ,\n\tSTEVEN_SISTARE ,\n\tPasha Tatashin ,\n\tMike Kravetz ,\n\tRob Gardner , mingo@kernel.org,\n\tNitin Gupta , kirill.shutemov@linux.intel.com,\n\tTom Hromatka ,\n\tEric Saint Etienne ,\n\tAllen Pais , cmetcalf@mellanox.com,\n\takpm@linux-foundation.org, geert@linux-m68k.org, pmladek@suse.com,\n\ttklauser@distanz.ch, Atish Patra ,\n\tShannon Nelson ,\n\tVijay Kumar , peterz@infradead.org,\n\tmhocko@suse.com, jack@suse.cz, lstoakes@gmail.com,\n\tpunit.agrawal@arm.com, hughd@google.com, thomas.tai@oracle.com,\n\tpaul.gortmaker@windriver.com, ross.zwisler@linux.intel.com,\n\tdave.jiang@intel.com, willy@infradead.org, ying.huang@intel.com,\n\tzhongjiang@huawei.com, minchan@kernel.org,\n\timbrenda@linux.vnet.ibm.com, aneesh.kumar@linux.vnet.ibm.com,\n\taarcange@redhat.com, linux-doc@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,\n\tlinux-mm@kvack.org, Khalid Aziz ","References":"\n\t\n\t<9e3a8c90ade57d94d1ab2100c6d9508fc2d0a212.1506089472.git.khalid.aziz@oracle.com>\n\t\n\t<5edaf7dc-6bc7-c365-0b54-b78975c08894@oracle.com>\n\t<782BD060-74C5-4D9B-B013-731249A72F87@oracle.com>\n\t<44c3473b-a8fb-197e-7fd3-03613569f339@oracle.com>","Organization":"Oracle Corp","Message-ID":"","Date":"Fri, 13 Oct 2017 10:18:34 -0600","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<44c3473b-a8fb-197e-7fd3-03613569f339@oracle.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-Source-IP":"userv0021.oracle.com [156.151.31.71]","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1786487,"web_url":"http://patchwork.ozlabs.org/comment/1786487/","msgid":"<520A262E-1F2A-4717-8766-CD434BBF716C@oracle.com>","list_archive_url":null,"date":"2017-10-13T17:19:16","subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","submitter":{"id":71125,"url":"http://patchwork.ozlabs.org/api/people/71125/","name":"Anthony Yznaga","email":"anthony.yznaga@oracle.com"},"content":"> On Oct 13, 2017, at 9:18 AM, Khalid Aziz wrote:\n> \n> On 10/13/2017 08:14 AM, Khalid Aziz wrote:\n>> On 10/12/2017 02:27 PM, Anthony Yznaga wrote:\n>>> \n>>>> On Oct 12, 2017, at 7:44 AM, Khalid Aziz wrote:\n>>>> \n>>>> \n>>>> On 10/06/2017 04:12 PM, Anthony Yznaga wrote:\n>>>>>> On Sep 25, 2017, at 9:49 AM, Khalid Aziz wrote:\n>>>>>> \n>>>>>> This patch extends mprotect to enable ADI (TSTATE.mcde), enable/disable\n>>>>>> MCD (Memory Corruption Detection) on selected memory ranges, enable\n>>>>>> TTE.mcd in PTEs, return ADI parameters to userspace and save/restore ADI\n>>>>>> version tags on page swap out/in or migration. ADI is not enabled by\n>>>>> I still don't believe migration is properly supported. Your\n>>>>> implementation is relying on a fault happening on a page while its\n>>>>> migration is in progress so that do_swap_page() will be called, but\n>>>>> I don't see how do_swap_page() will be called if a fault does not\n>>>>> happen until after the migration has completed.\n>>>> \n>>>> User pages are on LRU list and for the mapped pages on LRU list, migrate_pages() ultimately calls try_to_unmap_one and makes a migration swap entry for the page being migrated. This forces a page fault upon access on the destination node and the page is swapped back in from swap cache. The fault is forced by the migration swap entry, rather than fault being an accidental event. If page fault happens on the destination node while migration is in progress, do_swap_page() waits until migration is done. Please take a look at the code in __unmap_and_move().\n>>> \n>>> I looked at the code again, and I now believe ADI tags are never restored for migrated pages. Here's why:\n>>> \n>> I will take a look at it again. I have run extensive tests migrating pages of a process across multiple NUMA nodes over and over again and ADI tags were never lost, so this does work. I won't rule out the possibility of having missed a code path where tags are not restored and I will look for it.\n> \n> Anthony,\n> \n> I just ran my migration test again which:\n> \n> - malloc's 16 GB of memory\n> - Assigns a rotating ADI tag every 64 bytes to the malloc'd buffer\n> - Writes a pattern to the entire buffer\n> - Verifies the pattern it wrote using ADI tagged addresses.\n\nThe verification will appear to succeed if the tags have been cleared.\n\nTo be complete the test should also manually verify that the in-memory tag values remain non-zero after migration. migrate_page_copy() will call copy_huge_page() or copy_highpage() which will result in the tags being cleared at the destination because the stores will be done to kernel physical mapping VAs using block initializing stores.\n\nAnthony\n\n> \n> While this test was running, I had a script migrate test program pages across two NUMA nodes every 30 seconds using migratepages command. I did not see an ADI tag mismatch over multiple runs of this test. This test shows migration is working.\n> \n> Can you give me a test that shows the failure you think we should see and I will debug it.\n> \n> Thanks,\n> Khalid\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yDDz605Xqz9t42\n\tfor ;\n\tSat, 14 Oct 2017 04:21:38 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752386AbdJMRVQ convert rfc822-to-8bit (ORCPT\n\t);\n\tFri, 13 Oct 2017 13:21:16 -0400","from userp1040.oracle.com ([156.151.31.81]:33115 \"EHLO\n\tuserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751632AbdJMRVO (ORCPT\n\t); Fri, 13 Oct 2017 13:21:14 -0400","from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74])\n\tby userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with\n\tESMTP id v9DHJQJk005986\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Fri, 13 Oct 2017 17:19:26 GMT","from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72])\n\tby userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id v9DHJQfK005103\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Fri, 13 Oct 2017 17:19:26 GMT","from abhmp0014.oracle.com (abhmp0014.oracle.com [141.146.116.20])\n\tby userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id\n\tv9DHJLom016992; Fri, 13 Oct 2017 17:19:21 GMT","from dhcp-whq-twvpn-3-vpnpool-10-159-250-116.vpn.oracle.com\n\t(/10.159.250.116) by default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Fri, 13 Oct 2017 10:19:20 -0700"],"Subject":"Re: [PATCH v8 9/9] sparc64: Add support for ADI (Application Data\n\tIntegrity)","Mime-Version":"1.0 (Mac OS X Mail 9.3 \\(3124\\))","Content-Type":"text/plain; charset=us-ascii","From":"Anthony Yznaga ","In-Reply-To":"","Date":"Fri, 13 Oct 2017 10:19:16 -0700","Cc":"David Miller , dave.hansen@linux.intel.com,\n\tcorbet@lwn.net, Bob Picco ,\n\tSTEVEN_SISTARE ,\n\tPasha Tatashin ,\n\tMike Kravetz ,\n\tRob Gardner , mingo@kernel.org,\n\tNitin Gupta , kirill.shutemov@linux.intel.com,\n\tTom Hromatka ,\n\tEric Saint Etienne ,\n\tAllen Pais , cmetcalf@mellanox.com,\n\takpm@linux-foundation.org, geert@linux-m68k.org, pmladek@suse.com,\n\ttklauser@distanz.ch, Atish Patra ,\n\tShannon Nelson ,\n\tVijay Kumar , peterz@infradead.org,\n\tmhocko@suse.com, jack@suse.cz, lstoakes@gmail.com,\n\tpunit.agrawal@arm.com, hughd@google.com, thomas.tai@oracle.com,\n\tpaul.gortmaker@windriver.com, ross.zwisler@linux.intel.com,\n\tdave.jiang@intel.com, willy@infradead.org, ying.huang@intel.com,\n\tzhongjiang@huawei.com, minchan@kernel.org,\n\timbrenda@linux.vnet.ibm.com, aneesh.kumar@linux.vnet.ibm.com,\n\taarcange@redhat.com, linux-doc@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org,\n\tSparc kernel list ,\n\tlinux-mm@kvack.org, Khalid Aziz ","Content-Transfer-Encoding":"8BIT","Message-Id":"<520A262E-1F2A-4717-8766-CD434BBF716C@oracle.com>","References":"\n\t\n\t<9e3a8c90ade57d94d1ab2100c6d9508fc2d0a212.1506089472.git.khalid.aziz@oracle.com>\n\t\n\t<5edaf7dc-6bc7-c365-0b54-b78975c08894@oracle.com>\n\t<782BD060-74C5-4D9B-B013-731249A72F87@oracle.com>\n\t<44c3473b-a8fb-197e-7fd3-03613569f339@oracle.com>\n\t","To":"Khalid Aziz ","X-Mailer":"Apple Mail (2.3124)","X-Source-IP":"userv0022.oracle.com [156.151.31.74]","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"sparclinux@vger.kernel.org"}}]