qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Stefan Weil <weil@mail.berlios.de>
Cc: Blue Swirl <blauwirbel@gmail.com>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
Date: Fri, 11 Feb 2011 09:24:40 +0100	[thread overview]
Message-ID: <4D54F248.2020501@codemonkey.ws> (raw)
In-Reply-To: <4D4D612F.2010904@mail.berlios.de>

On 02/05/2011 03: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 *).
>
> 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?

Portability is not a binary concept.  We could constantly change the 
code base to work around some silly decision that a random platform 
makes (like negative errno values....).

So on their own, these changes are not useful and I'm not in favor of 
it.  However, if someone was going to do proper w64 support that was 
maintained, tested, and enhanced over the next few years, it'd be 
something I'd be very supportive of.

A, "hey, look what I did last weekend" port is not terribly interesting 
to me.  It'll just bitrot like the current w32 is.

Regards,

Anthony Liguori

> Are there suggestions how it should be done?
> What about ram_addr_t? Should it be replaced by uintptr_t?
> Should we use macros like QEMU_PTR2UINT(p), QEMU_UINT2PTR(u)?
>
> My current version of the patch is available from
> http://qemu.weilnetz.de/0001-Fix-conversions-from-pointer-to-integral-type-and-vi.patch. 
>
>
> Kind regards,
> Stefan Weil
>
>

      parent reply	other threads:[~2011-02-11  8:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-05 14:39 [Qemu-devel] Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *)) Stefan Weil
2011-02-05 15:35 ` [Qemu-devel] " Blue Swirl
2011-02-05 21:47   ` Stefan Weil
2011-02-07  7:23     ` Markus Armbruster
2011-02-07 17:51       ` Stefan Weil
2011-02-07 18:16         ` Markus Armbruster
2011-02-05 16:03 ` [Qemu-devel] " Stefan Hajnoczi
2011-02-05 17:40   ` Stefan Weil
2011-02-11  5:05 ` Rob Landley
2011-02-11 12:47   ` [Qemu-devel] " Paolo Bonzini
2011-02-11 13:04     ` Tristan Gingold
2011-02-11 18:50     ` Blue Swirl
2011-02-11 19:28       ` malc
2011-02-11  8:24 ` Anthony Liguori [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D54F248.2020501@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=blauwirbel@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=weil@mail.berlios.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).