Re: [Xen-merge] First 6 patches

[Chris Wright <chrisw@xxxxxxxx>]

* Martin J. Bligh (mbligh@xxxxxxxxxx) wrote:
> OK, this is where I got to today, between meetings ;-). I'm fairly sure 
> none of it works properly yet, not even test compiled, but I'll polish 
> it up tommorow. This is the basic idea I'm on though, so if people 
> disagree, please yell ASAP (I'm going to bed for now though ;-))
> 
> Chris, don't bother trying to stack on top of this until I've at least
> got it compile tested and booted on non-xen. These are against 2.6.12
> and should form the bottom of the stack ...

> Pointing out my stupid mistakes is welcome, but this is more to garner
> feedback on the approach.
> 
> http://mbligh.org/xen/001-xen-cpu_common.c

I still think this one needs further refinement.  Most of that stuff can
be factored out.

> http://mbligh.org/xen/002-xen-i386_ksyms.c

This one turns out to be moot, just make it unconditionally conditional ;-)
(it's already moved upstream to a location that we'll override anyway).

> http://mbligh.org/xen/003-ioport.c

When I had looked at this one a while back, I preferred to do it with
some generic helpers to keep from splitting it.  That was only for one
reason...it's quite security sensitive area (albeit rarely changing)
and had wanted to keep it in a single file as much as possible.

> http://mbligh.org/xen/004-ldt.c

This one is nice.

> http://mbligh.org/xen/005-pci-dma.c

Couple things here.  Typo (inlinee).  Header should be ASM_MACH...
And you can't add dma_map_$foo here like that.  They're already in
header files, and in fact I already did this one in the header cleanup.
The normal bits are now in mach-default/mach_dma_mapping.h as static inlines
as they already were.  And the xen bits are non-inlined (declared only)
in mach-xen/mach_dma_mapping.h.  And I don't think xen_contig_memory
should be in global namespace.  Finally, sidenote (a latent bug, not your
fault), dma_map_single calls kmalloc(GFP_KERNEL) but can be called from
contexts where it can't sleep.

> http://mbligh.org/xen/006-signal.c

+#ifdef CONFIG_XEN
+       if ((regs->xcs & 2) != 2)
+#else
        if ((regs->xcs & 3) != 3)
+#endif

That change is all over the place.  That should be a macro value that's
defined by the arch.  The only reason I didn't do that in the other
patch I submitted is I couldn't think of a good name.  KERNEL_CS_PL?
Something...at any rate, change should only be in header.  Also, I
missed why the breakout of sigframe.h?  They are identical?  It's an
unncessary change.

> I looked at io_apic.c and ... well, I'll need to go back when I'm
> feeling braver ;-) Will knock through a bunch more tommorow, I hope.

Here's to your bravery ;-)

thanks,
-chris

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