|
|
|
|
|
|
|
|
|
|
xen-merge
Re: [Xen-merge] First 6 patches
>> 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.
Agreed, but it's a question of how much we have to do before merge.
I'll come back to it at the end of the cycle if need be.
>> 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).
Don't know enough about why that was done to comment. I can see
people w/o ACPI using APM or something, but it's not my area. If you're
sure, we can just do that, but need a justification for it somehow.
>> 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.
That's going to make a huge mess - there seemed to be a lot of changes
in that file - only set_bitmap was left unscathed ...
>> 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.
OK, I'll revisit this.
>> 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.
Sure, that was a late-night quick-hack ;-) What exactly is the underlying
change here for (2 vs 3)? sigframe.h is being moved from arch/i386 to
include/asm-i386 just because having header files in the wrong place is
morally offensive. Was a generic cleanup that maybe should have been
split out.
M.
_______________________________________________
Xen-merge mailing list
Xen-merge@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-merge
|
|
|
|
|