From 278c63d9617e43f1a7e561363009cace54897356 Mon Sep 17 00:00:00 2001 From: Mark Poliakov Date: Thu, 5 Dec 2024 22:02:01 +0200 Subject: [PATCH] ahci: properly use ATA PRDs --- kernel/driver/block/ahci/src/command.rs | 37 +++++++++++-------- kernel/driver/block/ahci/src/data.rs | 28 +++++++++++--- kernel/driver/block/ahci/src/error.rs | 2 + kernel/driver/block/ahci/src/lib.rs | 2 +- kernel/driver/block/ahci/src/port.rs | 21 ++++++++--- lib/qemu/src/device.rs | 49 ++++++++++++++++++------- lib/qemu/src/lib.rs | 2 + xtask/src/qemu.rs | 26 ++++++++++++- 8 files changed, 125 insertions(+), 42 deletions(-) diff --git a/kernel/driver/block/ahci/src/command.rs b/kernel/driver/block/ahci/src/command.rs index b98efcf7..2c9f84d2 100644 --- a/kernel/driver/block/ahci/src/command.rs +++ b/kernel/driver/block/ahci/src/command.rs @@ -6,7 +6,7 @@ use libk_mm::{ }; use tock_registers::register_structs; -use crate::{data::AtaString, error::AhciError, SECTOR_SIZE}; +use crate::{data::AtaString, error::AhciError, MAX_PRD_SIZE, SECTOR_SIZE}; #[derive(Clone, Copy, PartialEq, Eq, Debug)] #[repr(u8)] @@ -22,8 +22,16 @@ pub trait AtaCommand { fn lba(&self) -> u64; fn sector_count(&self) -> usize; - fn regions(&self) -> &[(PhysicalAddress, usize)]; + fn buffer(&self) -> Option<(PhysicalAddress, usize)>; unsafe fn into_response(self) -> Self::Response; + + fn prd_count(&self) -> usize { + if let Some((_, size)) = self.buffer() { + size.div_ceil(MAX_PRD_SIZE) + } else { + 0 + } + } } register_structs! { @@ -57,13 +65,13 @@ register_structs! { pub struct AtaIdentify { buffer: PageBox>, - regions: [(PhysicalAddress, usize); 1], } pub struct AtaReadDmaEx { lba: u64, sector_count: usize, - regions: [(PhysicalAddress, usize); 1], + buffer_base: PhysicalAddress, + buffer_size: usize, } impl AtaIdentify { @@ -74,13 +82,7 @@ impl AtaIdentify { } pub fn with_data(buffer: PageBox>) -> Self { - Self { - regions: [( - unsafe { buffer.as_physical_address() }, - size_of::(), - )], - buffer, - } + Self { buffer } } } @@ -92,7 +94,8 @@ impl AtaReadDmaEx { Self { lba, sector_count, - regions: [(unsafe { buffer.as_physical_address() }, buffer.len())], + buffer_base: unsafe { buffer.as_physical_address() }, + buffer_size: buffer.len(), } } } @@ -110,8 +113,10 @@ impl AtaCommand for AtaIdentify { 0 } - fn regions(&self) -> &[(PhysicalAddress, usize)] { - &self.regions + fn buffer(&self) -> Option<(PhysicalAddress, usize)> { + let base = unsafe { self.buffer.as_physical_address() }; + let size = size_of::(); + Some((base, size)) } unsafe fn into_response(self) -> Self::Response { @@ -132,8 +137,8 @@ impl AtaCommand for AtaReadDmaEx { self.sector_count } - fn regions(&self) -> &[(PhysicalAddress, usize)] { - &self.regions + fn buffer(&self) -> Option<(PhysicalAddress, usize)> { + Some((self.buffer_base, self.buffer_size)) } unsafe fn into_response(self) -> Self::Response {} diff --git a/kernel/driver/block/ahci/src/data.rs b/kernel/driver/block/ahci/src/data.rs index a5db0564..e84be7ea 100644 --- a/kernel/driver/block/ahci/src/data.rs +++ b/kernel/driver/block/ahci/src/data.rs @@ -145,10 +145,28 @@ impl CommandTable { }; } - let regions = command.regions(); - for (i, &(base, size)) in regions.iter().enumerate() { - let last = i == regions.len() - 1; - self.prdt[i] = PhysicalRegionDescriptor::new(base, size, last)?; + // Setup PRDs + if let Some((base, size)) = command.buffer() { + let mut remaining = size; + let mut prd = 0; + while remaining != 0 { + let rem = remaining.min(MAX_PRD_SIZE); + let last = rem <= MAX_PRD_SIZE; + log::trace!( + target: "io", + "prd[{prd}] base={:#x}, size={rem}", + base.add(prd * MAX_PRD_SIZE) + ); + + self.prdt[prd] = + PhysicalRegionDescriptor::new(base.add(prd * MAX_PRD_SIZE), rem, last)?; + + prd += 1; + remaining -= rem; + } + assert_eq!(prd, command.prd_count()); + + self.prdt[prd..].fill_with(|| PhysicalRegionDescriptor::zeroed()); } Ok(()) @@ -188,7 +206,7 @@ impl PhysicalRegionDescriptor { byte_count: usize, is_last: bool, ) -> Result { - if byte_count >= MAX_PRD_SIZE { + if byte_count > MAX_PRD_SIZE { return Err(AhciError::RegionTooLarge); } diff --git a/kernel/driver/block/ahci/src/error.rs b/kernel/driver/block/ahci/src/error.rs index 2cf7189f..b94f28da 100644 --- a/kernel/driver/block/ahci/src/error.rs +++ b/kernel/driver/block/ahci/src/error.rs @@ -3,6 +3,7 @@ use yggdrasil_abi::error::Error; #[derive(Debug)] pub enum AhciError { MemoryError(#[allow(dead_code)] Error), + InvalidBufferSize(usize), RegionTooLarge, DeviceError, FeatureNotImplemented, @@ -12,6 +13,7 @@ impl From for Error { fn from(value: AhciError) -> Self { match value { // TODO: Error::DeviceError + AhciError::InvalidBufferSize(_) => Error::InvalidArgument, AhciError::DeviceError => Error::InvalidArgument, AhciError::RegionTooLarge => Error::InvalidArgument, AhciError::MemoryError(err) => err, diff --git a/kernel/driver/block/ahci/src/lib.rs b/kernel/driver/block/ahci/src/lib.rs index b2af8dfb..7f3761ad 100644 --- a/kernel/driver/block/ahci/src/lib.rs +++ b/kernel/driver/block/ahci/src/lib.rs @@ -36,7 +36,7 @@ mod error; mod port; mod regs; -const MAX_PRD_SIZE: usize = 8192; +const MAX_PRD_SIZE: usize = 65536; const MAX_COMMANDS: usize = u32::BITS as usize; const SECTOR_SIZE: usize = 512; const MAX_DRIVES: usize = (b'z' - b'a') as usize; diff --git a/kernel/driver/block/ahci/src/port.rs b/kernel/driver/block/ahci/src/port.rs index 45067c0d..3a75f530 100644 --- a/kernel/driver/block/ahci/src/port.rs +++ b/kernel/driver/block/ahci/src/port.rs @@ -19,7 +19,7 @@ use crate::{ data::{CommandListEntry, CommandTable, ReceivedFis, COMMAND_LIST_LENGTH}, error::AhciError, regs::{PortRegs, CMD_PENDING, CMD_READY, IE, TFD}, - AhciController, MAX_COMMANDS, SECTOR_SIZE, + AhciController, MAX_COMMANDS, MAX_PRD_SIZE, SECTOR_SIZE, }; #[derive(Clone, Copy, PartialEq, Debug)] @@ -94,7 +94,7 @@ impl PortInner { table_entry.setup_command(command)?; *list_entry = CommandListEntry::new( unsafe { table_entry.as_physical_address() }, - command.regions().len(), + command.prd_count(), )?; // Sync before send @@ -127,7 +127,8 @@ impl AhciPort { regs.stop()?; if !ahci.has_64_bit { - todo!("Handle controllers incapable of 64 bit"); + log::error!("Handle controllers incapable of 64 bit"); + return Err(AhciError::DeviceError); } let received_fis = PageBox::new(ReceivedFis::zeroed()).map_err(AhciError::MemoryError)?; @@ -174,6 +175,7 @@ impl AhciPort { pub async fn init(&'static self) -> Result<(), AhciError> { let identify = self.perform_command(AtaIdentify::create()?).await?; + let model = identify.model_number.to_string(); let serial = identify.serial_number.to_string(); let lba_count = identify.logical_sector_count(); @@ -222,6 +224,10 @@ impl AhciPort { } async fn submit(&self, command: &C) -> Result { + if command.prd_count() > 2 { + log::warn!("TODO: AHCI doesn't like 3+ PRD transfers"); + return Err(AhciError::RegionTooLarge); + } let index = self.allocate_command().await; if let Err(error) = self.inner.lock().submit_command(index, command) { self.free_command(index); @@ -277,7 +283,7 @@ impl AhciPort { if ci & (1 << i) == 0 && self.command_completion[i].1.swap(status, Ordering::Release) == CMD_PENDING { - log::info!("port{}: completion on slot {}", self.index, i); + log::trace!(target: "io", "port{}: completion on slot {}", self.index, i); self.command_completion[i].0.wake(); } } @@ -295,6 +301,10 @@ impl NgBlockDevice for AhciPort { lba: u64, buffer: &mut PageSlice>, ) -> Result<(), AhciError> { + if buffer.len() % SECTOR_SIZE != 0 { + return Err(AhciError::InvalidBufferSize(buffer.len())); + } + let command = AtaReadDmaEx::new(lba, buffer.len() / SECTOR_SIZE, buffer); self.submit(&command).await?.wait_for_completion().await } @@ -313,7 +323,6 @@ impl NgBlockDevice for AhciPort { } fn max_blocks_per_request(&self) -> usize { - // TODO - 1 + (MAX_PRD_SIZE * 2) / SECTOR_SIZE } } diff --git a/lib/qemu/src/device.rs b/lib/qemu/src/device.rs index cf1623a4..10f42a5e 100644 --- a/lib/qemu/src/device.rs +++ b/lib/qemu/src/device.rs @@ -7,7 +7,7 @@ pub enum QemuNic { VirtioPci { mac: Option }, } -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq)] pub enum QemuDrive { Nvme, Sata, @@ -73,20 +73,43 @@ impl IntoArgs for QemuDevice { nic.add_args(command); } Self::Drive { ty, file, serial } => { - command.arg("-drive"); - command.arg(format!("file={},if=none,id=drive0", file.display())); + match ty { + QemuDrive::Nvme => { + command.arg("-drive"); + command.arg(format!("file={},if=none,id=drive0", file.display())); - command.arg("-device"); - let mut val = match ty { - QemuDrive::Nvme => "nvme".to_owned(), - _ => todo!(), - }; - if let Some(serial) = serial { - val.push_str(",serial="); - val.push_str(serial); + command.arg("-device"); + let mut device = "nvme".to_owned(); + if let Some(serial) = serial { + device.push_str(",serial="); + device.push_str(serial); + } + device.push_str(",drive=drive0"); + } + QemuDrive::Sata => { + command.arg("-drive"); + command.arg(format!( + "file={},if=ide,bus=0,index=1,id=drive0", + file.display() + )); + } } - val.push_str(",drive=drive0"); - command.arg(val); + // command.arg("-drive"); + // command.arg(format!("file={},if=none,id=drive0", file.display())); + + // command.arg("-device"); + // let mut val = match ty { + // QemuDrive::Nvme => "nvme".to_owned(), + // QemuDrive::Sata => "ahci".to_owned(), + // }; + // if *ty == QemuDrive::Nvme + // && let Some(serial) = serial + // { + // val.push_str(",serial="); + // val.push_str(serial); + // } + // val.push_str(",drive=drive0"); + // command.arg(val); } } } diff --git a/lib/qemu/src/lib.rs b/lib/qemu/src/lib.rs index 9ac9967f..d92ac65e 100644 --- a/lib/qemu/src/lib.rs +++ b/lib/qemu/src/lib.rs @@ -1,3 +1,5 @@ +#![feature(let_chains)] + use std::{ fmt::Debug, path::{Path, PathBuf}, diff --git a/xtask/src/qemu.rs b/xtask/src/qemu.rs index 5b56d3e6..5d227ed9 100644 --- a/xtask/src/qemu.rs +++ b/xtask/src/qemu.rs @@ -26,6 +26,20 @@ struct QemuNetworkConfig { mac: String, } +#[derive(Debug, Default, Clone, Copy, serde::Deserialize)] +#[serde(rename_all = "kebab-case")] +enum QemuDiskInterface { + #[default] + Nvme, + Ahci, +} + +#[derive(Debug, Default, serde::Deserialize)] +#[serde(rename_all = "kebab-case", default)] +struct QemuDiskConfig { + interface: QemuDiskInterface, +} + #[derive(Debug, serde::Deserialize)] #[serde(rename_all = "kebab-case", default)] struct QemuX86_64MachineConfig { @@ -59,6 +73,7 @@ struct QemuMachineConfig { #[serde(rename_all = "kebab-case", default)] struct QemuConfig { network: QemuNetworkConfig, + disk: QemuDiskConfig, machine: QemuMachineConfig, } @@ -107,6 +122,15 @@ impl Default for QemuNetworkConfig { } } +impl From for QemuDrive { + fn from(value: QemuDiskInterface) -> Self { + match value { + QemuDiskInterface::Nvme => Self::Nvme, + QemuDiskInterface::Ahci => Self::Sata, + } + } +} + fn make_kernel_bin, D: AsRef>(src: S, dst: D) -> Result<(), Error> { run_external_command( "llvm-objcopy", @@ -234,7 +258,7 @@ fn add_devices_from_config( } if let Some(disk) = disk { devices.push(QemuDevice::Drive { - ty: QemuDrive::Nvme, + ty: config.disk.interface.into(), file: disk.clone(), serial: Some("deadbeef".into()), });