From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH v2] x86: debugging code for testing 16Tb support on smaller memory systems Date: Thu, 24 Jan 2013 12:36:37 +0000 Message-ID: <20130124123637.GE18850@ocelot.phlegethon.org> References: <50FE7BF502000078000B82F8@nat28.tlf.novell.com> <50FE7EFD02000078000B8359@nat28.tlf.novell.com> <5100012402000078000B8AB9@nat28.tlf.novell.com> <20130124113657.GC18850@ocelot.phlegethon.org> <510135DD02000078000B91E3@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <510135DD02000078000B91E3@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel List-Id: xen-devel@lists.xenproject.org At 12:23 +0000 on 24 Jan (1359030221), Jan Beulich wrote: > >>> On 24.01.13 at 12:36, Tim Deegan wrote: > > At 14:26 +0000 on 23 Jan (1358951188), Jan Beulich wrote: > >> --- a/xen/arch/x86/setup.c > >> +++ b/xen/arch/x86/setup.c > >> @@ -82,6 +82,11 @@ boolean_param("noapic", skip_ioapic_setu > >> s8 __read_mostly xen_cpuidle = -1; > >> boolean_param("cpuidle", xen_cpuidle); > >> > >> +#ifndef NDEBUG > >> +unsigned long __initdata highmem_start; > >> +size_param("highmem-start", highmem_start); > >> +#endif > >> + > >> cpumask_t __read_mostly cpu_present_map; > >> > >> unsigned long __read_mostly xen_phys_start; > >> @@ -787,6 +792,14 @@ void __init __start_xen(unsigned long mb > >> modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end); > >> bootstrap_map(NULL); > >> > >> +#ifndef highmem_start > >> + /* Don't allow split below 4Gb. */ > >> + if ( highmem_start < GB(4) ) > >> + highmem_start = 0; > >> + else /* align to L3 entry boundary */ > >> + highmem_start &= ~((1UL << L3_PAGETABLE_SHIFT) - 1); > >> +#endif > > > > DYM #ifndef NDEBUG ? I can see that checking for highmem_start being a > > macro is strictly correct > > I intended it to be that way, because there could be other uses > for having the symbol #define-d/real. Yes - but if it ever ends up being a #define _and_ user-settable, these checks will silently disappear. Since there's no indication in the places where you might make it a #define that doing so will remove these checks, I'd be inclined to leave it gated on NDEBUG so it's fail in an obvious way. Or add a #define CONFIG_HIGHMEM_START (default to == !NDEBUG), and gate everything on that? Tim.