[Xen-changelog] [xen master] hvmloader: Correct bug in low mmio region accounting

[patchbot@xxxxxxx]

commit 703e150231e26079d72ecca76701a825e7b9b927
Author:     George Dunlap <george.dunlap@xxxxxxxxxxxxx>
AuthorDate: Tue Jun 18 15:11:03 2013 +0100
Commit:     Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CommitDate: Mon Jun 24 12:20:29 2013 +0100

    hvmloader: Correct bug in low mmio region accounting
    
    When deciding whether to map a device in low MMIO space (<4GiB),
    hvmloader compares it with "mmio_left", which is set to the size of
    the low MMIO range (pci_mem_end - pci_mem_start).  However, even if it
    does map a device in high MMIO space, it still removes the size of its
    BAR from mmio_left.
    
    In reality we don't need to do a separate accounting of the low memory
    available -- this can be calculated from mem_resource.  Just get rid
    of the variable and the duplicate accounting entirely.  This will make
    the code more robust.
    
    Note also that the calculation of whether to move a device to 64-bit
    is fragile at the moment, depending on some unstated assumptions.
    State those assumptions in a comment for future reference.
    
    v5:
     - Add comment documenting fragility of the move-to-highmem check
    v3:
     - Use mem_resource values directly instead of doing duplicate
       accounting
    
    Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
    Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
    CC: Ian Campbell <ian.campbell@xxxxxxxxxx>
    CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
    CC: Hanweidong <hanweidong@xxxxxxxxxx>
    CC: Keir Fraser <keir@xxxxxxx>
---
 tools/firmware/hvmloader/pci.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
index 6792ed4..1fe250d 100644
--- a/tools/firmware/hvmloader/pci.c
+++ b/tools/firmware/hvmloader/pci.c
@@ -42,7 +42,6 @@ void pci_setup(void)
     uint32_t vga_devfn = 256;
     uint16_t class, vendor_id, device_id;
     unsigned int bar, pin, link, isa_irq;
-    int64_t mmio_left;
 
     /* Resources assignable to PCI devices via BARs. */
     struct resource {
@@ -264,8 +263,6 @@ void pci_setup(void)
     io_resource.base = 0xc000;
     io_resource.max = 0x10000;
 
-    mmio_left = pci_mem_end - pci_mem_start;
-
     /* Assign iomem and ioport resources in descending order of size. */
     for ( i = 0; i < nr_bars; i++ )
     {
@@ -273,7 +270,21 @@ void pci_setup(void)
         bar_reg = bars[i].bar_reg;
         bar_sz  = bars[i].bar_sz;
 
-        using_64bar = bars[i].is_64bar && bar64_relocate && (mmio_left < 
bar_sz);
+        /*
+         * NB: The code here is rather fragile, as the check here to see
+         * whether bar_sz will fit in the low MMIO region doesn't match the
+         * real check made below, which involves aligning the base offset of 
the
+         * bar with the size of the bar itself.  As it happens, this will 
always
+         * be satisfied because:
+         * - The first one will succeed because the MMIO hole can only start at
+         *   0x{f,e,c,8}00000000.  If it fits, it will be aligned properly.
+         * - All subsequent ones will be aligned because the list is ordered
+         *   large to small, and bar_sz is always a power of 2. (At least
+         *   the code here assumes it to be.)
+         * Should either of those two conditions change, this code will break.
+         */
+        using_64bar = bars[i].is_64bar && bar64_relocate
+            && (bar_sz > (mem_resource.max - mem_resource.base));
         bar_data = pci_readl(devfn, bar_reg);
 
         if ( (bar_data & PCI_BASE_ADDRESS_SPACE) ==
@@ -295,7 +306,6 @@ void pci_setup(void)
                 resource = &mem_resource;
                 bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
             }
-            mmio_left -= bar_sz;
         }
         else
         {
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog