* [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
@ 2012-10-04 10:36 Avi Kivity
2012-10-04 10:42 ` Edgar E. Iglesias
` (7 more replies)
0 siblings, 8 replies; 14+ messages in thread
From: Avi Kivity @ 2012-10-04 10:36 UTC (permalink / raw)
To: Blue Swirl, Anthony Liguori, qemu-devel
Cc: Jia Liu, Max Filippov, Michael Walle, Paul Brook,
Edgar E. Iglesias, Aurelien Jarno
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(-)
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
@ 2012-10-04 10:42 ` Edgar E. Iglesias
2012-10-04 11:42 ` Max Filippov
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Edgar E. Iglesias @ 2012-10-04 10:42 UTC (permalink / raw)
To: Avi Kivity
Cc: Jia Liu, qemu-devel, Blue Swirl, Max Filippov, Michael Walle,
Paul Brook, Anthony Liguori, Aurelien Jarno
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
2012-10-04 10:42 ` Edgar E. Iglesias
@ 2012-10-04 11:42 ` Max Filippov
2012-10-04 13:56 ` Anthony Liguori
` (5 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Max Filippov @ 2012-10-04 11:42 UTC (permalink / raw)
To: Avi Kivity
Cc: Jia Liu, qemu-devel, Blue Swirl, Michael Walle, Paul Brook,
Anthony Liguori, Edgar E. Iglesias, Aurelien Jarno
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.
--
Thanks.
-- Max
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
2012-10-04 10:42 ` Edgar E. Iglesias
2012-10-04 11:42 ` Max Filippov
@ 2012-10-04 13:56 ` Anthony Liguori
2012-10-04 16:54 ` Blue Swirl
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Anthony Liguori @ 2012-10-04 13:56 UTC (permalink / raw)
To: Avi Kivity, Blue Swirl, qemu-devel
Cc: Jia Liu, Max Filippov, Michael Walle, Paul Brook,
Edgar E. Iglesias, Aurelien Jarno
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
` (2 preceding siblings ...)
2012-10-04 13:56 ` Anthony Liguori
@ 2012-10-04 16:54 ` Blue Swirl
2012-10-04 17:08 ` Michael Walle
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Blue Swirl @ 2012-10-04 16:54 UTC (permalink / raw)
To: Avi Kivity
Cc: Jia Liu, qemu-devel, Max Filippov, Michael Walle, Paul Brook,
Anthony Liguori, Edgar E. Iglesias, Aurelien Jarno
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
` (3 preceding siblings ...)
2012-10-04 16:54 ` Blue Swirl
@ 2012-10-04 17:08 ` Michael Walle
2012-10-04 18:15 ` Stefan Weil
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Michael Walle @ 2012-10-04 17:08 UTC (permalink / raw)
To: Avi Kivity
Cc: Jia Liu, qemu-devel, Blue Swirl, Max Filippov, Paul Brook,
Anthony Liguori, Edgar E. Iglesias, Aurelien Jarno
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>
--
michael
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
` (4 preceding siblings ...)
2012-10-04 17:08 ` Michael Walle
@ 2012-10-04 18:15 ` Stefan Weil
2012-10-07 10:25 ` Avi Kivity
2012-10-05 2:10 ` Anthony Liguori
2012-10-05 16:45 ` Peter Maydell
7 siblings, 1 reply; 14+ messages in thread
From: Stefan Weil @ 2012-10-04 18:15 UTC (permalink / raw)
To: Avi Kivity
Cc: Jia Liu, qemu-devel, Blue Swirl, Max Filippov, Michael Walle,
Paul Brook, Anthony Liguori, Edgar E. Iglesias, Aurelien Jarno
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;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
` (5 preceding siblings ...)
2012-10-04 18:15 ` Stefan Weil
@ 2012-10-05 2:10 ` Anthony Liguori
2012-10-05 5:39 ` Stefan Weil
2012-10-05 16:45 ` Peter Maydell
7 siblings, 1 reply; 14+ messages in thread
From: Anthony Liguori @ 2012-10-05 2:10 UTC (permalink / raw)
To: Avi Kivity, Blue Swirl, qemu-devel
Cc: Jia Liu, Max Filippov, Michael Walle, Paul Brook,
Edgar E. Iglesias, Aurelien Jarno
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-05 2:10 ` Anthony Liguori
@ 2012-10-05 5:39 ` Stefan Weil
2012-10-05 15:46 ` Blue Swirl
0 siblings, 1 reply; 14+ messages in thread
From: Stefan Weil @ 2012-10-05 5:39 UTC (permalink / raw)
To: Anthony Liguori
Cc: Jia Liu, qemu-devel, Blue Swirl, Max Filippov, Michael Walle,
Avi Kivity, Edgar E. Iglesias, Aurelien Jarno, Paul Brook
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-05 5:39 ` Stefan Weil
@ 2012-10-05 15:46 ` Blue Swirl
2012-10-05 16:07 ` Anthony Liguori
0 siblings, 1 reply; 14+ messages in thread
From: Blue Swirl @ 2012-10-05 15:46 UTC (permalink / raw)
To: Stefan Weil
Cc: Jia Liu, qemu-devel, Max Filippov, Michael Walle, Avi Kivity,
Anthony Liguori, Edgar E. Iglesias, Aurelien Jarno, Paul Brook
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.
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-05 15:46 ` Blue Swirl
@ 2012-10-05 16:07 ` Anthony Liguori
0 siblings, 0 replies; 14+ messages in thread
From: Anthony Liguori @ 2012-10-05 16:07 UTC (permalink / raw)
To: Blue Swirl, Stefan Weil
Cc: Jia Liu, qemu-devel, Max Filippov, Michael Walle, Paul Brook,
Edgar E. Iglesias, Aurelien Jarno, Avi Kivity
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.
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
` (6 preceding siblings ...)
2012-10-05 2:10 ` Anthony Liguori
@ 2012-10-05 16:45 ` Peter Maydell
2012-10-07 13:16 ` Avi Kivity
7 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2012-10-05 16:45 UTC (permalink / raw)
To: Avi Kivity
Cc: Jia Liu, qemu-devel, Blue Swirl, Max Filippov, Michael Walle,
Paul Brook, Anthony Liguori, Edgar E. Iglesias, Aurelien Jarno
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-04 18:15 ` Stefan Weil
@ 2012-10-07 10:25 ` Avi Kivity
0 siblings, 0 replies; 14+ messages in thread
From: Avi Kivity @ 2012-10-07 10:25 UTC (permalink / raw)
To: Stefan Weil
Cc: Jia Liu, qemu-devel, Blue Swirl, Max Filippov, Michael Walle,
Paul Brook, Anthony Liguori, Edgar E. Iglesias, Aurelien Jarno
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.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally
2012-10-05 16:45 ` Peter Maydell
@ 2012-10-07 13:16 ` Avi Kivity
0 siblings, 0 replies; 14+ messages in thread
From: Avi Kivity @ 2012-10-07 13:16 UTC (permalink / raw)
To: Peter Maydell
Cc: Jia Liu, qemu-devel, Blue Swirl, Max Filippov, Michael Walle,
Paul Brook, Anthony Liguori, Edgar E. Iglesias, Aurelien Jarno
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.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-10-07 13:16 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-04 10:36 [Qemu-devel] [PATCH v2] Make target_phys_addr_t 64 bits unconditionally Avi Kivity
2012-10-04 10:42 ` Edgar E. Iglesias
2012-10-04 11:42 ` Max Filippov
2012-10-04 13:56 ` Anthony Liguori
2012-10-04 16:54 ` Blue Swirl
2012-10-04 17:08 ` Michael Walle
2012-10-04 18:15 ` Stefan Weil
2012-10-07 10:25 ` Avi Kivity
2012-10-05 2:10 ` Anthony Liguori
2012-10-05 5:39 ` Stefan Weil
2012-10-05 15:46 ` Blue Swirl
2012-10-05 16:07 ` Anthony Liguori
2012-10-05 16:45 ` Peter Maydell
2012-10-07 13:16 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).