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

SVSM assumes APIC IDs are sequential and fails to boot if there are gaps #623

Open
AdamCDunlap opened this issue Feb 20, 2025 · 2 comments

Comments

@AdamCDunlap
Copy link
Contributor

Currently, SVSM assumes APIC IDs go from 0 to cpu count: https://github.com/coconut-svsm/svsm/blob/035bb52967ad938bee3820283b4b6ae4a8ce6a33/kernel/src/igvm_params.rs#L253C1-L259C11 . However, some hypervisors do not assign APIC IDs this way and instead leave gaps for reasons beyond my understanding (something about last level caches and threads per core). When SVSM tries to boot an AP with an APIC ID that was not assigned by the hypervisor, SVSM panics.

So we need to modify SVSM to instead get the list of APIC IDs from the hypervisor. I believe there's 3 reasonable ways to do this:

  1. ACPI tables. It appears this is what happens in the fw_cfg version load_acpi_cpu_info. This probably wouldn't work with IGVM since the ACPI tables aren't supplied to SVSM
  2. A new IGVM parameter. I don't see this parameter currently existing in the IGVM documentation -- it just has the number of VPs, not which APIC IDs they use
  3. The SVM_VMGEXIT_GET_APIC_IDS hypercall

I'm inclined to go with the hypercall. My only reservation is that we had some internal concern about the KVM implementation of the hypercall, but since EDK2 uses it as well, it's probably OK to add it here as well.

I can draft a PR but would appreciate any feedback

@AdamCDunlap
Copy link
Contributor Author

I'm realizing that the SVM_VMGEXIT_GET_APIC_IDS is a GHCB NAE and therefore won't work on the TDX or native platforms. I'm not sure of the best way to resolve this. Maybe on SNP it uses the NAE and on other platforms it uses ACPI tables or the sequential IDs?

@AdamCDunlap
Copy link
Contributor Author

#626 resolves this using strategy 2. There's an existing IGVM parameter for the MADT which includes the list of APIC IDs.

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 a pull request may close this issue.

1 participant