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

x86: set ApicId in cpuid for each vcpu #5512

Merged
merged 1 commit into from
Jun 15, 2023

Conversation

jongwu
Copy link
Contributor

@jongwu jongwu commented Jun 15, 2023

When start clh from edk2 with vcpu number larger than 2, edk2 will crash at Ap initialization. The root cause is that when get ApicId from cpuid, it returns a wrong value that ApicId is the same for each Ap. Further more, stack selection depends on ApicId, that leads to all Aps share the same stack space.

Explicitly set ApicId in cpuid for each vcpu can solve this issue.

Fixes: #5475

@maburlik
Copy link

I took a build of @jongwu's branch and it appears to address the issue. I am no longer hitting this with the same repro conditions described in the issue.

Summary:
#5475 (comment)

let cpuid = core::arch::x86_64::__cpuid(1);
cpu_ebx = cpuid.ebx
}
cpu_ebx &= 0xffffff;
Copy link
Member

Choose a reason for hiding this comment

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

Can simplify this a bit...

let mut cpu_ebx = { unsafe core::arch::x86_64::__cpuid(1) }.ebx;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks @rbradford

@weltling
Copy link
Member

After this has landed, one can see then whether the Windows guest integration tests can be restored on CI.

Also, for AMD, encoding the APIC ID into the topology is required, so this value set here could be probably reused at some other place.

Thanks

Program the APIC ID (CPUID leaf 0x1 EBX) with the CPU id. This resolves
an issue where the EDKII firmware expects the APIC ID to vary per-CPU.

Fixes: cloud-hypervisor#5475
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
@likebreath likebreath merged commit 57fdaa3 into cloud-hypervisor:main Jun 15, 2023
21 checks passed
@likebreath likebreath added the bug-fix Bug fix to include in release notes label Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix Bug fix to include in release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Boot with 3+ cores & OVMF hits !!!! X64 Exception Type - 06(#UD - Invalid Opcode)
5 participants