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-merge

Re: [Xen-merge] First 6 patches

To: Chris Wright <chrisw@xxxxxxxx>
Subject: Re: [Xen-merge] First 6 patches
From: "Martin J. Bligh" <mbligh@xxxxxxxxxx>
Date: Fri, 05 Aug 2005 07:06:47 -0700
Cc: xen-merge <xen-merge@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 05 Aug 2005 14:04:58 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20050805065814.GK7762@xxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-merge-request@lists.xensource.com?subject=help>
List-id: xen-merge <xen-merge.lists.xensource.com>
List-post: <mailto:xen-merge@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-merge>, <mailto:xen-merge-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-merge>, <mailto:xen-merge-request@lists.xensource.com?subject=unsubscribe>
References: <142260000.1123222102@[10.10.2.4]> <20050805065814.GK7762@xxxxxxxxxxxxxxxxxxx>
Reply-to: "Martin J. Bligh" <mbligh@xxxxxxxxxx>
Sender: xen-merge-bounces@xxxxxxxxxxxxxxxxxxx
>> 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

<Prev in Thread] Current Thread [Next in Thread>