[PATCH 00/22] Implement the mmap system call for FreeBSD.

[PATCH 00/22] Implement the mmap system call for FreeBSD.

[Karim Taha]

Upstream the implementation of the following mmap system calls, from the
qemu-bsd-user fork:
   mmap(2), munmap(2), mprotect(2), msync(2), mlock(2), munlock(2), mlockall(2)
   munlockall(2), madvise(2), minherit(2), mincore(2),
   shm_open(2),shm_open2(2), shm_rename2(2), shm_unlink(2), shmget(2), shmctl(2), shmat(2),
   shmdt(2)

Karim Taha (1):
  Add bsd-mem.c to meson.build

Kyle Evans (2):
  Implement shm_open2(2) system call
  Implement shm_rename(2) system call

Stacey Son (19):
  Implement struct target_ipc_perm
  Implement struct target_shmid_ds
  Declarations for ipc_perm and shmid_ds conversion functions
  Introduce freebsd/os-misc.h to the source tree
  Implement target_set_brk function in bsd-mem.c instead of os-syscall.c
  Implement ipc_perm conversion between host and target.
  Implement shmid_ds conversion between host and target.
  Introduce bsd-mem.h to the source tree
  Implement mmap(2) and munmap(2)
  Implement mprotect(2)
  Implement msync(2)
  Implement mlock(2), munlock(2), mlockall(2), munlockall(2),
    madvise(2), minherit(2)
  Implement mincore(2)
  Implement do_obreak function
  Implement shm_open(2)
  Implement shm_unlink(2) and shmget(2)
  Implement shmctl(2)
  Implement shmat(2) and shmdt(2)
  Add stubs for vadvise(), sbrk() and sstk()

 bsd-user/bsd-mem.c            | 125 ++++++++++
 bsd-user/bsd-mem.h            | 431 ++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-misc.h    | 104 ++++++++
 bsd-user/freebsd/os-syscall.c | 112 ++++++++-
 bsd-user/meson.build          |   1 +
 bsd-user/qemu-bsd.h           |  45 ++++
 bsd-user/syscall_defs.h       |  37 +++
 7 files changed, 851 insertions(+), 4 deletions(-)
 create mode 100644 bsd-user/bsd-mem.c
 create mode 100644 bsd-user/bsd-mem.h
 create mode 100644 bsd-user/freebsd/os-misc.h
 create mode 100644 bsd-user/qemu-bsd.h

-- 
2.40.0

[PATCH 01/22] Implement struct target_ipc_perm

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/syscall_defs.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
index e4825f2662..39a9bc8ed7 100644
--- a/bsd-user/syscall_defs.h
+++ b/bsd-user/syscall_defs.h
@@ -55,6 +55,23 @@ struct target_iovec {
     abi_long iov_len;   /* Number of bytes */
 };
 
+/*
+ * sys/ipc.h
+ */
+struct target_ipc_perm {
+    uint32_t    cuid;       /* creator user id */
+    uint32_t    cgid;       /* creator group id */
+    uint32_t    uid;        /* user id */
+    uint32_t    gid;        /* group id */
+    uint16_t    mode;       /* r/w permission */
+    uint16_t    seq;        /* sequence # */
+    abi_long    key;        /* user specified msg/sem/shm key */
+};
+
+#define TARGET_IPC_RMID 0   /* remove identifier */
+#define TARGET_IPC_SET  1   /* set options */
+#define TARGET_IPC_STAT 2   /* get options */
+
 /*
  *  sys/mman.h
  */
-- 
2.40.0

Re: [PATCH 01/22] Implement struct target_ipc_perm

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/syscall_defs.h | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 01/22] Implement struct target_ipc_perm

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

One general thing about all the patches in this series. The first line in
the
commit message should start with 'bsd-user: ' which I'll only say once.
You can fix it in v2 easily enough.

On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/syscall_defs.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>


> diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
> index e4825f2662..39a9bc8ed7 100644
> --- a/bsd-user/syscall_defs.h
> +++ b/bsd-user/syscall_defs.h
> @@ -55,6 +55,23 @@ struct target_iovec {
>      abi_long iov_len;   /* Number of bytes */
>  };
>
> +/*
> + * sys/ipc.h
> + */
> +struct target_ipc_perm {
> +    uint32_t    cuid;       /* creator user id */
> +    uint32_t    cgid;       /* creator group id */
> +    uint32_t    uid;        /* user id */
> +    uint32_t    gid;        /* group id */
> +    uint16_t    mode;       /* r/w permission */
> +    uint16_t    seq;        /* sequence # */
> +    abi_long    key;        /* user specified msg/sem/shm key */
> +};
> +
> +#define TARGET_IPC_RMID 0   /* remove identifier */
> +#define TARGET_IPC_SET  1   /* set options */
> +#define TARGET_IPC_STAT 2   /* get options */
> +
>  /*
>   *  sys/mman.h
>   */
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 2269 bytes --]

[PATCH 02/22] Implement struct target_shmid_ds

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/syscall_defs.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
index 39a9bc8ed7..074df7bdd6 100644
--- a/bsd-user/syscall_defs.h
+++ b/bsd-user/syscall_defs.h
@@ -72,6 +72,26 @@ struct target_ipc_perm {
 #define TARGET_IPC_SET  1   /* set options */
 #define TARGET_IPC_STAT 2   /* get options */
 
+/*
+ * sys/shm.h
+ */
+struct target_shmid_ds {
+    struct  target_ipc_perm shm_perm; /* peration permission structure */
+    abi_ulong   shm_segsz;  /* size of segment in bytes */
+    int32_t     shm_lpid;   /* process ID of last shared memory op */
+    int32_t     shm_cpid;   /* process ID of creator */
+    int32_t     shm_nattch; /* number of current attaches */
+    target_time_t shm_atime;  /* time of last shmat() */
+    target_time_t shm_dtime;  /* time of last shmdt() */
+    target_time_t shm_ctime;  /* time of last change by shmctl() */
+};
+
+#define N_BSD_SHM_REGIONS   32
+struct bsd_shm_regions {
+    abi_long start;
+    abi_long size;
+};
+
 /*
  *  sys/mman.h
  */
-- 
2.40.0

Re: [PATCH 02/22] Implement struct target_shmid_ds

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/syscall_defs.h | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 02/22] Implement struct target_shmid_ds

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]

On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/syscall_defs.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>


> diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
> index 39a9bc8ed7..074df7bdd6 100644
> --- a/bsd-user/syscall_defs.h
> +++ b/bsd-user/syscall_defs.h
> @@ -72,6 +72,26 @@ struct target_ipc_perm {
>  #define TARGET_IPC_SET  1   /* set options */
>  #define TARGET_IPC_STAT 2   /* get options */
>
> +/*
> + * sys/shm.h
> + */
> +struct target_shmid_ds {
> +    struct  target_ipc_perm shm_perm; /* peration permission structure */
> +    abi_ulong   shm_segsz;  /* size of segment in bytes */
> +    int32_t     shm_lpid;   /* process ID of last shared memory op */
> +    int32_t     shm_cpid;   /* process ID of creator */
> +    int32_t     shm_nattch; /* number of current attaches */
> +    target_time_t shm_atime;  /* time of last shmat() */
> +    target_time_t shm_dtime;  /* time of last shmdt() */
> +    target_time_t shm_ctime;  /* time of last change by shmctl() */
> +};
> +
> +#define N_BSD_SHM_REGIONS   32
> +struct bsd_shm_regions {
> +    abi_long start;
> +    abi_long size;
> +};
> +
>  /*
>   *  sys/mman.h
>   */
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 2220 bytes --]

[PATCH 03/22] Declarations for ipc_perm and shmid_ds conversion functions

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/qemu-bsd.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 bsd-user/qemu-bsd.h

diff --git a/bsd-user/qemu-bsd.h b/bsd-user/qemu-bsd.h
new file mode 100644
index 0000000000..f98942ea91
--- /dev/null
+++ b/bsd-user/qemu-bsd.h
@@ -0,0 +1,45 @@
+/*
+ *  BSD conversion extern declarations
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_BSD_H
+#define QEMU_BSD_H
+
+#include <sys/types.h>
+#include <sys/ipc.h>
+#include <sys/msg.h>
+#include <sys/resource.h>
+#include <sys/sem.h>
+#include <sys/shm.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <sys/uuid.h>
+#include <sys/wait.h>
+#include <netinet/in.h>
+
+/* bsd-mem.c */
+abi_long target_to_host_ipc_perm(struct ipc_perm *host_ip,
+        abi_ulong target_addr);
+abi_long host_to_target_ipc_perm(abi_ulong target_addr,
+        struct ipc_perm *host_ip);
+abi_long target_to_host_shmid_ds(struct shmid_ds *host_sd,
+        abi_ulong target_addr);
+abi_long host_to_target_shmid_ds(abi_ulong target_addr,
+        struct shmid_ds *host_sd);
+
+#endif /* QEMU_BSD_H */
-- 
2.40.0

Re: [PATCH 03/22] Declarations for ipc_perm and shmid_ds conversion functions

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/qemu-bsd.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
>   create mode 100644 bsd-user/qemu-bsd.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 03/22] Declarations for ipc_perm and shmid_ds conversion functions

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2259 bytes --]

On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/qemu-bsd.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 bsd-user/qemu-bsd.h
>

Reviewed-by: Warner Losh <imp@bsdimp.com>


> diff --git a/bsd-user/qemu-bsd.h b/bsd-user/qemu-bsd.h
> new file mode 100644
> index 0000000000..f98942ea91
> --- /dev/null
> +++ b/bsd-user/qemu-bsd.h
> @@ -0,0 +1,45 @@
> +/*
> + *  BSD conversion extern declarations
> + *
> + *  Copyright (c) 2013 Stacey D. Son
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef QEMU_BSD_H
> +#define QEMU_BSD_H
> +
> +#include <sys/types.h>
> +#include <sys/ipc.h>
> +#include <sys/msg.h>
> +#include <sys/resource.h>
> +#include <sys/sem.h>
> +#include <sys/shm.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <sys/uuid.h>
> +#include <sys/wait.h>
> +#include <netinet/in.h>
> +
> +/* bsd-mem.c */
> +abi_long target_to_host_ipc_perm(struct ipc_perm *host_ip,
> +        abi_ulong target_addr);
> +abi_long host_to_target_ipc_perm(abi_ulong target_addr,
> +        struct ipc_perm *host_ip);
> +abi_long target_to_host_shmid_ds(struct shmid_ds *host_sd,
> +        abi_ulong target_addr);
> +abi_long host_to_target_shmid_ds(abi_ulong target_addr,
> +        struct shmid_ds *host_sd);
> +
> +#endif /* QEMU_BSD_H */
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 3160 bytes --]

[PATCH 04/22] Introduce freebsd/os-misc.h to the source tree

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

To preserve the copyright notice and help with the 'Author' info for
subsequent changes to the file.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/freebsd/os-misc.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 bsd-user/freebsd/os-misc.h

diff --git a/bsd-user/freebsd/os-misc.h b/bsd-user/freebsd/os-misc.h
new file mode 100644
index 0000000000..8436ccb2f7
--- /dev/null
+++ b/bsd-user/freebsd/os-misc.h
@@ -0,0 +1,28 @@
+/*
+ *  miscellaneous FreeBSD system call shims
+ *
+ *  Copyright (c) 2013-14 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef OS_MISC_H
+#define OS_MISC_H
+
+#include <sys/cpuset.h>
+#include <sys/random.h>
+#include <sched.h>
+
+
+#endif /* OS_MISC_H */
-- 
2.40.0

Re: [PATCH 04/22] Introduce freebsd/os-misc.h to the source tree

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> To preserve the copyright notice and help with the 'Author' info for
> subsequent changes to the file.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/freebsd/os-misc.h | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
>   create mode 100644 bsd-user/freebsd/os-misc.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 04/22] Introduce freebsd/os-misc.h to the source tree

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 546 bytes --]

On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> To preserve the copyright notice and help with the 'Author' info for
> subsequent changes to the file.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/freebsd/os-misc.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 bsd-user/freebsd/os-misc.h
>

 Reviewed-by: Warner Losh <imp@bsdimp.com>

[-- Attachment #2: Type: text/html, Size: 1034 bytes --]

[PATCH 05/22] Implement shm_open2(2) system call

[Karim Taha]

From: Kyle Evans <kevans@FreeBSD.org>

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/freebsd/os-misc.h    | 52 +++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c | 13 +++++++++
 2 files changed, 65 insertions(+)

diff --git a/bsd-user/freebsd/os-misc.h b/bsd-user/freebsd/os-misc.h
index 8436ccb2f7..993e4598f9 100644
--- a/bsd-user/freebsd/os-misc.h
+++ b/bsd-user/freebsd/os-misc.h
@@ -24,5 +24,57 @@
 #include <sys/random.h>
 #include <sched.h>
 
+int shm_open2(const char *path, int flags, mode_t mode, int shmflags,
+    const char *);
+
+#if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
+/* shm_open2(2) */
+static inline abi_long do_freebsd_shm_open2(abi_ulong pathptr, abi_ulong flags,
+    abi_long mode, abi_ulong shmflags, abi_ulong nameptr)
+{
+    int ret;
+    void *uname, *upath;
+
+#ifdef SHM_ANON
+#define SHM_PATH(p) (p) == SHM_ANON ? (p) : path(p)
+    if (pathptr == (uintptr_t)SHM_ANON) {
+        upath = SHM_ANON;
+    } else
+#else
+#define SHM_PATH(p) path(p)
+#endif
+    {
+        upath = lock_user_string(pathptr);
+        if (upath == NULL) {
+            return -TARGET_EFAULT;
+        }
+    }
+
+    uname = NULL;
+    if (nameptr != 0) {
+        uname = lock_user_string(nameptr);
+        if (uname == NULL) {
+            unlock_user(upath, pathptr, 0);
+            return -TARGET_EFAULT;
+        }
+    }
+    ret = get_errno(shm_open2(SHM_PATH(upath),
+                target_to_host_bitmask(flags, fcntl_flags_tbl), mode,
+                target_to_host_bitmask(shmflags, shmflag_flags_tbl), uname));
+
+#ifdef SHM_ANON
+    if (upath != SHM_ANON)
+#endif
+    {
+        unlock_user(upath, pathptr, 0);
+    }
+    if (uname != NULL) {
+        unlock_user(uname, nameptr, 0);
+    }
+    return ret;
+}
+#undef SHM_PATH
+#endif /* __FreeBSD_version >= 1300048 */
+
 
 #endif /* OS_MISC_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 2224a280ea..b4311db578 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -33,9 +33,13 @@
 #include "signal-common.h"
 #include "user/syscall-trace.h"
 
+/* BSD independent syscall shims */
 #include "bsd-file.h"
 #include "bsd-proc.h"
 
+/* *BSD dependent syscall shims */
+#include "os-misc.h"
+
 /* I/O */
 safe_syscall3(int, open, const char *, path, int, flags, mode_t, mode);
 safe_syscall4(int, openat, int, fd, const char *, path, int, flags, mode_t,
@@ -482,6 +486,15 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_undelete(arg1);
         break;
 
+        /*
+         * Memory management system calls.
+         */
+#if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
+    case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
+        ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
+        break;
+#endif
+
         /*
          * sys{ctl, arch, call}
          */
-- 
2.40.0

Re: [PATCH 05/22] Implement shm_open2(2) system call

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Kyle Evans <kevans@FreeBSD.org>
> 
> Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>   bsd-user/freebsd/os-misc.h    | 52 +++++++++++++++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c | 13 +++++++++
>   2 files changed, 65 insertions(+)
> 
> diff --git a/bsd-user/freebsd/os-misc.h b/bsd-user/freebsd/os-misc.h
> index 8436ccb2f7..993e4598f9 100644
> --- a/bsd-user/freebsd/os-misc.h
> +++ b/bsd-user/freebsd/os-misc.h
> @@ -24,5 +24,57 @@
>   #include <sys/random.h>
>   #include <sched.h>
>   
> +int shm_open2(const char *path, int flags, mode_t mode, int shmflags,
> +    const char *);
> +
> +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
> +/* shm_open2(2) */
> +static inline abi_long do_freebsd_shm_open2(abi_ulong pathptr, abi_ulong flags,
> +    abi_long mode, abi_ulong shmflags, abi_ulong nameptr)
> +{
> +    int ret;
> +    void *uname, *upath;
> +
> +#ifdef SHM_ANON

Why would SHM_ANON not be defined?  You've already restricted the function to freebsd13+ 
(presumably so that shm_open2() is in libc.a).


> +#define SHM_PATH(p) (p) == SHM_ANON ? (p) : path(p)
> +    if (pathptr == (uintptr_t)SHM_ANON) {
> +        upath = SHM_ANON;
> +    } else
> +#else
> +#define SHM_PATH(p) path(p)
> +#endif
> +    {
> +        upath = lock_user_string(pathptr);
> +        if (upath == NULL) {
> +            return -TARGET_EFAULT;
> +        }
> +    }
> +
> +    uname = NULL;
> +    if (nameptr != 0) {
> +        uname = lock_user_string(nameptr);
> +        if (uname == NULL) {
> +            unlock_user(upath, pathptr, 0);
> +            return -TARGET_EFAULT;
> +        }
> +    }
> +    ret = get_errno(shm_open2(SHM_PATH(upath),

There is no need for SHM_PATH because you've already handled the condition while setting 
upath above.



r~

Re: [PATCH 05/22] Implement shm_open2(2) system call

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]

On Sat, Aug 19, 2023 at 9:10 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 8/19/23 02:47, Karim Taha wrote:
> > From: Kyle Evans <kevans@FreeBSD.org>
> >
> > Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
> > Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> > ---
> >   bsd-user/freebsd/os-misc.h    | 52 +++++++++++++++++++++++++++++++++++
> >   bsd-user/freebsd/os-syscall.c | 13 +++++++++
> >   2 files changed, 65 insertions(+)
> >
> > diff --git a/bsd-user/freebsd/os-misc.h b/bsd-user/freebsd/os-misc.h
> > index 8436ccb2f7..993e4598f9 100644
> > --- a/bsd-user/freebsd/os-misc.h
> > +++ b/bsd-user/freebsd/os-misc.h
> > @@ -24,5 +24,57 @@
> >   #include <sys/random.h>
> >   #include <sched.h>
> >
> > +int shm_open2(const char *path, int flags, mode_t mode, int shmflags,
> > +    const char *);
> > +
> > +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
> > +/* shm_open2(2) */
> > +static inline abi_long do_freebsd_shm_open2(abi_ulong pathptr,
> abi_ulong flags,
> > +    abi_long mode, abi_ulong shmflags, abi_ulong nameptr)
> > +{
> > +    int ret;
> > +    void *uname, *upath;
> > +
> > +#ifdef SHM_ANON
>
> Why would SHM_ANON not be defined?  You've already restricted the function
> to freebsd13+
> (presumably so that shm_open2() is in libc.a).
>

Good catch Richard. It's like this in the bsd-user fork, but after talking
it over with Kyle, we're
sure that's due to a cut and past from shm_open when this was implemented.
We should change
it before we submit it upstream absolutely. It's always defined on FreeBSD
and that will make
this code simpler.

Warner


> > +#define SHM_PATH(p) (p) == SHM_ANON ? (p) : path(p)
> > +    if (pathptr == (uintptr_t)SHM_ANON) {
> > +        upath = SHM_ANON;
> > +    } else
> > +#else
> > +#define SHM_PATH(p) path(p)
> > +#endif
> > +    {
> > +        upath = lock_user_string(pathptr);
> > +        if (upath == NULL) {
> > +            return -TARGET_EFAULT;
> > +        }
> > +    }
> > +
> > +    uname = NULL;
> > +    if (nameptr != 0) {
> > +        uname = lock_user_string(nameptr);
> > +        if (uname == NULL) {
> > +            unlock_user(upath, pathptr, 0);
> > +            return -TARGET_EFAULT;
> > +        }
> > +    }
> > +    ret = get_errno(shm_open2(SHM_PATH(upath),
>
> There is no need for SHM_PATH because you've already handled the condition
> while setting
> upath above.
>
>
>
> r~
>

[-- Attachment #2: Type: text/html, Size: 3525 bytes --]

[PATCH 06/22] Implement shm_rename(2) system call

[Karim Taha]

From: Kyle Evans <kevans@FreeBSD.org>

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/freebsd/os-misc.h    | 24 ++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  6 ++++++
 2 files changed, 30 insertions(+)

diff --git a/bsd-user/freebsd/os-misc.h b/bsd-user/freebsd/os-misc.h
index 993e4598f9..3509ef026e 100644
--- a/bsd-user/freebsd/os-misc.h
+++ b/bsd-user/freebsd/os-misc.h
@@ -76,5 +76,29 @@ static inline abi_long do_freebsd_shm_open2(abi_ulong pathptr, abi_ulong flags,
 #undef SHM_PATH
 #endif /* __FreeBSD_version >= 1300048 */
 
+#if defined(__FreeBSD_version) && __FreeBSD_version >= 1300049
+/* shm_rename(2) */
+static inline abi_long do_freebsd_shm_rename(abi_ulong fromptr, abi_ulong toptr,
+        abi_ulong flags)
+{
+    int ret;
+    void *ufrom, *uto;
+
+    ufrom = lock_user_string(fromptr);
+    if (ufrom == NULL) {
+        return -TARGET_EFAULT;
+    }
+    uto = lock_user_string(toptr);
+    if (uto == NULL) {
+        unlock_user(ufrom, fromptr, 0);
+        return -TARGET_EFAULT;
+    }
+    ret = get_errno(shm_rename(ufrom, uto, flags));
+    unlock_user(ufrom, fromptr, 0);
+    unlock_user(uto, toptr, 0);
+
+    return ret;
+}
+#endif /* __FreeBSD_version >= 1300049 */
 
 #endif /* OS_MISC_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index b4311db578..2920370ad2 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -495,6 +495,12 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #endif
 
+#if defined(__FreeBSD_version) && __FreeBSD_version >= 1300049
+    case TARGET_FREEBSD_NR_shm_rename: /* shm_rename(2) */
+        ret = do_freebsd_shm_rename(arg1, arg2, arg3);
+        break;
+#endif
+
         /*
          * sys{ctl, arch, call}
          */
-- 
2.40.0

Re: [PATCH 06/22] Implement shm_rename(2) system call

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]

On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Kyle Evans <kevans@FreeBSD.org>
>
> Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/freebsd/os-misc.h    | 24 ++++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  6 ++++++
>  2 files changed, 30 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

The FreeBSD 13 ifdef below still is needed since FreeBSD 12 is still
supported by the
project until the end of the year.


> diff --git a/bsd-user/freebsd/os-misc.h b/bsd-user/freebsd/os-misc.h
> index 993e4598f9..3509ef026e 100644
> --- a/bsd-user/freebsd/os-misc.h
> +++ b/bsd-user/freebsd/os-misc.h
> @@ -76,5 +76,29 @@ static inline abi_long do_freebsd_shm_open2(abi_ulong
> pathptr, abi_ulong flags,
>  #undef SHM_PATH
>  #endif /* __FreeBSD_version >= 1300048 */
>
> +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1300049
> +/* shm_rename(2) */
> +static inline abi_long do_freebsd_shm_rename(abi_ulong fromptr, abi_ulong
> toptr,
> +        abi_ulong flags)
> +{
> +    int ret;
> +    void *ufrom, *uto;
> +
> +    ufrom = lock_user_string(fromptr);
> +    if (ufrom == NULL) {
> +        return -TARGET_EFAULT;
> +    }
> +    uto = lock_user_string(toptr);
> +    if (uto == NULL) {
> +        unlock_user(ufrom, fromptr, 0);
> +        return -TARGET_EFAULT;
> +    }
> +    ret = get_errno(shm_rename(ufrom, uto, flags));
> +    unlock_user(ufrom, fromptr, 0);
> +    unlock_user(uto, toptr, 0);
> +
> +    return ret;
> +}
> +#endif /* __FreeBSD_version >= 1300049 */
>
>  #endif /* OS_MISC_H */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index b4311db578..2920370ad2 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -495,6 +495,12 @@ static abi_long freebsd_syscall(void *cpu_env, int
> num, abi_long arg1,
>          break;
>  #endif
>
> +#if defined(__FreeBSD_version) && __FreeBSD_version >= 1300049
> +    case TARGET_FREEBSD_NR_shm_rename: /* shm_rename(2) */
> +        ret = do_freebsd_shm_rename(arg1, arg2, arg3);
> +        break;
> +#endif
> +
>          /*
>           * sys{ctl, arch, call}
>           */
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 3192 bytes --]

Re: [PATCH 06/22] Implement shm_rename(2) system call

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Kyle Evans<kevans@FreeBSD.org>
> 
> Signed-off-by: Kyle Evans<kevans@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/freebsd/os-misc.h    | 24 ++++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c |  6 ++++++
>   2 files changed, 30 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 07/22] Add bsd-mem.c to meson.build

[Karim Taha]

---
 bsd-user/bsd-mem.c   | 0
 bsd-user/meson.build | 1 +
 2 files changed, 1 insertion(+)
 create mode 100644 bsd-user/bsd-mem.c

diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/bsd-user/meson.build b/bsd-user/meson.build
index 5243122fc5..6ee68fdfe7 100644
--- a/bsd-user/meson.build
+++ b/bsd-user/meson.build
@@ -7,6 +7,7 @@ bsd_user_ss = ss.source_set()
 common_user_inc += include_directories('include')
 
 bsd_user_ss.add(files(
+  'bsd-mem.c',
   'bsdload.c',
   'elfload.c',
   'main.c',
-- 
2.40.0

Re: [PATCH 07/22] Add bsd-mem.c to meson.build

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> ---
>  bsd-user/bsd-mem.c   | 0
>  bsd-user/meson.build | 1 +
>  2 files changed, 1 insertion(+)
>  create mode 100644 bsd-user/bsd-mem.c
>

Reviewed-by: Warner Losh <imp@bsdimp.com>


> diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> index 5243122fc5..6ee68fdfe7 100644
> --- a/bsd-user/meson.build
> +++ b/bsd-user/meson.build
> @@ -7,6 +7,7 @@ bsd_user_ss = ss.source_set()
>  common_user_inc += include_directories('include')
>
>  bsd_user_ss.add(files(
> +  'bsd-mem.c',
>    'bsdload.c',
>    'elfload.c',
>    'main.c',
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 1396 bytes --]

Re: [PATCH 07/22] Add bsd-mem.c to meson.build

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> ---
>   bsd-user/bsd-mem.c   | 0
>   bsd-user/meson.build | 1 +
>   2 files changed, 1 insertion(+)
>   create mode 100644 bsd-user/bsd-mem.c
> 
> diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> index 5243122fc5..6ee68fdfe7 100644
> --- a/bsd-user/meson.build
> +++ b/bsd-user/meson.build
> @@ -7,6 +7,7 @@ bsd_user_ss = ss.source_set()
>   common_user_inc += include_directories('include')
>   
>   bsd_user_ss.add(files(
> +  'bsd-mem.c',
>     'bsdload.c',
>     'elfload.c',
>     'main.c',

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 08/22] Implement target_set_brk function in bsd-mem.c instead of os-syscall.c

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Co-authored-by: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.c            | 38 +++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  4 ----
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
index e69de29bb2..6c123abf04 100644
--- a/bsd-user/bsd-mem.c
+++ b/bsd-user/bsd-mem.c
@@ -0,0 +1,38 @@
+/*
+ *  memory management system conversion routines
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "qemu/osdep.h"
+
+#include <sys/ipc.h>
+#include <sys/shm.h>
+
+#include "qemu.h"
+#include "qemu-bsd.h"
+
+struct bsd_shm_regions bsd_shm_regions[N_BSD_SHM_REGIONS];
+
+abi_ulong bsd_target_brk;
+abi_ulong bsd_target_original_brk;
+abi_ulong brk_page;
+
+void target_set_brk(abi_ulong new_brk)
+{
+
+    bsd_target_original_brk = bsd_target_brk = HOST_PAGE_ALIGN(new_brk);
+    brk_page = HOST_PAGE_ALIGN(bsd_target_brk);
+}
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 2920370ad2..c0a22eb746 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -59,10 +59,6 @@ safe_syscall3(ssize_t, writev, int, fd, const struct iovec *, iov, int, iovcnt);
 safe_syscall4(ssize_t, pwritev, int, fd, const struct iovec *, iov, int, iovcnt,
     off_t, offset);
 
-void target_set_brk(abi_ulong new_brk)
-{
-}
-
 /*
  * errno conversion.
  */
-- 
2.40.0

Re: [PATCH 08/22] Implement target_set_brk function in bsd-mem.c instead of os-syscall.c

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2918 bytes --]

On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Co-authored-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.c            | 38 +++++++++++++++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  4 ----
>  2 files changed, 38 insertions(+), 4 deletions(-)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

but see below


> diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
> index e69de29bb2..6c123abf04 100644
> --- a/bsd-user/bsd-mem.c
> +++ b/bsd-user/bsd-mem.c
> @@ -0,0 +1,38 @@
> +/*
> + *  memory management system conversion routines
> + *
> + *  Copyright (c) 2013 Stacey D. Son
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +
> +#include <sys/ipc.h>
> +#include <sys/shm.h>
> +
>

I'll let others comment if this is a good thing here or it needs to be
elsewhere.
I can't recall what the project's rules are for system headers on code that
runs on a limited subset of systems. For code that can run anywhere, I
know that the preference is to put this in osdep.h, but in this case I'm
unsure.

Warner


> +#include "qemu.h"
> +#include "qemu-bsd.h"
> +
> +struct bsd_shm_regions bsd_shm_regions[N_BSD_SHM_REGIONS];
> +
> +abi_ulong bsd_target_brk;
> +abi_ulong bsd_target_original_brk;
> +abi_ulong brk_page;
> +
> +void target_set_brk(abi_ulong new_brk)
> +{
> +
> +    bsd_target_original_brk = bsd_target_brk = HOST_PAGE_ALIGN(new_brk);
> +    brk_page = HOST_PAGE_ALIGN(bsd_target_brk);
> +}
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 2920370ad2..c0a22eb746 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -59,10 +59,6 @@ safe_syscall3(ssize_t, writev, int, fd, const struct
> iovec *, iov, int, iovcnt);
>  safe_syscall4(ssize_t, pwritev, int, fd, const struct iovec *, iov, int,
> iovcnt,
>      off_t, offset);
>
> -void target_set_brk(abi_ulong new_brk)
> -{
> -}
> -
>  /*
>   * errno conversion.
>   */
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 4199 bytes --]

Re: [PATCH 08/22] Implement target_set_brk function in bsd-mem.c instead of os-syscall.c

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Stacey Son <sson@FreeBSD.org>
> 
> Co-authored-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.c            | 38 +++++++++++++++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c |  4 ----
>   2 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
> index e69de29bb2..6c123abf04 100644
> --- a/bsd-user/bsd-mem.c
> +++ b/bsd-user/bsd-mem.c
> @@ -0,0 +1,38 @@
> +/*
> + *  memory management system conversion routines
> + *
> + *  Copyright (c) 2013 Stacey D. Son
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +
> +#include <sys/ipc.h>
> +#include <sys/shm.h>
> +
> +#include "qemu.h"
> +#include "qemu-bsd.h"
> +
> +struct bsd_shm_regions bsd_shm_regions[N_BSD_SHM_REGIONS];


This and the ipc/shm includes don't belong with this patch.


> +
> +abi_ulong bsd_target_brk;
> +abi_ulong bsd_target_original_brk;
> +abi_ulong brk_page;
> +
> +void target_set_brk(abi_ulong new_brk)
> +{
> +

Blank line.

> +    bsd_target_original_brk = bsd_target_brk = HOST_PAGE_ALIGN(new_brk);
> +    brk_page = HOST_PAGE_ALIGN(bsd_target_brk);

I encourage you to use TARGET_PAGE_ALIGN instead.
This will match changes made in linux-user during the 8.1 cycle.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 09/22] Implement ipc_perm conversion between host and target.

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
index 6c123abf04..e69250cc0d 100644
--- a/bsd-user/bsd-mem.c
+++ b/bsd-user/bsd-mem.c
@@ -36,3 +36,44 @@ void target_set_brk(abi_ulong new_brk)
     bsd_target_original_brk = bsd_target_brk = HOST_PAGE_ALIGN(new_brk);
     brk_page = HOST_PAGE_ALIGN(bsd_target_brk);
 }
+
+abi_long target_to_host_ipc_perm(struct ipc_perm *host_ip,
+        abi_ulong target_addr)
+{
+    struct target_ipc_perm *target_ip;
+
+    if (!lock_user_struct(VERIFY_READ, target_ip, target_addr, 1)) {
+        return -TARGET_EFAULT;
+    }
+    __get_user(host_ip->cuid, &target_ip->cuid);
+    __get_user(host_ip->cgid, &target_ip->cgid);
+    __get_user(host_ip->uid, &target_ip->uid);
+    __get_user(host_ip->gid, &target_ip->gid);
+    __get_user(host_ip->mode, &target_ip->mode);
+    __get_user(host_ip->seq, &target_ip->seq);
+    __get_user(host_ip->key, &target_ip->key);
+    unlock_user_struct(target_ip, target_addr, 0);
+
+    return 0;
+}
+
+abi_long host_to_target_ipc_perm(abi_ulong target_addr,
+        struct ipc_perm *host_ip)
+{
+    struct target_ipc_perm *target_ip;
+
+    if (!lock_user_struct(VERIFY_WRITE, target_ip, target_addr, 0)) {
+        return -TARGET_EFAULT;
+    }
+    __put_user(host_ip->cuid, &target_ip->cuid);
+    __put_user(host_ip->cgid, &target_ip->cgid);
+    __put_user(host_ip->uid, &target_ip->uid);
+    __put_user(host_ip->gid, &target_ip->gid);
+    __put_user(host_ip->mode, &target_ip->mode);
+    __put_user(host_ip->seq, &target_ip->seq);
+    __put_user(host_ip->key, &target_ip->key);
+    unlock_user_struct(target_ip, target_addr, 1);
+
+    return 0;
+}
+
-- 
2.40.0

Re: [PATCH 09/22] Implement ipc_perm conversion between host and target.

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2213 bytes --]

On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>


> diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
> index 6c123abf04..e69250cc0d 100644
> --- a/bsd-user/bsd-mem.c
> +++ b/bsd-user/bsd-mem.c
> @@ -36,3 +36,44 @@ void target_set_brk(abi_ulong new_brk)
>      bsd_target_original_brk = bsd_target_brk = HOST_PAGE_ALIGN(new_brk);
>      brk_page = HOST_PAGE_ALIGN(bsd_target_brk);
>  }
> +
> +abi_long target_to_host_ipc_perm(struct ipc_perm *host_ip,
> +        abi_ulong target_addr)
> +{
> +    struct target_ipc_perm *target_ip;
> +
> +    if (!lock_user_struct(VERIFY_READ, target_ip, target_addr, 1)) {
> +        return -TARGET_EFAULT;
> +    }
> +    __get_user(host_ip->cuid, &target_ip->cuid);
> +    __get_user(host_ip->cgid, &target_ip->cgid);
> +    __get_user(host_ip->uid, &target_ip->uid);
> +    __get_user(host_ip->gid, &target_ip->gid);
> +    __get_user(host_ip->mode, &target_ip->mode);
> +    __get_user(host_ip->seq, &target_ip->seq);
> +    __get_user(host_ip->key, &target_ip->key);
> +    unlock_user_struct(target_ip, target_addr, 0);
> +
> +    return 0;
> +}
> +
> +abi_long host_to_target_ipc_perm(abi_ulong target_addr,
> +        struct ipc_perm *host_ip)
> +{
> +    struct target_ipc_perm *target_ip;
> +
> +    if (!lock_user_struct(VERIFY_WRITE, target_ip, target_addr, 0)) {
> +        return -TARGET_EFAULT;
> +    }
> +    __put_user(host_ip->cuid, &target_ip->cuid);
> +    __put_user(host_ip->cgid, &target_ip->cgid);
> +    __put_user(host_ip->uid, &target_ip->uid);
> +    __put_user(host_ip->gid, &target_ip->gid);
> +    __put_user(host_ip->mode, &target_ip->mode);
> +    __put_user(host_ip->seq, &target_ip->seq);
> +    __put_user(host_ip->key, &target_ip->key);
> +    unlock_user_struct(target_ip, target_addr, 1);
> +
> +    return 0;
> +}
> +
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 3137 bytes --]

Re: [PATCH 09/22] Implement ipc_perm conversion between host and target.

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.c | 41 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 10/22] Implement shmid_ds conversion between host and target.

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
index e69250cc0d..4446c94725 100644
--- a/bsd-user/bsd-mem.c
+++ b/bsd-user/bsd-mem.c
@@ -77,3 +77,49 @@ abi_long host_to_target_ipc_perm(abi_ulong target_addr,
     return 0;
 }
 
+abi_long target_to_host_shmid_ds(struct shmid_ds *host_sd,
+        abi_ulong target_addr)
+{
+    struct target_shmid_ds *target_sd;
+
+    if (!lock_user_struct(VERIFY_READ, target_sd, target_addr, 1)) {
+        return -TARGET_EFAULT;
+    }
+    if (target_to_host_ipc_perm(&(host_sd->shm_perm), target_addr)) {
+        return -TARGET_EFAULT;
+    }
+    __get_user(host_sd->shm_segsz, &target_sd->shm_segsz);
+    __get_user(host_sd->shm_lpid, &target_sd->shm_lpid);
+    __get_user(host_sd->shm_cpid, &target_sd->shm_cpid);
+    __get_user(host_sd->shm_nattch, &target_sd->shm_nattch);
+    __get_user(host_sd->shm_atime, &target_sd->shm_atime);
+    __get_user(host_sd->shm_dtime, &target_sd->shm_dtime);
+    __get_user(host_sd->shm_ctime, &target_sd->shm_ctime);
+    unlock_user_struct(target_sd, target_addr, 0);
+
+    return 0;
+}
+
+abi_long host_to_target_shmid_ds(abi_ulong target_addr,
+        struct shmid_ds *host_sd)
+{
+    struct target_shmid_ds *target_sd;
+
+    if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0)) {
+        return -TARGET_EFAULT;
+    }
+    if (host_to_target_ipc_perm(target_addr, &(host_sd->shm_perm))) {
+        return -TARGET_EFAULT;
+    }
+    __put_user(host_sd->shm_segsz, &target_sd->shm_segsz);
+    __put_user(host_sd->shm_lpid, &target_sd->shm_lpid);
+    __put_user(host_sd->shm_cpid, &target_sd->shm_cpid);
+    __put_user(host_sd->shm_nattch, &target_sd->shm_nattch);
+    __put_user(host_sd->shm_atime, &target_sd->shm_atime);
+    __put_user(host_sd->shm_dtime, &target_sd->shm_dtime);
+    __put_user(host_sd->shm_ctime, &target_sd->shm_ctime);
+    unlock_user_struct(target_sd, target_addr, 1);
+
+    return 0;
+}
+
-- 
2.40.0

Re: [PATCH 10/22] Implement shmid_ds conversion between host and target.

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2508 bytes --]

On Sat, Aug 19, 2023 at 3:48 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

diff --git a/bsd-user/bsd-mem.c b/bsd-user/bsd-mem.c
> index e69250cc0d..4446c94725 100644
> --- a/bsd-user/bsd-mem.c
> +++ b/bsd-user/bsd-mem.c
> @@ -77,3 +77,49 @@ abi_long host_to_target_ipc_perm(abi_ulong target_addr,
>      return 0;
>  }
>
> +abi_long target_to_host_shmid_ds(struct shmid_ds *host_sd,
> +        abi_ulong target_addr)
> +{
> +    struct target_shmid_ds *target_sd;
> +
> +    if (!lock_user_struct(VERIFY_READ, target_sd, target_addr, 1)) {
> +        return -TARGET_EFAULT;
> +    }
> +    if (target_to_host_ipc_perm(&(host_sd->shm_perm), target_addr)) {
> +        return -TARGET_EFAULT;
> +    }
> +    __get_user(host_sd->shm_segsz, &target_sd->shm_segsz);
> +    __get_user(host_sd->shm_lpid, &target_sd->shm_lpid);
> +    __get_user(host_sd->shm_cpid, &target_sd->shm_cpid);
> +    __get_user(host_sd->shm_nattch, &target_sd->shm_nattch);
> +    __get_user(host_sd->shm_atime, &target_sd->shm_atime);
> +    __get_user(host_sd->shm_dtime, &target_sd->shm_dtime);
> +    __get_user(host_sd->shm_ctime, &target_sd->shm_ctime);
> +    unlock_user_struct(target_sd, target_addr, 0);
> +
> +    return 0;
> +}
> +
> +abi_long host_to_target_shmid_ds(abi_ulong target_addr,
> +        struct shmid_ds *host_sd)
> +{
> +    struct target_shmid_ds *target_sd;
> +
> +    if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0)) {
> +        return -TARGET_EFAULT;
> +    }
> +    if (host_to_target_ipc_perm(target_addr, &(host_sd->shm_perm))) {
> +        return -TARGET_EFAULT;
> +    }
> +    __put_user(host_sd->shm_segsz, &target_sd->shm_segsz);
> +    __put_user(host_sd->shm_lpid, &target_sd->shm_lpid);
> +    __put_user(host_sd->shm_cpid, &target_sd->shm_cpid);
> +    __put_user(host_sd->shm_nattch, &target_sd->shm_nattch);
> +    __put_user(host_sd->shm_atime, &target_sd->shm_atime);
> +    __put_user(host_sd->shm_dtime, &target_sd->shm_dtime);
> +    __put_user(host_sd->shm_ctime, &target_sd->shm_ctime);
> +    unlock_user_struct(target_sd, target_addr, 1);
> +
> +    return 0;
> +}
> +
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 3476 bytes --]

Re: [PATCH 10/22] Implement shmid_ds conversion between host and target.

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> +    if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0)) {
> +        return -TARGET_EFAULT;
> +    }
> +    if (host_to_target_ipc_perm(target_addr, &(host_sd->shm_perm))) {
> +        return -TARGET_EFAULT;
> +    }

While it works, ideally you wouldn't double-lock a memory range, once here and once in 
host_to_target_ipc_perm.  You could split out the middle of the function as 
host_to_target_ipc_perm__locked.

Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 10/22] Implement shmid_ds conversion between host and target.

[Kariiem Taha]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 8/19/23 02:47, Karim Taha wrote:
>> +    if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0)) {
>> +        return -TARGET_EFAULT;
>> +    }
>> +    if (host_to_target_ipc_perm(target_addr, &(host_sd->shm_perm))) {
>> +        return -TARGET_EFAULT;
>> +    }
>
> While it works, ideally you wouldn't double-lock a memory range, once here and once in 
> host_to_target_ipc_perm.  You could split out the middle of the function as 
> host_to_target_ipc_perm__locked.

Hi Richard,

Can you please verify the correctness of the following refactoring?
    void host_to_target_ipc_perm__locked(abi_ulong target_addr,
            struct ipc_perm *host_ip)
    {
        struct target_ipc_perm *target_ip = g2h_untagged(target_addr);
        __put_user(host_ip->cuid, &target_ip->cuid);
        __put_user(host_ip->cgid, &target_ip->cgid);
        __put_user(host_ip->uid, &target_ip->uid);
        __put_user(host_ip->gid, &target_ip->gid);
        __put_user(host_ip->mode, &target_ip->mode);
        __put_user(host_ip->seq, &target_ip->seq);
        __put_user(host_ip->key, &target_ip->key);
    }

    abi_long host_to_target_shmid_ds(abi_ulong target_addr,
            struct shmid_ds *host_sd)
    {
        struct target_shmid_ds *target_sd;
        target_sd = lock_user(VERIFY_WRITE, target_addr, sizeof(*target_sd), 0);
        if (!target_sd){
            return -TARGET_EFAULT;
        }

        host_to_target_ipc_perm__locked(target_addr, &(host_sd->shm_perm));
        __put_user(host_sd->shm_segsz, &target_sd->shm_segsz);
        __put_user(host_sd->shm_lpid, &target_sd->shm_lpid);
        __put_user(host_sd->shm_cpid, &target_sd->shm_cpid);
        __put_user(host_sd->shm_nattch, &target_sd->shm_nattch);
        __put_user(host_sd->shm_atime, &target_sd->shm_atime);
        __put_user(host_sd->shm_dtime, &target_sd->shm_dtime);
        __put_user(host_sd->shm_ctime, &target_sd->shm_ctime);
        unlock_user_struct(target_sd, target_addr, 1);

        return 0;
    }

As far as I understood the `page_check_range` function, defined at
accel/tcg/user-exec.c::523:

-The locked range is (target_addr, target_addr + sizeof(target_ipc_perm) -1) in case of
     host_to_target_ipc_perm function.

-The locked range is (target_addr, taregt_addr + sizeof(target_shmid_ds) -1) in case of
      host_to_target_shmid_ds function.

Since `host_to_target_shmid_ds` struct has larger size, in the original
code, is the sucess of the first lock guarantees the sucess of the
second?

If I got it wrong, please elaborate further.

If I'm correct, do you think I should call g2h_untagged in
`host_to_target_ipc_perm__locked` directly, or should I receive it as a
paremeter?

--
Kariiem Taha

Re: [PATCH 10/22] Implement shmid_ds conversion between host and target.

[Richard Henderson]

On 9/3/23 01:45, Kariiem Taha wrote:
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 8/19/23 02:47, Karim Taha wrote:
>>> +    if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0)) {
>>> +        return -TARGET_EFAULT;
>>> +    }
>>> +    if (host_to_target_ipc_perm(target_addr, &(host_sd->shm_perm))) {
>>> +        return -TARGET_EFAULT;
>>> +    }
>>
>> While it works, ideally you wouldn't double-lock a memory range, once here and once in
>> host_to_target_ipc_perm.  You could split out the middle of the function as
>> host_to_target_ipc_perm__locked.
> 
> Hi Richard,
> 
> Can you please verify the correctness of the following refactoring?
>      void host_to_target_ipc_perm__locked(abi_ulong target_addr,
>              struct ipc_perm *host_ip)
>      {
>          struct target_ipc_perm *target_ip = g2h_untagged(target_addr);
>          __put_user(host_ip->cuid, &target_ip->cuid);
>          __put_user(host_ip->cgid, &target_ip->cgid);
>          __put_user(host_ip->uid, &target_ip->uid);
>          __put_user(host_ip->gid, &target_ip->gid);
>          __put_user(host_ip->mode, &target_ip->mode);
>          __put_user(host_ip->seq, &target_ip->seq);
>          __put_user(host_ip->key, &target_ip->key);
>      }
> 
>      abi_long host_to_target_shmid_ds(abi_ulong target_addr,
>              struct shmid_ds *host_sd)
>      {
>          struct target_shmid_ds *target_sd;
>          target_sd = lock_user(VERIFY_WRITE, target_addr, sizeof(*target_sd), 0);
>          if (!target_sd){
>              return -TARGET_EFAULT;
>          }
> 
>          host_to_target_ipc_perm__locked(target_addr, &(host_sd->shm_perm));

No.  You'd pass &target_sd->shm_perm, not target_addr, and you don't use g2h at all.


r~

[PATCH 11/22] Introduce bsd-mem.h to the source tree

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

To preserve the copyright notice and help with the 'Author' info for
subsequent changes to the file.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 65 +++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  1 +
 2 files changed, 66 insertions(+)
 create mode 100644 bsd-user/bsd-mem.h

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
new file mode 100644
index 0000000000..fed7b7cd80
--- /dev/null
+++ b/bsd-user/bsd-mem.h
@@ -0,0 +1,65 @@
+/*
+ *  memory management system call shims and definitions
+ *
+ *  Copyright (c) 2013-15 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Copyright (c) 1982, 1986, 1993
+ *      The Regents of the University of California.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 4. Neither the name of the University nor the names of its contributors
+ *    may be used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef BSD_USER_BSD_MEM_H
+#define BSD_USER_BSD_MEM_H
+
+#include <sys/types.h>
+#include <sys/ipc.h>
+#include <sys/mman.h>
+#include <sys/shm.h>
+#include <fcntl.h>
+
+#include "qemu-bsd.h"
+
+extern struct bsd_shm_regions bsd_shm_regions[];
+extern abi_ulong bsd_target_brk;
+extern abi_ulong bsd_target_original_brk;
+extern abi_ulong brk_page;
+
+#endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index c0a22eb746..7e2a395e0f 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -35,6 +35,7 @@
 
 /* BSD independent syscall shims */
 #include "bsd-file.h"
+#include "bsd-mem.h"
 #include "bsd-proc.h"
 
 /* *BSD dependent syscall shims */
-- 
2.40.0

Re: [PATCH 11/22] Introduce bsd-mem.h to the source tree

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 4297 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> To preserve the copyright notice and help with the 'Author' info for
> subsequent changes to the file.
>

I'd drop the first 'To' in this message. It's clearer without it what is
meant.


> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 65 +++++++++++++++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  1 +
>  2 files changed, 66 insertions(+)
>  create mode 100644 bsd-user/bsd-mem.h
>

otherwise
Reviewed-by: Warner Losh <imp@bsdimp.com>

Warner


> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> new file mode 100644
> index 0000000000..fed7b7cd80
> --- /dev/null
> +++ b/bsd-user/bsd-mem.h
> @@ -0,0 +1,65 @@
> +/*
> + *  memory management system call shims and definitions
> + *
> + *  Copyright (c) 2013-15 Stacey D. Son
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/*
> + * Copyright (c) 1982, 1986, 1993
> + *      The Regents of the University of California.  All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 4. Neither the name of the University nor the names of its contributors
> + *    may be used to endorse or promote products derived from this
> software
> + *    without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> PURPOSE
> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE
> LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
> WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +
> +#ifndef BSD_USER_BSD_MEM_H
> +#define BSD_USER_BSD_MEM_H
> +
> +#include <sys/types.h>
> +#include <sys/ipc.h>
> +#include <sys/mman.h>
> +#include <sys/shm.h>
> +#include <fcntl.h>
> +
> +#include "qemu-bsd.h"
> +
> +extern struct bsd_shm_regions bsd_shm_regions[];
> +extern abi_ulong bsd_target_brk;
> +extern abi_ulong bsd_target_original_brk;
> +extern abi_ulong brk_page;
> +
> +#endif /* BSD_USER_BSD_MEM_H */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index c0a22eb746..7e2a395e0f 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -35,6 +35,7 @@
>
>  /* BSD independent syscall shims */
>  #include "bsd-file.h"
> +#include "bsd-mem.h"
>  #include "bsd-proc.h"
>
>  /* *BSD dependent syscall shims */
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 5495 bytes --]

Re: [PATCH 11/22] Introduce bsd-mem.h to the source tree

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> To preserve the copyright notice and help with the 'Author' info for
> subsequent changes to the file.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.h            | 65 +++++++++++++++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c |  1 +
>   2 files changed, 66 insertions(+)
>   create mode 100644 bsd-user/bsd-mem.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 12/22] Implement mmap(2) and munmap(2)

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 20 ++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  9 +++++++++
 2 files changed, 29 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index fed7b7cd80..057c995a6e 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -62,4 +62,24 @@ extern abi_ulong bsd_target_brk;
 extern abi_ulong bsd_target_original_brk;
 extern abi_ulong brk_page;
 
+/* mmap(2) */
+static inline abi_long do_bsd_mmap(void *cpu_env, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5, abi_long arg6, abi_long arg7,
+    abi_long arg8)
+{
+    if (regpairs_aligned(cpu_env) != 0) {
+        arg6 = arg7;
+        arg7 = arg8;
+    }
+    return get_errno(target_mmap(arg1, arg2, arg3,
+                                 target_to_host_bitmask(arg4, mmap_flags_tbl),
+                                 arg5, target_arg64(arg6, arg7)));
+}
+
+/* munmap(2) */
+static inline abi_long do_bsd_munmap(abi_long arg1, abi_long arg2)
+{
+    return get_errno(target_munmap(arg1, arg2));
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 7e2a395e0f..3525d39d7b 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -486,6 +486,15 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         /*
          * Memory management system calls.
          */
+    case TARGET_FREEBSD_NR_mmap: /* mmap(2) */
+        ret = do_bsd_mmap(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6, arg7,
+           arg8);
+        break;
+
+    case TARGET_FREEBSD_NR_munmap: /* munmap(2) */
+        ret = do_bsd_munmap(arg1, arg2);
+        break;
+
 #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
     case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
         ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
-- 
2.40.0

Re: [PATCH 12/22] Implement mmap(2) and munmap(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2362 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 20 ++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  9 +++++++++
>  2 files changed, 29 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>


> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index fed7b7cd80..057c995a6e 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -62,4 +62,24 @@ extern abi_ulong bsd_target_brk;
>  extern abi_ulong bsd_target_original_brk;
>  extern abi_ulong brk_page;
>
> +/* mmap(2) */
> +static inline abi_long do_bsd_mmap(void *cpu_env, abi_long arg1, abi_long
> arg2,
> +    abi_long arg3, abi_long arg4, abi_long arg5, abi_long arg6, abi_long
> arg7,
> +    abi_long arg8)
> +{
> +    if (regpairs_aligned(cpu_env) != 0) {
> +        arg6 = arg7;
> +        arg7 = arg8;
> +    }
> +    return get_errno(target_mmap(arg1, arg2, arg3,
> +                                 target_to_host_bitmask(arg4,
> mmap_flags_tbl),
> +                                 arg5, target_arg64(arg6, arg7)));
> +}
> +
> +/* munmap(2) */
> +static inline abi_long do_bsd_munmap(abi_long arg1, abi_long arg2)
> +{
> +    return get_errno(target_munmap(arg1, arg2));
> +}
> +
>  #endif /* BSD_USER_BSD_MEM_H */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 7e2a395e0f..3525d39d7b 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -486,6 +486,15 @@ static abi_long freebsd_syscall(void *cpu_env, int
> num, abi_long arg1,
>          /*
>           * Memory management system calls.
>           */
> +    case TARGET_FREEBSD_NR_mmap: /* mmap(2) */
> +        ret = do_bsd_mmap(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6,
> arg7,
> +           arg8);
> +        break;
> +
> +    case TARGET_FREEBSD_NR_munmap: /* munmap(2) */
> +        ret = do_bsd_munmap(arg1, arg2);
> +        break;
> +
>  #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
>      case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
>          ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 3193 bytes --]

Re: [PATCH 12/22] Implement mmap(2) and munmap(2)

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> +    case TARGET_FREEBSD_NR_mmap: /* mmap(2) */
> +        ret = do_bsd_mmap(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6, arg7,
> +           arg8);

Indentation.  Align after (.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 13/22] Implement mprotect(2)

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 7 +++++++
 bsd-user/freebsd/os-syscall.c | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index 057c995a6e..a6446a454c 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -82,4 +82,11 @@ static inline abi_long do_bsd_munmap(abi_long arg1, abi_long arg2)
     return get_errno(target_munmap(arg1, arg2));
 }
 
+/* mprotect(2) */
+static inline abi_long do_bsd_mprotect(abi_long arg1, abi_long arg2,
+        abi_long arg3)
+{
+    return get_errno(target_mprotect(arg1, arg2, arg3));
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 3525d39d7b..aea4e337ff 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -495,6 +495,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_munmap(arg1, arg2);
         break;
 
+    case TARGET_FREEBSD_NR_mprotect: /* mprotect(2) */
+        ret = do_bsd_mprotect(arg1, arg2, arg3);
+        break;
+
 #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
     case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
         ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
-- 
2.40.0

Re: [PATCH 13/22] Implement mprotect(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 7 +++++++
>  bsd-user/freebsd/os-syscall.c | 4 ++++
>  2 files changed, 11 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>


> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index 057c995a6e..a6446a454c 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -82,4 +82,11 @@ static inline abi_long do_bsd_munmap(abi_long arg1,
> abi_long arg2)
>      return get_errno(target_munmap(arg1, arg2));
>  }
>
> +/* mprotect(2) */
> +static inline abi_long do_bsd_mprotect(abi_long arg1, abi_long arg2,
> +        abi_long arg3)
> +{
> +    return get_errno(target_mprotect(arg1, arg2, arg3));
> +}
> +
>  #endif /* BSD_USER_BSD_MEM_H */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 3525d39d7b..aea4e337ff 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -495,6 +495,10 @@ static abi_long freebsd_syscall(void *cpu_env, int
> num, abi_long arg1,
>          ret = do_bsd_munmap(arg1, arg2);
>          break;
>
> +    case TARGET_FREEBSD_NR_mprotect: /* mprotect(2) */
> +        ret = do_bsd_mprotect(arg1, arg2, arg3);
> +        break;
> +
>  #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
>      case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
>          ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 2404 bytes --]

Re: [PATCH 13/22] Implement mprotect(2)

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.h            | 7 +++++++
>   bsd-user/freebsd/os-syscall.c | 4 ++++
>   2 files changed, 11 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 14/22] Implement msync(2)

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Co-authored-by: Kyle Evans <kevans@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 11 +++++++++++
 bsd-user/freebsd/os-syscall.c |  4 ++++
 2 files changed, 15 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index a6446a454c..68d79ac080 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -89,4 +89,15 @@ static inline abi_long do_bsd_mprotect(abi_long arg1, abi_long arg2,
     return get_errno(target_mprotect(arg1, arg2, arg3));
 }
 
+/* msync(2) */
+static inline abi_long do_bsd_msync(abi_long addr, abi_long len, abi_long flags)
+{
+    if (!access_ok(VERIFY_WRITE, addr, len)) {
+        /* XXX Should be EFAULT, but FreeBSD seems to get this wrong. */
+        return -TARGET_ENOMEM;
+    }
+
+    return get_errno(msync(g2h_untagged(addr), len, flags));
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index aea4e337ff..3871b15309 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -499,6 +499,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_mprotect(arg1, arg2, arg3);
         break;
 
+    case TARGET_FREEBSD_NR_msync: /* msync(2) */
+        ret = do_bsd_msync(arg1, arg2, arg3);
+        break;
+
 #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
     case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
         ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
-- 
2.40.0

Re: [PATCH 14/22] Implement msync(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Co-authored-by: Kyle Evans <kevans@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 11 +++++++++++
>  bsd-user/freebsd/os-syscall.c |  4 ++++
>  2 files changed, 15 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

but see below


> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index a6446a454c..68d79ac080 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -89,4 +89,15 @@ static inline abi_long do_bsd_mprotect(abi_long arg1,
> abi_long arg2,
>      return get_errno(target_mprotect(arg1, arg2, arg3));
>  }
>
> +/* msync(2) */
> +static inline abi_long do_bsd_msync(abi_long addr, abi_long len, abi_long
> flags)
> +{
> +    if (!access_ok(VERIFY_WRITE, addr, len)) {
> +        /* XXX Should be EFAULT, but FreeBSD seems to get this wrong. */
>

I'd reword this here (and in the bsd-user fork):

/* It seems odd, but POSIX wants this to be ENOMEM */

since it isn't FreeBSD getting it wrong, and this matches the behavior
that's documented for Linux as well (though in less detail). POSIX open
group appears to document ENOMEM for this.


> +        return -TARGET_ENOMEM;
> +    }
> +
> +    return get_errno(msync(g2h_untagged(addr), len, flags));
> +}
> +
>  #endif /* BSD_USER_BSD_MEM_H */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index aea4e337ff..3871b15309 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -499,6 +499,10 @@ static abi_long freebsd_syscall(void *cpu_env, int
> num, abi_long arg1,
>          ret = do_bsd_mprotect(arg1, arg2, arg3);
>          break;
>
> +    case TARGET_FREEBSD_NR_msync: /* msync(2) */
> +        ret = do_bsd_msync(arg1, arg2, arg3);
> +        break;
> +
>  #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
>      case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
>          ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 3313 bytes --]

Re: [PATCH 14/22] Implement msync(2)

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> +static inline abi_long do_bsd_msync(abi_long addr, abi_long len, abi_long flags)
> +{
> +    if (!access_ok(VERIFY_WRITE, addr, len)) {

I think this check is wrong.  There's nothing in the kernel that requires writability, or 
even that the entire range be mapped.

I think you want guest_range_valid_untagged to simply check that the bounds are ok.

With that,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



r~

[PATCH 15/22] Implement mlock(2), munlock(2), mlockall(2), munlockall(2), madvise(2), minherit(2)

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 44 +++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c | 24 +++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index 68d79ac080..f76881519c 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -100,4 +100,48 @@ static inline abi_long do_bsd_msync(abi_long addr, abi_long len, abi_long flags)
     return get_errno(msync(g2h_untagged(addr), len, flags));
 }
 
+/* mlock(2) */
+static inline abi_long do_bsd_mlock(abi_long arg1, abi_long arg2)
+{
+    return get_errno(mlock(g2h_untagged(arg1), arg2));
+}
+
+/* munlock(2) */
+static inline abi_long do_bsd_munlock(abi_long arg1, abi_long arg2)
+{
+    return get_errno(munlock(g2h_untagged(arg1), arg2));
+}
+
+/* mlockall(2) */
+static inline abi_long do_bsd_mlockall(abi_long arg1)
+{
+    return get_errno(mlockall(arg1));
+}
+
+/* munlockall(2) */
+static inline abi_long do_bsd_munlockall(void)
+{
+    return get_errno(munlockall());
+}
+
+/* madvise(2) */
+static inline abi_long do_bsd_madvise(abi_long arg1, abi_long arg2,
+        abi_long arg3)
+{
+    /*
+     * A straight passthrough may not be safe because qemu sometimes
+     * turns private file-backed mapping into anonymous mappings. This
+     * will break MADV_DONTNEED.  This is a hint, so ignoring and returing
+     * success is ok.
+     */
+    return get_errno(0);
+}
+
+/* minherit(2) */
+static inline abi_long do_bsd_minherit(abi_long addr, abi_long len,
+        abi_long inherit)
+{
+    return get_errno(minherit(g2h_untagged(addr), len, inherit));
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 3871b15309..96469f6a63 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -503,6 +503,30 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_msync(arg1, arg2, arg3);
         break;
 
+    case TARGET_FREEBSD_NR_mlock: /* mlock(2) */
+        ret = do_bsd_mlock(arg1, arg2);
+        break;
+
+    case TARGET_FREEBSD_NR_munlock: /* munlock(2) */
+        ret = do_bsd_munlock(arg1, arg2);
+        break;
+
+    case TARGET_FREEBSD_NR_mlockall: /* mlockall(2) */
+        ret = do_bsd_mlockall(arg1);
+        break;
+
+    case TARGET_FREEBSD_NR_munlockall: /* munlockall(2) */
+        ret = do_bsd_munlockall();
+        break;
+
+    case TARGET_FREEBSD_NR_madvise: /* madvise(2) */
+        ret = do_bsd_madvise(arg1, arg2, arg3);
+        break;
+
+    case TARGET_FREEBSD_NR_minherit: /* minherit(2) */
+        ret = do_bsd_minherit(arg1, arg2, arg3);
+        break;
+
 #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
     case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
         ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
-- 
2.40.0

Re: [PATCH 15/22] Implement mlock(2), munlock(2), mlockall(2), munlockall(2), madvise(2), minherit(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 3613 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 44 +++++++++++++++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c | 24 +++++++++++++++++++
>  2 files changed, 68 insertions(+)
>
> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index 68d79ac080..f76881519c 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -100,4 +100,48 @@ static inline abi_long do_bsd_msync(abi_long addr,
> abi_long len, abi_long flags)
>      return get_errno(msync(g2h_untagged(addr), len, flags));
>  }
>
> +/* mlock(2) */
> +static inline abi_long do_bsd_mlock(abi_long arg1, abi_long arg2)
> +{
> +    return get_errno(mlock(g2h_untagged(arg1), arg2));
> +}
> +
> +/* munlock(2) */
> +static inline abi_long do_bsd_munlock(abi_long arg1, abi_long arg2)
> +{
> +    return get_errno(munlock(g2h_untagged(arg1), arg2));
> +}
> +
> +/* mlockall(2) */
> +static inline abi_long do_bsd_mlockall(abi_long arg1)
> +{
> +    return get_errno(mlockall(arg1));
> +}
> +
> +/* munlockall(2) */
> +static inline abi_long do_bsd_munlockall(void)
> +{
> +    return get_errno(munlockall());
> +}
> +
> +/* madvise(2) */
> +static inline abi_long do_bsd_madvise(abi_long arg1, abi_long arg2,
> +        abi_long arg3)
> +{
> +    /*
> +     * A straight passthrough may not be safe because qemu sometimes
> +     * turns private file-backed mapping into anonymous mappings. This
> +     * will break MADV_DONTNEED.  This is a hint, so ignoring and returing
> +     * success is ok.
> +     */
> +    return get_errno(0);
>

This looks like it was copied from an early linux-user implementation, and
that seems to have been fixed to no longer cause problems. Can someone
that knows about the linux-user history here comment?

Warner


> +}
> +
> +/* minherit(2) */
> +static inline abi_long do_bsd_minherit(abi_long addr, abi_long len,
> +        abi_long inherit)
> +{
> +    return get_errno(minherit(g2h_untagged(addr), len, inherit));
> +}
> +
>  #endif /* BSD_USER_BSD_MEM_H */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 3871b15309..96469f6a63 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -503,6 +503,30 @@ static abi_long freebsd_syscall(void *cpu_env, int
> num, abi_long arg1,
>          ret = do_bsd_msync(arg1, arg2, arg3);
>          break;
>
> +    case TARGET_FREEBSD_NR_mlock: /* mlock(2) */
> +        ret = do_bsd_mlock(arg1, arg2);
> +        break;
> +
> +    case TARGET_FREEBSD_NR_munlock: /* munlock(2) */
> +        ret = do_bsd_munlock(arg1, arg2);
> +        break;
> +
> +    case TARGET_FREEBSD_NR_mlockall: /* mlockall(2) */
> +        ret = do_bsd_mlockall(arg1);
> +        break;
> +
> +    case TARGET_FREEBSD_NR_munlockall: /* munlockall(2) */
> +        ret = do_bsd_munlockall();
> +        break;
> +
> +    case TARGET_FREEBSD_NR_madvise: /* madvise(2) */
> +        ret = do_bsd_madvise(arg1, arg2, arg3);
> +        break;
> +
> +    case TARGET_FREEBSD_NR_minherit: /* minherit(2) */
> +        ret = do_bsd_minherit(arg1, arg2, arg3);
> +        break;
> +
>  #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
>      case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
>          ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 4547 bytes --]

Re: [PATCH 15/22] Implement mlock(2), munlock(2), mlockall(2), munlockall(2), madvise(2), minherit(2)

[Richard Henderson]

On 8/19/23 21:37, Warner Losh wrote:
>     +/* madvise(2) */
>     +static inline abi_long do_bsd_madvise(abi_long arg1, abi_long arg2,
>     +        abi_long arg3)
>     +{
>     +    /*
>     +     * A straight passthrough may not be safe because qemu sometimes
>     +     * turns private file-backed mapping into anonymous mappings. This
>     +     * will break MADV_DONTNEED.  This is a hint, so ignoring and returing
>     +     * success is ok.
>     +     */
>     +    return get_errno(0);
> 
> 
> This looks like it was copied from an early linux-user implementation, and
> that seems to have been fixed to no longer cause problems. Can someone
> that knows about the linux-user history here comment?

We now track pages that are "passthrough" and ok for DONTNEED etc.


     case MADV_DONTNEED:
         if (page_check_range(start, len, PAGE_PASSTHROUGH)) {
             ret = get_errno(madvise(g2h_untagged(start), len, advice));
             if ((advice == MADV_DONTNEED) && (ret == 0)) {
                 page_reset_target_data(start, start + len - 1);
             }
         }


It's still not ideal, but it's something.


r~

Re: [PATCH 15/22] Implement mlock(2), munlock(2), mlockall(2), munlockall(2), madvise(2), minherit(2)

[Richard Henderson]

On 8/19/23 02:47, Karim Taha wrote:
> +static inline abi_long do_bsd_mlock(abi_long arg1, abi_long arg2)
> +{
> +    return get_errno(mlock(g2h_untagged(arg1), arg2));
> +}
> +
> +/* munlock(2) */
> +static inline abi_long do_bsd_munlock(abi_long arg1, abi_long arg2)
> +{
> +    return get_errno(munlock(g2h_untagged(arg1), arg2));
> +}

I think these two need guest_range_valid_untagged.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 16/22] Implement mincore(2)

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 23 +++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  4 ++++
 2 files changed, 27 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index f76881519c..edbccd3111 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -144,4 +144,27 @@ static inline abi_long do_bsd_minherit(abi_long addr, abi_long len,
     return get_errno(minherit(g2h_untagged(addr), len, inherit));
 }
 
+/* mincore(2) */
+static inline abi_long do_bsd_mincore(abi_ulong target_addr, abi_ulong len,
+        abi_ulong target_vec)
+{
+    abi_long ret;
+    void *p, *a;
+
+    a = lock_user(VERIFY_WRITE, target_addr, len, 0);
+    if (a == NULL) {
+        return -TARGET_EFAULT;
+    }
+    p = lock_user_string(target_vec);
+    if (p == NULL) {
+        unlock_user(a, target_addr, 0);
+        return -TARGET_EFAULT;
+    }
+    ret = get_errno(mincore(a, len, p));
+    unlock_user(p, target_vec, ret);
+    unlock_user(a, target_addr, 0);
+
+    return ret;
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 96469f6a63..1db0907504 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -527,6 +527,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_minherit(arg1, arg2, arg3);
         break;
 
+    case TARGET_FREEBSD_NR_mincore: /* mincore(2) */
+        ret = do_bsd_mincore(arg1, arg2, arg3);
+        break;
+
 #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
     case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
         ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
-- 
2.40.0

Re: [PATCH 16/22] Implement mincore(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 23 +++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  4 ++++
>  2 files changed, 27 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

[-- Attachment #2: Type: text/html, Size: 908 bytes --]

Re: [PATCH 16/22] Implement mincore(2)

[Richard Henderson]

On 8/19/23 02:48, Karim Taha wrote:
> +/* mincore(2) */
> +static inline abi_long do_bsd_mincore(abi_ulong target_addr, abi_ulong len,
> +        abi_ulong target_vec)
> +{
> +    abi_long ret;
> +    void *p, *a;
> +
> +    a = lock_user(VERIFY_WRITE, target_addr, len, 0);
> +    if (a == NULL) {
> +        return -TARGET_EFAULT;
> +    }
> +    p = lock_user_string(target_vec);
> +    if (p == NULL) {
> +        unlock_user(a, target_addr, 0);
> +        return -TARGET_EFAULT;
> +    }
> +    ret = get_errno(mincore(a, len, p));
> +    unlock_user(p, target_vec, ret);
> +    unlock_user(a, target_addr, 0);
> +
> +    return ret;

This is wrong.

(1) VERIFY_WRITE is incorrect.  Here you need a combination of guest_range_valid_untagged 
and page_check_range(addr, len, PAGE_VALID).

(2) vec is not a string, it is an array of size DIV_ROUND_UP(len, TARGET_PAGE_SIZE).  For 
that, you do want lock_user(VERIFY_WRITE).


r~

[PATCH 17/22] Implement do_obreak function

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Co-authored-by: Mikaël Urankar <mikael.urankar@gmail.com>

Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 85 +++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  7 +++
 2 files changed, 92 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index edbccd3111..6f33148eb7 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -167,4 +167,89 @@ static inline abi_long do_bsd_mincore(abi_ulong target_addr, abi_ulong len,
     return ret;
 }
 
+#ifdef DO_DEBUG
+#define DEBUGF_BRK(message, args...) \
+    do { fprintf(stderr, (message), ## args); } while (0)
+#else
+#define DEBUGF_BRK(message, args...)
+#endif
+
+/* do_brk() must return target values and target errnos. */
+static inline abi_long do_obreak(abi_ulong new_brk)
+{
+    abi_long mapped_addr;
+    int new_alloc_size;
+
+    DEBUGF_BRK("do_brk(" TARGET_ABI_FMT_lx ") -> ", new_brk);
+
+    if (!new_brk) {
+        DEBUGF_BRK(TARGET_ABI_FMT_lx " (!new_brk)\n", bsd_target_brk);
+        return bsd_target_brk;
+    }
+    if (new_brk < bsd_target_original_brk) {
+        DEBUGF_BRK(TARGET_ABI_FMT_lx " (new_brk < bsd_target_original_brk)\n",
+                   bsd_target_brk);
+        return bsd_target_brk;
+    }
+
+    /*
+     * If the new brk is less than the highest page reserved to the target heap
+     * allocation, set it and we're almost done...
+     */
+    if (new_brk <= brk_page) {
+        /*
+         * Heap contents are initialized to zero, as for anonymous mapped pages.
+         */
+        if (new_brk > bsd_target_brk) {
+            memset(g2h_untagged(bsd_target_brk), 0, new_brk - bsd_target_brk);
+        }
+        bsd_target_brk = new_brk;
+        DEBUGF_BRK(TARGET_ABI_FMT_lx " (new_brk <= brk_page)\n",
+                   bsd_target_brk);
+        return bsd_target_brk;
+    }
+
+    /*
+     * We need to allocate more memory after the brk... Note that we don't use
+     * MAP_FIXED because that will map over the top of any existing mapping
+     * (like the one with the host libc or qemu itself); instead we treat
+     * "mapped but at wrong address" as a failure and unmap again.
+     */
+    new_alloc_size = HOST_PAGE_ALIGN(new_brk - brk_page);
+    mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
+                                        PROT_READ | PROT_WRITE,
+                                        MAP_ANON | MAP_PRIVATE, -1, 0));
+
+    if (mapped_addr == brk_page) {
+        /*
+         * Heap contents are initialized to zero, as for anonymous mapped pages.
+         * Technically the new pages are already initialized to zero since they
+         * *are* anonymous mapped pages, however we have to take care with the
+         * contents that come from the remaining part of the previous page: it
+         * may contains garbage data due to a previous heap usage (grown then
+         * shrunken).
+         */
+        memset(g2h_untagged(bsd_target_brk), 0, brk_page - bsd_target_brk);
+
+        bsd_target_brk = new_brk;
+        brk_page = HOST_PAGE_ALIGN(bsd_target_brk);
+        DEBUGF_BRK(TARGET_ABI_FMT_lx " (mapped_addr == brk_page)\n",
+            bsd_target_brk);
+        return bsd_target_brk;
+    } else if (mapped_addr != -1) {
+        /*
+         * Mapped but at wrong address, meaning there wasn't actually enough
+         * space for this brk.
+         */
+        target_munmap(mapped_addr, new_alloc_size);
+        mapped_addr = -1;
+        DEBUGF_BRK(TARGET_ABI_FMT_lx " (mapped_addr != -1)\n", bsd_target_brk);
+    } else {
+        DEBUGF_BRK(TARGET_ABI_FMT_lx " (otherwise)\n", bsd_target_brk);
+    }
+
+    /* For everything else, return the previous break. */
+    return bsd_target_brk;
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 1db0907504..1b9dca9164 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -543,6 +543,13 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #endif
 
+        /*
+         * Misc
+         */
+    case TARGET_FREEBSD_NR_break:
+        ret = do_obreak(arg1);
+        break;
+
         /*
          * sys{ctl, arch, call}
          */
-- 
2.40.0

Re: [PATCH 17/22] Implement do_obreak function

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 5220 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Co-authored-by: Mikaël Urankar <mikael.urankar@gmail.com>
>
> Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 85 +++++++++++++++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  7 +++
>  2 files changed, 92 insertions(+)
>

emacs works with this, iirc, and it's super picky about brk being
pedantically correct.
This also doesn't match current linux-user. Perhaps the caution it uses is
no longer
needed? I think it's another area we should invite the linux-user
maintainers to
comment.

Warner



> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index edbccd3111..6f33148eb7 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -167,4 +167,89 @@ static inline abi_long do_bsd_mincore(abi_ulong
> target_addr, abi_ulong len,
>      return ret;
>  }
>
> +#ifdef DO_DEBUG
> +#define DEBUGF_BRK(message, args...) \
> +    do { fprintf(stderr, (message), ## args); } while (0)
> +#else
> +#define DEBUGF_BRK(message, args...)
> +#endif
> +
> +/* do_brk() must return target values and target errnos. */
> +static inline abi_long do_obreak(abi_ulong new_brk)
> +{
> +    abi_long mapped_addr;
> +    int new_alloc_size;
> +
> +    DEBUGF_BRK("do_brk(" TARGET_ABI_FMT_lx ") -> ", new_brk);
> +
> +    if (!new_brk) {
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (!new_brk)\n", bsd_target_brk);
> +        return bsd_target_brk;
> +    }
> +    if (new_brk < bsd_target_original_brk) {
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (new_brk <
> bsd_target_original_brk)\n",
> +                   bsd_target_brk);
> +        return bsd_target_brk;
> +    }
> +
> +    /*
> +     * If the new brk is less than the highest page reserved to the
> target heap
> +     * allocation, set it and we're almost done...
> +     */
> +    if (new_brk <= brk_page) {
> +        /*
> +         * Heap contents are initialized to zero, as for anonymous mapped
> pages.
> +         */
> +        if (new_brk > bsd_target_brk) {
> +            memset(g2h_untagged(bsd_target_brk), 0, new_brk -
> bsd_target_brk);
> +        }
> +        bsd_target_brk = new_brk;
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (new_brk <= brk_page)\n",
> +                   bsd_target_brk);
> +        return bsd_target_brk;
> +    }
> +
> +    /*
> +     * We need to allocate more memory after the brk... Note that we
> don't use
> +     * MAP_FIXED because that will map over the top of any existing
> mapping
> +     * (like the one with the host libc or qemu itself); instead we treat
> +     * "mapped but at wrong address" as a failure and unmap again.
> +     */
> +    new_alloc_size = HOST_PAGE_ALIGN(new_brk - brk_page);
> +    mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
> +                                        PROT_READ | PROT_WRITE,
> +                                        MAP_ANON | MAP_PRIVATE, -1, 0));
> +
> +    if (mapped_addr == brk_page) {
> +        /*
> +         * Heap contents are initialized to zero, as for anonymous mapped
> pages.
> +         * Technically the new pages are already initialized to zero
> since they
> +         * *are* anonymous mapped pages, however we have to take care
> with the
> +         * contents that come from the remaining part of the previous
> page: it
> +         * may contains garbage data due to a previous heap usage (grown
> then
> +         * shrunken).
> +         */
> +        memset(g2h_untagged(bsd_target_brk), 0, brk_page -
> bsd_target_brk);
> +
> +        bsd_target_brk = new_brk;
> +        brk_page = HOST_PAGE_ALIGN(bsd_target_brk);
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (mapped_addr == brk_page)\n",
> +            bsd_target_brk);
> +        return bsd_target_brk;
> +    } else if (mapped_addr != -1) {
> +        /*
> +         * Mapped but at wrong address, meaning there wasn't actually
> enough
> +         * space for this brk.
> +         */
> +        target_munmap(mapped_addr, new_alloc_size);
> +        mapped_addr = -1;
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (mapped_addr != -1)\n",
> bsd_target_brk);
> +    } else {
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (otherwise)\n", bsd_target_brk);
> +    }
> +
> +    /* For everything else, return the previous break. */
> +    return bsd_target_brk;
> +}
> +
>  #endif /* BSD_USER_BSD_MEM_H */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 1db0907504..1b9dca9164 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -543,6 +543,13 @@ static abi_long freebsd_syscall(void *cpu_env, int
> num, abi_long arg1,
>          break;
>  #endif
>
> +        /*
> +         * Misc
> +         */
> +    case TARGET_FREEBSD_NR_break:
> +        ret = do_obreak(arg1);
> +        break;
> +
>          /*
>           * sys{ctl, arch, call}
>           */
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 6649 bytes --]

Re: [PATCH 17/22] Implement do_obreak function

[Richard Henderson]

On 8/19/23 02:48, Karim Taha wrote:
> From: Stacey Son <sson@FreeBSD.org>
> 
> Co-authored-by: Mikaël Urankar <mikael.urankar@gmail.com>
> 
> Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.h            | 85 +++++++++++++++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c |  7 +++
>   2 files changed, 92 insertions(+)
> 
> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index edbccd3111..6f33148eb7 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -167,4 +167,89 @@ static inline abi_long do_bsd_mincore(abi_ulong target_addr, abi_ulong len,
>       return ret;
>   }
>   
> +#ifdef DO_DEBUG
> +#define DEBUGF_BRK(message, args...) \
> +    do { fprintf(stderr, (message), ## args); } while (0)
> +#else
> +#define DEBUGF_BRK(message, args...)
> +#endif

We are trying to get rid of all of this old-style debugging.
We are replacing them with tracepoints.

> +
> +/* do_brk() must return target values and target errnos. */
> +static inline abi_long do_obreak(abi_ulong new_brk)
> +{
> +    abi_long mapped_addr;
> +    int new_alloc_size;
> +
> +    DEBUGF_BRK("do_brk(" TARGET_ABI_FMT_lx ") -> ", new_brk);
> +
> +    if (!new_brk) {
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (!new_brk)\n", bsd_target_brk);
> +        return bsd_target_brk;
> +    }
> +    if (new_brk < bsd_target_original_brk) {
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (new_brk < bsd_target_original_brk)\n",
> +                   bsd_target_brk);
> +        return bsd_target_brk;
> +    }

These two cases may be combined, since original_brk will never be 0.

> +
> +    /*
> +     * If the new brk is less than the highest page reserved to the target heap
> +     * allocation, set it and we're almost done...
> +     */
> +    if (new_brk <= brk_page) {
> +        /*
> +         * Heap contents are initialized to zero, as for anonymous mapped pages.
> +         */
> +        if (new_brk > bsd_target_brk) {
> +            memset(g2h_untagged(bsd_target_brk), 0, new_brk - bsd_target_brk);
> +        }
> +        bsd_target_brk = new_brk;
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (new_brk <= brk_page)\n",
> +                   bsd_target_brk);
> +        return bsd_target_brk;
> +    }
> +
> +    /*
> +     * We need to allocate more memory after the brk... Note that we don't use
> +     * MAP_FIXED because that will map over the top of any existing mapping
> +     * (like the one with the host libc or qemu itself); instead we treat
> +     * "mapped but at wrong address" as a failure and unmap again.
> +     */
> +    new_alloc_size = HOST_PAGE_ALIGN(new_brk - brk_page);

We removed all HOST_PAGE_ALIGN from brk in linux-user this cycle.

> +    mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
> +                                        PROT_READ | PROT_WRITE,
> +                                        MAP_ANON | MAP_PRIVATE, -1, 0));
> +
> +    if (mapped_addr == brk_page) {
> +        /*
> +         * Heap contents are initialized to zero, as for anonymous mapped pages.
> +         * Technically the new pages are already initialized to zero since they
> +         * *are* anonymous mapped pages, however we have to take care with the
> +         * contents that come from the remaining part of the previous page: it
> +         * may contains garbage data due to a previous heap usage (grown then
> +         * shrunken).
> +         */
> +        memset(g2h_untagged(bsd_target_brk), 0, brk_page - bsd_target_brk);
> +
> +        bsd_target_brk = new_brk;
> +        brk_page = HOST_PAGE_ALIGN(bsd_target_brk);
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (mapped_addr == brk_page)\n",
> +            bsd_target_brk);
> +        return bsd_target_brk;
> +    } else if (mapped_addr != -1) {
> +        /*
> +         * Mapped but at wrong address, meaning there wasn't actually enough
> +         * space for this brk.
> +         */
> +        target_munmap(mapped_addr, new_alloc_size);
> +        mapped_addr = -1;
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (mapped_addr != -1)\n", bsd_target_brk);
> +    } else {
> +        DEBUGF_BRK(TARGET_ABI_FMT_lx " (otherwise)\n", bsd_target_brk);
> +    }

It may require additional work in target_mmap, but you probably want to use MAP_FIXED | 
MAP_EXCL here.  On the linux-user side we changed to use MAP_FIXED_NOREPLACE, which is 
approximately the same thing.


r~

[PATCH 18/22] Implement shm_open(2)

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Co-authored-by: Kyle Evans <kevans@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 27 +++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  4 ++++
 2 files changed, 31 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index 6f33148eb7..013b82f49a 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -252,4 +252,31 @@ static inline abi_long do_obreak(abi_ulong new_brk)
     return bsd_target_brk;
 }
 
+/* shm_open(2) */
+static inline abi_long do_bsd_shm_open(abi_ulong arg1, abi_long arg2,
+        abi_long arg3)
+{
+    int ret;
+    void *p;
+
+#define SHM_PATH(p) ((p) == SHM_ANON ? (p) : path(p))
+    if (arg1 == (uintptr_t)SHM_ANON) {
+        p = SHM_ANON;
+    } else {
+        p = lock_user_string(arg1);
+        if (p == NULL) {
+            return -TARGET_EFAULT;
+        }
+    }
+    ret = get_errno(shm_open(SHM_PATH(p),
+                target_to_host_bitmask(arg2, fcntl_flags_tbl), arg3));
+
+    if (p != SHM_ANON) {
+        unlock_user(p, arg1, 0);
+    }
+
+    return ret;
+}
+#undef SHM_PATH
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 1b9dca9164..2d15255c20 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -531,6 +531,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_mincore(arg1, arg2, arg3);
         break;
 
+    case TARGET_FREEBSD_NR_freebsd12_shm_open: /* shm_open(2) */
+        ret = do_bsd_shm_open(arg1, arg2, arg3);
+        break;
+
 #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
     case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
         ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);
-- 
2.40.0

Re: [PATCH 18/22] Implement shm_open(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Co-authored-by: Kyle Evans <kevans@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 27 +++++++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  4 ++++
>  2 files changed, 31 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

[-- Attachment #2: Type: text/html, Size: 1073 bytes --]

Re: [PATCH 18/22] Implement shm_open(2)

[Richard Henderson]

On 8/19/23 02:48, Karim Taha wrote:
> +#define SHM_PATH(p) ((p) == SHM_ANON ? (p) : path(p))
> +    if (arg1 == (uintptr_t)SHM_ANON) {
> +        p = SHM_ANON;
> +    } else {
> +        p = lock_user_string(arg1);
> +        if (p == NULL) {
> +            return -TARGET_EFAULT;
> +        }
> +    }
> +    ret = get_errno(shm_open(SHM_PATH(p),

Again, SHM_PATH is not needed, because the condition is handled by this IF.


r~

Re: [PATCH 18/22] Implement shm_open(2)

[Richard Henderson]

On 8/20/23 08:04, Richard Henderson wrote:
> On 8/19/23 02:48, Karim Taha wrote:
>> +#define SHM_PATH(p) ((p) == SHM_ANON ? (p) : path(p))
>> +    if (arg1 == (uintptr_t)SHM_ANON) {
>> +        p = SHM_ANON;
>> +    } else {
>> +        p = lock_user_string(arg1);
>> +        if (p == NULL) {
>> +            return -TARGET_EFAULT;
>> +        }
>> +    }
>> +    ret = get_errno(shm_open(SHM_PATH(p),
> 
> Again, SHM_PATH is not needed, because the condition is handled by this IF.

Oh, no, that's something different.

But path() is wrong for shm_open, because it's not a path in the regular file system.


r~

[PATCH 19/22] Implement shm_unlink(2) and shmget(2)

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 23 +++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  8 ++++++++
 2 files changed, 31 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index 013b82f49a..3d91d3eb30 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -279,4 +279,27 @@ static inline abi_long do_bsd_shm_open(abi_ulong arg1, abi_long arg2,
 }
 #undef SHM_PATH
 
+/* shm_unlink(2) */
+static inline abi_long do_bsd_shm_unlink(abi_ulong arg1)
+{
+    int ret;
+    void *p;
+
+    p = lock_user_string(arg1);
+    if (p == NULL) {
+        return -TARGET_EFAULT;
+    }
+    ret = get_errno(shm_unlink(p)); /* XXX path(p)? */
+    unlock_user(p, arg1, 0);
+
+    return ret;
+}
+
+/* shmget(2) */
+static inline abi_long do_bsd_shmget(abi_long arg1, abi_ulong arg2,
+        abi_long arg3)
+{
+    return get_errno(shmget(arg1, arg2, arg3));
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 2d15255c20..a7db78b9b4 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -547,6 +547,14 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #endif
 
+    case TARGET_FREEBSD_NR_shm_unlink: /* shm_unlink(2) */
+        ret = do_bsd_shm_unlink(arg1);
+        break;
+
+    case TARGET_FREEBSD_NR_shmget: /* shmget(2) */
+        ret = do_bsd_shmget(arg1, arg2, arg3);
+        break;
+
         /*
          * Misc
          */
-- 
2.40.0

Re: [PATCH 19/22] Implement shm_unlink(2) and shmget(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 23 +++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  8 ++++++++
>  2 files changed, 31 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

[-- Attachment #2: Type: text/html, Size: 912 bytes --]

Re: [PATCH 19/22] Implement shm_unlink(2) and shmget(2)

[Richard Henderson]

On 8/19/23 02:48, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.h            | 23 +++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c |  8 ++++++++
>   2 files changed, 31 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 19/22] Implement shm_unlink(2) and shmget(2)

[Richard Henderson]

On 8/19/23 02:48, Karim Taha wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Karim Taha<kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.h            | 23 +++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c |  8 ++++++++
>   2 files changed, 31 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 20/22] Implement shmctl(2)

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 33 +++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  4 ++++
 2 files changed, 37 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index 3d91d3eb30..221ad76d8c 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -302,4 +302,37 @@ static inline abi_long do_bsd_shmget(abi_long arg1, abi_ulong arg2,
     return get_errno(shmget(arg1, arg2, arg3));
 }
 
+/* shmctl(2) */
+static inline abi_long do_bsd_shmctl(abi_long shmid, abi_long cmd,
+        abi_ulong buff)
+{
+    struct shmid_ds dsarg;
+    abi_long ret = -TARGET_EINVAL;
+
+    cmd &= 0xff;
+
+    switch (cmd) {
+    case IPC_STAT:
+    case IPC_SET:
+        if (target_to_host_shmid_ds(&dsarg, buff)) {
+            return -TARGET_EFAULT;
+        }
+        ret = get_errno(shmctl(shmid, cmd, &dsarg));
+        if (host_to_target_shmid_ds(buff, &dsarg)) {
+            return -TARGET_EFAULT;
+        }
+        break;
+
+    case IPC_RMID:
+        ret = get_errno(shmctl(shmid, cmd, NULL));
+        break;
+
+    default:
+        ret = -TARGET_EINVAL;
+        break;
+    }
+
+    return ret;
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index a7db78b9b4..9681c65ce9 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -555,6 +555,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_shmget(arg1, arg2, arg3);
         break;
 
+    case TARGET_FREEBSD_NR_shmctl: /* shmctl(2) */
+        ret = do_bsd_shmctl(arg1, arg2, arg3);
+        break;
+
         /*
          * Misc
          */
-- 
2.40.0

Re: [PATCH 20/22] Implement shmctl(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 33 +++++++++++++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  4 ++++
>  2 files changed, 37 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

[-- Attachment #2: Type: text/html, Size: 926 bytes --]

Re: [PATCH 20/22] Implement shmctl(2)

[Richard Henderson]

On 8/19/23 02:48, Karim Taha wrote:
> +    switch (cmd) {
> +    case IPC_STAT:
> +    case IPC_SET:
> +        if (target_to_host_shmid_ds(&dsarg, buff)) {
> +            return -TARGET_EFAULT;
> +        }
> +        ret = get_errno(shmctl(shmid, cmd, &dsarg));
> +        if (host_to_target_shmid_ds(buff, &dsarg)) {
> +            return -TARGET_EFAULT;
> +        }
> +        break;

IPC_STAT treats buff as output, IPC_SET treats buff as input,
so these cases can't be combined.


r~

Re: [PATCH 20/22] Implement shmctl(2)

[Karim Taha]

Richard Henderson <richard.henderson@linaro.org> wrote:

> On 8/19/23 02:48, Karim Taha wrote:
>> +    switch (cmd) {
>> +    case IPC_STAT:
>> +    case IPC_SET:
>> +        if (target_to_host_shmid_ds(&dsarg, buff)) {
>> +            return -TARGET_EFAULT;
>> +        }
>> +        ret = get_errno(shmctl(shmid, cmd, &dsarg));
>> +        if (host_to_target_shmid_ds(buff, &dsarg)) {
>> +            return -TARGET_EFAULT;
>> +        }
>> +        break;
>
> IPC_STAT treats buff as output, IPC_SET treats buff as input,
> so these cases can't be combined.
>
>
> r~

I think they can be combined:
1- we marshal the struct `buff` from target to host
2- call `shmctl`
3- marshal the results back.

--
Karim Taha

Re: [PATCH 20/22] Implement shmctl(2)

[Richard Henderson]

On 9/8/23 18:59, Karim Taha wrote:
> Richard Henderson <richard.henderson@linaro.org> wrote:
> 
>> On 8/19/23 02:48, Karim Taha wrote:
>>> +    switch (cmd) {
>>> +    case IPC_STAT:
>>> +    case IPC_SET:
>>> +        if (target_to_host_shmid_ds(&dsarg, buff)) {
>>> +            return -TARGET_EFAULT;
>>> +        }
>>> +        ret = get_errno(shmctl(shmid, cmd, &dsarg));
>>> +        if (host_to_target_shmid_ds(buff, &dsarg)) {
>>> +            return -TARGET_EFAULT;
>>> +        }
>>> +        break;
>>
>> IPC_STAT treats buff as output, IPC_SET treats buff as input,
>> so these cases can't be combined.
>>
>>
>> r~
> 
> I think they can be combined:
> 1- we marshal the struct `buff` from target to host
> 2- call `shmctl`
> 3- marshal the results back.

No.  For IPC_SET, the target page need not be writable, as you assume here.


r~

[PATCH 21/22] Implement shmat(2) and shmdt(2)

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 72 +++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  8 ++++
 2 files changed, 80 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index 221ad76d8c..f737b94885 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -335,4 +335,76 @@ static inline abi_long do_bsd_shmctl(abi_long shmid, abi_long cmd,
     return ret;
 }
 
+/* shmat(2) */
+static inline abi_long do_bsd_shmat(int shmid, abi_ulong shmaddr, int shmflg)
+{
+    abi_ulong raddr;
+    abi_long ret;
+    void *host_raddr;
+    struct shmid_ds shm_info;
+    int i;
+
+    /* Find out the length of the shared memory segment. */
+    ret = get_errno(shmctl(shmid, IPC_STAT, &shm_info));
+    if (is_error(ret)) {
+        /* Can't get the length */
+        return ret;
+    }
+
+    mmap_lock();
+
+    if (shmaddr) {
+        host_raddr = shmat(shmid, (void *)g2h_untagged(shmaddr), shmflg);
+    } else {
+        abi_ulong mmap_start;
+
+        mmap_start = mmap_find_vma(0, shm_info.shm_segsz);
+
+        if (mmap_start == -1) {
+            errno = ENOMEM;
+            host_raddr = (void *)-1;
+        } else {
+            host_raddr = shmat(shmid, g2h_untagged(mmap_start),
+                shmflg); /* | SHM_REMAP XXX WHY? */
+        }
+    }
+
+    if (host_raddr == (void *)-1) {
+        mmap_unlock();
+        return get_errno((long)host_raddr);
+    }
+    raddr = h2g((unsigned long)host_raddr);
+
+    page_set_flags(raddr, raddr + shm_info.shm_segsz,
+        PAGE_VALID | PAGE_READ | ((shmflg & SHM_RDONLY) ? 0 : PAGE_WRITE));
+
+    for (i = 0; i < N_BSD_SHM_REGIONS; i++) {
+        if (bsd_shm_regions[i].start == 0) {
+            bsd_shm_regions[i].start = raddr;
+            bsd_shm_regions[i].size = shm_info.shm_segsz;
+            break;
+        }
+    }
+
+    mmap_unlock();
+    return raddr;
+}
+
+/* shmdt(2) */
+static inline abi_long do_bsd_shmdt(abi_ulong shmaddr)
+{
+    int i;
+
+    for (i = 0; i < N_BSD_SHM_REGIONS; ++i) {
+        if (bsd_shm_regions[i].start == shmaddr) {
+            bsd_shm_regions[i].start = 0;
+            page_set_flags(shmaddr,
+                shmaddr + bsd_shm_regions[i].size, 0);
+            break;
+        }
+    }
+
+    return get_errno(shmdt(g2h_untagged(shmaddr)));
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 9681c65ce9..f76bc1eb38 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -559,6 +559,14 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_shmctl(arg1, arg2, arg3);
         break;
 
+    case TARGET_FREEBSD_NR_shmat: /* shmat(2) */
+        ret = do_bsd_shmat(arg1, arg2, arg3);
+        break;
+
+    case TARGET_FREEBSD_NR_shmdt: /* shmdt(2) */
+        ret = do_bsd_shmdt(arg1);
+        break;
+
         /*
          * Misc
          */
-- 
2.40.0

Re: [PATCH 21/22] Implement shmat(2) and shmdt(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 3682 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 72 +++++++++++++++++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c |  8 ++++
>  2 files changed, 80 insertions(+)
>
> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index 221ad76d8c..f737b94885 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -335,4 +335,76 @@ static inline abi_long do_bsd_shmctl(abi_long shmid,
> abi_long cmd,
>      return ret;
>  }
>
> +/* shmat(2) */
> +static inline abi_long do_bsd_shmat(int shmid, abi_ulong shmaddr, int
> shmflg)
> +{
> +    abi_ulong raddr;
> +    abi_long ret;
> +    void *host_raddr;
> +    struct shmid_ds shm_info;
> +    int i;
> +
> +    /* Find out the length of the shared memory segment. */
> +    ret = get_errno(shmctl(shmid, IPC_STAT, &shm_info));
> +    if (is_error(ret)) {
> +        /* Can't get the length */
> +        return ret;
> +    }
> +
> +    mmap_lock();
> +
> +    if (shmaddr) {
> +        host_raddr = shmat(shmid, (void *)g2h_untagged(shmaddr), shmflg);
> +    } else {
> +        abi_ulong mmap_start;
> +
> +        mmap_start = mmap_find_vma(0, shm_info.shm_segsz);
> +
> +        if (mmap_start == -1) {
> +            errno = ENOMEM;
> +            host_raddr = (void *)-1;
> +        } else {
> +            host_raddr = shmat(shmid, g2h_untagged(mmap_start),
> +                shmflg); /* | SHM_REMAP XXX WHY? */
>

I was all set to hit reviewed by on this, but this has me curious. Kyle (or
anybody else) do
you know the back story here. git blame is less than helpful.

Warner


> +        }
> +    }
> +
> +    if (host_raddr == (void *)-1) {
> +        mmap_unlock();
> +        return get_errno((long)host_raddr);
> +    }
> +    raddr = h2g((unsigned long)host_raddr);
> +
> +    page_set_flags(raddr, raddr + shm_info.shm_segsz,
> +        PAGE_VALID | PAGE_READ | ((shmflg & SHM_RDONLY) ? 0 :
> PAGE_WRITE));
> +
> +    for (i = 0; i < N_BSD_SHM_REGIONS; i++) {
> +        if (bsd_shm_regions[i].start == 0) {
> +            bsd_shm_regions[i].start = raddr;
> +            bsd_shm_regions[i].size = shm_info.shm_segsz;
> +            break;
> +        }
> +    }
> +
> +    mmap_unlock();
> +    return raddr;
> +}
> +
> +/* shmdt(2) */
> +static inline abi_long do_bsd_shmdt(abi_ulong shmaddr)
> +{
> +    int i;
> +
> +    for (i = 0; i < N_BSD_SHM_REGIONS; ++i) {
> +        if (bsd_shm_regions[i].start == shmaddr) {
> +            bsd_shm_regions[i].start = 0;
> +            page_set_flags(shmaddr,
> +                shmaddr + bsd_shm_regions[i].size, 0);
> +            break;
> +        }
> +    }
> +
> +    return get_errno(shmdt(g2h_untagged(shmaddr)));
> +}
> +
>  #endif /* BSD_USER_BSD_MEM_H */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 9681c65ce9..f76bc1eb38 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -559,6 +559,14 @@ static abi_long freebsd_syscall(void *cpu_env, int
> num, abi_long arg1,
>          ret = do_bsd_shmctl(arg1, arg2, arg3);
>          break;
>
> +    case TARGET_FREEBSD_NR_shmat: /* shmat(2) */
> +        ret = do_bsd_shmat(arg1, arg2, arg3);
> +        break;
> +
> +    case TARGET_FREEBSD_NR_shmdt: /* shmdt(2) */
> +        ret = do_bsd_shmdt(arg1);
> +        break;
> +
>          /*
>           * Misc
>           */
> --
> 2.40.0
>
>

[-- Attachment #2: Type: text/html, Size: 4740 bytes --]

Re: [PATCH 21/22] Implement shmat(2) and shmdt(2)

[Richard Henderson]

On 8/19/23 02:48, Karim Taha wrote:
> From: Stacey Son <sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.h            | 72 +++++++++++++++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c |  8 ++++
>   2 files changed, 80 insertions(+)
> 
> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index 221ad76d8c..f737b94885 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -335,4 +335,76 @@ static inline abi_long do_bsd_shmctl(abi_long shmid, abi_long cmd,
>       return ret;
>   }
>   
> +/* shmat(2) */
> +static inline abi_long do_bsd_shmat(int shmid, abi_ulong shmaddr, int shmflg)
> +{
> +    abi_ulong raddr;
> +    abi_long ret;
> +    void *host_raddr;
> +    struct shmid_ds shm_info;
> +    int i;
> +
> +    /* Find out the length of the shared memory segment. */
> +    ret = get_errno(shmctl(shmid, IPC_STAT, &shm_info));
> +    if (is_error(ret)) {
> +        /* Can't get the length */
> +        return ret;
> +    }
> +
> +    mmap_lock();
> +
> +    if (shmaddr) {
> +        host_raddr = shmat(shmid, (void *)g2h_untagged(shmaddr), shmflg);

Missing

     if (!guest_range_valid_untagged(shmaddr, shm_info.shm_segsz)) {
         return -TARGET_EINVAL;
     }

> +    } else {
> +        abi_ulong mmap_start;
> +
> +        mmap_start = mmap_find_vma(0, shm_info.shm_segsz);
> +
> +        if (mmap_start == -1) {
> +            errno = ENOMEM;
> +            host_raddr = (void *)-1;
> +        } else {
> +            host_raddr = shmat(shmid, g2h_untagged(mmap_start),
> +                shmflg); /* | SHM_REMAP XXX WHY? */

With reserved_va, the entire guest address space is mapped PROT_NONE so that it is 
reserved, so that the kernel does not use it for something else.  You need the SHM_REMAP 
to replace the reservation mapping.

> +/* shmdt(2) */
> +static inline abi_long do_bsd_shmdt(abi_ulong shmaddr)
> +{
> +    int i;
> +
> +    for (i = 0; i < N_BSD_SHM_REGIONS; ++i) {
> +        if (bsd_shm_regions[i].start == shmaddr) {
> +            bsd_shm_regions[i].start = 0;
> +            page_set_flags(shmaddr,
> +                shmaddr + bsd_shm_regions[i].size, 0);
> +            break;
> +        }
> +    }
> +
> +    return get_errno(shmdt(g2h_untagged(shmaddr)));
> +}

Hmm, bug with linux-user as well, because here we should re-establish the reserved_va 
reservation.

Also, we should not be using a fixed sized array.  Nothing good happens when the array 
fills up.


r~

Re: [PATCH 21/22] Implement shmat(2) and shmdt(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 3202 bytes --]

On Sun, Aug 20, 2023 at 9:30 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 8/19/23 02:48, Karim Taha wrote:
> > From: Stacey Son <sson@FreeBSD.org>
> >
> > Signed-off-by: Stacey Son <sson@FreeBSD.org>
> > Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> > ---
> >   bsd-user/bsd-mem.h            | 72 +++++++++++++++++++++++++++++++++++
> >   bsd-user/freebsd/os-syscall.c |  8 ++++
> >   2 files changed, 80 insertions(+)
> >
> > diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> > index 221ad76d8c..f737b94885 100644
> > --- a/bsd-user/bsd-mem.h
> > +++ b/bsd-user/bsd-mem.h
> > @@ -335,4 +335,76 @@ static inline abi_long do_bsd_shmctl(abi_long
> shmid, abi_long cmd,
> >       return ret;
> >   }
> >
> > +/* shmat(2) */
> > +static inline abi_long do_bsd_shmat(int shmid, abi_ulong shmaddr, int
> shmflg)
> > +{
> > +    abi_ulong raddr;
> > +    abi_long ret;
> > +    void *host_raddr;
> > +    struct shmid_ds shm_info;
> > +    int i;
> > +
> > +    /* Find out the length of the shared memory segment. */
> > +    ret = get_errno(shmctl(shmid, IPC_STAT, &shm_info));
> > +    if (is_error(ret)) {
> > +        /* Can't get the length */
> > +        return ret;
> > +    }
> > +
> > +    mmap_lock();
> > +
> > +    if (shmaddr) {
> > +        host_raddr = shmat(shmid, (void *)g2h_untagged(shmaddr),
> shmflg);
>
> Missing
>
>      if (!guest_range_valid_untagged(shmaddr, shm_info.shm_segsz)) {
>          return -TARGET_EINVAL;
>      }
>
> > +    } else {
> > +        abi_ulong mmap_start;
> > +
> > +        mmap_start = mmap_find_vma(0, shm_info.shm_segsz);
> > +
> > +        if (mmap_start == -1) {
> > +            errno = ENOMEM;
> > +            host_raddr = (void *)-1;
> > +        } else {
> > +            host_raddr = shmat(shmid, g2h_untagged(mmap_start),
> > +                shmflg); /* | SHM_REMAP XXX WHY? */
>
> With reserved_va, the entire guest address space is mapped PROT_NONE so
> that it is
> reserved, so that the kernel does not use it for something else.  You need
> the SHM_REMAP
> to replace the reservation mapping.
>
> > +/* shmdt(2) */
> > +static inline abi_long do_bsd_shmdt(abi_ulong shmaddr)
> > +{
> > +    int i;
> > +
> > +    for (i = 0; i < N_BSD_SHM_REGIONS; ++i) {
> > +        if (bsd_shm_regions[i].start == shmaddr) {
> > +            bsd_shm_regions[i].start = 0;
> > +            page_set_flags(shmaddr,
> > +                shmaddr + bsd_shm_regions[i].size, 0);
> > +            break;
> > +        }
> > +    }
> > +
> > +    return get_errno(shmdt(g2h_untagged(shmaddr)));
> > +}
>
> Hmm, bug with linux-user as well, because here we should re-establish the
> reserved_va
> reservation.
>

... of the shared memory region we just detached? Right?


> Also, we should not be using a fixed sized array.  Nothing good happens
> when the array
> fills up.
>

File this as https://github.com/qemu-bsd-user/qemu-bsd-user/issues/47 so we
don't forget.
It's good enough for the moment since the programs we've seen have a very
limited number
of segments... but longer term, it should be dynamic.

Warner

[-- Attachment #2: Type: text/html, Size: 4411 bytes --]

Re: [PATCH 21/22] Implement shmat(2) and shmdt(2)

[Richard Henderson]

On 8/22/23 11:03, Warner Losh wrote:
>     Hmm, bug with linux-user as well, because here we should re-establish the reserved_va
>     reservation.
> 
> 
> ... of the shared memory region we just detached? Right?

Correct.

On a related note, on FreeBSD is there any practical difference between

     PROT_NONE, MAP_ANON
and
     PROT_NONE, MAP_GUARD

for large memory regions?

I ask since FreeBSD doesn't have MAP_NORESERVE, which Linux uses to avoid allocation of 
gigabytes.


r~

Re: [PATCH 21/22] Implement shmat(2) and shmdt(2)

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2046 bytes --]

On Tue, Aug 22, 2023 at 12:11 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 8/22/23 11:03, Warner Losh wrote:
> >     Hmm, bug with linux-user as well, because here we should
> re-establish the reserved_va
> >     reservation.
> >
> >
> > ... of the shared memory region we just detached? Right?
>
> Correct.
>
> On a related note, on FreeBSD is there any practical difference between
>
>      PROT_NONE, MAP_ANON
> and
>      PROT_NONE, MAP_GUARD
>
> for large memory regions?
>

They do different things. MAP_ANON maps the memory without a backing
device. This means it allocates the VA space right away, but lazily
allocates
the backing pages as the pages are dirtied.

MAP_GUARD creates the VA mapping, but never maps any pages to those
pages (well, until it's remapped). Any read/write/exec access to MAP_GUARD
pages results in a SIGSEGV.


> I ask since FreeBSD doesn't have MAP_NORESERVE, which Linux uses to avoid
> allocation of
> gigabytes
>

Yea. It sounds like MAP_NORESERVE is what FreeBSD's default behavior is: We
don't
allocate backing store in the swap areas until there's memory pressure. You
can safely
allocate GB of space with MAP_ANON and get similar behavior to the
MAP_NORESERVE.

MAP_GUARD could be used if you wanted to reserve the VA space, but didn't
want to assign
anything to the VA space until later.

As a practical matter, they both consume about the same resources until the
MAP_ANON
region starts to get populated with data...

With PROT_NONE, I think they would have the same effect. If it is to be a
backing store for
something like malloc, then MAP_ANON would be best. If you are replacing it
with a lot of
things, like a mix of files, devices and/or anon memory, then MAP_GUARD and
replace it
with MAP_FIXED later. Most likely you'll want MAP_GUARD to reserve the
area, and then
MAP_FIXED to use it for mmap'd memory, shared memory, executable pages, etc.

Does that tell you what you need to know?

Warner


> r~
>

[-- Attachment #2: Type: text/html, Size: 3006 bytes --]

Re: [PATCH 21/22] Implement shmat(2) and shmdt(2)

[Richard Henderson]

On 8/22/23 12:54, Warner Losh wrote:
> As a practical matter, they both consume about the same resources until the MAP_ANON
> region starts to get populated with data...
> 
> With PROT_NONE, I think they would have the same effect. If it is to be a backing store for
> something like malloc, then MAP_ANON would be best. If you are replacing it with a lot of
> things, like a mix of files, devices and/or anon memory, then MAP_GUARD and replace it
> with MAP_FIXED later. Most likely you'll want MAP_GUARD to reserve the area, and then
> MAP_FIXED to use it for mmap'd memory, shared memory, executable pages, etc.
> 
> Does that tell you what you need to know?

Yes.

The reserved_va area is replaced with a mix of files, anon, etc, based on whatever the 
guest requires.  So it might be reasonable to adjust bsd-user/mmap.c to use MAP_GUARD for 
managing the reserved_va area instead of MAP_ANON.  No rush, of course.


r~

[PATCH 22/22] Add stubs for vadvise(), sbrk() and sstk()

[Karim Taha]

From: Stacey Son <sson@FreeBSD.org>

The above system calls are not supported by qemu.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
---
 bsd-user/bsd-mem.h            | 21 +++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c | 12 ++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index f737b94885..274178bef7 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -407,4 +407,25 @@ static inline abi_long do_bsd_shmdt(abi_ulong shmaddr)
     return get_errno(shmdt(g2h_untagged(shmaddr)));
 }
 
+static inline abi_long do_bsd_vadvise(void)
+{
+    /* See sys_ovadvise() in vm_unix.c */
+    qemu_log("qemu: Unsupported syscall vadvise()\n");
+    return -TARGET_ENOSYS;
+}
+
+static inline abi_long do_bsd_sbrk(void)
+{
+    /* see sys_sbrk() in vm_mmap.c */
+    qemu_log("qemu: Unsupported syscall sbrk()\n");
+    return -TARGET_ENOSYS;
+}
+
+static inline abi_long do_bsd_sstk(void)
+{
+    /* see sys_sstk() in vm_mmap.c */
+    qemu_log("qemu: Unsupported syscall sstk()\n");
+    return -TARGET_ENOSYS;
+}
+
 #endif /* BSD_USER_BSD_MEM_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index f76bc1eb38..cf4b894fee 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -567,6 +567,18 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_shmdt(arg1);
         break;
 
+    case TARGET_FREEBSD_NR_freebsd11_vadvise:
+        ret = do_bsd_vadvise();
+        break;
+
+    case TARGET_FREEBSD_NR_sbrk:
+        ret = do_bsd_sbrk();
+        break;
+
+    case TARGET_FREEBSD_NR_sstk:
+        ret = do_bsd_sstk();
+        break;
+
         /*
          * Misc
          */
-- 
2.40.0

Re: [PATCH 22/22] Add stubs for vadvise(), sbrk() and sstk()

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

On Sat, Aug 19, 2023 at 3:49 AM Karim Taha <kariem.taha2.7@gmail.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> The above system calls are not supported by qemu.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>  bsd-user/bsd-mem.h            | 21 +++++++++++++++++++++
>  bsd-user/freebsd/os-syscall.c | 12 ++++++++++++
>  2 files changed, 33 insertions(+)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

[-- Attachment #2: Type: text/html, Size: 974 bytes --]

Re: [PATCH 22/22] Add stubs for vadvise(), sbrk() and sstk()

[Richard Henderson]

On 8/19/23 02:48, Karim Taha wrote:
> From: Stacey Son <sson@FreeBSD.org>
> 
> The above system calls are not supported by qemu.
> 
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.h            | 21 +++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c | 12 ++++++++++++
>   2 files changed, 33 insertions(+)
> 
> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index f737b94885..274178bef7 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -407,4 +407,25 @@ static inline abi_long do_bsd_shmdt(abi_ulong shmaddr)
>       return get_errno(shmdt(g2h_untagged(shmaddr)));
>   }
>   
> +static inline abi_long do_bsd_vadvise(void)
> +{
> +    /* See sys_ovadvise() in vm_unix.c */
> +    qemu_log("qemu: Unsupported syscall vadvise()\n");
> +    return -TARGET_ENOSYS;
> +}

I see EINVAL not ENOSYS.

> +static inline abi_long do_bsd_sbrk(void)
> +{
> +    /* see sys_sbrk() in vm_mmap.c */
> +    qemu_log("qemu: Unsupported syscall sbrk()\n");
> +    return -TARGET_ENOSYS;
> +}
> +
> +static inline abi_long do_bsd_sstk(void)
> +{
> +    /* see sys_sstk() in vm_mmap.c */
> +    qemu_log("qemu: Unsupported syscall sstk()\n");
> +    return -TARGET_ENOSYS;
> +}

I see EOPNOTSUPP not ENOSYS.

I don't see any point in logging these.


r~

Re: [PATCH 22/22] Add stubs for vadvise(), sbrk() and sstk()

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]

On Sun, Aug 20, 2023 at 9:35 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 8/19/23 02:48, Karim Taha wrote:
> > From: Stacey Son <sson@FreeBSD.org>
> >
> > The above system calls are not supported by qemu.
> >
> > Signed-off-by: Stacey Son <sson@FreeBSD.org>
> > Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> > ---
> >   bsd-user/bsd-mem.h            | 21 +++++++++++++++++++++
> >   bsd-user/freebsd/os-syscall.c | 12 ++++++++++++
> >   2 files changed, 33 insertions(+)
> >
> > diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> > index f737b94885..274178bef7 100644
> > --- a/bsd-user/bsd-mem.h
> > +++ b/bsd-user/bsd-mem.h
> > @@ -407,4 +407,25 @@ static inline abi_long do_bsd_shmdt(abi_ulong
> shmaddr)
> >       return get_errno(shmdt(g2h_untagged(shmaddr)));
> >   }
> >
> > +static inline abi_long do_bsd_vadvise(void)
> > +{
> > +    /* See sys_ovadvise() in vm_unix.c */
> > +    qemu_log("qemu: Unsupported syscall vadvise()\n");
> > +    return -TARGET_ENOSYS;
> > +}
>
> I see EINVAL not ENOSYS.
>

When the system call isn't present at all, it's ENOSYS + SIGSYS (I have
patches to implement this
that developers love, but users hate it since many programs cope OK when an
error is returned for
obscure system calls). When it is present, it's implemented as EINVAL. So
maybe the right thing
here is to remove the log and just return EINVAL for the implementation.


> > +static inline abi_long do_bsd_sbrk(void)
> > +{
> > +    /* see sys_sbrk() in vm_mmap.c */
> > +    qemu_log("qemu: Unsupported syscall sbrk()\n");
> > +    return -TARGET_ENOSYS;
> > +}
> > +
> > +static inline abi_long do_bsd_sstk(void)
> > +{
> > +    /* see sys_sstk() in vm_mmap.c */
> > +    qemu_log("qemu: Unsupported syscall sstk()\n");
> > +    return -TARGET_ENOSYS;
> > +}
>
> I see EOPNOTSUPP not ENOSYS.
>

Same comment as above: we should just return EOPNOSUPP and call it
implemented.


> I don't see any point in logging these.
>

Yea, that's a general pattern that we have upstream, but there's a
catch-all 'default' case that does
the logging with the right mask.

Warner

[-- Attachment #2: Type: text/html, Size: 3278 bytes --]