Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor api.rs, using VmState on Vm #27

Merged
merged 33 commits into from
Aug 20, 2019
Merged

Conversation

efenniht
Copy link
Collaborator

@efenniht efenniht commented Aug 17, 2019

#17, #20 의 일부입니다.

  • Closes Which data structures can we modify their layout? #21.
    • C 코드 중 vcpuvm의 내부를 직접 접근하는 코드를 getter/setter로 바꾸었습니다.
    • 이것을 확인하는 겸 해서 {cpu, vm}.h 에는 forward declaration만 남겨두었습니다.
    • offset.c 는 상수로 정의된 각 필드의 오프셋이 실제로 맞는 지 테스트하는 코드인데, 이것은 aarch64.rs 에 옮겼습니다.
      • Filename convention #15 를 해결할 때 같이 수정해야 합니다.
      • 사실 가장 좋은 것은 빌드스크립트를 만들어서 자동으로 헤더를 생성하는 건데... 나중으로 미루는 걸로...
  • Vm 중 락으로 보호해야 하는 데이터를 VmState로 추려서 SpinLock으로 보호했습니다.
    • "락으로 보호해야 하는 데이터" 에는 wait_entries 가 포함됩니다. 이것을 손보는 것은 다음 기회에.
    • "락으로 보호해야 하는 데이터" 에는 vcpus가 포함되지 않습니다. vcpus의 락으로 보호해야 하는 데이터는 VCpuState에 걸린 락으로 보호되기 때문입니다.
  • 이 락을 사용하도록 api.rs 코드의 많은 부분을 변경했습니다.
    • 하지만 아직 goto 문의 잔재는 남아있습니다. 페이지 테이블에 대한 트랜잭션? 같은 걸 만들어 볼까 생각 중입니다.
  • VmState 와 관련된 api_* 함수들을 VmState의 메서드로 옮겼습니다.
    • 그러면서 public한 멤버 변수를 줄이기로 노력했는데, 깔끔하게 모두 없애지는 못했습니다. (Remove unneccessary pubs. #12)
    • 예를 들어, api_mailbox_clear는 일견 Mailbox의 멤버 함수가 될 수 있을 것 같지만 나중에 api_switch_to_primary 를 내부적으로 호출해서 간단하게는 되지 않습니다.
    • 나중에 좀 더 분석해서 API 파트와 VM 파트를 분리하는 걸로...
  • Vm 관련 함수들을 리팩토링하면서 unsafe를 제거하거나 필요한 부분을 explicit하게 나타냈습니다.
  • VmLockedSpinLockGuard<VmState>를 저장하도록 바꾸고 싶었는데, C 코드에서 vm_locked 내부의 vm을 사용하는 부분이 있어서 당장은 하지 않았습니다. spci_architected_message.c 를 Rust로 먼저 포팅해야 가능할 것으로 보입니다.
    • api 내부의 함수가 SpinLockGuard<VmState>를 가지고 있는 상태로 VmLocked을 인자로 받는 C 함수를 호출하는 게 문제인데요. 이 C 함수가 *mut VmLocked를 받도록 바꾸면 될 것 같지만 어차피 포팅할 코드이므로 나중에...

아직 #20 은 끝나지 않았습니다. 다음으로는 VCpu 에 있는 락을 수정할 예정입니다.

여담으로, 이 리팩토링을 하면서 Rust에서 mut 키워드가 가지는 의미에 대해서 좀 더 깊게 이해하게 되었습니다:

  • &mut은 mutability가 아니라 배타적인 레퍼런스를 의미합니다. 배타적인 레퍼런스는 안전한 수정 가능성을 의미하지만, 역은 성립하지 않고 Rust에서는 안전한 수정을 보장하는 다른 방법이 많습니다.
  • let mut x; 로 선언된 변수 또한 &mut 레퍼런스를 만들 수 있다는 것을 의미합니다. 이게 꼭 이 변수를 수정한다는 뜻은 아닙니다. 예를 들어 SpinLockGuard&mut self를 받아서 mutable한 내부 레퍼런스를 줍니다.

Copy link
Member

@jeehoonkang jeehoonkang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR이 커서 일단 일부 코멘트만 남깁니다. 몇번 더 iteration을 돌고 머지하면 좋을 것 같습니다.

고생하셨습니다!

hfo2/src/arch/aarch64.rs Show resolved Hide resolved
hfo2/src/cpu.rs Show resolved Hide resolved
hfo2/src/cpu.rs Outdated Show resolved Hide resolved
hfo2/src/cpu.rs Outdated Show resolved Hide resolved
hfo2/src/dlog.rs Outdated Show resolved Hide resolved
hfo2/src/vm.rs Outdated Show resolved Hide resolved
hfo2/src/vm.rs Outdated Show resolved Hide resolved
hfo2/src/vm.rs Outdated Show resolved Hide resolved
hfo2/src/vm.rs Outdated Show resolved Hide resolved
hfo2/src/vm.rs Outdated Show resolved Hide resolved
Copy link
Member

@jeehoonkang jeehoonkang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일단 이정도만 남깁니다... 추후에 다시 리뷰 또 드리겠습니다.

hfo2/src/api.rs Outdated Show resolved Hide resolved
hfo2/src/api.rs Show resolved Hide resolved
hfo2/src/api.rs Show resolved Hide resolved
hfo2/src/api.rs Show resolved Hide resolved
hfo2/src/api.rs Show resolved Hide resolved
hfo2/src/api.rs Outdated Show resolved Hide resolved
hfo2/src/api.rs Show resolved Hide resolved
hfo2/src/api.rs Show resolved Hide resolved
hfo2/src/api.rs Outdated Show resolved Hide resolved
hfo2/src/api.rs Outdated Show resolved Hide resolved
hfo2/src/api.rs Outdated Show resolved Hide resolved
hfo2/src/api.rs Outdated Show resolved Hide resolved
hfo2/src/api.rs Show resolved Hide resolved
hfo2/src/vm.rs Show resolved Hide resolved
hfo2/src/arch/aarch64.rs Show resolved Hide resolved
@efenniht
Copy link
Collaborator Author

모두 resolve 했습니다!

@jeehoonkang jeehoonkang merged commit c8cc29b into kaist-cp:hfo2 Aug 20, 2019
@jeehoonkang
Copy link
Member

고생하셨어요-

@efenniht efenniht deleted the hfo2-17-api branch August 21, 2019 03:51
@efenniht efenniht mentioned this pull request Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Which data structures can we modify their layout?
2 participants