Kernel/Graphics: Propagate errors properly around in the VirtIO driver

This happens to be a sad truth for the VirtIOGPU driver - it lacked any
error propagation measures and generally relied on clunky assumptions
that most operations with the GPU device are infallible, although in
reality much of them could fail, so we do need to handle errors.

To fix this, synchronous GPU commands no longer rely on the wait queue
mechanism anymore, so instead we introduce a timeout-based mechanism,
similar to how other Kernel drivers use a polling based mechanism with
the assumption that hardware could get stuck in an error state and we
could abort gracefully.

Then, we change most of the VirtIOGraphicsAdapter methods to propagate
errors properly to the original callers, to ensure that if a synchronous
GPU command failed, either the Kernel or userspace could do something
meaningful about this situation.

Author: supercomputer7

Kernel/Graphics/VirtIOGPU/DisplayConnector.cpp

@@ -122,9 +122,9 @@ ErrorOr<void> VirtIODisplayConnector::flush_rectangle(size_t buffer_index, FBRec
         .height = rect.height
     };
 
-    m_graphics_adapter->transfer_framebuffer_data_to_host({}, *this, dirty_rect, true);
+    TRY(m_graphics_adapter->transfer_framebuffer_data_to_host({}, *this, dirty_rect, true));
     // Flushing directly to screen
-    flush_displayed_image(dirty_rect, true);
+    TRY(flush_displayed_image(dirty_rect, true));
     return {};
 }
 
@@ -139,9 +139,9 @@ ErrorOr<void> VirtIODisplayConnector::flush_first_surface()
         .height = m_display_info.rect.height
     };
 
-    m_graphics_adapter->transfer_framebuffer_data_to_host({}, *this, dirty_rect, true);
+    TRY(m_graphics_adapter->transfer_framebuffer_data_to_host({}, *this, dirty_rect, true));
     // Flushing directly to screen
-    flush_displayed_image(dirty_rect, true);
+    TRY(flush_displayed_image(dirty_rect, true));
     return {};
 }
 
@@ -241,10 +241,11 @@ void VirtIODisplayConnector::draw_ntsc_test_pattern(Badge<VirtIOGraphicsAdapter>
     dbgln_if(VIRTIO_DEBUG, "Finish drawing the pattern");
 }
 
-void VirtIODisplayConnector::flush_displayed_image(Graphics::VirtIOGPU::Protocol::Rect const& dirty_rect, bool main_buffer)
+ErrorOr<void> VirtIODisplayConnector::flush_displayed_image(Graphics::VirtIOGPU::Protocol::Rect const& dirty_rect, bool main_buffer)
 {
     VERIFY(m_graphics_adapter->operation_lock().is_locked());
-    m_graphics_adapter->flush_displayed_image({}, *this, dirty_rect, main_buffer);
+    TRY(m_graphics_adapter->flush_displayed_image({}, *this, dirty_rect, main_buffer));
+    return {};
 }
 
 void VirtIODisplayConnector::set_dirty_displayed_rect(Graphics::VirtIOGPU::Protocol::Rect const& dirty_rect, bool main_buffer)

Kernel/Graphics/VirtIOGPU/DisplayConnector.h

@@ -68,7 +68,7 @@ class VirtIODisplayConnector final : public DisplayConnector {
 private:
     VirtIODisplayConnector(VirtIOGraphicsAdapter& graphics_adapter, Graphics::VirtIOGPU::ScanoutID scanout_id);
 
-    void flush_displayed_image(Graphics::VirtIOGPU::Protocol::Rect const& dirty_rect, bool main_buffer);
+    ErrorOr<void> flush_displayed_image(Graphics::VirtIOGPU::Protocol::Rect const& dirty_rect, bool main_buffer);
     void set_dirty_displayed_rect(Graphics::VirtIOGPU::Protocol::Rect const& dirty_rect, bool main_buffer);
 
     void query_display_information();

Kernel/Graphics/VirtIOGPU/GPU3DDevice.cpp

@@ -94,12 +94,12 @@ ErrorOr<void> VirtIOGPU3DDevice::ioctl(OpenFileDescription& description, unsigne
         SpinlockLocker locker(m_graphics_adapter->operation_lock());
         auto user_command_buffer = static_ptr_cast<VirGLCommandBuffer const*>(arg);
         auto command_buffer = TRY(copy_typed_from_user(user_command_buffer));
-        m_graphics_adapter->submit_command_buffer(context_id, [&](Bytes buffer) {
+        TRY(m_graphics_adapter->submit_command_buffer(context_id, [&](Bytes buffer) {
             auto num_bytes = command_buffer.num_elems * sizeof(u32);
             VERIFY(num_bytes <= buffer.size());
             MUST(copy_from_user(buffer.data(), command_buffer.data, num_bytes));
             return num_bytes;
-        });
+        }));
         return {};
     }
     case VIRGL_IOCTL_CREATE_RESOURCE: {
@@ -121,10 +121,12 @@ ErrorOr<void> VirtIOGPU3DDevice::ioctl(OpenFileDescription& description, unsigne
             .padding = 0,
         };
         SpinlockLocker locker(m_graphics_adapter->operation_lock());
-        auto resource_id = m_graphics_adapter->create_3d_resource(resource_spec).value();
-        m_graphics_adapter->attach_resource_to_context(resource_id, per_context_state->context_id());
-        m_graphics_adapter->ensure_backing_storage(resource_id, per_context_state->transfer_buffer_region(), 0, NUM_TRANSFER_REGION_PAGES * PAGE_SIZE);
-        spec.created_resource_id = resource_id;
+        // FIXME: What would be an appropriate resource free-ing mechanism to use in case anything
+        // after this fails?
+        auto resource_id = TRY(m_graphics_adapter->create_3d_resource(resource_spec));
+        TRY(m_graphics_adapter->attach_resource_to_context(resource_id, per_context_state->context_id()));
+        TRY(m_graphics_adapter->ensure_backing_storage(resource_id, per_context_state->transfer_buffer_region(), 0, NUM_TRANSFER_REGION_PAGES * PAGE_SIZE));
+        spec.created_resource_id = resource_id.value();
         // FIXME: We should delete the resource we just created if we fail to copy the resource id out
         return copy_to_user(static_ptr_cast<VirGL3DResourceSpec*>(arg), &spec);
     }