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

Arm VM Multicore Improvements #84

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Apave24
Copy link

@Apave24 Apave24 commented Nov 11, 2022

These commits allow for more flexible core configurations for Arm VM systems

libsel4vm/src/boot.c Outdated Show resolved Hide resolved
smc_set_return_value(&regs, PSCI_SUCCESS);
} else {
ZF_LOGE("[vCPU %u] no unused physical core left", vcpu->vcpu_id);
Copy link
Member

Choose a reason for hiding this comment

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

...or starting failed. I think separating the errors is worth it, because that make debugging easier when something fails here or got out if sync somehow.

@@ -92,6 +92,15 @@ int vm_assign_vcpu_target(vm_vcpu_t *vcpu, int target_cpu)
ZF_LOGE("Failed to assign target cpu - A VCPU is already assigned to core %d", target_cpu);
return -1;
}

#if CONFIG_MAX_NUM_NODES > 1
int err = seL4_TCB_SetAffinity(vcpu->tcb.tcb.cptr, target_cpu);
Copy link
Member

Choose a reason for hiding this comment

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

I think to commit is ok. But it makes this generic code now, how do other architectures cope with this change then? Do we gave to change something in the x86 VMM part also?

Copy link
Author

Choose a reason for hiding this comment

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

I believe vm_assign_vcpu_target is currently only used by ARM

@axel-h axel-h requested a review from yyshen November 17, 2022 21:26
@axel-h
Copy link
Member

axel-h commented Nov 17, 2022

Since this PR has multiple commits that seem independent, but have dependencies on other PRs, could you split this up that we can merge things independently and avoid unnecessary blocking?

@Apave24
Copy link
Author

Apave24 commented Nov 17, 2022

Since this PR has multiple commits that seem independent, but have dependencies on other PRs, could you split this up that we can merge things independently and avoid unnecessary blocking?

Dropped commit libsel4vm: guest cpu requests to vcpus from pcpus from this PR and added to #85

@Apave24
Copy link
Author

Apave24 commented Mar 8, 2023

Bumping this PR. I believe everything has been addressed.

@axel-h
Copy link
Member

axel-h commented Mar 10, 2023

@Apave24 could you rebase this, then we also get another CI run there to see if this passes.

Alex Pavey added 3 commits February 1, 2024 09:57
Calling seL4_TCB_SetAffinity in the vm_assign_vcpu_target function
allows the vcpu to correctly run on the cpu it is assigned to.
Previously the vcpu was getting assigned to the same core as it's ID.

Signed-off-by: Alex Pavey <[email protected]>
Previously the target_cpu member was being used to determine if a vcpu
was started in conjunction with the is_vcpu_online function. This is
unneccessary and has been repurposed to keep track of whether or not the
vcpu has been previously assigned a pcpu or not.

Signed-off-by: Alex Pavey <[email protected]>
@chrisguikema
Copy link
Contributor

@axel-h @abrandnewusername @kent-mcleod

Hey guys, just wanted to ping on this PR. This series of PRs makes it so that when the VM kicks off a secondary core, the VMM pins the VCPU to a PCPU defined in the vmX.pcpus list. I feel like this behavior makes the most sense given the current VMM configuration. Is this a good path forward? Can we work on getting this merged?

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.

3 participants