| Message ID | 1349346964-4151-1-git-send-email-avi@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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.
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
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 >
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>
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;
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
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.
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. >
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. >>
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
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.
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.
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
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(-)