* Re: [PATCH v5 08/26] powerpc/book3s64/pkeys: Convert execute key support to static key
From: Aneesh Kumar K.V @ 2020-07-06 8:47 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87o8ot5cv5.fsf@mpe.ellerman.id.au>
On 7/6/20 12:49 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Convert the bool to a static key like pkey_disabled.
>
> I'm not convinced this is worth the added complexity of a static key.
>
> It's not used in any performance critical paths, except for context
> switch, but that's already guarded by:
>
> if (old_thread->iamr != new_thread->iamr)
>
> Which should always be false on machines which don't have the execute
> key enabled.
>
Ok will drop the patch.
-aneesh
^ permalink raw reply
* Re: [PATCH v5 11/26] powerpc/book3s64/pkeys: Make initial_allocation_mask static
From: Aneesh Kumar K.V @ 2020-07-06 8:48 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87sge55dkh.fsf@mpe.ellerman.id.au>
On 7/6/20 12:34 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> initial_allocation_mask is not used outside this file.
>
> And never modified after init, so make it __ro_after_init as well?
>
ok, will update reserved_allocation_maask too.
> cheers
>
>> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> index 652bad7334f3..47c81d41ea9a 100644
>> --- a/arch/powerpc/include/asm/pkeys.h
>> +++ b/arch/powerpc/include/asm/pkeys.h
>> @@ -13,7 +13,6 @@
>>
>> DECLARE_STATIC_KEY_FALSE(pkey_disabled);
>> extern int max_pkey;
>> -extern u32 initial_allocation_mask; /* bits set for the initially allocated keys */
>> extern u32 reserved_allocation_mask; /* bits set for reserved keys */
>>
>> #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index a4d7287082a8..73b5ef1490c8 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -15,11 +15,11 @@
>> DEFINE_STATIC_KEY_FALSE(pkey_disabled);
>> DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);
>> int max_pkey; /* Maximum key value supported */
>> -u32 initial_allocation_mask; /* Bits set for the initially allocated keys */
>> /*
>> * Keys marked in the reservation list cannot be allocated by userspace
>> */
>> u32 reserved_allocation_mask;
>> +static u32 initial_allocation_mask; /* Bits set for the initially allocated keys */
>> static u64 default_amr;
>> static u64 default_iamr;
>> /* Allow all keys to be modified by default */
>> --
>> 2.26.2
-aneesh
^ permalink raw reply
* Re: [PATCH v5 15/26] powerpc/book3s64/pkeys: Use execute_pkey_disable static key
From: Aneesh Kumar K.V @ 2020-07-06 8:49 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87mu4d5cu8.fsf@mpe.ellerman.id.au>
On 7/6/20 12:50 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Use execute_pkey_disabled static key to check for execute key support instead
>> of pkey_disabled.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/pkeys.h | 10 +---------
>> arch/powerpc/mm/book3s64/pkeys.c | 5 ++++-
>> 2 files changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> index 47c81d41ea9a..09fbaa409ac4 100644
>> --- a/arch/powerpc/include/asm/pkeys.h
>> +++ b/arch/powerpc/include/asm/pkeys.h
>> @@ -126,15 +126,7 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
>> * Try to dedicate one of the protection keys to be used as an
>> * execute-only protection key.
>> */
>> -extern int __execute_only_pkey(struct mm_struct *mm);
>> -static inline int execute_only_pkey(struct mm_struct *mm)
>> -{
>> - if (static_branch_likely(&pkey_disabled))
>> - return -1;
>> -
>> - return __execute_only_pkey(mm);
>> -}
>> -
>> +extern int execute_only_pkey(struct mm_struct *mm);
>> extern int __arch_override_mprotect_pkey(struct vm_area_struct *vma,
>> int prot, int pkey);
>> static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index bbba9c601e14..fed4f159011b 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -345,8 +345,11 @@ void thread_pkey_regs_init(struct thread_struct *thread)
>> write_uamor(default_uamor);
>> }
>>
>> -int __execute_only_pkey(struct mm_struct *mm)
>> +int execute_only_pkey(struct mm_struct *mm)
>> {
>> + if (static_branch_likely(&execute_pkey_disabled))
>> + return -1;
>> +
>> return mm->context.execute_only_pkey;
>> }
>
> That adds the overhead of a function call, but then uses a static_key to
> avoid an easy to predict branch, which seems like a bad tradeoff. And
> it's not a performance critical path AFAICS.
>
> Anyway this seems unnecessary:
>
> pkey_early_init_devtree()
> {
> ...
> if (unlikely(max_pkey <= execute_only_key)) {
> /*
> * Insufficient number of keys to support
> * execute only key. Mark it unavailable.
> */
> execute_only_key = -1;
>
> void pkey_mm_init(struct mm_struct *mm)
> {
> ...
> mm->context.execute_only_pkey = execute_only_key;
> }
>
>
> ie. Can't it just be:
>
> static inline int execute_only_pkey(struct mm_struct *mm)
> {
> return mm->context.execute_only_pkey;
> }
>
ok updated with
modified arch/powerpc/mm/book3s64/pkeys.c
@@ -151,7 +151,7 @@ void __init pkey_early_init_devtree(void)
max_pkey = pkeys_total;
#endif
- if (unlikely(max_pkey <= execute_only_key)) {
+ if (unlikely(max_pkey <= execute_only_key) ||
!pkey_execute_disable_supported) {
/*
* Insufficient number of keys to support
* execute only key. Mark it unavailable.
@@ -368,9 +368,6 @@ int __arch_set_user_pkey_access(struct task_struct
*tsk, int pkey,
int execute_only_pkey(struct mm_struct *mm)
{
- if (static_branch_likely(&execute_pkey_disabled))
- return -1;
-
return mm->context.execute_only_pkey;
}
-aneesh
^ permalink raw reply
* Re: [PATCH v2 01/12] kexec_file: allow archs to handle special regions while locating memory hole
From: Dave Young @ 2020-07-06 9:07 UTC (permalink / raw)
To: Hari Bathini
Cc: Thiago Jung Bauermann, kernel test robot, Pingfan Liu, Kexec-ml,
Mahesh J Salgaonkar, Petr Tesarik, lkml, linuxppc-dev, Mimi Zohar,
Sourabh Jain, Andrew Morton, Vivek Goyal, Eric Biederman
In-Reply-To: <159371964681.21555.573193508667543223.stgit@hbathini.in.ibm.com>
On 07/03/20 at 01:24am, Hari Bathini wrote:
> Some architectures may have special memory regions, within the given
> memory range, which can't be used for the buffer in a kexec segment.
> Implement weak arch_kexec_locate_mem_hole() definition which arch code
> may override, to take care of special regions, while trying to locate
> a memory hole.
>
> Also, add the missing declarations for arch overridable functions and
> and drop the __weak descriptors in the declarations to avoid non-weak
> definitions from becoming weak.
>
> Reported-by: kernel test robot <lkp@intel.com>
> [lkp: In v1, arch_kimage_file_post_load_cleanup() declaration was missing]
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>
> Changes in v2:
> * Introduced arch_kexec_locate_mem_hole() for override and dropped
> weak arch_kexec_add_buffer().
> * Dropped __weak identifier for arch overridable functions.
> * Fixed the missing declaration for arch_kimage_file_post_load_cleanup()
> reported by lkp. lkp report for reference:
> - https://lore.kernel.org/patchwork/patch/1264418/
>
>
> include/linux/kexec.h | 29 ++++++++++++++++++-----------
> kernel/kexec_file.c | 16 ++++++++++++++--
> 2 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index ea67910..9e93bef 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -183,17 +183,24 @@ int kexec_purgatory_get_set_symbol(struct kimage *image, const char *name,
> bool get_value);
> void *kexec_purgatory_get_symbol_addr(struct kimage *image, const char *name);
>
> -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> - unsigned long buf_len);
> -void * __weak arch_kexec_kernel_image_load(struct kimage *image);
> -int __weak arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> - Elf_Shdr *section,
> - const Elf_Shdr *relsec,
> - const Elf_Shdr *symtab);
> -int __weak arch_kexec_apply_relocations(struct purgatory_info *pi,
> - Elf_Shdr *section,
> - const Elf_Shdr *relsec,
> - const Elf_Shdr *symtab);
> +/* Architectures may override the below functions */
> +int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> + unsigned long buf_len);
> +void *arch_kexec_kernel_image_load(struct kimage *image);
> +int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> + Elf_Shdr *section,
> + const Elf_Shdr *relsec,
> + const Elf_Shdr *symtab);
> +int arch_kexec_apply_relocations(struct purgatory_info *pi,
> + Elf_Shdr *section,
> + const Elf_Shdr *relsec,
> + const Elf_Shdr *symtab);
> +int arch_kimage_file_post_load_cleanup(struct kimage *image);
> +#ifdef CONFIG_KEXEC_SIG
> +int arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> + unsigned long buf_len);
> +#endif
> +int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
>
> extern int kexec_add_buffer(struct kexec_buf *kbuf);
> int kexec_locate_mem_hole(struct kexec_buf *kbuf);
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 09cc78d..e89912d 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -636,6 +636,19 @@ int kexec_locate_mem_hole(struct kexec_buf *kbuf)
> }
>
> /**
> + * arch_kexec_locate_mem_hole - Find free memory to place the segments.
> + * @kbuf: Parameters for the memory search.
> + *
> + * On success, kbuf->mem will have the start address of the memory region found.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int __weak arch_kexec_locate_mem_hole(struct kexec_buf *kbuf)
> +{
> + return kexec_locate_mem_hole(kbuf);
> +}
> +
> +/**
> * kexec_add_buffer - place a buffer in a kexec segment
> * @kbuf: Buffer contents and memory parameters.
> *
> @@ -647,7 +660,6 @@ int kexec_locate_mem_hole(struct kexec_buf *kbuf)
> */
> int kexec_add_buffer(struct kexec_buf *kbuf)
> {
> -
> struct kexec_segment *ksegment;
> int ret;
>
> @@ -675,7 +687,7 @@ int kexec_add_buffer(struct kexec_buf *kbuf)
> kbuf->buf_align = max(kbuf->buf_align, PAGE_SIZE);
>
> /* Walk the RAM ranges and allocate a suitable range for the buffer */
> - ret = kexec_locate_mem_hole(kbuf);
> + ret = arch_kexec_locate_mem_hole(kbuf);
> if (ret)
> return ret;
>
>
Acked-by: Dave Young <dyoung@redhat.com>
Thanks
Dave
^ permalink raw reply
* Re: [PATCH 01/14] powerpc/eeh: Remove eeh_dev_phb_init_dynamic()
From: kernel test robot @ 2020-07-06 9:12 UTC (permalink / raw)
To: Oliver O'Halloran, linuxppc-dev
Cc: Oliver O'Halloran, kbuild-all, mahesh
In-Reply-To: <20200706013619.459420-2-oohall@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3420 bytes --]
Hi Oliver,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.8-rc4 next-20200706]
[cannot apply to scottwood/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Oliver-O-Halloran/powerpc-eeh-Remove-eeh_dev_phb_init_dynamic/20200706-103630
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-r006-20200706 (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=powerpc64
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 >>):
arch/powerpc/kernel/of_platform.c: In function 'of_pci_phb_probe':
>> arch/powerpc/kernel/of_platform.c:66:2: error: implicit declaration of function 'eeh_phb_pe_create' [-Werror=implicit-function-declaration]
66 | eeh_phb_pe_create(phb);
| ^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/eeh_phb_pe_create +66 arch/powerpc/kernel/of_platform.c
28
29 /* The probing of PCI controllers from of_platform is currently
30 * 64 bits only, mostly due to gratuitous differences between
31 * the 32 and 64 bits PCI code on PowerPC and the 32 bits one
32 * lacking some bits needed here.
33 */
34
35 static int of_pci_phb_probe(struct platform_device *dev)
36 {
37 struct pci_controller *phb;
38
39 /* Check if we can do that ... */
40 if (ppc_md.pci_setup_phb == NULL)
41 return -ENODEV;
42
43 pr_info("Setting up PCI bus %pOF\n", dev->dev.of_node);
44
45 /* Alloc and setup PHB data structure */
46 phb = pcibios_alloc_controller(dev->dev.of_node);
47 if (!phb)
48 return -ENODEV;
49
50 /* Setup parent in sysfs */
51 phb->parent = &dev->dev;
52
53 /* Setup the PHB using arch provided callback */
54 if (ppc_md.pci_setup_phb(phb)) {
55 pcibios_free_controller(phb);
56 return -ENODEV;
57 }
58
59 /* Process "ranges" property */
60 pci_process_bridge_OF_ranges(phb, dev->dev.of_node, 0);
61
62 /* Init pci_dn data structures */
63 pci_devs_phb_init_dynamic(phb);
64
65 /* Create EEH PE for the PHB */
> 66 eeh_phb_pe_create(phb);
67
68 /* Scan the bus */
69 pcibios_scan_phb(phb);
70 if (phb->bus == NULL)
71 return -ENXIO;
72
73 /* Claim resources. This might need some rework as well depending
74 * whether we are doing probe-only or not, like assigning unassigned
75 * resources etc...
76 */
77 pcibios_claim_one_bus(phb->bus);
78
79 /* Add probed PCI devices to the device model */
80 pci_bus_add_devices(phb->bus);
81
82 return 0;
83 }
84
---
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: 36529 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: select ARCH_HAS_MEMBARRIER_SYNC_CORE
From: Christophe Leroy @ 2020-07-06 9:53 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: linux-arch, Mathieu Desnoyers
In-Reply-To: <20200706021822.1515189-1-npiggin@gmail.com>
Le 06/07/2020 à 04:18, Nicholas Piggin a écrit :
> powerpc return from interrupt and return from system call sequences are
> context synchronising.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> .../features/sched/membarrier-sync-core/arch-support.txt | 4 ++--
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/exception-64s.h | 4 ++++
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/features/sched/membarrier-sync-core/arch-support.txt b/Documentation/features/sched/membarrier-sync-core/arch-support.txt
> index 8a521a622966..52ad74a25f54 100644
> --- a/Documentation/features/sched/membarrier-sync-core/arch-support.txt
> +++ b/Documentation/features/sched/membarrier-sync-core/arch-support.txt
> @@ -5,7 +5,7 @@
> #
> # Architecture requirements
> #
> -# * arm/arm64
> +# * arm/arm64/powerpc
> #
> # Rely on implicit context synchronization as a result of exception return
> # when returning from IPI handler, and when returning to user-space.
> @@ -45,7 +45,7 @@
> | nios2: | TODO |
> | openrisc: | TODO |
> | parisc: | TODO |
> - | powerpc: | TODO |
> + | powerpc: | ok |
> | riscv: | TODO |
> | s390: | TODO |
> | sh: | TODO |
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 9fa23eb320ff..920c4e3ca4ef 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -131,6 +131,7 @@ config PPC
> select ARCH_HAS_PTE_DEVMAP if PPC_BOOK3S_64
> select ARCH_HAS_PTE_SPECIAL
> select ARCH_HAS_MEMBARRIER_CALLBACKS
> + select ARCH_HAS_MEMBARRIER_SYNC_CORE
> select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
> select ARCH_HAS_STRICT_KERNEL_RWX if (PPC32 && !HIBERNATION)
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
> index 47bd4ea0837d..b88cb3a989b6 100644
> --- a/arch/powerpc/include/asm/exception-64s.h
> +++ b/arch/powerpc/include/asm/exception-64s.h
> @@ -68,6 +68,10 @@
> *
> * The nop instructions allow us to insert one or more instructions to flush the
> * L1-D cache when returning to userspace or a guest.
> + *
> + * powerpc relies on return from interrupt/syscall being context synchronising
> + * (which hrfid, rfid, and rfscv are) to support ARCH_HAS_MEMBARRIER_SYNC_CORE
> + * without additional additional synchronisation instructions.
This file is dedicated to BOOK3S/64. What about other ones ?
On 32 bits, this is also valid as 'rfi' is also context synchronising,
but then why just add some comment in exception-64s.h and only there ?
> */
> #define RFI_FLUSH_SLOT \
> RFI_FLUSH_FIXUP_SECTION; \
>
Christophe
^ permalink raw reply
* Re: [PATCH v12 00/31] Speculative page faults
From: Chinwen Chang @ 2020-07-06 9:25 UTC (permalink / raw)
To: Haiyan Song
Cc: jack, sergey.senozhatsky.work, peterz, Will Deacon, mhocko,
linux-mm, paulus, Punit Agrawal, hpa, Michel Lespinasse,
Alexei Starovoitov, Andrea Arcangeli, ak, Minchan Kim,
aneesh.kumar, x86, Matthew Wilcox, Daniel Jordan, Ingo Molnar,
David Rientjes, paulmck, npiggin, sj38.park, Jerome Glisse, dave,
kemi.wang, kirill, Thomas Gleixner, Laurent Dufour, zhong jiang,
Ganesh Mahendran, Yang Shi, Mike Rapoport, linuxppc-dev,
linux-kernel, Sergey Senozhatsky, miles.chen, vinayak menon, akpm,
Tim Chen, haren
In-Reply-To: <20190620081945.hwj6ruqddefnxg6z@haiyan.sh.intel.com>
On Thu, 2019-06-20 at 16:19 +0800, Haiyan Song wrote:
> Hi Laurent,
>
> I downloaded your script and run it on Intel 2s skylake platform with spf-v12 patch
> serials.
>
> Here attached the output results of this script.
>
> The following comparison result is statistics from the script outputs.
>
> a). Enable THP
> SPF_0 change SPF_1
> will-it-scale.page_fault2.per_thread_ops 2664190.8 -11.7% 2353637.6
> will-it-scale.page_fault3.per_thread_ops 4480027.2 -14.7% 3819331.9
>
>
> b). Disable THP
> SPF_0 change SPF_1
> will-it-scale.page_fault2.per_thread_ops 2653260.7 -10% 2385165.8
> will-it-scale.page_fault3.per_thread_ops 4436330.1 -12.4% 3886734.2
>
>
> Thanks,
> Haiyan Song
>
>
> On Fri, Jun 14, 2019 at 10:44:47AM +0200, Laurent Dufour wrote:
> > Le 14/06/2019 à 10:37, Laurent Dufour a écrit :
> > > Please find attached the script I run to get these numbers.
> > > This would be nice if you could give it a try on your victim node and share the result.
> >
> > Sounds that the Intel mail fitering system doesn't like the attached shell script.
> > Please find it there: https://gist.github.com/ldu4/a5cc1a93f293108ea387d43d5d5e7f44
> >
> > Thanks,
> > Laurent.
> >
Hi Laurent,
We merged SPF v11 and some patches from v12 into our platforms. After
several experiments, we observed SPF has obvious improvements on the
launch time of applications, especially for those high-TLP ones,
# launch time of applications(s):
package version w/ SPF w/o SPF improve(%)
------------------------------------------------------------------
Baidu maps 10.13.3 0.887 0.98 9.49
Taobao 8.4.0.35 1.227 1.293 5.10
Meituan 9.12.401 1.107 1.543 28.26
WeChat 7.0.3 2.353 2.68 12.20
Honor of Kings 1.43.1.6 6.63 6.713 1.24
By the way, we have verified our platforms with those patches and
achieved the goal of mass production.
Thanks.
Chinwen Chang
^ permalink raw reply
* [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*()
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas
Cc: Mike Marciniszyn, linuxppc-dev,
Greg Kroah-Hartman linux-rdma @ vger . kernel . org,
Arnd Bergmann, Jason Gunthorpe, Sam Bobroff,
Bolarinwa Olayemi Saheed, Dennis Dalessandro, skhan,
Rafael J. Wysocki, linux-kernel, wunner.de, linux-acpi,
Doug Ledford, linux-pci, bjorn, Oliver O'Halloran,
linux-kernel-mentees, linux-rdma
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
*** BLURB HERE ***
Bolarinwa Olayemi Saheed (9):
IB/hfi1: Confirm that pcie_capability_read_dword() is successful
misc: rtsx: Confirm that pcie_capability_read_word() is successful
PCI/AER: Use error return value from pcie_capability_read_*()
PCI/ASPM: Use error return value from pcie_capability_read_*()
PCI: pciehp: Fix wrong failure check on pcie_capability_read_*()
PCI: pciehp: Prevent wrong failure check on pcie_capability_read_*()
PCI: pciehp: Make "Power On" the default in pciehp_get_power_status()
PCI/ACPI: Prevent wrong failure check on pcie_capability_read_*()
PCI: Prevent wrong failure check on pcie_capability_read_*()
PCI: Remove "*val = 0" fom pcie_capability_read_*()
drivers/infiniband/hw/hfi1/aspm.c | 7 ++++---
drivers/misc/cardreader/rts5227.c | 5 +++--
drivers/misc/cardreader/rts5249.c | 5 +++--
drivers/misc/cardreader/rts5260.c | 5 +++--
drivers/misc/cardreader/rts5261.c | 5 +++--
drivers/pci/pcie/aer.c | 5 +++--
drivers/pci/pcie/aspm.c | 33 +++++++++++++++++----------------
drivers/pci/hotplug/pciehp_hpc.c | 47 ++++++++++++++++----------------
drivers/pci/pci-acpi.c | 10 ++++---
drivers/pci/probe.c | 29 ++++++++++++--------
drivers/pci/access.c | 14 --------------
11 files changed, 82 insertions(+), 83 deletions(-)
--
2.18.2
^ permalink raw reply
* [PATCH 11/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*()
From: Saheed Olayemi Bolarinwa @ 2020-07-06 9:31 UTC (permalink / raw)
To: helgaas
Cc: Mike Marciniszyn, linuxppc-dev,
Greg Kroah-Hartman linux-rdma @ vger . kernel . org,
Arnd Bergmann, Jason Gunthorpe, Sam Bobroff,
Bolarinwa Olayemi Saheed, Dennis Dalessandro, skhan,
Rafael J. Wysocki, linux-kernel, wunner.de, linux-acpi,
Doug Ledford, linux-pci, bjorn, Oliver O'Halloran,
linux-kernel-mentees, linux-rdma
In-Reply-To: <20200706093121.9731-1-refactormyself@gmail.com>
From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
**TODO**
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
This patch depends on all of the preceeding patches in this series,
otherwise it will introduce bugs as pointed out in the commit message
of each.
drivers/pci/access.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 79c4a2ef269a..ec95edbb1ac8 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -413,13 +413,6 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
if (pcie_capability_reg_implemented(dev, pos)) {
ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
- /*
- * Reset *val to 0 if pci_read_config_word() fails, it may
- * have been written as 0xFFFF if hardware error happens
- * during pci_read_config_word().
- */
- if (ret)
- *val = 0;
return ret;
}
@@ -448,13 +441,6 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val)
if (pcie_capability_reg_implemented(dev, pos)) {
ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val);
- /*
- * Reset *val to 0 if pci_read_config_dword() fails, it may
- * have been written as 0xFFFFFFFF if hardware error happens
- * during pci_read_config_dword().
- */
- if (ret)
- *val = 0;
return ret;
}
--
2.18.2
^ permalink raw reply related
* Re: [PATCH v6 00/11] Add the multiple PF support for DWC and Layerscape
From: Lorenzo Pieralisi @ 2020-07-06 10:46 UTC (permalink / raw)
To: Xiaowei Bao
Cc: roy.zang, devicetree, jingoohan1, Zhiqiang.Hou, linuxppc-dev,
linux-pci, linux-kernel, leoyang.li, Minghuan.Lian, robh+dt,
linux-arm-kernel, gustavo.pimentel, bhelgaas, andrew.murray,
kishon, shawnguo, mingkai.hu, amurray
In-Reply-To: <20200314033038.24844-1-xiaowei.bao@nxp.com>
On Sat, Mar 14, 2020 at 11:30:27AM +0800, Xiaowei Bao wrote:
> Add the PCIe EP multiple PF support for DWC and Layerscape, add
> the doorbell MSIX function for DWC, use list to manage the PF of
> one PCIe controller, and refactor the Layerscape EP driver due to
> some platforms difference.
>
> Xiaowei Bao (11):
> PCI: designware-ep: Add multiple PFs support for DWC
> PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
> PCI: designware-ep: Move the function of getting MSI capability
> forward
> PCI: designware-ep: Modify MSI and MSIX CAP way of finding
> dt-bindings: pci: layerscape-pci: Add compatible strings for ls1088a
> and ls2088a
> PCI: layerscape: Fix some format issue of the code
> PCI: layerscape: Modify the way of getting capability with different
> PEX
> PCI: layerscape: Modify the MSIX to the doorbell mode
> PCI: layerscape: Add EP mode support for ls1088a and ls2088a
> arm64: dts: layerscape: Add PCIe EP node for ls1088a
> misc: pci_endpoint_test: Add LS1088a in pci_device_id table
>
> .../devicetree/bindings/pci/layerscape-pci.txt | 2 +
> arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 31 +++
> drivers/misc/pci_endpoint_test.c | 2 +
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 100 ++++++--
> drivers/pci/controller/dwc/pcie-designware-ep.c | 255 +++++++++++++++++----
> drivers/pci/controller/dwc/pcie-designware.c | 59 +++--
> drivers/pci/controller/dwc/pcie-designware.h | 48 +++-
> 7 files changed, 404 insertions(+), 93 deletions(-)
Can you rebase it against v5.8-rc1 please ?
Thanks,
Lorenzo
^ permalink raw reply
* Re: [PATCH] kbuild: introduce ccflags-remove-y and asflags-remove-y
From: Anders Roxell @ 2020-07-06 11:24 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michal Marek, Yoshinori Sato, Linux Kbuild mailing list,
Linux-sh list, Linux Kernel Mailing List, Steven Rostedt,
Russell King, linuxppc-dev, Ingo Molnar, Paul Mackerras,
Sami Tolvanen, Rich Felker, linux-arm-kernel
In-Reply-To: <CAK7LNATusciypBJ4dYZcyrugdi_rXEV_s=zxAehDxsX+Sd5z4g@mail.gmail.com>
Hi,
When I built an allmodconfig kernel for arm64, and boot that in qemu
guest I see the following issues:
[...]
[ 10.451561][ T1] Testing tracer function: PASSED
[ 33.087895][ T1] Testing dynamic ftrace: .. filter did not
filter .. FAILED!
[ 51.127094][ T1] ------------[ cut here ]------------
[ 51.132387][ T1] WARNING: CPU: 0 PID: 1 at
kernel/trace/trace.c:1814 run_tracer_selftest+0x314/0x40c
[ 51.140805][ T1] Modules linked in:
[ 51.145398][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
5.8.0-rc3-next-20200703-00004-g8cd4bc531754 #1
[ 51.154146][ T1] Hardware name: linux,dummy-virt (DT)
[ 51.159536][ T1] pstate: 80400005 (Nzcv daif +PAN -UAO BTYPE=--)
[ 51.165711][ T1] pc : run_tracer_selftest+0x314/0x40c
[ 51.171167][ T1] lr : run_tracer_selftest+0x308/0x40c
[ 51.176475][ T1] sp : ffff000069c07c40
[ 51.180855][ T1] x29: ffff000069c07c40 x28: ffffa00015e80320
[ 51.187187][ T1] x27: ffffa00013e074a0 x26: ffffa000150f22ee
[ 51.193520][ T1] x25: ffffa000125b35a0 x24: ffffa00013f6a000
[ 51.199868][ T1] x23: ffffa00013f6adc0 x22: 00000000ffffffff
[ 51.206225][ T1] x21: ffffa000150f2250 x20: ffffa00015e801c0
[ 51.212582][ T1] x19: ffffa00015e7fa80 x18: 0000000000002750
[ 51.218887][ T1] x17: 00000000000014c0 x16: 00000000000016f0
[ 51.225229][ T1] x15: 00000000000014f8 x14: ffffa0001000a3c8
[ 51.231584][ T1] x13: ffffa000124a5fb8 x12: ffff80000d494822
[ 51.237935][ T1] x11: 1fffe0000d494821 x10: ffff80000d494821
[ 51.244293][ T1] x9 : ffffa000101763b0 x8 : ffff000069c077d7
[ 51.250704][ T1] x7 : 0000000000000001 x6 : ffff000069c077d0
[ 51.257060][ T1] x5 : ffff00006a7fc040 x4 : 0000000000000000
[ 51.263423][ T1] x3 : ffffa00010211118 x2 : 8af0509000018b00
[ 51.269757][ T1] x1 : 0000000000000000 x0 : 0000000000000001
[ 51.276084][ T1] Call trace:
[ 51.279934][ T1] run_tracer_selftest+0x314/0x40c
[ 51.285174][ T1] init_trace_selftests+0x110/0x31c
[ 51.290391][ T1] do_one_initcall+0x410/0x960
[ 51.295315][ T1] kernel_init_freeable+0x430/0x4f0
[ 51.300595][ T1] kernel_init+0x20/0x1d8
[ 51.305180][ T1] ret_from_fork+0x10/0x18
[ 51.309741][ T1] irq event stamp: 1401832
[ 51.314399][ T1] hardirqs last enabled at (1401831):
[<ffffa0001020d10c>] console_unlock+0xc04/0xd10
[ 51.322973][ T1] hardirqs last disabled at (1401832):
[<ffffa00010050fe4>] debug_exception_enter+0xbc/0x200
[ 51.331993][ T1] softirqs last enabled at (1401828):
[<ffffa000100023a4>] __do_softirq+0x95c/0x9f8
[ 51.340490][ T1] softirqs last disabled at (1401821):
[<ffffa0001010f7d0>] irq_exit+0x118/0x198
[ 51.348717][ T1] _warn_unseeded_randomness: 5 callbacks suppressed
[ 51.349263][ T1] random: get_random_bytes called from
print_oops_end_marker+0x48/0x80 with crng_init=0
[ 51.350502][ T1] ---[ end trace c566e8a71c933d3c ]---
[...]
[40709.672335][ C0] pool 2: cpus=0 flags=0x5 nice=0 hung=3s
workers=3 manager: 1455
[40739.960593][ T26] INFO: lockdep is turned off.
[40775.312499][ T26] Kernel panic - not syncing: hung_task: blocked tasks
[40775.341521][ T26] CPU: 0 PID: 26 Comm: khungtaskd Tainted: G
W 5.8.0-rc3-next-20200703-00004-g8cd4bc531754 #1
[40775.352848][ T26] Hardware name: linux,dummy-virt (DT)
[40775.359304][ T26] Call trace:
[40775.364148][ T26] dump_backtrace+0x0/0x418
[40775.369918][ T26] show_stack+0x34/0x48
[40775.375468][ T26] dump_stack+0x1f4/0x2b0
[40775.381136][ T26] panic+0x2dc/0x6ec
[40775.386430][ T26] watchdog+0x1400/0x1460
[40775.392103][ T26] kthread+0x23c/0x250
[40775.397548][ T26] ret_from_fork+0x10/0x18
[40775.407039][ T26] Kernel Offset: disabled
[40775.412634][ T26] CPU features: 0x240002,20002004
[40775.418751][ T26] Memory Limit: none
[40775.425823][ T26] ---[ end Kernel panic - not syncing: hung_task:
blocked tasks ]---
The full log can be found here [1].
Without this patch for 'trace_selftest_dynamic' for instance, CC_FLAGS_FTRACE
was removed from kernel/trace/*, and then added back to
kernel/trace/trace_selftest_dynamic.
While with this patch it looks like we add the flag (even though it is
already there), and then
removes the flag for all files in kernel/trace/* .
Cheers,
Anders
[1] https://people.linaro.org/~anders.roxell/output-next-20200703.log
On Tue, 30 Jun 2020 at 04:09, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Mon, Jun 29, 2020 at 2:55 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > Masahiro Yamada <masahiroy@kernel.org> writes:
> > > CFLAGS_REMOVE_<file>.o works per object, that is, there is no
> > > convenient way to filter out flags for every object in a directory.
> > >
> > > Add ccflags-remove-y and asflags-remove-y to make it easily.
> > >
> > > Use ccflags-remove-y to clean up some Makefiles.
> > >
> > > Suggested-by: Sami Tolvanen <samitolvanen@google.com>
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > > arch/arm/boot/compressed/Makefile | 6 +-----
> > > arch/powerpc/xmon/Makefile | 3 +--
> > > arch/sh/boot/compressed/Makefile | 5 +----
> > > kernel/trace/Makefile | 4 ++--
> > > lib/Makefile | 5 +----
> > > scripts/Makefile.lib | 4 ++--
> > > 6 files changed, 8 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> > > index 89c76ca35640..55cbcdd88ac0 100644
> > > --- a/arch/powerpc/xmon/Makefile
> > > +++ b/arch/powerpc/xmon/Makefile
> > > @@ -7,8 +7,7 @@ UBSAN_SANITIZE := n
> > > KASAN_SANITIZE := n
> > >
> > > # Disable ftrace for the entire directory
> > > -ORIG_CFLAGS := $(KBUILD_CFLAGS)
> > > -KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
> > > +ccflags-remove-y += $(CC_FLAGS_FTRACE)
> >
> > This could be:
> >
> > ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
> >
> > Similar to kernel/trace/Makefile below.
>
>
> I fixed it up, and applied to linux-kbuild.
> Thanks.
>
>
> > I don't mind though.
> >
> > Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> >
> > cheers
> >
> > > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> > > index 6575bb0a0434..7492844a8b1b 100644
> > > --- a/kernel/trace/Makefile
> > > +++ b/kernel/trace/Makefile
> > > @@ -2,9 +2,9 @@
> > >
> > > # Do not instrument the tracer itself:
> > >
> > > +ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
> > > +
> > > ifdef CONFIG_FUNCTION_TRACER
> > > -ORIG_CFLAGS := $(KBUILD_CFLAGS)
> > > -KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
> > >
> > > # Avoid recursion due to instrumentation.
> > > KCSAN_SANITIZE := n
>
>
>
> --
> Best Regards
> Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] kbuild: introduce ccflags-remove-y and asflags-remove-y
From: Anders Roxell @ 2020-07-06 12:00 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michal Marek, Yoshinori Sato, Linux Kbuild mailing list,
Linux-sh list, Linux Kernel Mailing List, Steven Rostedt,
Russell King, linuxppc-dev, Ingo Molnar, Paul Mackerras,
Sami Tolvanen, Rich Felker, linux-arm-kernel
In-Reply-To: <CADYN=9JnzPC6Ja9s3_01k-CDTSuxKBMRdrqU5rqp0xw1r9XpRw@mail.gmail.com>
On Mon, 6 Jul 2020 at 13:24, Anders Roxell <anders.roxell@linaro.org> wrote:
>
> Hi,
>
> When I built an allmodconfig kernel for arm64, and boot that in qemu
> guest I see the following issues:
>
> [...]
> [ 10.451561][ T1] Testing tracer function: PASSED
> [ 33.087895][ T1] Testing dynamic ftrace: .. filter did not
> filter .. FAILED!
> [ 51.127094][ T1] ------------[ cut here ]------------
> [ 51.132387][ T1] WARNING: CPU: 0 PID: 1 at
> kernel/trace/trace.c:1814 run_tracer_selftest+0x314/0x40c
> [ 51.140805][ T1] Modules linked in:
> [ 51.145398][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 5.8.0-rc3-next-20200703-00004-g8cd4bc531754 #1
> [ 51.154146][ T1] Hardware name: linux,dummy-virt (DT)
> [ 51.159536][ T1] pstate: 80400005 (Nzcv daif +PAN -UAO BTYPE=--)
> [ 51.165711][ T1] pc : run_tracer_selftest+0x314/0x40c
> [ 51.171167][ T1] lr : run_tracer_selftest+0x308/0x40c
> [ 51.176475][ T1] sp : ffff000069c07c40
> [ 51.180855][ T1] x29: ffff000069c07c40 x28: ffffa00015e80320
> [ 51.187187][ T1] x27: ffffa00013e074a0 x26: ffffa000150f22ee
> [ 51.193520][ T1] x25: ffffa000125b35a0 x24: ffffa00013f6a000
> [ 51.199868][ T1] x23: ffffa00013f6adc0 x22: 00000000ffffffff
> [ 51.206225][ T1] x21: ffffa000150f2250 x20: ffffa00015e801c0
> [ 51.212582][ T1] x19: ffffa00015e7fa80 x18: 0000000000002750
> [ 51.218887][ T1] x17: 00000000000014c0 x16: 00000000000016f0
> [ 51.225229][ T1] x15: 00000000000014f8 x14: ffffa0001000a3c8
> [ 51.231584][ T1] x13: ffffa000124a5fb8 x12: ffff80000d494822
> [ 51.237935][ T1] x11: 1fffe0000d494821 x10: ffff80000d494821
> [ 51.244293][ T1] x9 : ffffa000101763b0 x8 : ffff000069c077d7
> [ 51.250704][ T1] x7 : 0000000000000001 x6 : ffff000069c077d0
> [ 51.257060][ T1] x5 : ffff00006a7fc040 x4 : 0000000000000000
> [ 51.263423][ T1] x3 : ffffa00010211118 x2 : 8af0509000018b00
> [ 51.269757][ T1] x1 : 0000000000000000 x0 : 0000000000000001
> [ 51.276084][ T1] Call trace:
> [ 51.279934][ T1] run_tracer_selftest+0x314/0x40c
> [ 51.285174][ T1] init_trace_selftests+0x110/0x31c
> [ 51.290391][ T1] do_one_initcall+0x410/0x960
> [ 51.295315][ T1] kernel_init_freeable+0x430/0x4f0
> [ 51.300595][ T1] kernel_init+0x20/0x1d8
> [ 51.305180][ T1] ret_from_fork+0x10/0x18
> [ 51.309741][ T1] irq event stamp: 1401832
> [ 51.314399][ T1] hardirqs last enabled at (1401831):
> [<ffffa0001020d10c>] console_unlock+0xc04/0xd10
> [ 51.322973][ T1] hardirqs last disabled at (1401832):
> [<ffffa00010050fe4>] debug_exception_enter+0xbc/0x200
> [ 51.331993][ T1] softirqs last enabled at (1401828):
> [<ffffa000100023a4>] __do_softirq+0x95c/0x9f8
> [ 51.340490][ T1] softirqs last disabled at (1401821):
> [<ffffa0001010f7d0>] irq_exit+0x118/0x198
> [ 51.348717][ T1] _warn_unseeded_randomness: 5 callbacks suppressed
> [ 51.349263][ T1] random: get_random_bytes called from
> print_oops_end_marker+0x48/0x80 with crng_init=0
> [ 51.350502][ T1] ---[ end trace c566e8a71c933d3c ]---
> [...]
> [40709.672335][ C0] pool 2: cpus=0 flags=0x5 nice=0 hung=3s
> workers=3 manager: 1455
> [40739.960593][ T26] INFO: lockdep is turned off.
> [40775.312499][ T26] Kernel panic - not syncing: hung_task: blocked tasks
> [40775.341521][ T26] CPU: 0 PID: 26 Comm: khungtaskd Tainted: G
> W 5.8.0-rc3-next-20200703-00004-g8cd4bc531754 #1
> [40775.352848][ T26] Hardware name: linux,dummy-virt (DT)
> [40775.359304][ T26] Call trace:
> [40775.364148][ T26] dump_backtrace+0x0/0x418
> [40775.369918][ T26] show_stack+0x34/0x48
> [40775.375468][ T26] dump_stack+0x1f4/0x2b0
> [40775.381136][ T26] panic+0x2dc/0x6ec
> [40775.386430][ T26] watchdog+0x1400/0x1460
> [40775.392103][ T26] kthread+0x23c/0x250
> [40775.397548][ T26] ret_from_fork+0x10/0x18
> [40775.407039][ T26] Kernel Offset: disabled
> [40775.412634][ T26] CPU features: 0x240002,20002004
> [40775.418751][ T26] Memory Limit: none
> [40775.425823][ T26] ---[ end Kernel panic - not syncing: hung_task:
> blocked tasks ]---
>
> The full log can be found here [1].
>
> Without this patch for 'trace_selftest_dynamic' for instance, CC_FLAGS_FTRACE
> was removed from kernel/trace/*, and then added back to
> kernel/trace/trace_selftest_dynamic.
> While with this patch it looks like we add the flag (even though it is
> already there), and then
> removes the flag for all files in kernel/trace/* .
Hi again,
I think the patch below solved the issue for trace_selftest_dynamic. However,
I think there is still differences in lib/* .
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index cc5e3c6aaa20..5632a711921f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -111,12 +111,12 @@ basename_flags = -DKBUILD_BASENAME=$(call
name-fix,$(basetarget))
modname_flags = -DKBUILD_MODNAME=$(call name-fix,$(modname))
modfile_flags = -DKBUILD_MODFILE=$(call stringify,$(modfile))
-orig_c_flags = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) \
- $(ccflags-y) $(CFLAGS_$(target-stem).o)
+orig_c_flags = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(ccflags-y)
_c_flags = $(filter-out $(ccflags-remove-y)
$(CFLAGS_REMOVE_$(target-stem).o), $(orig_c_flags))
-orig_a_flags = $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) \
- $(asflags-y) $(AFLAGS_$(target-stem).o)
+_c_flags += $(CFLAGS_$(target-stem).o)
+orig_a_flags = $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) $(asflags-y)
_a_flags = $(filter-out $(asflags-remove-y)
$(AFLAGS_REMOVE_$(target-stem).o), $(orig_a_flags))
+_a_flags += $(AFLAGS_$(target-stem).o)
_cpp_flags = $(KBUILD_CPPFLAGS) $(cppflags-y)
$(CPPFLAGS_$(target-stem).lds)
#
>
> Cheers,
> Anders
> [1] https://people.linaro.org/~anders.roxell/output-next-20200703.log
>
> On Tue, 30 Jun 2020 at 04:09, Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Mon, Jun 29, 2020 at 2:55 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> > >
> > > Masahiro Yamada <masahiroy@kernel.org> writes:
> > > > CFLAGS_REMOVE_<file>.o works per object, that is, there is no
> > > > convenient way to filter out flags for every object in a directory.
> > > >
> > > > Add ccflags-remove-y and asflags-remove-y to make it easily.
> > > >
> > > > Use ccflags-remove-y to clean up some Makefiles.
> > > >
> > > > Suggested-by: Sami Tolvanen <samitolvanen@google.com>
> > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > > ---
> > > >
> > > > arch/arm/boot/compressed/Makefile | 6 +-----
> > > > arch/powerpc/xmon/Makefile | 3 +--
> > > > arch/sh/boot/compressed/Makefile | 5 +----
> > > > kernel/trace/Makefile | 4 ++--
> > > > lib/Makefile | 5 +----
> > > > scripts/Makefile.lib | 4 ++--
> > > > 6 files changed, 8 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> > > > index 89c76ca35640..55cbcdd88ac0 100644
> > > > --- a/arch/powerpc/xmon/Makefile
> > > > +++ b/arch/powerpc/xmon/Makefile
> > > > @@ -7,8 +7,7 @@ UBSAN_SANITIZE := n
> > > > KASAN_SANITIZE := n
> > > >
> > > > # Disable ftrace for the entire directory
> > > > -ORIG_CFLAGS := $(KBUILD_CFLAGS)
> > > > -KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
> > > > +ccflags-remove-y += $(CC_FLAGS_FTRACE)
> > >
> > > This could be:
> > >
> > > ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
> > >
> > > Similar to kernel/trace/Makefile below.
> >
> >
> > I fixed it up, and applied to linux-kbuild.
> > Thanks.
> >
> >
> > > I don't mind though.
> > >
> > > Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> > >
> > > cheers
> > >
> > > > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> > > > index 6575bb0a0434..7492844a8b1b 100644
> > > > --- a/kernel/trace/Makefile
> > > > +++ b/kernel/trace/Makefile
> > > > @@ -2,9 +2,9 @@
> > > >
> > > > # Do not instrument the tracer itself:
> > > >
> > > > +ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
> > > > +
> > > > ifdef CONFIG_FUNCTION_TRACER
> > > > -ORIG_CFLAGS := $(KBUILD_CFLAGS)
> > > > -KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
> > > >
> > > > # Avoid recursion due to instrumentation.
> > > > KCSAN_SANITIZE := n
> >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
^ permalink raw reply related
* Re: [PATCH V4 0/4] mm/debug_vm_pgtable: Add some more tests
From: Alexander Gordeev @ 2020-07-06 12:06 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-doc, Heiko Carstens, linux-mm, Paul Mackerras,
H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
Jonathan Corbet, x86, Mike Rapoport, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-snps-arc, Vasily Gorbik,
Borislav Petkov, Paul Walmsley, Kirill A . Shutemov,
Thomas Gleixner, linux-arm-kernel, Vineet Gupta, linux-kernel,
Palmer Dabbelt, Andrew Morton, linuxppc-dev
In-Reply-To: <1593996516-7186-1-git-send-email-anshuman.khandual@arm.com>
On Mon, Jul 06, 2020 at 06:18:32AM +0530, Anshuman Khandual wrote:
[...]
> Tested on arm64, x86 platforms but only build tested on all other enabled
> platforms through ARCH_HAS_DEBUG_VM_PGTABLE i.e powerpc, arc, s390. The
Sorry for missing to test earlier. Works for me on s390. Also, tried with
few relevant config options to set/unset.
Thanks!
^ permalink raw reply
* Re: [PATCH v5 18/26] powerpc/book3s64/keys/kuap: Reset AMR/IAMR values on kexec
From: Michael Ellerman @ 2020-07-06 12:29 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-19-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> As we kexec across kernels that use AMR/IAMR for different purposes
> we need to ensure that new kernels get kexec'd with a reset value
> of AMR/IAMR. For ex: the new kernel can 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.
I like the idea of cleaning this stuff up.
But I think tying it into kup/kuep/kuap and the MMU features and ifdefs
and so on is overly complicated.
We should just have eg:
void reset_sprs(void)
{
if (early_cpu_has_feature(CPU_FTR_ARCH_206)) {
mtspr(SPRN_AMR, 0);
mtspr(SPRN_UAMOR, 0);
}
if (early_cpu_has_feature(CPU_FTR_ARCH_207S)) {
mtspr(SPRN_IAMR, 0);
}
}
And call that from the kexec paths.
cheers
> diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
> index 3ee1ec60be84..c57063c35833 100644
> --- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
> @@ -180,6 +180,26 @@ static inline unsigned long kuap_get_and_check_amr(void)
> }
> #endif /* CONFIG_PPC_KUAP */
>
> +#define reset_kuap reset_kuap
> +static inline void reset_kuap(void)
> +{
> + if (mmu_has_feature(MMU_FTR_RADIX_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();
> + }
> +}
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H */
> diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
> index c745ee41ad66..4dc23a706910 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
* Re: [PATCH v12 00/31] Speculative page faults
From: Laurent Dufour @ 2020-07-06 12:27 UTC (permalink / raw)
To: Chinwen Chang, Haiyan Song
Cc: jack, sergey.senozhatsky.work, peterz, Will Deacon, mhocko,
linux-mm, paulus, Punit Agrawal, hpa, Michel Lespinasse,
Alexei Starovoitov, Andrea Arcangeli, ak, Minchan Kim,
aneesh.kumar, x86, Matthew Wilcox, Daniel Jordan, Ingo Molnar,
David Rientjes, paulmck, npiggin, sj38.park, Jerome Glisse, dave,
kemi.wang, kirill, Thomas Gleixner, zhong jiang, Ganesh Mahendran,
Yang Shi, Mike Rapoport, linuxppc-dev, linux-kernel,
Sergey Senozhatsky, miles.chen, vinayak menon, akpm, Tim Chen,
haren
In-Reply-To: <1594027500.30360.32.camel@mtkswgap22>
Le 06/07/2020 à 11:25, Chinwen Chang a écrit :
> On Thu, 2019-06-20 at 16:19 +0800, Haiyan Song wrote:
>> Hi Laurent,
>>
>> I downloaded your script and run it on Intel 2s skylake platform with spf-v12 patch
>> serials.
>>
>> Here attached the output results of this script.
>>
>> The following comparison result is statistics from the script outputs.
>>
>> a). Enable THP
>> SPF_0 change SPF_1
>> will-it-scale.page_fault2.per_thread_ops 2664190.8 -11.7% 2353637.6
>> will-it-scale.page_fault3.per_thread_ops 4480027.2 -14.7% 3819331.9
>>
>>
>> b). Disable THP
>> SPF_0 change SPF_1
>> will-it-scale.page_fault2.per_thread_ops 2653260.7 -10% 2385165.8
>> will-it-scale.page_fault3.per_thread_ops 4436330.1 -12.4% 3886734.2
>>
>>
>> Thanks,
>> Haiyan Song
>>
>>
>> On Fri, Jun 14, 2019 at 10:44:47AM +0200, Laurent Dufour wrote:
>>> Le 14/06/2019 à 10:37, Laurent Dufour a écrit :
>>>> Please find attached the script I run to get these numbers.
>>>> This would be nice if you could give it a try on your victim node and share the result.
>>>
>>> Sounds that the Intel mail fitering system doesn't like the attached shell script.
>>> Please find it there: https://gist.github.com/ldu4/a5cc1a93f293108ea387d43d5d5e7f44
>>>
>>> Thanks,
>>> Laurent.
>>>
>
> Hi Laurent,
>
> We merged SPF v11 and some patches from v12 into our platforms. After
> several experiments, we observed SPF has obvious improvements on the
> launch time of applications, especially for those high-TLP ones,
>
> # launch time of applications(s):
>
> package version w/ SPF w/o SPF improve(%)
> ------------------------------------------------------------------
> Baidu maps 10.13.3 0.887 0.98 9.49
> Taobao 8.4.0.35 1.227 1.293 5.10
> Meituan 9.12.401 1.107 1.543 28.26
> WeChat 7.0.3 2.353 2.68 12.20
> Honor of Kings 1.43.1.6 6.63 6.713 1.24
That's great news, thanks for reporting this!
>
> By the way, we have verified our platforms with those patches and
> achieved the goal of mass production.
Another good news!
For my information, what is your targeted hardware?
Cheers,
Laurent.
^ permalink raw reply
* Re: [PATCH v5 19/26] powerpc/book3s64/kuap: Move KUAP related function outside radix
From: Michael Ellerman @ 2020-07-06 12:41 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-20-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> The next set of patches adds support for kuap with hash translation.
That's no longer true of this series.
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index 0d72c0246052..e93b65a0e6e7 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -199,6 +200,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
This makes this code depend on CONFIG_PPC_MEM_KEYS=y, which it didn't
used to.
That risks breaking people's existing .configs, if they have
PPC_MEM_KEYS=n they will now lose KUAP.
And I'm not convinced the two features should be tied together, at least
at the user-visible Kconfig level.
cheers
^ permalink raw reply
* Re: [PATCH v5 13/26] powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY
From: Michael Ellerman @ 2020-07-06 13:10 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-14-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Parse storage keys related device tree entry in early_init_devtree
> and enable MMU feature MMU_FTR_PKEY if pkeys are supported.
>
> MMU feature is used instead of CPU feature because this enables us
> to group MMU_FTR_KUAP and MMU_FTR_PKEY in asm feature fixup code.
Subject should probably be "Add MMU_FTR_PKEY".
> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
> index f4ac25d4df05..72966d3d8f64 100644
> --- a/arch/powerpc/include/asm/mmu.h
> +++ b/arch/powerpc/include/asm/mmu.h
> @@ -23,6 +23,7 @@
>
> /* Radix page table supported and enabled */
> #define MMU_FTR_TYPE_RADIX ASM_CONST(0x00000040)
> +#define MMU_FTR_PKEY ASM_CONST(0x00000080)
It's not a type, so it should be with the individual feature bits below:
> /*
> * Individual features below.
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 6a3bac357e24..6d70797352d8 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -815,6 +815,11 @@ void __init early_init_devtree(void *params)
> /* Now try to figure out if we are running on LPAR and so on */
> pseries_probe_fw_features();
>
> + /*
> + * Initialize pkey features and default AMR/IAMR values
> + */
> + pkey_early_init_devtree();
Not your fault, but the fact that we're having to do more and more
initialisation this early, based on the flat device tree, makes me
wonder if we need to rethink how we're doing the CPU/MMU feature setup.
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index 0ff59acdbb84..bbba9c601e14 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -38,38 +39,45 @@ static int execute_only_key = 2;
> #define PKEY_REG_BITS (sizeof(u64) * 8)
> #define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))
>
> +static int __init dt_scan_storage_keys(unsigned long node,
> + const char *uname, int depth,
> + void *data)
> +{
> + const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> + const __be32 *prop;
> + int pkeys_total;
> +
> + /* We are scanning "cpu" nodes only */
> + if (type == NULL || strcmp(type, "cpu") != 0)
> + return 0;
> +
> + prop = of_get_flat_dt_prop(node, "ibm,processor-storage-keys", NULL);
> + if (!prop)
> + return 0;
> + pkeys_total = be32_to_cpu(prop[0]);
> + return pkeys_total;
That's not really the way the return value is meant to be used for these
scanning functions.
The usual return values are 0 to keep scanning and !0 means we've found
the node we're looking for and we should stop scanning.
Doing it this way means if you find 0 pkeys it will keep scanning.
Instead you should pass &pkeys_total as the data pointer and set that.
> +}
> +
> static int scan_pkey_feature(void)
> {
> - u32 vals[2];
> - int pkeys_total = 0;
> - struct device_node *cpu;
> + int pkeys_total;
>
> /*
> * Pkey is not supported with Radix translation.
> */
> - if (radix_enabled())
> + if (early_radix_enabled())
> return 0;
>
> - cpu = of_find_node_by_type(NULL, "cpu");
> - if (!cpu)
> - return 0;
> + pkeys_total = of_scan_flat_dt(dt_scan_storage_keys, NULL);
> + if (pkeys_total == 0) {
>
> - if (of_property_read_u32_array(cpu,
> - "ibm,processor-storage-keys", vals, 2) == 0) {
> - /*
> - * Since any pkey can be used for data or execute, we will
> - * just treat all keys as equal and track them as one entity.
> - */
> - pkeys_total = vals[0];
> - /* Should we check for IAMR support FIXME!! */
???
> - } else {
This loses the ability to differentiate between no storage-keys property
at all vs a property that specifies zero keys, which I don't think is a
good change.
> /*
> * Let's assume 32 pkeys on P8 bare metal, if its not defined by device
> * tree. We make this exception since skiboot forgot to expose this
> * property on power8.
> */
> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
> - cpu_has_feature(CPU_FTRS_POWER8))
> + early_cpu_has_feature(CPU_FTRS_POWER8))
> pkeys_total = 32;
That's not how cpu_has_feature() works, we'll need to fix that.
cheers
^ permalink raw reply
* [PATCH] powerpc/spufs: add CONFIG_COREDUMP dependency
From: Arnd Bergmann @ 2020-07-06 13:22 UTC (permalink / raw)
To: Arnd Bergmann, Michael Ellerman
Cc: kernel test robot, linux-kernel, Paul Mackerras, Jeremy Kerr,
linuxppc-dev, Christoph Hellwig
The kernel test robot pointed out a slightly different error message
after recent commit 5456ffdee666 ("powerpc/spufs: simplify spufs core
dumping") to spufs for a configuration that never worked:
powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in function `.spufs_proxydma_info_dump':
>> file.c:(.text+0x4c68): undefined reference to `.dump_emit'
powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in function `.spufs_dma_info_dump':
file.c:(.text+0x4d70): undefined reference to `.dump_emit'
powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in function `.spufs_wbox_info_dump':
file.c:(.text+0x4df4): undefined reference to `.dump_emit'
Add a Kconfig dependency to prevent this from happening again.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/powerpc/platforms/cell/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/cell/Kconfig b/arch/powerpc/platforms/cell/Kconfig
index 0f7c8241912b..f2ff359041ee 100644
--- a/arch/powerpc/platforms/cell/Kconfig
+++ b/arch/powerpc/platforms/cell/Kconfig
@@ -44,6 +44,7 @@ config SPU_FS
tristate "SPU file system"
default m
depends on PPC_CELL
+ depends on COREDUMP
select SPU_BASE
help
The SPU file system is used to access Synergistic Processing
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v5 13/26] powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY
From: Aneesh Kumar K.V @ 2020-07-06 14:09 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <878sfw6b7v.fsf@mpe.ellerman.id.au>
On 7/6/20 6:40 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Parse storage keys related device tree entry in early_init_devtree
>> and enable MMU feature MMU_FTR_PKEY if pkeys are supported.
>>
>> MMU feature is used instead of CPU feature because this enables us
>> to group MMU_FTR_KUAP and MMU_FTR_PKEY in asm feature fixup code.
>
> Subject should probably be "Add MMU_FTR_PKEY".
>
>> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
>> index f4ac25d4df05..72966d3d8f64 100644
>> --- a/arch/powerpc/include/asm/mmu.h
>> +++ b/arch/powerpc/include/asm/mmu.h
>> @@ -23,6 +23,7 @@
>>
>> /* Radix page table supported and enabled */
>> #define MMU_FTR_TYPE_RADIX ASM_CONST(0x00000040)
>> +#define MMU_FTR_PKEY ASM_CONST(0x00000080)
>
> It's not a type, so it should be with the individual feature bits below:
>
We don't have free bit in the other group. For now i will move this to
modified arch/powerpc/include/asm/mmu.h
@@ -23,12 +23,15 @@
/* Radix page table supported and enabled */
#define MMU_FTR_TYPE_RADIX ASM_CONST(0x00000040)
-#define MMU_FTR_PKEY ASM_CONST(0x00000080)
/*
* Individual features below.
*/
+/*
+ * Support for memory protection keys.
+ */
+#define MMU_FTR_PKEY ASM_CONST(0x00001000)
/*
* Support for 68 bit VA space. We added that from ISA 2.05
*/
>> /*
>> * Individual features below.
>
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index 6a3bac357e24..6d70797352d8 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -815,6 +815,11 @@ void __init early_init_devtree(void *params)
>> /* Now try to figure out if we are running on LPAR and so on */
>> pseries_probe_fw_features();
>>
>> + /*
>> + * Initialize pkey features and default AMR/IAMR values
>> + */
>> + pkey_early_init_devtree();
>
> Not your fault, but the fact that we're having to do more and more
> initialisation this early, based on the flat device tree, makes me
> wonder if we need to rethink how we're doing the CPU/MMU feature setup.
>
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index 0ff59acdbb84..bbba9c601e14 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -38,38 +39,45 @@ static int execute_only_key = 2;
>> #define PKEY_REG_BITS (sizeof(u64) * 8)
>> #define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))
>>
>> +static int __init dt_scan_storage_keys(unsigned long node,
>> + const char *uname, int depth,
>> + void *data)
>> +{
>> + const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
>> + const __be32 *prop;
>> + int pkeys_total;
>> +
>> + /* We are scanning "cpu" nodes only */
>> + if (type == NULL || strcmp(type, "cpu") != 0)
>> + return 0;
>> +
>> + prop = of_get_flat_dt_prop(node, "ibm,processor-storage-keys", NULL);
>> + if (!prop)
>> + return 0;
>> + pkeys_total = be32_to_cpu(prop[0]);
>> + return pkeys_total;
>
> That's not really the way the return value is meant to be used for these
> scanning functions.
>
> The usual return values are 0 to keep scanning and !0 means we've found
> the node we're looking for and we should stop scanning.
>
> Doing it this way means if you find 0 pkeys it will keep scanning.
>
> Instead you should pass &pkeys_total as the data pointer and set that.
>
>> +}
>> +
done
modified arch/powerpc/mm/book3s64/pkeys.c
@@ -52,7 +52,7 @@ static int __init dt_scan_storage_keys(unsigned long node,
{
const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
const __be32 *prop;
- int pkeys_total;
+ int *pkeys_total = (int *) data;
/* We are scanning "cpu" nodes only */
if (type == NULL || strcmp(type, "cpu") != 0)
@@ -61,12 +61,13 @@ static int __init dt_scan_storage_keys(unsigned long
node,
prop = of_get_flat_dt_prop(node, "ibm,processor-storage-keys", NULL);
if (!prop)
return 0;
- pkeys_total = be32_to_cpu(prop[0]);
- return pkeys_total;
+ *pkeys_total = be32_to_cpu(prop[0]);
+ return 1;
}
static int scan_pkey_feature(void)
{
+ int ret;
int pkeys_total;
/*
@@ -75,8 +76,8 @@ static int scan_pkey_feature(void)
if (early_radix_enabled())
return 0;
- pkeys_total = of_scan_flat_dt(dt_scan_storage_keys, NULL);
- if (pkeys_total == 0) {
+ ret = of_scan_flat_dt(dt_scan_storage_keys, &pkeys_total);
+ if (ret == 0) {
/*
* Let's assume 32 pkeys on P8/P9 bare metal, if its not defined by
device
@@ -87,7 +88,6 @@ static int scan_pkey_feature(void)
(early_cpu_has_feature(CPU_FTR_ARCH_207S) ||
early_cpu_has_feature(CPU_FTR_ARCH_300)))
pkeys_total = 32;
-
}
/*
>> static int scan_pkey_feature(void)
>> {
>> - u32 vals[2];
>> - int pkeys_total = 0;
>> - struct device_node *cpu;
>> + int pkeys_total;
>>
>> /*
>> * Pkey is not supported with Radix translation.
>> */
>> - if (radix_enabled())
>> + if (early_radix_enabled())
>> return 0;
>>
>> - cpu = of_find_node_by_type(NULL, "cpu");
>> - if (!cpu)
>> - return 0;
>> + pkeys_total = of_scan_flat_dt(dt_scan_storage_keys, NULL);
>> + if (pkeys_total == 0) {
>>
>> - if (of_property_read_u32_array(cpu,
>> - "ibm,processor-storage-keys", vals, 2) == 0) {
>> - /*
>> - * Since any pkey can be used for data or execute, we will
>> - * just treat all keys as equal and track them as one entity.
>> - */
>> - pkeys_total = vals[0];
>> - /* Should we check for IAMR support FIXME!! */
>
> ???
The device tree allows us to have different count for both AMR and IAMR.
The current code skip that. I guess i added a comment in earlier patch
to check that whether we need to handle different AMR and IAMR counts.
The same comment get dropped here.
>
>> - } else {
>
> This loses the ability to differentiate between no storage-keys property
> at all vs a property that specifies zero keys, which I don't think is a
> good change.
>
>> /*
>> * Let's assume 32 pkeys on P8 bare metal, if its not defined by device
>> * tree. We make this exception since skiboot forgot to expose this
>> * property on power8.
>> */
>> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
>> - cpu_has_feature(CPU_FTRS_POWER8))
>> + early_cpu_has_feature(CPU_FTRS_POWER8))
>> pkeys_total = 32;
>
> That's not how cpu_has_feature() works, we'll need to fix that.
>
> cheers
>
I did a separate patch to handle that which switch the above to
/*
* Let's assume 32 pkeys on P8/P9 bare metal, if its not defined by device
* tree. We make this exception since skiboot forgot to expose this
* property on power8/9.
*/
if (!firmware_has_feature(FW_FEATURE_LPAR) &&
(early_cpu_has_feature(CPU_FTR_ARCH_207S) ||
early_cpu_has_feature(CPU_FTR_ARCH_300)))
pkeys_total = 32;
^ permalink raw reply
* Re: [PATCH v5 18/26] powerpc/book3s64/keys/kuap: Reset AMR/IAMR values on kexec
From: Aneesh Kumar K.V @ 2020-07-06 14:39 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87h7uk6d3m.fsf@mpe.ellerman.id.au>
On 7/6/20 5:59 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> As we kexec across kernels that use AMR/IAMR for different purposes
>> we need to ensure that new kernels get kexec'd with a reset value
>> of AMR/IAMR. For ex: the new kernel can 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.
>
> I like the idea of cleaning this stuff up.
>
> But I think tying it into kup/kuep/kuap and the MMU features and ifdefs
> and so on is overly complicated.
>
> We should just have eg:
>
> void reset_sprs(void)
> {
> if (early_cpu_has_feature(CPU_FTR_ARCH_206)) {
> mtspr(SPRN_AMR, 0);
> mtspr(SPRN_UAMOR, 0);
> }
>
> if (early_cpu_has_feature(CPU_FTR_ARCH_207S)) {
> mtspr(SPRN_IAMR, 0);
> }
> }
>
> And call that from the kexec paths.
>
Will fix. Should that be early_cpu_has_feature()? cpu_has_feature()
should work there right?
-aneesh
^ permalink raw reply
* Re: [PATCH v5 19/26] powerpc/book3s64/kuap: Move KUAP related function outside radix
From: Aneesh Kumar K.V @ 2020-07-06 14:41 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87eepo6cjm.fsf@mpe.ellerman.id.au>
On 7/6/20 6:11 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> The next set of patches adds support for kuap with hash translation.
>
> That's no longer true of this series.
>
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index 0d72c0246052..e93b65a0e6e7 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -199,6 +200,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
>
> This makes this code depend on CONFIG_PPC_MEM_KEYS=y, which it didn't
> used to.
>
> That risks breaking people's existing .configs, if they have
> PPC_MEM_KEYS=n they will now lose KUAP.
>
> And I'm not convinced the two features should be tied together, at least
> at the user-visible Kconfig level.
>
That simplifies the addition of hash kuap a lot. Especially in the
exception entry and return paths. I did try to consider them as
independent options. But then the feature fixup in asm code gets
unnecessarily complicated. Also the UAMOR handling also get complicated.
-aneesh
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Andi Kleen @ 2020-07-06 16:08 UTC (permalink / raw)
To: Michal Hocko
Cc: Gautham R Shenoy, Srikar Dronamraju, David Hildenbrand,
linuxppc-dev, linux-kernel, linux-mm, Satheesh Rajendran,
Mel Gorman, Kirill A. Shutemov, Andrew Morton,
Michal Suchánek, Linus Torvalds, Christopher Lameter,
Vlastimil Babka
In-Reply-To: <20200703092414.GR18446@dhcp22.suse.cz>
> > What's the point of this indirection other than another way of avoiding
> > empty node 0?
>
> Honestly, I do not have any idea. I've traced it down to
> Author: Andi Kleen <ak@suse.de>
> Date: Tue Jan 11 15:35:48 2005 -0800
I don't remember all the details, and I can't even find the commit
(is it in linux-historic?).
But AFAIK there's no guarantee PXMs are small and continuous, so it
seemed better to have a clean zero based space.
Back then we had a lot of problems with buggy SRAT tables in BIOS,
so we really tried to avoid trusting the BIOS as much as possible.
-Andi
^ permalink raw reply
* Re: [PATCH v5 13/26] powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY
From: Aneesh Kumar K.V @ 2020-07-06 17:17 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <cddb4987-860f-f4be-43b0-f164031f9f6a@linux.ibm.com>
>>
>>> /*
>>> * Let's assume 32 pkeys on P8 bare metal, if its not
>>> defined by device
>>> * tree. We make this exception since skiboot forgot to
>>> expose this
>>> * property on power8.
>>> */
>>> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
>>> - cpu_has_feature(CPU_FTRS_POWER8))
>>> + early_cpu_has_feature(CPU_FTRS_POWER8))
>>> pkeys_total = 32;
>>
>> That's not how cpu_has_feature() works, we'll need to fix that.
>>
>> cheers
>>
>
> I did a separate patch to handle that which switch the above to
>
> /*
> * Let's assume 32 pkeys on P8/P9 bare metal, if its not
> defined by device
> * tree. We make this exception since skiboot forgot to expose
> this
> * property on power8/9.
> */
> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
> (early_cpu_has_feature(CPU_FTR_ARCH_207S) ||
> early_cpu_has_feature(CPU_FTR_ARCH_300)))
> pkeys_total = 32;
>
We should do a PVR check here i guess.
ret = of_scan_flat_dt(dt_scan_storage_keys, &pkeys_total);
if (ret == 0) {
/*
* Let's assume 32 pkeys on P8/P9 bare metal, if its not defined by device
* tree. We make this exception since skiboot forgot to expose this
* property on power8/9.
*/
if (!firmware_has_feature(FW_FEATURE_LPAR) &&
(pvr_version_is(PVR_POWER8) || pvr_version_is(PVR_POWER9)))
pkeys_total = 32;
}
-aneesh
^ permalink raw reply
* Re: [PATCH] kbuild: introduce ccflags-remove-y and asflags-remove-y
From: Masahiro Yamada @ 2020-07-06 18:14 UTC (permalink / raw)
To: Anders Roxell
Cc: Michal Marek, Yoshinori Sato, Linux Kbuild mailing list,
Linux-sh list, Linux Kernel Mailing List, Steven Rostedt,
Russell King, linuxppc-dev, Ingo Molnar, Paul Mackerras,
Sami Tolvanen, Rich Felker, linux-arm-kernel
In-Reply-To: <CADYN=9JnzPC6Ja9s3_01k-CDTSuxKBMRdrqU5rqp0xw1r9XpRw@mail.gmail.com>
Hi Anders,
On Mon, Jul 6, 2020 at 8:24 PM Anders Roxell <anders.roxell@linaro.org> wrote:
>
> The full log can be found here [1].
>
> Without this patch for 'trace_selftest_dynamic' for instance, CC_FLAGS_FTRACE
> was removed from kernel/trace/*, and then added back to
> kernel/trace/trace_selftest_dynamic.
> While with this patch it looks like we add the flag (even though it is
> already there), and then
> removes the flag for all files in kernel/trace/* .
You are right.
I will drop this patch,
and send v2.
Thank you.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v3 0/6] powerpc: queued spinlocks and rwlocks
From: Waiman Long @ 2020-07-06 18:39 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel, kvm-ppc,
virtualization, Ingo Molnar, Will Deacon
In-Reply-To: <20200706043540.1563616-1-npiggin@gmail.com>
On 7/6/20 12:35 AM, Nicholas Piggin wrote:
> v3 is updated to use __pv_queued_spin_unlock, noticed by Waiman (thank you).
>
> Thanks,
> Nick
>
> Nicholas Piggin (6):
> powerpc/powernv: must include hvcall.h to get PAPR defines
> powerpc/pseries: move some PAPR paravirt functions to their own file
> powerpc: move spinlock implementation to simple_spinlock
> powerpc/64s: implement queued spinlocks and rwlocks
> powerpc/pseries: implement paravirt qspinlocks for SPLPAR
> powerpc/qspinlock: optimised atomic_try_cmpxchg_lock that adds the
> lock hint
>
> arch/powerpc/Kconfig | 13 +
> arch/powerpc/include/asm/Kbuild | 2 +
> arch/powerpc/include/asm/atomic.h | 28 ++
> arch/powerpc/include/asm/paravirt.h | 89 +++++
> arch/powerpc/include/asm/qspinlock.h | 91 ++++++
> arch/powerpc/include/asm/qspinlock_paravirt.h | 7 +
> arch/powerpc/include/asm/simple_spinlock.h | 292 +++++++++++++++++
> .../include/asm/simple_spinlock_types.h | 21 ++
> arch/powerpc/include/asm/spinlock.h | 308 +-----------------
> arch/powerpc/include/asm/spinlock_types.h | 17 +-
> arch/powerpc/lib/Makefile | 3 +
> arch/powerpc/lib/locks.c | 12 +-
> arch/powerpc/platforms/powernv/pci-ioda-tce.c | 1 +
> arch/powerpc/platforms/pseries/Kconfig | 5 +
> arch/powerpc/platforms/pseries/setup.c | 6 +-
> include/asm-generic/qspinlock.h | 4 +
> 16 files changed, 577 insertions(+), 322 deletions(-)
> create mode 100644 arch/powerpc/include/asm/paravirt.h
> create mode 100644 arch/powerpc/include/asm/qspinlock.h
> create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h
> create mode 100644 arch/powerpc/include/asm/simple_spinlock.h
> create mode 100644 arch/powerpc/include/asm/simple_spinlock_types.h
>
This patch looks OK to me.
I had run some microbenchmark on powerpc system with or w/o the patch.
On a 2-socket 160-thread SMT4 POWER9 system (not virtualized):
5.8.0-rc4
=========
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 160, Min/Mean/Max = 77,665/90,153/106,895
Threads = 160, Total Rate = 1,441,759 op/s; Percpu Rate = 9,011 op/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 160, Min/Mean/Max = 47,879/53,807/63,689
Threads = 160, Total Rate = 860,192 op/s; Percpu Rate = 5,376 op/s
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 80, Min/Mean/Max = 242,907/319,514/463,161
Threads = 80, Total Rate = 2,555 kop/s; Percpu Rate = 32 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 80, Min/Mean/Max = 146,161/187,474/259,270
Threads = 80, Total Rate = 1,498 kop/s; Percpu Rate = 19 kop/s
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 40, Min/Mean/Max = 646,639/1,000,817/1,455,205
Threads = 40, Total Rate = 4,001 kop/s; Percpu Rate = 100 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 40, Min/Mean/Max = 402,165/597,132/814,555
Threads = 40, Total Rate = 2,388 kop/s; Percpu Rate = 60 kop/s
5.8.0-rc4-qlock+
================
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 160, Min/Mean/Max = 123,835/124,580/124,587
Threads = 160, Total Rate = 1,992 kop/s; Percpu Rate = 12 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 160, Min/Mean/Max = 254,210/264,714/276,784
Threads = 160, Total Rate = 4,231 kop/s; Percpu Rate = 26 kop/s
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 80, Min/Mean/Max = 599,715/603,397/603,450
Threads = 80, Total Rate = 4,825 kop/s; Percpu Rate = 60 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 80, Min/Mean/Max = 492,687/525,224/567,456
Threads = 80, Total Rate = 4,199 kop/s; Percpu Rate = 52 kop/s
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 40, Min/Mean/Max = 1,325,623/1,325,628/1,325,636
Threads = 40, Total Rate = 5,299 kop/s; Percpu Rate = 132 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 40, Min/Mean/Max = 1,249,731/1,292,977/1,342,815
Threads = 40, Total Rate = 5,168 kop/s; Percpu Rate = 129 kop/s
On systems on large number of cpus, qspinlock lock is faster and more fair.
With some tuning, we may be able to squeeze out more performance.
Cheers,
Longman
^ permalink raw reply
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