WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Kouya Shimura <kouya@xxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] hvm bios: add PMM (a memory manager during POST)
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
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <7kr62v8ui1.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acl8he527MkWIbiY8E2vFUWhOl0eWQ==
Thread-topic: [Xen-devel] [PATCH] hvm bios: add PMM (a memory manager during POST)
User-agent: Microsoft-Entourage/12.14.0.081024
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