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

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 18.03.20 00:08, Simon Kuenzer wrote:
> 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
> > +
I forgot to mention that mode_t should be k_mode_t because of the same 
API-definition and libc mismatch potential. You should probably create 
another header: linuxu/mode.h and define the macro K_O_RDONLY also there.
> > +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;
Move k_mode_t to linuxu/mode.h and include that header in this file, too.

>      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
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel