6.828 Lab syscall - System calls

• https://pdos.csail.mit.edu/6.828/2020/labs/syscall.html
• Req
  • XV6 Book
    • Chapter 2
    • Section 4.3 & 4.4
  • The user-space code for systems calls is in user/user.h and user/usys.pl.
  • The kernel-space code is kernel/syscall.h, kernel/syscall.c.
  • The process-related code is kernel/proc.h and kernel/proc.c.

System call tracing

• This requires that the kernel remember whether or not a given process has called trace, and on what syscalls, so at the very least this will require adding an entry to the process state, probably just containing the mask.
• Calling trace updates the mask, and syscall.c checks the mask before executing a syscall.
• proc.h marks some bits of the process state as " p->lock must be held when using these"; why?
• user.h defines the syscall stub implemented by usys.pl, and kernel/defs.h defines the syscall implementation in proc.c.
• This is actually working! 😮

Sysinfo

• The hints indicate that the recommended way to do this is to copy the struct from user memory to kernel memory, modify it, and copy it back. That sounds ok, but why not have the kernel simply modify the struct directly in user memory?

  • I tried this to start with, but the change isn’t “persisted” to user space:

    uint64
    sys_sysinfo(void)
    {
      uint64 ip;
      argaddr(0, &ip);
      printf("Virt Addr: %p\n", ip);
    
      uint64 phy = walkaddr(myproc()->pagetable, ip);
      printf("Phy Addr: %p\n", phy);
    
      if (phy == 0) {
    	return -1;
      }
    
      struct sysinfo *info = (struct sysinfo *) phy;
    
      info->freemem = 20000;
      info->nproc = 2;
    
      // sysinfo(&s);
      return 0;
    }
    

  • Either my virtual->physical address translation is incorrect, or there’s a more fundamental issue that makes it difficult for the kernel to directly modify user memory.

  • I’m going to try and use gdb to inspect that section of memory.

    • Setting a breakpoint is surprisingly easy. In this case (^^^^) it’s sufficient to just run b sys_sysinfo
    • I’m running into this issue with gdb; going to switch to Linux/Arch instead of debugging this
    • I’m still seeing “corrupted DWARF expression” on both OSes, but only on some expressions. Leaving this here for now.

  • I looked at the implementation of copyin a bit more and figured this out

    • The key issue is that walkaddr returns the start address of the translated page, so you have to add the offset in manually
    • Replacing the translation code above with this gets it working:

        uint64 virt_page_start = PGROUNDDOWN(ip);
        uint64 phy_page_start = walkaddr(myproc()->pagetable, virt_page_start);
      
        if (phy_page_start == 0) {
      	return -1;
        }
      
        uint64 phy = phy_page_start + (ip - virt_page_start);
        printf("Phy Addr: %p\n", phy);
      

• Implemented at https://github.com/timothyandrew/6.828/commit/5cdcd28a