[os-d]

Description

X64 UEFI systems rely on APIC virtual wire mode being programmed during execution, e.g. wiring up legacy interrupts to the APIC to service them. Without this, interrupts can not be serviced.

Patina has relied on pre-DXE to enable this mode or MpDxe from patina-edk2 to do so. However, MpDxe has been removed in favor of MpServices. A platform that does not set virtual wire mode pre-DXE can fail to boot.

This PR adds setting virtual wire mode for all X64 platforms, so long as the MSR for determining which APIC mode the platform supports is supported (as it has long been). The MSR is then used to determine whether this is an XAPIC or X2APIC and programs it accordingly.

• Impacts functionality?
• Impacts security?
• Breaking change?
• Includes tests?
• Includes documentation?

How This Was Tested

Tested on Hyper-V guest FW where removing MpDxe in favor of MP services caused a hang. Adding this booted to the OS again. Tested on a physical Intel device as well booting to the OS.

Note: I plan to test this on an enlightened TDX guest as well before checking in, because in the C code this has required special handling.

Both platforms are X2APIC code; the XAPIC code is untested but the logic is copied from edk2 handling of XAPICs.

Integration Instructions

N/A.

[patina-automation]

✅ QEMU Validation Passed

All QEMU validation jobs completed successfully.

Note: Q35 is only built on Windows hosts (QEMU boot is disabled due to a QEMU vfat issue).

Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/40003599926

Boot Time to EFI Shell

Platform	Elapsed
ArmVirt (Linux Host)	29.4s
Q35 (Linux Host)	29.0s

Dependencies

Repository	Ref
patina	d76fbb0
patina-dxe-core-qemu	28cfae7
patina-fw-patcher	5e02eb7
patina-qemu firmware	v4.1.3
patina-qemu build script	3c89c5a

This comment was automatically generated by the Patina QEMU PR Validation Post workflow.

[cfernald]

Looks good. Once the supervisor is merged we should do the cpu reefactor and merge this apic.rs with the one from mp, but not for this PR to avoid churn that affects that PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[makubacki]

initialize_cpu_subsystem in cpu.rs does:

    // Confidential VMs cannot log between GDT install and IDT install because this creates a #VE/#VC exception
    // which cannot be handled until the IDT is installed. Disable logging globally through this transition.
    #[cfg(feature = "confidential_compute")]
    log::set_max_level(log::LevelFilter::Off);

    cpu.initialize().inspect_err(|err| {
        log::error!("Failed to initialize CPU subsystem: {err}");
    })?;

    let mut interrupt_manager = Interrupts::new();
    interrupt_manager.initialize().inspect_err(|err| {
        log::error!("Failed to initialize Interrupt Manager: {err}");
    })?;

    for &(exception_type, handler) in exception_handlers {
        interrupt_manager.register_exception_handler(exception_type, HandlerType::Handler(handler))?;
    }

Basically:

• Install GDT

• Install IDT

• Call this new code super::apic::initialize_virtual_wire_mode

• Then install platform exception handlers:

    for &(exception_type, handler) in exception_handlers {
        interrupt_manager.register_exception_handler(exception_type, HandlerType::Handler(handler))?;
    }
  

Is there a reason not to register the exception handlers then initialize virtual wire mode?

[os-d]

I agree with moving it after that point, I will update.