From d419d42677562f8aafe3dbff42009766721232fb Mon Sep 17 00:00:00 2001 From: Arthur Heymans Date: Fri, 10 Jul 2026 07:08:03 +0200 Subject: [PATCH 1/3] Add configurable 64-bit register access Allow callers to select native, low-high, or high-low access for xHCI 64-bit MMIO registers. Keep native access as the default for existing users while making split access available for platforms and controllers that cannot accept native 64-bit MMIO operations. --- CHANGELOG.md | 1 + src/registers/mod.rs | 31 ++++- src/registers/operational.rs | 52 +++++++- src/registers/register64.rs | 244 +++++++++++++++++++++++++++++++++++ src/registers/runtime.rs | 67 ++++++++-- 5 files changed, 377 insertions(+), 18 deletions(-) create mode 100644 src/registers/register64.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index faad3eea..649a6281 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased - ReleaseDate ### Added +- Configurable native, low-high, or high-low access for 64-bit registers. - `registers::Doorbell` as an alias of `registers::doorbell::Doorbell`. ([#170]) ### Changed diff --git a/src/registers/mod.rs b/src/registers/mod.rs index 53f436f3..5149fa11 100644 --- a/src/registers/mod.rs +++ b/src/registers/mod.rs @@ -6,12 +6,14 @@ use accessor::Mapper; pub use capability::Capability; pub use doorbell::Doorbell; pub use operational::{Operational, PortRegisterSet}; +pub use register64::Access64; pub use runtime::InterrupterRegisterSet; pub use runtime::Runtime; pub mod capability; pub mod doorbell; pub mod operational; +pub mod register64; pub mod runtime; /// The access point to xHCI registers. @@ -75,14 +77,35 @@ where /// let r = unsafe { xhci::Registers::new(MMIO_BASE, mapper) }; /// ``` pub unsafe fn new(mmio_base: usize, mapper: M) -> Self { + Self::new_with_64bit_access(mmio_base, mapper, Access64::Native) + } + + /// Creates an instance of [`Registers`] with the selected access mode for 64-bit registers. + /// + /// # Safety + /// + /// The caller must ensure that the xHCI registers are accessed only through this struct. + /// + /// # Panics + /// + /// This method panics if `mmio_base` is not aligned correctly. + pub unsafe fn new_with_64bit_access(mmio_base: usize, mapper: M, access64: Access64) -> Self { let capability = Capability::new(mmio_base, &mapper); let doorbell = Doorbell::new(mmio_base, &capability, mapper.clone()); - let operational = - Operational::new(mmio_base, capability.caplength.read_volatile(), &mapper); + let operational = Operational::new_with_64bit_access( + mmio_base, + capability.caplength.read_volatile(), + &mapper, + access64, + ); let port_register_set = PortRegisterSet::new(mmio_base, &capability, mapper.clone()); let runtime = Runtime::new(mmio_base, capability.rtsoff.read_volatile(), mapper.clone()); - let interrupter_register_set = - InterrupterRegisterSet::new(mmio_base, capability.rtsoff.read_volatile(), mapper); + let interrupter_register_set = InterrupterRegisterSet::new_with_64bit_access( + mmio_base, + capability.rtsoff.read_volatile(), + mapper, + access64, + ); Self { capability, diff --git a/src/registers/operational.rs b/src/registers/operational.rs index 02f1050f..8ae3e33b 100644 --- a/src/registers/operational.rs +++ b/src/registers/operational.rs @@ -1,6 +1,7 @@ //! Host Controller Operational Registers use super::capability::{Capability, CapabilityRegistersLength}; +use super::register64::{self, Access64}; use accessor::array; use accessor::single; use accessor::Mapper; @@ -27,9 +28,9 @@ where /// Device Notification Control pub dnctrl: single::ReadWrite, /// Command Ring Control Register - pub crcr: single::ReadWrite, + pub crcr: register64::ReadWrite, /// Device Context Base Address Array Pointer Register - pub dcbaap: single::ReadWrite, + pub dcbaap: register64::ReadWrite, /// Configure Register pub config: single::ReadWrite, } @@ -49,6 +50,29 @@ where /// This method panics if the base address of the Host Controller Operational Registers is not /// aligned correctly. pub unsafe fn new(mmio_base: usize, caplength: CapabilityRegistersLength, mapper: &M) -> Self + where + M: Mapper, + { + Self::new_with_64bit_access(mmio_base, caplength, mapper, Access64::Native) + } + + /// Creates a new accessor with the selected access mode for 64-bit registers. + /// + /// # Safety + /// + /// The caller must ensure that the Host Controller Operational Registers are accessed only + /// through this struct. + /// + /// # Panics + /// + /// This method panics if the base address of the Host Controller Operational Registers is not + /// aligned correctly. + pub unsafe fn new_with_64bit_access( + mmio_base: usize, + caplength: CapabilityRegistersLength, + mapper: &M, + access64: Access64, + ) -> Self where M: Mapper, { @@ -65,8 +89,8 @@ where usbsts: m!(0x04), pagesize: m!(0x08), dnctrl: m!(0x14), - crcr: m!(0x18), - dcbaap: m!(0x30), + crcr: register64::ReadWrite::new(base + 0x18, access64, mapper.clone()), + dcbaap: register64::ReadWrite::new(base + 0x30, access64, mapper.clone()), config: m!(0x38), } } @@ -211,6 +235,16 @@ impl DeviceNotificationControl { #[repr(transparent)] #[derive(Copy, Clone)] pub struct CommandRingControlRegister(u64); +impl From for CommandRingControlRegister { + fn from(value: u64) -> Self { + Self(value) + } +} +impl From for u64 { + fn from(value: CommandRingControlRegister) -> Self { + value.0 + } +} impl CommandRingControlRegister { wo_bit!(0, ring_cycle_state, "Ring Cycle State"); w1s_bit!(1, command_stop, "Command Stop"); @@ -242,6 +276,16 @@ impl_debug_from_methods! { #[repr(transparent)] #[derive(Copy, Clone, Debug, Default)] pub struct DeviceContextBaseAddressArrayPointerRegister(u64); +impl From for DeviceContextBaseAddressArrayPointerRegister { + fn from(value: u64) -> Self { + Self(value) + } +} +impl From for u64 { + fn from(value: DeviceContextBaseAddressArrayPointerRegister) -> Self { + value.0 + } +} impl DeviceContextBaseAddressArrayPointerRegister { /// Returns the value of the Device Context Base Address Array Pointer. #[must_use] diff --git a/src/registers/register64.rs b/src/registers/register64.rs new file mode 100644 index 00000000..b71949e1 --- /dev/null +++ b/src/registers/register64.rs @@ -0,0 +1,244 @@ +//! Accessors for 64-bit xHCI registers. + +use accessor::marker::{self, AccessorTypeSpecifier, Readable, Writable}; +use accessor::Mapper; +use core::fmt; +use core::marker::PhantomData; +use core::mem::{align_of, size_of}; +use core::ptr; + +/// The access width and ordering used for 64-bit xHCI registers. +/// +/// xHCI permits 64-bit registers to be accessed as two 32-bit operations on +/// platforms that cannot issue 64-bit MMIO accesses. Some controllers require +/// a specific order for those operations. +#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)] +pub enum Access64 { + /// Access the register with one 64-bit operation. + #[default] + Native, + /// Access the low 32 bits before the high 32 bits. + LowHigh, + /// Access the high 32 bits before the low 32 bits. + HighLow, +} + +/// A readable and writable 64-bit register accessor. +pub type ReadWrite = Generic; + +/// A read-only 64-bit register accessor. +pub type ReadOnly = Generic; + +/// An accessor for a 64-bit xHCI register. +pub struct Generic +where + M: Mapper, + A: AccessorTypeSpecifier, +{ + virt: usize, + bytes: usize, + access: Access64, + mapper: M, + _register: PhantomData, + _accessor: PhantomData, +} + +impl Generic +where + M: Mapper, + A: AccessorTypeSpecifier, +{ + pub(crate) unsafe fn new(phys_base: usize, access: Access64, mut mapper: M) -> Self { + assert_eq!(size_of::(), size_of::()); + let alignment = match access { + Access64::Native => align_of::(), + Access64::LowHigh | Access64::HighLow => align_of::(), + }; + assert_eq!(phys_base % alignment, 0, "base is not aligned"); + + let bytes = size_of::(); + let virt = mapper.map(phys_base, bytes).get(); + Self { + virt, + bytes, + access, + mapper, + _register: PhantomData, + _accessor: PhantomData, + } + } +} + +impl Generic +where + T: From, + M: Mapper, + A: AccessorTypeSpecifier + Readable, +{ + /// Reads the register using the configured access width and ordering. + pub fn read_volatile(&self) -> T { + let raw = unsafe { + match self.access { + Access64::Native => return ptr::read_volatile(self.virt as *const T), + Access64::LowHigh => { + let low = ptr::read_volatile(self.virt as *const u32); + let high = ptr::read_volatile((self.virt + 4) as *const u32); + u64::from(low) | (u64::from(high) << 32) + } + Access64::HighLow => { + let high = ptr::read_volatile((self.virt + 4) as *const u32); + let low = ptr::read_volatile(self.virt as *const u32); + u64::from(low) | (u64::from(high) << 32) + } + } + }; + raw.into() + } + + /// Alias of [`Generic::read_volatile`]. + #[deprecated(since = "0.9.3", note = "use `read_volatile`")] + pub fn read(&self) -> T { + self.read_volatile() + } +} + +impl Generic +where + T: Into, + M: Mapper, + A: AccessorTypeSpecifier + Writable, +{ + /// Writes the register using the configured access width and ordering. + pub fn write_volatile(&mut self, value: T) { + unsafe { + match self.access { + Access64::Native => ptr::write_volatile(self.virt as *mut T, value), + Access64::LowHigh => { + let raw = value.into(); + let low = u32::try_from(raw & u64::from(u32::MAX)).unwrap(); + let high = u32::try_from(raw >> 32).unwrap(); + ptr::write_volatile(self.virt as *mut u32, low); + ptr::write_volatile((self.virt + 4) as *mut u32, high); + } + Access64::HighLow => { + let raw = value.into(); + let low = u32::try_from(raw & u64::from(u32::MAX)).unwrap(); + let high = u32::try_from(raw >> 32).unwrap(); + ptr::write_volatile((self.virt + 4) as *mut u32, high); + ptr::write_volatile(self.virt as *mut u32, low); + } + } + } + } + + /// Alias of [`Generic::write_volatile`]. + #[deprecated(since = "0.9.3", note = "use `write_volatile`")] + pub fn write(&mut self, value: T) { + self.write_volatile(value); + } +} + +impl Generic +where + T: From + Into, + M: Mapper, + A: AccessorTypeSpecifier + Readable + Writable, +{ + /// Updates the register using the configured access width and ordering. + pub fn update_volatile(&mut self, update: F) + where + F: FnOnce(&mut T), + { + let mut value = self.read_volatile(); + update(&mut value); + self.write_volatile(value); + } + + /// Alias of [`Generic::update_volatile`]. + #[deprecated(since = "0.9.3", note = "use `update_volatile`")] + pub fn update(&mut self, update: F) + where + F: FnOnce(&mut T), + { + self.update_volatile(update); + } +} + +impl fmt::Debug for Generic +where + T: From + fmt::Debug, + M: Mapper, + A: AccessorTypeSpecifier + Readable, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.read_volatile().fmt(f) + } +} + +impl Drop for Generic +where + M: Mapper, + A: AccessorTypeSpecifier, +{ + fn drop(&mut self) { + self.mapper.unmap(self.virt, self.bytes); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use core::num::NonZeroUsize; + + #[derive(Clone, Copy)] + struct IdentityMapper; + + impl Mapper for IdentityMapper { + unsafe fn map(&mut self, phys_start: usize, _bytes: usize) -> NonZeroUsize { + NonZeroUsize::new(phys_start).unwrap() + } + + fn unmap(&mut self, _virt_start: usize, _bytes: usize) {} + } + + #[repr(transparent)] + #[derive(Debug, Eq, PartialEq)] + struct TestRegister(u64); + + impl From for TestRegister { + fn from(value: u64) -> Self { + Self(value) + } + } + + impl From for u64 { + fn from(value: TestRegister) -> Self { + value.0 + } + } + + #[test] + fn access_modes_read_and_write_the_complete_register() { + for access in [Access64::Native, Access64::LowHigh, Access64::HighLow] { + let mut value = 0u64; + let pointer = ptr::from_mut(&mut value); + let mut register = unsafe { + ReadWrite::::new( + pointer as usize, + access, + IdentityMapper, + ) + }; + + register.write_volatile(TestRegister(0x1122_3344_5566_7788)); + assert_eq!(value, 0x1122_3344_5566_7788); + assert_eq!( + register.read_volatile(), + TestRegister(0x1122_3344_5566_7788) + ); + + register.update_volatile(|value| value.0 ^= u64::MAX); + assert_eq!(value, 0xeedd_ccbb_aa99_8877); + } + } +} diff --git a/src/registers/runtime.rs b/src/registers/runtime.rs index 159e8471..560e3925 100644 --- a/src/registers/runtime.rs +++ b/src/registers/runtime.rs @@ -1,6 +1,7 @@ //! Host Controller Runtime Registers. use super::capability::RuntimeRegisterSpaceOffset; +use super::register64::{self, Access64}; use accessor::marker::AccessorTypeSpecifier; use accessor::marker::ReadOnly; use accessor::marker::ReadWrite; @@ -68,6 +69,7 @@ where { base: usize, mapper: M, + access64: Access64, } impl InterrupterRegisterSet @@ -86,10 +88,35 @@ where /// This method panics if the base address of the Interrupter Register Sets is not aligned /// correctly. pub unsafe fn new(mmio_base: usize, rtoff: RuntimeRegisterSpaceOffset, mapper: M) -> Self { + Self::new_with_64bit_access(mmio_base, rtoff, mapper, Access64::Native) + } + + /// Creates an accessor with the selected access mode for 64-bit registers. + /// + /// # Safety + /// + /// The caller must ensure that the Host Controller Runtime Registers are accessed only through + /// this struct. + /// + /// # Panics + /// + /// This method panics if the base address of the Interrupter Register Sets is not aligned + /// correctly. + #[allow(clippy::too_many_arguments)] + pub unsafe fn new_with_64bit_access( + mmio_base: usize, + rtoff: RuntimeRegisterSpaceOffset, + mapper: M, + access64: Access64, + ) -> Self { let base = mmio_base + usize::try_from(rtoff.get()).unwrap() + 0x20; assert!(base % 0x20 == 0, "base is not aligned"); - Self { base, mapper } + Self { + base, + mapper, + access64, + } } /// Returns a handler for an interrupter. @@ -98,7 +125,8 @@ where /// /// This method panics if `index > 1023`. pub fn interrupter(&self, index: usize) -> Interrupter<'_, M, ReadOnly> { - unsafe { Interrupter::new(self.base, index, self.mapper.clone()) } + assert!(index < 1024, "index out of range"); + unsafe { Interrupter::new(self.base + index * 0x20, self.mapper.clone(), self.access64) } } /// Returns a mutable handler for an interrupter. @@ -107,7 +135,8 @@ where /// /// This method panics if `index > 1023`. pub fn interrupter_mut(&mut self, index: usize) -> Interrupter<'_, M, ReadWrite> { - unsafe { Interrupter::new(self.base, index, self.mapper.clone()) } + assert!(index < 1024, "index out of range"); + unsafe { Interrupter::new(self.base + index * 0x20, self.mapper.clone(), self.access64) } } } @@ -125,9 +154,9 @@ where /// Event Ring Segment Table Size Register pub erstsz: single::Generic, /// Event Ring Segment Table Base Address Register - pub erstba: single::Generic, + pub erstba: register64::Generic, /// Event Ring Dequeue Pointer Register - pub erdp: single::Generic, + pub erdp: register64::Generic, // Tie the lifetime of this Interrupter to the parent InterrupterRegisterSet. // This prevents multiple mutable handlers from being created. _marker: PhantomData<&'a InterrupterRegisterSet>, @@ -147,15 +176,13 @@ where /// # Panics /// /// This method panics if `index > 1023`. - unsafe fn new(interrupter_register_set_base: usize, index: usize, mapper: M) -> Self { - assert!(index < 1024, "index out of range"); - let base = interrupter_register_set_base + index * 0x20; + unsafe fn new(base: usize, mapper: M, access64: Access64) -> Self { Self { iman: single::Generic::new(base, mapper.clone()), imod: single::Generic::new(base + 0x4, mapper.clone()), erstsz: single::Generic::new(base + 0x8, mapper.clone()), - erstba: single::Generic::new(base + 0x10, mapper.clone()), - erdp: single::Generic::new(base + 0x18, mapper), + erstba: register64::Generic::new(base + 0x10, access64, mapper.clone()), + erdp: register64::Generic::new(base + 0x18, access64, mapper), _marker: PhantomData, } } @@ -222,6 +249,16 @@ impl EventRingSegmentTableSizeRegister { #[repr(transparent)] #[derive(Copy, Clone, Debug)] pub struct EventRingSegmentTableBaseAddressRegister(u64); +impl From for EventRingSegmentTableBaseAddressRegister { + fn from(value: u64) -> Self { + Self(value) + } +} +impl From for u64 { + fn from(value: EventRingSegmentTableBaseAddressRegister) -> Self { + value.0 + } +} impl EventRingSegmentTableBaseAddressRegister { /// Returns the base address of the Event Ring Segment Table. #[must_use] @@ -247,6 +284,16 @@ impl EventRingSegmentTableBaseAddressRegister { #[repr(transparent)] #[derive(Copy, Clone, Default)] pub struct EventRingDequeuePointerRegister(u64); +impl From for EventRingDequeuePointerRegister { + fn from(value: u64) -> Self { + Self(value) + } +} +impl From for u64 { + fn from(value: EventRingDequeuePointerRegister) -> Self { + value.0 + } +} impl EventRingDequeuePointerRegister { rw_field!( 0..=2, From e1b8600752fdca868af8de361e772bd6a74346be Mon Sep 17 00:00:00 2001 From: Arthur Heymans Date: Fri, 10 Jul 2026 07:09:07 +0200 Subject: [PATCH 2/3] Fix the USB legacy control/status offset The USB legacy support control/status register is the second dword of the capability. Map it at base + 4 instead of aliasing the legacy support semaphore register. --- CHANGELOG.md | 3 ++ .../usb_legacy_support_capability.rs | 32 ++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 649a6281..d17eb9a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ ### Deprecated - `registers::doorbell::Register` in favor of `registers::doorbell::Doorbell`. ([#167]) +### Fixed +- The USB legacy support control/status accessor now uses its correct offset. + ## 0.9.2 - 2023-07-19 ### Added - The sponsor button is now shown on the repository page on GitHub. diff --git a/src/extended_capabilities/usb_legacy_support_capability.rs b/src/extended_capabilities/usb_legacy_support_capability.rs index a55db0bc..7206d79c 100644 --- a/src/extended_capabilities/usb_legacy_support_capability.rs +++ b/src/extended_capabilities/usb_legacy_support_capability.rs @@ -31,7 +31,7 @@ where /// This method panics if `base` is not aligned correctly. pub unsafe fn new(base: usize, m: M) -> Self { let usblegsup = single::ReadWrite::new(base, m.clone()); - let usblegctlsts = single::ReadWrite::new(base, m); + let usblegctlsts = single::ReadWrite::new(base + 4, m); Self { usblegsup, @@ -97,3 +97,33 @@ impl_debug_from_methods! { smi_on_bar, } } + +#[cfg(test)] +mod tests { + use super::*; + use core::num::NonZeroUsize; + + #[derive(Clone, Copy)] + struct IdentityMapper; + + impl Mapper for IdentityMapper { + unsafe fn map(&mut self, phys_start: usize, _bytes: usize) -> NonZeroUsize { + NonZeroUsize::new(phys_start).unwrap() + } + + fn unmap(&mut self, _virt_start: usize, _bytes: usize) {} + } + + #[test] + fn control_status_uses_the_second_dword() { + let mut registers = [0u32; 2]; + let mut capability = + unsafe { UsbLegacySupport::new(registers.as_mut_ptr() as usize, IdentityMapper) }; + + capability.usblegctlsts.update_volatile(|register| { + register.set_usb_smi_enable(); + }); + + assert_eq!(registers, [0, 1]); + } +} From 6b83f05bb4c0852321d8506ed498a59c5daedf83 Mon Sep 17 00:00:00 2001 From: Arthur Heymans Date: Fri, 10 Jul 2026 07:09:38 +0200 Subject: [PATCH 3/3] Include the host controller doorbell The Doorbell Array contains entry zero for the host controller followed by one entry for each device slot. Include that first entry in the accessor length so the highest valid slot ID remains addressable. --- CHANGELOG.md | 1 + src/registers/doorbell.rs | 45 ++++++++++++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d17eb9a3..268b4c64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - `registers::doorbell::Register` in favor of `registers::doorbell::Doorbell`. ([#167]) ### Fixed +- The Doorbell Array now includes the Host Controller Doorbell. - The USB legacy support control/status accessor now uses its correct offset. ## 0.9.2 - 2023-07-19 diff --git a/src/registers/doorbell.rs b/src/registers/doorbell.rs index a162f060..d4876f05 100644 --- a/src/registers/doorbell.rs +++ b/src/registers/doorbell.rs @@ -36,11 +36,12 @@ impl Doorbell { let base = mmio_base + usize::try_from(capability.dboff.read_volatile().get()).unwrap(); array::ReadWrite::new( base, - capability - .hcsparams1 - .read_volatile() - .number_of_device_slots() - .into(), + usize::from( + capability + .hcsparams1 + .read_volatile() + .number_of_device_slots(), + ) + 1, mapper, ) } @@ -52,3 +53,37 @@ impl_debug_from_methods!(Doorbell { doorbell_target, doorbell_stream_id, }); + +#[cfg(test)] +mod tests { + use super::*; + use core::num::NonZeroUsize; + + #[derive(Clone, Copy)] + struct IdentityMapper; + + impl Mapper for IdentityMapper { + unsafe fn map(&mut self, phys_start: usize, _bytes: usize) -> NonZeroUsize { + NonZeroUsize::new(phys_start).unwrap() + } + + fn unmap(&mut self, _virt_start: usize, _bytes: usize) {} + } + + #[test] + fn array_includes_the_host_controller_doorbell() { + let mut mmio = [0u32; 19]; + mmio[1] = 2; + mmio[5] = 0x40; + let base = mmio.as_mut_ptr() as usize; + let mapper = IdentityMapper; + let capability = unsafe { Capability::new(base, &mapper) }; + let mut doorbells = unsafe { Doorbell::new(base, &capability, mapper) }; + + doorbells.update_volatile_at(2, |doorbell| { + doorbell.set_doorbell_target(1); + }); + + assert_eq!(mmio[18], 1); + } +}