Skip to content

Enable x2apic sec #11033

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

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

Conversation

MelodyHuibo
Copy link

This is a RFC pull request for this: (https://edk2.groups.io/g/devel/message/121291)
"I am currently working on enabling Alternate Injection for AMD SEV-SNP guests and have encountered a design issue.

The Alternate Injection specification, which is still preliminary, defines a so-called SVSM APIC protocol through a subset
of X2APIC MSRs while timer support is configurable.
[ This means, if timer functionality is not supported, the guest must rely on the hypervisor to emulate timer
support through use of the #HV Timer GHCB protocol. ]

When the OVMF firmware starts, it is in XAPIC mode by default and then, later, during the init phase it switches the guest to X2APIC.
However, with Alternate Injection enabled, the OVMF in its very first phase - SEC - does XAPIC accesses.

The SVSM, however, which is part of the guest, uses the so-called SVSM APIC protocol which uses a subset of the X2APIC MSRs.

The OVMF, however, assumes it starts off in XAPIC memory-mapped mode and thus there's a protocol mismatch of sorts
because with Alternate Injection already enabled in the SEC phase, it mandates X2APIC MSR accesses.

The registers (timer registers) when not handled by SVSM will get routed to the hypervisor (KVM) which at that point is operating the guest
in XAPIC mode until the PEI phase switches to X2APIC.

If X2APIC enablement is moved from the PEI to the SEC phase, the problem can be resolved. I have tested it and it works.
However, I dont know if there is any concern or potential design issues with that move.

Do folks think this is ok to do - i.e., move the X2APIC enablement to the SEC phase?

Or do you have any suggestions for a better solution?"

Ard suggested that I should send it this way in order to collect some comments on the approach.

Thank you for taking a look and sorry if have not done something right because I am new.

@kraxel
Copy link
Member

kraxel commented May 5, 2025

The main reason why the firmware does not go straight to x2apic mode is backward compatibility. So the current strategy is to use x2apic mode only in case the number of CPUs is larger than 255, otherwise stick to xapic mode, for better compatibility with older OS versions which might not have support for x2apic mode.

Confidential computing is very new compared to x2apic mode, so I'd expect a guest os which supports CC should also support x2apic. On that ground I think we can simplify this and enable x2apic mode unconditionally when running as confidential guest.

@ardbiesheuvel
Copy link
Member

Please check the README for how to structure and document your patches. The commit log must describe what the patch does, but more importantly, why it does so. You'll also need to use the correct patch titles and add a signoff.

Patches should not modify more than one package at the same time.

As for the technical side, as @kraxel suggests, it might be easier to always enable X2APIC mode when running as as confidential guest (SEV-SNP or TDX)

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