From 63c239496da61c1fbdc1c2c866f5719534e22503 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 6 Mar 2020 16:32:20 -0800 Subject: Fix into_iter() usage where iter() suffices Fixes warnings of the form: warning: this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter` (due to autoref coercions), but that might change in the future when `IntoIterator` impls for arrays are added. BUG=None TEST=emerge-nami crosvm Change-Id: I2b46b55f0e967d985d04678c240604b542e27e07 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2093287 Reviewed-by: Dylan Reid Tested-by: kokoro Commit-Queue: Daniel Verkamp --- src/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/linux.rs') diff --git a/src/linux.rs b/src/linux.rs index 2de1aaa..d63ab32 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -1751,7 +1751,7 @@ fn run_control( .map_err(Error::PollContextAdd)?; if let Some(gsi_relay) = &linux.gsi_relay { - for (gsi, evt) in gsi_relay.irqfd.into_iter().enumerate() { + for (gsi, evt) in gsi_relay.irqfd.iter().enumerate() { if let Some(evt) = evt { poll_ctx .add(evt, Token::IrqFd { gsi }) -- cgit 1.4.1 From d5c1e968c9454af0da573242a8dc3dcf63dd1820 Mon Sep 17 00:00:00 2001 From: Judy Hsiao Date: Tue, 4 Feb 2020 12:30:01 +0800 Subject: audio: Create AC97 device with --ac97 option 1. Replace --cras-audio, --cras-capture, null-audio options by --ac97 option to create audio devices. 2. "--ac97 backend=BACKEND\ [capture=true,capture_effect=EFFECT]" is comma separated key=value pairs for setting up Ac97 devices. It can be given more than once to create multiple devices. Possible key values are: backend=(null, cras) - Where to route the audio device. `null` for /dev/null, and cras for CRAS server. capture=true - Enable audio capture. capture_effects - | separated effects to be enabled for recording. The only supported effect value now is EchoCancellation or aec. BUG=b:140866281 TEST=1.crosvm run -r ./vm_rootfs.img -c 4 -m 1024 -s /run --cid 5 --host_ip\ 100.115.92.25 --netmask 255.255.255.252 --ac97\ backend=cras,capture=true,capture_effect=aec\ --mac d2:47:f7:c5:9e:53 ./vm_kernel 2. Record with the vm by: arecord -D hw:0,0 -d5 -fS16_LE -c2 -r48000 /tmp/test.mp3 3. Verify that AEC is enabled within the recording stream by cras_test_cleint. Cq-Depend: chromium:2053654 Cq-Depend: chromium:2095644 Cq-Depend: chromium:2038221 Change-Id: Ia9e0e7cda1671a4842ec77a354efaa4a2dc745eb Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2038413 Tested-by: Judy Hsiao Commit-Queue: Judy Hsiao Reviewed-by: Chih-Yang Hsia Auto-Submit: Judy Hsiao --- Cargo.lock | 1 + devices/Cargo.toml | 1 + devices/src/lib.rs | 4 +- devices/src/pci/ac97.rs | 88 +++++++++++++++++++++++++++++++- devices/src/pci/ac97_bus_master.rs | 11 +++- devices/src/pci/mod.rs | 2 +- devices/src/pci/pci_device.rs | 3 ++ src/crosvm.rs | 10 ++-- src/linux.rs | 37 ++++---------- src/main.rs | 101 ++++++++++++++++++++++++++++++++----- 10 files changed, 208 insertions(+), 50 deletions(-) (limited to 'src/linux.rs') diff --git a/Cargo.lock b/Cargo.lock index bb75235..e9da7f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -180,6 +180,7 @@ dependencies = [ "kvm 0.1.0", "kvm_sys 0.1.0", "libc 0.2.44 (registry+https://github.com/rust-lang/crates.io-index)", + "libcras 0.1.0", "linux_input_sys 0.1.0", "msg_on_socket_derive 0.1.0", "msg_socket 0.1.0", diff --git a/devices/Cargo.toml b/devices/Cargo.toml index 83aa406..629f29b 100644 --- a/devices/Cargo.toml +++ b/devices/Cargo.toml @@ -25,6 +25,7 @@ io_jail = { path = "../io_jail" } kvm = { path = "../kvm" } kvm_sys = { path = "../kvm_sys" } libc = "*" +libcras = "*" linux_input_sys = { path = "../linux_input_sys" } msg_on_socket_derive = { path = "../msg_socket/msg_on_socket_derive" } msg_socket = { path = "../msg_socket" } diff --git a/devices/src/lib.rs b/devices/src/lib.rs index 1ecfc9c..9ed8417 100644 --- a/devices/src/lib.rs +++ b/devices/src/lib.rs @@ -30,8 +30,8 @@ pub use self::cmos::Cmos; pub use self::i8042::I8042Device; pub use self::ioapic::{Ioapic, IOAPIC_BASE_ADDRESS, IOAPIC_MEM_LENGTH_BYTES}; pub use self::pci::{ - Ac97Dev, PciConfigIo, PciConfigMmio, PciDevice, PciDeviceError, PciInterruptPin, PciRoot, - VfioPciDevice, + Ac97Backend, Ac97Dev, Ac97Parameters, PciConfigIo, PciConfigMmio, PciDevice, PciDeviceError, + PciInterruptPin, PciRoot, VfioPciDevice, }; pub use self::pic::Pic; pub use self::pit::{Pit, PitError}; diff --git a/devices/src/pci/ac97.rs b/devices/src/pci/ac97.rs index 792df24..5f59165 100644 --- a/devices/src/pci/ac97.rs +++ b/devices/src/pci/ac97.rs @@ -2,9 +2,17 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +use std::default::Default; +use std::error; +use std::fmt::{self, Display}; use std::os::unix::io::RawFd; +use std::str::FromStr; -use audio_streams::shm_streams::ShmStreamSource; +use audio_streams::{ + shm_streams::{NullShmStreamSource, ShmStreamSource}, + StreamEffect, +}; +use libcras::CrasClient; use resources::{Alloc, MmioType, SystemAllocator}; use sys_util::{error, EventFd, GuestMemory}; @@ -25,6 +33,53 @@ const PCI_DEVICE_ID_INTEL_82801AA_5: u16 = 0x2415; /// Internally the `Ac97BusMaster` and `Ac97Mixer` structs are used to emulated the bus master and /// mixer registers respectively. `Ac97BusMaster` handles moving smaples between guest memory and /// the audio backend. +#[derive(Debug, Clone)] +pub enum Ac97Backend { + NULL, + CRAS, +} + +impl Default for Ac97Backend { + fn default() -> Self { + Ac97Backend::NULL + } +} + +/// Errors that are possible from a `Ac97`. +#[derive(Debug)] +pub enum Ac97Error { + InvalidBackend, +} + +impl error::Error for Ac97Error {} + +impl Display for Ac97Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Ac97Error::InvalidBackend => write!(f, "Must be cras or null"), + } + } +} + +impl FromStr for Ac97Backend { + type Err = Ac97Error; + fn from_str(s: &str) -> std::result::Result { + match s { + "cras" => Ok(Ac97Backend::CRAS), + "null" => Ok(Ac97Backend::NULL), + _ => Err(Ac97Error::InvalidBackend), + } + } +} + +/// Holds the parameters for a AC97 device +#[derive(Default, Debug, Clone)] +pub struct Ac97Parameters { + pub backend: Ac97Backend, + pub capture: bool, + pub capture_effects: Vec, +} + pub struct Ac97Dev { config_regs: PciConfiguration, pci_bus_dev: Option<(u8, u8)>, @@ -61,6 +116,37 @@ impl Ac97Dev { } } + fn create_cras_audio_device(params: Ac97Parameters, mem: GuestMemory) -> Result { + let mut server = + Box::new(CrasClient::new().map_err(|e| pci_device::Error::CreateCrasClientFailed(e))?); + if params.capture { + server.enable_cras_capture(); + } + + let mut cras_audio = Ac97Dev::new(mem, server); + cras_audio.set_capture_effects(params.capture_effects); + Ok(cras_audio) + } + + fn create_null_audio_device(mem: GuestMemory) -> Result { + let server = Box::new(NullShmStreamSource::new()); + let null_audio = Ac97Dev::new(mem, server); + Ok(null_audio) + } + + /// Creates an 'Ac97Dev' with suitable audio server inside based on Ac97Parameters + pub fn try_new(mem: GuestMemory, param: Ac97Parameters) -> Result { + match param.backend { + Ac97Backend::CRAS => Ac97Dev::create_cras_audio_device(param, mem), + Ac97Backend::NULL => Ac97Dev::create_null_audio_device(mem), + } + } + + /// Provides the effect needed in capture stream creation + pub fn set_capture_effects(&mut self, effect: Vec) { + self.bus_master.set_capture_effects(effect); + } + fn read_mixer(&mut self, offset: u64, data: &mut [u8]) { match data.len() { // The mixer is only accessed with 16-bit words. diff --git a/devices/src/pci/ac97_bus_master.rs b/devices/src/pci/ac97_bus_master.rs index cb28c5a..5f4ca75 100644 --- a/devices/src/pci/ac97_bus_master.rs +++ b/devices/src/pci/ac97_bus_master.rs @@ -165,6 +165,9 @@ pub struct Ac97BusMaster { po_info: AudioThreadInfo, pi_info: AudioThreadInfo, + // Audio effect + capture_effects: Vec, + // Audio server used to create playback or capture streams. audio_server: Box, @@ -184,12 +187,18 @@ impl Ac97BusMaster { po_info: AudioThreadInfo::new(), pi_info: AudioThreadInfo::new(), + capture_effects: Vec::new(), audio_server, irq_resample_thread: None, } } + /// Provides the effect needed in capture stream creation. + pub fn set_capture_effects(&mut self, effects: Vec) { + self.capture_effects = effects; + } + /// Returns any file descriptors that need to be kept open when entering a jail. pub fn keep_fds(&self) -> Option> { let mut fds = self.audio_server.keep_fds(); @@ -518,7 +527,7 @@ impl Ac97BusMaster { SampleFormat::S16LE, DEVICE_SAMPLE_RATE, buffer_frames, - StreamEffect::NoEffect, + self.capture_effects.as_slice(), self.mem.as_ref(), starting_offsets, ) diff --git a/devices/src/pci/mod.rs b/devices/src/pci/mod.rs index a9e8749..8c5380e 100644 --- a/devices/src/pci/mod.rs +++ b/devices/src/pci/mod.rs @@ -14,7 +14,7 @@ mod pci_device; mod pci_root; mod vfio_pci; -pub use self::ac97::Ac97Dev; +pub use self::ac97::{Ac97Backend, Ac97Dev, Ac97Parameters}; pub use self::msix::{MsixCap, MsixConfig}; pub use self::pci_configuration::{ PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, PciCapability, PciCapabilityID, diff --git a/devices/src/pci/pci_device.rs b/devices/src/pci/pci_device.rs index 4c62e05..a1a3cca 100644 --- a/devices/src/pci/pci_device.rs +++ b/devices/src/pci/pci_device.rs @@ -22,6 +22,8 @@ pub enum Error { IoAllocationFailed(u64, SystemAllocatorFaliure), /// Registering an IO BAR failed. IoRegistrationFailed(u64, pci_configuration::Error), + /// Create cras client failed. + CreateCrasClientFailed(libcras::Error), } pub type Result = std::result::Result; @@ -31,6 +33,7 @@ impl Display for Error { match self { CapabilitiesSetup(e) => write!(f, "failed to add capability {}", e), + CreateCrasClientFailed(e) => write!(f, "failed to create CRAS Client: {}", e), IoAllocationFailed(size, e) => write!( f, "failed to allocate space for an IO BAR, size={}: {}", diff --git a/src/crosvm.rs b/src/crosvm.rs index b3a9233..81344c3 100644 --- a/src/crosvm.rs +++ b/src/crosvm.rs @@ -20,7 +20,7 @@ use arch::Pstore; use devices::virtio::fs::passthrough; #[cfg(feature = "gpu")] use devices::virtio::gpu::GpuParameters; -use devices::SerialParameters; +use devices::{Ac97Parameters, SerialParameters}; use libc::{getegid, geteuid}; static SECCOMP_POLICY_DIR: &str = "/usr/share/policy/crosvm"; @@ -189,11 +189,9 @@ pub struct Config { #[cfg(feature = "gpu")] pub gpu_parameters: Option, pub software_tpm: bool, - pub cras_audio: bool, - pub cras_capture: bool, - pub null_audio: bool, pub display_window_keyboard: bool, pub display_window_mouse: bool, + pub ac97_parameters: Vec, pub serial_parameters: BTreeMap, pub syslog_tag: Option, pub virtio_single_touch: Option, @@ -240,9 +238,7 @@ impl Default for Config { sandbox: !cfg!(feature = "default-no-sandbox"), seccomp_policy_dir: PathBuf::from(SECCOMP_POLICY_DIR), seccomp_log_failures: false, - cras_audio: false, - cras_capture: false, - null_audio: false, + ac97_parameters: Vec::new(), serial_parameters: BTreeMap::new(), syslog_tag: None, virtio_single_touch: None, diff --git a/src/linux.rs b/src/linux.rs index d63ab32..130644d 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -27,17 +27,15 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use libc::{self, c_int, gid_t, uid_t}; -use audio_streams::shm_streams::NullShmStreamSource; #[cfg(feature = "gpu")] use devices::virtio::EventDevice; use devices::virtio::{self, VirtioDevice}; use devices::{ - self, HostBackendDeviceProvider, PciDevice, VfioContainer, VfioDevice, VfioPciDevice, - VirtioPciDevice, XhciController, + self, Ac97Backend, Ac97Dev, HostBackendDeviceProvider, PciDevice, VfioContainer, VfioDevice, + VfioPciDevice, VirtioPciDevice, XhciController, }; use io_jail::{self, Minijail}; use kvm::*; -use libcras::CrasClient; use msg_socket::{MsgError, MsgReceiver, MsgSender, MsgSocket}; use net_util::{Error as NetError, MacAddress, Tap}; use rand_ish::SimpleRng; @@ -83,7 +81,7 @@ pub enum Error { BuildVm(::Error), ChownTpmStorage(sys_util::Error), CloneEventFd(sys_util::Error), - CreateCrasClient(libcras::Error), + CreateAc97(devices::PciDeviceError), CreateDiskError(disk::Error), CreateEventFd(sys_util::Error), CreatePollContext(sys_util::Error), @@ -168,7 +166,7 @@ impl Display for Error { BuildVm(e) => write!(f, "The architecture failed to build the vm: {}", e), ChownTpmStorage(e) => write!(f, "failed to chown tpm storage: {}", e), CloneEventFd(e) => write!(f, "failed to clone eventfd: {}", e), - CreateCrasClient(e) => write!(f, "failed to create cras client: {}", e), + CreateAc97(e) => write!(f, "failed to create ac97 device: {}", e), CreateDiskError(e) => write!(f, "failed to create virtual disk: {}", e), CreateEventFd(e) => write!(f, "failed to create eventfd: {}", e), CreatePollContext(e) => write!(f, "failed to create poll context: {}", e), @@ -1150,27 +1148,14 @@ fn create_devices( pci_devices.push((dev, stub.jail)); } - if cfg.cras_audio { - let mut server = Box::new(CrasClient::new().map_err(Error::CreateCrasClient)?); - if cfg.cras_capture { - server.enable_cras_capture(); - } - let cras_audio = devices::Ac97Dev::new(mem.clone(), server); - - pci_devices.push(( - Box::new(cras_audio), - simple_jail(&cfg, "cras_audio_device")?, - )); - } - - if cfg.null_audio { - let server = Box::new(NullShmStreamSource::new()); - let null_audio = devices::Ac97Dev::new(mem.clone(), server); + for ac97_param in &cfg.ac97_parameters { + let dev = Ac97Dev::try_new(mem.clone(), ac97_param.clone()).map_err(Error::CreateAc97)?; + let policy = match ac97_param.backend { + Ac97Backend::CRAS => "cras_audio_device", + Ac97Backend::NULL => "null_audio_device", + }; - pci_devices.push(( - Box::new(null_audio), - simple_jail(&cfg, "null_audio_device")?, - )); + pci_devices.push((Box::new(dev), simple_jail(&cfg, &policy)?)); } // Create xhci controller. let usb_controller = Box::new(XhciController::new(mem.clone(), usb_provider)); diff --git a/src/main.rs b/src/main.rs index a068057..3afca8e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,6 +6,7 @@ pub mod panic_hook; +use std::default::Default; use std::fmt; use std::fs::{File, OpenOptions}; use std::io::{BufRead, BufReader}; @@ -17,13 +18,14 @@ use std::thread::sleep; use std::time::Duration; use arch::Pstore; +use audio_streams::StreamEffect; use crosvm::{ argument::{self, print_help, set_arguments, Argument}, linux, BindMount, Config, DiskOption, Executable, GidMap, SharedDir, TouchDeviceOption, }; #[cfg(feature = "gpu")] use devices::virtio::gpu::{GpuMode, GpuParameters}; -use devices::{SerialParameters, SerialType}; +use devices::{Ac97Backend, Ac97Parameters, SerialParameters, SerialType}; use disk::QcowFile; use msg_socket::{MsgReceiver, MsgSender, MsgSocket}; use sys_util::{ @@ -249,6 +251,53 @@ fn parse_gpu_options(s: Option<&str>) -> argument::Result { Ok(gpu_params) } +fn parse_ac97_options(s: &str) -> argument::Result { + let mut ac97_params: Ac97Parameters = Default::default(); + + 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 { + "backend" => { + ac97_params.backend = + v.parse::() + .map_err(|e| argument::Error::InvalidValue { + value: v.to_string(), + expected: e.to_string(), + })?; + } + "capture" => { + ac97_params.capture = v.parse::().map_err(|e| { + argument::Error::Syntax(format!("invalid capture option: {}", e)) + })?; + } + "capture_effects" => { + ac97_params.capture_effects = v + .split("|") + .map(|val| { + val.parse::() + .map_err(|e| argument::Error::InvalidValue { + value: val.to_string(), + expected: e.to_string(), + }) + }) + .collect::>>()?; + } + _ => { + return Err(argument::Error::UnknownArgument(format!( + "unknown ac97 parameter {}", + k + ))); + } + } + } + + Ok(ac97_params) +} + fn parse_serial_options(s: &str) -> argument::Result { let mut serial_setting = SerialParameters { type_: SerialType::Sink, @@ -479,14 +528,9 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument:: })?, ) } - "cras-audio" => { - cfg.cras_audio = true; - } - "cras-capture" => { - cfg.cras_capture = true; - } - "null-audio" => { - cfg.null_audio = true; + "ac97" => { + let ac97_params = parse_ac97_options(value.unwrap())?; + cfg.ac97_parameters.push(ac97_params); } "serial" => { let serial_params = parse_serial_options(value.unwrap())?; @@ -1193,9 +1237,14 @@ fn run_vm(args: std::env::Args) -> std::result::Result<(), ()> { "IP address to assign to host tap interface."), Argument::value("netmask", "NETMASK", "Netmask for VM subnet."), Argument::value("mac", "MAC", "MAC address for VM."), - Argument::flag("cras-audio", "Add an audio device to the VM that plays samples through CRAS server"), - Argument::flag("cras-capture", "Enable capturing audio from CRAS server to the cras-audio device"), - Argument::flag("null-audio", "Add an audio device to the VM that plays samples to /dev/null"), + Argument::value("ac97", + "[backend=BACKEND,capture=true,capture_effect=EFFECT]", + "Comma separated key=value pairs for setting up Ac97 devices. Can be given more than once . + Possible key values: + backend=(null, cras) - Where to route the audio device. If not provided, backend will default to null. + `null` for /dev/null, and cras for CRAS server. + capture - Enable audio capture + capture_effects - | separated effects to be enabled for recording. The only supported effect value now is EchoCancellation or aec."), Argument::value("serial", "type=TYPE,[num=NUM,path=PATH,console,stdin]", "Comma separated key=value pairs for setting up serial devices. Can be given more than once. @@ -1844,6 +1893,34 @@ mod tests { parse_cpu_set("0,1,2,").expect_err("parse should have failed"); } + #[test] + fn parse_ac97_vaild() { + parse_ac97_options("backend=cras").expect("parse should have succeded"); + } + + #[test] + fn parse_ac97_null_vaild() { + parse_ac97_options("backend=null").expect("parse should have succeded"); + } + + #[test] + fn parse_ac97_dup_effect_vaild() { + parse_ac97_options("backend=cras,capture=true,capture_effects=aec|aec") + .expect("parse should have succeded"); + } + + #[test] + fn parse_ac97_effect_invaild() { + parse_ac97_options("backend=cras,capture=true,capture_effects=abc") + .expect_err("parse should have failed"); + } + + #[test] + fn parse_ac97_effect_vaild() { + parse_ac97_options("backend=cras,capture=true,capture_effects=aec") + .expect("parse should have succeded"); + } + #[test] fn parse_serial_vaild() { parse_serial_options("type=syslog,num=1,console=true,stdin=true") -- cgit 1.4.1