* [PATCH v1 2/4] lib/vdso: Add vdso_data pointer as input to __arch_get_timens_vdso_data()
From: Christophe Leroy @ 2021-03-31 14:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1617202094.git.christophe.leroy@csgroup.eu>
For the same reason as commit e876f0b69dc9 ("lib/vdso: Allow
architectures to provide the vdso data pointer"), powerpc wants to
avoid calculation of relative position to code.
As the timens_vdso_data is next page to vdso_data, provide
vdso_data pointer to __arch_get_timens_vdso_data() in order
to ease the calculation on powerpc in following patches.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/arm64/include/asm/vdso/compat_gettimeofday.h | 3 ++-
arch/arm64/include/asm/vdso/gettimeofday.h | 2 +-
arch/s390/include/asm/vdso/gettimeofday.h | 3 ++-
arch/x86/include/asm/vdso/gettimeofday.h | 3 ++-
lib/vdso/gettimeofday.c | 15 +++++++++------
5 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
index 7508b0ac1d21..ecb6fd4c3c64 100644
--- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
@@ -155,7 +155,8 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
}
#ifdef CONFIG_TIME_NS
-static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
const struct vdso_data *ret;
diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
index 631ab1281633..de86230a9436 100644
--- a/arch/arm64/include/asm/vdso/gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/gettimeofday.h
@@ -100,7 +100,7 @@ const struct vdso_data *__arch_get_vdso_data(void)
#ifdef CONFIG_TIME_NS
static __always_inline
-const struct vdso_data *__arch_get_timens_vdso_data(void)
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
return _timens_data;
}
diff --git a/arch/s390/include/asm/vdso/gettimeofday.h b/arch/s390/include/asm/vdso/gettimeofday.h
index ed89ef742530..383c53c3dddd 100644
--- a/arch/s390/include/asm/vdso/gettimeofday.h
+++ b/arch/s390/include/asm/vdso/gettimeofday.h
@@ -68,7 +68,8 @@ long clock_getres_fallback(clockid_t clkid, struct __kernel_timespec *ts)
}
#ifdef CONFIG_TIME_NS
-static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
return _timens_data;
}
diff --git a/arch/x86/include/asm/vdso/gettimeofday.h b/arch/x86/include/asm/vdso/gettimeofday.h
index df01d7349d79..1936f21ed8cd 100644
--- a/arch/x86/include/asm/vdso/gettimeofday.h
+++ b/arch/x86/include/asm/vdso/gettimeofday.h
@@ -58,7 +58,8 @@ extern struct ms_hyperv_tsc_page hvclock_page
#endif
#ifdef CONFIG_TIME_NS
-static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
return __timens_vdso_data;
}
diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index c6f6dee08746..ce2f69552003 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -49,13 +49,15 @@ static inline bool vdso_cycles_ok(u64 cycles)
static __always_inline int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
struct __kernel_timespec *ts)
{
- const struct vdso_data *vd = __arch_get_timens_vdso_data();
+ const struct vdso_data *vd;
const struct timens_offset *offs = &vdns->offset[clk];
const struct vdso_timestamp *vdso_ts;
u64 cycles, last, ns;
u32 seq;
s64 sec;
+ vd = vdns - (clk == CLOCK_MONOTONIC_RAW ? CS_RAW : CS_HRES_COARSE);
+ vd = __arch_get_timens_vdso_data(vd);
if (clk != CLOCK_MONOTONIC_RAW)
vd = &vd[CS_HRES_COARSE];
else
@@ -92,7 +94,8 @@ static __always_inline int do_hres_timens(const struct vdso_data *vdns, clockid_
return 0;
}
#else
-static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
return NULL;
}
@@ -162,7 +165,7 @@ static __always_inline int do_hres(const struct vdso_data *vd, clockid_t clk,
static __always_inline int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
struct __kernel_timespec *ts)
{
- const struct vdso_data *vd = __arch_get_timens_vdso_data();
+ const struct vdso_data *vd = __arch_get_timens_vdso_data(vdns);
const struct vdso_timestamp *vdso_ts = &vd->basetime[clk];
const struct timens_offset *offs = &vdns->offset[clk];
u64 nsec;
@@ -310,7 +313,7 @@ __cvdso_gettimeofday_data(const struct vdso_data *vd,
if (unlikely(tz != NULL)) {
if (IS_ENABLED(CONFIG_TIME_NS) &&
vd->clock_mode == VDSO_CLOCKMODE_TIMENS)
- vd = __arch_get_timens_vdso_data();
+ vd = __arch_get_timens_vdso_data(vd);
tz->tz_minuteswest = vd[CS_HRES_COARSE].tz_minuteswest;
tz->tz_dsttime = vd[CS_HRES_COARSE].tz_dsttime;
@@ -333,7 +336,7 @@ __cvdso_time_data(const struct vdso_data *vd, __kernel_old_time_t *time)
if (IS_ENABLED(CONFIG_TIME_NS) &&
vd->clock_mode == VDSO_CLOCKMODE_TIMENS)
- vd = __arch_get_timens_vdso_data();
+ vd = __arch_get_timens_vdso_data(vd);
t = READ_ONCE(vd[CS_HRES_COARSE].basetime[CLOCK_REALTIME].sec);
@@ -363,7 +366,7 @@ int __cvdso_clock_getres_common(const struct vdso_data *vd, clockid_t clock,
if (IS_ENABLED(CONFIG_TIME_NS) &&
vd->clock_mode == VDSO_CLOCKMODE_TIMENS)
- vd = __arch_get_timens_vdso_data();
+ vd = __arch_get_timens_vdso_data(vd);
/*
* Convert the clockid to a bitmask and use it to check which
--
2.25.0
^ permalink raw reply related
* [PATCH v1 1/4] lib/vdso: Mark do_hres_timens() and do_coarse_timens() __always_inline()
From: Christophe Leroy @ 2021-03-31 14:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1617202094.git.christophe.leroy@csgroup.eu>
In the same spirit as commit c966533f8c6c ("lib/vdso: Mark do_hres()
and do_coarse() as __always_inline"), mark do_hres_timens() and
do_coarse_timens() __always_inline.
The measurement below in on a non timens process, ie on the fastest path.
On powerpc32, without the patch:
clock-gettime-monotonic-raw: vdso: 1155 nsec/call
clock-gettime-monotonic-coarse: vdso: 813 nsec/call
clock-gettime-monotonic: vdso: 1076 nsec/call
With the patch:
clock-gettime-monotonic-raw: vdso: 1100 nsec/call
clock-gettime-monotonic-coarse: vdso: 667 nsec/call
clock-gettime-monotonic: vdso: 1025 nsec/call
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
lib/vdso/gettimeofday.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index 2919f1698140..c6f6dee08746 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -46,8 +46,8 @@ static inline bool vdso_cycles_ok(u64 cycles)
#endif
#ifdef CONFIG_TIME_NS
-static int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
- struct __kernel_timespec *ts)
+static __always_inline int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
+ struct __kernel_timespec *ts)
{
const struct vdso_data *vd = __arch_get_timens_vdso_data();
const struct timens_offset *offs = &vdns->offset[clk];
@@ -97,8 +97,8 @@ static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
return NULL;
}
-static int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
- struct __kernel_timespec *ts)
+static __always_inline int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
+ struct __kernel_timespec *ts)
{
return -EINVAL;
}
@@ -159,8 +159,8 @@ static __always_inline int do_hres(const struct vdso_data *vd, clockid_t clk,
}
#ifdef CONFIG_TIME_NS
-static int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
- struct __kernel_timespec *ts)
+static __always_inline int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
+ struct __kernel_timespec *ts)
{
const struct vdso_data *vd = __arch_get_timens_vdso_data();
const struct vdso_timestamp *vdso_ts = &vd->basetime[clk];
@@ -188,8 +188,8 @@ static int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
return 0;
}
#else
-static int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
- struct __kernel_timespec *ts)
+static __always_inline int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
+ struct __kernel_timespec *ts)
{
return -1;
}
--
2.25.0
^ permalink raw reply related
* [PATCH v1 3/4] powerpc/vdso: Separate vvar vma from vdso
From: Christophe Leroy @ 2021-03-31 14:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1617202094.git.christophe.leroy@csgroup.eu>
From: Dmitry Safonov <dima@arista.com>
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>
Signed-off-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.25.0
^ permalink raw reply related
* [PATCH v1 4/4] powerpc/vdso: Add support for time namespaces
From: Christophe Leroy @ 2021-03-31 14:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1617202094.git.christophe.leroy@csgroup.eu>
This patch adds the necessary glue to provide time namespaces.
Things are mainly copied from ARM64.
__arch_get_timens_vdso_data() calculates timens vdso data position
based on the vdso data position, knowing it is the next page in vvar.
This avoids having to redo the mflr/bcl/mflr/mtlr dance to locate
the page relative to running code position.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/Kconfig | 3 +-
arch/powerpc/include/asm/vdso/gettimeofday.h | 10 ++
arch/powerpc/include/asm/vdso_datapage.h | 2 -
arch/powerpc/kernel/vdso.c | 116 ++++++++++++++++---
arch/powerpc/kernel/vdso32/vdso32.lds.S | 2 +-
arch/powerpc/kernel/vdso64/vdso64.lds.S | 2 +-
6 files changed, 114 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c1344c05226c..71daff5f15d5 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -172,6 +172,7 @@ config PPC
select GENERIC_CPU_AUTOPROBE
select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
select GENERIC_EARLY_IOREMAP
+ select GENERIC_GETTIMEOFDAY
select GENERIC_IRQ_SHOW
select GENERIC_IRQ_SHOW_LEVEL
select GENERIC_PCI_IOMAP if PCI
@@ -179,7 +180,7 @@ config PPC
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
- select GENERIC_GETTIMEOFDAY
+ select GENERIC_VDSO_TIME_NS
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
select HAVE_ARCH_JUMP_LABEL
diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h b/arch/powerpc/include/asm/vdso/gettimeofday.h
index d453e725c79f..e448df1dd071 100644
--- a/arch/powerpc/include/asm/vdso/gettimeofday.h
+++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
@@ -2,6 +2,8 @@
#ifndef _ASM_POWERPC_VDSO_GETTIMEOFDAY_H
#define _ASM_POWERPC_VDSO_GETTIMEOFDAY_H
+#include <asm/page.h>
+
#ifdef __ASSEMBLY__
#include <asm/ppc_asm.h>
@@ -153,6 +155,14 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode,
const struct vdso_data *__arch_get_vdso_data(void);
+#ifdef CONFIG_TIME_NS
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
+{
+ return (void *)vd + PAGE_SIZE;
+}
+#endif
+
static inline bool vdso_clocksource_ok(const struct vdso_data *vd)
{
return true;
diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
index 3f958ecf2beb..a585c8e538ff 100644
--- a/arch/powerpc/include/asm/vdso_datapage.h
+++ b/arch/powerpc/include/asm/vdso_datapage.h
@@ -107,9 +107,7 @@ extern struct vdso_arch_data *vdso_data;
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
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index b14907209822..717f2c9a7573 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -18,6 +18,7 @@
#include <linux/security.h>
#include <linux/memblock.h>
#include <linux/syscalls.h>
+#include <linux/time_namespace.h>
#include <vdso/datapage.h>
#include <asm/syscall.h>
@@ -50,6 +51,12 @@ static union {
} vdso_data_store __page_aligned_data;
struct vdso_arch_data *vdso_data = &vdso_data_store.data;
+enum vvar_pages {
+ VVAR_DATA_PAGE_OFFSET,
+ VVAR_TIMENS_PAGE_OFFSET,
+ VVAR_NR_PAGES,
+};
+
static int vdso_mremap(const struct vm_special_mapping *sm, struct vm_area_struct *new_vma,
unsigned long text_size)
{
@@ -73,8 +80,12 @@ 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 vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
+ struct vm_area_struct *vma, struct vm_fault *vmf);
+
static struct vm_special_mapping vvar_spec __ro_after_init = {
.name = "[vvar]",
+ .fault = vvar_fault,
};
static struct vm_special_mapping vdso32_spec __ro_after_init = {
@@ -87,6 +98,94 @@ static struct vm_special_mapping vdso64_spec __ro_after_init = {
.mremap = vdso64_mremap,
};
+#ifdef CONFIG_TIME_NS
+struct vdso_data *arch_get_vdso_data(void *vvar_page)
+{
+ return ((struct vdso_arch_data *)vvar_page)->data;
+}
+
+/*
+ * The vvar mapping contains data for a specific time namespace, so when a task
+ * changes namespace we must unmap its vvar data for the old namespace.
+ * Subsequent faults will map in data for the new namespace.
+ *
+ * For more details see timens_setup_vdso_data().
+ */
+int vdso_join_timens(struct task_struct *task, struct time_namespace *ns)
+{
+ struct mm_struct *mm = task->mm;
+ struct vm_area_struct *vma;
+
+ mmap_read_lock(mm);
+
+ for (vma = mm->mmap; vma; vma = vma->vm_next) {
+ unsigned long size = vma->vm_end - vma->vm_start;
+
+ if (vma_is_special_mapping(vma, &vvar_spec))
+ zap_page_range(vma, vma->vm_start, size);
+ }
+
+ mmap_read_unlock(mm);
+ return 0;
+}
+
+static struct page *find_timens_vvar_page(struct vm_area_struct *vma)
+{
+ if (likely(vma->vm_mm == current->mm))
+ return current->nsproxy->time_ns->vvar_page;
+
+ /*
+ * VM_PFNMAP | VM_IO protect .fault() handler from being called
+ * through interfaces like /proc/$pid/mem or
+ * process_vm_{readv,writev}() as long as there's no .access()
+ * in special_mapping_vmops.
+ * For more details check_vma_flags() and __access_remote_vm()
+ */
+ WARN(1, "vvar_page accessed remotely");
+
+ return NULL;
+}
+#else
+static struct page *find_timens_vvar_page(struct vm_area_struct *vma)
+{
+ return NULL;
+}
+#endif
+
+static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
+ struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ struct page *timens_page = find_timens_vvar_page(vma);
+ unsigned long pfn;
+
+ switch (vmf->pgoff) {
+ case VVAR_DATA_PAGE_OFFSET:
+ if (timens_page)
+ pfn = page_to_pfn(timens_page);
+ else
+ pfn = virt_to_pfn(vdso_data);
+ break;
+#ifdef CONFIG_TIME_NS
+ case VVAR_TIMENS_PAGE_OFFSET:
+ /*
+ * If a task belongs to a time namespace then a namespace
+ * specific VVAR is mapped with the VVAR_DATA_PAGE_OFFSET and
+ * the real VVAR page is mapped with the VVAR_TIMENS_PAGE_OFFSET
+ * offset.
+ * See also the comment near timens_setup_vdso_data().
+ */
+ if (!timens_page)
+ return VM_FAULT_SIGBUS;
+ pfn = virt_to_pfn(vdso_data);
+ break;
+#endif /* CONFIG_TIME_NS */
+ default:
+ return VM_FAULT_SIGBUS;
+ }
+
+ return vmf_insert_pfn(vma, vmf->address, pfn);
+}
+
/*
* This is called from binfmt_elf, we create the special vma for the
* vDSO and insert it into the mm struct tree
@@ -95,7 +194,7 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
{
unsigned long vdso_size, vdso_base, mappings_size;
struct vm_special_mapping *vdso_spec;
- unsigned long vvar_size = PAGE_SIZE;
+ unsigned long vvar_size = VVAR_NR_PAGES * PAGE_SIZE;
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
@@ -266,19 +365,6 @@ static struct page ** __init vdso_setup_pages(void *start, void *end)
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;
-}
-
static int __init vdso_init(void)
{
#ifdef CONFIG_PPC64
@@ -317,8 +403,6 @@ 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;
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index a4b806b0d618..58e0099f70f4 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -17,7 +17,7 @@ ENTRY(_start)
SECTIONS
{
- PROVIDE(_vdso_datapage = . - PAGE_SIZE);
+ PROVIDE(_vdso_datapage = . - 2 * PAGE_SIZE);
. = SIZEOF_HEADERS;
.hash : { *(.hash) } :text
diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
index 2f3c359cacd3..0288cad428b0 100644
--- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
+++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
@@ -17,7 +17,7 @@ ENTRY(_start)
SECTIONS
{
- PROVIDE(_vdso_datapage = . - PAGE_SIZE);
+ PROVIDE(_vdso_datapage = . - 2 * PAGE_SIZE);
. = SIZEOF_HEADERS;
.hash : { *(.hash) } :text
--
2.25.0
^ permalink raw reply related
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Will Deacon @ 2021-03-31 15:32 UTC (permalink / raw)
To: Robin Murphy
Cc: kvm, linux-arm-msm, linuxppc-dev, dri-devel, Li Yang, iommu,
Christoph Hellwig, netdev, virtualization, freedreno,
David Woodhouse, linux-arm-kernel
In-Reply-To: <ef895942-e115-7878-ab86-37e8a1614df5@arm.com>
On Wed, Mar 31, 2021 at 02:09:37PM +0100, Robin Murphy wrote:
> On 2021-03-31 12:49, Will Deacon wrote:
> > On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote:
> > > On 2021-03-30 14:58, Will Deacon wrote:
> > > > On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote:
> > > > > On 2021-03-30 14:11, Will Deacon wrote:
> > > > > > On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote:
> > > > > > > From: Robin Murphy <robin.murphy@arm.com>
> > > > > > >
> > > > > > > Instead make the global iommu_dma_strict paramete in iommu.c canonical by
> > > > > > > exporting helpers to get and set it and use those directly in the drivers.
> > > > > > >
> > > > > > > This make sure that the iommu.strict parameter also works for the AMD and
> > > > > > > Intel IOMMU drivers on x86. As those default to lazy flushing a new
> > > > > > > IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to
> > > > > > > represent the default if not overriden by an explicit parameter.
> > > > > > >
> > > > > > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>.
> > > > > > > [ported on top of the other iommu_attr changes and added a few small
> > > > > > > missing bits]
> > > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > > > > ---
> > > > > > > drivers/iommu/amd/iommu.c | 23 +-------
> > > > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +---------------
> > > > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 -
> > > > > > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +--------
> > > > > > > drivers/iommu/dma-iommu.c | 9 +--
> > > > > > > drivers/iommu/intel/iommu.c | 64 ++++-----------------
> > > > > > > drivers/iommu/iommu.c | 27 ++++++---
> > > > > > > include/linux/iommu.h | 4 +-
> > > > > > > 8 files changed, 40 insertions(+), 165 deletions(-)
> > > > > >
> > > > > > I really like this cleanup, but I can't help wonder if it's going in the
> > > > > > wrong direction. With SoCs often having multiple IOMMU instances and a
> > > > > > distinction between "trusted" and "untrusted" devices, then having the
> > > > > > flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound
> > > > > > unreasonable to me, but this change makes it a global property.
> > > > >
> > > > > The intent here was just to streamline the existing behaviour of stuffing a
> > > > > global property into a domain attribute then pulling it out again in the
> > > > > illusion that it was in any way per-domain. We're still checking
> > > > > dev_is_untrusted() before making an actual decision, and it's not like we
> > > > > can't add more factors at that point if we want to.
> > > >
> > > > Like I say, the cleanup is great. I'm just wondering whether there's a
> > > > better way to express the complicated logic to decide whether or not to use
> > > > the flush queue than what we end up with:
> > > >
> > > > if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) &&
> > > > domain->ops->flush_iotlb_all && !iommu_get_dma_strict())
> > > >
> > > > which is mixing up globals, device properties and domain properties. The
> > > > result is that the driver code ends up just using the global to determine
> > > > whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code,
> > > > which is a departure from the current way of doing things.
> > >
> > > But previously, SMMU only ever saw the global policy piped through the
> > > domain attribute by iommu_group_alloc_default_domain(), so there's no
> > > functional change there.
> >
> > For DMA domains sure, but I don't think that's the case for unmanaged
> > domains such as those used by VFIO.
>
> Eh? This is only relevant to DMA domains anyway. Flush queues are part of
> the IOVA allocator that VFIO doesn't even use. It's always been the case
> that unmanaged domains only use strict invalidation.
Maybe I'm going mad. With this patch, the SMMU driver unconditionally sets
IO_PGTABLE_QUIRK_NON_STRICT for page-tables if iommu_get_dma_strict() is
true, no? In which case, that will get set for page-tables corresponding
to unmanaged domains as well as DMA domains when it is enabled. That didn't
happen before because you couldn't set the attribute for unmanaged domains.
What am I missing?
> > > Obviously some of the above checks could be factored out into some kind of
> > > iommu_use_flush_queue() helper that IOMMU drivers can also call if they need
> > > to keep in sync. Or maybe we just allow iommu-dma to set
> > > IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're
> > > treating that as a generic thing now.
> >
> > I think a helper that takes a domain would be a good starting point.
>
> You mean device, right? The one condition we currently have is at the device
> level, and there's really nothing inherent to the domain itself that matters
> (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this).
Device would probably work too; you'd pass the first device to attach to the
domain when querying this from the SMMU driver, I suppose.
Will
^ permalink raw reply
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Robin Murphy @ 2021-03-31 16:05 UTC (permalink / raw)
To: Will Deacon
Cc: kvm, linux-arm-msm, linuxppc-dev, dri-devel, Li Yang, iommu,
Christoph Hellwig, netdev, virtualization, freedreno,
David Woodhouse, linux-arm-kernel
In-Reply-To: <20210331153256.GA7815@willie-the-truck>
On 2021-03-31 16:32, Will Deacon wrote:
> On Wed, Mar 31, 2021 at 02:09:37PM +0100, Robin Murphy wrote:
>> On 2021-03-31 12:49, Will Deacon wrote:
>>> On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote:
>>>> On 2021-03-30 14:58, Will Deacon wrote:
>>>>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote:
>>>>>> On 2021-03-30 14:11, Will Deacon wrote:
>>>>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote:
>>>>>>>> From: Robin Murphy <robin.murphy@arm.com>
>>>>>>>>
>>>>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by
>>>>>>>> exporting helpers to get and set it and use those directly in the drivers.
>>>>>>>>
>>>>>>>> This make sure that the iommu.strict parameter also works for the AMD and
>>>>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new
>>>>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to
>>>>>>>> represent the default if not overriden by an explicit parameter.
>>>>>>>>
>>>>>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>.
>>>>>>>> [ported on top of the other iommu_attr changes and added a few small
>>>>>>>> missing bits]
>>>>>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>>>>>>> ---
>>>>>>>> drivers/iommu/amd/iommu.c | 23 +-------
>>>>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +---------------
>>>>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 -
>>>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +--------
>>>>>>>> drivers/iommu/dma-iommu.c | 9 +--
>>>>>>>> drivers/iommu/intel/iommu.c | 64 ++++-----------------
>>>>>>>> drivers/iommu/iommu.c | 27 ++++++---
>>>>>>>> include/linux/iommu.h | 4 +-
>>>>>>>> 8 files changed, 40 insertions(+), 165 deletions(-)
>>>>>>>
>>>>>>> I really like this cleanup, but I can't help wonder if it's going in the
>>>>>>> wrong direction. With SoCs often having multiple IOMMU instances and a
>>>>>>> distinction between "trusted" and "untrusted" devices, then having the
>>>>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound
>>>>>>> unreasonable to me, but this change makes it a global property.
>>>>>>
>>>>>> The intent here was just to streamline the existing behaviour of stuffing a
>>>>>> global property into a domain attribute then pulling it out again in the
>>>>>> illusion that it was in any way per-domain. We're still checking
>>>>>> dev_is_untrusted() before making an actual decision, and it's not like we
>>>>>> can't add more factors at that point if we want to.
>>>>>
>>>>> Like I say, the cleanup is great. I'm just wondering whether there's a
>>>>> better way to express the complicated logic to decide whether or not to use
>>>>> the flush queue than what we end up with:
>>>>>
>>>>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) &&
>>>>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict())
>>>>>
>>>>> which is mixing up globals, device properties and domain properties. The
>>>>> result is that the driver code ends up just using the global to determine
>>>>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code,
>>>>> which is a departure from the current way of doing things.
>>>>
>>>> But previously, SMMU only ever saw the global policy piped through the
>>>> domain attribute by iommu_group_alloc_default_domain(), so there's no
>>>> functional change there.
>>>
>>> For DMA domains sure, but I don't think that's the case for unmanaged
>>> domains such as those used by VFIO.
>>
>> Eh? This is only relevant to DMA domains anyway. Flush queues are part of
>> the IOVA allocator that VFIO doesn't even use. It's always been the case
>> that unmanaged domains only use strict invalidation.
>
> Maybe I'm going mad. With this patch, the SMMU driver unconditionally sets
> IO_PGTABLE_QUIRK_NON_STRICT for page-tables if iommu_get_dma_strict() is
> true, no? In which case, that will get set for page-tables corresponding
> to unmanaged domains as well as DMA domains when it is enabled. That didn't
> happen before because you couldn't set the attribute for unmanaged domains.
>
> What am I missing?
Oh cock... sorry, all this time I've been saying what I *expect* it to
do, while overlooking the fact that the IO_PGTABLE_QUIRK_NON_STRICT
hunks were the bits I forgot to write and Christoph had to fix up.
Indeed, those should be checking the domain type too to preserve the
existing behaviour. Apologies for the confusion.
Robin.
>>>> Obviously some of the above checks could be factored out into some kind of
>>>> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need
>>>> to keep in sync. Or maybe we just allow iommu-dma to set
>>>> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're
>>>> treating that as a generic thing now.
>>>
>>> I think a helper that takes a domain would be a good starting point.
>>
>> You mean device, right? The one condition we currently have is at the device
>> level, and there's really nothing inherent to the domain itself that matters
>> (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this).
>
> Device would probably work too; you'd pass the first device to attach to the
> domain when querying this from the SMMU driver, I suppose.
>
> Will
>
^ permalink raw reply
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Robin Murphy @ 2021-03-31 16:07 UTC (permalink / raw)
To: Christoph Hellwig, Joerg Roedel, Will Deacon, Li Yang
Cc: kvm, linux-arm-msm, linuxppc-dev, dri-devel, virtualization,
iommu, netdev, freedreno, David Woodhouse, linux-arm-kernel
In-Reply-To: <20210316153825.135976-17-hch@lst.de>
On 2021-03-16 15:38, Christoph Hellwig wrote:
[...]
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index f1e38526d5bd40..996dfdf9d375dd 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2017,7 +2017,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
> .iommu_dev = smmu->dev,
> };
>
> - if (smmu_domain->non_strict)
> + if (!iommu_get_dma_strict())
As Will raised, this also needs to be checking "domain->type ==
IOMMU_DOMAIN_DMA" to maintain equivalent behaviour to the attribute code
below.
> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>
> pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain);
> @@ -2449,52 +2449,6 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev)
> return group;
> }
>
> -static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
> - enum iommu_attr attr, void *data)
> -{
> - struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> -
> - switch (domain->type) {
> - case IOMMU_DOMAIN_DMA:
> - switch (attr) {
> - case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
> - *(int *)data = smmu_domain->non_strict;
> - return 0;
> - default:
> - return -ENODEV;
> - }
> - break;
> - default:
> - return -EINVAL;
> - }
> -}
[...]
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index f985817c967a25..edb1de479dd1a7 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -668,7 +668,6 @@ struct arm_smmu_domain {
> struct mutex init_mutex; /* Protects smmu pointer */
>
> struct io_pgtable_ops *pgtbl_ops;
> - bool non_strict;
> atomic_t nr_ats_masters;
>
> enum arm_smmu_domain_stage stage;
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 0aa6d667274970..3dde22b1f8ffb0 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -761,6 +761,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> .iommu_dev = smmu->dev,
> };
>
> + if (!iommu_get_dma_strict())
Ditto here.
Sorry for not spotting that sooner :(
Robin.
> + pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
> +
> if (smmu->impl && smmu->impl->init_context) {
> ret = smmu->impl->init_context(smmu_domain, &pgtbl_cfg, dev);
> if (ret)
^ permalink raw reply
* Re: [PATCH 6/8] drivers: firmware: efi: libstub: enable generic commandline
From: Ard Biesheuvel @ 2021-03-31 16:10 UTC (permalink / raw)
To: Daniel Walker, Arvind Sankar
Cc: ob Herring, linux-efi, Daniel Gimpelevich,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), X86 ML,
open list:MIPS, Linux Kernel Mailing List, xe-linux-external,
Andrew Morton, Will Deacon
In-Reply-To: <e5d98d566c38d6f8516b8d9d1fd603ec1f131037.1617126961.git.danielwa@cisco.com>
(+ Arvind)
On Tue, 30 Mar 2021 at 19:57, Daniel Walker <danielwa@cisco.com> wrote:
>
> This adds code to handle the generic command line changes.
> The efi code appears that it doesn't benefit as much from this design
> as it could.
>
> For example, if you had a prepend command line with "nokaslr" then
> you might be helpful to re-enable it in the boot loader or dts,
> but there appears to be no way to re-enable kaslr or some of the
> other options.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> .../firmware/efi/libstub/efi-stub-helper.c | 35 +++++++++++++++++++
> drivers/firmware/efi/libstub/efi-stub.c | 7 ++++
> drivers/firmware/efi/libstub/efistub.h | 1 +
> drivers/firmware/efi/libstub/x86-stub.c | 13 +++++--
> 4 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index aa8da0a49829..c155837cedc9 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -13,6 +13,7 @@
> #include <linux/efi.h>
> #include <linux/kernel.h>
> #include <linux/printk.h> /* For CONSOLE_LOGLEVEL_* */
> +#include <linux/cmdline.h>
> #include <asm/efi.h>
> #include <asm/setup.h>
>
> @@ -172,6 +173,40 @@ int efi_printk(const char *fmt, ...)
> return printed;
> }
>
> +/**
> + * efi_handle_cmdline() - handle adding in building parts of the command line
> + * @cmdline: kernel command line
> + *
> + * Add in the generic parts of the commandline and start the parsing of the
> + * command line.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_handle_cmdline(char const *cmdline)
> +{
> + efi_status_t status;
> +
> + status = efi_parse_options(CMDLINE_PREPEND);
> + if (status != EFI_SUCCESS) {
> + efi_err("Failed to parse options\n");
> + return status;
> + }
Even though I am not a fan of the 'success handling' pattern,
duplicating the exact same error handling three times is not great
either. Could we reuse more of the code here?
> +
> + status = efi_parse_options(IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) ? "" : cmdline);
What is the point of calling efi_parse_options() with an empty string?
> + if (status != EFI_SUCCESS) {
> + efi_err("Failed to parse options\n");
> + return status;
> + }
> +
> + status = efi_parse_options(CMDLINE_APPEND);
> + if (status != EFI_SUCCESS) {
> + efi_err("Failed to parse options\n");
> + return status;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> * efi_parse_options() - Parse EFI command line options
> * @cmdline: kernel command line
> diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
> index 26e69788f27a..760480248adf 100644
> --- a/drivers/firmware/efi/libstub/efi-stub.c
> +++ b/drivers/firmware/efi/libstub/efi-stub.c
> @@ -172,6 +172,12 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
> goto fail;
> }
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> + status = efi_handle_cmdline(cmdline_ptr);
> + if (status != EFI_SUCCESS) {
> + goto fail_free_cmdline;
> + }
> +#else
> if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
> IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
Does this mean CONFIG_GENERIC_CMDLINE does not replace CMDLINE_EXTEND
/ CMDLINE_FORCE etc, but introduces yet another variant on top of
those?
That does not seem like an improvement to me. I think it is great that
you are cleaning this up, but only if it means we can get rid of the
old implementation.
> cmdline_size == 0) {
> @@ -189,6 +195,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
> goto fail_free_cmdline;
> }
> }
> +#endif
>
> efi_info("Booting Linux Kernel...\n");
>
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index cde0a2ef507d..07c7f9fdfffc 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -800,6 +800,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
> unsigned long alignment,
> unsigned long min_addr);
>
> +efi_status_t efi_handle_cmdline(char const *cmdline);
> efi_status_t efi_parse_options(char const *cmdline);
>
> void efi_parse_option_graphics(char *option);
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index f14c4ff5839f..30ad8fb7122d 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -673,6 +673,8 @@ unsigned long efi_main(efi_handle_t handle,
> unsigned long bzimage_addr = (unsigned long)startup_32;
> unsigned long buffer_start, buffer_end;
> struct setup_header *hdr = &boot_params->hdr;
> + unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> + ((u64)boot_params->ext_cmd_line_ptr << 32));
> efi_status_t status;
>
> efi_system_table = sys_table_arg;
> @@ -735,6 +737,14 @@ unsigned long efi_main(efi_handle_t handle,
> image_offset = 0;
> }
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> + status = efi_handle_cmdline((char *)cmdline_paddr);
> + if (status != EFI_SUCCESS) {
> + efi_err("Failed to parse options\n");
> + goto fail;
> + }
> +#else /* CONFIG_GENERIC_CMDLINE */
> +
> #ifdef CONFIG_CMDLINE_BOOL
> status = efi_parse_options(CONFIG_CMDLINE);
> if (status != EFI_SUCCESS) {
> @@ -743,8 +753,6 @@ unsigned long efi_main(efi_handle_t handle,
> }
> #endif
> if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
> - unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> - ((u64)boot_params->ext_cmd_line_ptr << 32));
> status = efi_parse_options((char *)cmdline_paddr);
> if (status != EFI_SUCCESS) {
> efi_err("Failed to parse options\n");
> @@ -752,6 +760,7 @@ unsigned long efi_main(efi_handle_t handle,
> }
> }
>
> +#endif
> /*
> * At this point, an initrd may already have been loaded by the
> * bootloader and passed via bootparams. We permit an initrd loaded
> --
> 2.25.1
>
^ permalink raw reply
* [PATCH RESEND v1 0/4] powerpc/vdso: Add support for time namespaces
From: Christophe Leroy @ 2021-03-31 16:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-arch, arnd, dima, linux-kernel, avagin, luto, tglx,
vincenzo.frascino, linuxppc-dev
[Sorry, resending with complete destination list, I used the wrong script on the first delivery]
This series adds support for time namespaces on powerpc.
All timens selftests are successfull.
Christophe Leroy (3):
lib/vdso: Mark do_hres_timens() and do_coarse_timens()
__always_inline()
lib/vdso: Add vdso_data pointer as input to
__arch_get_timens_vdso_data()
powerpc/vdso: Add support for time namespaces
Dmitry Safonov (1):
powerpc/vdso: Separate vvar vma from vdso
.../include/asm/vdso/compat_gettimeofday.h | 3 +-
arch/arm64/include/asm/vdso/gettimeofday.h | 2 +-
arch/powerpc/Kconfig | 3 +-
arch/powerpc/include/asm/mmu_context.h | 2 +-
arch/powerpc/include/asm/vdso/gettimeofday.h | 10 ++
arch/powerpc/include/asm/vdso_datapage.h | 2 -
arch/powerpc/kernel/vdso.c | 138 ++++++++++++++++--
arch/powerpc/kernel/vdso32/vdso32.lds.S | 2 +-
arch/powerpc/kernel/vdso64/vdso64.lds.S | 2 +-
arch/s390/include/asm/vdso/gettimeofday.h | 3 +-
arch/x86/include/asm/vdso/gettimeofday.h | 3 +-
lib/vdso/gettimeofday.c | 31 ++--
12 files changed, 162 insertions(+), 39 deletions(-)
--
2.25.0
^ permalink raw reply
* [PATCH RESEND v1 2/4] lib/vdso: Add vdso_data pointer as input to __arch_get_timens_vdso_data()
From: Christophe Leroy @ 2021-03-31 16:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-arch, arnd, dima, linux-kernel, avagin, luto, tglx,
vincenzo.frascino, linuxppc-dev
In-Reply-To: <cover.1617209141.git.christophe.leroy@csgroup.eu>
For the same reason as commit e876f0b69dc9 ("lib/vdso: Allow
architectures to provide the vdso data pointer"), powerpc wants to
avoid calculation of relative position to code.
As the timens_vdso_data is next page to vdso_data, provide
vdso_data pointer to __arch_get_timens_vdso_data() in order
to ease the calculation on powerpc in following patches.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/arm64/include/asm/vdso/compat_gettimeofday.h | 3 ++-
arch/arm64/include/asm/vdso/gettimeofday.h | 2 +-
arch/s390/include/asm/vdso/gettimeofday.h | 3 ++-
arch/x86/include/asm/vdso/gettimeofday.h | 3 ++-
lib/vdso/gettimeofday.c | 15 +++++++++------
5 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
index 7508b0ac1d21..ecb6fd4c3c64 100644
--- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
@@ -155,7 +155,8 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
}
#ifdef CONFIG_TIME_NS
-static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
const struct vdso_data *ret;
diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
index 631ab1281633..de86230a9436 100644
--- a/arch/arm64/include/asm/vdso/gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/gettimeofday.h
@@ -100,7 +100,7 @@ const struct vdso_data *__arch_get_vdso_data(void)
#ifdef CONFIG_TIME_NS
static __always_inline
-const struct vdso_data *__arch_get_timens_vdso_data(void)
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
return _timens_data;
}
diff --git a/arch/s390/include/asm/vdso/gettimeofday.h b/arch/s390/include/asm/vdso/gettimeofday.h
index ed89ef742530..383c53c3dddd 100644
--- a/arch/s390/include/asm/vdso/gettimeofday.h
+++ b/arch/s390/include/asm/vdso/gettimeofday.h
@@ -68,7 +68,8 @@ long clock_getres_fallback(clockid_t clkid, struct __kernel_timespec *ts)
}
#ifdef CONFIG_TIME_NS
-static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
return _timens_data;
}
diff --git a/arch/x86/include/asm/vdso/gettimeofday.h b/arch/x86/include/asm/vdso/gettimeofday.h
index df01d7349d79..1936f21ed8cd 100644
--- a/arch/x86/include/asm/vdso/gettimeofday.h
+++ b/arch/x86/include/asm/vdso/gettimeofday.h
@@ -58,7 +58,8 @@ extern struct ms_hyperv_tsc_page hvclock_page
#endif
#ifdef CONFIG_TIME_NS
-static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
return __timens_vdso_data;
}
diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index c6f6dee08746..ce2f69552003 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -49,13 +49,15 @@ static inline bool vdso_cycles_ok(u64 cycles)
static __always_inline int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
struct __kernel_timespec *ts)
{
- const struct vdso_data *vd = __arch_get_timens_vdso_data();
+ const struct vdso_data *vd;
const struct timens_offset *offs = &vdns->offset[clk];
const struct vdso_timestamp *vdso_ts;
u64 cycles, last, ns;
u32 seq;
s64 sec;
+ vd = vdns - (clk == CLOCK_MONOTONIC_RAW ? CS_RAW : CS_HRES_COARSE);
+ vd = __arch_get_timens_vdso_data(vd);
if (clk != CLOCK_MONOTONIC_RAW)
vd = &vd[CS_HRES_COARSE];
else
@@ -92,7 +94,8 @@ static __always_inline int do_hres_timens(const struct vdso_data *vdns, clockid_
return 0;
}
#else
-static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
return NULL;
}
@@ -162,7 +165,7 @@ static __always_inline int do_hres(const struct vdso_data *vd, clockid_t clk,
static __always_inline int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
struct __kernel_timespec *ts)
{
- const struct vdso_data *vd = __arch_get_timens_vdso_data();
+ const struct vdso_data *vd = __arch_get_timens_vdso_data(vdns);
const struct vdso_timestamp *vdso_ts = &vd->basetime[clk];
const struct timens_offset *offs = &vdns->offset[clk];
u64 nsec;
@@ -310,7 +313,7 @@ __cvdso_gettimeofday_data(const struct vdso_data *vd,
if (unlikely(tz != NULL)) {
if (IS_ENABLED(CONFIG_TIME_NS) &&
vd->clock_mode == VDSO_CLOCKMODE_TIMENS)
- vd = __arch_get_timens_vdso_data();
+ vd = __arch_get_timens_vdso_data(vd);
tz->tz_minuteswest = vd[CS_HRES_COARSE].tz_minuteswest;
tz->tz_dsttime = vd[CS_HRES_COARSE].tz_dsttime;
@@ -333,7 +336,7 @@ __cvdso_time_data(const struct vdso_data *vd, __kernel_old_time_t *time)
if (IS_ENABLED(CONFIG_TIME_NS) &&
vd->clock_mode == VDSO_CLOCKMODE_TIMENS)
- vd = __arch_get_timens_vdso_data();
+ vd = __arch_get_timens_vdso_data(vd);
t = READ_ONCE(vd[CS_HRES_COARSE].basetime[CLOCK_REALTIME].sec);
@@ -363,7 +366,7 @@ int __cvdso_clock_getres_common(const struct vdso_data *vd, clockid_t clock,
if (IS_ENABLED(CONFIG_TIME_NS) &&
vd->clock_mode == VDSO_CLOCKMODE_TIMENS)
- vd = __arch_get_timens_vdso_data();
+ vd = __arch_get_timens_vdso_data(vd);
/*
* Convert the clockid to a bitmask and use it to check which
--
2.25.0
^ permalink raw reply related
* [PATCH RESEND v1 1/4] lib/vdso: Mark do_hres_timens() and do_coarse_timens() __always_inline()
From: Christophe Leroy @ 2021-03-31 16:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-arch, arnd, dima, linux-kernel, avagin, luto, tglx,
vincenzo.frascino, linuxppc-dev
In-Reply-To: <cover.1617209141.git.christophe.leroy@csgroup.eu>
In the same spirit as commit c966533f8c6c ("lib/vdso: Mark do_hres()
and do_coarse() as __always_inline"), mark do_hres_timens() and
do_coarse_timens() __always_inline.
The measurement below in on a non timens process, ie on the fastest path.
On powerpc32, without the patch:
clock-gettime-monotonic-raw: vdso: 1155 nsec/call
clock-gettime-monotonic-coarse: vdso: 813 nsec/call
clock-gettime-monotonic: vdso: 1076 nsec/call
With the patch:
clock-gettime-monotonic-raw: vdso: 1100 nsec/call
clock-gettime-monotonic-coarse: vdso: 667 nsec/call
clock-gettime-monotonic: vdso: 1025 nsec/call
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
lib/vdso/gettimeofday.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index 2919f1698140..c6f6dee08746 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -46,8 +46,8 @@ static inline bool vdso_cycles_ok(u64 cycles)
#endif
#ifdef CONFIG_TIME_NS
-static int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
- struct __kernel_timespec *ts)
+static __always_inline int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
+ struct __kernel_timespec *ts)
{
const struct vdso_data *vd = __arch_get_timens_vdso_data();
const struct timens_offset *offs = &vdns->offset[clk];
@@ -97,8 +97,8 @@ static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
return NULL;
}
-static int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
- struct __kernel_timespec *ts)
+static __always_inline int do_hres_timens(const struct vdso_data *vdns, clockid_t clk,
+ struct __kernel_timespec *ts)
{
return -EINVAL;
}
@@ -159,8 +159,8 @@ static __always_inline int do_hres(const struct vdso_data *vd, clockid_t clk,
}
#ifdef CONFIG_TIME_NS
-static int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
- struct __kernel_timespec *ts)
+static __always_inline int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
+ struct __kernel_timespec *ts)
{
const struct vdso_data *vd = __arch_get_timens_vdso_data();
const struct vdso_timestamp *vdso_ts = &vd->basetime[clk];
@@ -188,8 +188,8 @@ static int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
return 0;
}
#else
-static int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
- struct __kernel_timespec *ts)
+static __always_inline int do_coarse_timens(const struct vdso_data *vdns, clockid_t clk,
+ struct __kernel_timespec *ts)
{
return -1;
}
--
2.25.0
^ permalink raw reply related
* [PATCH RESEND v1 3/4] powerpc/vdso: Separate vvar vma from vdso
From: Christophe Leroy @ 2021-03-31 16:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-arch, arnd, dima, linux-kernel, avagin, luto, tglx,
vincenzo.frascino, linuxppc-dev
In-Reply-To: <cover.1617209141.git.christophe.leroy@csgroup.eu>
From: Dmitry Safonov <dima@arista.com>
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>
Signed-off-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.25.0
^ permalink raw reply related
* [PATCH RESEND v1 4/4] powerpc/vdso: Add support for time namespaces
From: Christophe Leroy @ 2021-03-31 16:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-arch, arnd, dima, linux-kernel, avagin, luto, tglx,
vincenzo.frascino, linuxppc-dev
In-Reply-To: <cover.1617209141.git.christophe.leroy@csgroup.eu>
This patch adds the necessary glue to provide time namespaces.
Things are mainly copied from ARM64.
__arch_get_timens_vdso_data() calculates timens vdso data position
based on the vdso data position, knowing it is the next page in vvar.
This avoids having to redo the mflr/bcl/mflr/mtlr dance to locate
the page relative to running code position.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/Kconfig | 3 +-
arch/powerpc/include/asm/vdso/gettimeofday.h | 10 ++
arch/powerpc/include/asm/vdso_datapage.h | 2 -
arch/powerpc/kernel/vdso.c | 116 ++++++++++++++++---
arch/powerpc/kernel/vdso32/vdso32.lds.S | 2 +-
arch/powerpc/kernel/vdso64/vdso64.lds.S | 2 +-
6 files changed, 114 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c1344c05226c..71daff5f15d5 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -172,6 +172,7 @@ config PPC
select GENERIC_CPU_AUTOPROBE
select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
select GENERIC_EARLY_IOREMAP
+ select GENERIC_GETTIMEOFDAY
select GENERIC_IRQ_SHOW
select GENERIC_IRQ_SHOW_LEVEL
select GENERIC_PCI_IOMAP if PCI
@@ -179,7 +180,7 @@ config PPC
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
- select GENERIC_GETTIMEOFDAY
+ select GENERIC_VDSO_TIME_NS
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_HUGE_VMAP if PPC_BOOK3S_64 && PPC_RADIX_MMU
select HAVE_ARCH_JUMP_LABEL
diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h b/arch/powerpc/include/asm/vdso/gettimeofday.h
index d453e725c79f..e448df1dd071 100644
--- a/arch/powerpc/include/asm/vdso/gettimeofday.h
+++ b/arch/powerpc/include/asm/vdso/gettimeofday.h
@@ -2,6 +2,8 @@
#ifndef _ASM_POWERPC_VDSO_GETTIMEOFDAY_H
#define _ASM_POWERPC_VDSO_GETTIMEOFDAY_H
+#include <asm/page.h>
+
#ifdef __ASSEMBLY__
#include <asm/ppc_asm.h>
@@ -153,6 +155,14 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode,
const struct vdso_data *__arch_get_vdso_data(void);
+#ifdef CONFIG_TIME_NS
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
+{
+ return (void *)vd + PAGE_SIZE;
+}
+#endif
+
static inline bool vdso_clocksource_ok(const struct vdso_data *vd)
{
return true;
diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
index 3f958ecf2beb..a585c8e538ff 100644
--- a/arch/powerpc/include/asm/vdso_datapage.h
+++ b/arch/powerpc/include/asm/vdso_datapage.h
@@ -107,9 +107,7 @@ extern struct vdso_arch_data *vdso_data;
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
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index b14907209822..717f2c9a7573 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -18,6 +18,7 @@
#include <linux/security.h>
#include <linux/memblock.h>
#include <linux/syscalls.h>
+#include <linux/time_namespace.h>
#include <vdso/datapage.h>
#include <asm/syscall.h>
@@ -50,6 +51,12 @@ static union {
} vdso_data_store __page_aligned_data;
struct vdso_arch_data *vdso_data = &vdso_data_store.data;
+enum vvar_pages {
+ VVAR_DATA_PAGE_OFFSET,
+ VVAR_TIMENS_PAGE_OFFSET,
+ VVAR_NR_PAGES,
+};
+
static int vdso_mremap(const struct vm_special_mapping *sm, struct vm_area_struct *new_vma,
unsigned long text_size)
{
@@ -73,8 +80,12 @@ 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 vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
+ struct vm_area_struct *vma, struct vm_fault *vmf);
+
static struct vm_special_mapping vvar_spec __ro_after_init = {
.name = "[vvar]",
+ .fault = vvar_fault,
};
static struct vm_special_mapping vdso32_spec __ro_after_init = {
@@ -87,6 +98,94 @@ static struct vm_special_mapping vdso64_spec __ro_after_init = {
.mremap = vdso64_mremap,
};
+#ifdef CONFIG_TIME_NS
+struct vdso_data *arch_get_vdso_data(void *vvar_page)
+{
+ return ((struct vdso_arch_data *)vvar_page)->data;
+}
+
+/*
+ * The vvar mapping contains data for a specific time namespace, so when a task
+ * changes namespace we must unmap its vvar data for the old namespace.
+ * Subsequent faults will map in data for the new namespace.
+ *
+ * For more details see timens_setup_vdso_data().
+ */
+int vdso_join_timens(struct task_struct *task, struct time_namespace *ns)
+{
+ struct mm_struct *mm = task->mm;
+ struct vm_area_struct *vma;
+
+ mmap_read_lock(mm);
+
+ for (vma = mm->mmap; vma; vma = vma->vm_next) {
+ unsigned long size = vma->vm_end - vma->vm_start;
+
+ if (vma_is_special_mapping(vma, &vvar_spec))
+ zap_page_range(vma, vma->vm_start, size);
+ }
+
+ mmap_read_unlock(mm);
+ return 0;
+}
+
+static struct page *find_timens_vvar_page(struct vm_area_struct *vma)
+{
+ if (likely(vma->vm_mm == current->mm))
+ return current->nsproxy->time_ns->vvar_page;
+
+ /*
+ * VM_PFNMAP | VM_IO protect .fault() handler from being called
+ * through interfaces like /proc/$pid/mem or
+ * process_vm_{readv,writev}() as long as there's no .access()
+ * in special_mapping_vmops.
+ * For more details check_vma_flags() and __access_remote_vm()
+ */
+ WARN(1, "vvar_page accessed remotely");
+
+ return NULL;
+}
+#else
+static struct page *find_timens_vvar_page(struct vm_area_struct *vma)
+{
+ return NULL;
+}
+#endif
+
+static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
+ struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ struct page *timens_page = find_timens_vvar_page(vma);
+ unsigned long pfn;
+
+ switch (vmf->pgoff) {
+ case VVAR_DATA_PAGE_OFFSET:
+ if (timens_page)
+ pfn = page_to_pfn(timens_page);
+ else
+ pfn = virt_to_pfn(vdso_data);
+ break;
+#ifdef CONFIG_TIME_NS
+ case VVAR_TIMENS_PAGE_OFFSET:
+ /*
+ * If a task belongs to a time namespace then a namespace
+ * specific VVAR is mapped with the VVAR_DATA_PAGE_OFFSET and
+ * the real VVAR page is mapped with the VVAR_TIMENS_PAGE_OFFSET
+ * offset.
+ * See also the comment near timens_setup_vdso_data().
+ */
+ if (!timens_page)
+ return VM_FAULT_SIGBUS;
+ pfn = virt_to_pfn(vdso_data);
+ break;
+#endif /* CONFIG_TIME_NS */
+ default:
+ return VM_FAULT_SIGBUS;
+ }
+
+ return vmf_insert_pfn(vma, vmf->address, pfn);
+}
+
/*
* This is called from binfmt_elf, we create the special vma for the
* vDSO and insert it into the mm struct tree
@@ -95,7 +194,7 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
{
unsigned long vdso_size, vdso_base, mappings_size;
struct vm_special_mapping *vdso_spec;
- unsigned long vvar_size = PAGE_SIZE;
+ unsigned long vvar_size = VVAR_NR_PAGES * PAGE_SIZE;
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
@@ -266,19 +365,6 @@ static struct page ** __init vdso_setup_pages(void *start, void *end)
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;
-}
-
static int __init vdso_init(void)
{
#ifdef CONFIG_PPC64
@@ -317,8 +403,6 @@ 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;
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index a4b806b0d618..58e0099f70f4 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -17,7 +17,7 @@ ENTRY(_start)
SECTIONS
{
- PROVIDE(_vdso_datapage = . - PAGE_SIZE);
+ PROVIDE(_vdso_datapage = . - 2 * PAGE_SIZE);
. = SIZEOF_HEADERS;
.hash : { *(.hash) } :text
diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
index 2f3c359cacd3..0288cad428b0 100644
--- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
+++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
@@ -17,7 +17,7 @@ ENTRY(_start)
SECTIONS
{
- PROVIDE(_vdso_datapage = . - PAGE_SIZE);
+ PROVIDE(_vdso_datapage = . - 2 * PAGE_SIZE);
. = SIZEOF_HEADERS;
.hash : { *(.hash) } :text
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v2 3/7] powerpc: convert config files to generic cmdline
From: Daniel Walker @ 2021-03-31 17:14 UTC (permalink / raw)
To: Will Deacon
Cc: Rob Herring, Daniel Gimpelevich, X86 ML, open list:MIPS,
linux-kernel@vger.kernel.org, Paul Mackerras, xe-linux-external,
Andrew Morton, linuxppc-dev
In-Reply-To: <20210331115218.GB7626@willie-the-truck>
On Wed, Mar 31, 2021 at 12:52:19PM +0100, Will Deacon wrote:
> On Tue, Mar 30, 2021 at 10:35:21AM -0700, Daniel Walker wrote:
> > On Mon, Mar 29, 2021 at 11:07:51AM +0100, Will Deacon wrote:
> > > On Thu, Mar 25, 2021 at 12:59:56PM -0700, Daniel Walker wrote:
> > > > On Thu, Mar 25, 2021 at 01:03:55PM +0100, Christophe Leroy wrote:
> > > > >
> > > > > Ok, so you agree we don't need to provide two CMDLINE, one to be appended and one to be prepended.
> > > > >
> > > > > Let's only provide once CMDLINE as of today, and ask the user to select
> > > > > whether he wants it appended or prepended or replacee. Then no need to
> > > > > change all existing config to rename CONFIG_CMDLINE into either of the new
> > > > > ones.
> > > > >
> > > > > That's the main difference between my series and Daniel's series. So I'll
> > > > > finish taking Will's comment into account and we'll send out a v3 soon.
> > > >
> > > > It doesn't solve the needs of Cisco, I've stated many times your changes have
> > > > little value. Please stop submitting them.
> > >
> > > FWIW, they're useful for arm64 and I will gladly review the updated series.
> > >
> > > I don't think asking people to stop submitting patches is ever the right
> > > answer. Please don't do that.
> >
> > Why ? It's me nacking his series, is that not allowed anymore ?
>
> If you're that way inclined then you can "nack" whatever you want, but
> please allow the rest of us to continue reviewing the patches. You don't
> have any basis on which to veto other people's contributions and so
> demanding that somebody stops posting code is neither constructive nor
> meaningful.
I understand , but that's not what's happening. I've dealt with Christophe on
these changes repeatedly, and he's demonstrated he doesn't understand the feature set or
the motivation of the changes. I've tried to work with him in the past, but it
doesn't work unless he's giving me comments on my changes.
His changes don't solve Cisco problems, and likely never will regardless of
feedback. Maybe that could change, but I don't see that happening.
Daniel
^ permalink raw reply
* Re: [PATCH] powerpc/vdso: Separate vvar vma from vdso
From: Dmitry Safonov @ 2021-03-31 18:15 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: <47623d02-eb29-0fcb-0cfd-a9c11c9fab02@csgroup.eu>
On 3/30/21 11:17 AM, Christophe Leroy wrote:
>
>
> Le 26/03/2021 à 20:17, Dmitry Safonov a écrit :
[..]
>> --- 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;
>
> In ARM64 you have removed the above test in commit 871402e05b24cb56
> ("mm: forbid splitting special mappings"). Do we need to keep it here ?
>
>> - current->mm->context.vdso = (void __user *)new_vma->vm_start +
>> PAGE_SIZE;
>> + current->mm->context.vdso = (void __user *)new_vma->vm_start;
>> return 0;
>> }
>
Yes, right you are, this can be dropped.
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH 6/8] drivers: firmware: efi: libstub: enable generic commandline
From: Daniel Walker @ 2021-03-31 18:21 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: ob Herring, linux-efi, Daniel Gimpelevich,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), X86 ML,
open list:MIPS, Linux Kernel Mailing List, Arvind Sankar,
xe-linux-external, Andrew Morton, Will Deacon
In-Reply-To: <CAMj1kXG_rRLU2Hp_GaZyayxx6J+HaWyPHPmE-hEZawuxzZ4JXw@mail.gmail.com>
On Wed, Mar 31, 2021 at 06:10:08PM +0200, Ard Biesheuvel wrote:
> (+ Arvind)
>
> On Tue, 30 Mar 2021 at 19:57, Daniel Walker <danielwa@cisco.com> wrote:
> >
> > This adds code to handle the generic command line changes.
> > The efi code appears that it doesn't benefit as much from this design
> > as it could.
> >
> > For example, if you had a prepend command line with "nokaslr" then
> > you might be helpful to re-enable it in the boot loader or dts,
> > but there appears to be no way to re-enable kaslr or some of the
> > other options.
> >
> > Cc: xe-linux-external@cisco.com
> > Signed-off-by: Daniel Walker <danielwa@cisco.com>
> > ---
> > .../firmware/efi/libstub/efi-stub-helper.c | 35 +++++++++++++++++++
> > drivers/firmware/efi/libstub/efi-stub.c | 7 ++++
> > drivers/firmware/efi/libstub/efistub.h | 1 +
> > drivers/firmware/efi/libstub/x86-stub.c | 13 +++++--
> > 4 files changed, 54 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > index aa8da0a49829..c155837cedc9 100644
> > --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > @@ -13,6 +13,7 @@
> > #include <linux/efi.h>
> > #include <linux/kernel.h>
> > #include <linux/printk.h> /* For CONSOLE_LOGLEVEL_* */
> > +#include <linux/cmdline.h>
> > #include <asm/efi.h>
> > #include <asm/setup.h>
> >
> > @@ -172,6 +173,40 @@ int efi_printk(const char *fmt, ...)
> > return printed;
> > }
> >
> > +/**
> > + * efi_handle_cmdline() - handle adding in building parts of the command line
> > + * @cmdline: kernel command line
> > + *
> > + * Add in the generic parts of the commandline and start the parsing of the
> > + * command line.
> > + *
> > + * Return: status code
> > + */
> > +efi_status_t efi_handle_cmdline(char const *cmdline)
> > +{
> > + efi_status_t status;
> > +
> > + status = efi_parse_options(CMDLINE_PREPEND);
> > + if (status != EFI_SUCCESS) {
> > + efi_err("Failed to parse options\n");
> > + return status;
> > + }
>
> Even though I am not a fan of the 'success handling' pattern,
> duplicating the exact same error handling three times is not great
> either. Could we reuse more of the code here?
How about
efi_status_t status = 0;
status |= efi_parse_options(CMDLINE_PREPEND);
then error checking once ?
> > +
> > + status = efi_parse_options(IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) ? "" : cmdline);
>
> What is the point of calling efi_parse_options() with an empty string?
I could change it to if ((IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) ?
> > --- a/drivers/firmware/efi/libstub/efi-stub.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub.c
> > @@ -172,6 +172,12 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
> > goto fail;
> > }
> >
> > +#ifdef CONFIG_GENERIC_CMDLINE
> > + status = efi_handle_cmdline(cmdline_ptr);
> > + if (status != EFI_SUCCESS) {
> > + goto fail_free_cmdline;
> > + }
> > +#else
> > if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
> > IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
>
> Does this mean CONFIG_GENERIC_CMDLINE does not replace CMDLINE_EXTEND
> / CMDLINE_FORCE etc, but introduces yet another variant on top of
> those?
>
> That does not seem like an improvement to me. I think it is great that
> you are cleaning this up, but only if it means we can get rid of the
> old implementation.
It does replace extend and force. I was under the impression this code was
shared between arm64 and arm32. If that's not the case I can delete the extend
and force section. I haven't submitted a conversion for arm32 yet.
Daniel
^ permalink raw reply
* Re: [PATCH] powerpc/vdso: Separate vvar vma from vdso
From: Dmitry Safonov @ 2021-03-31 18:53 UTC (permalink / raw)
To: Michael Ellerman, Christophe Leroy, linux-kernel
Cc: Dmitry Safonov, stable, Andrei Vagin, Paul Mackerras,
Andy Lutomirski, Laurent Dufour, linuxppc-dev
In-Reply-To: <8735wby77v.fsf@mpe.ellerman.id.au>
On 3/31/21 10:59 AM, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
[..]
>>
>>> @@ -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
>>
>>
>> IIUC, VM_PFNMAP is for when we have a vvar_fault handler.
>> Allthough we will soon have one for handle TIME_NS, at the moment
>> powerpc doesn't have that handler.
>> Isn't it dangerous to set VM_PFNMAP then ?
I believe, it's fine, special_mapping_fault() does:
: if (sm->fault)
: return sm->fault(sm, vmf->vma, vmf);
> Some of the other flags seem odd too.
> eg. VM_IO ? VM_DONTDUMP ?
Yeah, so:
VM_PFNMAP | VM_IO is a protection from remote access on pages. So one
can't access such page with ptrace(), /proc/$pid/mem or
process_vm_write(). Otherwise, it would create COW mapping and the
tracee will stop working with stale vvar.
VM_DONTDUMP restricts the area from coredumping and gdb will also avoid
accessing those[1][2].
I agree that VM_PFNMAP was probably excessive in this patch alone and
rather synchronized code with other architectures, but it makes more
sense now in the new patches set by Christophe:
https://lore.kernel.org/linux-arch/cover.1617209141.git.christophe.leroy@csgroup.eu/
[1] https://lore.kernel.org/lkml/550731AF.6080904@redhat.com/T/
[2] https://sourceware.org/legacy-ml/gdb-patches/2015-03/msg00383.html
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH] docs: powerpc: Fix misspellings and grammar errors
From: Jonathan Corbet @ 2021-03-31 19:51 UTC (permalink / raw)
To: He Ying, mpe, benh, paulus, ruscur, oohall, heying24
Cc: linuxppc-dev, linux-kernel, linux-doc
In-Reply-To: <20210326100853.173586-1-heying24@huawei.com>
He Ying <heying24@huawei.com> writes:
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: He Ying <heying24@huawei.com>
> ---
> Documentation/powerpc/booting.rst | 2 +-
> Documentation/powerpc/dawr-power9.rst | 2 +-
> Documentation/powerpc/eeh-pci-error-recovery.rst | 2 +-
> Documentation/powerpc/elfnote.rst | 2 +-
> Documentation/powerpc/firmware-assisted-dump.rst | 2 +-
> Documentation/powerpc/kaslr-booke32.rst | 2 +-
> Documentation/powerpc/mpc52xx.rst | 2 +-
> Documentation/powerpc/papr_hcalls.rst | 4 ++--
> Documentation/powerpc/transactional_memory.rst | 4 ++--
> 9 files changed, 11 insertions(+), 11 deletions(-)
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH v2] powerpc/traps: Enhance readability for trap types
From: Segher Boessenkool @ 2021-03-31 21:25 UTC (permalink / raw)
To: Michael Ellerman
Cc: pmladek, peterz, peterx, linux-kernel, paulus, kan.liang,
leobras.c, mikey, maddy, aneesh.kumar, haren, aik, kjain,
msuchanek, ravi.bangoria, john.ogness, alistair, npiggin, kvm-ppc,
jniethe5, atrajeev, Xiongwei Song, Xiongwei Song, oleg, akpm,
linuxppc-dev, rppt
In-Reply-To: <875z17y79i.fsf@mpe.ellerman.id.au>
On Wed, Mar 31, 2021 at 08:58:17PM +1100, Michael Ellerman wrote:
> So perhaps:
>
> EXC_SYSTEM_RESET
> EXC_MACHINE_CHECK
> EXC_DATA_STORAGE
> EXC_DATA_SEGMENT
> EXC_INST_STORAGE
> EXC_INST_SEGMENT
> EXC_EXTERNAL_INTERRUPT
> EXC_ALIGNMENT
> EXC_PROGRAM_CHECK
> EXC_FP_UNAVAILABLE
> EXC_DECREMENTER
> EXC_HV_DECREMENTER
> EXC_SYSTEM_CALL
> EXC_HV_DATA_STORAGE
> EXC_PERF_MONITOR
These are interrupt (vectors), not exceptions. It doesn't matter all
that much, but confusing things more isn't useful either! There can be
multiple exceptions that all can trigger the same interrupt.
Segher
^ permalink raw reply
* Re: [PATCH v2] powerpc/traps: Enhance readability for trap types
From: Michael Ellerman @ 2021-04-01 2:39 UTC (permalink / raw)
To: Segher Boessenkool
Cc: pmladek, peterz, peterx, linux-kernel, paulus, kan.liang,
leobras.c, mikey, maddy, aneesh.kumar, haren, aik, kjain,
msuchanek, ravi.bangoria, john.ogness, alistair, npiggin, kvm-ppc,
jniethe5, atrajeev, Xiongwei Song, Xiongwei Song, oleg, akpm,
linuxppc-dev, rppt
In-Reply-To: <20210331212550.GD13863@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Wed, Mar 31, 2021 at 08:58:17PM +1100, Michael Ellerman wrote:
>> So perhaps:
>>
>> EXC_SYSTEM_RESET
>> EXC_MACHINE_CHECK
>> EXC_DATA_STORAGE
>> EXC_DATA_SEGMENT
>> EXC_INST_STORAGE
>> EXC_INST_SEGMENT
>> EXC_EXTERNAL_INTERRUPT
>> EXC_ALIGNMENT
>> EXC_PROGRAM_CHECK
>> EXC_FP_UNAVAILABLE
>> EXC_DECREMENTER
>> EXC_HV_DECREMENTER
>> EXC_SYSTEM_CALL
>> EXC_HV_DATA_STORAGE
>> EXC_PERF_MONITOR
>
> These are interrupt (vectors), not exceptions. It doesn't matter all
> that much, but confusing things more isn't useful either! There can be
> multiple exceptions that all can trigger the same interrupt.
Yeah I know, but I think that ship has already sailed as far as the
naming we have in the kernel.
We have over 250 uses of "exc", and several files called "exception"
something.
Using "interrupt" can also be confusing because Linux uses that to mean
"external interrupt".
But I dunno, maybe INT or VEC is clearer? .. or TRAP :)
cheers
^ permalink raw reply
* Re: [PATCH v3 1/9] powerpc/xive: Use cpu_to_node() instead of "ibm,chip-id" property
From: David Gibson @ 2021-04-01 2:49 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev, Greg Kurz
In-Reply-To: <20210331144514.892250-2-clg@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]
On Wed, Mar 31, 2021 at 04:45:06PM +0200, Cédric Le Goater wrote:
> The 'chip_id' field of the XIVE CPU structure is used to choose a
> target for a source located on the same chip when possible. The XIVE
> driver queries the chip id value from the "ibm,chip-id" DT property
> but this property is not available on all platforms. It was first
> introduced on the PowerNV platform and later, under QEMU for pseries.
> However, the property does not exist under PowerVM since it is not
> specified in PAPR.
>
> cpu_to_node() is a better alternative. On the PowerNV platform, the
> node id is computed from the "ibm,associativity" property of the CPU.
> Its value is built in the OPAL firmware from the physical chip id and
> is equivalent to "ibm,chip-id".
Hrm... I mean, for powernv this is certainly correct, but seems to be
relying on pretty specific specifics of the OPAL / chip behaviour,
namely that the NUMA id == chip ID.
> On pSeries, the hcall
> H_HOME_NODE_ASSOCIATIVITY returns the node id.
AFAICT, the chip_id field is never actually used in the PAPR version
of XIVE. The only access to the field outside native.c is in
xive_pick_irq_target(), and it only looks at chip_id if src_chip is
valid. But src_chip is initialized to XIVE_INVALID_CHIP_ID in papr.c
So it would make more sense to me to also initialize chip_id to
XIVE_INVALID_CHIP_ID for PAPR to make it clearer that it's not
relevant.
> Also to be noted that under QEMU/KVM "ibm,chip-id" is badly calculated
> with unusual SMT configuration. This leads to a bogus chip id value
> being returned by of_get_ibm_chip_id().
I *still* don't clearly understand what you think is bogus about the
chip id value that qemu generates. It's clearly not a problem for
XIVE, since PAPR XIVE never uses it.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2] powerpc/traps: Enhance readability for trap types
From: Xiongwei Song @ 2021-04-01 2:44 UTC (permalink / raw)
To: Michael Ellerman
Cc: pmladek, peterz, peterx, linux-kernel, paulus, kan.liang,
leobras.c, mikey, maddy, aneesh.kumar, haren, aik, kjain,
msuchanek, ravi.bangoria, john.ogness, alistair, npiggin, kvm-ppc,
jniethe5, atrajeev, Xiongwei Song, oleg, akpm, linuxppc-dev, rppt
In-Reply-To: <875z17y79i.fsf@mpe.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 3318 bytes --]
Michael Ellerman <mpe@ellerman.id.au> 于2021年3月31日周三 下午5:58写道：
> Xiongwei Song <sxwjean@me.com> writes:
> > From: Xiongwei Song <sxwjean@gmail.com>
> >
> > Create a new header named traps.h, define macros to list ppc exception
> > types in traps.h, replace the reference of the real trap values with
> > these macros.
>
> Personally I find the hex values easier to recognise, but I realise
> that's probably not true of other people :)
>
> I'm one of the "other people".
> ...
> > diff --git a/arch/powerpc/include/asm/traps.h
> b/arch/powerpc/include/asm/traps.h
> > new file mode 100644
> > index 000000000000..a31b6122de23
> > --- /dev/null
> > +++ b/arch/powerpc/include/asm/traps.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _ASM_PPC_TRAPS_H
> > +#define _ASM_PPC_TRAPS_H
> > +
> > +#define TRAP_RESET 0x100 /* System reset */
> > +#define TRAP_MCE 0x200 /* Machine check */
> > +#define TRAP_DSI 0x300 /* Data storage */
> > +#define TRAP_DSEGI 0x380 /* Data segment */
> > +#define TRAP_ISI 0x400 /* Instruction storage */
> > +#define TRAP_ISEGI 0x480 /* Instruction segment */
> > +#define TRAP_ALIGN 0x600 /* Alignment */
> > +#define TRAP_PROG 0x700 /* Program */
> > +#define TRAP_DEC 0x900 /* Decrementer */
> > +#define TRAP_SYSCALL 0xc00 /* System call */
> > +#define TRAP_TRACEI 0xd00 /* Trace */
> > +#define TRAP_FPA 0xe00 /* Floating-point Assist */
> > +#define TRAP_PMI 0xf00 /* Performance monitor */
>
> I know the macro is called TRAP and the field in pt_regs is called trap,
> but the terminology in the architecture is "exception", and we already
> have many uses of that. In particular we have a lot of uses of "exc" as
> an abbreviation for "exception". So I think I'd rather we use that than
> "TRAP".
>
Ok.
>
> I think we should probably use the names from the ISA, unless they are
> really over long.
>
> Which are:
>
> 0x100 System Reset
> 0x200 Machine Check
> 0x300 Data Storage
> 0x380 Data Segment
> 0x400 Instruction Storage
> 0x480 Instruction Segment
> 0x500 External
> 0x600 Alignment
> 0x700 Program
> 0x800 Floating-Point Unavailable
> 0x900 Decrementer
> 0x980 Hypervisor Decrementer
> 0xA00 Directed Privileged Doorbell
> 0xC00 System Call
> 0xD00 Trace
> 0xE00 Hypervisor Data Storage
> 0xE20 Hypervisor Instruction Storage
> 0xE40 Hypervisor Emulation Assistance
> 0xE60 Hypervisor Maintenance
> 0xE80 Directed Hypervisor Doorbell
> 0xEA0 Hypervisor Virtualization
> 0xF00 Performance Monitor
> 0xF20 Vector Unavailable
> 0xF40 VSX Unavailable
> 0xF60 Facility Unavailable
> 0xF80 Hypervisor Facility Unavailable
> 0xFA0 Directed Ultravisor Doorbell
>
>
> So perhaps:
>
> EXC_SYSTEM_RESET
> EXC_MACHINE_CHECK
> EXC_DATA_STORAGE
> EXC_DATA_SEGMENT
> EXC_INST_STORAGE
> EXC_INST_SEGMENT
> EXC_EXTERNAL_INTERRUPT
> EXC_ALIGNMENT
> EXC_PROGRAM_CHECK
> EXC_FP_UNAVAILABLE
> EXC_DECREMENTER
> EXC_HV_DECREMENTER
> EXC_SYSTEM_CALL
> EXC_HV_DATA_STORAGE
> EXC_PERF_MONITOR
>
> Thanks for the suggestions. I'm ok with the prefix. Let me change.
[-- Attachment #2: Type: text/html, Size: 4488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] powerpc/traps: Enhance readability for trap types
From: Xiongwei Song @ 2021-04-01 2:55 UTC (permalink / raw)
To: Segher Boessenkool
Cc: pmladek, peterz, peterx, linux-kernel, paulus, kan.liang,
leobras.c, mikey, maddy, aneesh.kumar, haren, aik, kjain,
msuchanek, ravi.bangoria, john.ogness, alistair, npiggin, kvm-ppc,
jniethe5, atrajeev, Xiongwei Song, oleg, akpm, linuxppc-dev, rppt
In-Reply-To: <20210331212550.GD13863@gate.crashing.org>
[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]
Segher Boessenkool <segher@kernel.crashing.org> 于2021年4月1日周四 上午6:15写道：
> On Wed, Mar 31, 2021 at 08:58:17PM +1100, Michael Ellerman wrote:
> > So perhaps:
> >
> > EXC_SYSTEM_RESET
> > EXC_MACHINE_CHECK
> > EXC_DATA_STORAGE
> > EXC_DATA_SEGMENT
> > EXC_INST_STORAGE
> > EXC_INST_SEGMENT
> > EXC_EXTERNAL_INTERRUPT
> > EXC_ALIGNMENT
> > EXC_PROGRAM_CHECK
> > EXC_FP_UNAVAILABLE
> > EXC_DECREMENTER
> > EXC_HV_DECREMENTER
> > EXC_SYSTEM_CALL
> > EXC_HV_DATA_STORAGE
> > EXC_PERF_MONITOR
>
> These are interrupt (vectors), not exceptions. It doesn't matter all
> that much, but confusing things more isn't useful either! There can be
> multiple exceptions that all can trigger the same interrupt.
>
> When looking at the reference manual of e500 and e600 from NXP
official, they call them as interrupts.While looking at the "The
Programming Environments"
that is also from NXP, they call them exceptions. Looks like there is
no explicit distinction between interrupts and exceptions.
Here are the links for the reference manual of e600 and e500:
https://www.nxp.com.cn/docs/en/reference-manual/E600CORERM.pdf
https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf
Here is the "The Programming Environments" link:
https://www.nxp.com.cn/docs/en/user-guide/MPCFPE_AD_R1.pdf
As far as I know, the values of interrupts or exceptions above are defined
explicitly in reference manual or the programming environments. Could
you please provide more details about multiple exceptions with the same
interrupts?
Xiongwei
[-- Attachment #2: Type: text/html, Size: 2782 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: Load modules closer to kernel text
From: Michael Ellerman @ 2021-04-01 4:33 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: Jordan Niethe, linuxppc-dev, linux-kernel
In-Reply-To: <e50f8127-3a87-92eb-d88a-72aa1557ffe7@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 31/03/2021 à 15:39, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> On the 8xx, TASK_SIZE is 0x80000000. The space between TASK_SIZE and
>>> PAGE_OFFSET is not used.
>>>
>>> Use it to load modules in order to minimise the distance between
>>> kernel text and modules and avoid trampolines in modules to access
>>> kernel functions or other module functions.
>>>
>>> Define a 16Mbytes area for modules, that's more than enough.
>>
>> 16MB seems kind of small.
>>
>> At least on 64-bit we could potentially have hundreds of MBs of modules.
>>
>
> Well, with a 16 MB kernel and 16 MB modules, my board is full :)
Heh.
> Even on the more recent board that has 128 MB, I don't expect more than a few MBs of modules in
> addition to the kernel which is approx 8M.
>
> But ok, I'll do something more generic, though it will conflict with Jordan's series.
Don't feel you have to. You're the expert on 8xx, not me.
cheers
^ permalink raw reply
* Re: [PATCH v10 01/10] powerpc/mm: Implement set_memory() routines
From: Aneesh Kumar K.V @ 2021-04-01 4:37 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: ajd, Jordan Niethe, npiggin, cmr, naveen.n.rao, dja
In-Reply-To: <20210330045132.722243-2-jniethe5@gmail.com>
Jordan Niethe <jniethe5@gmail.com> writes:
> From: Russell Currey <ruscur@russell.cc>
>
> The set_memory_{ro/rw/nx/x}() functions are required for STRICT_MODULE_RWX,
> and are generally useful primitives to have. This implementation is
> designed to be completely generic across powerpc's many MMUs.
>
> It's possible that this could be optimised to be faster for specific
> MMUs, but the focus is on having a generic and safe implementation for
> now.
>
> This implementation does not handle cases where the caller is attempting
> to change the mapping of the page it is executing from, or if another
> CPU is concurrently using the page being altered. These cases likely
> shouldn't happen, but a more complex implementation with MMU-specific code
> could safely handle them, so that is left as a TODO for now.
>
> On hash the linear mapping is not kept in the linux pagetable, so this
> will not change the protection if used on that range. Currently these
> functions are not used on the linear map so just WARN for now.
>
> These functions do nothing if STRICT_KERNEL_RWX is not enabled.
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> [jpn: -rebase on next plus "powerpc/mm/64s: Allow STRICT_KERNEL_RWX again"
> - WARN on hash linear map]
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v10: WARN if trying to change the hash linear map
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/set_memory.h | 32 ++++++++++
> arch/powerpc/mm/Makefile | 2 +-
> arch/powerpc/mm/pageattr.c | 88 +++++++++++++++++++++++++++
> 4 files changed, 122 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/include/asm/set_memory.h
> create mode 100644 arch/powerpc/mm/pageattr.c
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index fc7f5c5933e6..4498a27ac9db 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -135,6 +135,7 @@ config PPC
> 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_SET_MEMORY
> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UACCESS_FLUSHCACHE
> diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
> new file mode 100644
> index 000000000000..64011ea444b4
> --- /dev/null
> +++ b/arch/powerpc/include/asm/set_memory.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_POWERPC_SET_MEMORY_H
> +#define _ASM_POWERPC_SET_MEMORY_H
> +
> +#define SET_MEMORY_RO 0
> +#define SET_MEMORY_RW 1
> +#define SET_MEMORY_NX 2
> +#define SET_MEMORY_X 3
> +
> +int change_memory_attr(unsigned long addr, int numpages, long action);
> +
> +static inline int set_memory_ro(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_RO);
> +}
> +
> +static inline int set_memory_rw(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_RW);
> +}
> +
> +static inline int set_memory_nx(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_NX);
> +}
> +
> +static inline int set_memory_x(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_X);
> +}
> +
> +#endif
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index 3b4e9e4e25ea..d8a08abde1ae 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -5,7 +5,7 @@
>
> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>
> -obj-y := fault.o mem.o pgtable.o mmap.o maccess.o \
> +obj-y := fault.o mem.o pgtable.o mmap.o maccess.o pageattr.o \
> init_$(BITS).o pgtable_$(BITS).o \
> pgtable-frag.o ioremap.o ioremap_$(BITS).o \
> init-common.o mmu_context.o drmem.o
> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> new file mode 100644
> index 000000000000..9efcb01088da
> --- /dev/null
> +++ b/arch/powerpc/mm/pageattr.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * MMU-generic set_memory implementation for powerpc
> + *
> + * Copyright 2019, IBM Corporation.
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/set_memory.h>
> +
> +#include <asm/mmu.h>
> +#include <asm/page.h>
> +#include <asm/pgtable.h>
> +
> +
> +/*
> + * Updates the attributes of a page in three steps:
> + *
> + * 1. invalidate the page table entry
> + * 2. flush the TLB
> + * 3. install the new entry with the updated attributes
> + *
> + * This is unsafe if the caller is attempting to change the mapping of the
> + * page it is executing from, or if another CPU is concurrently using the
> + * page being altered.
> + *
> + * TODO make the implementation resistant to this.
> + *
> + * NOTE: can be dangerous to call without STRICT_KERNEL_RWX
> + */
> +static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
> +{
> + long action = (long)data;
> + pte_t pte;
> +
> + spin_lock(&init_mm.page_table_lock);
> +
> + /* invalidate the PTE so it's safe to modify */
> + pte = ptep_get_and_clear(&init_mm, addr, ptep);
> + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> +
> + /* modify the PTE bits as desired, then apply */
> + switch (action) {
> + case SET_MEMORY_RO:
> + pte = pte_wrprotect(pte);
> + break;
> + case SET_MEMORY_RW:
> + pte = pte_mkwrite(pte);
> + break;
> + case SET_MEMORY_NX:
> + pte = pte_exprotect(pte);
> + break;
> + case SET_MEMORY_X:
> + pte = pte_mkexec(pte);
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + break;
> + }
> +
> + set_pte_at(&init_mm, addr, ptep, pte);
> + spin_unlock(&init_mm.page_table_lock);
> +
> + return 0;
> +}
> +
> +int change_memory_attr(unsigned long addr, int numpages, long action)
> +{
> + unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> + unsigned long sz = numpages * PAGE_SIZE;
> +
> + if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> + return 0;
What restrictions imposed by that config are we dependent on here?
> +
> + if (numpages <= 0)
> + return 0;
> +
> +#ifdef CONFIG_PPC_BOOK3S_64
> + if (WARN_ON_ONCE(!radix_enabled() &&
> + get_region_id(addr) == LINEAR_MAP_REGION_ID)) {
> + return -1;
> + }
> +#endif
What about VMEMMAP_REGIOND_ID
> +
> + return apply_to_existing_page_range(&init_mm, start, sz,
> + change_page_attr, (void *)action);
That handles on 64K mapping. What about linear map? Also there is a
patchset implementing hugepage for vmalloc mapping.
> +}
> --
> 2.25.1
^ 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