Merge branch 'fluke/gfx-vfio-optional' into 'main'

Put vfio behind config option

See merge request asus-linux/asus-nb-ctrl!43

CHANGELOG.md

@@ -5,6 +5,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+# [3.3.0] - 2021-04-3
+### Changed
+- Add ledmodes for G733QS
+- Add ledmodes for GA401Q
+- Default to vfio disabled in configuration. Will now hard-error if enabled and
+  the kernel modules are builtin. To enable vfio switching `"gfx_vfio_enable": false,`
+  must be changed to `true` in `/etc/asusd/asusd.conf`
+
+# [3.2.4] - 2021-03-24
+### Changed
+- Ignore vfio-builtin error if switching to integrated
 
 # [3.2.3] - 2021-03-24
 ### Changed

Cargo.lock

@@ -197,7 +197,7 @@ dependencies = [
 
 [[package]]
 name = "daemon"
-version = "3.2.3"
+version = "3.3.0"
 dependencies = [
  "env_logger",
  "intel-pstate",

README.md

@@ -81,6 +81,12 @@ stray configs blocking nvidia modules from loading in:
 - `/etc/modprobe.d/`
 - `/usr/lib/modprope.d/`
 
+**VFIO NOTE:** The vfio modules *must not* be compiled into the kernel, they need
+to be separate modules. If you don't plan to use vfio mode then you can ignore this
+otherwise you may need a custom built kernel.
+
+To enable vfio switching you need to edit `/etc/asusd/asusd.conf` and change `"gfx_vfio_enable": false,` to true.
+
 ### Power management udev rule
 
 If you have installed the Nvidia driver manually you will require the
@@ -89,8 +95,8 @@ If you have installed the Nvidia driver manually you will require the
 ### fedora and openSUSE
 
 You *may* need a file `/etc/dracut.conf.d/90-nvidia-dracut-G05.conf` installed
-to stop dracut including the nvidia modules in the ramdisk. This is espeically
-true if you manually installed the nvidia drivers.
+to stop dracut including the nvidia modules in the ramdisk if you manually
+installed the nvidia drivers.
 
 ```
 # filename /etc/dracut.conf.d/90-nvidia-dracut-G05.conf
@@ -122,7 +128,8 @@ If you model isn't getting the correct led modes, you can edit the file
 - Comet
 - Flash
 
-use `cat /sys/class/dmi/id/product_name` to get details about your laptop.
+use `cat /sys/class/dmi/id/product_name` to get details about your laptop. You
+must restart the `asusd.service` after editing.
 
 # Keybinds
 
@@ -152,8 +159,6 @@ Packaging and auto-builds are available [here](https://build.opensuse.org/packag
 
 Download repositories are available [here](https://download.opensuse.org/repositories/home:/luke_nukem:/asus/)
 
-Alternatively check the releases page for f33 RPM.
-
 ---
 
 Run `make` then `sudo make install` then reboot.
@@ -208,6 +213,7 @@ can enable the user service to get basic notifications when something changes.
 systemctl --user enable asus-notify.service
 systemctl --user start asus-notify.service
 ```
+
 # OTHER
 
 ## AniMe input

asusctl/examples/animatrix.rs

@@ -1,12 +1,11 @@
 use rog_dbus::AuraDbusClient;
-use rog_types::anime_matrix::{AniMeImageBuffer, AniMePacketType, HEIGHT, WIDTH};
+use rog_types::anime_matrix::{AniMeImageBuffer, HEIGHT, WIDTH};
 use tinybmp::{Bmp, Pixel};
 
 fn main() {
     let (client, _) = AuraDbusClient::new().unwrap();
 
-    let bmp =
-        Bmp::from_slice(include_bytes!("non-skewed_r.bmp")).expect("Failed to parse BMP image");
+    let bmp = Bmp::from_slice(include_bytes!("rust.bmp")).expect("Failed to parse BMP image");
     let pixels: Vec<Pixel> = bmp.into_iter().collect();
     //assert_eq!(pixels.len(), 56 * 56);
 
@@ -16,27 +15,30 @@ fn main() {
     // Aligned left
     for (i, px) in pixels.iter().enumerate() {
         if (px.x as usize / 2) < WIDTH && (px.y as usize) < HEIGHT && px.x % 2 == 0 {
-            let mut c = px.color as u32;
+            let c = px.color as u32;
             matrix.get_mut()[px.y as usize][px.x as usize / 2] = c as u8;
         }
     }
 
     // Throw an alignment border up
-    // {
-    //     let tmp = matrix.get_mut();
-    //     for x in tmp[0].iter_mut() {
-    //         *x = 0xff;
-    //     }
-    //     for row in tmp.iter_mut() {
-    //         row[row.len() - 1] = 0xff;
-    //     }
-    // }
+    {
+        let tmp = matrix.get_mut();
+        for x in tmp[0].iter_mut() {
+            *x = 0xff;
+        }
+        for (i, row) in tmp.iter_mut().enumerate() {
+            if i % 2 == 0 {
+                let l = row.len();
+                row[l - 1] = 0xff;
+            }
+        }
+    }
 
     matrix.debug_print();
 
-    let mut matrix: AniMePacketType = AniMePacketType::from(matrix);
+    //let mut matrix: AniMePacketType = AniMePacketType::from(matrix);
     // println!("{:?}", matrix[0].to_vec());
     // println!("{:?}", matrix[1].to_vec());
 
-    //client.proxies().anime().set_brightness(&mut matrix).unwrap();
+    client.proxies().anime().write_image(matrix).unwrap();
 }

asusctl/src/main.rs

@@ -143,6 +143,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 
     if parsed.help {
         print_supported_help(&supported, &parsed);
+        println!("\nSee https://asus-linux.org/faq/ for additional help");
         std::process::exit(1);
     }
 

daemon/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "daemon"
-version = "3.2.3"
+version = "3.3.0"
 license = "MPL-2.0"
 readme = "README.md"
 authors = ["Luke <luke@ljones.dev>"]

daemon/src/config.rs

@@ -15,6 +15,7 @@ pub static AURA_CONFIG_PATH: &str = "/etc/asusd/asusd.conf";
 pub struct Config {
     pub gfx_mode: GfxVendors,
     pub gfx_managed: bool,
+    pub gfx_vfio_enable: bool,
     pub active_profile: String,
     pub toggle_profiles: Vec<String>,
     #[serde(skip)]
@@ -33,6 +34,7 @@ impl Default for Config {
         Config {
             gfx_mode: GfxVendors::Hybrid,
             gfx_managed: true,
+            gfx_vfio_enable: false,
             active_profile: "normal".into(),
             toggle_profiles: vec!["normal".into(), "boost".into(), "silent".into()],
             curr_fan_mode: 0,
@@ -63,6 +65,11 @@ impl Config {
             } else {
                 if let Ok(data) = serde_json::from_str(&buf) {
                     return data;
+                } else if let Ok(data) = serde_json::from_str::<ConfigV324>(&buf) {
+                    let config = data.into_current();
+                    config.write();
+                    info!("Updated config version to: {}", VERSION);
+                    return config;
                 } else if let Ok(data) = serde_json::from_str::<ConfigV317>(&buf) {
                     let config = data.into_current();
                     config.write();

daemon/src/config_old.rs

@@ -25,6 +25,7 @@ impl ConfigV212 {
             gfx_mode: GfxVendors::Hybrid,
             gfx_managed: self.gfx_managed,
             active_profile: self.active_profile,
+            gfx_vfio_enable: false,
             toggle_profiles: self.toggle_profiles,
             curr_fan_mode: self.power_profile,
             bat_charge_limit: self.bat_charge_limit,
@@ -53,6 +54,7 @@ impl ConfigV222 {
         Config {
             gfx_mode: GfxVendors::Hybrid,
             gfx_managed: self.gfx_managed,
+            gfx_vfio_enable: false,
             active_profile: self.active_profile,
             toggle_profiles: self.toggle_profiles,
             curr_fan_mode: self.power_profile,
@@ -84,6 +86,7 @@ impl ConfigV301 {
         Config {
             gfx_mode: GfxVendors::Hybrid,
             gfx_managed: self.gfx_managed,
+            gfx_vfio_enable: false,
             active_profile: self.active_profile,
             toggle_profiles: self.toggle_profiles,
             curr_fan_mode: self.curr_fan_mode,
@@ -115,6 +118,34 @@ impl ConfigV317 {
         Config {
             gfx_mode: GfxVendors::Hybrid,
             gfx_managed: self.gfx_managed,
+            gfx_vfio_enable: false,
+            active_profile: self.active_profile,
+            toggle_profiles: self.toggle_profiles,
+            curr_fan_mode: self.curr_fan_mode,
+            bat_charge_limit: self.bat_charge_limit,
+            power_profiles: self.power_profiles,
+        }
+    }
+}
+
+#[derive(Deserialize, Serialize)]
+pub struct ConfigV324 {
+    pub gfx_mode: GfxVendors,
+    pub gfx_managed: bool,
+    pub active_profile: String,
+    pub toggle_profiles: Vec<String>,
+    #[serde(skip)]
+    pub curr_fan_mode: u8,
+    pub bat_charge_limit: u8,
+    pub power_profiles: BTreeMap<String, Profile>,
+}
+
+impl ConfigV324 {
+    pub(crate) fn into_current(self) -> Config {
+        Config {
+            gfx_mode: GfxVendors::Hybrid,
+            gfx_managed: self.gfx_managed,
+            gfx_vfio_enable: false,
             active_profile: self.active_profile,
             toggle_profiles: self.toggle_profiles,
             curr_fan_mode: self.curr_fan_mode,

daemon/src/ctrl_gfx/error.rs

@@ -11,7 +11,10 @@ pub enum GfxError {
     DisplayManagerTimeout(String),
     GsyncModeActive,
     VfioBuiltin,
+    VfioDisabled,
     MissingModule(String),
+    Modprobe(String),
+    Command(String, std::io::Error),
 }
 
 impl fmt::Display for GfxError {
@@ -34,7 +37,12 @@ impl fmt::Display for GfxError {
                 f,
                 "Can not switch to vfio mode if the modules are built in to kernel"
             ),
+            GfxError::VfioDisabled => {
+                write!(f, "Can not switch to vfio mode if disabled in config file")
+            }
             GfxError::MissingModule(m) => write!(f, "The module {} is missing", m),
+            GfxError::Modprobe(detail) => write!(f, "Modprobe error: {}", detail),
+            GfxError::Command(func, error) => write!(f, "Command exec error: {}: {}", func, error),
         }
     }
 }

daemon/src/ctrl_gfx/gfx.rs

@@ -95,10 +95,8 @@ impl Reloadable for CtrlGraphics {
 impl CtrlGraphics {
     pub fn new(config: Arc<Mutex<Config>>) -> std::io::Result<CtrlGraphics> {
         let bus = PciBus::new()?;
-
         info!("GFX: Rescanning PCI bus");
         bus.rescan()?;
-
         let devs = PciDevice::all()?;
 
         let functions = |parent: &PciDevice| -> Vec<PciDevice> {
@@ -169,7 +167,6 @@ impl CtrlGraphics {
             config.gfx_mode = vendor;
             config.write();
         }
-        // TODO: Error here
     }
 
     /// Associated method to get which vendor mode is set
@@ -187,6 +184,7 @@ impl CtrlGraphics {
         Ok(buf)
     }
 
+    /// Some systems have a fallback service to load nouveau if nvidia fails
     fn toggle_fallback_service(vendor: GfxVendors) -> Result<(), RogError> {
         let action = if vendor == GfxVendors::Nvidia {
             info!("GFX: Enabling nvidia-fallback.service");
@@ -213,6 +211,7 @@ impl CtrlGraphics {
         Ok(())
     }
 
+    /// Write the appropriate xorg config for the chosen mode
     fn write_xorg_conf(vendor: GfxVendors) -> Result<(), RogError> {
         let text = if vendor == GfxVendors::Nvidia {
             [PRIMARY_GPU_BEGIN, PRIMARY_GPU_NVIDIA, PRIMARY_GPU_END].concat()
@@ -239,6 +238,7 @@ impl CtrlGraphics {
         Ok(())
     }
 
+    /// Creates the full modprobe.conf required for vfio pass-through
     fn get_vfio_conf(devices: &[GraphicsDevice]) -> Vec<u8> {
         let mut vifo = MODPROBE_VFIO.to_vec();
         for (d_count, dev) in devices.iter().enumerate() {
@@ -303,7 +303,7 @@ impl CtrlGraphics {
             .map_err(|err| RogError::Command("device unbind error".into(), err))
     }
 
-    fn do_driver_action(driver: &str, action: &str) -> Result<(), RogError> {
+    fn do_driver_action(driver: &str, action: &str) -> Result<(), GfxError> {
         let mut cmd = Command::new(action);
         cmd.arg(driver);
 
@@ -318,7 +318,7 @@ impl CtrlGraphics {
 
             let output = cmd
                 .output()
-                .map_err(|err| RogError::Command(format!("{:?}", cmd), err))?;
+                .map_err(|err| GfxError::Command(format!("{:?}", cmd), err))?;
             if !output.status.success() {
                 if output
                     .stderr
@@ -327,7 +327,7 @@ impl CtrlGraphics {
                     return Ok(());
                 }
                 if output.stderr.ends_with("is builtin.\n".as_bytes()) {
-                    return Err(GfxError::VfioBuiltin.into());
+                    return Err(GfxError::VfioBuiltin);
                 }
                 if output.stderr.ends_with("Permission denied\n".as_bytes()) {
                     warn!(
@@ -342,7 +342,7 @@ impl CtrlGraphics {
                 if String::from_utf8_lossy(&output.stderr)
                     .contains(&format!("Module {} not found", driver))
                 {
-                    return Err(GfxError::MissingModule(driver.into()).into());
+                    return Err(GfxError::MissingModule(driver.into()));
                 }
                 if count >= MAX_TRIES {
                     let msg = format!(
@@ -351,7 +351,7 @@ impl CtrlGraphics {
                         driver,
                         String::from_utf8_lossy(&output.stderr)
                     );
-                    return Err(RogError::Modprobe(msg));
+                    return Err(GfxError::Modprobe(msg));
                 }
             } else if output.status.success() {
                 return Ok(());
@@ -427,6 +427,7 @@ impl CtrlGraphics {
     /// The daemon needs direct access to this function when it detects that the
     pub fn do_vendor_tasks(
         vendor: GfxVendors,
+        vfio_enable: bool,
         devices: &[GraphicsDevice],
         bus: &PciBus,
     ) -> Result<(), RogError> {
@@ -439,25 +440,33 @@ impl CtrlGraphics {
 
         match vendor {
             GfxVendors::Nvidia | GfxVendors::Hybrid | GfxVendors::Compute => {
-                for driver in VFIO_DRIVERS.iter() {
-                    Self::do_driver_action(driver, "rmmod")?;
+                if vfio_enable {
+                    for driver in VFIO_DRIVERS.iter() {
+                        Self::do_driver_action(driver, "rmmod")?;
+                    }
                 }
                 for driver in NVIDIA_DRIVERS.iter() {
                     Self::do_driver_action(driver, "modprobe")?;
                 }
             }
             GfxVendors::Vfio => {
-                Self::do_driver_action("nouveau", "rmmod")?;
-                for driver in NVIDIA_DRIVERS.iter() {
-                    Self::do_driver_action(driver, "rmmod")?;
+                if vfio_enable {
+                    Self::do_driver_action("nouveau", "rmmod")?;
+                    for driver in NVIDIA_DRIVERS.iter() {
+                        Self::do_driver_action(driver, "rmmod")?;
+                    }
+                    Self::unbind_only(&devices)?;
+                    Self::do_driver_action("vfio-pci", "modprobe")?;
+                } else {
+                    return Err(GfxError::VfioDisabled.into());
                 }
-                Self::unbind_only(&devices)?;
-                Self::do_driver_action("vfio-pci", "modprobe")?;
             }
             GfxVendors::Integrated => {
                 Self::do_driver_action("nouveau", "rmmod")?;
-                for driver in VFIO_DRIVERS.iter() {
-                    Self::do_driver_action(driver, "rmmod")?;
+                if vfio_enable {
+                    for driver in VFIO_DRIVERS.iter() {
+                        Self::do_driver_action(driver, "rmmod")?;
+                    }
                 }
                 for driver in NVIDIA_DRIVERS.iter() {
                     Self::do_driver_action(driver, "rmmod")?;
@@ -468,7 +477,8 @@ impl CtrlGraphics {
         Ok(())
     }
 
-    fn graphical_session_alive(
+    /// Check if the user has any graphical uiser sessions that are active or online
+    fn graphical_user_sessions_exist(
         connection: &Connection,
         sessions: &[SessionInfo],
     ) -> Result<bool, RogError> {
@@ -513,7 +523,7 @@ impl CtrlGraphics {
                 sessions = tmp;
             }
 
-            if !Self::graphical_session_alive(&connection, &sessions)? {
+            if !Self::graphical_user_sessions_exist(&connection, &sessions)? {
                 break;
             }
 
@@ -536,7 +546,13 @@ impl CtrlGraphics {
         Self::do_display_manager_action("stop")?;
         Self::wait_display_manager_state("inactive")?;
 
-        Self::do_vendor_tasks(vendor, &devices, &bus)?;
+        let vfio_enable = if let Ok(config) = config.lock() {
+            config.gfx_vfio_enable
+        } else {
+            false
+        };
+
+        Self::do_vendor_tasks(vendor, vfio_enable, &devices, &bus)?;
         Self::do_display_manager_action("restart")?;
         // Save selected mode in case of reboot
         Self::save_gfx_mode(vendor, config);
@@ -547,6 +563,7 @@ impl CtrlGraphics {
         Ok(format!("Graphics mode changed to {} successfully", v))
     }
 
+    /// Before starting a new thread the old one *must* be cancelled
     fn cancel_thread(&self) {
         if let Ok(lock) = self.thread_kill.lock() {
             if let Some(tx) = lock.as_ref() {
@@ -599,6 +616,17 @@ impl CtrlGraphics {
                 return Err(GfxError::GsyncModeActive.into());
             }
         }
+
+        let vfio_enable = if let Ok(config) = self.config.lock() {
+            config.gfx_vfio_enable
+        } else {
+            false
+        };
+
+        if !vfio_enable && matches!(vendor, GfxVendors::Vfio) {
+            return Err(GfxError::VfioDisabled.into());
+        }
+
         // Must always cancel any thread running
         self.cancel_thread();
         // determine which method we need here
@@ -612,7 +640,7 @@ impl CtrlGraphics {
             info!("GFX: mode change does not require logout");
             let devices = self.nvidia.clone();
             let bus = self.bus.clone();
-            Self::do_vendor_tasks(vendor, &devices, &bus)?;
+            Self::do_vendor_tasks(vendor, vfio_enable, &devices, &bus)?;
             info!("GFX: Graphics mode changed to {}", <&str>::from(vendor));
         }
         // TODO: undo if failed? Save last mode, catch errors...
@@ -624,7 +652,14 @@ impl CtrlGraphics {
         let vendor = self.get_gfx_mode()?;
         let devices = self.nvidia.clone();
         let bus = self.bus.clone();
-        Self::do_vendor_tasks(vendor, &devices, &bus)?;
+
+        let vfio_enable = if let Ok(config) = self.config.lock() {
+            config.gfx_vfio_enable
+        } else {
+            false
+        };
+
+        Self::do_vendor_tasks(vendor, vfio_enable, &devices, &bus)?;
         Self::toggle_fallback_service(vendor)?;
         Ok(())
     }

daemon/src/daemon.rs

@@ -109,7 +109,12 @@ fn start_daemon() -> Result<(), Box<dyn Error>> {
                                 warn!("Dedicated GFX toggle is on but driver mode is not nvidia \nSetting to nvidia driver mode");
                                 let devices = ctrl.devices();
                                 let bus = ctrl.bus();
-                                CtrlGraphics::do_vendor_tasks(GfxVendors::Nvidia, &devices, &bus)?;
+                                CtrlGraphics::do_vendor_tasks(
+                                    GfxVendors::Nvidia,
+                                    false,
+                                    &devices,
+                                    &bus,
+                                )?;
                             } else if ded == 0 {
                                 info!("Dedicated GFX toggle is off");
                             }

data/asusd-ledmodes.toml

@@ -35,14 +35,14 @@ per_key = false
 
 [[led_data]]
 prod_family = "ROG Strix"
-board_names = ["G531GW"]
+board_names = ["G531GW", "G733QS", "G733QR"]
 standard = ["Static", "Breathe", "Strobe", "Rainbow", "Star", "Rain", "Highlight", "Laser", "Ripple", "Pulse", "Comet", "Flash"]
 multizone = false
 per_key = true
 
 [[led_data]]
 prod_family = "ROG Strix"
-board_names = ["GX531", "G512LV", "G712LV"]
+board_names = ["GX531", "G512LV", "G712LV", "G712LW"]
 standard = ["Static", "Breathe", "Strobe", "Rainbow", "Pulse"]
 multizone = true
 per_key = false
@@ -80,4 +80,11 @@ prod_family = "ROG"
 board_names = ["GL553VE"]
 standard = ["Static", "Breathe", "Strobe"]
 multizone = true
+per_key = false
+
+[[led_data]]
+prod_family = "ROG Zephyrus G14"
+board_names = ["GA401Q"]
+standard = ["Static", "Breathe", "Pulse"]
+multizone = false
 per_key = false