+static inline u32 *siginfo_get_pkey_ptr(siginfo_t *si) +{ +#ifdef si_pkey
- return &si->si_pkey;
+#else
- return (u32 *)(((u8 *)si) + si_pkey_offset);
+#endif }
FWIW, this isn't ppc-specific.
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c index f43a319..88dfa40 100644 --- a/tools/testing/selftests/vm/protection_keys.c +++ b/tools/testing/selftests/vm/protection_keys.c @@ -197,17 +197,18 @@ void dump_mem(void *dumpme, int len_bytes) int pkey_faults; int last_si_pkey = -1; +void pkey_access_allow(int pkey); void signal_handler(int signum, siginfo_t *si, void *vucontext) { ucontext_t *uctxt = vucontext; int trapno; unsigned long ip; char *fpregs; +#if defined(__i386__) || defined(__x86_64__) /* arch */ pkey_reg_t *pkey_reg_ptr;
- u64 siginfo_pkey;
+#endif /* defined(__i386__) || defined(__x86_64__) */
- u32 siginfo_pkey; u32 *si_pkey_ptr;
- int pkey_reg_offset;
- fpregset_t fpregset;
dprint_in_signal = 1; dprintf1(">>>>===============SIGSEGV============================\n"); @@ -217,12 +218,14 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext) trapno = uctxt->uc_mcontext.gregs[REG_TRAPNO]; ip = uctxt->uc_mcontext.gregs[REG_IP_IDX];
- fpregset = uctxt->uc_mcontext.fpregs;
- fpregs = (void *)fpregset;
- fpregs = (char *) uctxt->uc_mcontext.fpregs;
dprintf2("%s() trapno: %d ip: 0x%016lx info->si_code: %s/%d\n", __func__, trapno, ip, si_code_str(si->si_code), si->si_code);
+#if defined(__i386__) || defined(__x86_64__) /* arch */
#ifdef __i386__ /* * 32-bit has some extra padding so that userspace can tell whether @@ -230,20 +233,28 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext) * state. We just assume that it is here. */ fpregs += 0x70; -#endif
- pkey_reg_offset = pkey_reg_xstate_offset();
- pkey_reg_ptr = (void *)(&fpregs[pkey_reg_offset]);
+#endif /* __i386__ */
- dprintf1("siginfo: %p\n", si);
- dprintf1(" fpregs: %p\n", fpregs);
- pkey_reg_ptr = (void *)(&fpregs[pkey_reg_xstate_offset()]); /*
* If we got a PKEY fault, we *HAVE* to have at least one bit set in
* If we got a key fault, we *HAVE* to have at least one bit set in*/ dprintf1("pkey_reg_xstate_offset: %d\n", pkey_reg_xstate_offset()); if (DEBUG_LEVEL > 4) dump_mem(pkey_reg_ptr - 128, 256); pkey_assert(*pkey_reg_ptr);
- here.
+#endif /* defined(__i386__) || defined(__x86_64__) */
The series up to this point has been looking pretty nice and broken out and easy to read. It goes off the rails a bit here. Adding #ifdefs and..
- dprintf1("siginfo: %p\n", si);
- dprintf1(" fpregs: %p\n", fpregs);
- si_pkey_ptr = siginfo_get_pkey_ptr(si);
- dprintf1("si_pkey_ptr: %p\n", si_pkey_ptr);
- dump_mem(si_pkey_ptr - 8, 24);
- siginfo_pkey = *si_pkey_ptr;
- pkey_assert(siginfo_pkey < NR_PKEYS);
- last_si_pkey = siginfo_pkey;
if ((si->si_code == SEGV_MAPERR) || (si->si_code == SEGV_ACCERR) || @@ -252,22 +263,21 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext) exit(4); }
- si_pkey_ptr = (u32 *)(((u8 *)si) + si_pkey_offset);
- dprintf1("si_pkey_ptr: %p\n", si_pkey_ptr);
- dump_mem((u8 *)si_pkey_ptr - 8, 24);
- siginfo_pkey = *si_pkey_ptr;
- pkey_assert(siginfo_pkey < NR_PKEYS);
- last_si_pkey = siginfo_pkey;
Moving random code around with no explanation.
- dprintf1("signal pkey_reg from xsave: "PKEY_REG_FMT"\n", *pkey_reg_ptr); /*
*/ dprintf1("signal pkey_reg from pkey_reg: "PKEY_REG_FMT"\n", __read_pkey_reg());
- need __read_pkey_reg() version so we do not do shadow_pkey_reg
- checking
- dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
- *(u64 *)pkey_reg_ptr = 0x00000000;
+#if defined(__i386__) || defined(__x86_64__) /* arch */
- dprintf1("signal pkey_reg from xsave: "PKEY_REG_FMT"\n", *pkey_reg_ptr);
- *(u64 *)pkey_reg_ptr &= clear_pkey_flags(siginfo_pkey,
PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE);+#elif __powerpc64__
- pkey_access_allow(siginfo_pkey);
+#endif
- shadow_pkey_reg &= clear_pkey_flags(siginfo_pkey,
PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE);@@ -1331,9 +1341,8 @@ void test_executing_on_unreadable_memory(int *ptr, u16 pkey) madvise(p1, PAGE_SIZE, MADV_DONTNEED); lots_o_noops_around_write(&scratch); do_not_expect_pkey_fault("executing on PROT_EXEC memory");
- ptr_contents = read_ptr(p1);
- dprintf2("ptr (%p) contents@%d: %x\n", p1, __LINE__, ptr_contents);
- expected_pkey_fault(pkey);
- expect_fault_on_read_execonly_key(p1, pkey);
}
While none of this is a deal-breaker (as I said, I feel like the selftests/ rules are a bit more lax) this does kinda break the illusion of a nice, broken out series.
Could you address this a bit in the changelog at least, please? -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html