Support configurable screen sizes

This change enables Cuttlefish to run with a user specified display size on top
of virtio gpu accelerated graphics rendering.

This change makes the width and height an argument/flag and adds the necessary
plumbing to pass this width and height through the gpu backend.

BUG=b:134086390
TEST=built crosvm and booted cuttlefish locally

Change-Id: Idabf7ef083b2377e3ebf3b50dd0296f4bf7e8ddc
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1927872
Tested-by: kokoro <noreply+kokoro@google.com>
Tested-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Commit-Queue: Jason Macnak <natsu@google.com>

devices/src/virtio/gpu/backend.rs

@@ -30,9 +30,6 @@ use crate::virtio::resource_bridge::*;
 
 use vm_control::{MaybeOwnedFd, VmMemoryControlRequestSocket, VmMemoryRequest, VmMemoryResponse};
 
-const DEFAULT_WIDTH: u32 = 1280;
-const DEFAULT_HEIGHT: u32 = 1024;
-
 struct VirtioGpuResource {
     width: u32,
     height: u32,
@@ -52,12 +49,15 @@ impl VirtioGpuResource {
         }
     }
 
-    pub fn v2_new(kvm_slot: u32, gpu_resource: GpuRendererResource) -> VirtioGpuResource {
-        // Choose DEFAULT_WIDTH and DEFAULT_HEIGHT, since that matches the default modes
-        // for virtgpu-kms.
+    pub fn v2_new(
+        width: u32,
+        height: u32,
+        kvm_slot: u32,
+        gpu_resource: GpuRendererResource,
+    ) -> VirtioGpuResource {
         VirtioGpuResource {
-            width: DEFAULT_WIDTH,
-            height: DEFAULT_HEIGHT,
+            width,
+            height,
             gpu_resource,
             display_import: None,
             kvm_slot: Some(kvm_slot),
@@ -155,6 +155,8 @@ impl VirtioGpuResource {
 /// failure, or requested data for the given command.
 pub struct Backend {
     display: Rc<RefCell<GpuDisplay>>,
+    display_width: u32,
+    display_height: u32,
     renderer: Renderer,
     resources: Map<u32, VirtioGpuResource>,
     contexts: Map<u32, RendererContext>,
@@ -174,12 +176,16 @@ impl Backend {
     /// data is copied as needed.
     pub fn new(
         display: GpuDisplay,
+        display_width: u32,
+        display_height: u32,
         renderer: Renderer,
         gpu_device_socket: VmMemoryControlRequestSocket,
         pci_bar: Alloc,
     ) -> Backend {
         Backend {
             display: Rc::new(RefCell::new(display)),
+            display_width,
+            display_height,
             renderer,
             gpu_device_socket,
             resources: Default::default(),
@@ -231,8 +237,8 @@ impl Backend {
     }
 
     /// Gets the list of supported display resolutions as a slice of `(width, height)` tuples.
-    pub fn display_info(&self) -> &[(u32, u32)] {
-        &[(DEFAULT_WIDTH, DEFAULT_HEIGHT)]
+    pub fn display_info(&self) -> [(u32, u32); 1] {
+        [(self.display_width, self.display_height)]
     }
 
     /// Creates a 2D resource with the given properties and associated it with the given id.
@@ -291,7 +297,7 @@ impl Backend {
             self.scanout_resource = id;
 
             if self.scanout_surface.is_none() {
-                match display.create_surface(None, DEFAULT_WIDTH, DEFAULT_HEIGHT) {
+                match display.create_surface(None, self.display_width, self.display_height) {
                     Ok(surface) => self.scanout_surface = Some(surface),
                     Err(e) => error!("failed to create display surface: {}", e),
                 }
@@ -328,7 +334,13 @@ impl Backend {
             return GpuResponse::OkNoData;
         }
 
-        let fb = match display.framebuffer_region(surface_id, 0, 0, DEFAULT_WIDTH, DEFAULT_HEIGHT) {
+        let fb = match display.framebuffer_region(
+            surface_id,
+            0,
+            0,
+            self.display_width,
+            self.display_height,
+        ) {
             Some(fb) => fb,
             None => {
                 error!("failed to access framebuffer for surface {}", surface_id);
@@ -339,8 +351,8 @@ impl Backend {
         resource.read_to_volatile(
             0,
             0,
-            DEFAULT_WIDTH,
-            DEFAULT_HEIGHT,
+            self.display_width,
+            self.display_height,
             fb.as_volatile_slice(),
             fb.stride(),
         );
@@ -866,7 +878,12 @@ impl Backend {
                             Ok(_resq) => match self.gpu_device_socket.recv() {
                                 Ok(response) => match response {
                                     VmMemoryResponse::RegisterMemory { pfn: _, slot } => {
-                                        entry.insert(VirtioGpuResource::v2_new(slot, resource));
+                                        entry.insert(VirtioGpuResource::v2_new(
+                                            self.display_width,
+                                            self.display_height,
+                                            slot,
+                                            resource,
+                                        ));
                                         GpuResponse::OkNoData
                                     }
                                     VmMemoryResponse::Err(e) => {
@@ -891,8 +908,8 @@ impl Backend {
                     }
                     _ => {
                         entry.insert(VirtioGpuResource::new(
-                            DEFAULT_WIDTH,
-                            DEFAULT_HEIGHT,
+                            self.display_width,
+                            self.display_height,
                             resource,
                         ));
 

devices/src/virtio/gpu/mod.rs

@@ -39,6 +39,20 @@ use crate::pci::{PciBarConfiguration, PciBarPrefetchable, PciBarRegionType};
 
 use vm_control::VmMemoryControlRequestSocket;
 
+pub const DEFAULT_DISPLAY_WIDTH: u32 = 1280;
+pub const DEFAULT_DISPLAY_HEIGHT: u32 = 1024;
+
+#[derive(Debug)]
+pub struct GpuParameters {
+    pub display_width: u32,
+    pub display_height: u32,
+}
+
+pub const DEFAULT_GPU_PARAMS: GpuParameters = GpuParameters {
+    display_width: DEFAULT_DISPLAY_WIDTH,
+    display_height: DEFAULT_DISPLAY_HEIGHT,
+};
+
 // First queue is for virtio gpu commands. Second queue is for cursor commands, which we expect
 // there to be fewer of.
 const QUEUE_SIZES: &[u16] = &[256, 16];
@@ -656,6 +670,8 @@ impl DisplayBackend {
 // failed.
 fn build_backend(
     possible_displays: &[DisplayBackend],
+    display_width: u32,
+    display_height: u32,
     gpu_device_socket: VmMemoryControlRequestSocket,
     pci_bar: Alloc,
 ) -> Option<Backend> {
@@ -701,7 +717,14 @@ fn build_backend(
         }
     };
 
-    Some(Backend::new(display, renderer, gpu_device_socket, pci_bar))
+    Some(Backend::new(
+        display,
+        display_width,
+        display_height,
+        renderer,
+        gpu_device_socket,
+        pci_bar,
+    ))
 }
 
 pub struct Gpu {
@@ -713,6 +736,8 @@ pub struct Gpu {
     worker_thread: Option<thread::JoinHandle<()>>,
     num_scanouts: NonZeroU8,
     display_backends: Vec<DisplayBackend>,
+    display_width: u32,
+    display_height: u32,
     pci_bar: Option<Alloc>,
 }
 
@@ -723,6 +748,7 @@ impl Gpu {
         num_scanouts: NonZeroU8,
         resource_bridges: Vec<ResourceResponseSocket>,
         display_backends: Vec<DisplayBackend>,
+        gpu_parameters: &GpuParameters,
     ) -> Gpu {
         Gpu {
             config_event: false,
@@ -733,6 +759,8 @@ impl Gpu {
             worker_thread: None,
             num_scanouts,
             display_backends,
+            display_width: gpu_parameters.display_width,
+            display_height: gpu_parameters.display_height,
             pci_bar: None,
         }
     }
@@ -851,6 +879,8 @@ impl VirtioDevice for Gpu {
         let cursor_queue = queues.remove(0);
         let cursor_evt = queue_evts.remove(0);
         let display_backends = self.display_backends.clone();
+        let display_width = self.display_width;
+        let display_height = self.display_height;
         if let (Some(gpu_device_socket), Some(pci_bar)) =
             (self.gpu_device_socket.take(), self.pci_bar.take())
         {
@@ -858,11 +888,16 @@ impl VirtioDevice for Gpu {
                 thread::Builder::new()
                     .name("virtio_gpu".to_string())
                     .spawn(move || {
-                        let backend =
-                            match build_backend(&display_backends, gpu_device_socket, pci_bar) {
-                                Some(backend) => backend,
-                                None => return,
-                            };
+                        let backend = match build_backend(
+                            &display_backends,
+                            display_width,
+                            display_height,
+                            gpu_device_socket,
+                            pci_bar,
+                        ) {
+                            Some(backend) => backend,
+                            None => return,
+                        };
 
                         Worker {
                             interrupt,

devices/src/virtio/mod.rs

@@ -7,8 +7,6 @@
 mod balloon;
 mod block;
 mod descriptor_utils;
-#[cfg(feature = "gpu")]
-mod gpu;
 mod input;
 mod interrupt;
 mod net;
@@ -24,6 +22,8 @@ mod virtio_pci_device;
 mod wl;
 
 pub mod fs;
+#[cfg(feature = "gpu")]
+pub mod gpu;
 pub mod resource_bridge;
 pub mod vhost;
 

src/crosvm.rs

@@ -17,6 +17,8 @@ use std::path::PathBuf;
 use std::str::FromStr;
 
 use devices::virtio::fs::passthrough;
+#[cfg(feature = "gpu")]
+use devices::virtio::gpu::GpuParameters;
 use devices::SerialParameters;
 use libc::{getegid, geteuid};
 
@@ -146,7 +148,8 @@ pub struct Config {
     pub sandbox: bool,
     pub seccomp_policy_dir: PathBuf,
     pub seccomp_log_failures: bool,
-    pub gpu: bool,
+    #[cfg(feature = "gpu")]
+    pub gpu_parameters: Option<GpuParameters>,
     pub software_tpm: bool,
     pub cras_audio: bool,
     pub cras_capture: bool,
@@ -184,7 +187,8 @@ impl Default for Config {
             vhost_net: false,
             tap_fd: Vec::new(),
             cid: None,
-            gpu: false,
+            #[cfg(feature = "gpu")]
+            gpu_parameters: None,
             software_tpm: false,
             wayland_socket_path: None,
             wayland_dmabuf: false,

src/linux.rs

@@ -618,6 +618,7 @@ fn create_gpu_device(
         NonZeroU8::new(1).unwrap(), // number of scanouts
         gpu_sockets,
         display_backends,
+        cfg.gpu_parameters.as_ref().unwrap(),
     );
 
     let jail = match simple_jail(&cfg, "gpu_device.policy")? {
@@ -970,7 +971,7 @@ fn create_virtio_devices(
 
         #[cfg(feature = "gpu")]
         {
-            if cfg.gpu {
+            if cfg.gpu_parameters.is_some() {
                 let (wl_socket, gpu_socket) =
                     virtio::resource_bridge::pair().map_err(Error::CreateSocket)?;
                 resource_bridges.push(gpu_socket);
@@ -988,7 +989,7 @@ fn create_virtio_devices(
 
     #[cfg(feature = "gpu")]
     {
-        if cfg.gpu {
+        if cfg.gpu_parameters.is_some() {
             devs.push(create_gpu_device(
                 cfg,
                 _exit_evt,

src/main.rs

@@ -19,6 +19,8 @@ use crosvm::{
     argument::{self, print_help, set_arguments, Argument},
     linux, BindMount, Config, DiskOption, Executable, GidMap, SharedDir, TouchDeviceOption,
 };
+#[cfg(feature = "gpu")]
+use devices::virtio::gpu::{GpuParameters, DEFAULT_GPU_PARAMS};
 use devices::{SerialParameters, SerialType};
 use msg_socket::{MsgReceiver, MsgSender, MsgSocket};
 use qcow::QcowFile;
@@ -108,6 +110,48 @@ fn parse_cpu_set(s: &str) -> argument::Result<Vec<usize>> {
     Ok(cpuset)
 }
 
+#[cfg(feature = "gpu")]
+fn parse_gpu_options(s: Option<&str>) -> argument::Result<GpuParameters> {
+    let mut gpu_params = DEFAULT_GPU_PARAMS;
+
+    if let Some(s) = s {
+        let opts = s
+            .split(",")
+            .map(|frag| frag.split("="))
+            .map(|mut kv| (kv.next().unwrap_or(""), kv.next().unwrap_or("")));
+
+        for (k, v) in opts {
+            match k {
+                "width" => {
+                    gpu_params.display_width =
+                        v.parse::<u32>()
+                            .map_err(|_| argument::Error::InvalidValue {
+                                value: v.to_string(),
+                                expected: "gpu parameter 'width' must be a valid integer",
+                            })?;
+                }
+                "height" => {
+                    gpu_params.display_height =
+                        v.parse::<u32>()
+                            .map_err(|_| argument::Error::InvalidValue {
+                                value: v.to_string(),
+                                expected: "gpu parameter 'height' must be a valid integer",
+                            })?;
+                }
+                "" => {}
+                _ => {
+                    return Err(argument::Error::UnknownArgument(format!(
+                        "gpu parameter {}",
+                        k
+                    )));
+                }
+            }
+        }
+    }
+
+    Ok(gpu_params)
+}
+
 fn parse_serial_options(s: &str) -> argument::Result<SerialParameters> {
     let mut serial_setting = SerialParameters {
         type_: SerialType::Sink,
@@ -713,8 +757,10 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument::
                     })?,
             );
         }
+        #[cfg(feature = "gpu")]
         "gpu" => {
-            cfg.gpu = true;
+            let params = parse_gpu_options(value)?;
+            cfg.gpu_parameters = Some(params);
         }
         "software-tpm" => {
             cfg.software_tpm = true;
@@ -864,7 +910,7 @@ fn run_vm(args: std::env::Args) -> std::result::Result<(), ()> {
           Argument::flag("null-audio", "Add an audio device to the VM that plays samples to /dev/null"),
           Argument::value("serial",
                           "type=TYPE,[num=NUM,path=PATH,console,stdin]",
-                          "Comma seperated key=value pairs for setting up serial devices. Can be given more than once.
+                          "Comma separated key=value pairs for setting up serial devices. Can be given more than once.
                           Possible key values:
                           type=(stdout,syslog,sink,file) - Where to route the serial device
                           num=(1,2,3,4) - Serial Device Number. If not provided, num will default to 1.
@@ -909,7 +955,13 @@ writeback=BOOL - Indicates whether the VM can use writeback caching (default: fa
                           "fd",
                           "File descriptor for configured tap device. A different virtual network card will be added each time this argument is given."),
           #[cfg(feature = "gpu")]
-          Argument::flag("gpu", "(EXPERIMENTAL) enable virtio-gpu device"),
+          Argument::flag_or_value("gpu",
+                                  "[width=INT,height=INT]",
+                                  "(EXPERIMENTAL) Comma separated key=value pairs for setting up a virtio-gpu device
+                                  Possible key values:
+                                  width=INT - The width of the virtual display connected to the virtio-gpu.
+                                  height=INT - The height of the virtual display connected to the virtio-gpu.
+                                  "),
           #[cfg(feature = "tpm")]
           Argument::flag("software-tpm", "enable a software emulated trusted platform module device"),
           Argument::value("evdev", "PATH", "Path to an event device node. The device will be grabbed (unusable from the host) and made available to the guest with the same configuration it shows on the host"),