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

Lock split on VCpuInner #82

Open
wants to merge 4 commits into
base: hfo2
Choose a base branch
from
Open

Conversation

efenniht
Copy link
Collaborator

@efenniht efenniht commented Oct 25, 2020

  • vCPU 의 on/off 를 저장하는 is_on: SpinLock<bool> 변수를 따로 만들고, PSCI_AFFINITY_INFO 하이퍼바이저 콜은 해당 변수에 대한 락을 걸도록 수정합니다.

  • VCpuStatus::Off 는 그대로 남아 있습니다. vcpu_prepare_run 에서 is_on 의 락을 걸지 않기 위해서입니다.

  • is_on 의 lock order 를 결정해야 합니다.

    • is_on 을 먼저 걸고 VCpuInner 락을 걸도록 하면, vcpu_off 하이퍼바이저 콜 구현을 바꿔야 합니다. 내부에서 is_on 을 수정할 수 없고, primary로 context switch 가 끝난 시점인 api_regs_state_saved 에서 1) VCpuInner 락을 풀고 2) is_on 을 수정해야 합니다.
    • VCpuInner 락을 먼저 걸고 is_on 을 걸도록 하면, vcpu_secondary_reset_and_start 함수의 구현을 바꾸어야 합니다. 여기서 try_lock 으로 락이 안 걸리면 running 이므로 포기 / 그렇지 않으면 waiting or off 이므로 off 인 경우에만 reset 하도록 바꾸면 됩니다.
    • 어떤 것이 더 올바른 구현일까요?

@efenniht efenniht self-assigned this Oct 25, 2020
@jeehoonkang
Copy link
Member

느낌상으로 2번째가 더 좋아보입니다. @efenniht 님 의견은 어떠세요?

@@ -309,8 +310,8 @@ impl VCpu {

pub fn index(&self) -> spci_vcpu_index_t {
let vcpus = self.vm().vcpus.as_ptr();
let index = (self as *const VCpu).wrapping_offset_from(vcpus);
Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cargo-xbuild 가 오래된 rustc 에서 잘 컴파일이 안 되어서, rustc 를 업데이트하였는데, 그 여파로 wrapping_offset_from 함수가 사라져서 동일한 의미의 코드로 대체하였습니다.

@@ -440,7 +441,7 @@ impl CpuManager {
}

pub fn index_of(&self, c: *const Cpu) -> usize {
c.wrapping_offset_from(self.cpus.as_ptr()) as _
Copy link
Member

Choose a reason for hiding this comment

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

again, why?

hfo2/src/cpu.rs Outdated
// vCPU is defined as the index and does not match the ID of the
// pCPU it is running on.
// Set vCPU registers to a clean state ready for boot.
let mut state = vcpu.inner.lock();
Copy link
Member

Choose a reason for hiding this comment

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

lock order를 바꾼다면 vcpu.inner.is_locked() 로 확인해볼 수 있나요? try_lock 대신에..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reset 을 위해서는 inner 의 락을 결국 걸어야 할 것 같습니다.

@efenniht efenniht marked this pull request as ready for review October 27, 2020 16:46
@efenniht
Copy link
Collaborator Author

테스트를 다시 돌려서 잘 작동하는 것을 확인했습니다.

제 기억으로는 연구실 pc 에서 테스트에서 실패했었는데, 연구실 pc 에는 이 브랜치를 체크아웃한 기록이 없었습니다..! 꿈에서 테스트한 것 같습니다 😴

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.

2 participants