summary refs log tree commit diff
diff options
context:
space:
mode:
authorDaniel Verkamp <dverkamp@chromium.org>2020-01-31 15:19:55 -0800
committerCommit Bot <commit-bot@chromium.org>2020-02-05 09:18:36 +0000
commitf054de59b3d6efbfa7daa6d4300bec9ca94b778a (patch)
treebb67a333a2e06341077a943e252bc5182e491b8b
parent7e24a8e75934608c3475e0dc5c48d5a3bfc17b31 (diff)
downloadcrosvm-f054de59b3d6efbfa7daa6d4300bec9ca94b778a.tar
crosvm-f054de59b3d6efbfa7daa6d4300bec9ca94b778a.tar.gz
crosvm-f054de59b3d6efbfa7daa6d4300bec9ca94b778a.tar.bz2
crosvm-f054de59b3d6efbfa7daa6d4300bec9ca94b778a.tar.lz
crosvm-f054de59b3d6efbfa7daa6d4300bec9ca94b778a.tar.xz
crosvm-f054de59b3d6efbfa7daa6d4300bec9ca94b778a.tar.zst
crosvm-f054de59b3d6efbfa7daa6d4300bec9ca94b778a.zip
devices: pci: fix writable_bits for 64-bit BARs
The high 32 bits of writable_bits was set incorrectly when adding 64-bit
memory BARs to PciConfiguration: it would effectively always be all
zeroes (no writable bits) instead of all ones (all writable bits).

The writable_bits field is used to determine which bits to force to 0
when reading the BAR, which is used by the guest to determine the size
of a BAR: write an all-ones value to the BAR, read it back, and the
resulting value has only the writable bits still set.  Since PCI BARs
must be a power of two in size, the effective size of the BAR is the
bitwise inverse of the resulting value plus one.

For 64-bit BARs, this process is the same, except that two contiguous
32-bit registers are combined, so for a 4096-byte 64-bit BAR, the
writable_bits field should be 0xFFFFFFFF_FFFFF000; however, with the
previous (buggy) code, it was 0x00000000_FFFFF000.

Add checks to the unit tests to verify that the writable_bits field is
correctly calculated as well.

BUG=None
TEST=cargo test -p devices pci_configuration
TEST=Boot Linux 4.19 kernel in crosvm

Change-Id: Ib97aa5dccf9bf042328c0fc9defe1797fc67bb05
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2033620
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
-rw-r--r--devices/src/pci/pci_configuration.rs6
1 files changed, 5 insertions, 1 deletions
diff --git a/devices/src/pci/pci_configuration.rs b/devices/src/pci/pci_configuration.rs
index 0deee54..ebfa2a6 100644
--- a/devices/src/pci/pci_configuration.rs
+++ b/devices/src/pci/pci_configuration.rs
@@ -411,7 +411,7 @@ impl PciConfiguration {
                 }
 
                 self.registers[bar_idx + 1] = (config.addr >> 32) as u32;
-                self.writable_bits[bar_idx + 1] = !((config.size >> 32).wrapping_sub(1)) as u32;
+                self.writable_bits[bar_idx + 1] = !((config.size - 1) >> 32) as u32;
                 self.bar_used[config.reg_idx + 1] = true;
             }
         }
@@ -724,6 +724,8 @@ mod tests {
             Some(PciBarRegionType::Memory64BitRegion)
         );
         assert_eq!(cfg.get_bar_addr(0), 0x01234567_89ABCDE0);
+        assert_eq!(cfg.writable_bits[BAR0_REG + 1], 0xFFFFFFFF);
+        assert_eq!(cfg.writable_bits[BAR0_REG + 0], 0xFFFFFFFC);
     }
 
     #[test]
@@ -755,6 +757,7 @@ mod tests {
             Some(PciBarRegionType::Memory32BitRegion)
         );
         assert_eq!(cfg.get_bar_addr(0), 0x12345670);
+        assert_eq!(cfg.writable_bits[BAR0_REG], 0xFFFFFFFC);
     }
 
     #[test]
@@ -783,5 +786,6 @@ mod tests {
 
         assert_eq!(cfg.get_bar_type(0), Some(PciBarRegionType::IORegion));
         assert_eq!(cfg.get_bar_addr(0), 0x1230);
+        assert_eq!(cfg.writable_bits[BAR0_REG], 0xFFFFFFFC);
     }
 }