user-mode-linux-devel

Re: [uml-devel] [PATCH] um: Fix gcc 7 sys/sysmacros.h warning

[Richard W. <ric...@gm...>]

On Thu, Jul 6, 2017 at 10:47 PM, Thomas Meyer <th...@m3...> wrote:
> Signed-off-by: Thomas Meyer <th...@m3...>

When fixing such a problem, always include die compiler warning.
This has to wait for -rc2.

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH 3/3] um: Add kerneldoc for userspace_tramp() and start_userspace()

[Richard W. <ric...@gm...>]

On Thu, Jul 6, 2017 at 12:34 AM, Thomas Meyer <th...@m3...> wrote:
> Also use correct function name spelling (stub_segv_handler) for better grepping
>
> Signed-off-by: Thomas Meyer <th...@m3...>
> ---
>  arch/um/os-Linux/skas/process.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)

Series applied.

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH] um: userspace - be more verbose in ptrace set regs error

[Richard W. <ric...@gm...>]

On Thu, Jul 6, 2017 at 12:31 AM, Thomas Meyer <th...@m3...> wrote:
> When ptrace fails to set GP/FP regs for the target process,
> log the error before crashing the UML kernel.
>
> Signed-off-by: Thomas Meyer <th...@m3...>
> ---
>  arch/um/os-Linux/skas/process.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Applied.

-- 
Thanks,
//richard

[uml-devel] [PATCH] um: Use generic NOTES macro

[Thomas M. <th...@m3...>]

Signed-off-by: Thomas Meyer <th...@m3...>
---
 arch/um/include/asm/common.lds.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/um/include/asm/common.lds.S b/arch/um/include/asm/common.lds.S
index 133055311dce..de996e428ca4 100644
--- a/arch/um/include/asm/common.lds.S
+++ b/arch/um/include/asm/common.lds.S
@@ -15,8 +15,8 @@
   PROVIDE (_unprotected_end = .);
 
   . = ALIGN(4096);
-  .note : { *(.note.*) }
-  EXCEPTION_TABLE(0)
+  NOTES
+  EXCEPTION_TABLE(4)
 
   BUG_TABLE
 
-- 
2.13.0

[uml-devel] [PATCH] um: Fix FP register size for XSTATE/XSAVE

[Thomas M. <th...@m3...>]

Hard code max size. Taken from
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/common/x86-xstate.h

Signed-off-by: Thomas Meyer <th...@m3...>
---
 arch/x86/um/os-Linux/registers.c | 9 ++++-----
 arch/x86/um/user-offsets.c       | 2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/um/os-Linux/registers.c b/arch/x86/um/os-Linux/registers.c
index 00f54a91bb4b..7265511d8e3b 100644
--- a/arch/x86/um/os-Linux/registers.c
+++ b/arch/x86/um/os-Linux/registers.c
@@ -30,7 +30,7 @@ int save_fp_registers(int pid, unsigned long *fp_regs)
 
 	if (have_xstate_support) {
 		iov.iov_base = fp_regs;
-		iov.iov_len = sizeof(struct _xstate);
+		iov.iov_len = FP_SIZE * sizeof(unsigned long);
 		if (ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov) < 0)
 			return -errno;
 		return 0;
@@ -49,10 +49,9 @@ int restore_i387_registers(int pid, unsigned long *fp_regs)
 int restore_fp_registers(int pid, unsigned long *fp_regs)
 {
 	struct iovec iov;
-
 	if (have_xstate_support) {
 		iov.iov_base = fp_regs;
-		iov.iov_len = sizeof(struct _xstate);
+		iov.iov_len = FP_SIZE * sizeof(unsigned long);
 		if (ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov) < 0)
 			return -errno;
 		return 0;
@@ -122,11 +121,11 @@ int put_fp_registers(int pid, unsigned long *regs)
 
 void arch_init_registers(int pid)
 {
-	struct _xstate fp_regs;
+	unsigned long fp_regs[FP_SIZE];
 	struct iovec iov;
 
 	iov.iov_base = &fp_regs;
-	iov.iov_len = sizeof(struct _xstate);
+	iov.iov_len = FP_SIZE * sizeof(unsigned long);
 	if (ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov) == 0)
 		have_xstate_support = 1;
 }
diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c
index cb3c22370cf5..c0111f093361 100644
--- a/arch/x86/um/user-offsets.c
+++ b/arch/x86/um/user-offsets.c
@@ -50,7 +50,7 @@ void foo(void)
 	DEFINE(HOST_GS, GS);
 	DEFINE(HOST_ORIG_AX, ORIG_EAX);
 #else
-	DEFINE(HOST_FP_SIZE, sizeof(struct _xstate) / sizeof(unsigned long));
+	DEFINE_LONGS(HOST_FP_SIZE, 2696);
 	DEFINE_LONGS(HOST_BX, RBX);
 	DEFINE_LONGS(HOST_CX, RCX);
 	DEFINE_LONGS(HOST_DI, RDI);
-- 
2.13.0

[uml-devel] [PATCH] um: Fix gcc 7 sys/sysmacros.h warning

[Thomas M. <th...@m3...>]

Signed-off-by: Thomas Meyer <th...@m3...>
---
 arch/um/os-Linux/file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c
index 2db18cbbb0ea..d1901de8ce9f 100644
--- a/arch/um/os-Linux/file.c
+++ b/arch/um/os-Linux/file.c
@@ -14,6 +14,7 @@
 #include <sys/stat.h>
 #include <sys/un.h>
 #include <sys/types.h>
+#include <sys/sysmacros.h>
 #include <os.h>
 
 static void copy_stat(struct uml_stat *dst, const struct stat64 *src)
-- 
2.13.0

Re: [uml-devel] [uml:linux-next 7/9] arch/um/os-Linux/skas/process.c:579:1: warning: control reaches end of non-void function

[Richard W. <ri...@no...>]

Am 06.07.2017 um 07:48 schrieb kbuild test robot:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/rw/uml.git linux-next
> head:   1bcbfbfdeb0091036db7a32e1cd31b49cce5983a
> commit: f44f1e7da7c8e3f4575d5d61c4df978496903fcc [7/9] um: Avoid longjmp/setjmp symbol clashes with libpthread.a
> config: um-x86_64_defconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         git checkout f44f1e7da7c8e3f4575d5d61c4df978496903fcc
>         # save the attached .config to linux build tree
>         make ARCH=um SUBARCH=x86_64
> 
> All warnings (new ones prefixed by >>):
> 
>    arch/um/os-Linux/skas/process.c: In function 'start_idle_thread':
>>> arch/um/os-Linux/skas/process.c:579:1: warning: control reaches end of non-void function [-Wreturn-type]
>     }
>     ^
> 
> vim +579 arch/um/os-Linux/skas/process.c
> 
> abaf6977 Gennady Sharapov 2006-01-18  563  	case INIT_JMP_CALLBACK:
> abaf6977 Gennady Sharapov 2006-01-18  564  		(*cb_proc)(cb_arg);
> 77f6af77 Jeff Dike        2007-05-06  565  		longjmp(*cb_back, 1);
> abaf6977 Gennady Sharapov 2006-01-18  566  		break;
> abaf6977 Gennady Sharapov 2006-01-18  567  	case INIT_JMP_HALT:
> abaf6977 Gennady Sharapov 2006-01-18  568  		kmalloc_ok = 0;
> ba180fd4 Jeff Dike        2007-10-16  569  		return 0;
> abaf6977 Gennady Sharapov 2006-01-18  570  	case INIT_JMP_REBOOT:
> abaf6977 Gennady Sharapov 2006-01-18  571  		kmalloc_ok = 0;
> ba180fd4 Jeff Dike        2007-10-16  572  		return 1;
> abaf6977 Gennady Sharapov 2006-01-18  573  	default:
> 3e6f2ac4 Jeff Dike        2008-02-04  574  		printk(UM_KERN_ERR "Bad sigsetjmp return in "
> 3e6f2ac4 Jeff Dike        2008-02-04  575  		       "start_idle_thread - %d\n", n);
> 3e6f2ac4 Jeff Dike        2008-02-04  576  		fatal_sigsegv();
> abaf6977 Gennady Sharapov 2006-01-18  577  	}
> 77f6af77 Jeff Dike        2007-05-06  578  	longjmp(*switch_buf, 1);
> abaf6977 Gennady Sharapov 2006-01-18 @579  }
> abaf6977 Gennady Sharapov 2006-01-18  580  
> abaf6977 Gennady Sharapov 2006-01-18  581  void initial_thread_cb_skas(void (*proc)(void *), void *arg)
> abaf6977 Gennady Sharapov 2006-01-18  582  {
> ad28e029 Jeff Dike        2006-04-18  583  	jmp_buf here;
> abaf6977 Gennady Sharapov 2006-01-18  584  
> abaf6977 Gennady Sharapov 2006-01-18  585  	cb_proc = proc;
> abaf6977 Gennady Sharapov 2006-01-18  586  	cb_arg = arg;
> abaf6977 Gennady Sharapov 2006-01-18  587  	cb_back = &here;

I suggest this as fix:
diff --git a/arch/um/os-Linux/skas/process.c b/arch/um/os-Linux/skas/process.c
index 03b3c4cc7735..6869df60a722 100644
--- a/arch/um/os-Linux/skas/process.c
+++ b/arch/um/os-Linux/skas/process.c
@@ -576,6 +576,9 @@ int start_idle_thread(void *stack, jmp_buf *switch_buf)
                fatal_sigsegv();
        }
        longjmp(*switch_buf, 1);
+
+       /* Unreachable */
+       return 0;
 }

Unless I miss something the idle thread never exits here.

Thanks,
//richard

[uml-devel] [uml:linux-next 7/9] arch/um/os-Linux/skas/process.c:579:1: warning: control reaches end of non-void function

[kbuild t. r. <fen...@in...>]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/rw/uml.git linux-next
head:   1bcbfbfdeb0091036db7a32e1cd31b49cce5983a
commit: f44f1e7da7c8e3f4575d5d61c4df978496903fcc [7/9] um: Avoid longjmp/setjmp symbol clashes with libpthread.a
config: um-x86_64_defconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        git checkout f44f1e7da7c8e3f4575d5d61c4df978496903fcc
        # save the attached .config to linux build tree
        make ARCH=um SUBARCH=x86_64

All warnings (new ones prefixed by >>):

   arch/um/os-Linux/skas/process.c: In function 'start_idle_thread':
>> arch/um/os-Linux/skas/process.c:579:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +579 arch/um/os-Linux/skas/process.c

abaf6977 Gennady Sharapov 2006-01-18  563  	case INIT_JMP_CALLBACK:
abaf6977 Gennady Sharapov 2006-01-18  564  		(*cb_proc)(cb_arg);
77f6af77 Jeff Dike        2007-05-06  565  		longjmp(*cb_back, 1);
abaf6977 Gennady Sharapov 2006-01-18  566  		break;
abaf6977 Gennady Sharapov 2006-01-18  567  	case INIT_JMP_HALT:
abaf6977 Gennady Sharapov 2006-01-18  568  		kmalloc_ok = 0;
ba180fd4 Jeff Dike        2007-10-16  569  		return 0;
abaf6977 Gennady Sharapov 2006-01-18  570  	case INIT_JMP_REBOOT:
abaf6977 Gennady Sharapov 2006-01-18  571  		kmalloc_ok = 0;
ba180fd4 Jeff Dike        2007-10-16  572  		return 1;
abaf6977 Gennady Sharapov 2006-01-18  573  	default:
3e6f2ac4 Jeff Dike        2008-02-04  574  		printk(UM_KERN_ERR "Bad sigsetjmp return in "
3e6f2ac4 Jeff Dike        2008-02-04  575  		       "start_idle_thread - %d\n", n);
3e6f2ac4 Jeff Dike        2008-02-04  576  		fatal_sigsegv();
abaf6977 Gennady Sharapov 2006-01-18  577  	}
77f6af77 Jeff Dike        2007-05-06  578  	longjmp(*switch_buf, 1);
abaf6977 Gennady Sharapov 2006-01-18 @579  }
abaf6977 Gennady Sharapov 2006-01-18  580  
abaf6977 Gennady Sharapov 2006-01-18  581  void initial_thread_cb_skas(void (*proc)(void *), void *arg)
abaf6977 Gennady Sharapov 2006-01-18  582  {
ad28e029 Jeff Dike        2006-04-18  583  	jmp_buf here;
abaf6977 Gennady Sharapov 2006-01-18  584  
abaf6977 Gennady Sharapov 2006-01-18  585  	cb_proc = proc;
abaf6977 Gennady Sharapov 2006-01-18  586  	cb_arg = arg;
abaf6977 Gennady Sharapov 2006-01-18  587  	cb_back = &here;

:::::: The code at line 579 was first introduced by commit
:::::: abaf69773d8dda98b917d94c07757f6520da7bec [PATCH] uml: move libc-dependent skas process handling

:::::: TO: Gennady Sharapov <Gen...@in...>
:::::: CC: Linus Torvalds <tor...@g5...>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[uml-devel] [PATCH 3/3] um: Add kerneldoc for userspace_tramp() and start_userspace()

[Thomas M. <th...@m3...>]

Also use correct function name spelling (stub_segv_handler) for better grepping

Signed-off-by: Thomas Meyer <th...@m3...>
---
 arch/um/os-Linux/skas/process.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/um/os-Linux/skas/process.c b/arch/um/os-Linux/skas/process.c
index 03b3c4c..4530867 100644
--- a/arch/um/os-Linux/skas/process.c
+++ b/arch/um/os-Linux/skas/process.c
@@ -108,7 +108,7 @@ static void get_skas_faultinfo(int pid, struct faultinfo *fi)
 	wait_stub_done(pid);
 
 	/*
-	 * faultinfo is prepared by the stub-segv-handler at start of
+	 * faultinfo is prepared by the stub_segv_handler at start of
 	 * the stub stack page. We just have to copy it.
 	 */
 	memcpy(fi, (void *)current_stub_stack(), sizeof(*fi));
@@ -175,6 +175,21 @@ static void handle_trap(int pid, struct uml_pt_regs *regs,
 
 extern char __syscall_stub_start[];
 
+/**
+ * userspace_tramp() - userspace trampoline
+ * @stack:	pointer to the new userspace stack page, can be NULL, if? FIXME:
+ *
+ * The userspace trampoline is used to setup a new userspace process in start_userspace() after it was clone()'ed.
+ * This function will run on a temporary stack page.
+ * It ptrace()'es itself, then
+ * Two pages are mapped into the userspace address space:
+ * - STUB_CODE (with EXEC), which contains the skas stub code
+ * - STUB_DATA (with R/W), which contains a data page that is used to transfer certain data between the UML userspace process and the UML kernel.
+ * Also for the userspace process a SIGSEGV handler is installed to catch pagefaults in the userspace process.
+ * And last the process stops itself to give control to the UML kernel for this userspace process.
+ *
+ * Return: Always zero, otherwise the current userspace process is ended with non null exit() call
+ */
 static int userspace_tramp(void *stack)
 {
 	void *addr;
@@ -236,12 +251,24 @@ static int userspace_tramp(void *stack)
 
 int userspace_pid[NR_CPUS];
 
+/**
+ * start_userspace() - prepare a new userspace process
+ * @stub_stack:	pointer to the stub stack. Can be NULL, if? FIXME:
+ *
+ * Setups a new temporary stack page that is used while userspace_tramp() runs
+ * Clones the kernel process into a new userspace process, with FDs only.
+ *
+ * Return: When positive: the process id of the new userspace process,
+ *         when negative: an error number.
+ * FIXME: can PIDs become negative?!
+ */
 int start_userspace(unsigned long stub_stack)
 {
 	void *stack;
 	unsigned long sp;
 	int pid, status, n, flags, err;
 
+	/* setup a temporary stack page */
 	stack = mmap(NULL, UM_KERN_PAGE_SIZE,
 		     PROT_READ | PROT_WRITE | PROT_EXEC,
 		     MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
@@ -252,10 +279,12 @@ int start_userspace(unsigned long stub_stack)
 		return err;
 	}
 
+	/* set stack pointer to the end of the stack page, so it can grow downwards */
 	sp = (unsigned long) stack + UM_KERN_PAGE_SIZE - sizeof(void *);
 
 	flags = CLONE_FILES | SIGCHLD;
 
+	/* clone into new userspace process */
 	pid = clone(userspace_tramp, (void *) sp, flags, (void *) stub_stack);
 	if (pid < 0) {
 		err = -errno;
-- 
2.9.3

[uml-devel] [PATCH 2/3] um: Add kerneldoc for segv_handler

[Thomas M. <th...@m3...>]

Signed-off-by: Thomas Meyer <th...@m3...>
---
 arch/um/kernel/trap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c
index 5915887..4e6fcb3 100644
--- a/arch/um/kernel/trap.c
+++ b/arch/um/kernel/trap.c
@@ -183,6 +183,16 @@ void fatal_sigsegv(void)
 	os_dump_core();
 }
 
+/**
+ * segv_handler() - the SIGSEGV handler
+ * @sig:	the signal number
+ * @unused_si:	the signal info struct; unused in this handler
+ * @regs:	the ptrace register information
+ *
+ * The handler first extracts the faultinfo from the UML ptrace regs struct.
+ * If the userfault did not happen in an UML userspace process, bad_segv is called.
+ * Otherwise the signal did happen in a cloned userspace process, handle it.
+ */
 void segv_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs)
 {
 	struct faultinfo * fi = UPT_FAULTINFO(regs);
-- 
2.9.3

[uml-devel] [PATCH 1/3] um: stub-data.h: remove superfluous include

[Thomas M. <th...@m3...>]

Signed-off-by: Thomas Meyer <th...@m3...>
---
 arch/um/include/shared/skas/stub-data.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/um/include/shared/skas/stub-data.h b/arch/um/include/shared/skas/stub-data.h
index a9deece..13f404e 100644
--- a/arch/um/include/shared/skas/stub-data.h
+++ b/arch/um/include/shared/skas/stub-data.h
@@ -8,8 +8,6 @@
 #ifndef __STUB_DATA_H
 #define __STUB_DATA_H
 
-#include <time.h>
-
 struct stub_data {
 	unsigned long offset;
 	int fd;
-- 
2.9.3

[uml-devel] [PATCH] um: userspace - be more verbose in ptrace set regs error

[Thomas M. <th...@m3...>]

When ptrace fails to set GP/FP regs for the target process,
log the error before crashing the UML kernel.

Signed-off-by: Thomas Meyer <th...@m3...>
---
 arch/um/os-Linux/skas/process.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/um/os-Linux/skas/process.c b/arch/um/os-Linux/skas/process.c
index 4530867..819d686 100644
--- a/arch/um/os-Linux/skas/process.c
+++ b/arch/um/os-Linux/skas/process.c
@@ -352,11 +352,17 @@ void userspace(struct uml_pt_regs *regs)
 		 * fail.  In this case, there is nothing to do but
 		 * just kill the process.
 		 */
-		if (ptrace(PTRACE_SETREGS, pid, 0, regs->gp))
+		if (ptrace(PTRACE_SETREGS, pid, 0, regs->gp)) {
+			printk(UM_KERN_ERR "userspace - ptrace set regs "
+			       "failed, errno = %d\n", errno);
 			fatal_sigsegv();
+		}
 
-		if (put_fp_registers(pid, regs->fp))
+		if (put_fp_registers(pid, regs->fp)) {
+			printk(UM_KERN_ERR "userspace - ptrace set fp regs "
+			       "failed, errno = %d\n", errno);
 			fatal_sigsegv();
+		}
 
 		/* Now we set local_using_sysemu to be used for one loop */
 		local_using_sysemu = get_using_sysemu();
-- 
2.9.4

Re: [uml-devel] [PATCH] um: userspace - be more verbose in ptrace set regs error

[Richard W. <ri...@no...>]

Thomas,

Am 06.07.2017 um 00:06 schrieb Thomas Meyer:
>> This patch is also malformed. :-(
> 
> I wonder why evolution is messing up my emails...
> 
> will check and resend...

Just use git send-email :)

Thanks,
//richard

Re: [uml-devel] [PATCH] um: userspace - be more verbose in ptrace set regs error

[Thomas M. <th...@m3...>]

Am Mittwoch, den 05.07.2017, 23:34 +0200 schrieb Richard Weinberger:
> Thomas,
> 
> On Wed, May 24, 2017 at 12:45 AM, Thomas Meyer <th...@m3...>
> wrote:
> > When ptrace fails to set GP/FP regs for the target process,
> > log the error before crashing the UML kernel.
> > 
> > Signed-off-by: Thomas Meyer <th...@m3...>
> > ---
> >  arch/um/os-Linux/skas/process.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/um/os-Linux/skas/process.c b/arch/um/os-
> > Linux/skas/process.c
> > index 4530867..819d686 100644
> > --- a/arch/um/os-Linux/skas/process.c
> > +++ b/arch/um/os-Linux/skas/process.c
> > @@ -352,11 +352,17 @@ void userspace(struct uml_pt_regs *regs)
> >                  * fail.  In this case, there is nothing to do but
> >                  * just kill the process.
> >                  */
> > -               if (ptrace(PTRACE_SETREGS, pid, 0, regs->gp))
> > +               if (ptrace(PTRACE_SETREGS, pid, 0, regs->gp)) {
> > +                       printk(UM_KERN_ERR "userspace - ptrace set
> > regs "
> > +                              "failed, errno = %d\n", errno);
> >                         fatal_sigsegv();
> > +               }
> > 
> > -               if (put_fp_registers(pid, regs->fp))
> > +               if (put_fp_registers(pid, regs->fp)) {
> > +                       printk(UM_KERN_ERR "userspace - ptrace set
> > fp regs "
> > +                              "failed, errno = %d\n", errno);
> >                         fatal_sigsegv();
> > +               }
> > 
> >                 /* Now we set local_using_sysemu to be used for one
> > loop */
> >                 local_using_sysemu = get_using_sysemu();
> > 
> > -----------------------------------------------------------------
> > -------------
> > Check out the vibrant tech community on one of the world's most
> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> > _______________________________________________
> > User-mode-linux-devel mailing list
> > Use...@li...
> > https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
> 
> This patch is also malformed. :-(

I wonder why evolution is messing up my emails...

will check and resend...

sorry and kind regards
thomas

> 

Re: [uml-devel] [PATCH] um: userspace - be more verbose in ptrace set regs error

[Richard W. <ric...@gm...>]

Thomas,

On Wed, May 24, 2017 at 12:45 AM, Thomas Meyer <th...@m3...> wrote:
> When ptrace fails to set GP/FP regs for the target process,
> log the error before crashing the UML kernel.
>
> Signed-off-by: Thomas Meyer <th...@m3...>
> ---
>  arch/um/os-Linux/skas/process.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/um/os-Linux/skas/process.c b/arch/um/os-Linux/skas/process.c
> index 4530867..819d686 100644
> --- a/arch/um/os-Linux/skas/process.c
> +++ b/arch/um/os-Linux/skas/process.c
> @@ -352,11 +352,17 @@ void userspace(struct uml_pt_regs *regs)
>                  * fail.  In this case, there is nothing to do but
>                  * just kill the process.
>                  */
> -               if (ptrace(PTRACE_SETREGS, pid, 0, regs->gp))
> +               if (ptrace(PTRACE_SETREGS, pid, 0, regs->gp)) {
> +                       printk(UM_KERN_ERR "userspace - ptrace set regs "
> +                              "failed, errno = %d\n", errno);
>                         fatal_sigsegv();
> +               }
>
> -               if (put_fp_registers(pid, regs->fp))
> +               if (put_fp_registers(pid, regs->fp)) {
> +                       printk(UM_KERN_ERR "userspace - ptrace set fp regs "
> +                              "failed, errno = %d\n", errno);
>                         fatal_sigsegv();
> +               }
>
>                 /* Now we set local_using_sysemu to be used for one loop */
>                 local_using_sysemu = get_using_sysemu();
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> User-mode-linux-devel mailing list
> Use...@li...
> https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

This patch is also malformed. :-(

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH 1/3] um: stub-data.h: remove superfluous include

[Richard W. <ric...@gm...>]

Thomas,

On Sun, May 14, 2017 at 5:03 PM, Thomas Meyer <th...@m3...> wrote:
> Signed-off-by: Thomas Meyer <th...@m3...>
> ---
>  arch/um/include/shared/skas/stub-data.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/um/include/shared/skas/stub-data.h
> b/arch/um/include/shared/skas/stub-data.h
> index a9deece..13f404e 100644
> --- a/arch/um/include/shared/skas/stub-data.h
> +++ b/arch/um/include/shared/skas/stub-data.h
> @@ -8,8 +8,6 @@
>  #ifndef __STUB_DATA_H
>  #define __STUB_DATA_H
>
> -#include <time.h>
> -
>  struct stub_data {
>   unsigned long offset;
>   int fd;
>

All three patches are malformed and don't apply. :-(

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH v3 0/6] um: Output messages to stderr and support quiet option

[Richard W. <ric...@gm...>]

Masami,

On Wed, May 17, 2017 at 7:13 PM, Masami Hiramatsu <mhi...@ke...> wrote:
> Hello,
>
> Here is version 3 of um-quiet series. In this version
> I just fixed some printf format issues.
>
> V2 is here.
>
>  https://lkml.org/lkml/2017/5/8/35
>
> This series fixes some boot time printf output to stderr
> by adding os_info() and os_warn(). The information-level
> messages via os_info() are suppressed when "quiet" kernel
> option is specified.
> Also the last one allows user to pass "console=" option
> to kernel.
>
> Note that the output of --help and --version are still
> sent to stdout since they are intentionally shown by
> the user.
>
> Changes from v2:
>   - Cast rlim_min/max to unsigned long long explicitly
>     for avoiding printf-format warning.
>   - Fix printf format in physmem.c so that it matches
>     the type of arguments.
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (6):
>       um: Use printk instead of printf in make_uml_dir
>       um: Add os_info() for pre-boot information messages
>       um: Use os_info for the messages on normal path
>       um: Add os_warn() for pre-boot warning/error messages
>       um: Use os_warn to print out pre-boot warning/error messages
>       um: console: Ignore console= option
>
>

Series applied. Thanks!

-- 
Thanks,
//richard

Re: [uml-devel] [PATCH v2] um: Avoid longjmp/setjmp symbol clashes with libpthread.a

[Richard W. <ri...@no...>]

Florian,

Am 29.06.2017 um 00:40 schrieb Florian Fainelli:
>> Hehe, yes.
>> This patch is good, I like it. :)
>> It will part of the next pull request.
> 
> Humm okay, did you apply the patch in one of your kernel trees on
> git.kernel.org or somewhere else?

Will happen soon since the merge window is near where I will post
a pull request...

Thanks,
//richard

Re: [uml-devel] [PATCH v2] um: add dummy ioremap and iounmap functions

[Arnd B. <ar...@ar...>]

On Wed, Jun 21, 2017 at 11:31 PM, Logan Gunthorpe <lo...@de...> wrote:
>
>
> On 21/06/17 03:30 PM, Richard Weinberger wrote:
>> Since Arnd stays silent and um is the only remaining odd-ball, let's merge this
>> in v4.13.
>
> Great! Thanks!

Sorry for missing most of the discussion earlier. The last proposed version
(returning NULL to always fail) seems fine to me, too.

       Arnd

Re: [uml-devel] [PATCH v2] um: add dummy ioremap and iounmap functions

[Richard W. <ri...@no...>]

Logan,

Am 21.06.2017 um 23:25 schrieb Logan Gunthorpe:
> Poke.
> 
> On 08/06/17 01:17 PM, Richard Weinberger wrote:
>> Am 08.06.2017 um 20:53 schrieb Logan Gunthorpe:
>> IMHO an ifdef in scatterlist code does not hurt.
>> It is equally ugly than mocking ioremap for UML.
> 
> I disagree. Having ifdefs scattered around all call sites of a function
> is *much* worse than having an extra mock function tucked away in a
> header file somewhere.
> 
> It's a very common pattern in the kernel for providers of functions that
> depend on a configuration option to provide mock functions when the
> configuration option is not selected. This prevents needing every caller
> of said function to put #ifdefs around the call. For a few examples:
> 
> include/linux/blkdev.h:1952
> include/linux/dax.h:24
> include/linux/pci.h:1329
> 
> And, frankly, it's _exactly_ what Linus Torvalds himself was arguing
> against in the link I sent up-thread [1].
> 
>> So, I'm puzzled.
>> Arnd, what do you think?
>> Shall !HAS_IOMEM archs just mock these functions?
> 
> So, once again, 'um' is now the only architecture that has this problem
> since tile and s390 accepted my patches (and without a fuss too). So can
> you please consider merging this patch or proposing something that will
> also fix the problem?

Since Arnd stays silent and um is the only remaining odd-ball, let's merge this
in v4.13.

Thanks,
//richard

Re: [uml-devel] um: PTRACE_SETREGSET failure with XSTATE on Kabylake CPU

[Richard W. <ri...@no...>]

Thomas, Natale,

Am 21.06.2017 um 11:44 schrieb Thomas Meyer:
>> It is funny to see that this problem was firstly reported here [1] in
>> February 2017 without being considered until someone else bought a
>> new
>> laptop :)

sometimes mails/issues get lost...

> Yes, I like my new laptop :-)
> 
>> Anyway, thank you for digging into this; my temporary workaround at
>> the time was to use always the *_i387_registers functions.
> 
> Oops, there is the complete thread with the same problem. But sorry I
> don't follow uml-user, just uml-devel :-(

We really should merge these lists. ;-\

> As a quick fix you can try this:
> 
> diff --git a/arch/x86/um/os-Linux/registers.c b/arch/x86/um/os-Linux/registers.c
> index 00f54a91bb4b..6eac8220ab29 100644
> --- a/arch/x86/um/os-Linux/registers.c
> +++ b/arch/x86/um/os-Linux/registers.c
> @@ -30,7 +30,7 @@ int save_fp_registers(int pid, unsigned long *fp_regs)
>  
>  	if (have_xstate_support) {
>  		iov.iov_base = fp_regs;
> -		iov.iov_len = sizeof(struct _xstate);
> +		iov.iov_len = HOST_FP_SIZE * sizeof(unsigned long);
>  		if (ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov) < 0)
>  			return -errno;
>  		return 0;
> @@ -49,10 +49,9 @@ int restore_i387_registers(int pid, unsigned long *fp_regs)
>  int restore_fp_registers(int pid, unsigned long *fp_regs)
>  {
>  	struct iovec iov;
> -
>  	if (have_xstate_support) {
>  		iov.iov_base = fp_regs;
> -		iov.iov_len = sizeof(struct _xstate);
> +		iov.iov_len = HOST_FP_SIZE * sizeof(unsigned long);
>  		if (ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov) < 0)
>  			return -errno;
>  		return 0;
> @@ -126,7 +125,7 @@ void arch_init_registers(int pid)
>  	struct iovec iov;
>  
>  	iov.iov_base = &fp_regs;
> -	iov.iov_len = sizeof(struct _xstate);
> +	iov.iov_len = HOST_FP_SIZE * sizeof(unsigned long);
>  	if (ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov) == 0)
>  		have_xstate_support = 1;
>  }
> diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c
> index cb3c22370cf5..9dccbbbf2fd1 100644
> --- a/arch/x86/um/user-offsets.c
> +++ b/arch/x86/um/user-offsets.c
> @@ -50,7 +50,7 @@ void foo(void)
>  	DEFINE(HOST_GS, GS);
>  	DEFINE(HOST_ORIG_AX, ORIG_EAX);
>  #else
> -	DEFINE(HOST_FP_SIZE, sizeof(struct _xstate) / sizeof(unsigned long));
> +	DEFINE_LONGS(HOST_FP_SIZE, 2688);
>  	DEFINE_LONGS(HOST_BX, RBX);
>  	DEFINE_LONGS(HOST_CX, RCX);
>  	DEFINE_LONGS(HOST_DI, RDI);
> 
> 
> a better fix would be to make the fp regs in struct uml_pt_regs dynamic
> depending on the current kernel idea of the xsave area size.

And we should consider a mechanism to save/restore the full regset only when needed.
Otherwise the context switch is even more slower.

Thanks,
//richard

Re: [uml-devel] um: PTRACE_SETREGSET failure with XSTATE on Kabylake CPU

[Thomas M. <th...@m3...>]

Am Mittwoch, den 21.06.2017, 10:39 +0200 schrieb Natale Patriciello:
> 2017-06-20 22:13 GMT+02:00 Thomas Meyer <th...@m3...>:
> > 
> > > Am 20.06.2017 um 21:53 schrieb Yu-cheng Yu <yu-...@in...
> > > >:
> > > 
> > > > On Tue, 2017-06-20 at 20:59 +0200, Richard Weinberger wrote:
> > > > Yu-cheng,
> > > > 
> > > > > Am 20.06.2017 um 20:17 schrieb Richard Weinberger:
> > > > > Yu-cheng,
> > > > > 
> > > > > Am 20.06.2017 um 20:04 schrieb Yu-cheng Yu:
> > > > > > > > So to summarize:
> > > > > > > > 
> > > > > > > > - PTRACE_GETREGSET with NT_X86_XSTATE gets 832 and
> > > > > > > > return 832, with no
> > > > > > > > error.
> > > > > > > > 
> > > > > > > > - PTRACE_SETREGSET get 832 (sizeof struct _xstate) but
> > > > > > > > wants at least
> > > > > > > > 1088, otherwise it will fail with -EFAULT (why not
> > > > > > > > -EINVAL?)
> > > > > > > > 
> > > > > > > > Ideas?
> 
> [cut text and CC'ed persons]
> 
> It is funny to see that this problem was firstly reported here [1] in
> February 2017 without being considered until someone else bought a
> new
> laptop :)
> 
Yes, I like my new laptop :-)

> Anyway, thank you for digging into this; my temporary workaround at
> the time was to use always the *_i387_registers functions.

Oops, there is the complete thread with the same problem. But sorry I
don't follow uml-user, just uml-devel :-(

As a quick fix you can try this:

diff --git a/arch/x86/um/os-Linux/registers.c b/arch/x86/um/os-Linux/registers.c
index 00f54a91bb4b..6eac8220ab29 100644
--- a/arch/x86/um/os-Linux/registers.c
+++ b/arch/x86/um/os-Linux/registers.c
@@ -30,7 +30,7 @@ int save_fp_registers(int pid, unsigned long *fp_regs)
 
 	if (have_xstate_support) {
 		iov.iov_base = fp_regs;
-		iov.iov_len = sizeof(struct _xstate);
+		iov.iov_len = HOST_FP_SIZE * sizeof(unsigned long);
 		if (ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov) < 0)
 			return -errno;
 		return 0;
@@ -49,10 +49,9 @@ int restore_i387_registers(int pid, unsigned long *fp_regs)
 int restore_fp_registers(int pid, unsigned long *fp_regs)
 {
 	struct iovec iov;
-
 	if (have_xstate_support) {
 		iov.iov_base = fp_regs;
-		iov.iov_len = sizeof(struct _xstate);
+		iov.iov_len = HOST_FP_SIZE * sizeof(unsigned long);
 		if (ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov) < 0)
 			return -errno;
 		return 0;
@@ -126,7 +125,7 @@ void arch_init_registers(int pid)
 	struct iovec iov;
 
 	iov.iov_base = &fp_regs;
-	iov.iov_len = sizeof(struct _xstate);
+	iov.iov_len = HOST_FP_SIZE * sizeof(unsigned long);
 	if (ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov) == 0)
 		have_xstate_support = 1;
 }
diff --git a/arch/x86/um/user-offsets.c b/arch/x86/um/user-offsets.c
index cb3c22370cf5..9dccbbbf2fd1 100644
--- a/arch/x86/um/user-offsets.c
+++ b/arch/x86/um/user-offsets.c
@@ -50,7 +50,7 @@ void foo(void)
 	DEFINE(HOST_GS, GS);
 	DEFINE(HOST_ORIG_AX, ORIG_EAX);
 #else
-	DEFINE(HOST_FP_SIZE, sizeof(struct _xstate) / sizeof(unsigned long));
+	DEFINE_LONGS(HOST_FP_SIZE, 2688);
 	DEFINE_LONGS(HOST_BX, RBX);
 	DEFINE_LONGS(HOST_CX, RCX);
 	DEFINE_LONGS(HOST_DI, RDI);


a better fix would be to make the fp regs in struct uml_pt_regs dynamic
depending on the current kernel idea of the xsave area size.

> 
> HTH,
> Nat
> 
> [1] https://sourceforge.net/p/user-mode-linux/mailman/message/3566337
> 4/

Re: [uml-devel] um: PTRACE_SETREGSET failure with XSTATE on Kabylake CPU

[Natale P. <nat...@gm...>]

2017-06-20 22:13 GMT+02:00 Thomas Meyer <th...@m3...>:
>
>> Am 20.06.2017 um 21:53 schrieb Yu-cheng Yu <yu-...@in...>:
>>
>>> On Tue, 2017-06-20 at 20:59 +0200, Richard Weinberger wrote:
>>> Yu-cheng,
>>>
>>>> Am 20.06.2017 um 20:17 schrieb Richard Weinberger:
>>>> Yu-cheng,
>>>>
>>>> Am 20.06.2017 um 20:04 schrieb Yu-cheng Yu:
>>>>>>> So to summarize:
>>>>>>>
>>>>>>> - PTRACE_GETREGSET with NT_X86_XSTATE gets 832 and return 832, with no
>>>>>>> error.
>>>>>>>
>>>>>>> - PTRACE_SETREGSET get 832 (sizeof struct _xstate) but wants at least
>>>>>>> 1088, otherwise it will fail with -EFAULT (why not -EINVAL?)
>>>>>>>
>>>>>>> Ideas?

[cut text and CC'ed persons]

It is funny to see that this problem was firstly reported here [1] in
February 2017 without being considered until someone else bought a new
laptop :)

Anyway, thank you for digging into this; my temporary workaround at
the time was to use always the *_i387_registers functions.

HTH,
Nat

[1] https://sourceforge.net/p/user-mode-linux/mailman/message/35663374/

Re: [uml-devel] um: PTRACE_SETREGSET failure with XSTATE on Kabylake CPU

[Thomas M. <th...@m3...>]

> Am 20.06.2017 um 21:53 schrieb Yu-cheng Yu <yu-...@in...>:
> 
>> On Tue, 2017-06-20 at 20:59 +0200, Richard Weinberger wrote:
>> Yu-cheng,
>> 
>>> Am 20.06.2017 um 20:17 schrieb Richard Weinberger:
>>> Yu-cheng,
>>> 
>>> Am 20.06.2017 um 20:04 schrieb Yu-cheng Yu:
>>>>>> So to summarize:
>>>>>> 
>>>>>> - PTRACE_GETREGSET with NT_X86_XSTATE gets 832 and return 832, with no
>>>>>> error.
>>>>>> 
>>>>>> - PTRACE_SETREGSET get 832 (sizeof struct _xstate) but wants at least
>>>>>> 1088, otherwise it will fail with -EFAULT (why not -EINVAL?)
>>>>>> 
>>>>>> Ideas?
>>>> 
>>>> We considered allowing a partial XSAVE buffer for PTRACE_SETREGSET, but
>>>> it was that the XSAVE instruction requires a full-size buffer led to
>>>> this choice.  Using a smaller buffer for XSAVE causes a fault.
>>> 
>>> So, this code is not supposed to work?
>>> 
>>> iov.iov_base = fp_regs;
>>> iov.iov_len = sizeof(struct _xstate);
>>> ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov);
>>> ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov);
>>> 
>>> This is what UML does and on Thomas's new Laptop PTRACE_SETREGSET is failing.
>> 
>> Hmm, I think we need to do what gdb does, it uses a buffer of size X86_XSTATE_MAX_SIZE.
> 
> Linux kernel determines XSAVE buffer size from CPUID:
> http://elixir.free-electrons.com/linux/latest/source/arch/x86/kernel/fpu/xstate.c#L626

Hi,

Is there a user space API to get this value?

> 
> GDB has a fixed X86_XSTATE_MAX_SIZE of 2688.  That can become an issue.
> 
> Yu-cheng
> 
> 

Re: [uml-devel] um: PTRACE_SETREGSET failure with XSTATE on Kabylake CPU

[Richard W. <ri...@no...>]

Yu-cheng,

Am 20.06.2017 um 20:17 schrieb Richard Weinberger:
> Yu-cheng,
> 
> Am 20.06.2017 um 20:04 schrieb Yu-cheng Yu:
>>>> So to summarize:
>>>>
>>>> - PTRACE_GETREGSET with NT_X86_XSTATE gets 832 and return 832, with no
>>>> error.
>>>>
>>>> - PTRACE_SETREGSET get 832 (sizeof struct _xstate) but wants at least
>>>> 1088, otherwise it will fail with -EFAULT (why not -EINVAL?)
>>>>
>>>> Ideas?
>>
>> We considered allowing a partial XSAVE buffer for PTRACE_SETREGSET, but
>> it was that the XSAVE instruction requires a full-size buffer led to
>> this choice.  Using a smaller buffer for XSAVE causes a fault.
> 
> So, this code is not supposed to work?
> 
> iov.iov_base = fp_regs;
> iov.iov_len = sizeof(struct _xstate);
> ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov);
> ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov);
> 
> This is what UML does and on Thomas's new Laptop PTRACE_SETREGSET is failing.

Hmm, I think we need to do what gdb does, it uses a buffer of size X86_XSTATE_MAX_SIZE.

Thanks,
//richard