Re: [Minios-devel] [UNIKRAFT PATCH 1/5] plat/linuxu: Add fstat and open host system calls

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 28.01.20 05:01, Robert Hrusecky wrote:
> Add system call number for arm_32 and x86_64 for the fstat system call
> Implement system call wrapper function for open and fstat
> Add MAP_PRIVATE constant for use with mmap
>
> Signed-off-by: Robert Hrusecky <roberth@xxxxxxxxxxxxx>
> Signed-off-by: Omar Jamil <omarj2898@xxxxxxxxx>
> Signed-off-by: Sachin Beldona <sachinbeldona@xxxxxxxxxx>
> ---
>   plat/linuxu/include/linuxu/syscall-arm_32.h |  1 +
>   plat/linuxu/include/linuxu/syscall-x86_64.h |  1 +
>   plat/linuxu/include/linuxu/syscall.h        | 15 +++++++++++++++
>   3 files changed, 17 insertions(+)
>
> diff --git a/plat/linuxu/include/linuxu/syscall-arm_32.h 
> b/plat/linuxu/include/linuxu/syscall-arm_32.h
> index ef9323b..5d1b1e2 100644
> --- a/plat/linuxu/include/linuxu/syscall-arm_32.h
> +++ b/plat/linuxu/include/linuxu/syscall-arm_32.h
> @@ -46,6 +46,7 @@
>   #define __SC_MUNMAP    91
>   #define __SC_EXIT       1
>   #define __SC_IOCTL     54
> +#define __SC_FSTAT    108
>   #define __SC_RT_SIGPROCMASK   126
>   #define __SC_ARCH_PRCTL       172
>   #define __SC_RT_SIGACTION     174
> diff --git a/plat/linuxu/include/linuxu/syscall-x86_64.h 
> b/plat/linuxu/include/linuxu/syscall-x86_64.h
> index 553f0ba..0dd280e 100644
> --- a/plat/linuxu/include/linuxu/syscall-x86_64.h
> +++ b/plat/linuxu/include/linuxu/syscall-x86_64.h
> @@ -42,6 +42,7 @@
>   #define __SC_WRITE   1
>   #define __SC_OPEN    2
>   #define __SC_CLOSE   3
> +#define __SC_FSTAT   5
>   #define __SC_MMAP    9
>   #define __SC_MUNMAP 11
>   #define __SC_RT_SIGACTION   13
> diff --git a/plat/linuxu/include/linuxu/syscall.h 
> b/plat/linuxu/include/linuxu/syscall.h
> index 0dca7c5..c703b32 100644
> --- a/plat/linuxu/include/linuxu/syscall.h
> +++ b/plat/linuxu/include/linuxu/syscall.h
> @@ -48,6 +48,13 @@
>   #error "Unsupported architecture"
>   #endif
>   
> +
> +static inline int sys_open(const char *pathname, int flags, mode_t mode)
> +{
> +       return (int)syscall3(__SC_OPEN, (long)pathname, (long)flags,
> +                            (long)mode);
> +}
> +
You should also add a defintion for sys_close(). I saw in patch 2 that 
you are going to need it.
>   static inline ssize_t sys_read(int fd, const char *buf, size_t len)
>   {
>         return (ssize_t) syscall3(__SC_READ,
> @@ -64,6 +71,13 @@ static inline ssize_t sys_write(int fd, const char *buf, 
> size_t len)
>                                   (long) (len));
>   }
>   
> +struct stat;
Try to give a full definition of the struct as linuxu platform internal 
version. Currently `struct stat` relies on the Unikraft-internal libc 
defintion which might be different to the Linux ABI. We used the 
convention to prefix such linux-internal data types with `k_` (see 
plat/linuxu/include/linuxu/time.h). So, I would create similar to time.h 
a header in plat/linuxu/include/linuxu/stat.h and define `struct 
k_stat`. You should also define there all needed (and not yet available) 
datatypes (like blksize_t) as k_blksize_t. You can copy missing 
declarations from lib/nolibc/include/nolibc-internal/shareddefs.h (I 
hope this list is complete):
        #include <linuxu/time.h>

        typedef unsigned k_id_t;
        typedef __u64    k_dev_t;
        typedef __u64    k_ino_t;
        typedef unsigned k_mode_t;
        typedef unsigned k_uid_t;
        typedef unsigned k_gid_t;
        typedef __u32    k_nlink_t;
        typedef __u32    k_nlink_t;
        typedef long     k_blksize_t;
        typedef __s64    k_blkcnt_t;
        typedef __off    k_off_t;

k_timespec_t is already defined in linuxu/time.h and should be covered 
with the extra include.
You would get the following struct declaration if you copy it from 
nolibc. But please double-check that this one is inline with Linux's UAPI.
struct k_stat {
        k_dev_t st_dev;
        k_ino_t st_ino;
        k_nlink_t st_nlink;

        k_mode_t st_mode;
        k_uid_t st_uid;
        k_gid_t st_gid;
        unsigned int    __pad0;
        k_dev_t st_rdev;
        k_off_t st_size;
        k_blksize_t st_blksize;
        k_blkcnt_t st_blocks;

        struct k_timespec st_atim;
        struct k_timespec st_mtim;
        struct k_timespec st_ctim;
};

I actually found this because I got compile errors when I tried to do 
the most minimal build with linuxu. I only selected linuxu platform and 
switch every library off that was optional under `Library 
Configuration`, except ukboot and uklibparam. This leads to the 
following two compile errors:
        nolibc does not declare `struct timespec` with uktime.
        close is not defined when vfscore is not enabled.

> +
> +static inline int sys_fstat(int fd, struct stat *statbuf)
Change this one then to:

static inline int sys_fstat(int fd, struct k_stat *statbuf)

> +{
> +       return (int)syscall2(__SC_FSTAT, (long)(fd), (long)(statbuf));
> +}
> +
>   static inline int sys_exit(int status)
>   {
>         return (int) syscall1(__SC_EXIT,
> @@ -81,6 +95,7 @@ static inline int sys_clock_gettime(k_clockid_t clk_id, 
> struct k_timespec *tp)
>    * Please note that on failure sys_mmap() is returning -errno
>    */
>   #define MAP_SHARED    (0x01)
> +#define MAP_PRIVATE   (0x02)
>   #define MAP_ANONYMOUS (0x20)
>   #define PROT_NONE     (0x0)
>   #define PROT_READ     (0x1)
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel