* Re: [PATCH v3 05/17] arm: Convert to GENERIC_CMDLINE
From: Rob Herring @ 2021-03-26 18:22 UTC (permalink / raw)
To: Christophe Leroy, Nicolas Pitre
Cc: open list:GENERIC INCLUDE/ASM HEADER FILES, devicetree,
microblaze, Daniel Gimpelevich, linux-xtensa, SH-Linux,
Will Deacon, X86 ML, linux-kernel@vger.kernel.org, nios2,
open list:MIPS, Openrisc, linux-hexagon, sparclinux, linux-riscv,
linuxppc-dev, linux-arm-kernel, Daniel Walker
In-Reply-To: <c18ef8f7-8e79-a9d3-3853-f8b992a4fc93@csgroup.eu>
+Nico who added the line in question.
On Fri, Mar 26, 2021 at 9:50 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 26/03/2021 à 16:47, Rob Herring a écrit :
> > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >> This converts the architecture to GENERIC_CMDLINE.
> >>
> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> >> ---
> >> arch/arm/Kconfig | 38 +----------------------------------
> >> arch/arm/kernel/atags_parse.c | 15 +++++---------
> >> 2 files changed, 6 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 5da96f5df48f..67bc75f2da81 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -50,6 +50,7 @@ config ARM
> >> select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
> >> select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
> >> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> >> + select GENERIC_CMDLINE if ATAGS
> >
> > Don't we need this enabled for !ATAGS (i.e. DT boot)?
> >
> > Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE?
> >
>
> Don't know.
>
> Today ARM has:
>
> choice
> prompt "Kernel command line type" if CMDLINE != ""
> default CMDLINE_FROM_BOOTLOADER
> depends on ATAGS
I think that's a mistake. In a DT only case (no ATAGS), we'll get
different behaviour (in fdt.c) depending if CONFIG_ATAGS is enabled or
not. Note that at the time (2012) the above was added, the DT code
only supported CONFIG_CMDLINE and CONFIG_CMDLINE_FORCE.
CONFIG_CMDLINE_EXTEND was only added in 2016. And that has different
behavior for ATAGS vs. DT. In summary, it's a mess. We should drop the
depends either before this patch or just as part of this patch IMO.
I'd go with the latter given CONFIG_ATAGS is default y and enabled for
common configs. Without that, it looks like CONFIG_CMDLINE disappears
from menuconfig for at91_dt_defconfig.
Also, I think this code should be refactored a bit to eliminate
default_command_line. Instead, we should just save a pointer to the
ATAGS command line string, and then call cmdline_build here instead of
doing the extra copy:
/* parse_early_param needs a boot_command_line */
strlcpy(boot_command_line, from, COMMAND_LINE_SIZE);
Rob
^ permalink raw reply
* Re: VDSO ELF header
From: Christophe Leroy @ 2021-03-26 18:40 UTC (permalink / raw)
To: Dmitry Safonov, Michael Ellerman, Laurent Dufour; +Cc: linuxppc-dev
In-Reply-To: <46e49770-04ee-2455-f2f7-b3915ceea38a@gmail.com>
Le 26/03/2021 à 18:11, Dmitry Safonov a écrit :
> On 3/26/21 5:07 PM, Christophe Leroy wrote:
>> No, the problem is that user access has to be allowed for the flush()
>>
>> A hacky solution would be to call user_access_begin() , will test that
>> later
>
> Yeah, cool.
>
> Will it be fine if I send the vvar patch with your Tested-by?
>
Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
With the user access fixed on the flush, it sigreturn_vdso selftest is a success. I'll send a patch
for it in the coming days.
What is the status of your series which adds generic vdso_base tracking ?
Christophe
^ permalink raw reply
* Re: VDSO ELF header
From: Dmitry Safonov @ 2021-03-26 18:54 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Laurent Dufour; +Cc: linuxppc-dev
In-Reply-To: <ec257b8b-d611-bbf4-86f3-bb0b758dbeaf@csgroup.eu>
On 3/26/21 6:40 PM, Christophe Leroy wrote:
>
>
> Le 26/03/2021 à 18:11, Dmitry Safonov a écrit :
>> On 3/26/21 5:07 PM, Christophe Leroy wrote:
>>> No, the problem is that user access has to be allowed for the flush()
>>>
>>> A hacky solution would be to call user_access_begin() , will test that
>>> later
>>
>> Yeah, cool.
>>
>> Will it be fine if I send the vvar patch with your Tested-by?
>>
>
> Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Thank you!
I'll properly submit it shortly..
> With the user access fixed on the flush, it sigreturn_vdso selftest is a
> success. I'll send a patch for it in the coming days.
Nice!
> What is the status of your series which adds generic vdso_base tracking ?
Yeah, I was doing a new version of patches and I always was unsatisfied
by the result and stuck between "good" and "best" (more code rewriting).
And then had some other work to finish.
I'll try to finish and send it next week, thanks for pinging :-)
--
Dmitry
^ permalink raw reply
* [PATCH] powerpc/vdso: Separate vvar vma from vdso
From: Dmitry Safonov @ 2021-03-26 19:17 UTC (permalink / raw)
To: linux-kernel
Cc: Dmitry Safonov, Dmitry Safonov, Andrei Vagin, Paul Mackerras,
stable, Andy Lutomirski, Laurent Dufour, linuxppc-dev
Since commit 511157ab641e ("powerpc/vdso: Move vdso datapage up front")
VVAR page is in front of the VDSO area. In result it breaks CRIU
(Checkpoint Restore In Userspace) [1], where CRIU expects that "[vdso]"
from /proc/../maps points at ELF/vdso image, rather than at VVAR data page.
Laurent made a patch to keep CRIU working (by reading aux vector).
But I think it still makes sence to separate two mappings into different
VMAs. It will also make ppc64 less "special" for userspace and as
a side-bonus will make VVAR page un-writable by debugger (which previously
would COW page and can be unexpected).
I opportunistically Cc stable on it: I understand that usually such
stuff isn't a stable material, but that will allow us in CRIU have
one workaround less that is needed just for one release (v5.11) on
one platform (ppc64), which we otherwise have to maintain.
I wouldn't go as far as to say that the commit 511157ab641e is ABI
regression as no other userspace got broken, but I'd really appreciate
if it gets backported to v5.11 after v5.12 is released, so as not
to complicate already non-simple CRIU-vdso code. Thanks!
Cc: Andrei Vagin <avagin@gmail.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: stable@vger.kernel.org # v5.11
[1]: https://github.com/checkpoint-restore/criu/issues/1417
Signed-off-by: Dmitry Safonov <dima@arista.com>
Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/mmu_context.h | 2 +-
arch/powerpc/kernel/vdso.c | 54 +++++++++++++++++++-------
2 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 652ce85f9410..4bc45d3ed8b0 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -263,7 +263,7 @@ extern void arch_exit_mmap(struct mm_struct *mm);
static inline void arch_unmap(struct mm_struct *mm,
unsigned long start, unsigned long end)
{
- unsigned long vdso_base = (unsigned long)mm->context.vdso - PAGE_SIZE;
+ unsigned long vdso_base = (unsigned long)mm->context.vdso;
if (start <= vdso_base && vdso_base < end)
mm->context.vdso = NULL;
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index e839a906fdf2..b14907209822 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -55,10 +55,10 @@ static int vdso_mremap(const struct vm_special_mapping *sm, struct vm_area_struc
{
unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
- if (new_size != text_size + PAGE_SIZE)
+ if (new_size != text_size)
return -EINVAL;
- current->mm->context.vdso = (void __user *)new_vma->vm_start + PAGE_SIZE;
+ current->mm->context.vdso = (void __user *)new_vma->vm_start;
return 0;
}
@@ -73,6 +73,10 @@ static int vdso64_mremap(const struct vm_special_mapping *sm, struct vm_area_str
return vdso_mremap(sm, new_vma, &vdso64_end - &vdso64_start);
}
+static struct vm_special_mapping vvar_spec __ro_after_init = {
+ .name = "[vvar]",
+};
+
static struct vm_special_mapping vdso32_spec __ro_after_init = {
.name = "[vdso]",
.mremap = vdso32_mremap,
@@ -89,11 +93,11 @@ static struct vm_special_mapping vdso64_spec __ro_after_init = {
*/
static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
{
- struct mm_struct *mm = current->mm;
+ unsigned long vdso_size, vdso_base, mappings_size;
struct vm_special_mapping *vdso_spec;
+ unsigned long vvar_size = PAGE_SIZE;
+ struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
- unsigned long vdso_size;
- unsigned long vdso_base;
if (is_32bit_task()) {
vdso_spec = &vdso32_spec;
@@ -110,8 +114,8 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
vdso_base = 0;
}
- /* Add a page to the vdso size for the data page */
- vdso_size += PAGE_SIZE;
+ mappings_size = vdso_size + vvar_size;
+ mappings_size += (VDSO_ALIGNMENT - 1) & PAGE_MASK;
/*
* pick a base address for the vDSO in process space. We try to put it
@@ -119,9 +123,7 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
* and end up putting it elsewhere.
* Add enough to the size so that the result can be aligned.
*/
- vdso_base = get_unmapped_area(NULL, vdso_base,
- vdso_size + ((VDSO_ALIGNMENT - 1) & PAGE_MASK),
- 0, 0);
+ vdso_base = get_unmapped_area(NULL, vdso_base, mappings_size, 0, 0);
if (IS_ERR_VALUE(vdso_base))
return vdso_base;
@@ -133,7 +135,13 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
* install_special_mapping or the perf counter mmap tracking code
* will fail to recognise it as a vDSO.
*/
- mm->context.vdso = (void __user *)vdso_base + PAGE_SIZE;
+ mm->context.vdso = (void __user *)vdso_base + vvar_size;
+
+ vma = _install_special_mapping(mm, vdso_base, vvar_size,
+ VM_READ | VM_MAYREAD | VM_IO |
+ VM_DONTDUMP | VM_PFNMAP, &vvar_spec);
+ if (IS_ERR(vma))
+ return PTR_ERR(vma);
/*
* our vma flags don't have VM_WRITE so by default, the process isn't
@@ -145,9 +153,12 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
* It's fine to use that for setting breakpoints in the vDSO code
* pages though.
*/
- vma = _install_special_mapping(mm, vdso_base, vdso_size,
+ vma = _install_special_mapping(mm, vdso_base + vvar_size, vdso_size,
VM_READ | VM_EXEC | VM_MAYREAD |
VM_MAYWRITE | VM_MAYEXEC, vdso_spec);
+ if (IS_ERR(vma))
+ do_munmap(mm, vdso_base, vvar_size, NULL);
+
return PTR_ERR_OR_ZERO(vma);
}
@@ -249,11 +260,22 @@ static struct page ** __init vdso_setup_pages(void *start, void *end)
if (!pagelist)
panic("%s: Cannot allocate page list for VDSO", __func__);
- pagelist[0] = virt_to_page(vdso_data);
-
for (i = 0; i < pages; i++)
- pagelist[i + 1] = virt_to_page(start + i * PAGE_SIZE);
+ pagelist[i] = virt_to_page(start + i * PAGE_SIZE);
+
+ return pagelist;
+}
+
+static struct page ** __init vvar_setup_pages(void)
+{
+ struct page **pagelist;
+ /* .pages is NULL-terminated */
+ pagelist = kcalloc(2, sizeof(struct page *), GFP_KERNEL);
+ if (!pagelist)
+ panic("%s: Cannot allocate page list for VVAR", __func__);
+
+ pagelist[0] = virt_to_page(vdso_data);
return pagelist;
}
@@ -295,6 +317,8 @@ static int __init vdso_init(void)
if (IS_ENABLED(CONFIG_PPC64))
vdso64_spec.pages = vdso_setup_pages(&vdso64_start, &vdso64_end);
+ vvar_spec.pages = vvar_setup_pages();
+
smp_wmb();
return 0;
--
2.31.0
^ permalink raw reply related
* Re: [PATCH] powerpc/mm/book3s64: Use the correct storage key value when calling H_PROTECT
From: Murilo Opsfelder Araújo @ 2021-03-26 20:39 UTC (permalink / raw)
To: linuxppc-dev, mpe, Aneesh Kumar K.V; +Cc: Aneesh Kumar K.V
In-Reply-To: <20210326070755.304625-1-aneesh.kumar@linux.ibm.com>
On Friday, March 26, 2021 4:07:55 AM -03 Aneesh Kumar K.V wrote:
> H_PROTECT expect the flag value to include
> flags: AVPN, pp0, pp1, pp2, key0-key4, Noexec, CMO Option flags
>
> This patch updates hpte_updatepp() to fetch the storage key value from the
> linux page table and use the same in H_PROTECT hcall.
>
> native_hpte_updatepp() is not updated because the kernel doesn't clear the
> existing storage key value there. The kernel also doesn't use
> hpte_updatepp() callback for updating storage keys.
>
> This fixes the below kernel crash observed with KUAP enabled.
>
> BUG: Unable to handle kernel data access on write at 0xc009fffffc440000
> Faulting instruction address: 0xc0000000000b7030
> Key fault AMR: 0xfcffffffffffffff IAMR: 0xc0000077bc498100
> Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> .........
> CFAR: c000000000010100 DAR: c009fffffc440000 DSISR: 02200000 IRQMASK: 0
> ..........
> NIP [c0000000000b7030] memset+0x68/0x104
> LR [c0000000003ef00c] pcpu_alloc+0x54c/0xb50
> Call Trace:
> [c00000001c7534f0] [c0000000003ef01c] pcpu_alloc+0x55c/0xb50 (unreliable)
> [c00000001c753600] [c0000000008bb214] blk_stat_alloc_callback+0x94/0x150
> [c00000001c753650] [c0000000008b7a04]
> blk_mq_init_allocated_queue+0x64/0x560 [c00000001c7536b0]
> [c0000000008b8024] blk_mq_init_queue+0x54/0xb0 [c00000001c7536e0]
> [c000000000b87650] scsi_mq_alloc_queue+0x30/0xa0 [c00000001c753710]
> [c000000000b88b2c] scsi_alloc_sdev+0x1cc/0x300 [c00000001c7537b0]
> [c000000000b897b0] scsi_probe_and_add_lun+0xb50/0x1020 [c00000001c753950]
> [c000000000b8a35c] __scsi_scan_target+0x17c/0x790 [c00000001c753a80]
> [c000000000b8ab90] scsi_scan_channel+0x90/0xe0 [c00000001c753ad0]
> [c000000000b8ae48] scsi_scan_host_selected+0x148/0x1f0 [c00000001c753b60]
> [c000000000b8b31c] do_scan_async+0x2c/0x2a0
> [c00000001c753be0] [c000000000187a18] async_run_entry_fn+0x78/0x220
> [c00000001c753c70] [c000000000176a74] process_one_work+0x264/0x540
> [c00000001c753d10] [c000000000177338] worker_thread+0xa8/0x600
> [c00000001c753da0] [c0000000001807b0] kthread+0x190/0x1a0
> [c00000001c753e10] [c00000000000d8f0] ret_from_kernel_thread+0x5c/0x6c
>
> With KUAP enabled the kernel uses storage key 3 for all its translations.
> But as shown by the debug print, in this specific case we have the hash
> page table entry created with key value 0.
>
> [ 2.249497] Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194
>
> and DSISR indicates a key fault.
>
> This can happen due to parallel fault on the same EA by different CPUs
>
> CPU 0 CPU 1
> fault on X
>
> H_PAGE_BUSY set
> fault on X
>
> finish fault handling and
> clear H_PAGE_BUSY
> check for H_PAGE_BUSY
> continue with fault
handling.
>
> This implies CPU1 will end up calling hpte_updatepp for address X
> and the kernel updated the hash pte entry with key 0
>
> Fixes: d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3 for kernel mapping
> with hash translation")
>
> Debugged-by: Michael Ellerman <mpe@ellerman.id.au>
> Reported-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
I've tested this on top of commit db24726bfefa68c606947a86132591568a06bfb4
("Merge tag 'integrity-v5.12-fix' of git://git.kernel.org/pub/scm/linux/
kernel/git/zohar/linux-integrity"),
and the reported issue did not manifest.
Thank you, Michael and Aneesh, for the help.
Tested-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
--
Murilo
^ permalink raw reply
* Re: [PATCH V2 3/5] tools/perf: Add powerpc support for PERF_SAMPLE_WEIGHT_STRUCT
From: Athira Rajeev @ 2021-03-26 15:23 UTC (permalink / raw)
To: Jiri Olsa
Cc: ravi.bangoria, Madhavan Srinivasan, peterz, linux-kernel, acme,
linux-perf-users, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <YFuWb3S8p0ZGjmGu@krava>
[-- Attachment #1: Type: text/html, Size: 6519 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] exec: simplify the compat syscall handling
From: Eric W. Biederman @ 2021-03-26 21:22 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, linux-parisc, Arnd Bergmann, Brian Gerst,
x86, linux-mips, linux-kernel, linux-fsdevel, Luis Chamberlain,
Al Viro, sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20210326143831.1550030-4-hch@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> diff --git a/fs/exec.c b/fs/exec.c
> index 06e07278b456fa..b34c1eb9e7ad8e 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -391,47 +391,34 @@ static int bprm_mm_init(struct linux_binprm *bprm)
> return err;
> }
>
> -struct user_arg_ptr {
> -#ifdef CONFIG_COMPAT
> - bool is_compat;
> -#endif
> - union {
> - const char __user *const __user *native;
> -#ifdef CONFIG_COMPAT
> - const compat_uptr_t __user *compat;
> -#endif
> - } ptr;
> -};
> -
> -static const char __user *get_user_arg_ptr(struct user_arg_ptr argv, int nr)
> +static const char __user *
> +get_user_arg_ptr(const char __user *const __user *argv, int nr)
> {
> - const char __user *native;
> -
> -#ifdef CONFIG_COMPAT
> - if (unlikely(argv.is_compat)) {
> + if (in_compat_syscall()) {
> + const compat_uptr_t __user *compat_argv =
> + compat_ptr((unsigned long)argv);
Ouch! Passing a pointer around as the wrong type through the kernel!
Perhaps we should reduce everything to do_execveat and
do_execveat_compat. Then there would be no need for anything
to do anything odd with the pointer types.
I think the big change would be to factor out a copy_string out
of copy_strings, that performs all of the work once we know the proper
pointer value.
Casting pointers from one type to another scares me as one mistake means
we are doing something wrong and probably exploitable.
Eric
> compat_uptr_t compat;
>
> - if (get_user(compat, argv.ptr.compat + nr))
> + if (get_user(compat, compat_argv + nr))
> return ERR_PTR(-EFAULT);
> -
> return compat_ptr(compat);
> - }
> -#endif
> -
> - if (get_user(native, argv.ptr.native + nr))
> - return ERR_PTR(-EFAULT);
> + } else {
> + const char __user *native;
>
> - return native;
> + if (get_user(native, argv + nr))
> + return ERR_PTR(-EFAULT);
> + return native;
> + }
> }
>
^ permalink raw reply
* Re: [PATCH V2 1/5] powerpc/perf: Expose processor pipeline stage cycles using PERF_SAMPLE_WEIGHT_STRUCT
From: Michael Ellerman @ 2021-03-27 13:14 UTC (permalink / raw)
To: Arnaldo, Peter Zijlstra, Arnaldo Carvalho de Melo
Cc: ravi.bangoria, Athira Rajeev, kjain, linux-kernel,
linux-perf-users, Madhavan Srinivasan, jolsa, linuxppc-dev,
kan.liang
In-Reply-To: <2BAC42AE-6BD3-45EF-8867-1A15F25FE80B@gmail.com>
Arnaldo <arnaldo.melo@gmail.com> writes:
> On March 25, 2021 11:38:01 AM GMT-03:00, Peter Zijlstra <peterz@infradead.org> wrote:
>>On Thu, Mar 25, 2021 at 10:01:35AM -0300, Arnaldo Carvalho de Melo
>>wrote:.
>>> > > Also for CPU_FTR_ARCH_31, capture the two cycle counter
>>information in
>>> > > two 16 bit fields of perf_sample_weight structure.
>>> >
>>> > Changes looks fine to me.
>>> >
>>> > Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>>>
>>> So who will process the kernel bits? I'm merging the tooling parts,
>>
>>I was sorta expecting these to go through the powerpc tree. Let me know
>>if you want them in tip/perf/core instead.
>
> Shouldn't matter by which tree it gets upstream, as long as it gets picked :-)
I plan to take them, just haven't got around to it yet :}
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/vdso: Separate vvar vma from vdso
From: Christophe Leroy @ 2021-03-27 17:19 UTC (permalink / raw)
To: Dmitry Safonov, linux-kernel
Cc: Dmitry Safonov, stable, Andrei Vagin, Paul Mackerras,
Andy Lutomirski, Laurent Dufour, linuxppc-dev
In-Reply-To: <20210326191720.138155-1-dima@arista.com>
Le 26/03/2021 à 20:17, Dmitry Safonov a écrit :
> Since commit 511157ab641e ("powerpc/vdso: Move vdso datapage up front")
> VVAR page is in front of the VDSO area. In result it breaks CRIU
> (Checkpoint Restore In Userspace) [1], where CRIU expects that "[vdso]"
> from /proc/../maps points at ELF/vdso image, rather than at VVAR data page.
> Laurent made a patch to keep CRIU working (by reading aux vector).
> But I think it still makes sence to separate two mappings into different
> VMAs. It will also make ppc64 less "special" for userspace and as
> a side-bonus will make VVAR page un-writable by debugger (which previously
> would COW page and can be unexpected).
>
> I opportunistically Cc stable on it: I understand that usually such
> stuff isn't a stable material, but that will allow us in CRIU have
> one workaround less that is needed just for one release (v5.11) on
> one platform (ppc64), which we otherwise have to maintain.
Why is that a workaround, and why for one release only ? I think the solution proposed by Laurentto
use the aux vector AT_SYSINFO_EHDR should work with any past and future release.
> I wouldn't go as far as to say that the commit 511157ab641e is ABI
> regression as no other userspace got broken, but I'd really appreciate
> if it gets backported to v5.11 after v5.12 is released, so as not
> to complicate already non-simple CRIU-vdso code. Thanks!
>
> Cc: Andrei Vagin <avagin@gmail.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Laurent Dufour <ldufour@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: stable@vger.kernel.org # v5.11
> [1]: https://github.com/checkpoint-restore/criu/issues/1417
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
I tested it with sifreturn_vdso selftest and it worked, because that selftest doesn't involve VDSO data.
But if I do a mremap() on the VDSO text vma without remapping VVAR to keep the same distance between
the two vmas, gettimeofday() crashes. The reason is that the code obtains the address of the data by
calculating a fix difference from its own address with the below macro, the delta being resolved at
link time:
.macro get_datapage ptr
bcl 20, 31, .+4
999:
mflr \ptr
#if CONFIG_PPC_PAGE_SHIFT > 14
addis \ptr, \ptr, (_vdso_datapage - 999b)@ha
#endif
addi \ptr, \ptr, (_vdso_datapage - 999b)@l
.endm
So the datapage needs to remain at the same distance from the code at all time.
Wondering how the other architectures do to have two independant VMAs and be able to move one
independantly of the other.
Christophe
^ permalink raw reply
* Re: [PATCH v3 37/41] powerpc/32s: Move KUEP locking/unlocking in C
From: Christophe Leroy @ 2021-03-27 17:24 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4eadd873927e9a73c3d1dfe2f9497353465514cf.1615552867.git.christophe.leroy@csgroup.eu>
Le 12/03/2021 à 13:50, Christophe Leroy a écrit :
> This can be done in C, do it.
>
> Unrolling the loop gains approx. 15% performance.
>
> From now on, prepare_transfer_to_handler() is only for
> interrupts from kernel.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/book3s/32/kup.h | 31 -------------------
> arch/powerpc/include/asm/interrupt.h | 6 +++-
> arch/powerpc/include/asm/kup.h | 8 +++++
> arch/powerpc/kernel/entry_32.S | 16 ----------
> arch/powerpc/kernel/head_32.h | 3 ++
> arch/powerpc/kernel/head_booke.h | 3 ++
> arch/powerpc/kernel/interrupt.c | 4 +++
> arch/powerpc/mm/book3s32/Makefile | 1 +
> arch/powerpc/mm/book3s32/kuep.c | 38 ++++++++++++++++++++++++
> 9 files changed, 62 insertions(+), 48 deletions(-)
> create mode 100644 arch/powerpc/mm/book3s32/kuep.c
>
> diff --git a/arch/powerpc/mm/book3s32/kuep.c b/arch/powerpc/mm/book3s32/kuep.c
> new file mode 100644
> index 000000000000..c70532568a28
> --- /dev/null
> +++ b/arch/powerpc/mm/book3s32/kuep.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <asm/reg.h>
> +#include <asm/task_size_32.h>
> +#include <asm/mmu.h>
Need to add <asm/kup.h> to make Robot happy (missing prototypes of kuep_lock() and kuep_unlock())
> +
> +#define KUEP_UPDATE_TWO_USER_SEGMENTS(n) do { \
> + if (TASK_SIZE > ((n) << 28)) \
> + mtsr(val1, (n) << 28); \
> + if (TASK_SIZE > (((n) + 1) << 28)) \
> + mtsr(val2, ((n) + 1) << 28); \
> + val1 = (val1 + 0x222) & 0xf0ffffff; \
> + val2 = (val2 + 0x222) & 0xf0ffffff; \
> +} while (0)
> +
> +static __always_inline void kuep_update(u32 val)
> +{
> + int val1 = val;
> + int val2 = (val + 0x111) & 0xf0ffffff;
> +
> + KUEP_UPDATE_TWO_USER_SEGMENTS(0);
> + KUEP_UPDATE_TWO_USER_SEGMENTS(2);
> + KUEP_UPDATE_TWO_USER_SEGMENTS(4);
> + KUEP_UPDATE_TWO_USER_SEGMENTS(6);
> + KUEP_UPDATE_TWO_USER_SEGMENTS(8);
> + KUEP_UPDATE_TWO_USER_SEGMENTS(10);
> + KUEP_UPDATE_TWO_USER_SEGMENTS(12);
> + KUEP_UPDATE_TWO_USER_SEGMENTS(14);
> +}
> +
> +void kuep_lock(void)
> +{
> + kuep_update(mfsr(0) | SR_NX);
> +}
> +
> +void kuep_unlock(void)
> +{
> + kuep_update(mfsr(0) & ~SR_NX);
> +}
>
^ permalink raw reply
* Re: [PATCH] powerpc/vdso: Separate vvar vma from vdso
From: Dmitry Safonov @ 2021-03-27 17:43 UTC (permalink / raw)
To: Christophe Leroy, linux-kernel
Cc: Dmitry Safonov, stable, Andrei Vagin, Paul Mackerras,
Andy Lutomirski, Laurent Dufour, linuxppc-dev
In-Reply-To: <52562f46-6767-ba04-7301-04c6209fe4f1@csgroup.eu>
Hi Christophe,
On 3/27/21 5:19 PM, Christophe Leroy wrote:
[..]
>> I opportunistically Cc stable on it: I understand that usually such
>> stuff isn't a stable material, but that will allow us in CRIU have
>> one workaround less that is needed just for one release (v5.11) on
>> one platform (ppc64), which we otherwise have to maintain.
>
> Why is that a workaround, and why for one release only ? I think the
> solution proposed by Laurentto use the aux vector AT_SYSINFO_EHDR should
> work with any past and future release.
Yeah, I guess.
Previously, (before v5.11/power) all kernels had ELF start at "[vdso]"
VMA start, now we'll have to carry the offset in the VMA. Probably, not
the worst thing, but as it will be only for v5.11 release it can break,
so needs separate testing.
Kinda life was a bit easier without this additional code.
>> I wouldn't go as far as to say that the commit 511157ab641e is ABI
>> regression as no other userspace got broken, but I'd really appreciate
>> if it gets backported to v5.11 after v5.12 is released, so as not
>> to complicate already non-simple CRIU-vdso code. Thanks!
>>
>> Cc: Andrei Vagin <avagin@gmail.com>
>> Cc: Andy Lutomirski <luto@kernel.org>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: Laurent Dufour <ldufour@linux.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: stable@vger.kernel.org # v5.11
>> [1]: https://github.com/checkpoint-restore/criu/issues/1417
>> Signed-off-by: Dmitry Safonov <dima@arista.com>
>> Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>
> I tested it with sifreturn_vdso selftest and it worked, because that
> selftest doesn't involve VDSO data.
Thanks again on helping with testing it, I appreciate it!
> But if I do a mremap() on the VDSO text vma without remapping VVAR to
> keep the same distance between the two vmas, gettimeofday() crashes. The
> reason is that the code obtains the address of the data by calculating a
> fix difference from its own address with the below macro, the delta
> being resolved at link time:
>
> .macro get_datapage ptr
> bcl 20, 31, .+4
> 999:
> mflr \ptr
> #if CONFIG_PPC_PAGE_SHIFT > 14
> addis \ptr, \ptr, (_vdso_datapage - 999b)@ha
> #endif
> addi \ptr, \ptr, (_vdso_datapage - 999b)@l
> .endm
>
> So the datapage needs to remain at the same distance from the code at
> all time.
>
> Wondering how the other architectures do to have two independent VMAs
> and be able to move one independently of the other.
It's alright as far as I know. If userspace remaps vdso/vvar it should
be aware of this (CRIU keeps this in mind, also old vdso image is dumped
to compare on restore with the one that the host has).
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH 2/4] exec: remove compat_do_execve
From: Sergei Shtylyov @ 2021-03-27 19:56 UTC (permalink / raw)
To: Christoph Hellwig, Eric W. Biederman, Al Viro
Cc: linux-arch, linux-s390, linux-parisc, Arnd Bergmann, Brian Gerst,
x86, linux-mips, linux-kernel, linux-fsdevel, Luis Chamberlain,
sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20210326143831.1550030-3-hch@lst.de>
On 3/26/21 5:38 PM, Christoph Hellwig wrote:
> Just call compat_do_execve instead.
compat_do_execveat(), maybe?
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/exec.c | 17 +----------------
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index b63fb020909075..06e07278b456fa 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
[...]
> @@ -2072,7 +2057,7 @@ COMPAT_SYSCALL_DEFINE3(execve, const char __user *, filename,
> const compat_uptr_t __user *, argv,
> const compat_uptr_t __user *, envp)
> {
> - return compat_do_execve(getname(filename), argv, envp);
> + return compat_do_execveat(AT_FDCWD, getname(filename), argv, envp, 0);
> }
>
> COMPAT_SYSCALL_DEFINE5(execveat, int, fd,
MBR, Sergei
^ permalink raw reply
* [PATCH] powerpc: Fix HAVE_HARDLOCKUP_DETECTOR_ARCH build configuration
From: Chen Huang @ 2021-03-27 9:49 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: Don Zickus, Chen Huang, linux-kernel, Nicholas Piggin, Hulk Robot,
Andrew Morton, linuxppc-dev
When compiling the powerpc with the SMP disabled, it shows the issue:
arch/powerpc/kernel/watchdog.c: In function ‘watchdog_smp_panic’:
arch/powerpc/kernel/watchdog.c:177:4: error: implicit declaration of function ‘smp_send_nmi_ipi’; did you mean ‘smp_send_stop’? [-Werror=implicit-function-declaration]
177 | smp_send_nmi_ipi(c, wd_lockup_ipi, 1000000);
| ^~~~~~~~~~~~~~~~
| smp_send_stop
cc1: all warnings being treated as errors
make[2]: *** [scripts/Makefile.build:273: arch/powerpc/kernel/watchdog.o] Error 1
make[1]: *** [scripts/Makefile.build:534: arch/powerpc/kernel] Error 2
make: *** [Makefile:1980: arch/powerpc] Error 2
make: *** Waiting for unfinished jobs....
We found that powerpc used ipi to implement hardlockup watchdog, so the
HAVE_HARDLOCKUP_DETECTOR_ARCH should depend on the SMP.
Fixes: 2104180a5369 ("powerpc/64s: implement arch-specific hardlockup watchdog")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Chen Huang <chenhuang5@huawei.com>
---
arch/powerpc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 764df010baee..2d4f37b117ce 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -225,7 +225,7 @@ config PPC
select HAVE_LIVEPATCH if HAVE_DYNAMIC_FTRACE_WITH_REGS
select HAVE_MOD_ARCH_SPECIFIC
select HAVE_NMI if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
- select HAVE_HARDLOCKUP_DETECTOR_ARCH if (PPC64 && PPC_BOOK3S)
+ select HAVE_HARDLOCKUP_DETECTOR_ARCH if PPC64 && PPC_BOOK3S && SMP
select HAVE_OPTPROBES if PPC64
select HAVE_PERF_EVENTS
select HAVE_PERF_EVENTS_NMI if PPC64
--
2.17.1
^ permalink raw reply related
* [PATCH v5 0/7] riscv: Add qspinlock/qrwlock
From: guoren @ 2021-03-28 6:30 UTC (permalink / raw)
To: guoren
Cc: linux-arch, linux-xtensa, Guo Ren, linux-kernel, linux-csky,
openrisc, sparclinux, linux-riscv, linuxppc-dev
From: Guo Ren <guoren@linux.alibaba.com>
Current riscv is still using baby spinlock implementation. It'll cause
fairness and cache line bouncing problems. Many people are involved
and pay the efforts to improve it:
- The first version of patch was made in 2019.1:
https://lore.kernel.org/linux-riscv/20190211043829.30096-1-michaeljclark@mac.com/#r
- The second version was made in 2020.11:
https://lore.kernel.org/linux-riscv/1606225437-22948-2-git-send-email-guoren@kernel.org/
- A good discussion at Platform HSC.2021-03-08:
https://drive.google.com/drive/folders/1ooqdnIsYx7XKor5O1XTtM6D1CHp4hc0p
Hope your comments and Tested-by or Co-developed-by or Reviewed-by ...
Let's kick the qspinlock into riscv right now (Also for the
architecture which hasn't xchg16 atomic instruction.)
Change V5:
- Fixup #endif comment typo by Waiman
- Remove cmpxchg coding convention patches which will get into a
separate patchset later by Arnd's advice
- Try to involve more architectures in the discussion
Change V4:
- Remove custom sub-word xchg implementation
- Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32 in locking/qspinlock
Change V3:
- Coding convention by Peter Zijlstra's advices
Change V2:
- Coding convention in cmpxchg.h
- Re-implement short xchg
- Remove char & cmpxchg implementations
Guo Ren (6):
locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
csky: Convert custom spinlock/rwlock to generic qspinlock/qrwlock
powerpc/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
openrisc: qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
sparc: qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
xtensa: qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
Michael Clark (1):
riscv: Convert custom spinlock/rwlock to generic qspinlock/qrwlock
arch/csky/Kconfig | 2 +
arch/csky/include/asm/Kbuild | 2 +
arch/csky/include/asm/spinlock.h | 82 +--------------
arch/csky/include/asm/spinlock_types.h | 16 +--
arch/openrisc/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/riscv/Kconfig | 3 +
arch/riscv/include/asm/Kbuild | 3 +
arch/riscv/include/asm/spinlock.h | 126 +-----------------------
arch/riscv/include/asm/spinlock_types.h | 15 +--
arch/sparc/Kconfig | 1 +
arch/xtensa/Kconfig | 1 +
kernel/Kconfig.locks | 3 +
kernel/locking/qspinlock.c | 46 +++++----
14 files changed, 49 insertions(+), 253 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v5 1/7] locking/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
From: guoren @ 2021-03-28 6:30 UTC (permalink / raw)
To: guoren
Cc: linux-arch, linux-xtensa, Guo Ren, Arnd Bergmann, Peter Zijlstra,
Will Deacon, linux-kernel, linux-csky, openrisc, Anup Patel,
sparclinux, Waiman Long, linux-riscv, linuxppc-dev, Ingo Molnar
In-Reply-To: <1616913028-83376-1-git-send-email-guoren@kernel.org>
From: Guo Ren <guoren@linux.alibaba.com>
Some architectures don't have sub-word swap atomic instruction,
they only have the full word's one.
The sub-word swap only improve the performance when:
NR_CPUS < 16K
* 0- 7: locked byte
* 8: pending
* 9-15: not used
* 16-17: tail index
* 18-31: tail cpu (+1)
The 9-15 bits are wasted to use xchg16 in xchg_tail.
Please let architecture select xchg16/xchg32 to implement
xchg_tail.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Waiman Long <longman@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Anup Patel <anup@brainfault.org>
---
kernel/Kconfig.locks | 3 +++
kernel/locking/qspinlock.c | 46 +++++++++++++++++++++-----------------
2 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks
index 3de8fd11873b..d02f1261f73f 100644
--- a/kernel/Kconfig.locks
+++ b/kernel/Kconfig.locks
@@ -239,6 +239,9 @@ config LOCK_SPIN_ON_OWNER
config ARCH_USE_QUEUED_SPINLOCKS
bool
+config ARCH_USE_QUEUED_SPINLOCKS_XCHG32
+ bool
+
config QUEUED_SPINLOCKS
def_bool y if ARCH_USE_QUEUED_SPINLOCKS
depends on SMP
diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index cbff6ba53d56..4bfaa969bd15 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -163,26 +163,6 @@ static __always_inline void clear_pending_set_locked(struct qspinlock *lock)
WRITE_ONCE(lock->locked_pending, _Q_LOCKED_VAL);
}
-/*
- * xchg_tail - Put in the new queue tail code word & retrieve previous one
- * @lock : Pointer to queued spinlock structure
- * @tail : The new queue tail code word
- * Return: The previous queue tail code word
- *
- * xchg(lock, tail), which heads an address dependency
- *
- * p,*,* -> n,*,* ; prev = xchg(lock, node)
- */
-static __always_inline u32 xchg_tail(struct qspinlock *lock, u32 tail)
-{
- /*
- * We can use relaxed semantics since the caller ensures that the
- * MCS node is properly initialized before updating the tail.
- */
- return (u32)xchg_relaxed(&lock->tail,
- tail >> _Q_TAIL_OFFSET) << _Q_TAIL_OFFSET;
-}
-
#else /* _Q_PENDING_BITS == 8 */
/**
@@ -206,6 +186,30 @@ static __always_inline void clear_pending_set_locked(struct qspinlock *lock)
{
atomic_add(-_Q_PENDING_VAL + _Q_LOCKED_VAL, &lock->val);
}
+#endif /* _Q_PENDING_BITS == 8 */
+
+#if _Q_PENDING_BITS == 8 && !defined(CONFIG_ARCH_USE_QUEUED_SPINLOCKS_XCHG32)
+/*
+ * xchg_tail - Put in the new queue tail code word & retrieve previous one
+ * @lock : Pointer to queued spinlock structure
+ * @tail : The new queue tail code word
+ * Return: The previous queue tail code word
+ *
+ * xchg(lock, tail), which heads an address dependency
+ *
+ * p,*,* -> n,*,* ; prev = xchg(lock, node)
+ */
+static __always_inline u32 xchg_tail(struct qspinlock *lock, u32 tail)
+{
+ /*
+ * We can use relaxed semantics since the caller ensures that the
+ * MCS node is properly initialized before updating the tail.
+ */
+ return (u32)xchg_relaxed(&lock->tail,
+ tail >> _Q_TAIL_OFFSET) << _Q_TAIL_OFFSET;
+}
+
+#else
/**
* xchg_tail - Put in the new queue tail code word & retrieve previous one
@@ -236,7 +240,7 @@ static __always_inline u32 xchg_tail(struct qspinlock *lock, u32 tail)
}
return old;
}
-#endif /* _Q_PENDING_BITS == 8 */
+#endif
/**
* queued_fetch_set_pending_acquire - fetch the whole lock value and set pending
--
2.17.1
^ permalink raw reply related
* [PATCH v5 2/7] riscv: Convert custom spinlock/rwlock to generic qspinlock/qrwlock
From: guoren @ 2021-03-28 6:30 UTC (permalink / raw)
To: guoren
Cc: linux-arch, linux-xtensa, Guo Ren, Arnd Bergmann, Peter Zijlstra,
Anup Patel, Palmer Dabbelt, linux-kernel, linux-csky,
Michael Clark, openrisc, sparclinux, linux-riscv, linuxppc-dev
In-Reply-To: <1616913028-83376-1-git-send-email-guoren@kernel.org>
From: Michael Clark <michaeljclark@mac.com>
Update the RISC-V port to use the generic qspinlock and qrwlock.
This patch requires support for xchg_xtail for full-word which
are added by a previous patch:
Guo added select ARCH_USE_QUEUED_SPINLOCKS_XCHG32 in Kconfig
Guo fixed up compile error which made by below include sequence:
+#include <asm/qrwlock.h>
+#include <asm/qspinlock.h>
Signed-off-by: Michael Clark <michaeljclark@mac.com>
Co-developed-by: Guo Ren <guoren@linux.alibaba.com>
Tested-by: Guo Ren <guoren@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Link: https://lore.kernel.org/linux-riscv/20190211043829.30096-3-michaeljclark@mac.com/
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Anup Patel <anup@brainfault.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Palmer Dabbelt <palmerdabbelt@google.com>
---
arch/riscv/Kconfig | 3 +
arch/riscv/include/asm/Kbuild | 3 +
arch/riscv/include/asm/spinlock.h | 126 +-----------------------
arch/riscv/include/asm/spinlock_types.h | 15 +--
4 files changed, 11 insertions(+), 136 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 87d7b52f278f..67cc65ba1ea1 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -33,6 +33,9 @@ config RISCV
select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
select ARCH_WANT_FRAME_POINTERS
select ARCH_WANT_HUGE_PMD_SHARE if 64BIT
+ select ARCH_USE_QUEUED_RWLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS_XCHG32
select CLONE_BACKWARDS
select CLINT_TIMER if !MMU
select COMMON_CLK
diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
index 445ccc97305a..750c1056b90f 100644
--- a/arch/riscv/include/asm/Kbuild
+++ b/arch/riscv/include/asm/Kbuild
@@ -3,5 +3,8 @@ generic-y += early_ioremap.h
generic-y += extable.h
generic-y += flat.h
generic-y += kvm_para.h
+generic-y += mcs_spinlock.h
+generic-y += qrwlock.h
+generic-y += qspinlock.h
generic-y += user.h
generic-y += vmlinux.lds.h
diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h
index f4f7fa1b7ca8..a557de67a425 100644
--- a/arch/riscv/include/asm/spinlock.h
+++ b/arch/riscv/include/asm/spinlock.h
@@ -7,129 +7,7 @@
#ifndef _ASM_RISCV_SPINLOCK_H
#define _ASM_RISCV_SPINLOCK_H
-#include <linux/kernel.h>
-#include <asm/current.h>
-#include <asm/fence.h>
-
-/*
- * Simple spin lock operations. These provide no fairness guarantees.
- */
-
-/* FIXME: Replace this with a ticket lock, like MIPS. */
-
-#define arch_spin_is_locked(x) (READ_ONCE((x)->lock) != 0)
-
-static inline void arch_spin_unlock(arch_spinlock_t *lock)
-{
- smp_store_release(&lock->lock, 0);
-}
-
-static inline int arch_spin_trylock(arch_spinlock_t *lock)
-{
- int tmp = 1, busy;
-
- __asm__ __volatile__ (
- " amoswap.w %0, %2, %1\n"
- RISCV_ACQUIRE_BARRIER
- : "=r" (busy), "+A" (lock->lock)
- : "r" (tmp)
- : "memory");
-
- return !busy;
-}
-
-static inline void arch_spin_lock(arch_spinlock_t *lock)
-{
- while (1) {
- if (arch_spin_is_locked(lock))
- continue;
-
- if (arch_spin_trylock(lock))
- break;
- }
-}
-
-/***********************************************************/
-
-static inline void arch_read_lock(arch_rwlock_t *lock)
-{
- int tmp;
-
- __asm__ __volatile__(
- "1: lr.w %1, %0\n"
- " bltz %1, 1b\n"
- " addi %1, %1, 1\n"
- " sc.w %1, %1, %0\n"
- " bnez %1, 1b\n"
- RISCV_ACQUIRE_BARRIER
- : "+A" (lock->lock), "=&r" (tmp)
- :: "memory");
-}
-
-static inline void arch_write_lock(arch_rwlock_t *lock)
-{
- int tmp;
-
- __asm__ __volatile__(
- "1: lr.w %1, %0\n"
- " bnez %1, 1b\n"
- " li %1, -1\n"
- " sc.w %1, %1, %0\n"
- " bnez %1, 1b\n"
- RISCV_ACQUIRE_BARRIER
- : "+A" (lock->lock), "=&r" (tmp)
- :: "memory");
-}
-
-static inline int arch_read_trylock(arch_rwlock_t *lock)
-{
- int busy;
-
- __asm__ __volatile__(
- "1: lr.w %1, %0\n"
- " bltz %1, 1f\n"
- " addi %1, %1, 1\n"
- " sc.w %1, %1, %0\n"
- " bnez %1, 1b\n"
- RISCV_ACQUIRE_BARRIER
- "1:\n"
- : "+A" (lock->lock), "=&r" (busy)
- :: "memory");
-
- return !busy;
-}
-
-static inline int arch_write_trylock(arch_rwlock_t *lock)
-{
- int busy;
-
- __asm__ __volatile__(
- "1: lr.w %1, %0\n"
- " bnez %1, 1f\n"
- " li %1, -1\n"
- " sc.w %1, %1, %0\n"
- " bnez %1, 1b\n"
- RISCV_ACQUIRE_BARRIER
- "1:\n"
- : "+A" (lock->lock), "=&r" (busy)
- :: "memory");
-
- return !busy;
-}
-
-static inline void arch_read_unlock(arch_rwlock_t *lock)
-{
- __asm__ __volatile__(
- RISCV_RELEASE_BARRIER
- " amoadd.w x0, %1, %0\n"
- : "+A" (lock->lock)
- : "r" (-1)
- : "memory");
-}
-
-static inline void arch_write_unlock(arch_rwlock_t *lock)
-{
- smp_store_release(&lock->lock, 0);
-}
+#include <asm/qspinlock.h>
+#include <asm/qrwlock.h>
#endif /* _ASM_RISCV_SPINLOCK_H */
diff --git a/arch/riscv/include/asm/spinlock_types.h b/arch/riscv/include/asm/spinlock_types.h
index f398e7638dd6..d033a973f287 100644
--- a/arch/riscv/include/asm/spinlock_types.h
+++ b/arch/riscv/include/asm/spinlock_types.h
@@ -6,20 +6,11 @@
#ifndef _ASM_RISCV_SPINLOCK_TYPES_H
#define _ASM_RISCV_SPINLOCK_TYPES_H
-#ifndef __LINUX_SPINLOCK_TYPES_H
+#if !defined(__LINUX_SPINLOCK_TYPES_H) && !defined(_ASM_RISCV_SPINLOCK_H)
# error "please don't include this file directly"
#endif
-typedef struct {
- volatile unsigned int lock;
-} arch_spinlock_t;
-
-#define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
-
-typedef struct {
- volatile unsigned int lock;
-} arch_rwlock_t;
-
-#define __ARCH_RW_LOCK_UNLOCKED { 0 }
+#include <asm-generic/qspinlock_types.h>
+#include <asm-generic/qrwlock_types.h>
#endif /* _ASM_RISCV_SPINLOCK_TYPES_H */
--
2.17.1
^ permalink raw reply related
* [PATCH v5 3/7] csky: Convert custom spinlock/rwlock to generic qspinlock/qrwlock
From: guoren @ 2021-03-28 6:30 UTC (permalink / raw)
To: guoren
Cc: linux-arch, linux-xtensa, Guo Ren, Arnd Bergmann, Peter Zijlstra,
Will Deacon, linux-kernel, linux-csky, openrisc, sparclinux,
Waiman Long, linux-riscv, linuxppc-dev
In-Reply-To: <1616913028-83376-1-git-send-email-guoren@kernel.org>
From: Guo Ren <guoren@linux.alibaba.com>
Update the C-SKY port to use the generic qspinlock and qrwlock.
C-SKY only support ldex.w/stex.w with word(double word) size &
align access. So it must select XCHG32 to let qspinlock only use
word atomic xchg_tail.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Waiman Long <longman@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
---
arch/csky/Kconfig | 2 +
arch/csky/include/asm/Kbuild | 2 +
arch/csky/include/asm/spinlock.h | 82 +-------------------------
arch/csky/include/asm/spinlock_types.h | 16 +----
4 files changed, 6 insertions(+), 96 deletions(-)
diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
index 34e91224adc3..5910eb6ddde2 100644
--- a/arch/csky/Kconfig
+++ b/arch/csky/Kconfig
@@ -8,6 +8,8 @@ config CSKY
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_QUEUED_RWLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS_XCHG32
select ARCH_WANT_FRAME_POINTERS if !CPU_CK610
select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
select COMMON_CLK
diff --git a/arch/csky/include/asm/Kbuild b/arch/csky/include/asm/Kbuild
index cc24bb8e539f..2a2d09963bb9 100644
--- a/arch/csky/include/asm/Kbuild
+++ b/arch/csky/include/asm/Kbuild
@@ -2,6 +2,8 @@
generic-y += asm-offsets.h
generic-y += gpio.h
generic-y += kvm_para.h
+generic-y += mcs_spinlock.h
generic-y += qrwlock.h
+generic-y += qspinlock.h
generic-y += user.h
generic-y += vmlinux.lds.h
diff --git a/arch/csky/include/asm/spinlock.h b/arch/csky/include/asm/spinlock.h
index 69f5aa249c5f..fcff36753c25 100644
--- a/arch/csky/include/asm/spinlock.h
+++ b/arch/csky/include/asm/spinlock.h
@@ -3,87 +3,7 @@
#ifndef __ASM_CSKY_SPINLOCK_H
#define __ASM_CSKY_SPINLOCK_H
-#include <linux/spinlock_types.h>
-#include <asm/barrier.h>
-
-/*
- * Ticket-based spin-locking.
- */
-static inline void arch_spin_lock(arch_spinlock_t *lock)
-{
- arch_spinlock_t lockval;
- u32 ticket_next = 1 << TICKET_NEXT;
- u32 *p = &lock->lock;
- u32 tmp;
-
- asm volatile (
- "1: ldex.w %0, (%2) \n"
- " mov %1, %0 \n"
- " add %0, %3 \n"
- " stex.w %0, (%2) \n"
- " bez %0, 1b \n"
- : "=&r" (tmp), "=&r" (lockval)
- : "r"(p), "r"(ticket_next)
- : "cc");
-
- while (lockval.tickets.next != lockval.tickets.owner)
- lockval.tickets.owner = READ_ONCE(lock->tickets.owner);
-
- smp_mb();
-}
-
-static inline int arch_spin_trylock(arch_spinlock_t *lock)
-{
- u32 tmp, contended, res;
- u32 ticket_next = 1 << TICKET_NEXT;
- u32 *p = &lock->lock;
-
- do {
- asm volatile (
- " ldex.w %0, (%3) \n"
- " movi %2, 1 \n"
- " rotli %1, %0, 16 \n"
- " cmpne %1, %0 \n"
- " bt 1f \n"
- " movi %2, 0 \n"
- " add %0, %0, %4 \n"
- " stex.w %0, (%3) \n"
- "1: \n"
- : "=&r" (res), "=&r" (tmp), "=&r" (contended)
- : "r"(p), "r"(ticket_next)
- : "cc");
- } while (!res);
-
- if (!contended)
- smp_mb();
-
- return !contended;
-}
-
-static inline void arch_spin_unlock(arch_spinlock_t *lock)
-{
- smp_mb();
- WRITE_ONCE(lock->tickets.owner, lock->tickets.owner + 1);
-}
-
-static inline int arch_spin_value_unlocked(arch_spinlock_t lock)
-{
- return lock.tickets.owner == lock.tickets.next;
-}
-
-static inline int arch_spin_is_locked(arch_spinlock_t *lock)
-{
- return !arch_spin_value_unlocked(READ_ONCE(*lock));
-}
-
-static inline int arch_spin_is_contended(arch_spinlock_t *lock)
-{
- struct __raw_tickets tickets = READ_ONCE(lock->tickets);
-
- return (tickets.next - tickets.owner) > 1;
-}
-#define arch_spin_is_contended arch_spin_is_contended
-
+#include <asm/qspinlock.h>
#include <asm/qrwlock.h>
#endif /* __ASM_CSKY_SPINLOCK_H */
diff --git a/arch/csky/include/asm/spinlock_types.h b/arch/csky/include/asm/spinlock_types.h
index 8ff0f6ff3a00..757594760e65 100644
--- a/arch/csky/include/asm/spinlock_types.h
+++ b/arch/csky/include/asm/spinlock_types.h
@@ -7,21 +7,7 @@
# error "please don't include this file directly"
#endif
-#define TICKET_NEXT 16
-
-typedef struct {
- union {
- u32 lock;
- struct __raw_tickets {
- /* little endian */
- u16 owner;
- u16 next;
- } tickets;
- };
-} arch_spinlock_t;
-
-#define __ARCH_SPIN_LOCK_UNLOCKED { { 0 } }
-
+#include <asm-generic/qspinlock_types.h>
#include <asm-generic/qrwlock_types.h>
#endif /* __ASM_CSKY_SPINLOCK_TYPES_H */
--
2.17.1
^ permalink raw reply related
* [PATCH v5 4/7] powerpc/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
From: guoren @ 2021-03-28 6:30 UTC (permalink / raw)
To: guoren
Cc: linux-arch, linux-xtensa, Guo Ren, linux-kernel, linux-csky,
openrisc, Paul Mackerras, sparclinux, linux-riscv, linuxppc-dev
In-Reply-To: <1616913028-83376-1-git-send-email-guoren@kernel.org>
From: Guo Ren <guoren@linux.alibaba.com>
We don't have native hw xchg16 instruction, so let qspinlock
generic code to deal with it.
Using the full-word atomic xchg instructions implement xchg16 has
the semantic risk for atomic operations.
This patch cancels the dependency of on qspinlock generic code on
architecture's xchg16.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
---
arch/powerpc/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 386ae12d8523..69ec4ade6521 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -151,6 +151,7 @@ config PPC
select ARCH_USE_CMPXCHG_LOCKREF if PPC64
select ARCH_USE_QUEUED_RWLOCKS if PPC_QUEUED_SPINLOCKS
select ARCH_USE_QUEUED_SPINLOCKS if PPC_QUEUED_SPINLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS_XCHG32 if PPC_QUEUED_SPINLOCKS
select ARCH_WANT_IPC_PARSE_VERSION
select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
select ARCH_WANT_LD_ORPHAN_WARN
--
2.17.1
^ permalink raw reply related
* [PATCH v5 5/7] openrisc: qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
From: guoren @ 2021-03-28 6:30 UTC (permalink / raw)
To: guoren
Cc: linux-arch, linux-xtensa, Guo Ren, Arnd Bergmann, linux-kernel,
linux-csky, Stefan Kristiansson, openrisc, Stafford Horne,
sparclinux, linux-riscv, linuxppc-dev, Jonas Bonn
In-Reply-To: <1616913028-83376-1-git-send-email-guoren@kernel.org>
From: Guo Ren <guoren@linux.alibaba.com>
We don't have native hw xchg16 instruction, so let qspinlock
generic code to deal with it.
Using the full-word atomic xchg instructions implement xchg16 has
the semantic risk for atomic operations.
This patch cancels the dependency of on qspinlock generic code on
architecture's xchg16.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
Cc: Stafford Horne <shorne@gmail.com>
Cc: openrisc@lists.librecores.org
---
arch/openrisc/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 591acc5990dc..b299e409429f 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -33,6 +33,7 @@ config OPENRISC
select OR1K_PIC
select CPU_NO_EFFICIENT_FFS if !OPENRISC_HAVE_INST_FF1
select ARCH_USE_QUEUED_SPINLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS_XCHG32
select ARCH_USE_QUEUED_RWLOCKS
select OMPIC if SMP
select ARCH_WANT_FRAME_POINTERS
--
2.17.1
^ permalink raw reply related
* [PATCH v5 6/7] sparc: qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
From: guoren @ 2021-03-28 6:30 UTC (permalink / raw)
To: guoren
Cc: linux-arch, linux-xtensa, Guo Ren, Arnd Bergmann, Rob Gardner,
linux-kernel, linux-csky, openrisc, sparclinux, linux-riscv,
linuxppc-dev, David S . Miller
In-Reply-To: <1616913028-83376-1-git-send-email-guoren@kernel.org>
From: Guo Ren <guoren@linux.alibaba.com>
We don't have native hw xchg16 instruction, so let qspinlock
generic code to deal with it.
Using the full-word atomic xchg instructions implement xchg16 has
the semantic risk for atomic operations.
This patch cancels the dependency of on qspinlock generic code on
architecture's xchg16.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Rob Gardner <rob.gardner@oracle.com>
---
arch/sparc/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 164a5254c91c..1079fe3f058c 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -91,6 +91,7 @@ config SPARC64
select HAVE_REGS_AND_STACK_ACCESS_API
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS_XCHG32
select GENERIC_TIME_VSYSCALL
select ARCH_CLOCKSOURCE_DATA
select ARCH_HAS_PTE_SPECIAL
--
2.17.1
^ permalink raw reply related
* [PATCH v5 7/7] xtensa: qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
From: guoren @ 2021-03-28 6:30 UTC (permalink / raw)
To: guoren
Cc: linux-arch, linux-xtensa, Guo Ren, Arnd Bergmann, Chris Zankel,
linux-kernel, linux-csky, Max Filippov, openrisc, sparclinux,
linux-riscv, linuxppc-dev
In-Reply-To: <1616913028-83376-1-git-send-email-guoren@kernel.org>
From: Guo Ren <guoren@linux.alibaba.com>
We don't have native hw xchg16 instruction, so let qspinlock
generic code to deal with it.
Using the full-word atomic xchg instructions implement xchg16 has
the semantic risk for atomic operations.
This patch cancels the dependency of on qspinlock generic code on
architecture's xchg16.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
---
arch/xtensa/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 9ad6b7b82707..f19d780638f7 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -9,6 +9,7 @@ config XTENSA
select ARCH_HAS_DMA_SET_UNCACHED if MMU
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS_XCHG32
select ARCH_WANT_FRAME_POINTERS
select ARCH_WANT_IPC_PARSE_VERSION
select BUILDTIME_TABLE_SORT
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] powerpc: Fix HAVE_HARDLOCKUP_DETECTOR_ARCH build configuration
From: Christophe Leroy @ 2021-03-28 11:06 UTC (permalink / raw)
To: Chen Huang, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras
Cc: Don Zickus, linux-kernel, Nicholas Piggin, Hulk Robot,
Andrew Morton, linuxppc-dev
In-Reply-To: <20210327094900.938555-1-chenhuang5@huawei.com>
Le 27/03/2021 à 10:49, Chen Huang a écrit :
> When compiling the powerpc with the SMP disabled, it shows the issue:
>
> arch/powerpc/kernel/watchdog.c: In function ‘watchdog_smp_panic’:
> arch/powerpc/kernel/watchdog.c:177:4: error: implicit declaration of function ‘smp_send_nmi_ipi’; did you mean ‘smp_send_stop’? [-Werror=implicit-function-declaration]
> 177 | smp_send_nmi_ipi(c, wd_lockup_ipi, 1000000);
> | ^~~~~~~~~~~~~~~~
> | smp_send_stop
> cc1: all warnings being treated as errors
> make[2]: *** [scripts/Makefile.build:273: arch/powerpc/kernel/watchdog.o] Error 1
> make[1]: *** [scripts/Makefile.build:534: arch/powerpc/kernel] Error 2
> make: *** [Makefile:1980: arch/powerpc] Error 2
> make: *** Waiting for unfinished jobs....
>
> We found that powerpc used ipi to implement hardlockup watchdog, so the
> HAVE_HARDLOCKUP_DETECTOR_ARCH should depend on the SMP.
>
> Fixes: 2104180a5369 ("powerpc/64s: implement arch-specific hardlockup watchdog")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Chen Huang <chenhuang5@huawei.com>
> ---
> arch/powerpc/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 764df010baee..2d4f37b117ce 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -225,7 +225,7 @@ config PPC
> select HAVE_LIVEPATCH if HAVE_DYNAMIC_FTRACE_WITH_REGS
> select HAVE_MOD_ARCH_SPECIFIC
> select HAVE_NMI if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
> - select HAVE_HARDLOCKUP_DETECTOR_ARCH if (PPC64 && PPC_BOOK3S)
> + select HAVE_HARDLOCKUP_DETECTOR_ARCH if PPC64 && PPC_BOOK3S && SMP
While modifying this line, you should restore the alphabetic order by moving it up.
You can use PPC_BOOK3S_64 instead of PPC64 && PPC_BOOK3S
> select HAVE_OPTPROBES if PPC64
> select HAVE_PERF_EVENTS
> select HAVE_PERF_EVENTS_NMI if PPC64
>
^ permalink raw reply
* Re: [PATCH v5 4/7] powerpc/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
From: Christophe Leroy @ 2021-03-28 11:14 UTC (permalink / raw)
To: guoren
Cc: linux-arch, linux-xtensa, Guo Ren, linux-kernel, linux-csky,
openrisc, Paul Mackerras, sparclinux, linux-riscv, linuxppc-dev
In-Reply-To: <1616913028-83376-5-git-send-email-guoren@kernel.org>
Le 28/03/2021 à 08:30, guoren@kernel.org a écrit :
> From: Guo Ren <guoren@linux.alibaba.com>
>
> We don't have native hw xchg16 instruction, so let qspinlock
> generic code to deal with it.
We have lharx/sthcx pair on some versions of powerpc.
See https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20201107032328.2454582-1-npiggin@gmail.com/
Christophe
>
> Using the full-word atomic xchg instructions implement xchg16 has
> the semantic risk for atomic operations.
>
> This patch cancels the dependency of on qspinlock generic code on
> architecture's xchg16.
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> ---
> arch/powerpc/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 386ae12d8523..69ec4ade6521 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -151,6 +151,7 @@ config PPC
> select ARCH_USE_CMPXCHG_LOCKREF if PPC64
> select ARCH_USE_QUEUED_RWLOCKS if PPC_QUEUED_SPINLOCKS
> select ARCH_USE_QUEUED_SPINLOCKS if PPC_QUEUED_SPINLOCKS
> + select ARCH_USE_QUEUED_SPINLOCKS_XCHG32 if PPC_QUEUED_SPINLOCKS
> select ARCH_WANT_IPC_PARSE_VERSION
> select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
> select ARCH_WANT_LD_ORPHAN_WARN
>
^ permalink raw reply
* Re: [PATCH v5 4/7] powerpc/qspinlock: Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32
From: Guo Ren @ 2021-03-28 11:36 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, linux-xtensa, Guo Ren, Linux Kernel Mailing List,
linux-csky, openrisc, Paul Mackerras, sparclinux, linux-riscv,
linuxppc-dev
In-Reply-To: <e27af5e0-a462-cb75-6311-1b5a6b4ee4f1@csgroup.eu>
On Sun, Mar 28, 2021 at 7:14 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 28/03/2021 à 08:30, guoren@kernel.org a écrit :
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > We don't have native hw xchg16 instruction, so let qspinlock
> > generic code to deal with it.
>
> We have lharx/sthcx pair on some versions of powerpc.
>
> See https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20201107032328.2454582-1-npiggin@gmail.com/
Got it, thx for the information.
>
> Christophe
>
> >
> > Using the full-word atomic xchg instructions implement xchg16 has
> > the semantic risk for atomic operations.
> >
> > This patch cancels the dependency of on qspinlock generic code on
> > architecture's xchg16.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > ---
> > arch/powerpc/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 386ae12d8523..69ec4ade6521 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -151,6 +151,7 @@ config PPC
> > select ARCH_USE_CMPXCHG_LOCKREF if PPC64
> > select ARCH_USE_QUEUED_RWLOCKS if PPC_QUEUED_SPINLOCKS
> > select ARCH_USE_QUEUED_SPINLOCKS if PPC_QUEUED_SPINLOCKS
> > + select ARCH_USE_QUEUED_SPINLOCKS_XCHG32 if PPC_QUEUED_SPINLOCKS
> > select ARCH_WANT_IPC_PARSE_VERSION
> > select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
> > select ARCH_WANT_LD_ORPHAN_WARN
> >
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply
* [PATCH] powerpc/signal32: Fix Oops on sigreturn with unmapped VDSO
From: Christophe Leroy @ 2021-03-28 14:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
PPC32 encounters a KUAP fault when trying to handle a signal with
VDSO unmapped.
Kernel attempted to read user page (7fc07ec0) - exploit attempt? (uid: 0)
BUG: Unable to handle kernel data access on read at 0x7fc07ec0
Faulting instruction address: 0xc00111d4
Oops: Kernel access of bad area, sig: 11 [#1]
BE PAGE_SIZE=16K PREEMPT CMPC885
CPU: 0 PID: 353 Comm: sigreturn_vdso Not tainted 5.12.0-rc4-s3k-dev-01553-gb30c310ea220 #4814
NIP: c00111d4 LR: c0005a28 CTR: 00000000
REGS: cadb3dd0 TRAP: 0300 Not tainted (5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
MSR: 00009032 <EE,ME,IR,DR,RI> CR: 48000884 XER: 20000000
DAR: 7fc07ec0 DSISR: 88000000
GPR00: c0007788 cadb3e90 c28d4a40 7fc07ec0 7fc07ed0 000004e0 7fc07ce0 00000000
GPR08: 00000001 00000001 7fc07ec0 00000000 28000282 1001b828 100a0920 00000000
GPR16: 100cac0c 100b0000 105c43a4 105c5685 100d0000 100d0000 100d0000 100b2e9e
GPR24: ffffffff 105c43c8 00000000 7fc07ec8 cadb3f40 cadb3ec8 c28d4a40 00000000
NIP [c00111d4] flush_icache_range+0x90/0xb4
LR [c0005a28] handle_signal32+0x1bc/0x1c4
Call Trace:
[cadb3e90] [100d0000] 0x100d0000 (unreliable)
[cadb3ec0] [c0007788] do_notify_resume+0x260/0x314
[cadb3f20] [c000c764] syscall_exit_prepare+0x120/0x184
[cadb3f30] [c00100b4] ret_from_syscall+0xc/0x28
--- interrupt: c00 at 0xfe807f8
NIP: 0fe807f8 LR: 10001060 CTR: c0139378
REGS: cadb3f40 TRAP: 0c00 Not tainted (5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
MSR: 0000d032 <EE,PR,ME,IR,DR,RI> CR: 28000482 XER: 20000000
GPR00: 00000025 7fc081c0 77bb1690 00000000 0000000a 28000482 00000001 0ff03a38
GPR08: 0000d032 00006de5 c28d4a40 00000009 88000482 1001b828 100a0920 00000000
GPR16: 100cac0c 100b0000 105c43a4 105c5685 100d0000 100d0000 100d0000 100b2e9e
GPR24: ffffffff 105c43c8 00000000 77ba7628 10002398 10010000 10002124 00024000
NIP [0fe807f8] 0xfe807f8
LR [10001060] 0x10001060
--- interrupt: c00
Instruction dump:
38630010 7c001fac 38630010 4200fff0 7c0004ac 4c00012c 4e800020 7c001fac
2c0a0000 38630010 4082ffcc 4bffffe4 <7c00186c> 2c070000 39430010 4082ff8c
---[ end trace 3973fb72b049cb06 ]---
This is because flush_icache_range() is called on user addresses.
The same problem was detected some time ago on PPC64. It was fixed by
enabling KUAP in commit 59bee45b9712 ("powerpc/mm: Fix missing KUAP
disable in flush_coherent_icache()").
PPC32 doesn't use flush_coherent_icache() and fallbacks on
clean_dcache_range() and invalidate_icache_range().
We could fix it similarly by enabling user access in those functions,
but this is overkill for just flushing two instructions.
The two instructions are 8 bytes aligned, so a single dcbst/icbi is
enough to flush them. Do like __patch_instruction() and inline
a dcbst followed by an icbi just after the write of the instructions,
while user access is still allowed. The isync is not required because
rfi will be used to return to user.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/signal_32.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 75ee918a120a..5b2ba2731957 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -809,6 +809,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
unsafe_put_user(PPC_INST_ADDI + __NR_rt_sigreturn, &mctx->mc_pad[0],
failed);
unsafe_put_user(PPC_INST_SC, &mctx->mc_pad[1], failed);
+ asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
}
unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
@@ -817,9 +818,6 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
if (copy_siginfo_to_user(&frame->info, &ksig->info))
goto badframe;
- if (tramp == (unsigned long)mctx->mc_pad)
- flush_icache_range(tramp, tramp + 2 * sizeof(unsigned long));
-
regs->link = tramp;
#ifdef CONFIG_PPC_FPU_REGS
@@ -908,12 +906,10 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
/* Set up the sigreturn trampoline: li r0,sigret; sc */
unsafe_put_user(PPC_INST_ADDI + __NR_sigreturn, &mctx->mc_pad[0], failed);
unsafe_put_user(PPC_INST_SC, &mctx->mc_pad[1], failed);
+ asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
}
user_write_access_end();
- if (tramp == (unsigned long)mctx->mc_pad)
- flush_icache_range(tramp, tramp + 2 * sizeof(unsigned long));
-
regs->link = tramp;
#ifdef CONFIG_PPC_FPU_REGS
--
2.25.0
^ 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