Merge mit pdos [4] (related to #261)

[kimjungwow]

Rust 코드에 반영되었는지 한 번 더 확인 후 리뷰 요청 드리겠습니다 (related to #261 )

• mit-pdos/xv6-riscv@db0f092ae44f85db450718588c2deea080c27d0e: fix printf() in interrupts
  • Uart::putc_sync() 생성 + Uart::putc()의 bool 타입 인자 제거 -> Rust 반영 완료
• mit-pdos/xv6-riscv@050a696 : defines for UART register bits
  • uart.c에서 하드 코딩된 값을 사용하는 대신, define으로 설정함 -> uart.rs에 반영 완료
• mit-pdos/xv6-riscv@6c3099d : Change tf -> trapframe in a few comments
  • trampoline.S에만 적용되어 Rust 해당 사항 없음
• mit-pdos/xv6-riscv@8b9b799 : modify each page in usertests countfree()
  • vm.c의 walk(), freewalk()에서 static 제거 -> Rust는 해당 사항 없음 (pub 함수도 아님)
• mit-pdos/xv6-riscv@f01b1a8 : test pointer checking in copyin, copyout, copyinstr
  • usertests.c 변경 : Rust 해당 사항 없음
• mit-pdos/xv6-riscv@354adfd : streamline copyin/copyout code in usertests
  • console, inode, pipe의 read/write에서 에러 있을 때 return value 관련 버그 수정..
• mit-pdos/xv6-riscv@234391b : test copyinstr()'s handling of the terminating null
  • usertests.c 변경 : Rust 해당 사항 없음
• mit-pdos/xv6-riscv@6cb6764 : test string system call arguments that cross over the end of the last…
  • usertests.c 변경 : Rust 해당 사항 없음   
• aa902bb : [All usertests passed] To pass usertest copyin
  • copyin usertest 통과하기 위해 File::write, InodeGuard::write도 차후의 PR (mit-pdos/xv6-riscv@5e392531c07966fd8a6bee50e3e357c553fb2a2f)을 따라 미리 변경 (주석도 적었습니다)
• mit-pdos/xv6-riscv@897f6f3 : yet another toolchain name
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료
• mit-pdos/xv6-riscv@b62d4d4 : more grind
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료
• mit-pdos/xv6-riscv@672217a : allow more files 
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료
• mit-pdos/xv6-riscv@af9eb91 : make "echo hello > x" truncate file x.
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료
• mit-pdos/xv6-riscv@82981fa : drop QEMUEXTRA
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료
• mit-pdos/xv6-riscv@5494c91 : rename p->tf to p->trapframe, for consistency with p->context
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료
• mit-pdos/xv6-riscv@1e72d5c : cpu->scheduler -> cpu->context to reduce confusion
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료   
• mit-pdos/xv6-riscv@27057bc : interrupt-driven uart output, hopefully a nice example for teaching.
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료   
• mit-pdos/xv6-riscv@0f50e95 : fix printf() in interrupts
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료   
• mit-pdos/xv6-riscv@2ae9c8e : defines for UART register bits
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료   
• mit-pdos/xv6-riscv@7f35d7a : modify each page in usertests countfree()
  • xv6-riscv에서 merge하는 과정 : Rust에 이미 적용 완료   
• mit-pdos/xv6-riscv@1f55519 : Change tf -> trapframe in a few comments
  • trampoline.S 내 주석의 tf를 trapframe으로 변경 : Rust 해당 사항 없음

[jeehoonkang]

Bitflag면 bitflags! 를 사용하는게 어떨가 합니다.

[kimjungwow]

이 PR에서 uart.rs에 추가한 const들은 bitflags를 사용하기 껄끄러울 것 같습니다 😥 @jeehoonkang

• write 시 비트 별로 적는 것이 아닌 ptr::write_volatile()을 사용합니다. (bitflags::insert()와 다른 의미로 이해했습니다)
• 이 const 변수들은 UART0에 임의의 수를 더해 얻어지는 *mut u8 주소에 있는 값을 읽고 쓰는 용도입니다.
  • (UartCtrlRegs::read(), write() 할 때 사용되지만, UartCtrlRegs의 값을 읽고 쓰는 것이 아니어서 bitflags를 사용하기 애매합니다.)
    

    rv6/kernel-rs/src/uart.rs

    Lines 39 to 46 in 237d4be

    	fn reg(self) -> *mut u8 {
    	match self {
    	THR | RBR => UART0 as *mut u8,
    	IER => (UART0 + 1) as *mut u8,
    	FCR | ISR => (UART0 + 2) as *mut u8,
    	LCR => (UART0 + 3) as *mut u8,
    	LSR => (UART0 + 5) as *mut u8,
    	}

• Uart::init()에서 아직 하드코딩된 값을 write하는 경우가 있습니다.

[jeehoonkang]

• 넵 알겠습니다.
• 혹시 enum은 가능한가요.

[kimjungwow]

@jeehoonkang 교수님, enum이 가능하긴 한데 좋은 코드는 아닌 것 같습니다.

• IER_TX_ENABLE, FCR_FIFO_CLEAR, LSR_RX_READ는 모두 같은 값 (1)을 가져, enum 안에 모두 넣을 수는 없습니다.
• 아래와 같이 같은 enum으로 취급할 수 있긴 합니다.. enum과 const의 명명법이 달라서 아래와 같이 명명했는데, 가독성을 조금 포기하고 통일성을 주려면 IERTXENABLE, FCRFIFIOCLEAR, LSRRXREAD로 하면 될 것 같습니다!

enum UartRegBits {
    IerTxEnable= 1 << 0;
    생략
}

impl UartRegBtis {
    const FCR_FIFO_CLEAR:UartRegBits = UartRegBits::IerTxEnable;
    const LSR_RX_READ:UartRegBits = UartRegBits::IerTxEnable;
}

[jeehoonkang]

음.. 일반적으로 이 상수들에 어떤 structure가 있는지 제가 이해하지 못해서 조언드리기 어려울 것 같은데요, 정우님이 생각하시기에 structure가 가장 잘 드러나는 방식으로 리팩토링해주시면 감사하겠습니다.

[kimjungwow]

1. 리팩토링 전처럼 const 변수들을 그대로 사용하는 것은 적절치 않고
2. 해당 const 상수들 중 같은 값을 갖는 상수들이 있고
3. UartCtrlRegs::write(self, v: u8)의 v가 하드코딩된 값 혹은 임의의 값이 주어지기 때문에

UartRegBits::bits() 함수를 만들었습니다!

bf5fe7c

[coolofficials]

@kimjungwow 정우님 혹시 uartputc sync를 사용함에 있어서 ctrl+P 결과가 제대로 출력되거나 하지는 않나요?

[jeehoonkang]

• 넵 알겠습니다.
• 혹시 enum은 가능한가요.

[kimjungwow]

정우님 혹시 uartputc sync를 사용함에 있어서 ctrl+P 결과가 제대로 출력되거나 하지는 않나요?

@coolofficials 이 PR에서는 아직 ctrl+P 결과가 출력되지는 않습니다. 하지만 (논의가 진행 중인) #282 에서 변경한 내용들을 이 PR에 적용하면 정상적으로 출력됩니다!

[jeehoonkang]

이거랑 #285 중에 뭘 먼저 머지하는게 좋을까요? 정우님이 판단해주세요.

[kimjungwow]

이거랑 #285 중에 뭘 먼저 머지하는게 좋을까요? 정우님이 판단해주세요.

이 PR (#281 )을 먼저 머지하면 좋겠습니다! 😀 감사합니다.

[jeehoonkang]

넵 알겠습니다. 수정해주시고 리뷰 재요청 해주세요

[kimjungwow]

수정은 다 되었고, CI 통과하기를 기다리고 있었습니다! 리뷰 요청드립니다. 😊 @jeehoonkang

[coolofficials]

@kimjungwow
Uart::putc와 Uart::putc_sync의 차이점에 대해서 혹시 설명해주실 수 있나요??
Uart::putc는 현재 사용처가 안 보이고, Uart::putc_sync로 전부 변경된 것 같은데, Uart::putc_sync 함수는 self를 받지 않아서 소유권이 모호합니다.

Uart::putc_sync의 존재 이유가 kernel이 initialize 되지 않아도 print를 할 수 있게 하기 위함인가요? 이 경우 어제 논의했던 console struct의 design이 아니라 다른 방향을 가져야할 것 같아서 문의드립니다. Kernel이 Uart를 소유하면 안 될 것 같아서요.

[kimjungwow]

@coolofficials

Uart::putc와 Uart::putc_sync의 차이점에 대해서 혹시 설명해주실 수 있나요??

• 두 함수의 차이는 interrupt 사용 여부입니다. Uart::putc_sync()는 interrupt 사용하지만, Uart::putc()는 interrupt를 사용하지 않고 output buffer가 꽉 찬 경우 sleep()으로 block합니다.

Uart::putc는 현재 사용처가 안 보이고, Uart::putc_sync로 전부 변경된 것 같은데, Uart::putc_sync 함수는 self를 받지 않아서 소유권이 모호합니다.

• Uart::putc는 Console::write()에서 사용됩니다 (78번째 줄)
  

  rv6/kernel-rs/src/console.rs

  Lines 72 to 81 in 53d87f7

  	unsafe fn write(&mut self, src: UVAddr, n: i32) -> i32 {
  	for i in 0..n {
  	let mut c = [0 as u8];
  	if VAddr::copyin(&mut c, UVAddr::new(src.into_usize() + (i as usize))).is_err() {
  	return i;
  	}
  	self.uart.putc(c[0] as i32);
  	}
  	n
  	}

• Ownership of UartCtrlRegs to UART? #287 의 관점에서 보면 소유권이 모호하다고 볼 수 있음에 동의합니다

Uart::putc_sync의 존재 이유가 kernel이 initialize 되지 않아도 print를 할 수 있게 하기 위함인가요? 이 경우 어제 논의했던 console struct의 design이 아니라 다른 방향을 가져야할 것 같아서 문의드립니다. Kernel이 Uart를 소유하면 안 될 것 같아서요.

• Uart::putc_sync의 존재 이유는 xv6의 의도(uartputc_sync)를 반영하기 위함입니다. mit-pdos/xv6-riscv@db0f092
• Kernel이 Uart를 소유하면 안 되는 이유가 궁금합니다!

[kimjungwow]

@coolofficials #188 에서 논의를 이어가도 될까요? 😁

[coolofficials]

@coolofficials

Uart::putc와 Uart::putc_sync의 차이점에 대해서 혹시 설명해주실 수 있나요??

• 두 함수의 차이는 interrupt 사용 여부입니다. Uart::putc_sync()는 interrupt 사용하지만, Uart::putc()는 interrupt를 사용하지 않고 output buffer가 꽉 찬 경우 sleep()으로 block합니다.

Uart::putc는 현재 사용처가 안 보이고, Uart::putc_sync로 전부 변경된 것 같은데, Uart::putc_sync 함수는 self를 받지 않아서 소유권이 모호합니다.

• Uart::putc는 Console::write()에서 사용됩니다 (78번째 줄)
  

  rv6/kernel-rs/src/console.rs

  Lines 72 to 81 in 53d87f7

  	unsafe fn write(&mut self, src: UVAddr, n: i32) -> i32 {
  	for i in 0..n {
  	let mut c = [0 as u8];
  	if VAddr::copyin(&mut c, UVAddr::new(src.into_usize() + (i as usize))).is_err() {
  	return i;
  	}
  	self.uart.putc(c[0] as i32);
  	}
  	n
  	}

• Ownership of UartCtrlRegs to UART? #287 의 관점에서 보면 소유권이 모호하다고 볼 수 있음에 동의합니다

Uart::putc_sync의 존재 이유가 kernel이 initialize 되지 않아도 print를 할 수 있게 하기 위함인가요? 이 경우 어제 논의했던 console struct의 design이 아니라 다른 방향을 가져야할 것 같아서 문의드립니다. Kernel이 Uart를 소유하면 안 될 것 같아서요.

• Uart::putc_sync의 존재 이유는 xv6의 의도(uartputc_sync)를 반영하기 위함입니다. mit-pdos/xv6-riscv@db0f092
• Kernel이 Uart를 소유하면 안 되는 이유가 궁금합니다!

@kimjungwow 아 제가 저 당시 생각 정리가 좀 덜 되어 여쭤본건데, xv6 코드를 읽고 의도를 파악하여 #286 에 담아봤습니다. 리뷰 부탁드려요 👍