Skip to content

Add functions to get and clear pending events for Pm1EventRegisterBlock #325

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

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

Add functions to get and clear pending events for Pm1EventRegisterBlock #325

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
26 changes: 26 additions & 0 deletions src/address.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,32 @@ where
}
}
}

/// Ignores the GAS access size and does a 16-bit read.
pub fn read_u16(&self, byte_offset: u64) -> u16 {

@IsaacWoods IsaacWoods Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if a cleaner way of doing this would be to have a method to derive a GenericAddress with a different access width from another one? (i.e. 'split' the GAS into the different registers?

@ChocolateLoverRaj ChocolateLoverRaj Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We could do that. Then we could make functions for getting the status register and getting the control register. Would the derived GAS use lifetimes or Arc?

@martin-hughes martin-hughes Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a random thought - splitting a MappedGas has the potential to create fun with PhysicalMapping::drop, because you'll need to ensure the PhysicalMapping gets dropped only once. So if you create two new MappedGas objects you could:

  • Create two new PhysicalMapping objects, one for each child. This would be wasteful but safe (I think)
  • Wrap the child MappedGas Handlers in ManuallyDrop or similar - but then the split MappedGas objects have a different type to the originals.

I suppose what I'm trying to say is to be careful 😄

@IsaacWoods IsaacWoods Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm yes, very true Martin 😬

I wonder if the safest way would be to have a 'cast' effectively fn cast(self) -> MappedGas and some sort of way to duplicate a mapping, but that does seem inefficient? Maybe this isn't so bad after all?

match self.gas.address_space {
AddressSpace::SystemMemory => {
let addr = self.mapping.as_ref().unwrap().virtual_start.cast::<u16>();
unsafe { addr.byte_offset(byte_offset as isize).read_unaligned() }
}
AddressSpace::SystemIo => self.handler.read_io_u16(self.gas.address as u16 + byte_offset as u16),
address_space => todo!("{address_space:?}"),
}
}

/// Ignores the GAS access size and does a 16-bit write.
pub fn write_u16(&self, byte_offset: u64, value: u16) {
match self.gas.address_space {
AddressSpace::SystemMemory => {
let addr = self.mapping.as_ref().unwrap().virtual_start.cast::<u16>();
unsafe { addr.byte_offset(byte_offset as isize).write_unaligned(value) }
}
AddressSpace::SystemIo => {
self.handler.write_io_u16(self.gas.address as u16 + byte_offset as u16, value)
}
address_space => todo!("{address_space:?}"),
}
}
}

/// Returns the access size that should be made for a given `GenericAddress`, in bits.
Expand Down
33 changes: 33 additions & 0 deletions src/registers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{AcpiError, Handler, address::MappedGas, sdt::fadt::Fadt};
use bit_field::BitField;
use bitflags::{Flags, bitflags};

pub struct FixedRegisters<H: Handler> {
pub pm1_event_registers: Pm1EventRegisterBlock<H>,
Expand Down Expand Up @@ -58,6 +59,19 @@ pub enum Pm1Event {
Wake = 15,
}

bitflags! {
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Pm1EventFlags: u16 {
const TIMER = 1 << 0;
const GLOBAL_LOCK = 1 << 5;
const POWER_BUTTON = 1 << 8;
const SLEEP_BUTTON = 1 << 9;
const RTC = 1 << 10;
const PCIE_WAKE = 1 << 14;
const WAKE = 1 << 15;
}
}

impl<H> Pm1EventRegisterBlock<H>
where
H: Handler,
Expand Down Expand Up @@ -95,6 +109,25 @@ where

Ok(pm1a | pm1b)
}

pub fn pending_events(&self) -> Pm1EventFlags {
let bits = {
let mut bits = self.pm1a.read_u16(0);
if let Some(pm1b) = &self.pm1b {
bits |= pm1b.read_u16(0);
}
bits
};
Pm1EventFlags::from_bits_retain(bits)
}

pub fn clear_events(&self, events: Pm1EventFlags) {

@IsaacWoods IsaacWoods Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I haven't read the spec on this in quite a while (but remember wiring this up locally ages ago) - is it a useful API to clear all events, or would it be better to clear a specific event that you've handled (I assume by writing with an individual bit set)?

@ChocolateLoverRaj ChocolateLoverRaj Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The way I have it now where you specify which events to clear is better. If you clear all events, you might miss an event.

@IsaacWoods IsaacWoods Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Apologies, I somehow misread this as clearing all the events. Yes, this is good.

let bits = events.known_bits();
self.pm1a.write_u16(0, bits);
if let Some(pm1b) = &self.pm1b {
pm1b.write_u16(0, bits);
}
}
}

pub struct Pm1ControlRegisterBlock<H: Handler> {
Expand Down