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] pagetable cleanups

To: Michael A Fetterman <Michael.Fetterman@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] [patch] pagetable cleanups
From: Gerd Knorr <kraxel@xxxxxxxxxxx>
Date: Thu, 14 Apr 2005 17:01:50 +0200
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 14 Apr 2005 15:05:02 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <E1DM3Pa-0002e4-00@xxxxxxxxxxxxxxxxx>
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/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20050412185856.GA5832@bytesex> <E1DM3Pa-0002e4-00@xxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.9i
On Thu, Apr 14, 2005 at 01:25:19PM +0100, Michael A Fetterman wrote:
> Overall, I think the patch looks pretty good...
> 
> A couple of comments:
> 
> 1) There's no Signed-Off-By comment attached to this.  Could you please
> provide one?

Yes, thats easy ;)

> 2) About your question at the bottom of construct_dom0: The current
> code there is intended to allow booting of dom0's with translate mode
> enabled.  As such, it probably won't stay in the code base forever,
> but it was and is a useful hack.

Ah, ok.

> I'd like to just remove the halt you added there, OK?

Fine with me, I've added your changes instead.

> 3) HL2 tables are not tables of L2 entries.  They contain L1 entries.
> They are essentially shadows of guest L2 pages, which will be used by
> Xen to get a linear-pagetable-like mapping of the guest's L1 pages.

Ah, *thats* the point of these beasts.  The page manipulations done on
them look like l2 operations (well, they actually are as they really
point to l1 pages), that confused me ;)

> 4) There was probably a bunch of debate about this somewhere before,
> but I missed it.  The macros which set/clear page table types don't
> obey C's pass-by-value calling semantics.  That means that they can't
> be replaced with simple functions, if desired -- there would always
> have to be a macro layer.

Yep, I noticed that as well as the PAE versions became a bit more
complex and I tried to make them inline functions instead which didn't
work ...

I can change them to pass-by-reference instead, it's probably a good
idea.  Hope gcc is clever enougth to see that it's the same after all
and doesn't generate extra code then.

> There's also no macros for creating L1E or L2E as expressions -- only
> statements which assign them.  Perhaps this was intentional?  It means
> that you end up declaring extra variables to hold essentially
> temporary values in a few places...

Yes, was intentionally.  I think that isn't bad, it makes the code more
readable.  And I think it actually is impossible to return structs in C,
you can only return a pointer to a struct, which would't help for the
"building entries as expressions" case.

> 5) I found a couple compilation problems when by compiling with debug=y...

Merged, thanks.

Current patch set is at http://dl.bytesex.org/patches/xen-2/ now
(issue #4 isn't adressed yet).

  Gerd


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