RE: [EXTERNAL] Issue in handle_icr_write
Jon Lange <[email protected]> Thu, 14 Aug 2025 16:48:05 +0000
| Newsgroups | dev.linux.lists.coconut-svsm |
|---|---|
| Message-ID | <CH8PR21MB5222C4EFDED771DCDBEC910FCA35A@CH8PR21MB5222.namprd21.prod.outlook.com> |
I think it would be reasonable to remove those checks. -Jon -----Original Message----- From: Melody Wang <[email protected]> Sent: Thursday, August 14, 2025 8:33 AM To: Jon Lange <[email protected]> Cc: Lendacky, Thomas <[email protected]>; Rödel, Jörg <[email protected]>; Kaplan, David <[email protected]>; [email protected] Subject: [EXTERNAL] Issue in handle_icr_write Hi Jon, I have been working on the PoC of Alternate Injection, I found there seems to be somewhat of a problem in handle_icr_write: For the Message Type == Fixed you enforce trigger mode == 0 (edge triggered) and assert == 1 In practice, actually, those bits are largely ignored. The Intel SDM says: "Level: For the INIT level de-assert delivery mode this flag must be set to 0; for all other delivery modes it must be set to 1. (This flag has no meaning in Pentium 4 and Intel Xeon processors, and will always be issued as a 1.)" ... Trigger mode: Selects the trigger mode when using the INIT level de-assert delivery mode: edge (0) or level (1). It is ignored for all other delivery modes. (This flag has no meaning in Pentium 4 and Intel Xeon processors, and will always be issued as a 0." Basically, the hw ignores those bits and this is how Linux has been doing it since forever. Example from me tracing a Linux guest: [SVSM] write_register: 0x830: 0x28 That's only the interrupt vector and neither of those two bits are set. So I'm thinking we should simply remove those checks. What do you think about it? fn handle_icr_write(&mut self, value: u64) -> Result<(), SvsmError> { let icr = ApicIcr::from(value); // Verify that this message type is supported. let valid_type = match icr.message_type() { IcrMessageType::Fixed => { // Only asserted edge-triggered interrupts can be handled. !icr.trigger_mode() && icr.assert() } IcrMessageType::Nmi => true, _ => false, }; if !valid_type { return Err(SvsmError::Apic(Emulation)); } self.send_ipi(icr); Ok(()) } -- Thanks, Melody