[PATCH v2 0/6] arm64: add the time namespace support

[PATCH v2 0/6] arm64: add the time namespace support

[Andrei Vagin]

Allocate the time namespace page among VVAR pages and add the logic
to handle faults on VVAR properly.

If a task belongs to a time namespace then the VVAR page which contains
the system wide VDSO data is replaced with a namespace specific page
which has the same layout as the VVAR page. That page has vdso_data->seq
set to 1 to enforce the slow path and vdso_data->clock_mode set to
VCLOCK_TIMENS to enforce the time namespace handling path.

The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
update of the VDSO data is in progress, is not really affecting regular
tasks which are not part of a time namespace as the task is spin waiting
for the update to finish and vdso_data->seq to become even again.

If a time namespace task hits that code path, it invokes the corresponding
time getter function which retrieves the real VVAR page, reads host time
and then adds the offset for the requested clock which is stored in the
special VVAR page.

v2: Code cleanups suggested by Vincenzo.

Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Dmitry Safonov <dima@arista.com>

Andrei Vagin (6):
  arm64/vdso: use the fault callback to map vvar pages
  arm64/vdso: Zap vvar pages when switching to a time namespace
  arm64/vdso: Add time napespace page
  arm64/vdso: Handle faults on timens page
  arm64/vdso: Restrict splitting VVAR VMA
  arm64: enable time namespace support

 arch/arm64/Kconfig                            |   1 +
 .../include/asm/vdso/compat_gettimeofday.h    |  11 ++
 arch/arm64/include/asm/vdso/gettimeofday.h    |   8 ++
 arch/arm64/kernel/vdso.c                      | 134 ++++++++++++++++--
 arch/arm64/kernel/vdso/vdso.lds.S             |   3 +-
 arch/arm64/kernel/vdso32/vdso.lds.S           |   3 +-
 include/vdso/datapage.h                       |   1 +
 7 files changed, 147 insertions(+), 14 deletions(-)

-- 
2.24.1

[PATCH v2 1/6] arm64/vdso: use the fault callback to map vvar pages

[Andrei Vagin]

This is required to support time namespaces where a time namespace data
page is different for each namespace.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
 arch/arm64/kernel/vdso.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 354b11e27c07..290c36d74e03 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -114,28 +114,32 @@ static int __vdso_init(enum arch_vdso_type arch_index)
 			PAGE_SHIFT;
 
 	/* Allocate the vDSO pagelist, plus a page for the data. */
-	vdso_pagelist = kcalloc(vdso_lookup[arch_index].vdso_pages + 1,
+	vdso_pagelist = kcalloc(vdso_lookup[arch_index].vdso_pages,
 				sizeof(struct page *),
 				GFP_KERNEL);
 	if (vdso_pagelist == NULL)
 		return -ENOMEM;
 
-	/* Grab the vDSO data page. */
-	vdso_pagelist[0] = phys_to_page(__pa_symbol(vdso_data));
-
-
 	/* Grab the vDSO code pages. */
 	pfn = sym_to_pfn(vdso_lookup[arch_index].vdso_code_start);
 
 	for (i = 0; i < vdso_lookup[arch_index].vdso_pages; i++)
-		vdso_pagelist[i + 1] = pfn_to_page(pfn + i);
+		vdso_pagelist[i] = pfn_to_page(pfn + i);
 
-	vdso_lookup[arch_index].dm->pages = &vdso_pagelist[0];
-	vdso_lookup[arch_index].cm->pages = &vdso_pagelist[1];
+	vdso_lookup[arch_index].cm->pages = vdso_pagelist;
 
 	return 0;
 }
 
+static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
+			     struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+	if (vmf->pgoff == 0)
+		return vmf_insert_pfn(vma, vmf->address,
+				sym_to_pfn(vdso_data));
+	return VM_FAULT_SIGBUS;
+}
+
 static int __setup_additional_pages(enum arch_vdso_type arch_index,
 				    struct mm_struct *mm,
 				    struct linux_binprm *bprm,
@@ -155,7 +159,7 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
 	}
 
 	ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
-				       VM_READ|VM_MAYREAD,
+				       VM_READ|VM_MAYREAD|VM_PFNMAP,
 				       vdso_lookup[arch_index].dm);
 	if (IS_ERR(ret))
 		goto up_fail;
@@ -215,6 +219,7 @@ static struct vm_special_mapping aarch32_vdso_spec[C_PAGES] = {
 #ifdef CONFIG_COMPAT_VDSO
 	{
 		.name = "[vvar]",
+		.fault = vvar_fault,
 	},
 	{
 		.name = "[vdso]",
@@ -396,6 +401,7 @@ static int vdso_mremap(const struct vm_special_mapping *sm,
 static struct vm_special_mapping vdso_spec[A_PAGES] __ro_after_init = {
 	{
 		.name	= "[vvar]",
+		.fault = vvar_fault,
 	},
 	{
 		.name	= "[vdso]",
-- 
2.24.1

[PATCH v2 2/6] arm64/vdso: Zap vvar pages when switching to a time namespace

[Andrei Vagin]

The VVAR page layout depends on whether a task belongs to the root or
non-root time namespace. Whenever a task changes its namespace, the VVAR
page tables are cleared and then they will be re-faulted with a
corresponding layout.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
 arch/arm64/kernel/vdso.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 290c36d74e03..6ac9cdeac5be 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -131,6 +131,38 @@ static int __vdso_init(enum arch_vdso_type arch_index)
 	return 0;
 }
 
+#ifdef CONFIG_TIME_NS
+/*
+ * The vvar page layout depends on whether a task belongs to the root or
+ * non-root time namespace. Whenever a task changes its namespace, the VVAR
+ * page tables are cleared and then they will re-faulted with a
+ * corresponding layout.
+ * See also the comment near timens_setup_vdso_data() for details.
+ */
+int vdso_join_timens(struct task_struct *task, struct time_namespace *ns)
+{
+	struct mm_struct *mm = task->mm;
+	struct vm_area_struct *vma;
+
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
+
+	for (vma = mm->mmap; vma; vma = vma->vm_next) {
+		unsigned long size = vma->vm_end - vma->vm_start;
+
+		if (vma_is_special_mapping(vma, vdso_lookup[ARM64_VDSO].dm))
+			zap_page_range(vma, vma->vm_start, size);
+#ifdef CONFIG_COMPAT_VDSO
+		if (vma_is_special_mapping(vma, vdso_lookup[ARM64_VDSO32].dm))
+			zap_page_range(vma, vma->vm_start, size);
+#endif
+	}
+
+	up_write(&mm->mmap_sem);
+	return 0;
+}
+#endif
+
 static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
 			     struct vm_area_struct *vma, struct vm_fault *vmf)
 {
-- 
2.24.1

[PATCH v2 3/6] arm64/vdso: Add time napespace page

[Andrei Vagin]

Allocate the time namespace page among VVAR pages.  Provide
__arch_get_timens_vdso_data() helper for VDSO code to get the
code-relative position of VVARs on that special page.

If a task belongs to a time namespace then the VVAR page which contains
the system wide VDSO data is replaced with a namespace specific page
which has the same layout as the VVAR page. That page has vdso_data->seq
set to 1 to enforce the slow path and vdso_data->clock_mode set to
VCLOCK_TIMENS to enforce the time namespace handling path.

The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
update of the VDSO data is in progress, is not really affecting regular
tasks which are not part of a time namespace as the task is spin waiting
for the update to finish and vdso_data->seq to become even again.

If a time namespace task hits that code path, it invokes the corresponding
time getter function which retrieves the real VVAR page, reads host time
and then adds the offset for the requested clock which is stored in the
special VVAR page.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
 arch/arm64/include/asm/vdso.h                 |  6 ++++++
 .../include/asm/vdso/compat_gettimeofday.h    | 11 ++++++++++
 arch/arm64/include/asm/vdso/gettimeofday.h    |  8 ++++++++
 arch/arm64/kernel/vdso.c                      | 20 ++++++++++++++++---
 arch/arm64/kernel/vdso/vdso.lds.S             |  5 ++++-
 arch/arm64/kernel/vdso32/vdso.lds.S           |  5 ++++-
 include/vdso/datapage.h                       |  1 +
 7 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
index 07468428fd29..351c145d3808 100644
--- a/arch/arm64/include/asm/vdso.h
+++ b/arch/arm64/include/asm/vdso.h
@@ -12,6 +12,12 @@
  */
 #define VDSO_LBASE	0x0
 
+#ifdef CONFIG_TIME_NS
+#define __VVAR_PAGES    2
+#else
+#define __VVAR_PAGES    1
+#endif
+
 #ifndef __ASSEMBLY__
 
 #include <generated/vdso-offsets.h>
diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
index 81b0c394f1d8..042814339a63 100644
--- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
@@ -160,6 +160,17 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
 	return ret;
 }
 
+#ifdef CONFIG_TIME_NS
+static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
+{
+	const struct vdso_data *ret;
+
+	asm volatile("mov %0, %1" : "=r"(ret) : "r"(_timens_data));
+
+	return ret;
+}
+#endif
+
 #endif /* !__ASSEMBLY__ */
 
 #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
index 5a534432aa5d..553bdc19a91f 100644
--- a/arch/arm64/include/asm/vdso/gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/gettimeofday.h
@@ -97,6 +97,14 @@ const struct vdso_data *__arch_get_vdso_data(void)
 	return _vdso_data;
 }
 
+#ifdef CONFIG_TIME_NS
+static __always_inline
+const struct vdso_data *__arch_get_timens_vdso_data(void)
+{
+	return _timens_data;
+}
+#endif
+
 #endif /* !__ASSEMBLY__ */
 
 #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 6ac9cdeac5be..b3e7ce24e59b 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -46,6 +46,14 @@ enum arch_vdso_type {
 #define VDSO_TYPES		(ARM64_VDSO + 1)
 #endif /* CONFIG_COMPAT_VDSO */
 
+enum vvar_pages {
+	VVAR_DATA_PAGE_OFFSET = 0,
+#ifdef CONFIG_TIME_NS
+	VVAR_TIMENS_PAGE_OFFSET = 1,
+#endif /* CONFIG_TIME_NS */
+	VVAR_NR_PAGES = __VVAR_PAGES,
+};
+
 struct __vdso_abi {
 	const char *name;
 	const char *vdso_code_start;
@@ -81,6 +89,12 @@ static union {
 } vdso_data_store __page_aligned_data;
 struct vdso_data *vdso_data = vdso_data_store.data;
 
+
+struct vdso_data *arch_get_vdso_data(void *vvar_page)
+{
+	return (struct vdso_data *)(vvar_page);
+}
+
 static int __vdso_remap(enum arch_vdso_type arch_index,
 			const struct vm_special_mapping *sm,
 			struct vm_area_struct *new_vma)
@@ -182,7 +196,7 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
 
 	vdso_text_len = vdso_lookup[arch_index].vdso_pages << PAGE_SHIFT;
 	/* Be sure to map the data page */
-	vdso_mapping_len = vdso_text_len + PAGE_SIZE;
+	vdso_mapping_len = vdso_text_len + VVAR_NR_PAGES * PAGE_SIZE;
 
 	vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
 	if (IS_ERR_VALUE(vdso_base)) {
@@ -190,13 +204,13 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
 		goto up_fail;
 	}
 
-	ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
+	ret = _install_special_mapping(mm, vdso_base, VVAR_NR_PAGES * PAGE_SIZE,
 				       VM_READ|VM_MAYREAD|VM_PFNMAP,
 				       vdso_lookup[arch_index].dm);
 	if (IS_ERR(ret))
 		goto up_fail;
 
-	vdso_base += PAGE_SIZE;
+	vdso_base += VVAR_NR_PAGES * PAGE_SIZE;
 	mm->context.vdso = (void *)vdso_base;
 	ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
 				       VM_READ|VM_EXEC|
diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
index 7ad2d3a0cd48..d808ad31e01f 100644
--- a/arch/arm64/kernel/vdso/vdso.lds.S
+++ b/arch/arm64/kernel/vdso/vdso.lds.S
@@ -17,7 +17,10 @@ OUTPUT_ARCH(aarch64)
 
 SECTIONS
 {
-	PROVIDE(_vdso_data = . - PAGE_SIZE);
+	PROVIDE(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
+#ifdef CONFIG_TIME_NS
+	PROVIDE(_timens_data = _vdso_data + PAGE_SIZE);
+#endif
 	. = VDSO_LBASE + SIZEOF_HEADERS;
 
 	.hash		: { *(.hash) }			:text
diff --git a/arch/arm64/kernel/vdso32/vdso.lds.S b/arch/arm64/kernel/vdso32/vdso.lds.S
index a3944927eaeb..06cc60a9630f 100644
--- a/arch/arm64/kernel/vdso32/vdso.lds.S
+++ b/arch/arm64/kernel/vdso32/vdso.lds.S
@@ -17,7 +17,10 @@ OUTPUT_ARCH(arm)
 
 SECTIONS
 {
-	PROVIDE_HIDDEN(_vdso_data = . - PAGE_SIZE);
+	PROVIDE_HIDDEN(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
+#ifdef CONFIG_TIME_NS
+	PROVIDE_HIDDEN(_timens_data = _vdso_data + PAGE_SIZE);
+#endif
 	. = VDSO_LBASE + SIZEOF_HEADERS;
 
 	.hash		: { *(.hash) }			:text
diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
index 30c4cb0428d1..3494b5536b63 100644
--- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h
@@ -98,6 +98,7 @@ struct vdso_data {
  * relocation, and this is what we need.
  */
 extern struct vdso_data _vdso_data[CS_BASES] __attribute__((visibility("hidden")));
+extern struct vdso_data _timens_data[CS_BASES] __attribute__((visibility("hidden")));
 
 #endif /* !__ASSEMBLY__ */
 
-- 
2.24.1

[PATCH v2 4/6] arm64/vdso: Handle faults on timens page

[Andrei Vagin]

If a task belongs to a time namespace then the VVAR page which contains
the system wide VDSO data is replaced with a namespace specific page
which has the same layout as the VVAR page.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
 arch/arm64/kernel/vdso.c | 57 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index b3e7ce24e59b..fb32c6f76078 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/signal.h>
 #include <linux/slab.h>
+#include <linux/time_namespace.h>
 #include <linux/timekeeper_internal.h>
 #include <linux/vmalloc.h>
 #include <vdso/datapage.h>
@@ -175,15 +176,63 @@ int vdso_join_timens(struct task_struct *task, struct time_namespace *ns)
 	up_write(&mm->mmap_sem);
 	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 inline 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)
 {
-	if (vmf->pgoff == 0)
-		return vmf_insert_pfn(vma, vmf->address,
-				sym_to_pfn(vdso_data));
-	return VM_FAULT_SIGBUS;
+	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 = sym_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 = sym_to_pfn(vdso_data);
+		break;
+#endif /* CONFIG_TIME_NS */
+	default:
+		return VM_FAULT_SIGBUS;
+	}
+
+	return vmf_insert_pfn(vma, vmf->address, pfn);
 }
 
 static int __setup_additional_pages(enum arch_vdso_type arch_index,
-- 
2.24.1

[PATCH v2 5/6] arm64/vdso: Restrict splitting VVAR VMA

[Andrei Vagin]

Forbid splitting VVAR VMA resulting in a stricter ABI and reducing the
amount of corner-cases to consider while working further on VDSO time
namespace support.

As the offset from timens to VVAR page is computed compile-time, the pages
in VVAR should stay together and not being partically mremap()'ed.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
 arch/arm64/kernel/vdso.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index fb32c6f76078..c003f7ee383a 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -235,6 +235,17 @@ static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
 	return vmf_insert_pfn(vma, vmf->address, pfn);
 }
 
+static int vvar_mremap(const struct vm_special_mapping *sm,
+		       struct vm_area_struct *new_vma)
+{
+	unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
+
+	if (new_size != VVAR_NR_PAGES * PAGE_SIZE)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int __setup_additional_pages(enum arch_vdso_type arch_index,
 				    struct mm_struct *mm,
 				    struct linux_binprm *bprm,
@@ -315,6 +326,7 @@ static struct vm_special_mapping aarch32_vdso_spec[C_PAGES] = {
 	{
 		.name = "[vvar]",
 		.fault = vvar_fault,
+		.mremap = vvar_mremap,
 	},
 	{
 		.name = "[vdso]",
@@ -497,6 +509,7 @@ static struct vm_special_mapping vdso_spec[A_PAGES] __ro_after_init = {
 	{
 		.name	= "[vvar]",
 		.fault = vvar_fault,
+		.mremap = vvar_mremap,
 	},
 	{
 		.name	= "[vdso]",
-- 
2.24.1

[PATCH v2 6/6] arm64: enable time namespace support

[Andrei Vagin]

CONFIG_TIME_NS is dependes on GENERIC_VDSO_TIME_NS.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
---
 arch/arm64/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c6c32fb7f546..660ad0b0e6bb 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -110,6 +110,7 @@ config ARM64
 	select GENERIC_STRNLEN_USER
 	select GENERIC_TIME_VSYSCALL
 	select GENERIC_GETTIMEOFDAY
+	select GENERIC_VDSO_TIME_NS
 	select HANDLE_DOMAIN_IRQ
 	select HARDIRQS_SW_RESEND
 	select HAVE_PCI
-- 
2.24.1

Re: [PATCH v2 0/6] arm64: add the time namespace support

[Andrei Vagin]

Hi Vincenzo,

Have you had a chance to look at this series? Let me know if I need to
rebase this patch set and resend it again.

Thanks,
Andrei

Re: [PATCH v2 0/6] arm64: add the time namespace support

[Vincenzo Frascino]

Hi Andrei,

On 4/1/20 7:50 AM, Andrei Vagin wrote:
> Hi Vincenzo,
> 
> Have you had a chance to look at this series? Let me know if I need to
> rebase this patch set and resend it again.
> 

Sorry I still did not get the chance to have a look at your v2.
I will try to do it by the end of this week, beginning of next.

> Thanks,
> Andrei
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 1/6] arm64/vdso: use the fault callback to map vvar pages

[Vincenzo Frascino]

On 2/25/20 7:37 AM, Andrei Vagin wrote:
> This is required to support time namespaces where a time namespace data
> page is different for each namespace.
> 
> Signed-off-by: Andrei Vagin <avagin@gmail.com>

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  arch/arm64/kernel/vdso.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index 354b11e27c07..290c36d74e03 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -114,28 +114,32 @@ static int __vdso_init(enum arch_vdso_type arch_index)
>  			PAGE_SHIFT;
>  
>  	/* Allocate the vDSO pagelist, plus a page for the data. */
> -	vdso_pagelist = kcalloc(vdso_lookup[arch_index].vdso_pages + 1,
> +	vdso_pagelist = kcalloc(vdso_lookup[arch_index].vdso_pages,
>  				sizeof(struct page *),
>  				GFP_KERNEL);
>  	if (vdso_pagelist == NULL)
>  		return -ENOMEM;
>  
> -	/* Grab the vDSO data page. */
> -	vdso_pagelist[0] = phys_to_page(__pa_symbol(vdso_data));
> -
> -
>  	/* Grab the vDSO code pages. */
>  	pfn = sym_to_pfn(vdso_lookup[arch_index].vdso_code_start);
>  
>  	for (i = 0; i < vdso_lookup[arch_index].vdso_pages; i++)
> -		vdso_pagelist[i + 1] = pfn_to_page(pfn + i);
> +		vdso_pagelist[i] = pfn_to_page(pfn + i);
>  
> -	vdso_lookup[arch_index].dm->pages = &vdso_pagelist[0];
> -	vdso_lookup[arch_index].cm->pages = &vdso_pagelist[1];
> +	vdso_lookup[arch_index].cm->pages = vdso_pagelist;
>  
>  	return 0;
>  }
>  
> +static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
> +			     struct vm_area_struct *vma, struct vm_fault *vmf)
> +{
> +	if (vmf->pgoff == 0)
> +		return vmf_insert_pfn(vma, vmf->address,
> +				sym_to_pfn(vdso_data));
> +	return VM_FAULT_SIGBUS;
> +}
> +
>  static int __setup_additional_pages(enum arch_vdso_type arch_index,
>  				    struct mm_struct *mm,
>  				    struct linux_binprm *bprm,
> @@ -155,7 +159,7 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
>  	}
>  
>  	ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
> -				       VM_READ|VM_MAYREAD,
> +				       VM_READ|VM_MAYREAD|VM_PFNMAP,
>  				       vdso_lookup[arch_index].dm);
>  	if (IS_ERR(ret))
>  		goto up_fail;
> @@ -215,6 +219,7 @@ static struct vm_special_mapping aarch32_vdso_spec[C_PAGES] = {
>  #ifdef CONFIG_COMPAT_VDSO
>  	{
>  		.name = "[vvar]",
> +		.fault = vvar_fault,
>  	},
>  	{
>  		.name = "[vdso]",
> @@ -396,6 +401,7 @@ static int vdso_mremap(const struct vm_special_mapping *sm,
>  static struct vm_special_mapping vdso_spec[A_PAGES] __ro_after_init = {
>  	{
>  		.name	= "[vvar]",
> +		.fault = vvar_fault,
>  	},
>  	{
>  		.name	= "[vdso]",
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 2/6] arm64/vdso: Zap vvar pages when switching to a time namespace

[Vincenzo Frascino]

On 2/25/20 7:37 AM, Andrei Vagin wrote:
> The VVAR page layout depends on whether a task belongs to the root or
> non-root time namespace. Whenever a task changes its namespace, the VVAR
> page tables are cleared and then they will be re-faulted with a
> corresponding layout.
> 
> Signed-off-by: Andrei Vagin <avagin@gmail.com>

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  arch/arm64/kernel/vdso.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index 290c36d74e03..6ac9cdeac5be 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -131,6 +131,38 @@ static int __vdso_init(enum arch_vdso_type arch_index)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_TIME_NS
> +/*
> + * The vvar page layout depends on whether a task belongs to the root or
> + * non-root time namespace. Whenever a task changes its namespace, the VVAR
> + * page tables are cleared and then they will re-faulted with a
> + * corresponding layout.
> + * See also the comment near timens_setup_vdso_data() for details.
> + */
> +int vdso_join_timens(struct task_struct *task, struct time_namespace *ns)
> +{
> +	struct mm_struct *mm = task->mm;
> +	struct vm_area_struct *vma;
> +
> +	if (down_write_killable(&mm->mmap_sem))
> +		return -EINTR;
> +
> +	for (vma = mm->mmap; vma; vma = vma->vm_next) {
> +		unsigned long size = vma->vm_end - vma->vm_start;
> +
> +		if (vma_is_special_mapping(vma, vdso_lookup[ARM64_VDSO].dm))
> +			zap_page_range(vma, vma->vm_start, size);
> +#ifdef CONFIG_COMPAT_VDSO
> +		if (vma_is_special_mapping(vma, vdso_lookup[ARM64_VDSO32].dm))
> +			zap_page_range(vma, vma->vm_start, size);
> +#endif
> +	}
> +
> +	up_write(&mm->mmap_sem);
> +	return 0;
> +}
> +#endif
> +
>  static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
>  			     struct vm_area_struct *vma, struct vm_fault *vmf)
>  {
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 3/6] arm64/vdso: Add time napespace page

[Vincenzo Frascino]

On 2/25/20 7:37 AM, Andrei Vagin wrote:
> Allocate the time namespace page among VVAR pages.  Provide
> __arch_get_timens_vdso_data() helper for VDSO code to get the
> code-relative position of VVARs on that special page.
> 
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page. That page has vdso_data->seq
> set to 1 to enforce the slow path and vdso_data->clock_mode set to
> VCLOCK_TIMENS to enforce the time namespace handling path.
> 
> The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> update of the VDSO data is in progress, is not really affecting regular
> tasks which are not part of a time namespace as the task is spin waiting
> for the update to finish and vdso_data->seq to become even again.
> 
> If a time namespace task hits that code path, it invokes the corresponding
> time getter function which retrieves the real VVAR page, reads host time
> and then adds the offset for the requested clock which is stored in the
> special VVAR page.
> 
> Signed-off-by: Andrei Vagin <avagin@gmail.com>

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  arch/arm64/include/asm/vdso.h                 |  6 ++++++
>  .../include/asm/vdso/compat_gettimeofday.h    | 11 ++++++++++
>  arch/arm64/include/asm/vdso/gettimeofday.h    |  8 ++++++++
>  arch/arm64/kernel/vdso.c                      | 20 ++++++++++++++++---
>  arch/arm64/kernel/vdso/vdso.lds.S             |  5 ++++-
>  arch/arm64/kernel/vdso32/vdso.lds.S           |  5 ++++-
>  include/vdso/datapage.h                       |  1 +
>  7 files changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
> index 07468428fd29..351c145d3808 100644
> --- a/arch/arm64/include/asm/vdso.h
> +++ b/arch/arm64/include/asm/vdso.h
> @@ -12,6 +12,12 @@
>   */
>  #define VDSO_LBASE	0x0
>  
> +#ifdef CONFIG_TIME_NS
> +#define __VVAR_PAGES    2
> +#else
> +#define __VVAR_PAGES    1
> +#endif
> +
>  #ifndef __ASSEMBLY__
>  
>  #include <generated/vdso-offsets.h>
> diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> index 81b0c394f1d8..042814339a63 100644
> --- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> @@ -160,6 +160,17 @@ static __always_inline const struct vdso_data *__arch_get_vdso_data(void)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_TIME_NS
> +static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
> +{
> +	const struct vdso_data *ret;
> +
> +	asm volatile("mov %0, %1" : "=r"(ret) : "r"(_timens_data));
> +
> +	return ret;
> +}
> +#endif
> +
>  #endif /* !__ASSEMBLY__ */
>  
>  #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
> diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
> index 5a534432aa5d..553bdc19a91f 100644
> --- a/arch/arm64/include/asm/vdso/gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/gettimeofday.h
> @@ -97,6 +97,14 @@ const struct vdso_data *__arch_get_vdso_data(void)
>  	return _vdso_data;
>  }
>  
> +#ifdef CONFIG_TIME_NS
> +static __always_inline
> +const struct vdso_data *__arch_get_timens_vdso_data(void)
> +{
> +	return _timens_data;
> +}
> +#endif
> +
>  #endif /* !__ASSEMBLY__ */
>  
>  #endif /* __ASM_VDSO_GETTIMEOFDAY_H */
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index 6ac9cdeac5be..b3e7ce24e59b 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -46,6 +46,14 @@ enum arch_vdso_type {
>  #define VDSO_TYPES		(ARM64_VDSO + 1)
>  #endif /* CONFIG_COMPAT_VDSO */
>  
> +enum vvar_pages {
> +	VVAR_DATA_PAGE_OFFSET = 0,
> +#ifdef CONFIG_TIME_NS
> +	VVAR_TIMENS_PAGE_OFFSET = 1,
> +#endif /* CONFIG_TIME_NS */
> +	VVAR_NR_PAGES = __VVAR_PAGES,
> +};
> +
>  struct __vdso_abi {
>  	const char *name;
>  	const char *vdso_code_start;
> @@ -81,6 +89,12 @@ static union {
>  } vdso_data_store __page_aligned_data;
>  struct vdso_data *vdso_data = vdso_data_store.data;
>  
> +
> +struct vdso_data *arch_get_vdso_data(void *vvar_page)
> +{
> +	return (struct vdso_data *)(vvar_page);
> +}
> +
>  static int __vdso_remap(enum arch_vdso_type arch_index,
>  			const struct vm_special_mapping *sm,
>  			struct vm_area_struct *new_vma)
> @@ -182,7 +196,7 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
>  
>  	vdso_text_len = vdso_lookup[arch_index].vdso_pages << PAGE_SHIFT;
>  	/* Be sure to map the data page */
> -	vdso_mapping_len = vdso_text_len + PAGE_SIZE;
> +	vdso_mapping_len = vdso_text_len + VVAR_NR_PAGES * PAGE_SIZE;
>  
>  	vdso_base = get_unmapped_area(NULL, 0, vdso_mapping_len, 0, 0);
>  	if (IS_ERR_VALUE(vdso_base)) {
> @@ -190,13 +204,13 @@ static int __setup_additional_pages(enum arch_vdso_type arch_index,
>  		goto up_fail;
>  	}
>  
> -	ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
> +	ret = _install_special_mapping(mm, vdso_base, VVAR_NR_PAGES * PAGE_SIZE,
>  				       VM_READ|VM_MAYREAD|VM_PFNMAP,
>  				       vdso_lookup[arch_index].dm);
>  	if (IS_ERR(ret))
>  		goto up_fail;
>  
> -	vdso_base += PAGE_SIZE;
> +	vdso_base += VVAR_NR_PAGES * PAGE_SIZE;
>  	mm->context.vdso = (void *)vdso_base;
>  	ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
>  				       VM_READ|VM_EXEC|
> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
> index 7ad2d3a0cd48..d808ad31e01f 100644
> --- a/arch/arm64/kernel/vdso/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> @@ -17,7 +17,10 @@ OUTPUT_ARCH(aarch64)
>  
>  SECTIONS
>  {
> -	PROVIDE(_vdso_data = . - PAGE_SIZE);
> +	PROVIDE(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
> +#ifdef CONFIG_TIME_NS
> +	PROVIDE(_timens_data = _vdso_data + PAGE_SIZE);
> +#endif
>  	. = VDSO_LBASE + SIZEOF_HEADERS;
>  
>  	.hash		: { *(.hash) }			:text
> diff --git a/arch/arm64/kernel/vdso32/vdso.lds.S b/arch/arm64/kernel/vdso32/vdso.lds.S
> index a3944927eaeb..06cc60a9630f 100644
> --- a/arch/arm64/kernel/vdso32/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso32/vdso.lds.S
> @@ -17,7 +17,10 @@ OUTPUT_ARCH(arm)
>  
>  SECTIONS
>  {
> -	PROVIDE_HIDDEN(_vdso_data = . - PAGE_SIZE);
> +	PROVIDE_HIDDEN(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
> +#ifdef CONFIG_TIME_NS
> +	PROVIDE_HIDDEN(_timens_data = _vdso_data + PAGE_SIZE);
> +#endif
>  	. = VDSO_LBASE + SIZEOF_HEADERS;
>  
>  	.hash		: { *(.hash) }			:text
> diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
> index 30c4cb0428d1..3494b5536b63 100644
> --- a/include/vdso/datapage.h
> +++ b/include/vdso/datapage.h
> @@ -98,6 +98,7 @@ struct vdso_data {
>   * relocation, and this is what we need.
>   */
>  extern struct vdso_data _vdso_data[CS_BASES] __attribute__((visibility("hidden")));
> +extern struct vdso_data _timens_data[CS_BASES] __attribute__((visibility("hidden")));
>  
>  #endif /* !__ASSEMBLY__ */
>  
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 4/6] arm64/vdso: Handle faults on timens page

[Vincenzo Frascino]

On 2/25/20 7:37 AM, Andrei Vagin wrote:
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page.
> 
> Signed-off-by: Andrei Vagin <avagin@gmail.com>

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  arch/arm64/kernel/vdso.c | 57 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index b3e7ce24e59b..fb32c6f76078 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -18,6 +18,7 @@
>  #include <linux/sched.h>
>  #include <linux/signal.h>
>  #include <linux/slab.h>
> +#include <linux/time_namespace.h>
>  #include <linux/timekeeper_internal.h>
>  #include <linux/vmalloc.h>
>  #include <vdso/datapage.h>
> @@ -175,15 +176,63 @@ int vdso_join_timens(struct task_struct *task, struct time_namespace *ns)
>  	up_write(&mm->mmap_sem);
>  	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 inline 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)
>  {
> -	if (vmf->pgoff == 0)
> -		return vmf_insert_pfn(vma, vmf->address,
> -				sym_to_pfn(vdso_data));
> -	return VM_FAULT_SIGBUS;
> +	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 = sym_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 = sym_to_pfn(vdso_data);
> +		break;
> +#endif /* CONFIG_TIME_NS */
> +	default:
> +		return VM_FAULT_SIGBUS;
> +	}
> +
> +	return vmf_insert_pfn(vma, vmf->address, pfn);
>  }
>  
>  static int __setup_additional_pages(enum arch_vdso_type arch_index,
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 5/6] arm64/vdso: Restrict splitting VVAR VMA

[Vincenzo Frascino]

On 2/25/20 7:37 AM, Andrei Vagin wrote:
> Forbid splitting VVAR VMA resulting in a stricter ABI and reducing the
> amount of corner-cases to consider while working further on VDSO time
> namespace support.
> 
> As the offset from timens to VVAR page is computed compile-time, the pages
> in VVAR should stay together and not being partically mremap()'ed.
> 
> Signed-off-by: Andrei Vagin <avagin@gmail.com>

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  arch/arm64/kernel/vdso.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index fb32c6f76078..c003f7ee383a 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -235,6 +235,17 @@ static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
>  	return vmf_insert_pfn(vma, vmf->address, pfn);
>  }
>  
> +static int vvar_mremap(const struct vm_special_mapping *sm,
> +		       struct vm_area_struct *new_vma)
> +{
> +	unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
> +
> +	if (new_size != VVAR_NR_PAGES * PAGE_SIZE)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>  static int __setup_additional_pages(enum arch_vdso_type arch_index,
>  				    struct mm_struct *mm,
>  				    struct linux_binprm *bprm,
> @@ -315,6 +326,7 @@ static struct vm_special_mapping aarch32_vdso_spec[C_PAGES] = {
>  	{
>  		.name = "[vvar]",
>  		.fault = vvar_fault,
> +		.mremap = vvar_mremap,
>  	},
>  	{
>  		.name = "[vdso]",
> @@ -497,6 +509,7 @@ static struct vm_special_mapping vdso_spec[A_PAGES] __ro_after_init = {
>  	{
>  		.name	= "[vvar]",
>  		.fault = vvar_fault,
> +		.mremap = vvar_mremap,
>  	},
>  	{
>  		.name	= "[vdso]",
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 6/6] arm64: enable time namespace support

[Vincenzo Frascino]

On 2/25/20 7:37 AM, Andrei Vagin wrote:
> CONFIG_TIME_NS is dependes on GENERIC_VDSO_TIME_NS.
> 
> Signed-off-by: Andrei Vagin <avagin@gmail.com>

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  arch/arm64/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index c6c32fb7f546..660ad0b0e6bb 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -110,6 +110,7 @@ config ARM64
>  	select GENERIC_STRNLEN_USER
>  	select GENERIC_TIME_VSYSCALL
>  	select GENERIC_GETTIMEOFDAY
> +	select GENERIC_VDSO_TIME_NS
>  	select HANDLE_DOMAIN_IRQ
>  	select HARDIRQS_SW_RESEND
>  	select HAVE_PCI
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 0/6] arm64: add the time namespace support

[Vincenzo Frascino]

Hi Andrei,

On 2/25/20 7:37 AM, Andrei Vagin wrote:
> Allocate the time namespace page among VVAR pages and add the logic
> to handle faults on VVAR properly.
> 
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page. That page has vdso_data->seq
> set to 1 to enforce the slow path and vdso_data->clock_mode set to
> VCLOCK_TIMENS to enforce the time namespace handling path.
> 
> The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> update of the VDSO data is in progress, is not really affecting regular
> tasks which are not part of a time namespace as the task is spin waiting
> for the update to finish and vdso_data->seq to become even again.
> 
> If a time namespace task hits that code path, it invokes the corresponding
> time getter function which retrieves the real VVAR page, reads host time
> and then adds the offset for the requested clock which is stored in the
> special VVAR page.
> 
> v2: Code cleanups suggested by Vincenzo.
> 

Sorry for the delay, I completed this morning the review of your patches and I
do not have anymore comments on them. Thank you for making the effort and
bringing the time namespace support to arm64.

I have though a question on something I encountered during the testing of the
patches: I noticed that all the tests related to CLOCK_BOOTTIME_ALARM fail on
arm64 (please find the results below the scissors). Is this expected?

--->8---

1..4
ok 1 clockid: 1 abs:0
ok 2 clockid: 1 abs:1
not ok 3 # error clock_gettime: Invalid argument
not ok 4 # error clock_gettime: Invalid argument
Bail out!
# Pass 2 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 2
1..1
ok 1 exec
# Pass 1 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0
1..8
not ok 1 # error Warning: failed to find clock_gettime in vDSO

./timens.sh: line 5: 15382 Segmentation fault      (core dumped) ./gettime_perf
1..1
ok 1 Passed for /proc/uptime
# Pass 1 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0
1..10
ok 1 Passed for CLOCK_BOOTTIME (syscall)
ok 2 Passed for CLOCK_BOOTTIME (vdso)
not ok 3 # error syscall(SYS_clock_gettime(9)): Invalid argument
not ok 4 # error clock_gettime(9): Invalid argument
ok 5 Passed for CLOCK_MONOTONIC (syscall)
ok 6 Passed for CLOCK_MONOTONIC (vdso)
ok 7 Passed for CLOCK_MONOTONIC_COARSE (syscall)
ok 8 Passed for CLOCK_MONOTONIC_COARSE (vdso)
ok 9 Passed for CLOCK_MONOTONIC_RAW (syscall)
ok 10 Passed for CLOCK_MONOTONIC_RAW (vdso)
Bail out!
# Pass 8 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 2
1..3
ok 1 clockid=7
ok 2 clockid=1
not ok 3 # error timerfd_create: Operation not supported
Bail out!
# Pass 2 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 1
1..3
ok 1 clockid=7
ok 2 clockid=1
ok 3 clockid=9
# Pass 3 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0

[...]

-- 
Regards,
Vincenzo

Re: [PATCH v2 0/6] arm64: add the time namespace support

[Andrei Vagin]

On Thu, Apr 9, 2020 at 6:23 AM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Andrei,
>
> On 2/25/20 7:37 AM, Andrei Vagin wrote:
> > Allocate the time namespace page among VVAR pages and add the logic
> > to handle faults on VVAR properly.
> >
> > If a task belongs to a time namespace then the VVAR page which contains
> > the system wide VDSO data is replaced with a namespace specific page
> > which has the same layout as the VVAR page. That page has vdso_data->seq
> > set to 1 to enforce the slow path and vdso_data->clock_mode set to
> > VCLOCK_TIMENS to enforce the time namespace handling path.
> >
> > The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> > update of the VDSO data is in progress, is not really affecting regular
> > tasks which are not part of a time namespace as the task is spin waiting
> > for the update to finish and vdso_data->seq to become even again.
> >
> > If a time namespace task hits that code path, it invokes the corresponding
> > time getter function which retrieves the real VVAR page, reads host time
> > and then adds the offset for the requested clock which is stored in the
> > special VVAR page.
> >
> > v2: Code cleanups suggested by Vincenzo.
> >
>
> Sorry for the delay, I completed this morning the review of your patches and I
> do not have anymore comments on them. Thank you for making the effort and
> bringing the time namespace support to arm64.

Thank you for the review of these patches.

>
> I have though a question on something I encountered during the testing of the
> patches: I noticed that all the tests related to CLOCK_BOOTTIME_ALARM fail on
> arm64 (please find the results below the scissors). Is this expected?

static int alarm_clock_get_timespec(clockid_t which_clock, struct
timespec64 *tp)
{
        struct alarm_base *base = &alarm_bases[clock2alarm(which_clock)];

        if (!alarmtimer_get_rtcdev())
                return -EINVAL;

It is probably that you get EINVAL from here ^^^. I will send a
separate patch to handle this case in tests properly.

Thanks,
Andrei

Re: [PATCH v2 0/6] arm64: add the time namespace support

[Vincenzo Frascino]

Hi Andrei,

On 4/11/20 8:33 AM, Andrei Vagin wrote:
> On Thu, Apr 9, 2020 at 6:23 AM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> Hi Andrei,
>>
[...]

>> Sorry for the delay, I completed this morning the review of your patches and I
>> do not have anymore comments on them. Thank you for making the effort and
>> bringing the time namespace support to arm64.
> 
> Thank you for the review of these patches.
> 
>>
>> I have though a question on something I encountered during the testing of the
>> patches: I noticed that all the tests related to CLOCK_BOOTTIME_ALARM fail on
>> arm64 (please find the results below the scissors). Is this expected?
> 
> static int alarm_clock_get_timespec(clockid_t which_clock, struct
> timespec64 *tp)
> {
>         struct alarm_base *base = &alarm_bases[clock2alarm(which_clock)];
> 
>         if (!alarmtimer_get_rtcdev())
>                 return -EINVAL;
> 
> It is probably that you get EINVAL from here ^^^. I will send a
> separate patch to handle this case in tests properly.
>

This makes sense :) Please let me know when you post the fix so I can test it again.

Are you planning as well to rebase this set?

> Thanks,
> Andrei
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 3/6] arm64/vdso: Add time napespace page

[Mark Rutland]

On Mon, Feb 24, 2020 at 11:37:28PM -0800, Andrei Vagin wrote:
> Allocate the time namespace page among VVAR pages.  Provide
> __arch_get_timens_vdso_data() helper for VDSO code to get the
> code-relative position of VVARs on that special page.
> 
> If a task belongs to a time namespace then the VVAR page which contains
> the system wide VDSO data is replaced with a namespace specific page
> which has the same layout as the VVAR page. That page has vdso_data->seq
> set to 1 to enforce the slow path and vdso_data->clock_mode set to
> VCLOCK_TIMENS to enforce the time namespace handling path.
> 
> The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> update of the VDSO data is in progress, is not really affecting regular
> tasks which are not part of a time namespace as the task is spin waiting
> for the update to finish and vdso_data->seq to become even again.
> 
> If a time namespace task hits that code path, it invokes the corresponding
> time getter function which retrieves the real VVAR page, reads host time
> and then adds the offset for the requested clock which is stored in the
> special VVAR page.
> 
> Signed-off-by: Andrei Vagin <avagin@gmail.com>
> ---
>  arch/arm64/include/asm/vdso.h                 |  6 ++++++
>  .../include/asm/vdso/compat_gettimeofday.h    | 11 ++++++++++
>  arch/arm64/include/asm/vdso/gettimeofday.h    |  8 ++++++++
>  arch/arm64/kernel/vdso.c                      | 20 ++++++++++++++++---
>  arch/arm64/kernel/vdso/vdso.lds.S             |  5 ++++-
>  arch/arm64/kernel/vdso32/vdso.lds.S           |  5 ++++-
>  include/vdso/datapage.h                       |  1 +
>  7 files changed, 51 insertions(+), 5 deletions(-)

> +#ifdef CONFIG_TIME_NS
> +static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
> +{
> +	const struct vdso_data *ret;
> +
> +	asm volatile("mov %0, %1" : "=r"(ret) : "r"(_timens_data));
> +
> +	return ret;
> +}
> +#endif

What is this inline assembly for? The commit message doesn't mention it,
there's no explanation here, and the native version doesn't do likewise
so it seems rather surprising.

Thanks,
Mark.

> diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
> index 5a534432aa5d..553bdc19a91f 100644
> --- a/arch/arm64/include/asm/vdso/gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/gettimeofday.h
> @@ -97,6 +97,14 @@ const struct vdso_data *__arch_get_vdso_data(void)
>  	return _vdso_data;
>  }
>  
> +#ifdef CONFIG_TIME_NS
> +static __always_inline
> +const struct vdso_data *__arch_get_timens_vdso_data(void)
> +{
> +	return _timens_data;
> +}
> +#endif

Re: [PATCH v2 3/6] arm64/vdso: Add time napespace page

[Andrei Vagin]

On Tue, Apr 14, 2020 at 10:20 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Mon, Feb 24, 2020 at 11:37:28PM -0800, Andrei Vagin wrote:
> > Allocate the time namespace page among VVAR pages.  Provide
> > __arch_get_timens_vdso_data() helper for VDSO code to get the
> > code-relative position of VVARs on that special page.
> >
> > If a task belongs to a time namespace then the VVAR page which contains
> > the system wide VDSO data is replaced with a namespace specific page
> > which has the same layout as the VVAR page. That page has vdso_data->seq
> > set to 1 to enforce the slow path and vdso_data->clock_mode set to
> > VCLOCK_TIMENS to enforce the time namespace handling path.
> >
> > The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> > update of the VDSO data is in progress, is not really affecting regular
> > tasks which are not part of a time namespace as the task is spin waiting
> > for the update to finish and vdso_data->seq to become even again.
> >
> > If a time namespace task hits that code path, it invokes the corresponding
> > time getter function which retrieves the real VVAR page, reads host time
> > and then adds the offset for the requested clock which is stored in the
> > special VVAR page.
> >
> > Signed-off-by: Andrei Vagin <avagin@gmail.com>
> > ---
> >  arch/arm64/include/asm/vdso.h                 |  6 ++++++
> >  .../include/asm/vdso/compat_gettimeofday.h    | 11 ++++++++++
> >  arch/arm64/include/asm/vdso/gettimeofday.h    |  8 ++++++++
> >  arch/arm64/kernel/vdso.c                      | 20 ++++++++++++++++---
> >  arch/arm64/kernel/vdso/vdso.lds.S             |  5 ++++-
> >  arch/arm64/kernel/vdso32/vdso.lds.S           |  5 ++++-
> >  include/vdso/datapage.h                       |  1 +
> >  7 files changed, 51 insertions(+), 5 deletions(-)
>
> > +#ifdef CONFIG_TIME_NS
> > +static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
> > +{
> > +     const struct vdso_data *ret;
> > +
> > +     asm volatile("mov %0, %1" : "=r"(ret) : "r"(_timens_data));
> > +
> > +     return ret;
> > +}
> > +#endif
>
> What is this inline assembly for? The commit message doesn't mention it,
> there's no explanation here, and the native version doesn't do likewise
> so it seems rather surprising.

__arch_get_vdso_data is  right before this function and there is a
comment which explains this:
https://github.com/torvalds/linux/blob/master/arch/arm64/include/asm/vdso/compat_gettimeofday.h#L137

"""
/*
* This simply puts &_vdso_data into ret. The reason why we don't use
* `ret = _vdso_data` is that the compiler tends to optimize this in a
* very suboptimal way: instead of keeping &_vdso_data in a register,
* it goes through a relocation almost every time _vdso_data must be
* accessed (even in subfunctions). This is both time and space
* consuming: each relocation uses a word in the code section, and it
* has to be loaded at runtime.
*
* This trick hides the assignment from the compiler. Since it cannot
* track where the pointer comes from, it will only use one relocation
* where __arch_get_vdso_data() is called, and then keep the result in
* a register.
*/
"""

I decided to not duplicate the comment because these two functions are
very similar and close to each other.

Thanks,
Andrei

Re: [PATCH v2 3/6] arm64/vdso: Add time napespace page

[Mark Rutland]

On Tue, Apr 14, 2020 at 07:34:41PM -0700, Andrei Vagin wrote:
> On Tue, Apr 14, 2020 at 10:20 AM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Mon, Feb 24, 2020 at 11:37:28PM -0800, Andrei Vagin wrote:
> > > Allocate the time namespace page among VVAR pages.  Provide
> > > __arch_get_timens_vdso_data() helper for VDSO code to get the
> > > code-relative position of VVARs on that special page.
> > >
> > > If a task belongs to a time namespace then the VVAR page which contains
> > > the system wide VDSO data is replaced with a namespace specific page
> > > which has the same layout as the VVAR page. That page has vdso_data->seq
> > > set to 1 to enforce the slow path and vdso_data->clock_mode set to
> > > VCLOCK_TIMENS to enforce the time namespace handling path.
> > >
> > > The extra check in the case that vdso_data->seq is odd, e.g. a concurrent
> > > update of the VDSO data is in progress, is not really affecting regular
> > > tasks which are not part of a time namespace as the task is spin waiting
> > > for the update to finish and vdso_data->seq to become even again.
> > >
> > > If a time namespace task hits that code path, it invokes the corresponding
> > > time getter function which retrieves the real VVAR page, reads host time
> > > and then adds the offset for the requested clock which is stored in the
> > > special VVAR page.
> > >
> > > Signed-off-by: Andrei Vagin <avagin@gmail.com>
> > > ---
> > >  arch/arm64/include/asm/vdso.h                 |  6 ++++++
> > >  .../include/asm/vdso/compat_gettimeofday.h    | 11 ++++++++++
> > >  arch/arm64/include/asm/vdso/gettimeofday.h    |  8 ++++++++
> > >  arch/arm64/kernel/vdso.c                      | 20 ++++++++++++++++---
> > >  arch/arm64/kernel/vdso/vdso.lds.S             |  5 ++++-
> > >  arch/arm64/kernel/vdso32/vdso.lds.S           |  5 ++++-
> > >  include/vdso/datapage.h                       |  1 +
> > >  7 files changed, 51 insertions(+), 5 deletions(-)
> >
> > > +#ifdef CONFIG_TIME_NS
> > > +static __always_inline const struct vdso_data *__arch_get_timens_vdso_data(void)
> > > +{
> > > +     const struct vdso_data *ret;
> > > +
> > > +     asm volatile("mov %0, %1" : "=r"(ret) : "r"(_timens_data));
> > > +
> > > +     return ret;
> > > +}
> > > +#endif
> >
> > What is this inline assembly for? The commit message doesn't mention it,
> > there's no explanation here, and the native version doesn't do likewise
> > so it seems rather surprising.
> 
> __arch_get_vdso_data is  right before this function and there is a
> comment which explains this:
> https://github.com/torvalds/linux/blob/master/arch/arm64/include/asm/vdso/compat_gettimeofday.h#L137
> 
> """
> /*
> * This simply puts &_vdso_data into ret. The reason why we don't use
> * `ret = _vdso_data` is that the compiler tends to optimize this in a
> * very suboptimal way: instead of keeping &_vdso_data in a register,
> * it goes through a relocation almost every time _vdso_data must be
> * accessed (even in subfunctions). This is both time and space
> * consuming: each relocation uses a word in the code section, and it
> * has to be loaded at runtime.
> *
> * This trick hides the assignment from the compiler. Since it cannot
> * track where the pointer comes from, it will only use one relocation
> * where __arch_get_vdso_data() is called, and then keep the result in
> * a register.
> */
> """
> 
> I decided to not duplicate the comment because these two functions are
> very similar and close to each other.

Ah, I was not aware of that context. Could we just add a trivial comment
to say "see __arch_get_vdso_data" or something like that?

It's a shame we're not using OPTIMIZER_HIDE_VAR() for that, as it can
generate slightly better code and is easier to read than bare asm.

Thanks,
Mark.

Re: [PATCH v2 3/6] arm64/vdso: Add time napespace page

[Vincenzo Frascino]

Hi Mark,

On 4/15/20 11:05 AM, Mark Rutland wrote:
> It's a shame we're not using OPTIMIZER_HIDE_VAR() for that, as it can
> generate slightly better code and is easier to read than bare asm.

We are not because I was not aware when I wrote this code that such a macro
existed :)

But you are right it clearly makes the code more readable. To make up for it, I
am happy to make the effort to introduce it for both the cases and replace the
assembler once this patch series gets merged.

-- 
Regards,
Vincenzo

Re: [PATCH v2 3/6] arm64/vdso: Add time napespace page

[Mark Rutland]

On Wed, Apr 15, 2020 at 11:16:00AM +0100, Vincenzo Frascino wrote:
> Hi Mark,
> 
> On 4/15/20 11:05 AM, Mark Rutland wrote:
> > It's a shame we're not using OPTIMIZER_HIDE_VAR() for that, as it can
> > generate slightly better code and is easier to read than bare asm.
> 
> We are not because I was not aware when I wrote this code that such a macro
> existed :)
> 
> But you are right it clearly makes the code more readable. To make up for it, I
> am happy to make the effort to introduce it for both the cases and replace the
> assembler once this patch series gets merged.

Sounds good to me!

Mark.

Re: [PATCH v2 0/6] arm64: add the time namespace support

[Andrei Vagin]

On Tue, Apr 14, 2020 at 2:02 AM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Andrei,
>
> On 4/11/20 8:33 AM, Andrei Vagin wrote:
> > On Thu, Apr 9, 2020 at 6:23 AM Vincenzo Frascino
> > <vincenzo.frascino@arm.com> wrote:
> >>
> >> I have though a question on something I encountered during the testing of the
> >> patches: I noticed that all the tests related to CLOCK_BOOTTIME_ALARM fail on
> >> arm64 (please find the results below the scissors). Is this expected?
> >
> > static int alarm_clock_get_timespec(clockid_t which_clock, struct
> > timespec64 *tp)
> > {
> >         struct alarm_base *base = &alarm_bases[clock2alarm(which_clock)];
> >
> >         if (!alarmtimer_get_rtcdev())
> >                 return -EINVAL;
> >
> > It is probably that you get EINVAL from here ^^^. I will send a
> > separate patch to handle this case in tests properly.
> >
>
> This makes sense :) Please let me know when you post the fix so I can test it again.

I have sent this fix: https://lkml.org/lkml/2020/4/15/72

>
> Are you planning as well to rebase this set?

What is the right tree to rebase on?

Thanks,
Andrei

Re: [PATCH v2 0/6] arm64: add the time namespace support

[Vincenzo Frascino]

Hi Andrei,

On 4/15/20 5:14 PM, Andrei Vagin wrote:
> On Tue, Apr 14, 2020 at 2:02 AM Vincenzo Frascino
> <vincenzo.frascino@arm.com> wrote:
>>
>> Hi Andrei,
>>
>> On 4/11/20 8:33 AM, Andrei Vagin wrote:
>>> On Thu, Apr 9, 2020 at 6:23 AM Vincenzo Frascino
>>> <vincenzo.frascino@arm.com> wrote:
>>>>
>>>> I have though a question on something I encountered during the testing of the
>>>> patches: I noticed that all the tests related to CLOCK_BOOTTIME_ALARM fail on
>>>> arm64 (please find the results below the scissors). Is this expected?
>>>
>>> static int alarm_clock_get_timespec(clockid_t which_clock, struct
>>> timespec64 *tp)
>>> {
>>>         struct alarm_base *base = &alarm_bases[clock2alarm(which_clock)];
>>>
>>>         if (!alarmtimer_get_rtcdev())
>>>                 return -EINVAL;
>>>
>>> It is probably that you get EINVAL from here ^^^. I will send a
>>> separate patch to handle this case in tests properly.
>>>
>>
>> This makes sense :) Please let me know when you post the fix so I can test it again.
> 
> I have sent this fix: https://lkml.org/lkml/2020/4/15/72
>

That's good, I will try it by the end of this week or beginning of next and let
you know the results.

>>
>> Are you planning as well to rebase this set?>
> What is the right tree to rebase on?
> 

I guess master, I was asking because it would make easier my testing :)

> Thanks,
> Andrei
> 

-- 
Regards,
Vincenzo