Skip to content

Remove AML-triggerable panics from library #276

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
IsaacWoods wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Remove AML-triggerable panics from library #276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f34178
Remove panics in GAS handling code
IsaacWoods Mar 14, 2026
bd0a039
Remove some panics in AML interpreter
IsaacWoods Mar 14, 2026
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
71 changes: 43 additions & 28 deletions src/address.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//! in a wide range of address spaces.

use crate::{AcpiError, Handler, PhysicalMapping};
use core::ptr;
use log::warn;

/// This is the raw form of a Generic Address Structure, and follows the layout found in the ACPI tables.
Expand Down Expand Up @@ -143,7 +144,7 @@ where
}
AddressSpace::SystemIo => Ok(MappedGas { gas, handler: handler.clone(), mapping: None }),
other => {
warn!("Tried to map GAS of unsupported type {:?}", other);
warn!("Mapping a GAS in address space {:?} is not supported!", other);
Err(AcpiError::LibUnimplemented)
}
}
Expand All @@ -158,25 +159,24 @@ where
match self.gas.address_space {
AddressSpace::SystemMemory => {
let mapping = self.mapping.as_ref().unwrap();
let value = match access_size_bits {
8 => unsafe { core::ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u8) as u64 },
16 => unsafe { core::ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u16) as u64 },
32 => unsafe { core::ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u32) as u64 },
64 => unsafe { core::ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u64) },
_ => panic!(),
};
Ok(value)
match access_size_bits {
8 => Ok(unsafe { ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u8) as u64 }),
16 => Ok(unsafe { ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u16) as u64 }),
32 => Ok(unsafe { ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u32) as u64 }),
64 => Ok(unsafe { ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u64) }),
_ => Err(AcpiError::InvalidGenericAddress),
}
}
AddressSpace::SystemIo => {
let value = match access_size_bits {
8 => self.handler.read_io_u8(self.gas.address as u16) as u64,
16 => self.handler.read_io_u16(self.gas.address as u16) as u64,
32 => self.handler.read_io_u32(self.gas.address as u16) as u64,
_ => panic!(),
};
Ok(value)
AddressSpace::SystemIo => match access_size_bits {
8 => Ok(self.handler.read_io_u8(self.gas.address as u16) as u64),
16 => Ok(self.handler.read_io_u16(self.gas.address as u16) as u64),
32 => Ok(self.handler.read_io_u32(self.gas.address as u16) as u64),
_ => Err(AcpiError::InvalidGenericAddress),
},
_ => {
warn!("Read from GAS with address space {:?} is not supported. Ignored.", self.gas.address_space);
Err(AcpiError::LibUnimplemented)
}
_ => unimplemented!(),
}
}

Expand All @@ -188,16 +188,16 @@ where
let mapping = self.mapping.as_ref().unwrap();
match access_size_bits {
8 => unsafe {
core::ptr::write_volatile(mapping.virtual_start.as_ptr(), value as u8);
ptr::write_volatile(mapping.virtual_start.as_ptr(), value as u8);
},
16 => unsafe {
core::ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u16, value as u16);
ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u16, value as u16);
},
32 => unsafe {
core::ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u32, value as u32);
ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u32, value as u32);
},
64 => unsafe { core::ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u64, value) },
_ => panic!(),
64 => unsafe { ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u64, value) },
_ => return Err(AcpiError::InvalidGenericAddress),
}
Ok(())
}
Expand All @@ -206,11 +206,14 @@ where
8 => self.handler.write_io_u8(self.gas.address as u16, value as u8),
16 => self.handler.write_io_u16(self.gas.address as u16, value as u16),
32 => self.handler.write_io_u32(self.gas.address as u16, value as u32),
_ => panic!(),
_ => return Err(AcpiError::InvalidGenericAddress),
}
Ok(())
}
_ => unimplemented!(),
_ => {
warn!("Write to GAS with address space {:?} is not supported. Ignored.", self.gas.address_space);
Err(AcpiError::LibUnimplemented)
}
}
}
}
Expand All @@ -228,7 +231,7 @@ fn gas_decode_access_bit_width(gas: GenericAddress) -> Result<u8, AcpiError> {
*
* We use a third method, based on the alignment of the address, for registers that have
* non-zero bit offsets. These are not typically encountered in normal registers - they very
* often mean the GAS has come from APEI (ACPI Platform Error Interface), and so needs speical
* often mean the GAS has come from APEI (ACPI Platform Error Interface), and so needs special
* handling.
*/
if gas.bit_offset == 0 && [8, 16, 32, 64].contains(&gas.bit_width) {
Expand All @@ -242,7 +245,19 @@ fn gas_decode_access_bit_width(gas: GenericAddress) -> Result<u8, AcpiError> {
_ => Err(AcpiError::InvalidGenericAddress),
}
} else {
// TODO: work out access size based on alignment of the address
todo!()
/*
* Work out the access size based on the alignment of the address. We round up the total
* width (`bit_offset + bit_width`) to the next power-of-two, up to 64.
*/
let total_width = gas.bit_offset + gas.bit_width;
Ok(if total_width <= 8 {
8
} else if total_width <= 16 {
16
} else if total_width <= 32 {
32
} else {
64
})
}
}
96 changes: 82 additions & 14 deletions src/aml/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,17 @@ where
});
}
}
Opcode::Notify => {
// TODO: may need special handling on the node to get path?
let [Argument::Namestring(name), Argument::Object(value)] = &op.arguments[..] else {
panic!();
};
let value = value.as_integer()?;

info!("Notify {:?} with value {}", name, value);
// TODO: support
return Err(AmlError::LibUnimplemented);
}
Opcode::FromBCD => self.do_from_bcd(&mut context, op)?,
Opcode::ToBCD => self.do_to_bcd(&mut context, op)?,
Opcode::Name => {
Expand All @@ -581,6 +592,7 @@ where
let arg = arg.as_integer()?;
self.handler.handle_fatal_error(*typ, *code, arg);
context.retire_op(op);
return Err(AmlError::FatalErrorEncountered);
}
Opcode::OpRegion => {
let [
Expand Down Expand Up @@ -822,6 +834,35 @@ where
context.contribute_arg(Argument::Object(result));
context.retire_op(op);
}
Opcode::Load => {
let [Argument::Namestring(object), Argument::Object(result)] = &op.arguments[..] else {
panic!();
};
// TODO: read the AML from the object and load it
warn!("Ignoring unsupported DefLoad operation (object={}, result = {})", object, result);
context.retire_op(op);
return Err(AmlError::LibUnimplemented);
}
Opcode::LoadTable => {
let [
Argument::Object(signature),
Argument::Object(oem_id),
Argument::Object(oem_table_id),
Argument::Object(root_path),
Argument::Object(parameter_path),
Argument::Object(parameter_data),
] = &op.arguments[..]
else {
panic!();
};
// TODO: search for the table in the RSDT/XSDT and load the contained AML
warn!(
"Ignoring unsupported DefLoadTable operation (signature = {}, oem_id = {}, oem_table_id = {}, root_path = {}, parameter_path = {}, parameter_data = {})",
signature, oem_id, oem_table_id, root_path, parameter_path, parameter_data
);
context.retire_op(op);
return Err(AmlError::LibUnimplemented);
}
Opcode::Sleep => {
let [Argument::Object(msec)] = &op.arguments[..] else { panic!() };
self.handler.sleep(msec.as_integer()?);
Expand Down Expand Up @@ -936,7 +977,7 @@ where
// XXX: 15 is reserved
ObjectType::Debug => 16,
ObjectType::Reference => panic!(),
ObjectType::RawDataBuffer => todo!(),
ObjectType::RawDataBuffer => 17,
};

context.contribute_arg(Argument::Object(Object::Integer(typ).wrap()));
Expand Down Expand Up @@ -1266,8 +1307,17 @@ where
let name = name.resolve(&context.current_scope)?;
self.namespace.lock().insert(name, Object::Event(Arc::new(AtomicU64::new(0))).wrap())?;
}
Opcode::LoadTable => todo!(),
Opcode::Load => todo!(),
Opcode::LoadTable => {
context.start(OpInFlight::new(Opcode::LoadTable, &[ResolveBehaviour::TermArg; 6]));
}
Opcode::Load => {
let name = context.namestring()?;
context.start(OpInFlight::new_with(
Opcode::Load,
vec![Argument::Namestring(name)],
&[ResolveBehaviour::Target],
));
}
Opcode::Stall => context.start(OpInFlight::new(Opcode::Stall, &[ResolveBehaviour::TermArg])),
Opcode::Sleep => context.start(OpInFlight::new(Opcode::Sleep, &[ResolveBehaviour::TermArg])),
Opcode::Acquire => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])),
Expand Down Expand Up @@ -1710,7 +1760,7 @@ where
kind: FieldUnitKind,
start_pc: usize,
pkg_length: usize,
flags: u8,
mut flags: u8,
) -> Result<(), AmlError> {
const RESERVED_FIELD: u8 = 0x00;
const ACCESS_FIELD: u8 = 0x01;
Expand All @@ -1731,18 +1781,22 @@ where
* elements. They change the access type and attributes for remaining fields in
* the list.
*/
let _access_type = context.next()?;
let access_type = context.next()?;
let _access_attrib = context.next()?;
todo!()
flags.set_bits(0..4, access_type);
}
EXTENDED_ACCESS_FIELD => {
let access_type = context.next()?;
let _extended_access_attrib = context.next()?;
let _access_length = context.next()?;
flags.set_bits(0..4, access_type);
warn!("Ignoring extended attributes and length in ExtendedAccessField");
}
CONNECT_FIELD => {
// TODO: either consume a namestring or `BufferData` (it's not
// clear what a buffer data acc is lmao)
todo!("Connect field :(");
}
EXTENDED_ACCESS_FIELD => {
todo!("Extended access field :(");
}
_ => {
context.current_block.pc -= 1;
// TODO: this should only ever be a nameseg really...
Expand Down Expand Up @@ -2123,8 +2177,7 @@ where
Object::Package(_) => "[Package]".to_string(),
Object::PowerResource { .. } => "[Power Resource]".to_string(),
Object::Processor { .. } => "[Processor]".to_string(),
// TODO: what even is one of these??
Object::RawDataBuffer => todo!(),
Object::RawDataBuffer => "[Raw Data Buffer]".to_string(),
Object::String(value) => value.clone(),
Object::ThermalZone => "[Thermal Zone]".to_string(),
Object::Debug => "[Debug Object]".to_string(),
Expand Down Expand Up @@ -2339,10 +2392,21 @@ where
_ => panic!("Stores to objects like {:?} are not yet supported", target),
},

Argument::Namestring(_) => todo!(),
Argument::ByteData(_) | Argument::DWordData(_) | Argument::TrackedPc(_) | Argument::PkgLength(_) => {
panic!()
Argument::Namestring(name) => {
let existing = self.namespace.lock().get(name.clone());
match existing {
Ok(existing) => {
unsafe {
// TODO: this should likely be doing an implicit cast depending on object type?
*existing.gain_mut(&token) = (*object).clone();
}
}
Err(_) => {
self.namespace.lock().insert(name.clone(), object)?;
}
}
}
_ => panic!("Invalid argument type as DefStore target"),
}

Ok(to_return)
Expand Down Expand Up @@ -3330,6 +3394,10 @@ pub enum AmlError {
PrtInvalidSource,
PrtNoEntry,

/// An OEM-defined fatal error has occured. The specification states a host should log this
/// fatal error and then shutdown in a timely fashion.
FatalErrorEncountered,

/// This is emitted to signal that the library does not support the requested behaviour. This
/// should eventually never be emitted.
LibUnimplemented,
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ use core::{
pin::Pin,
ptr::NonNull,
};
use log::warn;
use log::{error, warn};
use rsdp::Rsdp;

/// `AcpiTables` should be constructed after finding the RSDP or RSDT/XSDT and allows enumeration
Expand Down Expand Up @@ -480,7 +480,7 @@ pub trait Handler: Clone {

#[cfg(feature = "aml")]
fn handle_fatal_error(&self, fatal_type: u8, fatal_code: u32, fatal_arg: u64) {
panic!(
error!(
"Fatal error while executing AML (encountered DefFatalOp). fatal_type = {}, fatal_code = {}, fatal_arg = {}",
fatal_type, fatal_code, fatal_arg
);
Expand Down
Loading