From 2e7f1faa0ef7b2aa406e74982e0980f2f06ca136 Mon Sep 17 00:00:00 2001 From: Runji Wang Date: Wed, 10 Jun 2020 22:08:06 +0800 Subject: [PATCH 1/4] fix PCI bugs on QEMU --- kernel-hal-bare/src/arch/x86_64/interrupt.rs | 6 +++--- zircon-object/src/dev/pci/bus.rs | 4 ++-- zircon-object/src/dev/pci/caps.rs | 10 +++++----- zircon-object/src/dev/pci/config.rs | 17 +++++++++++++++++ zircon-object/src/dev/pci/mod.rs | 1 + zircon-object/src/dev/pci/nodes.rs | 18 +++++++++--------- zircon-syscall/src/object.rs | 2 ++ 7 files changed, 39 insertions(+), 19 deletions(-) diff --git a/kernel-hal-bare/src/arch/x86_64/interrupt.rs b/kernel-hal-bare/src/arch/x86_64/interrupt.rs index 3b3860a8a..235b722bc 100644 --- a/kernel-hal-bare/src/arch/x86_64/interrupt.rs +++ b/kernel-hal-bare/src/arch/x86_64/interrupt.rs @@ -174,7 +174,7 @@ pub fn irq_remove_handle(irq: u8) -> bool { } #[export_name = "hal_irq_allocate_block"] -pub fn allocate_block(irq_num: u32) -> Option { +pub fn allocate_block(irq_num: u32) -> Option<(usize, usize)> { let irq_num = u32::next_power_of_two(irq_num) as usize; let mut irq_start = 0x20; let mut irq_cur = irq_start; @@ -184,13 +184,13 @@ pub fn allocate_block(irq_num: u32) -> Option { irq_cur += 1; } else { irq_start = (irq_cur & (irq_num - 1)) + irq_num; - irq_cur = irq_start + irq_cur = irq_start; } } for i in irq_start..irq_start + irq_num { table[i] = Some(Box::new(|| {})); } - Some(irq_start) + Some((irq_start, irq_num)) } #[export_name = "hal_irq_free_block"] diff --git a/zircon-object/src/dev/pci/bus.rs b/zircon-object/src/dev/pci/bus.rs index 58121537f..3a6204da4 100644 --- a/zircon-object/src/dev/pci/bus.rs +++ b/zircon-object/src/dev/pci/bus.rs @@ -373,8 +373,8 @@ impl MmioPcieAddressProvider { if ecam.bus_start > ecam.bus_end { return Err(ZxError::INVALID_ARGS); } - let bus_count = ecam.bus_end + 1 - ecam.bus_start; - if ecam.size != bus_count as usize * PCIE_ECAM_BYTES_PER_BUS { + let bus_count = (ecam.bus_end - ecam.bus_start) as usize + 1; + if ecam.size != bus_count * PCIE_ECAM_BYTES_PER_BUS { return Err(ZxError::INVALID_ARGS); } let mut inner = self.ecam_regions.lock(); diff --git a/zircon-object/src/dev/pci/caps.rs b/zircon-object/src/dev/pci/caps.rs index 0c474dc33..0eec32431 100644 --- a/zircon-object/src/dev/pci/caps.rs +++ b/zircon-object/src/dev/pci/caps.rs @@ -82,10 +82,10 @@ pub struct PciCapacityMsi { impl PciCapacityMsi { pub fn create(cfg: &PciConfig, base: usize, id: u8) -> PciCapacityMsi { assert_eq!(id, 0x5); // PCIE_CAP_ID_MSI - let ctrl = cfg.read16_offset(base + 0x2); + let ctrl = cfg.read16_(base + 0x2); let has_pvm = (ctrl & 0x100) != 0; let is_64bit = (ctrl & 0x80) != 0; - cfg.write16_offset(base as usize + 0x2, ctrl & !0x71); + cfg.write16_(base + 0x2, ctrl & !0x71); let mask_bits = base + if is_64bit { 0x10 } else { 0xC }; if has_pvm { cfg.write32_offset(mask_bits, 0xffff_ffff); @@ -140,8 +140,8 @@ pub struct PciCapPcie { impl PciCapPcie { pub fn create(cfg: &PciConfig, base: u16, id: u8) -> PciCapPcie { assert_eq!(id, 0x10); // PCIE_CAP_ID_PCI_EXPRESS - let caps = cfg.read8_offset(base as usize + 0x2); - let device_caps = cfg.read32_offset(base as usize + 0x4); + let caps = cfg.read8_(base as usize + 0x2); + let device_caps = cfg.read32_(base as usize + 0x4); PciCapPcie { version: caps & 0xF, dev_type: PcieDeviceType::try_from(((caps >> 4) & 0xF) as u8).unwrap(), @@ -159,7 +159,7 @@ pub struct PciCapAdvFeatures { impl PciCapAdvFeatures { pub fn create(cfg: &PciConfig, base: u16, id: u8) -> PciCapAdvFeatures { assert_eq!(id, 0x13); // PCIE_CAP_ID_ADVANCED_FEATURES - let caps = cfg.read8_offset(base as usize + 0x3); + let caps = cfg.read8_(base as usize + 0x3); PciCapAdvFeatures { has_flr: ((caps >> 1) & 0x1) != 0, has_tp: (caps & 0x1) != 0, diff --git a/zircon-object/src/dev/pci/config.rs b/zircon-object/src/dev/pci/config.rs index da4eab299..ec1ec1fb2 100644 --- a/zircon-object/src/dev/pci/config.rs +++ b/zircon-object/src/dev/pci/config.rs @@ -1,5 +1,7 @@ use super::*; use numeric_enum_macro::*; + +#[derive(Debug)] pub struct PciConfig { pub addr_space: PciAddrSpace, pub base: usize, @@ -31,12 +33,21 @@ impl PciConfig { pub fn read8(&self, addr: PciReg8) -> u8 { self.read8_offset(self.base + addr as usize) } + pub fn read8_(&self, addr: usize) -> u8 { + self.read8_offset(self.base + addr) + } pub fn read16(&self, addr: PciReg16) -> u16 { self.read16_offset(self.base + addr as usize) } + pub fn read16_(&self, addr: usize) -> u16 { + self.read16_offset(self.base + addr) + } pub fn read32(&self, addr: PciReg32) -> u32 { self.read32_offset(self.base + addr as usize) } + pub fn read32_(&self, addr: usize) -> u32 { + self.read32_offset(self.base + addr) + } pub fn read_bar(&self, bar_: usize) -> u32 { self.read32_offset(self.base + PciReg32::BARBase as usize + bar_ * 4) } @@ -70,9 +81,15 @@ impl PciConfig { pub fn write16(&self, addr: PciReg16, val: u16) { self.write16_offset(self.base + addr as usize, val) } + pub fn write16_(&self, addr: usize, val: u16) { + self.write16_offset(self.base + addr, val) + } pub fn write32(&self, addr: PciReg32, val: u32) { self.write32_offset(self.base + addr as usize, val) } + pub fn write32_(&self, addr: usize, val: u32) { + self.write32_offset(self.base + addr, val) + } pub fn write_bar(&self, bar_: usize, val: u32) { self.write32_offset(self.base + PciReg32::BARBase as usize + bar_ * 4, val) } diff --git a/zircon-object/src/dev/pci/mod.rs b/zircon-object/src/dev/pci/mod.rs index 800d3eac3..db57b8fe6 100644 --- a/zircon-object/src/dev/pci/mod.rs +++ b/zircon-object/src/dev/pci/mod.rs @@ -16,6 +16,7 @@ pub enum PciAddrSpace { } #[repr(C)] +#[derive(Debug)] pub struct PciInitArgsAddrWindows { pub base: u64, pub size: usize, diff --git a/zircon-object/src/dev/pci/nodes.rs b/zircon-object/src/dev/pci/nodes.rs index b1a308bc9..dd96c5439 100644 --- a/zircon-object/src/dev/pci/nodes.rs +++ b/zircon-object/src/dev/pci/nodes.rs @@ -543,7 +543,7 @@ impl PcieDevice { if cap_offset == 0xff || cap_offset < 64 || cap_offset > 252 { return Err(ZxError::INVALID_ARGS); } - let id = cfg.read8_offset(cap_offset as usize); + let id = cfg.read8_(cap_offset as usize); let std = PciCapacityStd::create(cap_offset as u16, id); let mut inner = self.inner.lock(); let cap = match id { @@ -561,7 +561,7 @@ impl PcieDevice { _ => PciCapacity::Std(std), }; inner.caps.push(cap); - cap_offset = cfg.read8_offset(cap_offset as usize + 1) & 0xFC; + cap_offset = cfg.read8_(cap_offset as usize + 1) & 0xFC; found_num += 1; } Ok(()) @@ -986,11 +986,11 @@ impl PcieDevice { let addr_reg = std.base + 0x4; let addr_reg_upper = std.base + 0x8; let data_reg = std.base + PciCapacityMsi::addr_offset(msi.is_64bit) as u16; - cfg.write32_offset(addr_reg as usize, target_addr as u32); + cfg.write32_(addr_reg as usize, target_addr as u32); if msi.is_64bit { - cfg.write32_offset(addr_reg_upper as usize, (target_addr >> 32) as u32); + cfg.write32_(addr_reg_upper as usize, (target_addr >> 32) as u32); } - cfg.write16_offset(data_reg as usize, target_data as u16); + cfg.write16_(data_reg as usize, target_data as u16); } fn set_msi_multi_message_enb(&self, inner: &MutexGuard, irq_num: u32) { assert!(1 <= irq_num && irq_num <= PCIE_MAX_MSI_IRQS); @@ -999,16 +999,16 @@ impl PcieDevice { let cfg = self.cfg.as_ref().unwrap(); let (std, msi) = inner.msi().unwrap(); let data_reg = std.base as usize + PciCapacityMsi::addr_offset(msi.is_64bit); - let mut val = cfg.read32_offset(data_reg as usize); + let mut val = cfg.read32_(data_reg as usize); val = (val & !0x70) | ((log2 & 0x7) << 4); - cfg.write32_offset(data_reg, val); + cfg.write32_(data_reg, val); } fn set_msi_enb(&self, inner: &MutexGuard, enable: bool) { let cfg = self.cfg.as_ref().unwrap(); let (std, _msi) = inner.msi().unwrap(); let ctrl_addr = std.base as usize + PciCapacityMsi::ctrl_offset(); - let val = cfg.read16_offset(ctrl_addr); - cfg.write16_offset(ctrl_addr, (val & !0x1) | (enable as u16)); + let val = cfg.read16_(ctrl_addr); + cfg.write16_(ctrl_addr, (val & !0x1) | (enable as u16)); } fn mask_all_msi_vectors(&self, inner: &MutexGuard) { for i in 0..inner.irq.handlers.len() { diff --git a/zircon-syscall/src/object.rs b/zircon-syscall/src/object.rs index 79792ac78..309aeeb92 100644 --- a/zircon-syscall/src/object.rs +++ b/zircon-syscall/src/object.rs @@ -240,6 +240,8 @@ impl Syscall<'_> { } let info = proc.get_handle_info(handle)?; UserOutPtr::::from(buffer).write(info)?; + actual.write_if_not_null(1)?; + avail.write_if_not_null(1)?; } Topic::Thread => { if buffer_size < core::mem::size_of::() { From 29f50d2312fd1d84cee90d220ab6bc853448b03b Mon Sep 17 00:00:00 2001 From: Ben Pig Chu Date: Sat, 13 Jun 2020 23:41:03 +0800 Subject: [PATCH 2/4] Various bug fix --- kernel-hal-bare/src/arch/x86_64/interrupt.rs | 5 ++++- zircon-loader/src/lib.rs | 2 +- zircon-object/src/dev/pci/nodes.rs | 10 +++++----- zircon-syscall/src/pci.rs | 4 ++-- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/kernel-hal-bare/src/arch/x86_64/interrupt.rs b/kernel-hal-bare/src/arch/x86_64/interrupt.rs index 235b722bc..6b20a6cf5 100644 --- a/kernel-hal-bare/src/arch/x86_64/interrupt.rs +++ b/kernel-hal-bare/src/arch/x86_64/interrupt.rs @@ -175,6 +175,7 @@ pub fn irq_remove_handle(irq: u8) -> bool { #[export_name = "hal_irq_allocate_block"] pub fn allocate_block(irq_num: u32) -> Option<(usize, usize)> { + info!("hal_irq_allocate_block: count={:#x?}",irq_num); let irq_num = u32::next_power_of_two(irq_num) as usize; let mut irq_start = 0x20; let mut irq_cur = irq_start; @@ -183,13 +184,14 @@ pub fn allocate_block(irq_num: u32) -> Option<(usize, usize)> { if table[irq_cur].is_none() { irq_cur += 1; } else { - irq_start = (irq_cur & (irq_num - 1)) + irq_num; + irq_start = (irq_cur - irq_cur % irq_num) + irq_num; irq_cur = irq_start; } } for i in irq_start..irq_start + irq_num { table[i] = Some(Box::new(|| {})); } + info!("hal_irq_allocate_block: start={:#x?} num={:#x?}",irq_start, irq_num); Some((irq_start, irq_num)) } @@ -203,6 +205,7 @@ pub fn free_block(irq_start: u32, irq_num: u32) { #[export_name = "hal_irq_overwrite_handler"] pub fn overwrite_handler(msi_id: u32, handle: Box) -> bool { + info!("IRQ overwrite handle {:#x?}", msi_id); let mut table = IRQ_TABLE.lock(); let set = table[msi_id as usize].is_none(); table[msi_id as usize] = Some(handle); diff --git a/zircon-loader/src/lib.rs b/zircon-loader/src/lib.rs index 397258b27..41912b696 100644 --- a/zircon-loader/src/lib.rs +++ b/zircon-loader/src/lib.rs @@ -227,7 +227,7 @@ fn spawn(thread: Arc) { { Ok(()) => {} Err(e) => { - error!("{:?}", e); + error!("proc={:?} thread={:?} err={:?}", thread.proc().name(), thread.name(), e); panic!("Page Fault from user mode {:#x?}", cx); } } diff --git a/zircon-object/src/dev/pci/nodes.rs b/zircon-object/src/dev/pci/nodes.rs index dd96c5439..e303dc39b 100644 --- a/zircon-object/src/dev/pci/nodes.rs +++ b/zircon-object/src/dev/pci/nodes.rs @@ -997,11 +997,11 @@ impl PcieDevice { let log2 = u32::next_power_of_two(irq_num).trailing_zeros(); assert!(log2 <= 5); let cfg = self.cfg.as_ref().unwrap(); - let (std, msi) = inner.msi().unwrap(); - let data_reg = std.base as usize + PciCapacityMsi::addr_offset(msi.is_64bit); - let mut val = cfg.read32_(data_reg as usize); - val = (val & !0x70) | ((log2 & 0x7) << 4); - cfg.write32_(data_reg, val); + let (std, _msi) = inner.msi().unwrap(); + let ctrl_addr = std.base as usize + PciCapacityMsi::ctrl_offset(); + let mut val = cfg.read16_(ctrl_addr); + val = (val & !0x70) | ((log2 as u16 & 0x7) << 4); + cfg.write16_(ctrl_addr, val); } fn set_msi_enb(&self, inner: &MutexGuard, enable: bool) { let cfg = self.cfg.as_ref().unwrap(); diff --git a/zircon-syscall/src/pci.rs b/zircon-syscall/src/pci.rs index e3beffc89..a3e7bae71 100644 --- a/zircon-syscall/src/pci.rs +++ b/zircon-syscall/src/pci.rs @@ -108,8 +108,8 @@ impl Syscall<'_> { // that collide with architectural registers. #[cfg(target_arch = "x86_64")] { - let num_buses: u8 = addr_win.bus_end - addr_win.bus_start + 1; - let mut end: u64 = addr_win.base + num_buses as u64 * PCIE_ECAM_BYTES_PER_BUS as u64; + let num_buses = (addr_win.bus_end - addr_win.bus_start) as u64 + 1; + let mut end: u64 = addr_win.base + num_buses * PCIE_ECAM_BYTES_PER_BUS as u64; let high_limit: u64 = 0xfec0_0000; if end > high_limit { end = high_limit; From f87e2a5b3f6f4ebdc511bb433ed3886fb6541289 Mon Sep 17 00:00:00 2001 From: Ben Pig Chu Date: Sun, 14 Jun 2020 18:07:00 +0800 Subject: [PATCH 3/4] disable virtcon as a workaround to the page fault --- zircon-loader/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zircon-loader/src/lib.rs b/zircon-loader/src/lib.rs index 41912b696..b345789bf 100644 --- a/zircon-loader/src/lib.rs +++ b/zircon-loader/src/lib.rs @@ -171,7 +171,7 @@ pub fn run_userboot(images: &Images>, cmdline: &str) -> Arc Date: Sun, 14 Jun 2020 18:33:42 +0800 Subject: [PATCH 4/4] Fix fmt --- kernel-hal-bare/src/arch/x86_64/interrupt.rs | 7 +++++-- zircon-loader/src/lib.rs | 10 ++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/kernel-hal-bare/src/arch/x86_64/interrupt.rs b/kernel-hal-bare/src/arch/x86_64/interrupt.rs index 6b20a6cf5..b62a7b55b 100644 --- a/kernel-hal-bare/src/arch/x86_64/interrupt.rs +++ b/kernel-hal-bare/src/arch/x86_64/interrupt.rs @@ -175,7 +175,7 @@ pub fn irq_remove_handle(irq: u8) -> bool { #[export_name = "hal_irq_allocate_block"] pub fn allocate_block(irq_num: u32) -> Option<(usize, usize)> { - info!("hal_irq_allocate_block: count={:#x?}",irq_num); + info!("hal_irq_allocate_block: count={:#x?}", irq_num); let irq_num = u32::next_power_of_two(irq_num) as usize; let mut irq_start = 0x20; let mut irq_cur = irq_start; @@ -191,7 +191,10 @@ pub fn allocate_block(irq_num: u32) -> Option<(usize, usize)> { for i in irq_start..irq_start + irq_num { table[i] = Some(Box::new(|| {})); } - info!("hal_irq_allocate_block: start={:#x?} num={:#x?}",irq_start, irq_num); + info!( + "hal_irq_allocate_block: start={:#x?} num={:#x?}", + irq_start, irq_num + ); Some((irq_start, irq_num)) } diff --git a/zircon-loader/src/lib.rs b/zircon-loader/src/lib.rs index b345789bf..3defd7686 100644 --- a/zircon-loader/src/lib.rs +++ b/zircon-loader/src/lib.rs @@ -171,7 +171,8 @@ pub fn run_userboot(images: &Images>, cmdline: &str) -> Arc) { { Ok(()) => {} Err(e) => { - error!("proc={:?} thread={:?} err={:?}", thread.proc().name(), thread.name(), e); + error!( + "proc={:?} thread={:?} err={:?}", + thread.proc().name(), + thread.name(), + e + ); panic!("Page Fault from user mode {:#x?}", cx); } }