[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [Xen-devel] [patch] pagetable cleanups
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? 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. The bringup process for this new shadow code was to get dom0's working first, and we're now working on (cleaner) domU support. I'm not too worried about x86_64 or pae support for this dom0 translate mode support. I'd like to just remove the halt you added there, OK? 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. A normal guest L2 has guest-physical pages in it. A normal shadow L2 points at shadows of the guest's L1s. An HL2 has machine addresses of the guest's L1 pages in it, and is *used* as an L1 table by Xen. So the things like "l1_pgentry_t *hl2_vtable" in domain.h were not typos, and should stay the way they were... The HL2 is a concept that works well for the simple 2 level page tables, and is a clever solution to avoid doing lots of extra map_domain_mem() calls to access the guest's L1 tables, but it both falls apart and is fortunately unnecessary for 64-bit mode. I haven't thought much about PAE yet, but HL2 are probably still useful there because of the cost of map_domain_mem()... 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. 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... Comments? 5) I found a couple compilation problems when by compiling with debug=y... I've dealt with issues #2, 3, and 5 in a slightly modified version of your patch, available at http://www.cl.cam.ac.uk/~maf46/kraxel.patch.v2 Take a look, and let me know what you think. Michael -----Original Message----- From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Gerd Knorr Sent: Tuesday, April 12, 2005 7:59 PM To: xen-devel@xxxxxxxxxxxxxxxxxxx Subject: [Xen-devel] [patch] pagetable cleanups Hi, Next version of the pagetable cleanup patch. Builds and boots domain 0 on x86_32. Changes from the last version: * macro names are changed. * adapted to the new shadow code checked in last week. * new macro: l1e_has_changed() to compare page table entries. Open issues: * I'm not sure how to handle the debug printk's best. These use the l1e_get_value() macro at the moment to get the raw bits and print them as unsigned long hex value. I'd like to get rid of the l1e_get_value() macro altogether though ... * x86_64 build needs fixing, will look into this tomorrow. Enjoy, Gerd _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |