Patchwork [v2] Make target_phys_addr_t 64 bits unconditionally

login
register
mail settings
Submitter Avi Kivity
Date Oct. 4, 2012, 10:36 a.m.
Message ID <1349346964-4151-1-git-send-email-avi@redhat.com>
Download mbox | patch
Permalink /patch/189086/
State New
Headers show

Comments

Avi Kivity - Oct. 4, 2012, 10:36 a.m.
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>
---

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(-)
Edgar Iglesias - Oct. 4, 2012, 10:42 a.m.
On Thu, Oct 04, 2012 at 12:36:04PM +0200, Avi Kivity wrote:
> 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>
> ---
> 
> 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.
>
 
Hi,

I ran some quick tests on CRIS and MicroBlaze guests. Didn't see anything
going wrong.

Cheers,
Edgar
Max Filippov - Oct. 4, 2012, 11:42 a.m.
On Thu, Oct 4, 2012 at 2:36 PM, Avi Kivity <avi@redhat.com> wrote:
> 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>
> ---
>
> 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.

Xtensa on x86_64 host: OK.
Anthony Liguori - Oct. 4, 2012, 1:56 p.m.
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
Blue Swirl - Oct. 4, 2012, 4:54 p.m.
On Thu, Oct 4, 2012 at 10:36 AM, Avi Kivity <avi@redhat.com> wrote:
> 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>
> ---
>
> 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.

Patch looks OK. After this change common CPU code like cputlb.c, which
only depends on sizeof(target_ulong), could be compiled as a library
like libhw32/libhw64 used to.

>
>  .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
>
Michael Walle - Oct. 4, 2012, 5:08 p.m.
Am Donnerstag 04 Oktober 2012, 12:36:04 schrieb Avi Kivity:
> 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>
> ---
> 
> 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.

For lm32 target:

Tested-by: Michael Walle <michael@walle.cc>
Stefan Weil - Oct. 4, 2012, 6:15 p.m.
Am 04.10.2012 12:36, schrieb Avi Kivity:
> 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>
> ---
>
> 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(-)
>


Hi,

I noticed that you replaced target_phys_addr_t by uint64_t in two lines.
Are there plans to replace target_phys_addr_t everywhere?
Should new code use uint64_t, or should it continue to use 
target_phys_addr_t?

Using target_phys_addr_t might make support for 128 bit in some years easier
because it allows identifying critical code,
although I think it will be difficult to avoid wrong use of either 
target_phys_addr_t
or uint64_t as long as both are the same size.

Regards

Stefan W.


> -#if TARGET_PHYS_ADDR_BITS > 32
> -    return low | ((target_phys_addr_t)high << 32);
> -#else
> -    return low;
> -#endif
> +    return low | ((uint64_t)high << 32);
>

> -        *raddrp |= (target_phys_addr_t)(tlb->RPN & 0xF) << 32;
> +        *raddrp |= (uint64_t)(tlb->RPN & 0xF) << 32;
Anthony Liguori - Oct. 5, 2012, 2:10 a.m.
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>

Applied. Thanks.

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
Stefan Weil - Oct. 5, 2012, 5:39 a.m.
Am 05.10.2012 04:10, schrieb Anthony Liguori:
> 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>
> Applied. Thanks.
>
> Regards,
>
> Anthony Liguori
>


In a next step, we can remove libhw completely:

All files from libhw/hw/*.o could as well be generated in hw/*.o,
and hw-obj should become common-obj.

Or is there still a reason why libhw is needed?

Regards

Stefan W.
Blue Swirl - Oct. 5, 2012, 3:46 p.m.
On Fri, Oct 5, 2012 at 5:39 AM, Stefan Weil <sw@weilnetz.de> wrote:
> Am 05.10.2012 04:10, schrieb Anthony Liguori:
>
>> 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>
>>
>> Applied. Thanks.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>
>
> In a next step, we can remove libhw completely:
>
> All files from libhw/hw/*.o could as well be generated in hw/*.o,
> and hw-obj should become common-obj.
>
> Or is there still a reason why libhw is needed?

At least the trivial change to Makefile.objs does not work.

>
> Regards
>
> Stefan W.
>
Anthony Liguori - Oct. 5, 2012, 4:07 p.m.
Blue Swirl <blauwirbel@gmail.com> writes:

> On Fri, Oct 5, 2012 at 5:39 AM, Stefan Weil <sw@weilnetz.de> wrote:
>> Am 05.10.2012 04:10, schrieb Anthony Liguori:
>>
>>> 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>
>>>
>>> Applied. Thanks.
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>
>>
>> In a next step, we can remove libhw completely:
>>
>> All files from libhw/hw/*.o could as well be generated in hw/*.o,
>> and hw-obj should become common-obj.
>>
>> Or is there still a reason why libhw is needed?
>
> At least the trivial change to Makefile.objs does not work.

It's a little more complicated than that but not that hard.

I just sent a patch.

Regards,

Anthony Liguori

>
>>
>> Regards
>>
>> Stefan W.
>>
Peter Maydell - Oct. 5, 2012, 4:45 p.m.
On 4 October 2012 11:36, Avi Kivity <avi@redhat.com> wrote:
> 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').  */

I've just noticed that this change means that linux-user binaries
now get a definition of target_phys_addr_t (where previously they
did not get that type at all). Was this intentional, and does it
make sense?

-- PMM
Avi Kivity - Oct. 7, 2012, 10:25 a.m.
On 10/04/2012 08:15 PM, Stefan Weil wrote:
> Am 04.10.2012 12:36, schrieb Avi Kivity:
>> 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>
> 
> Hi,
> 
> I noticed that you replaced target_phys_addr_t by uint64_t in two lines.

In those two lines, int64_t is more correct than t_p_a_t.  Explanation
below.

> Are there plans to replace target_phys_addr_t everywhere?

Yes, by hw_addr (or hwaddr, or phys, or ...).

> Should new code use uint64_t, or should it continue to use
> target_phys_addr_t?

target_phys_addr_t.

> 
> Using target_phys_addr_t might make support for 128 bit in some years
> easier
> because it allows identifying critical code,

Agree.

> although I think it will be difficult to avoid wrong use of either
> target_phys_addr_t
> or uint64_t as long as both are the same size.

Some languages allow enforcing this, but C doesn't.

> 
> 
>> -#if TARGET_PHYS_ADDR_BITS > 32
>> -    return low | ((target_phys_addr_t)high << 32);
>> -#else
>> -    return low;
>> -#endif
>> +    return low | ((uint64_t)high << 32);
>>

Shifting by 32 is not defined for types that are 32 bits or narrower.
On x86, for example, a shift by 32 of a 32-bit quantity is the identity
operation (where mathematically you would expect the result to be 0, not
the first argument).  Since the context does not guarantee that
target_phys_addr_t is wider than 32 bits, I cast it to a known-wide
type, then (implicitly) cast it back to target_phys_addr_t.

> 
>> -        *raddrp |= (target_phys_addr_t)(tlb->RPN & 0xF) << 32;
>> +        *raddrp |= (uint64_t)(tlb->RPN & 0xF) << 32;
> 

Same applies here, of course.

Since target_phys_addr_t is 64 bits, it would have worked "by accident",
but if we'd have switched back to 32 bits in the future (unlikely but
possible) then I would have introduced a bug here.
Avi Kivity - Oct. 7, 2012, 1:16 p.m.
On 10/05/2012 06:45 PM, Peter Maydell wrote:
> On 4 October 2012 11:36, Avi Kivity <avi@redhat.com> wrote:
>> 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').  */
> 
> I've just noticed that this change means that linux-user binaries
> now get a definition of target_phys_addr_t (where previously they
> did not get that type at all). Was this intentional, 

No.

> and does it make sense?

Not much.  Not very harmful either.  If you want it removed, I can post
a patch.

Patch

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