Skip to content

snp: inject event when ext_int is valid #1347

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

snp: inject event when ext_int is valid #1347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 40 additions & 38 deletions openhcl/virt_mshv_vtl/src/processor/snp/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,12 @@ use virt_support_x86emu::emulate::emulate_io;
use virt_support_x86emu::emulate::emulate_translate_gva;
use virt_support_x86emu::translate::TranslationRegisters;
use vmcore::vmtime::VmTimeAccess;
use x86defs::Exception;
use x86defs::RFlags;
use x86defs::cpuid::CpuidFunction;
use x86defs::snp::SevEventInjectInfo;
use x86defs::snp::SevExitCode;
use x86defs::snp::SevInterruptionType;
use x86defs::snp::SevInvlpgbEcx;
use x86defs::snp::SevInvlpgbEdx;
use x86defs::snp::SevInvlpgbRax;
Expand Down Expand Up @@ -242,7 +244,7 @@ impl HardwareIsolatedBacking for SnpBacked {
fn pending_event_vector(this: &UhProcessor<'_, Self>, vtl: GuestVtl) -> Option<u8> {
let event_inject = this.runner.vmsa(vtl).event_inject();
if event_inject.valid() {
Some(event_inject.vector())
Some(event_inject.vector().0)
} else {
None
}
Expand All @@ -257,8 +259,10 @@ impl HardwareIsolatedBacking for SnpBacked {
.with_valid(true)
.with_deliver_error_code(event.deliver_error_code())
.with_error_code(event.error_code())
.with_vector(event.vector().try_into().unwrap())
.with_interruption_type(x86defs::snp::SEV_INTR_TYPE_EXCEPT);
.with_vector(Exception(
event.vector().try_into().expect("valid exception vector"),
))
.with_interruption_type(SevInterruptionType::EXCEPT);

this.runner.vmsa_mut(vtl).set_event_inject(inject_info);
}
Expand Down Expand Up @@ -509,7 +513,7 @@ impl BackingPrivate for SnpBacked {
this.cvm_handle_cross_vtl_interrupts(|this, vtl, check_rflags| {
let vmsa = this.runner.vmsa_mut(vtl);
if vmsa.event_inject().valid()
&& vmsa.event_inject().interruption_type() == x86defs::snp::SEV_INTR_TYPE_NMI
&& vmsa.event_inject().interruption_type() == SevInterruptionType::NMI
{
return true;
}
Expand Down Expand Up @@ -860,8 +864,8 @@ impl<'b> hardware_cvm::apic::ApicBacking<'b, SnpBacked> for UhProcessor<'b, SnpB

vmsa.set_event_inject(
SevEventInjectInfo::new()
.with_interruption_type(x86defs::snp::SEV_INTR_TYPE_NMI)
.with_vector(2)
.with_interruption_type(SevInterruptionType::NMI)
.with_vector(Exception::NMI)
.with_valid(true),
);
self.backing.cvm.lapics[vtl].nmi_pending = false;
Expand Down Expand Up @@ -1060,8 +1064,8 @@ impl UhProcessor<'_, SnpBacked> {
if gp {
vmsa.set_event_inject(
SevEventInjectInfo::new()
.with_interruption_type(x86defs::snp::SEV_INTR_TYPE_EXCEPT)
.with_vector(x86defs::Exception::GENERAL_PROTECTION_FAULT.0)
.with_interruption_type(SevInterruptionType::EXCEPT)
.with_vector(Exception::GENERAL_PROTECTION_FAULT)
.with_deliver_error_code(true)
.with_valid(true),
);
Expand Down Expand Up @@ -1092,8 +1096,8 @@ impl UhProcessor<'_, SnpBacked> {
let mut vmsa = self.runner.vmsa_mut(entered_from_vtl);
vmsa.set_event_inject(
SevEventInjectInfo::new()
.with_interruption_type(x86defs::snp::SEV_INTR_TYPE_EXCEPT)
.with_vector(x86defs::Exception::GENERAL_PROTECTION_FAULT.0)
.with_interruption_type(SevInterruptionType::EXCEPT)
.with_vector(Exception::GENERAL_PROTECTION_FAULT)
.with_deliver_error_code(true)
.with_valid(true),
);
Expand Down Expand Up @@ -1196,29 +1200,28 @@ impl UhProcessor<'_, SnpBacked> {
self.backing.general_stats[entered_from_vtl]
.guest_busy
.increment();
// Software interrupts/exceptions cannot be automatically re-injected, but RIP still
// points to the instruction and the event should be re-generated when the
// instruction is re-executed. Note that hardware does not provide instruction
// length in this case so it's impossible to directly re-inject a software event if
// delivery generates an intercept.
//
// TODO SNP: Handle ICEBP.

let exit_int_info = SevEventInjectInfo::from(vmsa.exit_int_info());
debug_assert!(
exit_int_info.valid(),
"event inject info should be valid {exit_int_info:x?}"
);
if exit_int_info.valid() {
// Software interrupts/exceptions cannot be automatically re-injected, but RIP still
// points to the instruction and the event should be re-generated when the
// instruction is re-executed. Note that hardware does not provide instruction
// length in this case so it's impossible to directly re-inject a software event if
// delivery generates an intercept.
//
// TODO SNP: Handle ICEBP.
let inject = match exit_int_info.interruption_type() {
SevInterruptionType::EXCEPT => {
exit_int_info.vector() != Exception::BREAKPOINT
&& exit_int_info.vector() != Exception::OVERFLOW
}
SevInterruptionType::SW => false,
_ => true,
};

let inject = match exit_int_info.interruption_type() {
x86defs::snp::SEV_INTR_TYPE_EXCEPT => {
exit_int_info.vector() != 3 && exit_int_info.vector() != 4
if inject {
vmsa.set_event_inject(exit_int_info);
}
x86defs::snp::SEV_INTR_TYPE_SW => false,
_ => true,
};

if inject {
vmsa.set_event_inject(exit_int_info);
}
}
vmsa.v_intr_cntrl_mut().set_guest_busy(true);
Expand Down Expand Up @@ -1381,8 +1384,7 @@ impl UhProcessor<'_, SnpBacked> {
let exception_message =
exit_message.as_message::<hvdef::HvX64ExceptionInterceptMessage>();

exception_message.vector
== x86defs::Exception::SEV_VMM_COMMUNICATION.0 as u16
exception_message.vector == Exception::SEV_VMM_COMMUNICATION.0 as u16
}
HvMessageType::HvMessageTypeUnmappedGpa
| HvMessageType::HvMessageTypeGpaIntercept
Expand Down Expand Up @@ -1426,8 +1428,8 @@ impl UhProcessor<'_, SnpBacked> {
SevExitCode::INVLPGB | SevExitCode::ILLEGAL_INVLPGB => {
vmsa.set_event_inject(
SevEventInjectInfo::new()
.with_interruption_type(x86defs::snp::SEV_INTR_TYPE_EXCEPT)
.with_vector(x86defs::Exception::INVALID_OPCODE.0)
.with_interruption_type(SevInterruptionType::EXCEPT)
.with_vector(Exception::INVALID_OPCODE)
.with_valid(true),
);
&mut self.backing.exit_stats[entered_from_vtl].invlpgb
Expand All @@ -1442,8 +1444,8 @@ impl UhProcessor<'_, SnpBacked> {
{
vmsa.set_event_inject(
SevEventInjectInfo::new()
.with_interruption_type(x86defs::snp::SEV_INTR_TYPE_EXCEPT)
.with_vector(x86defs::Exception::GENERAL_PROTECTION_FAULT.0)
.with_interruption_type(SevInterruptionType::EXCEPT)
.with_vector(Exception::GENERAL_PROTECTION_FAULT)
.with_deliver_error_code(true)
.with_valid(true),
);
Expand Down Expand Up @@ -2412,8 +2414,8 @@ impl<T: CpuIo> hv1_hypercall::VtlSwitchOps for UhHypercallHandler<'_, '_, T, Snp
.set_event_inject(
SevEventInjectInfo::new()
.with_valid(true)
.with_interruption_type(x86defs::snp::SEV_INTR_TYPE_EXCEPT)
.with_vector(x86defs::Exception::INVALID_OPCODE.0),
.with_interruption_type(SevInterruptionType::EXCEPT)
.with_vector(Exception::INVALID_OPCODE),
);
}
}
Expand Down
11 changes: 11 additions & 0 deletions vm/x86/x86defs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,7 @@ open_enum! {
pub enum Exception: u8 {
DIVIDE_ERROR = 0x0,
DEBUG = 0x1,
NMI = 0x2,
Copy link
Member

Choose a reason for hiding this comment

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

Let's be careful here. Although NMIs use vector 2, they are not exceptions. I would leave this out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh, I see that now! Thanks!!

BREAKPOINT = 0x3,
OVERFLOW = 0x4,
BOUND_RANGE_EXCEEDED = 0x5,
Expand All @@ -435,6 +436,16 @@ open_enum! {
}
}

impl Exception {
const fn into_bits(self) -> u8 {
self.0
}

const fn from_bits(bits: u8) -> Self {
Self(bits)
}
}

#[bitfield(u32)]
pub struct PageFaultErrorCode {
pub present: bool,
Expand Down
37 changes: 29 additions & 8 deletions vm/x86/x86defs/src/snp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,49 @@

//! AMD SEV-SNP specific definitions.

use crate::Exception;
use bitfield_struct::bitfield;
use zerocopy::FromBytes;
use zerocopy::Immutable;
use zerocopy::IntoBytes;
use zerocopy::KnownLayout;

// Interruption Information Field
pub const SEV_INTR_TYPE_EXT: u32 = 0;
pub const SEV_INTR_TYPE_NMI: u32 = 2;
pub const SEV_INTR_TYPE_EXCEPT: u32 = 3;
pub const SEV_INTR_TYPE_SW: u32 = 4;

// Secrets page layout.
pub const REG_TWEAK_BITMAP_OFFSET: usize = 0x100;
pub const REG_TWEAK_BITMAP_SIZE: usize = 0x40;

open_enum::open_enum! {
/// SEV-SNP interruption type.
#[derive(IntoBytes, Immutable, KnownLayout, FromBytes)]
pub enum SevInterruptionType: u8 {
/// External interrupt
EXT = 0,
/// NMI
NMI = 2,
/// Exception
EXCEPT = 3,
/// Software interrupt
SW = 4,
}
}

impl SevInterruptionType {
const fn into_bits(self) -> u8 {
self.0
}

const fn from_bits(bits: u8) -> Self {
Self(bits)
}
}

#[bitfield(u64)]
#[derive(IntoBytes, Immutable, KnownLayout, FromBytes, PartialEq, Eq)]
pub struct SevEventInjectInfo {
pub vector: u8,
#[bits(8)]
pub vector: Exception,
Copy link
Member

Choose a reason for hiding this comment

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

I'd leave this as u8 since this can be used to inject non-exceptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed 100%! I'll work on this more after completing more of the main quest :)

#[bits(3)]
pub interruption_type: u32,
pub interruption_type: SevInterruptionType,
pub deliver_error_code: bool,
#[bits(19)]
_rsvd1: u64,
Expand Down
Loading