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

Re: [Xen-devel] [PATCH] hvm bios: add PMM (a memory manager during POST)


  • To: Kouya Shimura <kouya@xxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
  • Date: Thu, 22 Jan 2009 11:38:23 +0000
  • Cc:
  • Delivery-date: Thu, 22 Jan 2009 03:38:54 -0800
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: Acl8he527MkWIbiY8E2vFUWhOl0eWQ==
  • Thread-topic: [Xen-devel] [PATCH] hvm bios: add PMM (a memory manager during POST)

On 22/01/2009 11:12, "Kouya Shimura" <kouya@xxxxxxxxxxxxxx> wrote:

> The PMM (POST Memory Manager) offers malloc/free functionality
> for PCI option ROMs during POST (Power On Self Test).
> 
> This patch adds a PMM functionality to the guest BIOS.

Hardcoding the entry point and setting up $PMM in hvmloader is not very
nice. I think you should define the $PMM struct as data in rombios.c, and
then you can directly reference pmm_entry_point without needing a .org
directive. The only difficulty then is the checksum, which I think you
should calculate in rombios.c rather than doing it in hvmloader. It's a tiny
bit of C or pretty trivial asm to do it that way. Overall this avoids
further tying together hvmloader and rombios unnecessarily.

Beyond that, the allocator looks pretty complicated, but as long as it works
I suppose that is fine. It would be nice if you could add a top-of-file
comment explaining the allocator algorithm, where it allocates memory from
(scavenging from rombios's e820 perhaps), and other details like that.
Otherwise it's rather unnecessarily opaque new code. A few more comments
scattered through (especially at tops of functions) would be nice too!

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

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