On Mon, Nov 06, 2017 at 12:57:15AM -0800, Ram Pai wrote:
> PAPR defines 'ibm,processor-storage-keys' property. It exports two
> values. The first value holds the number of data-access keys and the
> second holds the number of instruction-access keys. Due to a bug in
> the firmware, instruction-access keys is always reported as zero.
> However any key can be configured to disable data-access and/or disable
> execution-access. The inavailablity of the second value is not a
> big handicap, though it could have been used to determine if the
> platform supported disable-execution-access.
>
> Non PAPR platforms do not define this property in the device tree yet.
> Here, we hardcode CPUs that support pkey by consulting
> PowerISA3.0
>
> This patch calculates the number of keys supported by the platform.
> Alsi it determines the platform support for read/write/execution access
> support for pkeys.
>
> Signed-off-by: Ram Pai <linuxram(a)us.ibm.com>
> ---
>
....snip...
> +static inline bool pkey_mmu_enabled(void)
> +{
> + if (firmware_has_feature(FW_FEATURE_LPAR))
> + return pkeys_total;
> + else
> + return cpu_has_feature(CPU_FTR_PKEY);
> +}
> +
> void __init pkey_initialize(void)
> {
> int os_reserved, i;
> @@ -46,14 +54,9 @@ void __init pkey_initialize(void)
> __builtin_popcountl(ARCH_VM_PKEY_FLAGS >> VM_PKEY_SHIFT)
> != (sizeof(u64) * BITS_PER_BYTE));
>
> - /*
> - * Disable the pkey system till everything is in place. A subsequent
> - * patch will enable it.
> - */
> - static_branch_enable(&pkey_disabled);
> -
> - /* Lets assume 32 keys */
> - pkeys_total = 32;
vvvvvvvvvvvvvvvvvvvv
> + /* Let's assume 32 keys if we are not told the number of pkeys. */
> + if (!pkeys_total)
> + pkeys_total = 32;
^^^^^^^^^^^^^^^^^^^^
There is a small bug here.
On a KVM guest or a LPAR, if the device tree
does not expose pkeys, the pkey-subsystem must be disabled.
Unfortunately, the code above blindly sets the pkeys_total to 32.
This confuses pkey_mmu_enabled() into returning true. Because of this
bug the guest errorneously enables pkey-subsystem.
The fix is to delete the code marked above.
>
> /*
> * Adjust the upper limit, based on the number of bits supported by
> @@ -62,11 +65,19 @@ void __init pkey_initialize(void)
> pkeys_total = min_t(int, pkeys_total,
> (ARCH_VM_PKEY_FLAGS >> VM_PKEY_SHIFT));
>
> + if (!pkey_mmu_enabled() || radix_enabled() || !pkeys_total)
> + static_branch_enable(&pkey_disabled);
> + else
> + static_branch_disable(&pkey_disabled);
> +
RP
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ram,
On Mon, Nov 06, 2017 at 12:57:36AM -0800, Ram Pai wrote:
> @@ -206,12 +209,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;
Since you removed all references for fpregset now, you probably want to
remove the declaration of the variable above.
> @@ -219,20 +224,21 @@ 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();
With this code, you removed all the reference for variable
pkey_reg_offset, thus, its declaration could be removed also.
> - *(u64 *)pkey_reg_ptr = 0x00000000;
> + dprintf1("si_pkey from siginfo: %lx\n", si_pkey);
> +#if defined(__i386__) || defined(__x86_64__) /* arch */
> + dprintf1("signal pkey_reg from xsave: %016lx\n", *pkey_reg_ptr);
> + *(u64 *)pkey_reg_ptr &= reset_bits(si_pkey, PKEY_DISABLE_ACCESS);
> +#elif __powerpc64__
Since the variable pkey_reg_ptr is only used for Intel code (inside
#ifdefs), you probably want to #ifdef the variable declaration also,
avoid triggering "unused variable" warning on non-Intel machines.
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 06, 2017 at 05:22:18PM -0800, Ram Pai wrote:
> On Mon, Nov 06, 2017 at 10:28:41PM +0100, Florian Weimer wrote:
> > * Ram Pai:
> >
> > > Testing:
> > > -------
> > > This patch series has passed all the protection key
> > > tests available in the selftest directory.The
> > > tests are updated to work on both x86 and powerpc.
> > > The selftests have passed on x86 and powerpc hardware.
> >
....snip....
> > What about siglongjmp from a signal handler?
>
> On powerpc there is some relief. the permissions on a key can be
> modified from anywhere, including from the signal handler, and the
> effect will be immediate. You dont have to wait till the
> signal handler returns for the key permissions to be restore.
>
> also after return from the sigsetjmp();
> possibly caused by siglongjmp(), the program can restore the permission
> on any key.
>
> Atleast that is my theory. Can you give me a testcase; if you have one
> handy.
>
> >
> > <https://sourceware.org/bugzilla/show_bug.cgi?id=22396>
> >
reading through the bug report, you mention that the following
"The application may not be able to save and restore the protection bits
for all keys because the kernel API does not actually specify that the
set of keys is a small, fixed set."
What exact kernel API do you need? This patch set exposes the total
number of keys and max keys, through sysfs.
https://marc.info/?l=linux-kernel&m=150995950219669&w=2
Is this sufficient? or do you need something else?
RP
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/05/2017 03:56 AM, Lei Yang wrote:
> Replace '%d' by '%zu' to fix the following compilation warning.
>
> memfd_test.c:517:3: warning: format ‘%d’ expects argument of
> type ‘int’,but argument 2 has type ‘size_t’ [-Wformat=]
> printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
> ^
> memfd_test.c: In function ‘mfd_fail_grow_write’:
> memfd_test.c:537:3: warning: format ‘%d’ expects argument
> of type ‘int’,but argument 2 has type ‘size_t’ [-Wformat=]
> printf("malloc(%d) failed: %m\n", mfd_def_size * 8);
>
> Signed-off-by: Lei Yang <Lei.Yang(a)windriver.com>
Thanks for the patch. Applied to linux-kselftest next for 4.15-rc1
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
At Linaro we run mainline/linux-next selftests on LTS releases and
run into few test failures due to kernel mismatch or missing upstream
functionality in older kernels. Discussed at length here:
https://lkml.org/lkml/2017/6/15/652
This patch series is an attempt to modify selftest firmware test scripts.
The proposed changes skip/ignore testing the upstream functionality
missing in the older kernel releases.
v2:
Changed the display message to make it consistent across all
the firmware test scripts. Added Fixes tag.
Regards,
Amit Pundir
Amit Pundir (2):
selftests: firmware: skip unsupported async loading tests
selftests: firmware: skip unsupported custom firmware fallback tests
tools/testing/selftests/firmware/fw_fallback.sh | 38 ++++++++++++++++-------
tools/testing/selftests/firmware/fw_filesystem.sh | 34 ++++++++++++--------
2 files changed, 47 insertions(+), 25 deletions(-)
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html