vasilito/RedBear-OS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

graphics/virtio-gpud: Remove unnecessary VolatileCell

Browse Source 
The virtio queue abstraction already provides the necessary
synchronization to make everything data-race free without volatile
accesses.
This commit is contained in:
bjorn3
2024-12-28 14:01:09 +01:00
parent 9afe3b390b
commit 1d89ca7bd2
2 changed files with 39 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
graphics/virtio-gpud/src/main.rs
+27 -52
View File
@@ -22,7 +22,6 @@
#![feature(int_roundings)]
use std::cell::UnsafeCell;
use std::os::fd::AsRawFd;
use std::sync::atomic::{AtomicU32, Ordering};
@@ -41,22 +40,16 @@ macro_rules! make_getter_setter {
($($field:ident: $return_ty:ty),*) => {
$(
pub fn $field(&self) -> $return_ty {
self.$field.get()
self.$field
}
paste::item! {
pub fn [<set_ $field>](&mut self, value: $return_ty) {
self.$field.set(value)
self.$field = value;
}
}
)*
};
(@$field:ident: $return_ty:ty) => {
pub fn $field(&mut self, value: $return_ty) {
self.$field.set(value)
}
};
}
#[repr(C)]
@@ -137,18 +130,18 @@ static_assertions::const_assert_eq!(core::mem::size_of::<CommandTy>(), 4);
#[derive(Debug)]
#[repr(C)]
pub struct ControlHeader {
pub ty: VolatileCell<CommandTy>,
pub flags: VolatileCell<u32>,
pub fence_id: VolatileCell<u64>,
pub ctx_id: VolatileCell<u32>,
pub ring_index: VolatileCell<u8>,
pub ty: CommandTy,
pub flags: u32,
pub fence_id: u64,
pub ctx_id: u32,
pub ring_index: u8,
padding: [u8; 3],
}
impl ControlHeader {
pub fn with_ty(ty: CommandTy) -> Self {
Self {
ty: VolatileCell::new(ty),
ty,
..Default::default()
}
}
@@ -157,11 +150,11 @@ impl ControlHeader {
impl Default for ControlHeader {
fn default() -> Self {
Self {
ty: VolatileCell::new(CommandTy::Undefined),
flags: VolatileCell::new(0),
fence_id: VolatileCell::new(0),
ctx_id: VolatileCell::new(0),
ring_index: VolatileCell::new(0),
ty: CommandTy::Undefined,
flags: 0,
fence_id: 0,
ctx_id: 0,
ring_index: 0,
padding: [0; 3],
}
}
@@ -190,16 +183,9 @@ impl GpuRect {
#[derive(Debug)]
#[repr(C)]
pub struct DisplayInfo {
rect: UnsafeCell<GpuRect>,
pub enabled: VolatileCell<u32>,
pub flags: VolatileCell<u32>,
}
impl DisplayInfo {
pub fn rect(&self) -> &GpuRect {
// SAFETY: We never give out mutable references to `self.rect`.
unsafe { &*self.rect.get() }
}
rect: GpuRect,
pub enabled: u32,
pub flags: u32,
}
#[derive(Debug)]
@@ -213,7 +199,7 @@ impl Default for GetDisplayInfo {
fn default() -> Self {
Self {
header: ControlHeader {
ty: VolatileCell::new(CommandTy::GetDisplayInfo),
ty: CommandTy::GetDisplayInfo,
..Default::default()
},
@@ -247,13 +233,10 @@ pub enum ResourceFormat {
#[repr(C)]
pub struct ResourceCreate2d {
pub header: ControlHeader,
// FIXME we can likely use regular loads and stores as the ring buffer should provide the
// necessary synchronization.
resource_id: VolatileCell<ResourceId>,
format: VolatileCell<ResourceFormat>,
width: VolatileCell<u32>,
height: VolatileCell<u32>,
resource_id: ResourceId,
format: ResourceFormat,
width: u32,
height: u32,
}
impl ResourceCreate2d {
@@ -263,15 +246,11 @@ impl ResourceCreate2d {
impl Default for ResourceCreate2d {
fn default() -> Self {
Self {
header: ControlHeader {
ty: VolatileCell::new(CommandTy::ResourceCreate2d),
..Default::default()
},
resource_id: VolatileCell::new(ResourceId(0)),
format: VolatileCell::new(ResourceFormat::Unknown),
width: VolatileCell::new(0),
height: VolatileCell::new(0),
header: ControlHeader::with_ty(CommandTy::ResourceCreate2d),
resource_id: ResourceId(0),
format: ResourceFormat::Unknown,
width: 0,
height: 0,
}
}
}
@@ -392,11 +371,7 @@ pub struct XferToHost2d {
impl XferToHost2d {
pub fn new(resource_id: ResourceId, rect: GpuRect, offset: u64) -> Self {
Self {
header: ControlHeader {
ty: VolatileCell::new(CommandTy::TransferToHost2d),
..Default::default()
},
header: ControlHeader::with_ty(CommandTy::TransferToHost2d),
rect,
offset,
resource_id,
graphics/virtio-gpud/src/scheme.rs
+12 -16
View File
@@ -8,7 +8,6 @@ use syscall::PAGE_SIZE;
use virtio_core::spec::{Buffer, ChainBuilder, DescriptorFlags};
use virtio_core::transport::{Error, Queue, Transport};
use virtio_core::utils::VolatileCell;
use crate::*;
@@ -67,7 +66,7 @@ impl VirtGpuAdapter<'_> {
async fn flush_resource_inner(&self, flush: ResourceFlush) -> Result<(), Error> {
let header = self.send_request(Dma::new(flush)?).await?;
assert_eq!(header.ty.get(), CommandTy::RespOkNodata);
assert_eq!(header.ty, CommandTy::RespOkNodata);
Ok(())
}
@@ -108,7 +107,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> {
request.set_resource_id(res_id);
let header = self.send_request(request).await.unwrap();
assert_eq!(header.ty.get(), CommandTy::RespOkNodata);
assert_eq!(header.ty, CommandTy::RespOkNodata);
// Use the allocated framebuffer from tthe guest ram, and attach it as backing
// storage to the resource just created, using `VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING`.
@@ -133,7 +132,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> {
.build();
self.control_queue.send(command).await;
assert_eq!(header.ty.get(), CommandTy::RespOkNodata);
assert_eq!(header.ty, CommandTy::RespOkNodata);
VirtGpuResource {
id: res_id,
@@ -162,7 +161,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> {
))
.unwrap();
let header = self.send_request(scanout_request).await.unwrap();
assert_eq!(header.ty.get(), CommandTy::RespOkNodata);
assert_eq!(header.ty, CommandTy::RespOkNodata);
});
self.flush_resource(display_id, resource, None);
@@ -187,7 +186,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> {
))
.unwrap();
let header = self.send_request(req).await.unwrap();
assert_eq!(header.ty.get(), CommandTy::RespOkNodata);
assert_eq!(header.ty, CommandTy::RespOkNodata);
if let Some(damage) = damage {
for damage in damage {
@@ -259,11 +258,11 @@ impl<'a> GpuScheme<'a> {
for info in displays.iter() {
log::info!(
"virtio-gpu: opening display ({}x{}px)",
info.rect().width,
info.rect().height
info.rect.width,
info.rect.height
);
if info.rect().width == 0 || info.rect().height == 0 {
if info.rect.width == 0 || info.rect.height == 0 {
// QEMU gives all displays other than the first a zero width and height, but trying
// to attach a zero sized framebuffer to the display will result an error, so
// default to 640x480px.
@@ -273,8 +272,8 @@ impl<'a> GpuScheme<'a> {
});
} else {
result.push(Display {
width: info.rect().width,
height: info.rect().height,
width: info.rect.width,
height: info.rect.height,
});
}
}
@@ -283,10 +282,7 @@ impl<'a> GpuScheme<'a> {
}
async fn get_display_info(control_queue: Arc<Queue<'a>>) -> Result<Dma<GetDisplayInfo>, Error> {
let header = Dma::new(ControlHeader {
ty: VolatileCell::new(CommandTy::GetDisplayInfo),
..Default::default()
})?;
let header = Dma::new(ControlHeader::with_ty(CommandTy::GetDisplayInfo))?;
let response = Dma::new(GetDisplayInfo::default())?;
let command = ChainBuilder::new()
@@ -295,7 +291,7 @@ impl<'a> GpuScheme<'a> {
.build();
control_queue.send(command).await;
assert!(response.header.ty.get() == CommandTy::RespOkDisplayInfo);
assert!(response.header.ty == CommandTy::RespOkDisplayInfo);
Ok(response)
}