[Xen-devel] [PATCH v2] x86/hvmloader: round up memory BAR size to 4K

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

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@xxxxxxxxx>
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
Cc: Jason Andryuk <jandryuk@xxxxxxxxx>
---
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.
---
 tools/firmware/hvmloader/pci.c  | 12 ++++++++++++
 tools/firmware/hvmloader/util.h |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
index 0b708bf578..ac4fb5aa82 100644
--- 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 7bca6418d2..e1ebfd6647 100644
--- 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)));
-- 
2.25.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel