| |
|
 |
|
 |
|
|
|
| |
|
|
xen-merge
Re: [Xen-merge] First 6 patches
* Martin J. Bligh (mbligh@xxxxxxxxxx) wrote:
> >> 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.
I think it's leftover from the fact that the symbol is undefined if
it's not dom0. May have broken compilation w/out in domU kernels (just
a guess).
> >> 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)?
It's for distinguishing kernelspace from userspace. And as Vincent
reminds us, it's already been handled upstream, with user_mode() macro.
> 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.
I figured as much. I don't think it's worth it. In fact, folks will
argue that local only headers should stay in local directories. Anyway,
let's drop that bit.
thanks,
-chris
_______________________________________________
Xen-merge mailing list
Xen-merge@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-merge
|
|
|
| |
|
Home