From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=48453 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Plpys-0004Dc-Dl for qemu-devel@nongnu.org; Sat, 05 Feb 2011 16:47:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Plpyf-0002yB-2b for qemu-devel@nongnu.org; Sat, 05 Feb 2011 16:47:34 -0500 Received: from moutng.kundenserver.de ([212.227.126.171]:63480) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Plpye-0002xM-M2 for qemu-devel@nongnu.org; Sat, 05 Feb 2011 16:47:33 -0500 Message-ID: <4D4DC570.8080204@mail.berlios.de> Date: Sat, 05 Feb 2011 22:47:28 +0100 From: Stefan Weil MIME-Version: 1.0 References: <4D4D612F.2010904@mail.berlios.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *)) List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: QEMU Developers Am 05.02.2011 16:35, schrieb Blue Swirl: > On Sat, Feb 5, 2011 at 2:39 PM, Stefan Weil wrote: >> Currently, most QEMU code assumes that pointers and long integers have >> the same size, typically 32 bit on 32 bit hosts, 64 bit on 64 bit hosts. >> >> While this assumption works on QEMU's major hosts, it is not >> generally true. >> >> There exist 64 bit host OS which use an ABI with 32 bit long integers, >> maybe to be more compatible with an older 32 bit OS version, so here is >> sizeof(long) < sizeof(void *). > > Oh, that OS-Which-Must-Not-Be-Named. Right. > >> Other ABIs might use "near" pointers which may reduce code size and >> improve >> code speed. This results in sizeof(long) > sizeof(void *). >> >> Both cases will break current QEMU, because lots of code lines use >> type casts from pointer to long or vice versa like these two examples: >> >> start = (long)mmap((void *)host_start, host_len ... >> code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + ...)) >> >> Both variants (unsigned long) and (long) can be found (relation 3:2). >> >> Changing the existing limitation of QEMU's code simply needs replacing >> all those type casts, variable declarations and printf format specifiers >> by portable code. >> >> The standard integral type which matches the size of a pointer is defined >> in stdint.h (which also defines int8_t, ...). It is intptr_t (signed >> version) >> or uintptr_t (unsigned version). There is no need to use both. >> >> => Replace (unsigned long), (long) type casts of pointers by (uintptr_t). >> >> All variables (auto, struct members, parameters) which hold such values >> must be fixed, too. In the following examples, ptr_var is of that kind. >> >> => Replace unsigned long ptr_var, long ptr_var by uintptr_t ptr_var. >> >> Finally, the fixed variables are used in printf-like statements, >> so here the format specifier needs a change. inttypes.h defines >> PRIxPTR which should be used. >> >> => Replace "%lx" by "%" PRIxPTR to print the integer value ptr_var. >> >> A single patch which includes all these changes touches 39 files. >> Splitting it into smaller patches is not trivial because of cross >> dependencies. Because of its size, it will raise merge conflicts >> when it is not applied soon. >> >> Would these kind of changes be interesting for QEMU? > > Does QEMU work in OS-Which-Must-Not-Be-Named when the patch is applied? Up to now, I never used OS-Which-Must-Not-Be-Named (64 bit) :-) The patch allows to build executables (which I have put on my web page). I noticed two remaining problems with time_t and GetProcessAffinityMask. Maybe these are not critical, but of course they have to be fixed, too. >> Are there suggestions how it should be done? > > The change, done properly, should not cause any problems for most > other hosts, where unsigned long size equals pointer type size. That's correct. The risk of breaking current hosts is minimal. >> What about ram_addr_t? Should it be replaced by uintptr_t? > > I'd use > typedef uintptr_t ram_addr_t; So did I. We could go further and eliminate ram_addr_t. >> Should we use macros like QEMU_PTR2UINT(p), QEMU_UINT2PTR(u)? > > Rather, inline functions if open coding is not clear. > >> My current version of the patch is available from >> http://qemu.weilnetz.de/0001-Fix-conversions-from-pointer-to-integral-type-and-vi.patch. > > Some comments: > > The patch changes also signed longs to uintptr_t. That could introduce > regressions, so please use signed/unsigned as original. I changed the code manually, and there was only one location where signed/unsigned made a difference. That single case was an int parameter passed in a void pointer, and I used intptr_t there. I had the impression that in the current code (long) was sometimes used because it is shorter than (unsigned long) :-) As long as changes are made manually with the necessary care, I'd recommend using uintptr_t where possible. > > For example in cpu_physical_memory_reset_dirty() you didn't change > length, but that should probably become uintptr_t too. Yes, that would be better (even if dirty memory ranges > 4 GiB are not common). I'll change that. > > */translate.c: exit_tb() helper uses tcg_target_long, so the cast > should use that instead. Obviously tcg_target_long has the same size as uintptr_t, so I can change this, too. Regards, Stefan