x86/hvmloader: round up memory BAR size to 4K

When placing memory BARs with sizes smaller than 4K multiple memory
BARs can end up mapped to the same guest physical address, and thus
won't work correctly.

Round up all memory BAR sizes to be at least 4K, so that they are
naturally aligned to a page size and thus don't end up sharing a page.
Also add a couple of asserts to the current code to make sure the MMIO
hole is properly sized and aligned.

Note that the guest can still move the BARs around and create this
collisions, and that BARs not filling up a physical page might leak
access to other MMIO regions placed in the same host physical page.

This is however no worse than what's currently done, and hence should
be considered an improvement over the current state.

Reported-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jason Andryuk <jandryuk@gmail.com>
---
Changes since v1:
 - Do the round up when sizing the BARs, so that the MMIO hole is
   correctly sized.
 - Add some asserts that the hole is properly sized and size-aligned.
 - Dropped Jason Tested-by since the code has changed.
---
Jason, can you give this a spin? Thanks.

diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
index 0b708bf578e37394d9a3d95a253f6872c819dfd4..ac4fb5aa82d85c1348c1664ebe5835da0e4443c7 100644 (file)
--- a/tools/firmware/hvmloader/pci.c
+++ b/tools/firmware/hvmloader/pci.c
@@ -253,6 +253,15 @@ void pci_setup(void)
             if ( bar_sz == 0 )
                 continue;
 
+            if ( ((bar_data & PCI_BASE_ADDRESS_SPACE) ==
+                  PCI_BASE_ADDRESS_SPACE_MEMORY) ||
+                 (bar_reg == PCI_ROM_ADDRESS) )
+                /*
+                 * Always roundup memory BAR sizes to the size of a page in
+                 * order to prevent BARs being placed in the same page.
+                 */
+                bar_sz = ROUNDUP(bar_sz, PAGE_SIZE);
+
             for ( i = 0; i < nr_bars; i++ )
                 if ( bars[i].bar_sz < bar_sz )
                     break;
@@ -295,6 +304,8 @@ void pci_setup(void)
         pci_writew(devfn, PCI_COMMAND, cmd);
     }
 
+    ASSERT(IS_ALIGNED(mmio_total, PAGE_SIZE));
+
     if ( mmio_hole_size )
     {
         uint64_t max_ram_below_4g = GB(4) - mmio_hole_size;
@@ -448,6 +459,7 @@ void pci_setup(void)
                 resource = &mem_resource;
                 bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
             }
+            ASSERT(bar_sz <= mmio_total);
             mmio_total -= bar_sz;
         }
         else

diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index 7bca6418d2d384e05e32d50c399f2492c8a4f9e3..e1ebfd66476380ee7edb287ce3d7e15ebbf84938 100644 (file)
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -51,6 +51,9 @@ void __bug(char *file, int line) __attribute__((noreturn));
 #define MB(mb) (mb##ULL << 20)
 #define GB(gb) (gb##ULL << 30)
 
+#define ROUNDUP(x, a) (((x) + (a) - 1) & ~((a) - 1))
+#define IS_ALIGNED(val, align) (((val) & ((align) - 1)) == 0)
+
 static inline int test_bit(unsigned int b, const void *p)
 {
     return !!(((const uint8_t *)p)[b>>3] & (1u<<(b&7)));