[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC][PATCH 12/13] hvmloader/pci: skip reserved ranges



On 2015/5/15 14:19, Jan Beulich wrote:
On 15.05.15 at 05:18, <tiejun.chen@xxxxxxxxx> wrote:
On 2015/4/20 22:21, Jan Beulich wrote:
On 10.04.15 at 11:22, <tiejun.chen@xxxxxxxxx> wrote:
--- a/tools/firmware/hvmloader/pci.c
+++ b/tools/firmware/hvmloader/pci.c
@@ -59,8 +59,8 @@ void pci_setup(void)
           uint32_t bar_reg;
           uint64_t bar_sz;
       } *bars = (struct bars *)scratch_start;
-    unsigned int i, nr_bars = 0;
-    uint64_t mmio_hole_size = 0;
+    unsigned int i, j, nr_bars = 0;
+    uint64_t mmio_hole_size = 0, reserved_end;

       const char *s;
       /*
@@ -393,8 +393,23 @@ void pci_setup(void)
           }

           base = (resource->base  + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
+ reallocate_mmio:
           bar_data |= (uint32_t)base;
           bar_data_upper = (uint32_t)(base >> 32);
+        for ( j = 0; j < memory_map.nr_map ; j++ )
+        {
+            if ( memory_map.map[j].type != E820_RAM )
+            {
+                reserved_end = memory_map.map[j].addr +
memory_map.map[j].size;
+                if ( check_hole_conflict(base, bar_sz,
+                                         memory_map.map[j].addr,
+                                         memory_map.map[j].size) )
+                {
+                    base = (reserved_end  + bar_sz - 1) & ~(uint64_t)(bar_sz -
1);
+                    goto reallocate_mmio;
+                }
+            }
+        }
           base += bar_sz;

           if ( (base < resource->base) || (base > resource->max) )


Actually some original codes are missing here,

          if ( (base < resource->base) || (base > resource->max) )
          {
              printf("pci dev %02x:%x bar %02x size "PRIllx": no space for "
                     "resource!\n", devfn>>3, devfn&7, bar_reg,
                     PRIllx_arg(bar_sz));
              continue;
          }

I think this can guarantee the MMIO regions just fit in the available RAM.

Or am I wrong?

The code you cite guarantees almost nothing, it simply skips assigning
resources. Your changes potentially growing the space needed to fit
all MMIO BARs therefore also needs to adjust the up front calculation,
such that if necessary more RAM can be relocated to make the hole
large enough.


You're right.

Just think about we're always trying to check pci_mem_start to populate more RAM to obtain enough PCI mempry,

    /* Relocate RAM that overlaps PCI space (in 64k-page chunks). */
    while ( (pci_mem_start >> PAGE_SHIFT) < hvm_info->low_mem_pgend )
    {
        struct xen_add_to_physmap xatp;
        unsigned int nr_pages = min_t(
            unsigned int,
            hvm_info->low_mem_pgend - (pci_mem_start >> PAGE_SHIFT),
            (1u << 16) - 1);
        if ( hvm_info->high_mem_pgend == 0 )
            hvm_info->high_mem_pgend = 1ull << (32 - PAGE_SHIFT);
        hvm_info->low_mem_pgend -= nr_pages;
        printf("Relocating 0x%x pages from "PRIllx" to "PRIllx\
               " for lowmem MMIO hole\n",
               nr_pages,
               PRIllx_arg(((uint64_t)hvm_info->low_mem_pgend)<<PAGE_SHIFT),

PRIllx_arg(((uint64_t)hvm_info->high_mem_pgend)<<PAGE_SHIFT));
        xatp.domid = DOMID_SELF;
        xatp.space = XENMAPSPACE_gmfn_range;
        xatp.idx   = hvm_info->low_mem_pgend;
        xatp.gpfn  = hvm_info->high_mem_pgend;
        xatp.size  = nr_pages;
        if ( hypercall_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
            BUG();
        hvm_info->high_mem_pgend += nr_pages;
    }


So I think we may need to adjust pci_mem_start like this,

@@ -301,6 +301,19 @@ void pci_setup(void)
             pci_mem_start <<= 1;
     }

+    /* Relocate PCI memory that overlaps reserved space, like RDM. */
+    for ( j = 0; j < memory_map.nr_map ; j++ )
+    {
+        if ( memory_map.map[j].type != E820_RAM )
+        {
+            reserved_end = memory_map.map[j].addr + memory_map.map[j].size;
+            if ( check_overlap(pci_mem_start, pci_mem_end,
+                               memory_map.map[j].addr,
+                               memory_map.map[j].size) )
+                pci_mem_start -= memory_map.map[j].size >> PAGE_SHIFT;
+        }
+    }
+
     if ( mmio_total > (pci_mem_end - pci_mem_start) )
     {
         printf("Low MMIO hole not large enough for all devices,"

Right?

Thanks
Tiejun


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.