From 2a4a19aed8c6c3190f0558b7460dcd24b514e7a5 Mon Sep 17 00:00:00 2001 From: Liav A Date: Thu, 2 Jan 2020 15:54:32 +0200 Subject: [PATCH] Kernel: Fixing PCI MMIO access mechanism During initialization of PCI MMIO access mechanism we ensure that we have an allocation from the kernel virtual address space that cannot be taken by other components in the OS. Also, now we ensure that interrupts are disabled so mapping the region doesn't fail. In order to reduce overhead, map_device() will map the requested PCI address only if it's not mapped already. The run script has been changed so now we can boot a Q35 machine, that supports PCI ECAM. To ensure we will be able to load the machine, a PIIX3 IDE controller was added to the Q35 machine configuration in the run script. An AHCI controller was added to the i440fx machine configuration. --- Kernel/PCI/Definitions.h | 37 +++++++++++++++++++++++- Kernel/PCI/MMIOAccess.cpp | 59 +++++++++++++++++++++++++-------------- Kernel/PCI/MMIOAccess.h | 8 ++++-- Kernel/run | 30 ++++++++++++++++++++ 4 files changed, 110 insertions(+), 24 deletions(-) diff --git a/Kernel/PCI/Definitions.h b/Kernel/PCI/Definitions.h index e2954b05ef..4760367ef8 100644 --- a/Kernel/PCI/Definitions.h +++ b/Kernel/PCI/Definitions.h @@ -51,6 +51,7 @@ struct ID { }; struct Address { +public: Address() {} Address(u16 seg) : m_seg(seg) @@ -80,13 +81,47 @@ struct Address { return 0x80000000u | (m_bus << 16u) | (m_slot << 11u) | (m_function << 8u) | (field & 0xfc); } -private: +protected: u32 m_seg { 0 }; u8 m_bus { 0 }; u8 m_slot { 0 }; u8 m_function { 0 }; }; +struct ChangeableAddress : public Address { + ChangeableAddress() + : Address(0) + { + } + explicit ChangeableAddress(u16 seg) + : Address(seg) + { + } + ChangeableAddress(u16 seg, u8 bus, u8 slot, u8 function) + : Address(seg, bus, slot, function) + { + } + void set_seg(u16 seg) { m_seg = seg; } + void set_bus(u8 bus) { m_bus = bus; } + void set_slot(u8 slot) { m_slot = slot; } + void set_function(u8 function) { m_function = function; } + bool operator==(const Address& address) + { + if (m_seg == address.seg() && m_bus == address.bus() && m_slot == address.slot() && m_function == address.function()) + return true; + else + return false; + } + const ChangeableAddress& operator=(const Address& address) + { + set_seg(address.seg()); + set_bus(address.bus()); + set_slot(address.slot()); + set_function(address.function()); + return *this; + } +}; + void enumerate_all(Function callback); u8 get_interrupt_line(Address); u32 get_BAR0(Address); diff --git a/Kernel/PCI/MMIOAccess.cpp b/Kernel/PCI/MMIOAccess.cpp index 3b148ae979..5161118d17 100644 --- a/Kernel/PCI/MMIOAccess.cpp +++ b/Kernel/PCI/MMIOAccess.cpp @@ -29,11 +29,13 @@ void PCI::MMIOAccess::initialize(ACPI_RAW::MCFG& mcfg) PCI::MMIOAccess::MMIOAccess(ACPI_RAW::MCFG& raw_mcfg) : m_mcfg(raw_mcfg) , m_segments(*new HashMap()) + , m_mapped_address(ChangeableAddress(0xFFFF, 0xFF, 0xFF, 0xFF)) { kprintf("PCI: Using MMIO Mechanism for PCI Configuartion Space Access\n"); - m_mmio_segment = MM.allocate_kernel_region(PAGE_ROUND_UP(PCI_MMIO_CONFIG_SPACE_SIZE), "PCI MMIO", Region::Access::Read | Region::Access::Write); + m_mmio_window = *AnonymousVMObject::create_with_size(PAGE_ROUND_UP(PCI_MMIO_CONFIG_SPACE_SIZE)); + m_mmio_window_region = MM.allocate_kernel_region_with_vmobject(*m_mmio_window, m_mmio_window->size(), "PCI MMIO", Region::Access::Read | Region::Access::Write); - OwnPtr checkup_region = MM.allocate_kernel_region((PAGE_SIZE * 2), "PCI MCFG Checkup", Region::Access::Read | Region::Access::Write); + auto checkup_region = MM.allocate_kernel_region((PAGE_SIZE * 2), "PCI MCFG Checkup", Region::Access::Read | Region::Access::Write); #ifdef PCI_DEBUG dbgprintf("PCI: Checking MCFG Table length to choose the correct mapping size\n"); #endif @@ -63,82 +65,97 @@ PCI::MMIOAccess::MMIOAccess(ACPI_RAW::MCFG& raw_mcfg) } mcfg_region->unmap(); kprintf("PCI: MMIO segments - %d\n", m_segments.size()); + InterruptDisabler disabler; +#ifdef PCI_DEBUG + dbgprintf("PCI: mapped address (%w:%b:%b.%b)\n", m_mapped_address.seg(), m_mapped_address.bus(), m_mapped_address.slot(), m_mapped_address.function()); +#endif map_device(Address(0, 0, 0, 0)); +#ifdef PCI_DEBUG + dbgprintf("PCI: Default mapped address (%w:%b:%b.%b)\n", m_mapped_address.seg(), m_mapped_address.bus(), m_mapped_address.slot(), m_mapped_address.function()); +#endif } void PCI::MMIOAccess::map_device(Address address) { + if (m_mapped_address == address) + return; // FIXME: Map and put some lock! -#ifdef PCI_DEBUG - dbgprintf("PCI: Mapping Device @ pci (%d:%d:%d:%d)\n", address.seg(), address.bus(), address.slot(), address.function()); -#endif + ASSERT_INTERRUPTS_DISABLED(); ASSERT(m_segments.contains(address.seg())); auto segment = m_segments.get(address.seg()); PhysicalAddress segment_lower_addr = segment.value()->get_paddr(); PhysicalAddress device_physical_mmio_space = segment_lower_addr.offset( PCI_MMIO_CONFIG_SPACE_SIZE * address.function() + (PCI_MMIO_CONFIG_SPACE_SIZE * PCI_MAX_FUNCTIONS_PER_DEVICE) * address.slot() + (PCI_MMIO_CONFIG_SPACE_SIZE * PCI_MAX_FUNCTIONS_PER_DEVICE * PCI_MAX_DEVICES_PER_BUS) * (address.bus() - segment.value()->get_start_bus())); + #ifdef PCI_DEBUG - dbgprintf("PCI: Mapping (%d:%d:%d:%d), V 0x%x, P 0x%x\n", address.seg(), address.bus(), address.slot(), address.function(), m_mmio_segment->vaddr().get(), device_physical_mmio_space.get()); + dbgprintf("PCI: Mapping device @ pci (%w:%b:%b.%b), V 0x%x, P 0x%x\n", address.seg(), address.bus(), address.slot(), address.function(), m_mmio_window_region->vaddr().get(), device_physical_mmio_space.get()); #endif - MM.map_for_kernel(m_mmio_segment->vaddr(), device_physical_mmio_space, false); + MM.map_for_kernel(m_mmio_window_region->vaddr(), device_physical_mmio_space); + m_mapped_address = address; } u8 PCI::MMIOAccess::read8_field(Address address, u32 field) { + InterruptDisabler disabler; ASSERT(field <= 0xfff); #ifdef PCI_DEBUG - dbgprintf("PCI: Reading field %u, Address(%u:%u:%u:%u)\n", field, address.seg(), address.bus(), address.slot(), address.function()); + dbgprintf("PCI: Reading field %u, Address(%w:%b:%b.%b)\n", field, address.seg(), address.bus(), address.slot(), address.function()); #endif map_device(address); - return *((u8*)(m_mmio_segment->vaddr().get() + (field & 0xfff))); + return *((u8*)(m_mmio_window_region->vaddr().get() + (field & 0xfff))); } u16 PCI::MMIOAccess::read16_field(Address address, u32 field) { + InterruptDisabler disabler; ASSERT(field < 0xfff); #ifdef PCI_DEBUG - dbgprintf("PCI: Reading field %u, Address(%u:%u:%u:%u)\n", field, address.seg(), address.bus(), address.slot(), address.function()); + dbgprintf("PCI: Reading field %u, Address(%w:%b:%b.%b)\n", field, address.seg(), address.bus(), address.slot(), address.function()); #endif map_device(address); - return *((u16*)(m_mmio_segment->vaddr().get() + (field & 0xfff))); + return *((u16*)(m_mmio_window_region->vaddr().get() + (field & 0xfff))); } u32 PCI::MMIOAccess::read32_field(Address address, u32 field) { + InterruptDisabler disabler; ASSERT(field <= 0xffc); #ifdef PCI_DEBUG - dbgprintf("PCI: Reading field %u, Address(%u:%u:%u:%u)\n", field, address.seg(), address.bus(), address.slot(), address.function()); + dbgprintf("PCI: Reading field %u, Address(%w:%b:%b.%b)\n", field, address.seg(), address.bus(), address.slot(), address.function()); #endif map_device(address); - return *((u32*)(m_mmio_segment->vaddr().get() + (field & 0xfff))); + return *((u32*)(m_mmio_window_region->vaddr().get() + (field & 0xfff))); } void PCI::MMIOAccess::write8_field(Address address, u32 field, u8 value) { + InterruptDisabler disabler; ASSERT(field <= 0xfff); #ifdef PCI_DEBUG - dbgprintf("PCI: Write to field %u, Address(%u:%u:%u:%u), value 0x%x\n", field, address.seg(), address.bus(), address.slot(), address.function(), value); + dbgprintf("PCI: Write to field %u, Address(%w:%b:%b.%b), value 0x%x\n", field, address.seg(), address.bus(), address.slot(), address.function(), value); #endif map_device(address); - *((u8*)(m_mmio_segment->vaddr().get() + (field & 0xfff))) = value; + *((u8*)(m_mmio_window_region->vaddr().get() + (field & 0xfff))) = value; } void PCI::MMIOAccess::write16_field(Address address, u32 field, u16 value) { + InterruptDisabler disabler; ASSERT(field < 0xfff); #ifdef PCI_DEBUG - dbgprintf("PCI: Write to field %u, Address(%u:%u:%u:%u), value 0x%x\n", field, address.seg(), address.bus(), address.slot(), address.function(), value); + dbgprintf("PCI: Write to field %u, Address(%w:%b:%b.%b), value 0x%x\n", field, address.seg(), address.bus(), address.slot(), address.function(), value); #endif map_device(address); - *((u16*)(m_mmio_segment->vaddr().get() + (field & 0xfff))) = value; + *((u16*)(m_mmio_window_region->vaddr().get() + (field & 0xfff))) = value; } void PCI::MMIOAccess::write32_field(Address address, u32 field, u32 value) { + InterruptDisabler disabler; ASSERT(field <= 0xffc); #ifdef PCI_DEBUG - dbgprintf("PCI: Write to field %u, Address(%u:%u:%u:%u), value 0x%x\n", field, address.seg(), address.bus(), address.slot(), address.function(), value); + dbgprintf("PCI: Write to field %u, Address(%w:%b:%b.%b), value 0x%x\n", field, address.seg(), address.bus(), address.slot(), address.function(), value); #endif map_device(address); - *((u32*)(m_mmio_segment->vaddr().get() + (field & 0xfff))) = value; + *((u32*)(m_mmio_window_region->vaddr().get() + (field & 0xfff))) = value; } void PCI::MMIOAccess::enumerate_all(Function& callback) @@ -167,7 +184,7 @@ void PCI::MMIOAccess::mmap(VirtualAddress vaddr, PhysicalAddress paddr, u32 leng unsigned i = 0; while (length >= PAGE_SIZE) { MM.map_for_kernel(VirtualAddress(vaddr.offset(i * PAGE_SIZE).get()), PhysicalAddress(paddr.offset(i * PAGE_SIZE).get())); -#ifdef ACPI_DEBUG +#ifdef PCI_DEBUG dbgprintf("PCI: map - V 0x%x -> P 0x%x\n", vaddr.offset(i * PAGE_SIZE).get(), paddr.offset(i * PAGE_SIZE).get()); #endif length -= PAGE_SIZE; @@ -176,7 +193,7 @@ void PCI::MMIOAccess::mmap(VirtualAddress vaddr, PhysicalAddress paddr, u32 leng if (length > 0) { MM.map_for_kernel(vaddr.offset(i * PAGE_SIZE), paddr.offset(i * PAGE_SIZE), true); } -#ifdef ACPI_DEBUG +#ifdef PCI_DEBUG dbgprintf("PCI: Finished mapping\n"); #endif } diff --git a/Kernel/PCI/MMIOAccess.h b/Kernel/PCI/MMIOAccess.h index de48563383..683bdf29f9 100644 --- a/Kernel/PCI/MMIOAccess.h +++ b/Kernel/PCI/MMIOAccess.h @@ -4,8 +4,10 @@ #include #include #include +#include #include #include +#include class PCI::MMIOAccess final : public PCI::Access { public: @@ -15,7 +17,7 @@ public: virtual String get_access_type() override final { return "MMIO-Access"; }; protected: - MMIOAccess(ACPI_RAW::MCFG&); + explicit MMIOAccess(ACPI_RAW::MCFG&); private: virtual u8 read8_field(Address address, u32) override final; @@ -35,7 +37,9 @@ private: ACPI_RAW::MCFG& m_mcfg; HashMap& m_segments; - OwnPtr m_mmio_segment; + RefPtr m_mmio_window; + OwnPtr m_mmio_window_region; + PCI::ChangeableAddress m_mapped_address; }; class PCI::MMIOSegment { diff --git a/Kernel/run b/Kernel/run index 303a7d6c17..12fdb75514 100755 --- a/Kernel/run +++ b/Kernel/run @@ -22,6 +22,22 @@ $SERENITY_EXTRA_QEMU_ARGS -d cpu_reset,guest_errors -device VGA,vgamem_mb=64 -hda _disk_image +-device ich9-ahci +-debugcon stdio +-soundhw pcspk +-soundhw sb16 +" + +[ -z "$SERENITY_COMMON_QEMU_Q35_ARGS" ] && SERENITY_COMMON_QEMU_Q35_ARGS=" +$SERENITY_EXTRA_QEMU_ARGS +-s -m $SERENITY_RAM_SIZE +-cpu max +-machine q35 +-d cpu_reset,guest_errors +-device VGA,vgamem_mb=64 +-device piix3-ide +-drive file=_disk_image,id=disk,if=none +-device ide-hd,bus=ide.6,drive=disk,unit=0 -debugcon stdio -soundhw pcspk -soundhw sb16 @@ -57,6 +73,20 @@ elif [ "$1" = "qgrub" ]; then $SERENITY_PACKET_LOGGING_ARG \ -netdev user,id=breh,hostfwd=tcp:127.0.0.1:8888-10.0.2.15:8888,hostfwd=tcp:127.0.0.1:8823-10.0.2.15:23 \ -device e1000,netdev=breh +elif [ "$1" = "q35_cmd" ]; then + SERENITY_KERNEL_CMDLINE="" + # FIXME: Someone who knows sh syntax better, please help: + for i in `seq 2 $#`; do + shift + SERENITY_KERNEL_CMDLINE="$SERENITY_KERNEL_CMDLINE $1" + done + echo "Starting SerenityOS, Commandline: ${SERENITY_KERNEL_CMDLINE}" + # ./run: qemu with SerenityOS with custom commandline + $SERENITY_QEMU_BIN \ + $SERENITY_COMMON_QEMU_Q35_ARGS \ + -device e1000 \ + -kernel kernel \ + -append "${SERENITY_KERNEL_CMDLINE}" elif [ "$1" = "qcmd" ]; then SERENITY_KERNEL_CMDLINE="" # FIXME: Someone who knows sh syntax better, please help: