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

Re: [Xen-devel] [PATCH v3 3/8] hvmloader: Set up highmem resouce appropriately if there is no RAM above 4G



On 21/06/13 08:01, Jan Beulich wrote:
On 20.06.13 at 18:33, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
@@ -244,7 +244,14 @@ void pci_setup(void)
          hvm_info->high_mem_pgend += nr_pages;
      }
- high_mem_resource.base = ((uint64_t)hvm_info->high_mem_pgend) << PAGE_SHIFT;

+    if ( hvm_info->high_mem_pgend )
To be on the safe side I'd make this

     if ( hvm_info->high_mem_pgend >= (1ull << 32) )

high_mem_pgend is in frames, not physical address; but I do take your point.

On the other hand, there is other code (for instance, in the memory relocation loop) that assumes that high_mem_pgend is either 0, or pointing into the high mem region. Normally the correct way to deal with internal invariants like this is to add an assert() to that effect, but adding a new assert() at this point is not really on either.

I'd just as soon leave it the way it is, but if people felt strongly about it I could change it to something like this:

  high_mem_resource.base = ((uint64_t)hvm_info->high_mem_pgend) << PAGE_SHIFT;
  if ( high_mem_resource.base < 1ull << 32 )
    high_mem_resource.base = 1ull << 32;


+        high_mem_resource.base = ((uint64_t)hvm_info->high_mem_pgend) << 
PAGE_SHIFT;
+    else
+        high_mem_resource.base = 1ull << 32;
+    printf("%sRAM in high memory; setting high_mem resource base to %x%08x\n",
+           hvm_info->high_mem_pgend?"":"No ",
+           (uint32_t)(high_mem_resource.base>>32),
+           (uint32_t)high_mem_resource.base);
Seeing the n-th incarnation of this - mind creating a macro to do the
splitting?

I'll give it a try and see how it looks.

 -George

_______________________________________________
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®.