eliott/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2026-04-25 17:15:42 +02:00
Code Issues Packages Projects Releases Wiki Activity

Kernel: Replace all usages of full_memory_barrier with full_memory_fence

Browse Source 
full_memory_barrier() is not strong enough when interacting with device
memory, as it uses C++ memory fences that don't have any guarantees
for MMIO/DMA memory ordering.
All usages of this function seem to be for synchronizing such device
memory, so use the new full_memory_fence() function.
This commit is contained in:
Sönke Holz
2025-10-16 20:52:54 +02:00
committed by Sönke Holz
parent 250a0f291c
commit 265416e33e
10 changed files with 62 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
AK/Atomic.h
View File

@@ -22,12 +22,6 @@ static inline void atomic_thread_fence(MemoryOrder order) noexcept
return __atomic_thread_fence(order);
}
static inline void full_memory_barrier() noexcept
{
atomic_signal_fence(AK::MemoryOrder::memory_order_acq_rel);
atomic_thread_fence(AK::MemoryOrder::memory_order_acq_rel);
}
template<typename T>
static inline T atomic_exchange(T volatile* var, T desired, MemoryOrder order = memory_order_seq_cst) noexcept
{
@@ -502,5 +496,4 @@ public:
#if USING_AK_GLOBALLY
using AK::Atomic;
using AK::full_memory_barrier;
#endif

5
Kernel/Bus/VirtIO/Queue.cpp
View File

@@ -5,6 +5,7 @@
*/
#include <AK/Atomic.h>
#include <Kernel/Arch/MemoryFences.h>
#include <Kernel/Bus/VirtIO/Queue.h>
#include <Kernel/Library/MiniStdLib.h>
@@ -74,7 +75,7 @@ QueueChain Queue::pop_used_buffer_chain(size_t& used)
return QueueChain(*this);
}
full_memory_barrier();
full_memory_fence();
// Determine used length
used = m_device->rings[m_used_tail % m_queue_size].length;
@@ -180,7 +181,7 @@ void QueueChain::submit_to_queue()
auto next_index = m_queue.m_driver_index_shadow % m_queue.m_queue_size;
m_queue.m_driver->rings[next_index] = m_start_of_chain_index.value();
m_queue.m_driver_index_shadow++;
full_memory_barrier();
full_memory_fence();
m_queue.m_driver->index = m_queue.m_driver_index_shadow;
// Reset internal chain state

23
Kernel/Devices/GPU/Bochs/QEMUDisplayConnector.cpp
View File

@@ -4,6 +4,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/
#include <Kernel/Arch/MemoryFences.h>
#include <Kernel/Debug.h>
#include <Kernel/Devices/Device.h>
#include <Kernel/Devices/GPU/Bochs/QEMUDisplayConnector.h>
@@ -91,32 +92,32 @@ void QEMUDisplayConnector::set_framebuffer_to_big_endian_format()
{
VERIFY(m_modeset_lock.is_locked());
dbgln_if(BXVGA_DEBUG, "QEMUDisplayConnector set_framebuffer_to_big_endian_format");
full_memory_barrier();
full_memory_fence();
if (m_registers->extension_regs.region_size == 0xFFFFFFFF || m_registers->extension_regs.region_size == 0)
return;
full_memory_barrier();
full_memory_fence();
m_registers->extension_regs.framebuffer_byteorder = BOCHS_DISPLAY_BIG_ENDIAN;
full_memory_barrier();
full_memory_fence();
}
void QEMUDisplayConnector::set_framebuffer_to_little_endian_format()
{
VERIFY(m_modeset_lock.is_locked());
dbgln_if(BXVGA_DEBUG, "QEMUDisplayConnector set_framebuffer_to_little_endian_format");
full_memory_barrier();
full_memory_fence();
if (m_registers->extension_regs.region_size == 0xFFFFFFFF || m_registers->extension_regs.region_size == 0)
return;
full_memory_barrier();
full_memory_fence();
m_registers->extension_regs.framebuffer_byteorder = BOCHS_DISPLAY_LITTLE_ENDIAN;
full_memory_barrier();
full_memory_fence();
}
ErrorOr<void> QEMUDisplayConnector::unblank()
{
SpinlockLocker locker(m_modeset_lock);
full_memory_barrier();
full_memory_fence();
m_registers->vga_ioports[0] = 0x20;
full_memory_barrier();
full_memory_fence();
return {};
}
@@ -139,15 +140,15 @@ ErrorOr<void> QEMUDisplayConnector::set_mode_setting(ModeSetting const& mode_set
dbgln_if(BXVGA_DEBUG, "QEMUDisplayConnector resolution registers set to - {}x{}", width, height);
m_registers->bochs_regs.enable = 0;
full_memory_barrier();
full_memory_fence();
m_registers->bochs_regs.xres = width;
m_registers->bochs_regs.yres = height;
m_registers->bochs_regs.virt_width = width;
m_registers->bochs_regs.virt_height = height * 2;
m_registers->bochs_regs.bpp = 32;
full_memory_barrier();
full_memory_fence();
m_registers->bochs_regs.enable = to_underlying(BochsFramebufferSettings::Enabled) | to_underlying(BochsFramebufferSettings::LinearFramebuffer);
full_memory_barrier();
full_memory_fence();
m_registers->bochs_regs.bank = 0;
if (index_id().value() == VBE_DISPI_ID5) {
set_framebuffer_to_little_endian_format();

19
Kernel/Devices/GPU/Intel/Auxiliary/GMBusConnector.cpp
View File

@@ -5,6 +5,7 @@
*/
#include <Kernel/Arch/Delay.h>
#include <Kernel/Arch/MemoryFences.h>
#include <Kernel/Devices/GPU/Intel/Auxiliary/GMBusConnector.h>
#include <Kernel/Memory/PhysicalAddress.h>
@@ -40,9 +41,9 @@ bool GMBusConnector::wait_for(GMBusStatus desired_status, size_t milliseconds_ti
while (1) {
if (milliseconds_timeout < milliseconds_passed)
return false;
full_memory_barrier();
full_memory_fence();
u32 status = m_gmbus_registers->status;
full_memory_barrier();
full_memory_fence();
VERIFY(!(status & (1 << 10))); // error happened
switch (desired_status) {
case GMBusStatus::HardwareReady:
@@ -65,11 +66,11 @@ ErrorOr<void> GMBusConnector::write(unsigned address, u32 data)
{
VERIFY(address < 256);
SpinlockLocker locker(m_access_lock);
full_memory_barrier();
full_memory_fence();
m_gmbus_registers->data = data;
full_memory_barrier();
full_memory_fence();
m_gmbus_registers->command = ((address << 1) | (1 << 16) | (GMBusCycle::Wait << 25) | (1 << 30));
full_memory_barrier();
full_memory_fence();
if (!wait_for(GMBusStatus::TransactionCompletion, 250))
return Error::from_errno(EBUSY);
return {};
@@ -96,9 +97,9 @@ ErrorOr<void> GMBusConnector::read(unsigned address, u8* buf, size_t length)
SpinlockLocker locker(m_access_lock);
size_t nread = 0;
auto read_set = [&] {
full_memory_barrier();
full_memory_fence();
u32 data = m_gmbus_registers->data;
full_memory_barrier();
full_memory_fence();
for (size_t index = 0; index < 4; index++) {
if (nread == length)
break;
@@ -107,9 +108,9 @@ ErrorOr<void> GMBusConnector::read(unsigned address, u8* buf, size_t length)
}
};
full_memory_barrier();
full_memory_fence();
m_gmbus_registers->command = (1 | (address << 1) | (length << 16) | (GMBusCycle::Wait << 25) | (1 << 30));
full_memory_barrier();
full_memory_fence();
while (nread < length) {
if (!wait_for(GMBusStatus::HardwareReady, 250))
return Error::from_errno(EBUSY);

3
Kernel/Devices/GPU/VirtIO/GraphicsAdapter.cpp
View File

@@ -6,6 +6,7 @@
#include <AK/BinaryBufferWriter.h>
#include <Kernel/Arch/Delay.h>
#include <Kernel/Arch/MemoryFences.h>
#include <Kernel/Bus/PCI/API.h>
#include <Kernel/Bus/PCI/IDs.h>
#include <Kernel/Bus/VirtIO/Transport/PCIe/TransportLink.h>
@@ -417,7 +418,7 @@ ErrorOr<void> VirtIOGraphicsAdapter::synchronous_virtio_gpu_command(size_t micro
chain.add_buffer_to_chain(buffer_start, request_size, VirtIO::BufferType::DeviceReadable);
chain.add_buffer_to_chain(buffer_start.offset(request_size), response_size, VirtIO::BufferType::DeviceWritable);
supply_chain_and_notify(CONTROLQ, chain);
full_memory_barrier();
full_memory_fence();
size_t current_time = 0;
ScopeGuard clear_used_buffers([&] {
queue.discard_used_buffers();

7
Kernel/Devices/Storage/AHCI/Controller.cpp
View File

@@ -9,6 +9,7 @@
#include <AK/OwnPtr.h>
#include <AK/Types.h>
#include <Kernel/Arch/Delay.h>
#include <Kernel/Arch/MemoryFences.h>
#include <Kernel/Boot/CommandLine.h>
#include <Kernel/Bus/PCI/API.h>
#include <Kernel/Bus/PCI/BarMapping.h>
@@ -36,7 +37,7 @@ ErrorOr<void> AHCIController::reset()
dbgln_if(AHCI_DEBUG, "{}: AHCI Controller reset", device_identifier().address());
full_memory_barrier();
full_memory_fence();
size_t retry = 0;
// Note: The HBA is locked or hung if we waited more than 1 second!
@@ -49,9 +50,9 @@ ErrorOr<void> AHCIController::reset()
retry++;
}
// Note: Turn on AHCI HBA and Global HBA Interrupts.
full_memory_barrier();
full_memory_fence();
hba().control_regs.ghc = (1 << 31) | (1 << 1);
full_memory_barrier();
full_memory_fence();
}
// Note: According to the AHCI spec the PI register indicates which ports are exposed by the HBA.

41
Kernel/Devices/Storage/AHCI/Port.cpp
View File

@@ -9,6 +9,7 @@
#include <AK/Atomic.h>
#include <Kernel/Arch/Delay.h>
#include <Kernel/Arch/MemoryFences.h>
#include <Kernel/Devices/Storage/AHCI/ATADiskDevice.h>
#include <Kernel/Devices/Storage/AHCI/Port.h>
#include <Kernel/Devices/Storage/StorageManagement.h>
@@ -200,14 +201,14 @@ bool AHCIPort::reset()
dmesgln("AHCI Port {}: Disabled by firmware ", representative_port_index());
return false;
}
full_memory_barrier();
full_memory_fence();
m_interrupt_enable.clear();
m_interrupt_status.clear();
full_memory_barrier();
full_memory_fence();
start_fis_receiving();
full_memory_barrier();
full_memory_fence();
clear_sata_error_register();
full_memory_barrier();
full_memory_fence();
if (!initiate_sata_reset()) {
return false;
}
@@ -243,10 +244,10 @@ bool AHCIPort::initialize()
m_interrupt_status.clear();
m_interrupt_enable.set_all();
full_memory_barrier();
full_memory_fence();
// This actually enables the port...
start_command_list_processing();
full_memory_barrier();
full_memory_fence();
size_t logical_sector_size = 512;
size_t physical_sector_size = 512;
@@ -361,16 +362,16 @@ void AHCIPort::rebase()
VERIFY(m_hard_lock.is_locked());
VERIFY(!m_command_list_page.is_null() && !m_fis_receive_page.is_null());
dbgln_if(AHCI_DEBUG, "AHCI Port {}: Rebasing.", representative_port_index());
full_memory_barrier();
full_memory_fence();
stop_command_list_processing();
stop_fis_receiving();
full_memory_barrier();
full_memory_fence();
// Try to wait 1 second for HBA to clear Command List Running and FIS Receive Running
wait_until_condition_met_or_timeout(1000, 1000, [this]() -> bool {
return !(m_port_registers.cmd & (1 << 15)) && !(m_port_registers.cmd & (1 << 14));
});
full_memory_barrier();
full_memory_fence();
m_port_registers.clbu = 0;
m_port_registers.clb = m_command_list_page->paddr().get();
m_port_registers.fbu = 0;
@@ -548,7 +549,7 @@ bool AHCIPort::access_device(AsyncBlockDeviceRequest::RequestType direction, u64
fis.command = ATA_CMD_READ_DMA_EXT;
}
full_memory_barrier();
full_memory_fence();
fis.device = ATA_USE_LBA_ADDRESSING;
fis.header.port_muliplier = (u8)FIS::HeaderAttributes::C;
@@ -565,9 +566,9 @@ bool AHCIPort::access_device(AsyncBlockDeviceRequest::RequestType direction, u64
if (!spin_until_ready())
return false;
full_memory_barrier();
full_memory_fence();
mark_command_header_ready_to_process(unused_command_header.value());
full_memory_barrier();
full_memory_fence();
dbgln_if(AHCI_DEBUG, "AHCI Port {}: Do a {}, lba {}, block count {} @ {}, ended", representative_port_index(), direction == AsyncBlockDeviceRequest::RequestType::Write ? "write" : "read", lba, block_count, m_dma_buffers[0]->paddr());
return true;
@@ -612,10 +613,10 @@ bool AHCIPort::identify_device()
m_interrupt_enable.clear();
m_interrupt_status.clear();
full_memory_barrier();
full_memory_fence();
dbgln_if(AHCI_DEBUG, "AHCI Port {}: Marking command header at index {} as ready to identify device", representative_port_index(), unused_command_header.value());
m_port_registers.ci = 1 << unused_command_header.value();
full_memory_barrier();
full_memory_fence();
size_t time_elapsed = 0;
bool success = false;
@@ -753,7 +754,7 @@ bool AHCIPort::initiate_sata_reset()
VERIFY(m_hard_lock.is_locked());
dbgln_if(AHCI_DEBUG, "AHCI Port {}: Initiate SATA reset", representative_port_index());
stop_command_list_processing();
full_memory_barrier();
full_memory_fence();
// Note: The AHCI specification says to wait now a 500 milliseconds
// Try to wait 1 second for HBA to clear Command List Running
@@ -761,15 +762,15 @@ bool AHCIPort::initiate_sata_reset()
return !(m_port_registers.cmd & (1 << 15));
});
full_memory_barrier();
full_memory_fence();
spin_up();
full_memory_barrier();
full_memory_fence();
set_interface_state(AHCI::DeviceDetectionInitialization::PerformInterfaceInitializationSequence);
// The AHCI specification says to wait now a 1 millisecond
microseconds_delay(1000);
full_memory_barrier();
full_memory_fence();
set_interface_state(AHCI::DeviceDetectionInitialization::NoActionRequested);
full_memory_barrier();
full_memory_fence();
wait_until_condition_met_or_timeout(10, 1000, [this]() -> bool {
return is_phy_enabled();
@@ -777,7 +778,7 @@ bool AHCIPort::initiate_sata_reset()
dmesgln("AHCI Port {}: {}", representative_port_index(), try_disambiguate_sata_status());
full_memory_barrier();
full_memory_fence();
clear_sata_error_register();
return (m_port_registers.ssts & 0xf) == 3;
}

5
Kernel/Devices/Storage/NVMe/NVMeController.cpp
View File

@@ -9,6 +9,7 @@
#include <AK/Types.h>
#include <Kernel/Arch/Delay.h>
#include <Kernel/Arch/Interrupts.h>
#include <Kernel/Arch/MemoryFences.h>
#include <Kernel/Arch/SafeMem.h>
#include <Kernel/Boot/CommandLine.h>
#include <Kernel/Bus/PCI/API.h>
@@ -110,7 +111,7 @@ ErrorOr<void> NVMeController::reset_controller()
m_controller_regs->cc = cc;
full_memory_barrier();
full_memory_fence();
// Wait until the RDY bit is cleared
if (!wait_for_ready(false))
@@ -136,7 +137,7 @@ ErrorOr<void> NVMeController::start_controller()
m_controller_regs->cc = cc;
full_memory_barrier();
full_memory_fence();
// Wait until the RDY bit is set
if (!wait_for_ready(true))

5
Kernel/Devices/Storage/NVMe/NVMeQueue.cpp
View File

@@ -5,6 +5,7 @@
*/
#include <Kernel/Arch/Delay.h>
#include <Kernel/Arch/MemoryFences.h>
#include <Kernel/Devices/Storage/NVMe/NVMeController.h>
#include <Kernel/Devices/Storage/NVMe/NVMeInterruptQueue.h>
#include <Kernel/Devices/Storage/NVMe/NVMePollQueue.h>
@@ -181,7 +182,7 @@ void NVMeQueue::read(AsyncBlockDeviceRequest& request, u16 nsid, u64 index, u32
requests.set(sub.cmdid, { request, nullptr });
});
full_memory_barrier();
full_memory_fence();
submit_sqe(sub);
}
@@ -206,7 +207,7 @@ void NVMeQueue::write(AsyncBlockDeviceRequest& request, u16 nsid, u64 index, u32
return;
}
full_memory_barrier();
full_memory_fence();
submit_sqe(sub);
}

7
Kernel/Devices/Storage/NVMe/NVMeQueue.h
View File

@@ -10,6 +10,7 @@
#include <AK/HashMap.h>
#include <AK/OwnPtr.h>
#include <AK/Types.h>
#include <Kernel/Arch/MemoryFences.h>
#include <Kernel/Bus/PCI/Device.h>
#include <Kernel/Devices/Storage/NVMe/NVMeDefinitions.h>
#include <Kernel/Interrupts/IRQHandler.h>
@@ -70,7 +71,7 @@ protected:
u32 const old = *dbbuf;
*dbbuf = new_value;
AK::full_memory_barrier();
full_memory_fence();
bool need_mmio = static_cast<u16>(new_value - *ei - 1) < static_cast<u16>(new_value - old);
return need_mmio;
@@ -78,7 +79,7 @@ protected:
void update_sq_doorbell()
{
full_memory_barrier();
full_memory_fence();
if (m_db_regs.dbbuf_shadow.paddr.is_null()
|| update_shadow_buf(m_sq_tail, &m_db_regs.dbbuf_shadow->sq_tail, &m_db_regs.dbbuf_eventidx->sq_tail))
m_db_regs.mmio_reg->sq_tail = m_sq_tail;
@@ -107,7 +108,7 @@ private:
void update_cqe_head();
void update_cq_doorbell()
{
full_memory_barrier();
full_memory_fence();
if (m_db_regs.dbbuf_shadow.paddr.is_null()
|| update_shadow_buf(m_cq_head, &m_db_regs.dbbuf_shadow->cq_head, &m_db_regs.dbbuf_eventidx->cq_head))
m_db_regs.mmio_reg->cq_head = m_cq_head;