From: Anthony Liguori <anthony@codemonkey.ws>
To: Avi Kivity <avi@redhat.com>, Blue Swirl <blauwirbel@gmail.com>,
qemu-devel@nongnu.org
Cc: Jia Liu <proljc@gmail.com>, Max Filippov <jcmvbkbc@gmail.com>,
Michael Walle <michael@walle.cc>,
Paul Brook <paul@codesourcery.com>,
"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
Date: Thu, 04 Oct 2012 08:56:55 -0500 [thread overview]
Message-ID: <874nmajq6w.fsf@codemonkey.ws> (raw)
In-Reply-To: <1349346964-4151-1-git-send-email-avi@redhat.com>
Avi Kivity <avi@redhat.com> writes:
> The hassle and compile time overhead of maintaining both 32-bit and 64-bit
> capable source isn't worth the tiny performance advantage which is seen on
> a minority of configurations. Switch to compiling libhw only once, with
> target_phys_addr_t unconditionally typedefed to uint64_t.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Regards,
Anthony Liguori
> ---
>
> v2: no changes, but copied the maintainers of architectures that will
> see their target_phys_addr_t changed as a result. Please view and/or
> test.
>
> .gitignore | 1 +
> Makefile | 2 +-
> Makefile.hw | 1 -
> Makefile.target | 3 ---
> configure | 34 ++++------------------------------
> cpu-common.h | 2 +-
> dma.h | 2 +-
> hw/hw.h | 2 +-
> hw/intel-hda.c | 8 +-------
> hw/rtl8139.c | 6 +-----
> monitor.c | 4 ----
> target-ppc/mmu_helper.c | 4 +---
> targphys.h | 19 +------------------
> 13 files changed, 13 insertions(+), 75 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 824c0d2..3ef77d0 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -12,6 +12,7 @@ trace-dtrace.dtrace
> *-linux-user
> *-bsd-user
> libdis*
> +libhw
> libhw32
> libhw64
> libuser
> diff --git a/Makefile b/Makefile
> index 0464297..1cebe3a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -214,7 +214,7 @@ $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
>
> qemu-ga$(EXESUF): qemu-ga.o $(qga-obj-y) $(tools-obj-y) $(qapi-obj-y) $(qobject-obj-y) $(version-obj-y)
>
> -QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
> +QEMULIBS=libhw libuser libdis libdis-user
>
> clean:
> # avoid old build problems by removing potentially incorrect old files
> diff --git a/Makefile.hw b/Makefile.hw
> index 59f5b48..86f0bf4 100644
> --- a/Makefile.hw
> +++ b/Makefile.hw
> @@ -2,7 +2,6 @@
>
> include ../config-host.mak
> include ../config-all-devices.mak
> -include config.mak
> include $(SRC_PATH)/rules.mak
>
> .PHONY: all
> diff --git a/Makefile.target b/Makefile.target
> index d9d54b8..4449444 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -4,9 +4,6 @@ include ../config-host.mak
> include config-devices.mak
> include config-target.mak
> include $(SRC_PATH)/rules.mak
> -ifneq ($(HWDIR),)
> -include $(HWDIR)/config.mak
> -endif
>
> $(call set-vpath, $(SRC_PATH))
> ifdef CONFIG_LINUX
> diff --git a/configure b/configure
> index 8f99b7b..65bd876 100755
> --- a/configure
> +++ b/configure
> @@ -3694,7 +3694,6 @@ TARGET_ABI_DIR=""
>
> case "$target_arch2" in
> i386)
> - target_phys_bits=64
> ;;
> x86_64)
> TARGET_BASE_ARCH=i386
> @@ -3702,7 +3701,6 @@ case "$target_arch2" in
> target_long_alignment=8
> ;;
> alpha)
> - target_phys_bits=64
> target_long_alignment=8
> target_nptl="yes"
> ;;
> @@ -3711,22 +3709,18 @@ case "$target_arch2" in
> bflt="yes"
> target_nptl="yes"
> gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
> - target_phys_bits=64
> target_llong_alignment=4
> target_libs_softmmu="$fdt_libs"
> ;;
> cris)
> target_nptl="yes"
> - target_phys_bits=32
> ;;
> lm32)
> - target_phys_bits=32
> target_libs_softmmu="$opengl_libs"
> ;;
> m68k)
> bflt="yes"
> gdb_xml_files="cf-core.xml cf-fp.xml"
> - target_phys_bits=32
> target_int_alignment=2
> target_long_alignment=2
> target_llong_alignment=2
> @@ -3735,36 +3729,30 @@ case "$target_arch2" in
> TARGET_ARCH=microblaze
> bflt="yes"
> target_nptl="yes"
> - target_phys_bits=32
> target_libs_softmmu="$fdt_libs"
> ;;
> mips|mipsel)
> TARGET_ARCH=mips
> echo "TARGET_ABI_MIPSO32=y" >> $config_target_mak
> target_nptl="yes"
> - target_phys_bits=64
> ;;
> mipsn32|mipsn32el)
> TARGET_ARCH=mipsn32
> TARGET_BASE_ARCH=mips
> echo "TARGET_ABI_MIPSN32=y" >> $config_target_mak
> - target_phys_bits=64
> ;;
> mips64|mips64el)
> TARGET_ARCH=mips64
> TARGET_BASE_ARCH=mips
> echo "TARGET_ABI_MIPSN64=y" >> $config_target_mak
> - target_phys_bits=64
> target_long_alignment=8
> ;;
> or32)
> TARGET_ARCH=openrisc
> TARGET_BASE_ARCH=openrisc
> - target_phys_bits=32
> ;;
> ppc)
> gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
> - target_phys_bits=64
> target_nptl="yes"
> target_libs_softmmu="$fdt_libs"
> ;;
> @@ -3772,7 +3760,6 @@ case "$target_arch2" in
> TARGET_BASE_ARCH=ppc
> TARGET_ABI_DIR=ppc
> gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
> - target_phys_bits=64
> target_nptl="yes"
> target_libs_softmmu="$fdt_libs"
> ;;
> @@ -3780,7 +3767,6 @@ case "$target_arch2" in
> TARGET_BASE_ARCH=ppc
> TARGET_ABI_DIR=ppc
> gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
> - target_phys_bits=64
> target_long_alignment=8
> target_libs_softmmu="$fdt_libs"
> ;;
> @@ -3790,21 +3776,17 @@ case "$target_arch2" in
> TARGET_ABI_DIR=ppc
> echo "TARGET_ABI32=y" >> $config_target_mak
> gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
> - target_phys_bits=64
> target_libs_softmmu="$fdt_libs"
> ;;
> sh4|sh4eb)
> TARGET_ARCH=sh4
> bflt="yes"
> target_nptl="yes"
> - target_phys_bits=32
> ;;
> sparc)
> - target_phys_bits=64
> ;;
> sparc64)
> TARGET_BASE_ARCH=sparc
> - target_phys_bits=64
> target_long_alignment=8
> ;;
> sparc32plus)
> @@ -3812,11 +3794,9 @@ case "$target_arch2" in
> TARGET_BASE_ARCH=sparc
> TARGET_ABI_DIR=sparc
> echo "TARGET_ABI32=y" >> $config_target_mak
> - target_phys_bits=64
> ;;
> s390x)
> target_nptl="yes"
> - target_phys_bits=64
> target_long_alignment=8
> ;;
> unicore32)
> @@ -3824,7 +3804,6 @@ case "$target_arch2" in
> ;;
> xtensa|xtensaeb)
> TARGET_ARCH=xtensa
> - target_phys_bits=32
> ;;
> *)
> echo "Unsupported target CPU"
> @@ -3859,7 +3838,6 @@ echo "TARGET_ABI_DIR=$TARGET_ABI_DIR" >> $config_target_mak
> case "$target_arch2" in
> i386|x86_64)
> if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
> - target_phys_bits=64
> echo "CONFIG_XEN=y" >> $config_target_mak
> if test "$xen_pci_passthrough" = yes; then
> echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
> @@ -3899,11 +3877,10 @@ if test "$target_bigendian" = "yes" ; then
> echo "TARGET_WORDS_BIGENDIAN=y" >> $config_target_mak
> fi
> if test "$target_softmmu" = "yes" ; then
> - echo "TARGET_PHYS_ADDR_BITS=$target_phys_bits" >> $config_target_mak
> echo "CONFIG_SOFTMMU=y" >> $config_target_mak
> echo "LIBS+=$libs_softmmu $target_libs_softmmu" >> $config_target_mak
> - echo "HWDIR=../libhw$target_phys_bits" >> $config_target_mak
> - echo "subdir-$target: subdir-libhw$target_phys_bits" >> $config_host_mak
> + echo "HWDIR=../libhw" >> $config_target_mak
> + echo "subdir-$target: subdir-libhw" >> $config_host_mak
> if test "$smartcard_nss" = "yes" ; then
> echo "subdir-$target: subdir-libcacard" >> $config_host_mak
> fi
> @@ -4145,11 +4122,8 @@ for rom in seabios vgabios ; do
> echo "LD=$ld" >> $config_mak
> done
>
> -for hwlib in 32 64; do
> - d=libhw$hwlib
> - symlink "$source_path/Makefile.hw" "$d/Makefile"
> - echo "QEMU_CFLAGS+=-DTARGET_PHYS_ADDR_BITS=$hwlib" > $d/config.mak
> -done
> +d=libhw
> +symlink "$source_path/Makefile.hw" "$d/Makefile"
>
> d=libuser
> symlink "$source_path/Makefile.user" "$d/Makefile"
> diff --git a/cpu-common.h b/cpu-common.h
> index 85548de..c0d27af 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -21,7 +21,7 @@ enum device_endian {
> };
>
> /* address in the RAM (different from a physical address) */
> -#if defined(CONFIG_XEN_BACKEND) && TARGET_PHYS_ADDR_BITS == 64
> +#if defined(CONFIG_XEN_BACKEND)
> typedef uint64_t ram_addr_t;
> # define RAM_ADDR_MAX UINT64_MAX
> # define RAM_ADDR_FMT "%" PRIx64
> diff --git a/dma.h b/dma.h
> index f35c4b6..1a33603 100644
> --- a/dma.h
> +++ b/dma.h
> @@ -31,7 +31,7 @@ struct QEMUSGList {
> DMAContext *dma;
> };
>
> -#if defined(TARGET_PHYS_ADDR_BITS)
> +#ifndef CONFIG_USER_ONLY
>
> /*
> * When an IOMMU is present, bus addresses become distinct from
> diff --git a/hw/hw.h b/hw/hw.h
> index e5cb9bf..16101de 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -4,7 +4,7 @@
>
> #include "qemu-common.h"
>
> -#if defined(TARGET_PHYS_ADDR_BITS) && !defined(NEED_CPU_H)
> +#if !defined(CONFIG_USER_ONLY) && !defined(NEED_CPU_H)
> #include "cpu-common.h"
> #endif
>
> diff --git a/hw/intel-hda.c b/hw/intel-hda.c
> index 127e818..d8e1b23 100644
> --- a/hw/intel-hda.c
> +++ b/hw/intel-hda.c
> @@ -210,13 +210,7 @@ static target_phys_addr_t intel_hda_addr(uint32_t lbase, uint32_t ubase)
> {
> target_phys_addr_t addr;
>
> -#if TARGET_PHYS_ADDR_BITS == 32
> - addr = lbase;
> -#else
> - addr = ubase;
> - addr <<= 32;
> - addr |= lbase;
> -#endif
> + addr = ((uint64_t)ubase << 32) | lbase;
> return addr;
> }
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index 844f1b8..b7c82ee 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -774,11 +774,7 @@ static void rtl8139_write_buffer(RTL8139State *s, const void *buf, int size)
> #define MIN_BUF_SIZE 60
> static inline dma_addr_t rtl8139_addr64(uint32_t low, uint32_t high)
> {
> -#if TARGET_PHYS_ADDR_BITS > 32
> - return low | ((target_phys_addr_t)high << 32);
> -#else
> - return low;
> -#endif
> + return low | ((uint64_t)high << 32);
> }
>
> /* Workaround for buggy guest driver such as linux who allocates rx
> diff --git a/monitor.c b/monitor.c
> index 67064e2..7beac9a 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3259,11 +3259,7 @@ static int64_t expr_unary(Monitor *mon)
> break;
> default:
> errno = 0;
> -#if TARGET_PHYS_ADDR_BITS > 32
> n = strtoull(pch, &p, 0);
> -#else
> - n = strtoul(pch, &p, 0);
> -#endif
> if (errno == ERANGE) {
> expr_error(mon, "number too large");
> }
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index d2664ac..532b114 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -1032,12 +1032,10 @@ static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
> return -1;
> }
> *raddrp = (tlb->RPN & mask) | (address & ~mask);
> -#if (TARGET_PHYS_ADDR_BITS >= 36)
> if (ext) {
> /* Extend the physical address to 36 bits */
> - *raddrp |= (target_phys_addr_t)(tlb->RPN & 0xF) << 32;
> + *raddrp |= (uint64_t)(tlb->RPN & 0xF) << 32;
> }
> -#endif
>
> return 0;
> }
> diff --git a/targphys.h b/targphys.h
> index bd4938f..08cade9 100644
> --- a/targphys.h
> +++ b/targphys.h
> @@ -3,25 +3,10 @@
> #ifndef TARGPHYS_H
> #define TARGPHYS_H
>
> -#ifdef TARGET_PHYS_ADDR_BITS
> +#define TARGET_PHYS_ADDR_BITS 64
> /* target_phys_addr_t is the type of a physical address (its size can
> be different from 'target_ulong'). */
>
> -#if TARGET_PHYS_ADDR_BITS == 32
> -typedef uint32_t target_phys_addr_t;
> -#define TARGET_PHYS_ADDR_MAX UINT32_MAX
> -#define TARGET_FMT_plx "%08x"
> -/* Format strings for printing target_phys_addr_t types.
> - * These are recommended over the less flexible TARGET_FMT_plx,
> - * which is retained for the benefit of existing code.
> - */
> -#define TARGET_PRIdPHYS PRId32
> -#define TARGET_PRIiPHYS PRIi32
> -#define TARGET_PRIoPHYS PRIo32
> -#define TARGET_PRIuPHYS PRIu32
> -#define TARGET_PRIxPHYS PRIx32
> -#define TARGET_PRIXPHYS PRIX32
> -#elif TARGET_PHYS_ADDR_BITS == 64
> typedef uint64_t target_phys_addr_t;
> #define TARGET_PHYS_ADDR_MAX UINT64_MAX
> #define TARGET_FMT_plx "%016" PRIx64
> @@ -31,7 +16,5 @@ typedef uint64_t target_phys_addr_t;
> #define TARGET_PRIuPHYS PRIu64
> #define TARGET_PRIxPHYS PRIx64
> #define TARGET_PRIXPHYS PRIX64
> -#endif
> -#endif
>
> #endif
> --
> 1.7.12
next prev parent reply other threads:[~2012-10-04 13:57 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
2012-10-04 10:42 ` Edgar E. Iglesias
2012-10-04 11:42 ` Max Filippov
2012-10-04 13:56 ` Anthony Liguori [this message]
2012-10-04 16:54 ` Blue Swirl
2012-10-04 17:08 ` Michael Walle
2012-10-04 18:15 ` Stefan Weil
2012-10-07 10:25 ` Avi Kivity
2012-10-05 2:10 ` Anthony Liguori
2012-10-05 5:39 ` Stefan Weil
2012-10-05 15:46 ` Blue Swirl
2012-10-05 16:07 ` Anthony Liguori
2012-10-05 16:45 ` Peter Maydell
2012-10-07 13:16 ` Avi Kivity
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=874nmajq6w.fsf@codemonkey.ws \
--to=anthony@codemonkey.ws \
--cc=aurelien@aurel32.net \
--cc=avi@redhat.com \
--cc=blauwirbel@gmail.com \
--cc=edgar.iglesias@gmail.com \
--cc=jcmvbkbc@gmail.com \
--cc=michael@walle.cc \
--cc=paul@codesourcery.com \
--cc=proljc@gmail.com \
--cc=qemu-devel@nongnu.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.