From 9d9e74d881500b581037b2cca61d0fe35b41a205 Mon Sep 17 00:00:00 2001 From: Josh Junon Date: Fri, 6 Sep 2024 08:06:01 +0200 Subject: [PATCH] type+acpi+acpica-sys+x86_64: wrap ACPI structures with `Le` type --- Cargo.lock | 1 + oro-acpi/src/lib.rs | 20 ++++---- oro-acpi/src/madt.rs | 16 +++---- oro-acpica-sys/Cargo.toml | 3 ++ oro-acpica-sys/build.rs | 82 ++++++++++++++++++++++++++++++--- oro-arch-x86_64/src/boot/mod.rs | 12 +++-- oro-type/src/lib.rs | 21 +++++++++ 7 files changed, 126 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f5b179b5..60f5f9b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -172,6 +172,7 @@ version = "20240322.0.2" dependencies = [ "bindgen", "convert_case", + "oro-type", "quote", "syn", ] diff --git a/oro-acpi/src/lib.rs b/oro-acpi/src/lib.rs index b0ca99a3..62b4f5a4 100644 --- a/oro-acpi/src/lib.rs +++ b/oro-acpi/src/lib.rs @@ -34,7 +34,6 @@ impl Rsdp

{ /// Caller must ensure that the physical address is valid and points /// to a valid RSDP structure. This typically means making the assumption /// the bootloader has done so, but we still must mark this as unsafe. - #[allow(clippy::missing_panics_doc)] // TODO(qix-): Remove once numbers commons lib is implemented pub unsafe fn get(physical_address: u64, pat: P) -> Option { #[doc(hidden)] const LAYOUT: Layout = Layout::new::(); @@ -60,11 +59,11 @@ impl Rsdp

{ return None; } - if ptr.Revision > 0 { + if ptr.Revision.read() > 0 { // Perform an extended checksum // SAFETY(qix-): The length field is only valid for revisions > 0. let mut checksum: u8 = 0; - for i in 0..u32::from_le(ptr.Length).try_into().unwrap() { + for i in 0..(ptr.Length.read() as usize) { checksum = checksum.wrapping_add(from_ref(ptr).cast::().add(i).read()); } if checksum != 0 { @@ -77,7 +76,7 @@ impl Rsdp

{ /// Gets the revision. pub fn revision(&self) -> u8 { - self.ptr.Revision + self.ptr.Revision.read() } /// Gets the (X)SDT. @@ -87,12 +86,15 @@ impl Rsdp

{ if self.revision() == 0 { // SAFETY(qix-): We've made sure we're casting to the right type. Some(RootSdt::Rsdt(unsafe { - Rsdt::new(u64::from(self.ptr.RsdtPhysicalAddress), self.pat.clone())? + Rsdt::new( + u64::from(self.ptr.RsdtPhysicalAddress.read()), + self.pat.clone(), + )? })) } else { // SAFETY(qix-): We've made sure we're casting to the right type. Some(RootSdt::Xsdt(unsafe { - Xsdt::new(self.ptr.XsdtPhysicalAddress, self.pat.clone())? + Xsdt::new(self.ptr.XsdtPhysicalAddress.read(), self.pat.clone())? })) } } @@ -192,7 +194,7 @@ pub trait AcpiTable: Sized { } let mut checksum = 0_u8; - for i in 0..u32::from_le(header.Length) { + for i in 0..header.Length.read() { assert::fits_within::(); checksum = checksum.wrapping_add(from_ref(ptr).cast::().add(i as usize).read()); } @@ -244,8 +246,8 @@ pub trait AcpiTable: Sized { // SAFETY(qix-): We perform a static assertion to make sure the convertion // SAFETY(qix-): from u32 to usize won't truncate. assert::fits_within::(); - let len = u32::from_le(header.Length) as usize - - core::mem::size_of::(); + let len = + header.Length.read() as usize - core::mem::size_of::(); let data_base = core::ptr::from_ref(header).add(1).cast::(); return core::slice::from_raw_parts(data_base, len); } diff --git a/oro-acpi/src/madt.rs b/oro-acpi/src/madt.rs index 51f17833..aadc5b86 100644 --- a/oro-acpi/src/madt.rs +++ b/oro-acpi/src/madt.rs @@ -13,13 +13,13 @@ impl crate::Madt

{ /// Returns whether or not the 8259 PIC is present in the MADT. #[must_use] pub fn has_8259(&self) -> bool { - self.ptr.Flags & PCAT_COMPAT != 0 + self.ptr.Flags.read() & PCAT_COMPAT != 0 } /// Returns the physical address of the local APIC. #[must_use] pub fn lapic_phys(&self) -> u64 { - u64::from(u32::from_le(self.ptr.Address)) + u64::from(self.ptr.Address.read()) } /// Returns an iterator over all of the MADT entries. @@ -31,7 +31,7 @@ impl crate::Madt

{ unsafe { core::slice::from_raw_parts( core::ptr::from_ref(self.ptr).cast::(), - self.ptr.Header.Length as usize, + self.ptr.Header.Length.read() as usize, ) }, ) @@ -68,10 +68,10 @@ impl<'a> Iterator for MadtIterator<'a> { } let un = unsafe { &*(core::ptr::from_ref(&self.slice[self.pos]).cast::()) }; - assert!(unsafe { un.header.Length as usize } <= self.slice.len() - self.pos); + assert!(unsafe { un.header.Length.read() as usize } <= self.slice.len() - self.pos); let pos = self.pos; - self.pos += unsafe { un.header.Length as usize }; + self.pos += unsafe { un.header.Length.read() as usize }; Some(match un.into() { Some(entry) => Ok(entry), @@ -106,7 +106,7 @@ macro_rules! madt_entries { impl<'a> From<&'a MadtData> for Option> { fn from(data: &'a MadtData) -> Option> { - Some(match unsafe { data.header.Type } { + Some(match unsafe { data.header.Type.read() } { $( $tyid => MadtEntry::%%(unsafe { &data.$name }), )* @@ -150,14 +150,14 @@ pub trait LocalApicEx { /// Returns the local APIC ID. fn id(&self) -> u8 { - u8::from_le(self.inner_ref().Id) + self.inner_ref().Id.read() } /// Returns whether or not this CPU can be initialized. /// If this returns `false`, this entry should be ignored /// and the boot routine should not attempt any initialization. fn can_init(&self) -> bool { - self.inner_ref().LapicFlags & 3 != 0 + self.inner_ref().LapicFlags.read() & 3 != 0 } } diff --git a/oro-acpica-sys/Cargo.toml b/oro-acpica-sys/Cargo.toml index 78159815..c8e3e9e3 100644 --- a/oro-acpica-sys/Cargo.toml +++ b/oro-acpica-sys/Cargo.toml @@ -8,6 +8,9 @@ publish = false build = "build.rs" +[dependencies] +oro-type.workspace = true + [build-dependencies] bindgen.workspace = true quote.workspace = true diff --git a/oro-acpica-sys/build.rs b/oro-acpica-sys/build.rs index 43d8746a..de41edbb 100644 --- a/oro-acpica-sys/build.rs +++ b/oro-acpica-sys/build.rs @@ -30,6 +30,8 @@ fn main() { .use_core() .disable_nested_struct_naming() .rust_target(::bindgen::RustTarget::Nightly) + .size_t_is_usize(true) + .translate_enum_integer_types(true) .detect_include_paths(true); #[cfg(target_arch = "x86_64")] @@ -49,28 +51,30 @@ fn main() { let bindings = bindings.generate().expect("unable to generate bindings"); - bindings - .write_to_file(dest_path) - .expect("unable to write bindings"); - let macro_dest_path = std::path::Path::new(&out_dir).join("tablegen_macro.rs"); let mut buf = Vec::with_capacity(1024 * 1024 * 10); bindings .write(Box::from(&mut buf)) .expect("unable to write bindings to string"); + let src = String::from_utf8(buf).expect("bindings are not utf-8"); - let bindings = ::syn::parse_file(&src).expect("unable to parse bindings"); - let macr = generate_tablegen_macro(bindings).expect("unable to generate tablegen macro"); + let mut bindings = ::syn::parse_file(&src).expect("unable to parse bindings"); + + wrap_table_types(&mut bindings).expect("unable to wrap table types"); + let macr = generate_tablegen_macro(&bindings).expect("unable to generate tablegen macro"); std::fs::write( macro_dest_path, macr.to_token_stream().to_string().as_bytes(), ) .expect("unable to write tablegen macro"); + + std::fs::write(dest_path, bindings.to_token_stream().to_string().as_bytes()) + .expect("unable to write wrapped type bindings"); } -fn generate_tablegen_macro(bindings: ::syn::File) -> ::syn::Result { +fn generate_tablegen_macro(bindings: &::syn::File) -> ::syn::Result { let mut strukts = std::collections::HashMap::new(); for item in &bindings.items { @@ -171,3 +175,67 @@ fn generate_tablegen_macro(bindings: ::syn::File) -> ::syn::Result syn::Result<()> { + for item in &mut bindings.items { + if let syn::Item::Struct(strukt) = item { + // Skip any structures that aren't ACPI tables / the header. + // + // NOTE(qix-): I'm tentatively disabling this check because it's + // NOTE(qix-): becoming apparent that all structs need to be wrapped. + // NOTE(qix-): If this breaks something we'll have to get a bit + // NOTE(qix-): more clever about it. + // + // let struct_ident = strukt.ident.to_string(); + // if !struct_ident.starts_with("acpi_table_") { + // continue; + // } + + // For every field in the structure, if it's one of the primitive numeric + // types (including f32 and f64 but not `bool`) then wrap the `foo: T` (where + // `T` is the primitive type) with `Le`. + for field in &mut strukt.fields { + match field.ty { + syn::Type::Path(ref mut path) => { + if path.path.segments.len() != 1 { + continue; + } + + if path.qself.is_some() { + continue; + } + + let first = path.path.segments.first().unwrap(); + if !first.arguments.is_empty() { + continue; + } + + let ident = first.ident.to_string(); + if !matches!( + ident.as_str(), + "u8" | "u16" + | "u32" | "u64" | "u128" | "usize" + | "i8" | "i16" | "i32" | "i64" + | "i128" | "isize" | "UINT32" + | "UINT64" | "UINT8" | "UINT16" + | "UINT128" | "UINTPTR" | "INT32" + | "INT64" | "INT8" | "INT16" + | "INT128" + ) { + continue; + } + } + _ => { + continue; + } + } + + // It's a numeric primitive; wrap it in `Le`. + let ty = &field.ty; + field.ty = syn::parse_quote!(::oro_type::Le<#ty>); + } + } + } + + Ok(()) +} diff --git a/oro-arch-x86_64/src/boot/mod.rs b/oro-arch-x86_64/src/boot/mod.rs index 720ec9d6..d69b09ce 100644 --- a/oro-arch-x86_64/src/boot/mod.rs +++ b/oro-arch-x86_64/src/boot/mod.rs @@ -79,18 +79,20 @@ pub unsafe fn boot_primary() -> ! { let fadt = fadt.inner_ref(); // Enable ACPI if need be. - if (fadt.Flags & acpi_sys::ACPI_FADT_HW_REDUCED) == 0 - && !(fadt.SmiCommand == 0 && fadt.AcpiEnable == 0 && (fadt.Pm1aControlBlock & 1) != 0) + if (fadt.Flags.read() & acpi_sys::ACPI_FADT_HW_REDUCED) == 0 + && !(fadt.SmiCommand.read() == 0 + && fadt.AcpiEnable.read() == 0 + && (fadt.Pm1aControlBlock.read() & 1) != 0) { dbg!("enabling ACPI"); crate::asm::outb( - u16::try_from(fadt.SmiCommand) + u16::try_from(fadt.SmiCommand.read()) .expect("ACPI provided an SMI command port that was too large"), - fadt.AcpiEnable, + fadt.AcpiEnable.read(), ); dbg!("enabled ACPI; waiting for it to take effect..."); - let pma1 = u16::try_from(fadt.Pm1aControlBlock) + let pma1 = u16::try_from(fadt.Pm1aControlBlock.read()) .expect("ACPI provided a PM1A control block port that was too large"); while (crate::asm::inw(pma1) & 1) == 0 { core::hint::spin_loop(); diff --git a/oro-type/src/lib.rs b/oro-type/src/lib.rs index f4301b2f..d7191958 100644 --- a/oro-type/src/lib.rs +++ b/oro-type/src/lib.rs @@ -30,6 +30,25 @@ impl Endian { pub const fn with_unchanged(value: T) -> Self { Self(value, PhantomData) } + + /// Reads the value. + /// + /// The same as calling `.into()` but doesn't + /// require type annotations. + #[inline(always)] + #[must_use] + pub fn read(self) -> T { + E::from_endian(self.0) + } + + /// Writes a new value. + /// + /// The same as calling `.from()` but doesn't + /// require type annotations. + #[inline(always)] + pub fn write(&mut self, value: T) { + self.0 = E::to_endian(value); + } } /// Specifies the endianness when using [`Endian`]. @@ -42,6 +61,7 @@ pub trait Endianness { } /// Little-endian endianness. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default)] pub struct LittleEndian; impl Endianness for LittleEndian { @@ -57,6 +77,7 @@ impl Endianness for LittleEndian { } /// Big-endian endianness. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default)] pub struct BigEndian; impl Endianness for BigEndian {