* Re: [PATCH v3 24/41] powerpc/book3s64/pkeys: Store/restore userspace AMR correctly on entry and exit from kernel
From: kernel test robot @ 2020-06-10 17:36 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe
Cc: Aneesh Kumar K.V, kbuild-all, bauerman, linuxram
In-Reply-To: <20200610095204.608183-25-aneesh.kumar@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 10620 bytes --]
Hi "Aneesh,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on next-20200610]
[cannot apply to scottwood/next mpe/next v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Aneesh-Kumar-K-V/Kernel-userspace-access-execution-prevention-with-hash-translation/20200610-191943
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r036-20200607 (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
In file included from arch/powerpc/include/asm/processor.h:9,
from arch/powerpc/include/asm/thread_info.h:40,
from include/linux/thread_info.h:38,
from include/asm-generic/preempt.h:5,
from ./arch/powerpc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/compat.h:10,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/book3s/64/kup.h: In function 'kuap_restore_user_amr':
>> arch/powerpc/include/asm/book3s/64/kup.h:142:22: error: 'struct pt_regs' has no member named 'kuap'
142 | mtspr(SPRN_AMR, regs->kuap);
| ^~
arch/powerpc/include/asm/reg.h:1386:33: note: in definition of macro 'mtspr'
1386 | : "r" ((unsigned long)(v)) | ^
In file included from include/linux/kernel.h:11,
from include/linux/list.h:9,
from include/linux/preempt.h:11,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/compat.h:10,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/book3s/64/kup.h: In function 'kuap_restore_kernel_amr':
arch/powerpc/include/asm/book3s/64/kup.h:155:20: error: 'struct pt_regs' has no member named 'kuap'
155 | if (unlikely(regs->kuap != amr)) {
| ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
In file included from arch/powerpc/include/asm/processor.h:9,
from arch/powerpc/include/asm/thread_info.h:40,
from include/linux/thread_info.h:38,
from include/asm-generic/preempt.h:5,
from ./arch/powerpc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/compat.h:10,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/book3s/64/kup.h:157:24: error: 'struct pt_regs' has no member named 'kuap'
157 | mtspr(SPRN_AMR, regs->kuap);
| ^~
arch/powerpc/include/asm/reg.h:1386:33: note: in definition of macro 'mtspr'
1386 | : "r" ((unsigned long)(v)) | ^
In file included from arch/powerpc/include/asm/bug.h:109,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from include/asm-generic/preempt.h:5,
from ./arch/powerpc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/compat.h:10,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/book3s/64/kup.h: In function 'bad_kuap_fault':
arch/powerpc/include/asm/book3s/64/kup.h:251:12: error: 'struct pt_regs' has no member named 'kuap'
251 | (regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ)),
| ^~
include/asm-generic/bug.h:122:25: note: in definition of macro 'WARN'
122 | int __ret_warn_on = !!(condition); | ^~~~~~~~~
In file included from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/kup.h: At top level:
>> arch/powerpc/include/asm/kup.h:56:20: error: redefinition of 'allow_user_access'
56 | static inline void allow_user_access(void __user *to, const void __user *from,
| ^~~~~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/kup.h:18,
from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/book3s/64/kup.h:212:29: note: previous definition of 'allow_user_access' was here
212 | static __always_inline void allow_user_access(void __user *to, const void __user *from,
| ^~~~~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
>> arch/powerpc/include/asm/kup.h:58:20: error: redefinition of 'prevent_user_access'
58 | static inline void prevent_user_access(void __user *to, const void __user *from,
| ^~~~~~~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/kup.h:18,
from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/book3s/64/kup.h:227:20: note: previous definition of 'prevent_user_access' was here
227 | static inline void prevent_user_access(void __user *to, const void __user *from,
| ^~~~~~~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
>> arch/powerpc/include/asm/kup.h:60:29: error: redefinition of 'prevent_user_access_return'
60 | static inline unsigned long prevent_user_access_return(void) { return 0UL; }
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/kup.h:18,
from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/book3s/64/kup.h:233:29: note: previous definition of 'prevent_user_access_return' was here
233 | static inline unsigned long prevent_user_access_return(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
>> arch/powerpc/include/asm/kup.h:61:20: error: redefinition of 'restore_user_access'
61 | static inline void restore_user_access(unsigned long flags) { }
| ^~~~~~~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/kup.h:18,
from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/book3s/64/kup.h:242:20: note: previous definition of 'restore_user_access' was here
242 | static inline void restore_user_access(unsigned long flags)
| ^~~~~~~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
>> arch/powerpc/include/asm/kup.h:63:1: error: redefinition of 'bad_kuap_fault'
63 | bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
| ^~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/kup.h:18,
from arch/powerpc/include/asm/uaccess.h:9,
from include/linux/uaccess.h:11,
from include/linux/crypto.h:21,
from include/crypto/hash.h:11,
from include/linux/uio.h:10,
from include/linux/socket.h:8,
from include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:14:
arch/powerpc/include/asm/book3s/64/kup.h:248:1: note: previous definition of 'bad_kuap_fault' was here
248 | bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
| ^~~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:100: arch/powerpc/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1141: prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:180: sub-make] Error 2
vim +142 arch/powerpc/include/asm/book3s/64/kup.h
135
136 static inline void kuap_restore_user_amr(struct pt_regs *regs)
137 {
138 if (!mmu_has_feature(MMU_FTR_PKEY))
139 return;
140
141 isync();
> 142 mtspr(SPRN_AMR, regs->kuap);
143 /*
144 * No isync required here because we are about to rfi
145 * back to previous context before any user accesses
146 * would be made, which is a CSI.
147 */
148 }
149
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33846 bytes --]
^ permalink raw reply
* Re: [PATCH v11 5/6] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
From: Dan Williams @ 2020-06-10 15:11 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Santosh Sivaraj, kbuild-all, kernel test robot, linux-nvdimm,
Aneesh Kumar K . V, Linux Kernel Mailing List, Steven Rostedt,
clang-built-linux, Oliver O'Halloran, linuxppc-dev
In-Reply-To: <87k10fw29r.fsf@linux.ibm.com>
On Wed, Jun 10, 2020 at 5:10 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
> Dan Williams <dan.j.williams@intel.com> writes:
>
> > On Tue, Jun 9, 2020 at 10:54 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
> >>
> >> Thanks Dan for the consideration and taking time to look into this.
> >>
> >> My responses below:
> >>
> >> Dan Williams <dan.j.williams@intel.com> writes:
> >>
> >> > On Mon, Jun 8, 2020 at 5:16 PM kernel test robot <lkp@intel.com> wrote:
> >> >>
> >> >> Hi Vaibhav,
> >> >>
> >> >> Thank you for the patch! Perhaps something to improve:
> >> >>
> >> >> [auto build test WARNING on powerpc/next]
> >> >> [also build test WARNING on linus/master v5.7 next-20200605]
> >> >> [cannot apply to linux-nvdimm/libnvdimm-for-next scottwood/next]
> >> >> [if your patch is applied to the wrong git tree, please drop us a note to help
> >> >> improve the system. BTW, we also suggest to use '--base' option to specify the
> >> >> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >> >>
> >> >> url: https://github.com/0day-ci/linux/commits/Vaibhav-Jain/powerpc-papr_scm-Add-support-for-reporting-nvdimm-health/20200607-211653
> >> >> base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> >> >> config: powerpc-randconfig-r016-20200607 (attached as .config)
> >> >> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project e429cffd4f228f70c1d9df0e5d77c08590dd9766)
> >> >> reproduce (this is a W=1 build):
> >> >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >> >> chmod +x ~/bin/make.cross
> >> >> # install powerpc cross compiling tool for clang build
> >> >> # apt-get install binutils-powerpc-linux-gnu
> >> >> # save the attached .config to linux build tree
> >> >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc
> >> >>
> >> >> If you fix the issue, kindly add following tag as appropriate
> >> >> Reported-by: kernel test robot <lkp@intel.com>
> >> >>
> >> >> All warnings (new ones prefixed by >>, old ones prefixed by <<):
> >> >>
> >> >> In file included from <built-in>:1:
> >> >> >> ./usr/include/asm/papr_pdsm.h:69:20: warning: field 'hdr' with variable sized type 'struct nd_cmd_pkg' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
> >> >> struct nd_cmd_pkg hdr; /* Package header containing sub-cmd */
> >> >
> >> > Hi Vaibhav,
> >> >
> >> [.]
> >> > This looks like it's going to need another round to get this fixed. I
> >> > don't think 'struct nd_pdsm_cmd_pkg' should embed a definition of
> >> > 'struct nd_cmd_pkg'. An instance of 'struct nd_cmd_pkg' carries a
> >> > payload that is the 'pdsm' specifics. As the code has it now it's
> >> > defined as a superset of 'struct nd_cmd_pkg' and the compiler warning
> >> > is pointing out a real 'struct' organization problem.
> >> >
> >> > Given the soak time needed in -next after the code is finalized this
> >> > there's no time to do another round of updates and still make the v5.8
> >> > merge window.
> >>
> >> Agreed that this looks bad, a solution will probably need some more
> >> review cycles resulting in this series missing the merge window.
> >>
> >> I am investigating into the possible solutions for this reported issue
> >> and made few observations:
> >>
> >> I see command pkg for Intel, Hpe, Msft and Hyperv families using a
> >> similar layout of embedding nd_cmd_pkg at the head of the
> >> command-pkg. struct nd_pdsm_cmd_pkg is following the same pattern.
> >>
> >> struct nd_pdsm_cmd_pkg {
> >> struct nd_cmd_pkg hdr;
> >> /* other members */
> >> };
> >>
> >> struct ndn_pkg_msft {
> >> struct nd_cmd_pkg gen;
> >> /* other members */
> >> };
> >> struct nd_pkg_intel {
> >> struct nd_cmd_pkg gen;
> >> /* other members */
> >> };
> >> struct ndn_pkg_hpe1 {
> >> struct nd_cmd_pkg gen;
> >> /* other members */
> [.]
> >
> > In those cases the other members are a union and there is no second
> > variable length array. Perhaps that is why those definitions are not
> > getting flagged? I'm not seeing anything in ndctl build options that
> > would explicitly disable this warning, but I'm not sure if the ndctl
> > build environment is missing this build warning by accident.
>
> I tried building ndctl master with clang-10 with CC=clang and
> CFLAGS="". Seeing the same warning messages reported for all command
> package struct for existing command families.
>
> ./hpe1.h:334:20: warning: field 'gen' with variable sized type 'struct nd_cmd_pkg' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
> struct nd_cmd_pkg gen;
> ^
> ./msft.h:59:20: warning: field 'gen' with variable sized type 'struct nd_cmd_pkg' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
> struct nd_cmd_pkg gen;
> ^
> ./hyperv.h:34:20: warning: field 'gen' with variable sized type 'struct nd_cmd_pkg' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
> struct nd_cmd_pkg gen;
> ^
Good to know, but ugh now I'm just realizing this warning is only
coming from clang and not gcc. Frankly I'm not as concerned about
clang warnings and I should have been more careful looking at the
source of this warning.
> >
> > Those variable size payloads are also not being used in any code paths
> > that would look at the size of the command payload, like the kernel
> > ioctl() path. The payload validation code needs static sizes and the
> > payload parsing code wants to cast the payload to a known type. I
> > don't think you can use the same struct definition for both those
> > cases which is why the ndctl parsing code uses the union layout, but
> > the kernel command marshaling code does strict layering.
> Even if I switch to union layout and replacing the flexible array 'payload'
> at end to a fixed size array something like below, I still see
> '-Wgnu-variable-sized-type-not-at-end' warning reported by clang:
>
> union nd_pdsm_cmd_payload {
> struct nd_papr_pdsm_health health;
> __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
> };
>
> struct nd_pdsm_cmd_pkg {
> struct nd_cmd_pkg hdr; /* Package header containing sub-cmd */
> __s32 cmd_status; /* Out: Sub-cmd status returned back */
> __u16 reserved[2]; /* Ignored and to be used in future */
> union nd_pdsm_cmd_payload payload;
> } __attribute__((packed));
Even though this is a clang warning, I'm still not sure it's a good
idea to copy the ndctl approach into the kernel. Could you perhaps
handle this the way that drivers/acpi/nfit/intel.c handles submitting
commands through the ND_CMD_CALL interface, i.e. by just defining the
command locally like this (from intel_security_flags()):
struct {
struct nd_cmd_pkg pkg;
struct nd_intel_get_security_state cmd;
} nd_cmd = {
.pkg = {
.nd_command = NVDIMM_INTEL_GET_SECURITY_STATE,
.nd_family = NVDIMM_FAMILY_INTEL,
.nd_size_out =
sizeof(struct nd_intel_get_security_state),
.nd_fw_size =
sizeof(struct nd_intel_get_security_state),
},
};
That way it's clear that the payload is 'struct
nd_intel_get_security_state' without needing to have a pre-existing
definition. For parsing in the ioctl path I think it's clearer to cast
the payload to the local pdsm structure for the command.
>
>
> >
> >> };
> >>
> >> Even though other command families implement similar command-package
> >> layout they were not flagged (yet) as they are (I am guessing) serviced
> >> in vendor acpi drivers rather than in kernel like in case of papr-scm
> >> command family.
> >
> > I sincerely hope there are no vendor acpi kernel drivers outside of
> > the upstream one.
> Apologies if I was not clear. Was referring to nvdimm vendor uefi
> drivers which ultimately service the DSM commands. Since CMD_CALL serves
> as a conduit to send the command payload to these vendor drivers,
> libnvdimm never needs to peek into the nd_cmd_pkg.payload
> field. Consequently nfit module never hit this warning in kernel before.
Ah, understood, and no, that's not the root reason this problem is not
present in the kernel. The expectation is that any payload that the
kernel would need to consider should probably have a kernel specific
translation defined. For example,
ND_CMD_GET_CONFIG_SIZE
ND_CMD_GET_CONFIG_DATA
ND_CMD_SET_CONFIG_DATA
...are payloads that the kernel needs to understand. However instead
of supporting each way to read / write the label area the expectation
is that all drivers just parse this common kernel payload and
translate it if necessary. For example ND_CMD_{GET,SET}_CONFIG_DATA is
optionally translated to the Intel DSMs, generic ACPI _LSR/_LSW, or
papr_scm_meta_{get,set}.
Outside of validating command numbers the expectation is that the
kernel does not validate/consume the contents of the ND_CMD_CALL
payload, it passes it to the backend where ACPI DSM or pdsm protocol
takes over.
>
> >
> >>
> >> So, I think this issue is not just specific to papr-scm command family
> >> introduced in this patch series but rather across all other command
> >> families. Every other command family assumes 'struct nd_cmd_pkg_hdr' to
> >> be embeddable and puts it at the beginning of their corresponding
> >> command-packages. And its only a matter of time when someone tries
> >> filtering/handling of vendor specific commands in nfit module when they
> >> hit similar issue.
> >>
> >> Possible Solutions:
> >>
> >> * One way would be to redefine 'struct nd_cmd_pkg' to mark field
> >> 'nd_payload[]' from a flexible array to zero sized array as
> >> 'nd_payload[0]'.
> >
> > I just went through a round of removing the usage of buf[0] in ndctl
> > since gcc10 now warns about that too.
> >
> >> This should make 'struct nd_cmd_pkg' embeddable and
> >> clang shouldn't report 'gnu-variable-sized-type-not-at-end'
> >> warning. Also I think this change shouldn't introduce any ABI change.
> >>
> >> * Another way to solve this issue might be to redefine 'struct
> >> nd_pdsm_cmd_pkg' to below removing the 'struct nd_cmd_pkg' member. This
> >> struct should immediately follow the 'struct nd_cmd_pkg' command package
> >> when sent to libnvdimm:
> >>
> >> struct nd_pdsm_cmd_pkg {
> >> __s32 cmd_status; /* Out: Sub-cmd status returned back */
> >> __u16 reserved[2]; /* Ignored and to be used in future */
> >> __u8 payload[];
> >> };
> >>
> >> This should remove the flexible member nc_cmd_pkg.nd_payload from the
> >> struct with just one remaining at the end. However this would make
> >> accessing the [in|out|fw]_size members of 'struct nd_cmd_pkg'
> >> difficult for the pdsm servicing functions.
> >>
> >>
> >> Any other solution that you think, may solve this issue ?
> >
> > The union might help, but per the above I think only for parsing the
> > command at which point I don't think the kernel needs a unified
> > structure defining both the generic envelope and the end-point
> > specific payload at once.
>
> As I tested above, switching to union too will not solve the clang
> warning.
>
> Having a unified structure for a command family defining both
> generic envelop and end-point specific payload, is what I see all the
> existing command families doing.
>
> However if I split 'struct nd_pdsm_cmd_pkg' to not have an embedded
> 'struct nd_cmd_pkg' then it goes opposite to what existing command family
> implementations.
>
> So to me it looks like no clear way to address this :-(
>
> Another non-conventional way to fix this might be to suppress this clang
> warning messages by adding "CFLAGS_papr_scm.o +=
> -Wno-gnu-variable-sized-type-not-at-end" to papr_scm Makefile.
No, I don't think it's appropriate to customize clang warnings. Have a
look at splitting parsing vs local command submission following the
approach taken in drivers/acpi/nfit/intel.c.
> Current implementation 'struct nd_cmd_pkg' clearly depends on gcc
> extension of having a flexible payload array which is allowed to be not
> necessarily placed at the end of a containing struct. So the problem can be
> attributed to difference in compiler implementations between GCC and
> Clang rather than how 'struct nd_pdsm_cmd_pkg' and 'struct nd_cmd_pkg'
> are laid out.
>
> --
> Cheers
> ~ Vaibhav
^ permalink raw reply
* Re: [PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too
From: Oleg Nesterov @ 2020-06-10 15:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Madhavan Srinivasan, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, Jan Kratochvil, linux-kernel
In-Reply-To: <20190917143753.GA12300@redhat.com>
Hi,
looks like this patch was forgotten.
Do you think this should be fixed or should we document that
PTRACE_GETREGS is not consistent with PTRACE_PEEKUSER on ppc64?
On 09/17, Oleg Nesterov wrote:
>
> I don't have a ppc machine, this patch wasn't even compile tested,
> could you please review?
>
> The commit a8a4b03ab95f ("powerpc: Hard wire PT_SOFTE value to 1 in
> ptrace & signals") changed ptrace_get_reg(PT_SOFTE) to report 0x1,
> but PTRACE_GETREGS still copies pt_regs->softe as is.
>
> This is not consistent and this breaks
> http://sourceware.org/systemtap/wiki/utrace/tests/user-regs-peekpoke
>
> Reported-by: Jan Kratochvil <jan.kratochvil@redhat.com>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
> arch/powerpc/kernel/ptrace.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 8c92feb..291acfb 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -363,11 +363,36 @@ static int gpr_get(struct task_struct *target, const struct user_regset *regset,
> BUILD_BUG_ON(offsetof(struct pt_regs, orig_gpr3) !=
> offsetof(struct pt_regs, msr) + sizeof(long));
>
> +#ifdef CONFIG_PPC64
> + if (!ret)
> + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> + &target->thread.regs->orig_gpr3,
> + offsetof(struct pt_regs, orig_gpr3),
> + offsetof(struct pt_regs, softe));
> +
> + if (!ret) {
> + unsigned long softe = 0x1;
> + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &softe,
> + offsetof(struct pt_regs, softe),
> + offsetof(struct pt_regs, softe) +
> + sizeof(softe));
> + }
> +
> + BUILD_BUG_ON(offsetof(struct pt_regs, trap) !=
> + offsetof(struct pt_regs, softe) + sizeof(long));
> +
> + if (!ret)
> + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> + &target->thread.regs->trap,
> + offsetof(struct pt_regs, trap),
> + sizeof(struct user_pt_regs));
> +#else
> if (!ret)
> ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> &target->thread.regs->orig_gpr3,
> offsetof(struct pt_regs, orig_gpr3),
> sizeof(struct user_pt_regs));
> +#endif
> if (!ret)
> ret = user_regset_copyout_zero(&pos, &count, &kbuf, &ubuf,
> sizeof(struct user_pt_regs), -1);
> --
> 2.5.0
>
^ permalink raw reply
* Re: [PATCH v2 1/4] powerpc/64s: implement probe_kernel_read/write without touching AMR
From: Christophe Leroy @ 2020-06-10 12:41 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20200403093529.43587-1-npiggin@gmail.com>
Hi Nick
Le 03/04/2020 à 11:35, Nicholas Piggin a écrit :
> There is no need to allow user accesses when probing kernel addresses.
You should have a look at
https://github.com/torvalds/linux/commit/fa94111d94354de76c47fea6e1187d1ee91e23a7
At seems to implement a generic way of achieving what you are trying to
do here.
Christophe
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> v2:
> - Enable for all powerpc (suggested by Christophe)
> - Fold helper function together (Christophe)
> - Rename uaccess.c to maccess.c to match kernel/maccess.c.
>
> arch/powerpc/include/asm/uaccess.h | 25 +++++++++++++++-------
> arch/powerpc/lib/Makefile | 2 +-
> arch/powerpc/lib/maccess.c | 34 ++++++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+), 9 deletions(-)
> create mode 100644 arch/powerpc/lib/maccess.c
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 2f500debae21..670910df3cc7 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -341,8 +341,8 @@ raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
> }
> #endif /* __powerpc64__ */
>
> -static inline unsigned long raw_copy_from_user(void *to,
> - const void __user *from, unsigned long n)
> +static inline unsigned long
> +raw_copy_from_user_allowed(void *to, const void __user *from, unsigned long n)
> {
> unsigned long ret;
> if (__builtin_constant_p(n) && (n <= 8)) {
> @@ -351,19 +351,19 @@ static inline unsigned long raw_copy_from_user(void *to,
> switch (n) {
> case 1:
> barrier_nospec();
> - __get_user_size(*(u8 *)to, from, 1, ret);
> + __get_user_size_allowed(*(u8 *)to, from, 1, ret);
> break;
> case 2:
> barrier_nospec();
> - __get_user_size(*(u16 *)to, from, 2, ret);
> + __get_user_size_allowed(*(u16 *)to, from, 2, ret);
> break;
> case 4:
> barrier_nospec();
> - __get_user_size(*(u32 *)to, from, 4, ret);
> + __get_user_size_allowed(*(u32 *)to, from, 4, ret);
> break;
> case 8:
> barrier_nospec();
> - __get_user_size(*(u64 *)to, from, 8, ret);
> + __get_user_size_allowed(*(u64 *)to, from, 8, ret);
> break;
> }
> if (ret == 0)
> @@ -371,9 +371,18 @@ static inline unsigned long raw_copy_from_user(void *to,
> }
>
> barrier_nospec();
> - allow_read_from_user(from, n);
> ret = __copy_tofrom_user((__force void __user *)to, from, n);
> - prevent_read_from_user(from, n);
> + return ret;
> +}
> +
> +static inline unsigned long
> +raw_copy_from_user(void *to, const void __user *from, unsigned long n)
> +{
> + unsigned long ret;
> +
> + allow_read_from_user(to, n);
> + ret = raw_copy_from_user_allowed(to, from, n);
> + prevent_read_from_user(to, n);
> return ret;
> }
>
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index b8de3be10eb4..77af10ad0b0d 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -16,7 +16,7 @@ CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
> CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
> endif
>
> -obj-y += alloc.o code-patching.o feature-fixups.o pmem.o
> +obj-y += alloc.o code-patching.o feature-fixups.o pmem.o maccess.o
>
> ifndef CONFIG_KASAN
> obj-y += string.o memcmp_$(BITS).o
> diff --git a/arch/powerpc/lib/maccess.c b/arch/powerpc/lib/maccess.c
> new file mode 100644
> index 000000000000..ce5465db1e2d
> --- /dev/null
> +++ b/arch/powerpc/lib/maccess.c
> @@ -0,0 +1,34 @@
> +#include <linux/mm.h>
> +#include <linux/uaccess.h>
> +
> +/*
> + * Override the generic weak linkage functions to avoid changing KUP state via
> + * the generic user access functions, as this is accessing kernel addresses.
> + */
> +long probe_kernel_read(void *dst, const void *src, size_t size)
> +{
> + long ret;
> + mm_segment_t old_fs = get_fs();
> +
> + set_fs(KERNEL_DS);
> + pagefault_disable();
> + ret = raw_copy_from_user_allowed(dst, (__force const void __user *)src, size);
> + pagefault_enable();
> + set_fs(old_fs);
> +
> + return ret ? -EFAULT : 0;
> +}
> +
> +long probe_kernel_write(void *dst, const void *src, size_t size)
> +{
> + long ret;
> + mm_segment_t old_fs = get_fs();
> +
> + set_fs(KERNEL_DS);
> + pagefault_disable();
> + ret = raw_copy_to_user_allowed((__force void __user *)dst, src, size);
> + pagefault_enable();
> + set_fs(old_fs);
> +
> + return ret ? -EFAULT : 0;
> +}
>
^ permalink raw reply
* Re: [PATCH v11 5/6] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
From: Vaibhav Jain @ 2020-06-10 12:09 UTC (permalink / raw)
To: Dan Williams
Cc: Santosh Sivaraj, kbuild-all, kernel test robot, linux-nvdimm,
Aneesh Kumar K . V, Linux Kernel Mailing List, Steven Rostedt,
clang-built-linux, Oliver O'Halloran, linuxppc-dev
In-Reply-To: <CAPcyv4jfeBoFCdg2sKP5ExpTTQ_+LyrJewTupcrTgh-qWykNxw@mail.gmail.com>
Dan Williams <dan.j.williams@intel.com> writes:
> On Tue, Jun 9, 2020 at 10:54 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>>
>> Thanks Dan for the consideration and taking time to look into this.
>>
>> My responses below:
>>
>> Dan Williams <dan.j.williams@intel.com> writes:
>>
>> > On Mon, Jun 8, 2020 at 5:16 PM kernel test robot <lkp@intel.com> wrote:
>> >>
>> >> Hi Vaibhav,
>> >>
>> >> Thank you for the patch! Perhaps something to improve:
>> >>
>> >> [auto build test WARNING on powerpc/next]
>> >> [also build test WARNING on linus/master v5.7 next-20200605]
>> >> [cannot apply to linux-nvdimm/libnvdimm-for-next scottwood/next]
>> >> [if your patch is applied to the wrong git tree, please drop us a note to help
>> >> improve the system. BTW, we also suggest to use '--base' option to specify the
>> >> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>> >>
>> >> url: https://github.com/0day-ci/linux/commits/Vaibhav-Jain/powerpc-papr_scm-Add-support-for-reporting-nvdimm-health/20200607-211653
>> >> base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
>> >> config: powerpc-randconfig-r016-20200607 (attached as .config)
>> >> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project e429cffd4f228f70c1d9df0e5d77c08590dd9766)
>> >> reproduce (this is a W=1 build):
>> >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> >> chmod +x ~/bin/make.cross
>> >> # install powerpc cross compiling tool for clang build
>> >> # apt-get install binutils-powerpc-linux-gnu
>> >> # save the attached .config to linux build tree
>> >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc
>> >>
>> >> If you fix the issue, kindly add following tag as appropriate
>> >> Reported-by: kernel test robot <lkp@intel.com>
>> >>
>> >> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> >>
>> >> In file included from <built-in>:1:
>> >> >> ./usr/include/asm/papr_pdsm.h:69:20: warning: field 'hdr' with variable sized type 'struct nd_cmd_pkg' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
>> >> struct nd_cmd_pkg hdr; /* Package header containing sub-cmd */
>> >
>> > Hi Vaibhav,
>> >
>> [.]
>> > This looks like it's going to need another round to get this fixed. I
>> > don't think 'struct nd_pdsm_cmd_pkg' should embed a definition of
>> > 'struct nd_cmd_pkg'. An instance of 'struct nd_cmd_pkg' carries a
>> > payload that is the 'pdsm' specifics. As the code has it now it's
>> > defined as a superset of 'struct nd_cmd_pkg' and the compiler warning
>> > is pointing out a real 'struct' organization problem.
>> >
>> > Given the soak time needed in -next after the code is finalized this
>> > there's no time to do another round of updates and still make the v5.8
>> > merge window.
>>
>> Agreed that this looks bad, a solution will probably need some more
>> review cycles resulting in this series missing the merge window.
>>
>> I am investigating into the possible solutions for this reported issue
>> and made few observations:
>>
>> I see command pkg for Intel, Hpe, Msft and Hyperv families using a
>> similar layout of embedding nd_cmd_pkg at the head of the
>> command-pkg. struct nd_pdsm_cmd_pkg is following the same pattern.
>>
>> struct nd_pdsm_cmd_pkg {
>> struct nd_cmd_pkg hdr;
>> /* other members */
>> };
>>
>> struct ndn_pkg_msft {
>> struct nd_cmd_pkg gen;
>> /* other members */
>> };
>> struct nd_pkg_intel {
>> struct nd_cmd_pkg gen;
>> /* other members */
>> };
>> struct ndn_pkg_hpe1 {
>> struct nd_cmd_pkg gen;
>> /* other members */
[.]
>
> In those cases the other members are a union and there is no second
> variable length array. Perhaps that is why those definitions are not
> getting flagged? I'm not seeing anything in ndctl build options that
> would explicitly disable this warning, but I'm not sure if the ndctl
> build environment is missing this build warning by accident.
I tried building ndctl master with clang-10 with CC=clang and
CFLAGS="". Seeing the same warning messages reported for all command
package struct for existing command families.
./hpe1.h:334:20: warning: field 'gen' with variable sized type 'struct nd_cmd_pkg' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
struct nd_cmd_pkg gen;
^
./msft.h:59:20: warning: field 'gen' with variable sized type 'struct nd_cmd_pkg' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
struct nd_cmd_pkg gen;
^
./hyperv.h:34:20: warning: field 'gen' with variable sized type 'struct nd_cmd_pkg' not at the end of a struct or class is a GNU extension [-Wgnu-variable-sized-type-not-at-end]
struct nd_cmd_pkg gen;
^
>
> Those variable size payloads are also not being used in any code paths
> that would look at the size of the command payload, like the kernel
> ioctl() path. The payload validation code needs static sizes and the
> payload parsing code wants to cast the payload to a known type. I
> don't think you can use the same struct definition for both those
> cases which is why the ndctl parsing code uses the union layout, but
> the kernel command marshaling code does strict layering.
Even if I switch to union layout and replacing the flexible array 'payload'
at end to a fixed size array something like below, I still see
'-Wgnu-variable-sized-type-not-at-end' warning reported by clang:
union nd_pdsm_cmd_payload {
struct nd_papr_pdsm_health health;
__u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
};
struct nd_pdsm_cmd_pkg {
struct nd_cmd_pkg hdr; /* Package header containing sub-cmd */
__s32 cmd_status; /* Out: Sub-cmd status returned back */
__u16 reserved[2]; /* Ignored and to be used in future */
union nd_pdsm_cmd_payload payload;
} __attribute__((packed));
>
>> };
>>
>> Even though other command families implement similar command-package
>> layout they were not flagged (yet) as they are (I am guessing) serviced
>> in vendor acpi drivers rather than in kernel like in case of papr-scm
>> command family.
>
> I sincerely hope there are no vendor acpi kernel drivers outside of
> the upstream one.
Apologies if I was not clear. Was referring to nvdimm vendor uefi
drivers which ultimately service the DSM commands. Since CMD_CALL serves
as a conduit to send the command payload to these vendor drivers,
libnvdimm never needs to peek into the nd_cmd_pkg.payload
field. Consequently nfit module never hit this warning in kernel before.
>
>>
>> So, I think this issue is not just specific to papr-scm command family
>> introduced in this patch series but rather across all other command
>> families. Every other command family assumes 'struct nd_cmd_pkg_hdr' to
>> be embeddable and puts it at the beginning of their corresponding
>> command-packages. And its only a matter of time when someone tries
>> filtering/handling of vendor specific commands in nfit module when they
>> hit similar issue.
>>
>> Possible Solutions:
>>
>> * One way would be to redefine 'struct nd_cmd_pkg' to mark field
>> 'nd_payload[]' from a flexible array to zero sized array as
>> 'nd_payload[0]'.
>
> I just went through a round of removing the usage of buf[0] in ndctl
> since gcc10 now warns about that too.
>
>> This should make 'struct nd_cmd_pkg' embeddable and
>> clang shouldn't report 'gnu-variable-sized-type-not-at-end'
>> warning. Also I think this change shouldn't introduce any ABI change.
>>
>> * Another way to solve this issue might be to redefine 'struct
>> nd_pdsm_cmd_pkg' to below removing the 'struct nd_cmd_pkg' member. This
>> struct should immediately follow the 'struct nd_cmd_pkg' command package
>> when sent to libnvdimm:
>>
>> struct nd_pdsm_cmd_pkg {
>> __s32 cmd_status; /* Out: Sub-cmd status returned back */
>> __u16 reserved[2]; /* Ignored and to be used in future */
>> __u8 payload[];
>> };
>>
>> This should remove the flexible member nc_cmd_pkg.nd_payload from the
>> struct with just one remaining at the end. However this would make
>> accessing the [in|out|fw]_size members of 'struct nd_cmd_pkg'
>> difficult for the pdsm servicing functions.
>>
>>
>> Any other solution that you think, may solve this issue ?
>
> The union might help, but per the above I think only for parsing the
> command at which point I don't think the kernel needs a unified
> structure defining both the generic envelope and the end-point
> specific payload at once.
As I tested above, switching to union too will not solve the clang
warning.
Having a unified structure for a command family defining both
generic envelop and end-point specific payload, is what I see all the
existing command families doing.
However if I split 'struct nd_pdsm_cmd_pkg' to not have an embedded
'struct nd_cmd_pkg' then it goes opposite to what existing command family
implementations.
So to me it looks like no clear way to address this :-(
Another non-conventional way to fix this might be to suppress this clang
warning messages by adding "CFLAGS_papr_scm.o +=
-Wno-gnu-variable-sized-type-not-at-end" to papr_scm Makefile.
Current implementation 'struct nd_cmd_pkg' clearly depends on gcc
extension of having a flexible payload array which is allowed to be not
necessarily placed at the end of a containing struct. So the problem can be
attributed to difference in compiler implementations between GCC and
Clang rather than how 'struct nd_pdsm_cmd_pkg' and 'struct nd_cmd_pkg'
are laid out.
--
Cheers
~ Vaibhav
^ permalink raw reply
* Re: PowerPC KVM-PR issue
From: Christian Zigotzky @ 2020-06-10 11:23 UTC (permalink / raw)
To: linuxppc-dev, Alexander Graf, kvm-ppc@vger.kernel.org
Cc: Darren Stevens, R.T.Dickinson, Christian Zigotzky
In-Reply-To: <cf99a8c0-3bad-d089-de54-e02d3dba7f72@xenosoft.de>
On 10 June 2020 at 11:06 am, Christian Zigotzky wrote:
> On 10 June 2020 at 00:18 am, Christian Zigotzky wrote:
>> Hello,
>>
>> KVM-PR doesn't work anymore on my Nemo board [1]. I figured out that
>> the Git kernels and the kernel 5.7 are affected.
>>
>> Error message: Fienix kernel: kvmppc_exit_pr_progint: emulation at
>> 700 failed (00000000)
>>
>> I can boot virtual QEMU PowerPC machines with KVM-PR with the kernel
>> 5.6 without any problems on my Nemo board.
>>
>> I tested it with QEMU 2.5.0 and QEMU 5.0.0 today.
>>
>> Could you please check KVM-PR on your PowerPC machine?
>>
>> Thanks,
>> Christian
>>
>> [1] https://en.wikipedia.org/wiki/AmigaOne_X1000
>
> I figured out that the PowerPC updates 5.7-1 [1] are responsible for
> the KVM-PR issue. Please test KVM-PR on your PowerPC machines and
> check the PowerPC updates 5.7-1 [1].
>
> Thanks
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d38c07afc356ddebaa3ed8ecb3f553340e05c969
>
>
I tested the latest Git kernel with Mac-on-Linux/KVM-PR today.
Unfortunately I can't use KVM-PR with MoL anymore because of this issue
(see screenshots [1]). Please check the PowerPC updates 5.7-1.
Thanks
[1]
-
https://i.pinimg.com/originals/0c/b3/64/0cb364a40241fa2b7f297d4272bbb8b7.png
-
https://i.pinimg.com/originals/9a/61/d1/9a61d170b1c9f514f7a78a3014ffd18f.png
^ permalink raw reply
* Re: [PATCH] powerpc/pseries/svm: Fixup align argument in alloc_shared_lppaca() function
From: Michael Ellerman @ 2020-06-10 11:21 UTC (permalink / raw)
To: Satheesh Rajendran, linuxppc-dev
Cc: Sukadev Bhattiprolu, Ram Pai, linux-kernel, Satheesh Rajendran,
Laurent Dufour, Thiago Jung Bauermann
In-Reply-To: <20200609113909.17236-1-sathnaga@linux.vnet.ibm.com>
Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> writes:
> Argument "align" in alloc_shared_lppaca() function was unused inside the
> function. Let's fix it and update code comment.
I think it would be better to drop the align argument entirely and keep
that logic, and the comment, internal to alloc_shared_lppaca().
cheers
^ permalink raw reply
* Re: [PATCH] tty: serial: cpm_uart: Fix behaviour for non existing GPIOs
From: Linus Walleij @ 2020-06-10 11:18 UTC (permalink / raw)
To: Christophe Leroy
Cc: Greg Kroah-Hartman, linuxppc-dev@lists.ozlabs.org list,
linux-kernel@vger.kernel.org, open list:SERIAL DRIVERS,
Jiri Slaby
In-Reply-To: <bafd8df9e743c433196c727293c5015620fae2b8.1591428452.git.christophe.leroy@csgroup.eu>
Hi Christophe!
On Sat, Jun 6, 2020 at 9:30 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
> gpiod = devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
>
> - if (gpiod) {
> + if (!IS_ERR_OR_NULL(gpiod)) {
> if (i == GPIO_RTS || i == GPIO_DTR)
> ret = gpiod_direction_output(gpiod, 0);
> else
This code, and the way descriptors are used in the driver leads
me to believe that the right solution is to use the optional
call with a hard error check:
gpiod = devm_gpiod_get_index_optional(...);
if (IS_ERR(gpiod))
return PTR_ERR(gpiod);
if (gpiod) {
... followed by the old code ...
This makes sure that the array member is left NULL if there is no
GPIO on this line, and all other errors, such as -EPROBE_DEFER
which currently absolutely does not work, will lead to us properly
exiting with an error.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 0/6] consolidate PowerPC instruction encoding macros
From: Michael Ellerman @ 2020-06-10 10:50 UTC (permalink / raw)
To: Balamuruhan S
Cc: christophe.leroy, ravi.bangoria, jniethe5, Balamuruhan S, paulus,
sandipan, naveen.n.rao, linuxppc-dev
In-Reply-To: <20200526081523.92463-1-bala24@linux.ibm.com>
Balamuruhan S <bala24@linux.ibm.com> writes:
> ppc-opcode.h have base instruction encoding wrapped with stringify_in_c()
> for raw encoding to have compatibility. But there are redundant macros for
> base instruction encodings in bpf, instruction emulation test infrastructure
> and powerpc selftests.
>
> Currently PPC_INST_* macros are used for encoding instruction opcode and PPC_*
> for raw instuction encoding, this rfc patchset introduces PPC_RAW_* macros for
> base instruction encoding and reuse it from elsewhere. With this change we can
> avoid redundant macro definitions in multiple files and start adding new
> instructions in ppc-opcode.h in future.
Sorry this series collided with the prefixed instruction support and I
didn't have time to resolve the conflicts.
Can you please rebase on top of current mainline and resend.
cheers
^ permalink raw reply
* Re: [PATCH 1/6] powerpc/ppc-opcode: introduce PPC_RAW_* macros for base instruction encoding
From: Michael Ellerman @ 2020-06-10 10:50 UTC (permalink / raw)
To: Balamuruhan S
Cc: christophe.leroy, ravi.bangoria, jniethe5, Balamuruhan S, paulus,
sandipan, naveen.n.rao, linuxppc-dev
In-Reply-To: <20200526081523.92463-2-bala24@linux.ibm.com>
Balamuruhan S <bala24@linux.ibm.com> writes:
> Introduce PPC_RAW_* macros to have all the bare encoding of ppc
> instructions. Move `VSX_XX*()` and `TMRN()` macros up to reuse it.
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Tested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/ppc-opcode.h | 183 ++++++++++++++++++++++++--
> 1 file changed, 175 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 2a39c716c343..e3540be1fc17 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -431,6 +431,181 @@
> #define __PPC_EH(eh) 0
> #endif
>
> +/* Base instruction encoding */
> +#define PPC_RAW_CP_ABORT (PPC_INST_CP_ABORT)
> +#define PPC_RAW_COPY(a, b) (PPC_INST_COPY | ___PPC_RA(a) | \
> + ___PPC_RB(b))
> +#define PPC_RAW_DARN(t, l) (PPC_INST_DARN | ___PPC_RT(t) | \
> + (((l) & 0x3) << 16))
I know you're copying the existing formatting by wrapping these lines,
but please don't.
It hurts rather than improves readability.
For a line like the one above, just let it be long, eg:
#define PPC_RAW_DARN(t, l) (PPC_INST_DARN | ___PPC_RT(t) | (((l) & 0x3) << 16))
Yeah it's 91 columns but who cares.
For the really long ones like:
> +#define PPC_RAW_TLBIE_5(rb, rs, ric, prs, r) \
> + (PPC_INST_TLBIE | \
> + ___PPC_RB(rb) | \
> + ___PPC_RS(rs) | \
> + ___PPC_RIC(ric) | \
> + ___PPC_PRS(prs) | \
> + ___PPC_R(r))
I think this is the best option:
#define PPC_RAW_TLBIE_5(rb, rs, ric, prs, r) \
(PPC_INST_TLBIE | ___PPC_RB(rb) | ___PPC_RS(rs) | ___PPC_RIC(ric) | ___PPC_PRS(prs) | ___PPC_R(r))
Which is long, but I don't think wrapping it helps.
If we didn't have those ridiculous ___PPC_RX macros it would be a bit
shorter, but I guess that's a rework for another day.
cheers
^ permalink raw reply
* [RFC PATCH v2 3/3] ASoC: fsl_asrc_dma: Reuse the dma channel if available in Back-End
From: Shengjiu Wang @ 2020-06-10 10:05 UTC (permalink / raw)
To: lars, perex, tiwai, lgirdwood, broonie, timur, nicoleotsuka,
Xiubo.Lee, festevam, alsa-devel, linux-kernel, linuxppc-dev
In-Reply-To: <cover.1591783089.git.shengjiu.wang@nxp.com>
The dma channel has been requested by Back-End cpu dai driver already.
If fsl_asrc_dma requests dma chan with same dma:tx symlink, then
there will be below warning with SDMA.
[ 48.174236] fsl-esai-dai 2024000.esai: Cannot create DMA dma:tx symlink
or with EDMA the request operation will fail for EDMA channel
can only be requested once.
So If we can reuse the dma channel of Back-End, then the issue can be
fixed.
In order to get the dma channel which is already requested in Back-End.
we use the exported two functions (snd_soc_lookup_component_nolocked
and soc_component_to_pcm). If we can get the dma channel, then reuse it,
if can't, then request a new one.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_asrc_common.h | 2 ++
sound/soc/fsl/fsl_asrc_dma.c | 52 +++++++++++++++++++++++++--------
2 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc_common.h b/sound/soc/fsl/fsl_asrc_common.h
index 77665b15c8db..09512bc79b80 100644
--- a/sound/soc/fsl/fsl_asrc_common.h
+++ b/sound/soc/fsl/fsl_asrc_common.h
@@ -32,6 +32,7 @@ enum asrc_pair_index {
* @dma_chan: inputer and output DMA channels
* @dma_data: private dma data
* @pos: hardware pointer position
+ * @req_dma_chan_dev_to_dev: flag for release dev_to_dev chan
* @private: pair private area
*/
struct fsl_asrc_pair {
@@ -45,6 +46,7 @@ struct fsl_asrc_pair {
struct dma_chan *dma_chan[2];
struct imx_dma_data dma_data;
unsigned int pos;
+ bool req_dma_chan_dev_to_dev;
void *private;
};
diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
index d6a3fc5f87e5..5ecb77d466d3 100644
--- a/sound/soc/fsl/fsl_asrc_dma.c
+++ b/sound/soc/fsl/fsl_asrc_dma.c
@@ -133,6 +133,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
struct snd_dmaengine_dai_dma_data *dma_params_fe = NULL;
struct snd_dmaengine_dai_dma_data *dma_params_be = NULL;
+ struct dma_chan *tmp_chan = NULL, *tmp_chan_new = NULL;
struct snd_pcm_runtime *runtime = substream->runtime;
struct fsl_asrc_pair *pair = runtime->private_data;
struct fsl_asrc *asrc = pair->asrc;
@@ -142,7 +143,6 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
int stream = substream->stream;
struct imx_dma_data *tmp_data;
struct snd_soc_dpcm *dpcm;
- struct dma_chan *tmp_chan;
struct device *dev_be;
u8 dir = tx ? OUT : IN;
dma_cap_mask_t mask;
@@ -152,6 +152,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
for_each_dpcm_be(rtd, stream, dpcm) {
struct snd_soc_pcm_runtime *be = dpcm->be;
struct snd_pcm_substream *substream_be;
+ struct snd_soc_component *component_be;
struct snd_soc_dai *dai = asoc_rtd_to_cpu(be, 0);
if (dpcm->fe != rtd)
@@ -160,6 +161,9 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
substream_be = snd_soc_dpcm_get_substream(be, stream);
dma_params_be = snd_soc_dai_get_dma_data(dai, substream_be);
dev_be = dai->dev;
+ component_be = snd_soc_lookup_component_nolocked(dev_be, SND_DMAENGINE_PCM_DRV_NAME);
+ if (component_be)
+ tmp_chan = soc_component_to_pcm(component_be)->chan[substream->stream];
break;
}
@@ -205,10 +209,14 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
*/
if (!asrc->use_edma) {
/* Get DMA request of Back-End */
- tmp_chan = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
+ if (!tmp_chan) {
+ tmp_chan_new = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
+ tmp_chan = tmp_chan_new;
+ }
tmp_data = tmp_chan->private;
pair->dma_data.dma_request = tmp_data->dma_request;
- dma_release_channel(tmp_chan);
+ if (tmp_chan_new)
+ dma_release_channel(tmp_chan_new);
/* Get DMA request of Front-End */
tmp_chan = asrc->get_dma_channel(pair, dir);
@@ -220,9 +228,26 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
pair->dma_chan[dir] =
dma_request_channel(mask, filter, &pair->dma_data);
+ pair->req_dma_chan_dev_to_dev = true;
} else {
- pair->dma_chan[dir] =
- asrc->get_dma_channel(pair, dir);
+ /*
+ * With EDMA, there is two dma channels can be used for p2p,
+ * one is from ASRC, one is from another peripheral
+ * (ESAI or SAI). Previously we select the dma channel of ASRC,
+ * but find an issue for ideal ratio case, there is no control
+ * for data copy speed, the speed is faster than sample
+ * frequency.
+ *
+ * So we switch to use dma channel of peripheral (ESAI or SAI),
+ * that copy speed of DMA is controlled by data consumption
+ * speed in the peripheral FIFO.
+ */
+ pair->req_dma_chan_dev_to_dev = false;
+ pair->dma_chan[dir] = tmp_chan;
+ if (!pair->dma_chan[dir]) {
+ pair->dma_chan[dir] = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
+ pair->req_dma_chan_dev_to_dev = true;
+ }
}
if (!pair->dma_chan[dir]) {
@@ -261,7 +286,8 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
ret = dmaengine_slave_config(pair->dma_chan[dir], &config_be);
if (ret) {
dev_err(dev, "failed to config DMA channel for Back-End\n");
- dma_release_channel(pair->dma_chan[dir]);
+ if (pair->req_dma_chan_dev_to_dev)
+ dma_release_channel(pair->dma_chan[dir]);
return ret;
}
@@ -273,19 +299,21 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
static int fsl_asrc_dma_hw_free(struct snd_soc_component *component,
struct snd_pcm_substream *substream)
{
+ bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
struct snd_pcm_runtime *runtime = substream->runtime;
struct fsl_asrc_pair *pair = runtime->private_data;
+ u8 dir = tx ? OUT : IN;
snd_pcm_set_runtime_buffer(substream, NULL);
- if (pair->dma_chan[IN])
- dma_release_channel(pair->dma_chan[IN]);
+ if (pair->dma_chan[!dir])
+ dma_release_channel(pair->dma_chan[!dir]);
- if (pair->dma_chan[OUT])
- dma_release_channel(pair->dma_chan[OUT]);
+ if (pair->dma_chan[dir] && pair->req_dma_chan_dev_to_dev)
+ dma_release_channel(pair->dma_chan[dir]);
- pair->dma_chan[IN] = NULL;
- pair->dma_chan[OUT] = NULL;
+ pair->dma_chan[!dir] = NULL;
+ pair->dma_chan[dir] = NULL;
return 0;
}
--
2.21.0
^ permalink raw reply related
* [RFC PATCH v2 2/3] ASoC: dmaengine_pcm: export soc_component_to_pcm
From: Shengjiu Wang @ 2020-06-10 10:05 UTC (permalink / raw)
To: lars, perex, tiwai, lgirdwood, broonie, timur, nicoleotsuka,
Xiubo.Lee, festevam, alsa-devel, linux-kernel, linuxppc-dev
In-Reply-To: <cover.1591783089.git.shengjiu.wang@nxp.com>
In DPCM case, Front-End needs to get the dma chan which has
been requested by Back-End and reuse it.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
include/sound/dmaengine_pcm.h | 11 +++++++++++
sound/soc/soc-generic-dmaengine-pcm.c | 12 ------------
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
index b65220685920..8c5e38180fb0 100644
--- a/include/sound/dmaengine_pcm.h
+++ b/include/sound/dmaengine_pcm.h
@@ -161,4 +161,15 @@ int snd_dmaengine_pcm_prepare_slave_config(struct snd_pcm_substream *substream,
#define SND_DMAENGINE_PCM_DRV_NAME "snd_dmaengine_pcm"
+struct dmaengine_pcm {
+ struct dma_chan *chan[SNDRV_PCM_STREAM_LAST + 1];
+ const struct snd_dmaengine_pcm_config *config;
+ struct snd_soc_component component;
+ unsigned int flags;
+};
+
+static inline struct dmaengine_pcm *soc_component_to_pcm(struct snd_soc_component *p)
+{
+ return container_of(p, struct dmaengine_pcm, component);
+}
#endif
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index f728309a0833..80a4e71f2d95 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -21,18 +21,6 @@
*/
#define SND_DMAENGINE_PCM_FLAG_NO_RESIDUE BIT(31)
-struct dmaengine_pcm {
- struct dma_chan *chan[SNDRV_PCM_STREAM_LAST + 1];
- const struct snd_dmaengine_pcm_config *config;
- struct snd_soc_component component;
- unsigned int flags;
-};
-
-static struct dmaengine_pcm *soc_component_to_pcm(struct snd_soc_component *p)
-{
- return container_of(p, struct dmaengine_pcm, component);
-}
-
static struct device *dmaengine_dma_dev(struct dmaengine_pcm *pcm,
struct snd_pcm_substream *substream)
{
--
2.21.0
^ permalink raw reply related
* [RFC PATCH v2 0/3] ASoC: fsl_asrc_dma: Reuse the dma channel if available in Back-End
From: Shengjiu Wang @ 2020-06-10 10:05 UTC (permalink / raw)
To: lars, perex, tiwai, lgirdwood, broonie, timur, nicoleotsuka,
Xiubo.Lee, festevam, alsa-devel, linux-kernel, linuxppc-dev
Reuse the dma channel if available in Back-End
Shengjiu Wang (3):
ASoC: soc-card: export snd_soc_lookup_component_nolocked
ASoC: dmaengine_pcm: export soc_component_to_pcm
ASoC: fsl_asrc_dma: Reuse the dma channel if available in Back-End
changes in v2:
- update according to Mark's comments and split the patch
include/sound/dmaengine_pcm.h | 11 ++++++
include/sound/soc.h | 2 ++
sound/soc/fsl/fsl_asrc_common.h | 2 ++
sound/soc/fsl/fsl_asrc_dma.c | 52 ++++++++++++++++++++-------
sound/soc/soc-core.c | 3 +-
sound/soc/soc-generic-dmaengine-pcm.c | 12 -------
6 files changed, 57 insertions(+), 25 deletions(-)
--
2.21.0
^ permalink raw reply
* [RFC PATCH v2 1/3] ASoC: soc-card: export snd_soc_lookup_component_nolocked
From: Shengjiu Wang @ 2020-06-10 10:05 UTC (permalink / raw)
To: lars, perex, tiwai, lgirdwood, broonie, timur, nicoleotsuka,
Xiubo.Lee, festevam, alsa-devel, linux-kernel, linuxppc-dev
In-Reply-To: <cover.1591783089.git.shengjiu.wang@nxp.com>
snd_soc_lookup_component_nolocked can be used for the DPCM case
that Front-End needs to get the unused platform component but
added by Back-End cpu dai driver.
If the component is gotten, then we can get the dma chan created
by Back-End component and reused it in Front-End.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
include/sound/soc.h | 2 ++
sound/soc/soc-core.c | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 74868436ac79..565612a8d690 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -444,6 +444,8 @@ int devm_snd_soc_register_component(struct device *dev,
const struct snd_soc_component_driver *component_driver,
struct snd_soc_dai_driver *dai_drv, int num_dai);
void snd_soc_unregister_component(struct device *dev);
+struct snd_soc_component *snd_soc_lookup_component_nolocked(struct device *dev,
+ const char *driver_name);
struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
const char *driver_name);
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b07eca2c6ccc..d4c73e86d058 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -310,7 +310,7 @@ struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
}
EXPORT_SYMBOL_GPL(snd_soc_rtdcom_lookup);
-static struct snd_soc_component
+struct snd_soc_component
*snd_soc_lookup_component_nolocked(struct device *dev, const char *driver_name)
{
struct snd_soc_component *component;
@@ -329,6 +329,7 @@ static struct snd_soc_component
return found_component;
}
+EXPORT_SYMBOL_GPL(snd_soc_lookup_component_nolocked);
struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
const char *driver_name)
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] powerpc/kprobes: Use probe_address() to read instructions
From: Michael Ellerman @ 2020-06-10 10:12 UTC (permalink / raw)
To: Christoph Hellwig, Michael Ellerman
Cc: Christophe Leroy, linux-kernel, Paul Mackerras, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20200609055320.GA14237@infradead.org>
Christoph Hellwig <hch@infradead.org> writes:
> On Tue, Jun 09, 2020 at 03:28:38PM +1000, Michael Ellerman wrote:
>> On Mon, 24 Feb 2020 18:02:10 +0000 (UTC), Christophe Leroy wrote:
>> > In order to avoid Oopses, use probe_address() to read the
>> > instruction at the address where the trap happened.
>>
>> Applied to powerpc/next.
>>
>> [1/1] powerpc/kprobes: Use probe_address() to read instructions
>> https://git.kernel.org/powerpc/c/9ed5df69b79a22b40b20bc2132ba2495708b19c4
>
> probe_addresss has been renamed to get_kernel_nofault in the -mm
> queue that Andrew sent off to Linus last night.
That commit above is actually already in mainline, I was just _really_
behind on sending the patch notifications.
cheers
^ permalink raw reply
* [PATCH v3 38/41] powerpc/selftest/ptrace-pkey: Update the test to mark an invalid pkey correctly
From: Aneesh Kumar K.V @ 2020-06-10 9:52 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
.../selftests/powerpc/ptrace/ptrace-pkey.c | 30 ++++++++-----------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
index f9216c7a1829..bc33d748d95b 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
@@ -66,11 +66,6 @@ static int sys_pkey_alloc(unsigned long flags, unsigned long init_access_rights)
return syscall(__NR_pkey_alloc, flags, init_access_rights);
}
-static int sys_pkey_free(int pkey)
-{
- return syscall(__NR_pkey_free, pkey);
-}
-
static int child(struct shared_info *info)
{
unsigned long reg;
@@ -100,7 +95,11 @@ static int child(struct shared_info *info)
info->amr1 |= 3ul << pkeyshift(pkey1);
info->amr2 |= 3ul << pkeyshift(pkey2);
- info->invalid_amr |= info->amr2 | 3ul << pkeyshift(pkey3);
+ /*
+ * invalid amr value where we try to force write
+ * things which are deined by a uamor setting.
+ */
+ info->invalid_amr = info->amr2 | (~0x0UL & ~info->expected_uamor);
if (disable_execute)
info->expected_iamr |= 1ul << pkeyshift(pkey1);
@@ -111,17 +110,12 @@ static int child(struct shared_info *info)
info->expected_uamor |= 3ul << pkeyshift(pkey1) |
3ul << pkeyshift(pkey2);
- info->invalid_iamr |= 1ul << pkeyshift(pkey1) | 1ul << pkeyshift(pkey2);
- info->invalid_uamor |= 3ul << pkeyshift(pkey1);
-
/*
- * We won't use pkey3. We just want a plausible but invalid key to test
- * whether ptrace will let us write to AMR bits we are not supposed to.
- *
- * This also tests whether the kernel restores the UAMOR permissions
- * after a key is freed.
+ * Create an IAMR value different from expected value.
+ * Kernel will reject an IAMR and UAMOR change.
*/
- sys_pkey_free(pkey3);
+ info->invalid_iamr = info->expected_iamr | (1ul << pkeyshift(pkey1) | 1ul << pkeyshift(pkey2));
+ info->invalid_uamor = info->expected_uamor & ~(0x3ul << pkeyshift(pkey1));
printf("%-30s AMR: %016lx pkey1: %d pkey2: %d pkey3: %d\n",
user_write, info->amr1, pkey1, pkey2, pkey3);
@@ -196,9 +190,9 @@ static int parent(struct shared_info *info, pid_t pid)
PARENT_SKIP_IF_UNSUPPORTED(ret, &info->child_sync);
PARENT_FAIL_IF(ret, &info->child_sync);
- info->amr1 = info->amr2 = info->invalid_amr = regs[0];
- info->expected_iamr = info->invalid_iamr = regs[1];
- info->expected_uamor = info->invalid_uamor = regs[2];
+ info->amr1 = info->amr2 = regs[0];
+ info->expected_iamr = regs[1];
+ info->expected_uamor = regs[2];
/* Wake up child so that it can set itself up. */
ret = prod_child(&info->child_sync);
--
2.26.2
^ permalink raw reply related
* [PATCH v3 41/41] powerpc/book3s64/hash/kup: Don't hardcode kup key
From: Aneesh Kumar K.V @ 2020-06-10 9:52 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
Make KUAP/KUEP key a variable and also check whether the platform
limit the max key such that we can't use the key for KUAP/KEUP.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
.../powerpc/include/asm/book3s/64/hash-pkey.h | 22 +--------
arch/powerpc/include/asm/book3s/64/kup.h | 1 +
arch/powerpc/mm/book3s64/pkeys.c | 46 +++++++++++++++++--
3 files changed, 43 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/hash-pkey.h b/arch/powerpc/include/asm/book3s/64/hash-pkey.h
index 9f44e208f036..ff9907c72ee3 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-pkey.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-pkey.h
@@ -2,9 +2,7 @@
#ifndef _ASM_POWERPC_BOOK3S_64_HASH_PKEY_H
#define _ASM_POWERPC_BOOK3S_64_HASH_PKEY_H
-/* We use key 3 for KERNEL */
-#define HASH_DEFAULT_KERNEL_KEY (HPTE_R_KEY_BIT0 | HPTE_R_KEY_BIT1)
-
+u64 pte_to_hpte_pkey_bits(u64 pteflags, unsigned long flags);
static inline u64 hash__vmflag_to_pte_pkey_bits(u64 vm_flags)
{
return (((vm_flags & VM_PKEY_BIT0) ? H_PTE_PKEY_BIT0 : 0x0UL) |
@@ -14,24 +12,6 @@ static inline u64 hash__vmflag_to_pte_pkey_bits(u64 vm_flags)
((vm_flags & VM_PKEY_BIT4) ? H_PTE_PKEY_BIT4 : 0x0UL));
}
-static inline u64 pte_to_hpte_pkey_bits(u64 pteflags, unsigned long flags)
-{
- unsigned long pte_pkey;
-
- pte_pkey = (((pteflags & H_PTE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL) |
- ((pteflags & H_PTE_PKEY_BIT3) ? HPTE_R_KEY_BIT3 : 0x0UL) |
- ((pteflags & H_PTE_PKEY_BIT2) ? HPTE_R_KEY_BIT2 : 0x0UL) |
- ((pteflags & H_PTE_PKEY_BIT1) ? HPTE_R_KEY_BIT1 : 0x0UL) |
- ((pteflags & H_PTE_PKEY_BIT0) ? HPTE_R_KEY_BIT0 : 0x0UL));
-
- if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_KUEP)) {
- if ((pte_pkey == 0) && (flags & HPTE_USE_KERNEL_KEY))
- return HASH_DEFAULT_KERNEL_KEY;
- }
-
- return pte_pkey;
-}
-
static inline u16 hash__pte_to_pkey_bits(u64 pteflags)
{
return (((pteflags & H_PTE_PKEY_BIT4) ? 0x10 : 0x0UL) |
diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index 44a80fa94079..56afb3bb9055 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -172,6 +172,7 @@
extern u64 default_uamor;
extern u64 default_amr;
extern u64 default_iamr;
+extern int kup_key;
/*
* For kernel thread that doesn't have thread.regs return
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 5d320ac2ba04..8ab2c377f315 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -29,6 +29,10 @@ u64 default_uamor = ~0x0UL;
* We pick key 2 because 0 is special key and 1 is reserved as per ISA.
*/
static int execute_only_key = 2;
+/*
+ * key used to implement KUAP/KUEP with hash translation.
+ */
+int kup_key = 3;
#define AMR_BITS_PER_PKEY 2
@@ -169,6 +173,18 @@ void __init pkey_early_init_devtree(void)
default_uamor &= ~(0x3ul << pkeyshift(execute_only_key));
}
+ if (unlikely(max_pkey <= kup_key)) {
+ /*
+ * Insufficient number of keys to support
+ * KUAP/KUEP feature.
+ */
+ kup_key = -1;
+ } else {
+ /* handle key which is used by kernel for KAUP */
+ reserved_allocation_mask |= (0x1 << kup_key);
+ default_uamor &= ~(0x3ul << pkeyshift(kup_key));
+ }
+
/*
* Allow access for only key 0. And prevent any other modification.
*/
@@ -189,9 +205,6 @@ void __init pkey_early_init_devtree(void)
reserved_allocation_mask |= (0x1 << 1);
default_uamor &= ~(0x3ul << pkeyshift(1));
- /* handle key 3 which is used by kernel for KAUP */
- reserved_allocation_mask |= (0x1 << 3);
- default_uamor &= ~(0x3ul << pkeyshift(3));
/*
* Prevent the usage of OS reserved keys. Update UAMOR
@@ -220,7 +233,7 @@ void __init pkey_early_init_devtree(void)
#ifdef CONFIG_PPC_KUEP
void __init setup_kuep(bool disabled)
{
- if (disabled)
+ if (disabled || kup_key == -1)
return;
/*
* On hash if PKEY feature is not enabled, disable KUAP too.
@@ -246,7 +259,7 @@ void __init setup_kuep(bool disabled)
#ifdef CONFIG_PPC_KUAP
void __init setup_kuap(bool disabled)
{
- if (disabled)
+ if (disabled || kup_key == -1)
return;
/*
* On hash if PKEY feature is not enabled, disable KUAP too.
@@ -449,3 +462,26 @@ void arch_dup_pkeys(struct mm_struct *oldmm, struct mm_struct *mm)
mm_pkey_allocation_map(mm) = mm_pkey_allocation_map(oldmm);
mm->context.execute_only_pkey = oldmm->context.execute_only_pkey;
}
+
+u64 pte_to_hpte_pkey_bits(u64 pteflags, unsigned long flags)
+{
+ unsigned long pte_pkey;
+
+ pte_pkey = (((pteflags & H_PTE_PKEY_BIT4) ? HPTE_R_KEY_BIT4 : 0x0UL) |
+ ((pteflags & H_PTE_PKEY_BIT3) ? HPTE_R_KEY_BIT3 : 0x0UL) |
+ ((pteflags & H_PTE_PKEY_BIT2) ? HPTE_R_KEY_BIT2 : 0x0UL) |
+ ((pteflags & H_PTE_PKEY_BIT1) ? HPTE_R_KEY_BIT1 : 0x0UL) |
+ ((pteflags & H_PTE_PKEY_BIT0) ? HPTE_R_KEY_BIT0 : 0x0UL));
+
+ if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_KUEP)) {
+ if ((pte_pkey == 0) &&
+ (flags & HPTE_USE_KERNEL_KEY) && (kup_key != -1)) {
+ u64 vm_flag = pkey_to_vmflag_bits(kup_key);
+ u64 pte_flag = hash__vmflag_to_pte_pkey_bits(vm_flag);
+ return pte_to_hpte_pkey_bits(pte_flag, 0);
+ }
+ }
+
+ return pte_pkey;
+}
+
--
2.26.2
^ permalink raw reply related
* [PATCH v3 17/41] powerpc/book3s64/kuap: Move KUAP related function outside radix
From: Aneesh Kumar K.V @ 2020-06-10 9:51 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
The next set of patches adds support for kuap with hash translation.
In preparation for that rename/move kuap related functions to
non radix names.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
.../asm/book3s/64/{kup-radix.h => kup.h} | 6 +++---
arch/powerpc/include/asm/kup.h | 2 +-
arch/powerpc/kernel/syscall_64.c | 2 +-
arch/powerpc/mm/book3s64/pkeys.c | 18 ++++++++++++++++++
arch/powerpc/mm/book3s64/radix_pgtable.c | 18 ------------------
5 files changed, 23 insertions(+), 23 deletions(-)
rename arch/powerpc/include/asm/book3s/64/{kup-radix.h => kup.h} (97%)
diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup.h
similarity index 97%
rename from arch/powerpc/include/asm/book3s/64/kup-radix.h
rename to arch/powerpc/include/asm/book3s/64/kup.h
index 3ee1ec60be84..dff1fef765fa 100644
--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H
-#define _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H
+#ifndef _ASM_POWERPC_BOOK3S_64_KUP_H
+#define _ASM_POWERPC_BOOK3S_64_KUP_H
#include <linux/const.h>
#include <asm/reg.h>
@@ -182,4 +182,4 @@ static inline unsigned long kuap_get_and_check_amr(void)
#endif /* __ASSEMBLY__ */
-#endif /* _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H */
+#endif /* _ASM_POWERPC_BOOK3S_64_KUP_H */
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index c745ee41ad66..015f51b02741 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -15,7 +15,7 @@
#define KUAP_CURRENT (KUAP_CURRENT_READ | KUAP_CURRENT_WRITE)
#ifdef CONFIG_PPC64
-#include <asm/book3s/64/kup-radix.h>
+#include <asm/book3s/64/kup.h>
#endif
#ifdef CONFIG_PPC_8xx
#include <asm/nohash/32/kup-8xx.h>
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
index 79edba3ab312..7e560a01afa4 100644
--- a/arch/powerpc/kernel/syscall_64.c
+++ b/arch/powerpc/kernel/syscall_64.c
@@ -2,7 +2,7 @@
#include <linux/err.h>
#include <asm/asm-prototypes.h>
-#include <asm/book3s/64/kup-radix.h>
+#include <asm/book3s/64/kup.h>
#include <asm/cputime.h>
#include <asm/hw_irq.h>
#include <asm/kprobes.h>
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 810118123e70..e923be3b52e7 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -198,6 +198,24 @@ void __init pkey_early_init_devtree(void)
return;
}
+#ifdef CONFIG_PPC_KUAP
+void __init setup_kuap(bool disabled)
+{
+ if (disabled || !early_radix_enabled())
+ return;
+
+ if (smp_processor_id() == boot_cpuid) {
+ pr_info("Activating Kernel Userspace Access Prevention\n");
+ cur_cpu_spec->mmu_features |= MMU_FTR_RADIX_KUAP;
+ }
+
+ /* Make sure userspace can't change the AMR */
+ mtspr(SPRN_UAMOR, 0);
+ mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
+ isync();
+}
+#endif
+
void pkey_mm_init(struct mm_struct *mm)
{
if (!mmu_has_feature(MMU_FTR_PKEY))
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 8acb96de0e48..7ebaf35ec21d 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -532,24 +532,6 @@ void setup_kuep(bool disabled)
}
#endif
-#ifdef CONFIG_PPC_KUAP
-void setup_kuap(bool disabled)
-{
- if (disabled || !early_radix_enabled())
- return;
-
- if (smp_processor_id() == boot_cpuid) {
- pr_info("Activating Kernel Userspace Access Prevention\n");
- cur_cpu_spec->mmu_features |= MMU_FTR_RADIX_KUAP;
- }
-
- /* Make sure userspace can't change the AMR */
- mtspr(SPRN_UAMOR, 0);
- mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
- isync();
-}
-#endif
-
void __init radix__early_init_mmu(void)
{
unsigned long lpcr;
--
2.26.2
^ permalink raw reply related
* [PATCH v3 40/41] powerpc/book3s64/keys/kuap: Reset AMR/IAMR values on kexec
From: Aneesh Kumar K.V @ 2020-06-10 9:52 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
We can kexec into a kernel that doesn't use memory keys for kernel
mapping (such as an older kernel which doesn't support kuap/kuep with hash
translation). We need to make sure we reset the AMR/IAMR value on kexec
otherwise, the new kernel will use key 0 for kernel mapping and the old
AMR value prevents access to key 0.
This patch also removes reset if IAMR and AMOR in kexec_sequence. Reset of AMOR
is not needed and the IAMR reset is partial (it doesn't do the reset
on secondary cpus) and is redundant with this patch.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/kup.h | 20 ++++++++++++++++++++
arch/powerpc/include/asm/kup.h | 14 ++++++++++++++
arch/powerpc/kernel/misc_64.S | 14 --------------
arch/powerpc/kexec/core_64.c | 3 +++
arch/powerpc/mm/book3s64/pgtable.c | 3 +++
5 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index f38748e1e37e..44a80fa94079 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -341,6 +341,26 @@ static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long address,
return !!(error_code & DSISR_KEYFAULT);
}
+#define reset_kuap reset_kuap
+static inline void reset_kuap(void)
+{
+ if (mmu_has_feature(MMU_FTR_KUAP)) {
+ mtspr(SPRN_AMR, 0);
+ /* Do we need isync()? We are going via a kexec reset */
+ isync();
+ }
+}
+
+#define reset_kuep reset_kuep
+static inline void reset_kuep(void)
+{
+ if (mmu_has_feature(MMU_FTR_KUEP)) {
+ mtspr(SPRN_IAMR, 0);
+ /* Do we need isync()? We are going via a kexec reset */
+ isync();
+ }
+}
+
#else /* CONFIG_PPC_MEM_KEYS */
static inline void kuap_restore_user_amr(struct pt_regs *regs)
{
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index a29f69bbf6ec..c7ab7310f230 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -113,6 +113,20 @@ static inline void prevent_current_write_to_user(void)
prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT_WRITE);
}
+#ifndef reset_kuap
+#define reset_kuap reset_kuap
+static inline void reset_kuap(void)
+{
+}
+#endif
+
+#ifndef reset_kuep
+#define reset_kuep reset_kuep
+static inline void reset_kuep(void)
+{
+}
+#endif
+
#endif /* !__ASSEMBLY__ */
#endif /* _ASM_POWERPC_KUAP_H_ */
diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index 1864605eca29..7bb46ad98207 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -413,20 +413,6 @@ _GLOBAL(kexec_sequence)
li r0,0
std r0,16(r1)
-BEGIN_FTR_SECTION
- /*
- * This is the best time to turn AMR/IAMR off.
- * key 0 is used in radix for supervisor<->user
- * protection, but on hash key 0 is reserved
- * ideally we want to enter with a clean state.
- * NOTE, we rely on r0 being 0 from above.
- */
- mtspr SPRN_IAMR,r0
-BEGIN_FTR_SECTION_NESTED(42)
- mtspr SPRN_AMOR,r0
-END_FTR_SECTION_NESTED_IFSET(CPU_FTR_HVMODE, 42)
-END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
-
/* save regs for local vars on new stack.
* yes, we won't go back, but ...
*/
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index b4184092172a..a124715f33ea 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -152,6 +152,9 @@ static void kexec_smp_down(void *arg)
if (ppc_md.kexec_cpu_down)
ppc_md.kexec_cpu_down(0, 1);
+ reset_kuap();
+ reset_kuep();
+
kexec_smp_wait();
/* NOTREACHED */
}
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index c58ad1049909..9673f4b74c9a 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -165,6 +165,9 @@ void mmu_cleanup_all(void)
radix__mmu_cleanup_all();
else if (mmu_hash_ops.hpte_clear_all)
mmu_hash_ops.hpte_clear_all();
+
+ reset_kuap();
+ reset_kuep();
}
#ifdef CONFIG_MEMORY_HOTPLUG
--
2.26.2
^ permalink raw reply related
* [PATCH v3 39/41] powerpc/selftest/ptrace-pkey: IAMR and uamor cannot be updated by ptrace
From: Aneesh Kumar K.V @ 2020-06-10 9:52 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
Both IAMR and uamor are privileged and cannot be updated by userspace. Hence
we also don't allow ptrace interface to update them. Don't update them in the
test. Also expected_iamr is only changed if we can allocate a DISABLE_EXECUTE
pkey.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
index bc33d748d95b..5c3c8222de46 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
@@ -101,15 +101,12 @@ static int child(struct shared_info *info)
*/
info->invalid_amr = info->amr2 | (~0x0UL & ~info->expected_uamor);
+ /*
+ * if PKEY_DISABLE_EXECUTE succeeded we should update the expected_iamr
+ */
if (disable_execute)
info->expected_iamr |= 1ul << pkeyshift(pkey1);
- else
- info->expected_iamr &= ~(1ul << pkeyshift(pkey1));
-
- info->expected_iamr &= ~(1ul << pkeyshift(pkey2) | 1ul << pkeyshift(pkey3));
- info->expected_uamor |= 3ul << pkeyshift(pkey1) |
- 3ul << pkeyshift(pkey2);
/*
* Create an IAMR value different from expected value.
* Kernel will reject an IAMR and UAMOR change.
--
2.26.2
^ permalink raw reply related
* [PATCH v3 37/41] powerpc/selftest/ptrave-pkey: Rename variables to make it easier to follow code
From: Aneesh Kumar K.V @ 2020-06-10 9:52 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
Rename variable to indicate that they are invalid values which we will use to
test ptrace update of pkeys.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
.../selftests/powerpc/ptrace/ptrace-pkey.c | 26 +++++++++----------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
index bdbbbe8431e0..f9216c7a1829 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
@@ -44,7 +44,7 @@ struct shared_info {
unsigned long amr2;
/* AMR value that ptrace should refuse to write to the child. */
- unsigned long amr3;
+ unsigned long invalid_amr;
/* IAMR value the parent expects to read from the child. */
unsigned long expected_iamr;
@@ -57,8 +57,8 @@ struct shared_info {
* (even though they're valid ones) because userspace doesn't have
* access to those registers.
*/
- unsigned long new_iamr;
- unsigned long new_uamor;
+ unsigned long invalid_iamr;
+ unsigned long invalid_uamor;
};
static int sys_pkey_alloc(unsigned long flags, unsigned long init_access_rights)
@@ -100,7 +100,7 @@ static int child(struct shared_info *info)
info->amr1 |= 3ul << pkeyshift(pkey1);
info->amr2 |= 3ul << pkeyshift(pkey2);
- info->amr3 |= info->amr2 | 3ul << pkeyshift(pkey3);
+ info->invalid_amr |= info->amr2 | 3ul << pkeyshift(pkey3);
if (disable_execute)
info->expected_iamr |= 1ul << pkeyshift(pkey1);
@@ -111,8 +111,8 @@ static int child(struct shared_info *info)
info->expected_uamor |= 3ul << pkeyshift(pkey1) |
3ul << pkeyshift(pkey2);
- info->new_iamr |= 1ul << pkeyshift(pkey1) | 1ul << pkeyshift(pkey2);
- info->new_uamor |= 3ul << pkeyshift(pkey1);
+ info->invalid_iamr |= 1ul << pkeyshift(pkey1) | 1ul << pkeyshift(pkey2);
+ info->invalid_uamor |= 3ul << pkeyshift(pkey1);
/*
* We won't use pkey3. We just want a plausible but invalid key to test
@@ -196,9 +196,9 @@ static int parent(struct shared_info *info, pid_t pid)
PARENT_SKIP_IF_UNSUPPORTED(ret, &info->child_sync);
PARENT_FAIL_IF(ret, &info->child_sync);
- info->amr1 = info->amr2 = info->amr3 = regs[0];
- info->expected_iamr = info->new_iamr = regs[1];
- info->expected_uamor = info->new_uamor = regs[2];
+ info->amr1 = info->amr2 = info->invalid_amr = regs[0];
+ info->expected_iamr = info->invalid_iamr = regs[1];
+ info->expected_uamor = info->invalid_uamor = regs[2];
/* Wake up child so that it can set itself up. */
ret = prod_child(&info->child_sync);
@@ -234,10 +234,10 @@ static int parent(struct shared_info *info, pid_t pid)
return ret;
/* Write invalid AMR value in child. */
- ret = ptrace_write_regs(pid, NT_PPC_PKEY, &info->amr3, 1);
+ ret = ptrace_write_regs(pid, NT_PPC_PKEY, &info->invalid_amr, 1);
PARENT_FAIL_IF(ret, &info->child_sync);
- printf("%-30s AMR: %016lx\n", ptrace_write_running, info->amr3);
+ printf("%-30s AMR: %016lx\n", ptrace_write_running, info->invalid_amr);
/* Wake up child so that it can verify it didn't change. */
ret = prod_child(&info->child_sync);
@@ -249,7 +249,7 @@ static int parent(struct shared_info *info, pid_t pid)
/* Try to write to IAMR. */
regs[0] = info->amr1;
- regs[1] = info->new_iamr;
+ regs[1] = info->invalid_iamr;
ret = ptrace_write_regs(pid, NT_PPC_PKEY, regs, 2);
PARENT_FAIL_IF(!ret, &info->child_sync);
@@ -257,7 +257,7 @@ static int parent(struct shared_info *info, pid_t pid)
ptrace_write_running, regs[0], regs[1]);
/* Try to write to IAMR and UAMOR. */
- regs[2] = info->new_uamor;
+ regs[2] = info->invalid_uamor;
ret = ptrace_write_regs(pid, NT_PPC_PKEY, regs, 3);
PARENT_FAIL_IF(!ret, &info->child_sync);
--
2.26.2
^ permalink raw reply related
* [PATCH v3 36/41] powerpc/book3s64/keys: Print information during boot.
From: Aneesh Kumar K.V @ 2020-06-10 9:51 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/book3s64/pkeys.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index bb127e4e2dd2..5d320ac2ba04 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -207,6 +207,7 @@ void __init pkey_early_init_devtree(void)
*/
initial_allocation_mask |= reserved_allocation_mask;
+ pr_info("Enabling Memory keys with max key count %d", max_pkey);
err_out:
/*
* Setup uamor on boot cpu
--
2.26.2
^ permalink raw reply related
* [PATCH v3 35/41] powerpc/book3s64/hash/kuep: Enable KUEP on hash
From: Aneesh Kumar K.V @ 2020-06-10 9:51 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/book3s64/pkeys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index e94585fad5c4..bb127e4e2dd2 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -219,7 +219,12 @@ void __init pkey_early_init_devtree(void)
#ifdef CONFIG_PPC_KUEP
void __init setup_kuep(bool disabled)
{
- if (disabled || !early_radix_enabled())
+ if (disabled)
+ return;
+ /*
+ * On hash if PKEY feature is not enabled, disable KUAP too.
+ */
+ if (!early_radix_enabled() && !early_mmu_has_feature(MMU_FTR_PKEY))
return;
if (smp_processor_id() == boot_cpuid) {
--
2.26.2
^ permalink raw reply related
* [PATCH v3 34/41] powerpc/book3s64/hash/kuap: Enable kuap on hash
From: Aneesh Kumar K.V @ 2020-06-10 9:51 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/book3s64/pkeys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 0f4fc2876fc8..e94585fad5c4 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -240,7 +240,12 @@ void __init setup_kuep(bool disabled)
#ifdef CONFIG_PPC_KUAP
void __init setup_kuap(bool disabled)
{
- if (disabled || !early_radix_enabled())
+ if (disabled)
+ return;
+ /*
+ * On hash if PKEY feature is not enabled, disable KUAP too.
+ */
+ if (!early_radix_enabled() && !early_mmu_has_feature(MMU_FTR_PKEY))
return;
if (smp_processor_id() == boot_cpuid) {
--
2.26.2
^ permalink raw reply related
* [PATCH v3 33/41] powerpc/book3s64/kuep: Use Key 3 to implement KUEP with hash translation.
From: Aneesh Kumar K.V @ 2020-06-10 9:51 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200610095204.608183-1-aneesh.kumar@linux.ibm.com>
Radix use IAMR Key 0 and hash translation use IAMR key 3.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/kup.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index 6d7c2de0d7f6..f38748e1e37e 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -7,7 +7,7 @@
#define AMR_KUAP_BLOCK_READ UL(0x5455555555555555)
#define AMR_KUAP_BLOCK_WRITE UL(0xa8aaaaaaaaaaaaaa)
-#define AMR_KUEP_BLOCKED (1UL << 62)
+#define AMR_KUEP_BLOCKED UL(0x5455555555555555)
#define AMR_KUAP_BLOCKED (AMR_KUAP_BLOCK_READ | AMR_KUAP_BLOCK_WRITE)
#ifdef __ASSEMBLY__
--
2.26.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox