From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E789FC04A95 for ; Sat, 22 Oct 2022 19:38:41 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 27C7884E4D; Sat, 22 Oct 2022 21:38:39 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.b="QGn9FJoG"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="zBCZfAjD"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 574B384D30; Sat, 22 Oct 2022 21:38:37 +0200 (CEST) Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 3EBCA84D30 for ; Sat, 22 Oct 2022 21:38:34 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=msuchanek@suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id EFF2E22C38; Sat, 22 Oct 2022 19:38:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1666467513; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6dg3O3s7cpRq2JDCAWteBW0rhUyOXRihfqp9n226kbM=; b=QGn9FJoGQUZkJaLlt05GUtYm41LEFWnymNHOdLNinofeqIvJVy890cp0XkEvIhNVh4xR04 mfWWOpu+7kCydxUdAhWaEGyl5+64P6+XnqU3pJnBvOUGbGaLniWDvQzRPxAzzaArX+n6tk s8vmHldkcQLcyWtSguXgqr19AEeO8Aw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1666467513; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6dg3O3s7cpRq2JDCAWteBW0rhUyOXRihfqp9n226kbM=; b=zBCZfAjDwhwQm6BGsyNBC7A70BLONEC5wG4BygSHFMgBgGShV8ws5tEfDXlAla6R1fgmCU JrdIZMGMkPwXlZCw== Received: from kitsune.suse.cz (kitsune.suse.cz [10.100.12.127]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id D66212C141; Sat, 22 Oct 2022 19:38:33 +0000 (UTC) Date: Sat, 22 Oct 2022 21:38:32 +0200 From: Michal =?iso-8859-1?Q?Such=E1nek?= To: Simon Glass Cc: Heinrich Schuchardt , u-boot@lists.denx.de Subject: Re: [PATCH v2] tests: Build correct sandbox configuration on 32bit Message-ID: <20221022193832.GY28810@kitsune.suse.cz> References: <1774657c-51cb-fde7-7f65-a76b8cdcd5f9@gmx.de> <3558da2e-3e66-0b66-cf77-29106653beec@gmx.de> <20221017072835.GQ28810@kitsune.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Fri, Oct 21, 2022 at 06:05:51PM -0700, Simon Glass wrote: > Hi, > > On Mon, 17 Oct 2022 at 01:28, Michal Suchánek wrote: > > > > On Sat, Oct 15, 2022 at 10:27:43PM +0200, Heinrich Schuchardt wrote: > > > On 10/15/22 21:46, Simon Glass wrote: > > > > Hi Heinrich, > > > > > > > > On Sat, 15 Oct 2022 at 13:29, Heinrich Schuchardt wrote: > > > > > > > > > > > > > > > > > > > > Am 15. Oktober 2022 21:24:36 MESZ schrieb Simon Glass : > > > > > > Hi Heinrich, > > > > > > > > > > > > On Sat, 15 Oct 2022 at 13:05, Heinrich Schuchardt wrote: > > > > > > > > > > > > > > On 10/15/22 20:39, Simon Glass wrote: > > > > > > > > Hi Heinrich, > > > > > > > > > > > > > > > > On Sat, 15 Oct 2022 at 12:31, Heinrich Schuchardt wrote: > > > > > > > > > > > > > > > > > > On 10/15/22 19:53, Simon Glass wrote: > > > > > > > > > > Hi Michal, > > > > > > > > > > > > > > > > > > > > On Fri, 14 Oct 2022 at 14:53, Michal Suchanek wrote: > > > > > > > > > > > > > > > > > > > > > > Currently sandbox configuration defautls to 64bit and there is no > > > > > > > > > > > automation for building 32bit sandbox on 32bit hosts. > > > > > > > > > > > > > > > > > > > > > > Use _LP64 macro as heuristic for detecting 64bit targets. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Michal Suchanek > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > > simplify and move detection to kconfig > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > arch/sandbox/Kconfig | 18 +++--------------- > > > > > > > > > > > scripts/Kconfig.include | 4 ++++ > > > > > > > > > > > 2 files changed, 7 insertions(+), 15 deletions(-) > > > > > > > > > > > > > > > > > > > > Reviewed-by: Simon Glass > > > > > > > > > > > > > > > > > > > > My only question is whether we can allow building the 32-bit version > > > > > > > > > > on a 64-bit machine? That would need a separate option I think, to > > > > > > > > > > say: > > > > > > > > > > > > > > > > > > > > I don't want you to automatically determine HOST_32/64BIT. Instead, > > > > > > > > > > use 32 (or 64). > > > > > > > > > > > > > > > > > > > > This is along the lines of what Heinrich is saying, except that I > > > > > > > > > > strongly feel that we must do the right thing by default, as your > > > > > > > > > > patch does. > > > > > > > > > > > > > > > > > > The whole point of phys_addr_t and phys_size_t is that it can be 64bit > > > > > > > > > or 32bit on ilp32. > > > > > > > > > > > > > > > > > > With this patch we cannot build with CONFIG_PHYS_64BIT=y on ilp32 and > > > > > > > > > that is bad. > > > > > > > > > > > > > > > > > > 32 bit phys_addr_t on lp64 is irrelevant for actual hardware but this is > > > > > > > > > what we currently test with sandbox_defconfig on Gitlab CI. > > > > > > > > > > > > > > > > > > My patch is ending up in the same behavior as Michal's patch except that > > > > > > > > > it allows to have 64bit phys_addr_t on ilp32. > > > > > > > > > > > > > > > > It needs to automatically default to 32 or 64 bit depending on the > > > > > > > > host. If the user wants to fiddle with Kconfig to force it to the > > > > > > > > other one, that should be possible to. > > > > > > > > > > > > > > > > It looks like your patch requires manual configuration, but perhaps I > > > > > > > > just misunderstood it? > > > > > > > > > > > > > > __LP64__ is a symbol defined by the compiler when compiling for 64bit > > > > > > > and not defined when compiling for 32bit systems. There is nothing > > > > > > > manual about it. > > > > > > > > > > > > > > My patch uses this symbol to replace HOST_32BIT and HOST_64BIT. > > > > > > > > > > > > > > Michal's patch compiles a program tools/bits-per-long.c that ends up > > > > > > > returning 64 on 64 bit systems (where __LP64__ is defined) and 32 on 32 > > > > > > > bit systems (where __LP64__ is not defined) and then chooses HOST_32BIT > > > > > > > and HOST_64BIT accordingly. This part of Michal's patch is not wrong. > > > > > > > The solution is only overly complicated. > > > > > > > > > > > > > > What has to be chosen manually with both patches is PHYS_64BIT e.g. by > > > > > > > selecting sandbox64_defconfig instead of sandbox_defconfig. > > > > > > > > > > > > > > Unfortunately Michal did not understand that PHYS_64BIT=y, HOST_32BIT=y > > > > > > > is a necessary test scenario and introduced an invalid dependency. > > > > > > > > > > > > > > With my patch sandbox64_defconfig on a 32bit system uses 64bit phys_addr_t. > > > > > > > > > > > > > > With Michal's patch sandbox64_defconfig on a 32bit system uses 32bit > > > > > > > phys_addr_t. > > > > > > > > > > > > That's all great, thank you, but please can you address my actual question? > > > > > > > > > > Your question in this thread was if my patch requires extra manual configuration compared to Michal's patch and the answer was no. > > > > > > > > > > What other question do you have? > > > > > > > > "It needs to automatically default to 32 or 64 bit depending on the > > > > host. If the user wants to fiddle with Kconfig to force it to the > > > > other one, that should be possible to." > > > > > > The user forces 32bit or 64bit by selecting a 32bit or 64bit compiler > > > not with Kconfig. PHYS_64BIT is the only thing that needs to be selected > > > via Kconfig. > > > > > > > > > > > I am not talking about anyone's patch, actually, just trying to state > > > > what I think should happen. > > > > > > The physical systems that U-Boot has to deal with are: > > > > > > * 32bit without physical address extension (PAE) > > > Here phys_addr_t must be 32 bit. > > > * 32bit with physical address extension. > > > Here phys_addr_t must be 64 bit. > > > * 64bit systems without PAE. > > > Here phys_addr_t must be 64 bit. > > > > > > We want to model these three scenarios with the sandbox. > > > > > > So we have to build: > > > > > > * Sandbox with PHYS_64BIT=n using a 32bit compiler. > > > * Sandbox with PHYS_64BIT=y using a 32bit compiler. > > > * Sandbox with PHYS_64BIT=y using a 64bit compiler. > > > > To get these three and not the fourth a kconfig option would still be > > needed, right? > > My concern is that 1 and 3 are built automatically by default, > depending on what bitness you are using (or compiler, that's fine too > as it might be equivalent). > > So NO Kconfig change to get that behaviour. My request for a Kconfig > to *manually* select which to use is not as important as the above, so > let's ignore it. > > For #2 that would need to do a config change as it isn't worth > creating a new sandbox build just for that. We could do it in CI with > 'buildman -a PHYS_64BIT=y' Then you may want the alternative patch instead: [PATCH] sandbox: Eliminate CONFIG_HOST_32/64BIT https://lists.denx.de/pipermail/u-boot/2022-October/497236.html hm, on a second thought you probably don't unless it's cleaned up to use __LP64__ only once to define the sandbox bits. Thanks Michal > > Regards, > Simon > > Applied to u-boot-dm, thanks!