From 4cef72d0425c34cca05f1df9796bd57a96318310 Mon Sep 17 00:00:00 2001 From: Alex Bennée Date: Mon, 21 Oct 2019 16:09:10 +0100 Subject: cputlb: ensure _cmmu helper functions follow the naming standard We document this in docs/devel/load-stores.rst so lets follow it. The 32 bit and 64 bit access functions have historically not included the sign so we leave those as is. We also introduce some signed helpers which are used for loading immediate values in the translator. Fixes: 282dffc8 Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20191021150910.23216-1-alex.bennee@linaro.org> Signed-off-by: Richard Henderson --- include/exec/cpu_ldst_template.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include/exec') diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h index af7e0b49f2..3d24ed9bd0 100644 --- a/include/exec/cpu_ldst_template.h +++ b/include/exec/cpu_ldst_template.h @@ -65,8 +65,8 @@ #ifdef SOFTMMU_CODE_ACCESS #define ADDR_READ addr_code #define MMUSUFFIX _cmmu -#define URETSUFFIX SUFFIX -#define SRETSUFFIX SUFFIX +#define URETSUFFIX USUFFIX +#define SRETSUFFIX glue(s, SUFFIX) #else #define ADDR_READ addr_read #define MMUSUFFIX _mmu -- cgit v1.2.3-55-g7522 From 50276a79aa15713decfede5cab183fc371e3e57d Mon Sep 17 00:00:00 2001 From: Wei Yang Date: Sun, 13 Oct 2019 10:11:44 +0800 Subject: cpu: use ROUND_UP() to define xxx_PAGE_ALIGN Use ROUND_UP() to define, which is a little bit easy to read. Reviewed-by: Alex Bennée Reviewed-by: Michael S. Tsirkin Reviewed-by: David Gibson Reviewed-by: Juan Quintela Signed-off-by: Wei Yang Message-Id: <20191013021145.16011-2-richardw.yang@linux.intel.com> Signed-off-by: Richard Henderson --- include/exec/cpu-all.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'include/exec') diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index ad9ab85eb3..255bb186ac 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -220,7 +220,7 @@ extern int target_page_bits; #define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS) #define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1) -#define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK) +#define TARGET_PAGE_ALIGN(addr) ROUND_UP((addr), TARGET_PAGE_SIZE) /* Using intptr_t ensures that qemu_*_page_mask is sign-extended even * when intptr_t is 32-bit and we are aligning a long long. @@ -228,9 +228,8 @@ extern int target_page_bits; extern uintptr_t qemu_host_page_size; extern intptr_t qemu_host_page_mask; -#define HOST_PAGE_ALIGN(addr) (((addr) + qemu_host_page_size - 1) & qemu_host_page_mask) -#define REAL_HOST_PAGE_ALIGN(addr) (((addr) + qemu_real_host_page_size - 1) & \ - qemu_real_host_page_mask) +#define HOST_PAGE_ALIGN(addr) ROUND_UP((addr), qemu_host_page_size) +#define REAL_HOST_PAGE_ALIGN(addr) ROUND_UP((addr), qemu_real_host_page_size) /* same as PROT_xxx */ #define PAGE_READ 0x0001 -- cgit v1.2.3-55-g7522 From bbc17caf81f94523e67c64a6e6cd8f869b7a1da5 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Fri, 13 Sep 2019 11:21:53 -0400 Subject: exec: Use const alias for TARGET_PAGE_BITS_VARY Using a variable that is declared "const" for this tells the compiler that it may read the value once and assume that it does not change across function calls. For target_page_size, this means we have only one assert per function, and one read of the variable. This reduces the size of qemu-system-aarch64 by 8k. Reviewed-by: Alex Bennée Reviewed-by: Paolo Bonzini Signed-off-by: Richard Henderson --- exec-vary.c | 66 ++++++++++++++++++++++++++++++++++++++++++++------ include/exec/cpu-all.h | 14 ++++++++--- 2 files changed, 68 insertions(+), 12 deletions(-) (limited to 'include/exec') diff --git a/exec-vary.c b/exec-vary.c index 48c0ab306c..8725fd0285 100644 --- a/exec-vary.c +++ b/exec-vary.c @@ -19,11 +19,55 @@ #include "qemu/osdep.h" #include "qemu-common.h" + +#define IN_EXEC_VARY 1 + #include "exec/exec-all.h" #ifdef TARGET_PAGE_BITS_VARY -int target_page_bits; -bool target_page_bits_decided; +# ifdef CONFIG_ATTRIBUTE_ALIAS +/* + * We want to declare the "target_page" variable as const, which tells + * the compiler that it can cache any value that it reads across calls. + * This avoids multiple assertions and multiple reads within any one user. + * + * This works because we finish initializing the data before we ever read + * from the "target_page" symbol. + * + * This also requires that we have a non-constant symbol by which we can + * perform the actual initialization, and which forces the data to be + * allocated within writable memory. Thus "init_target_page", and we use + * that symbol exclusively in the two functions that initialize this value. + * + * The "target_page" symbol is created as an alias of "init_target_page". + */ +static TargetPageBits init_target_page; + +/* + * Note that this is *not* a redundant decl, this is the definition of + * the "target_page" symbol. The syntax for this definition requires + * the use of the extern keyword. This seems to be a GCC bug in + * either the syntax for the alias attribute or in -Wredundant-decls. + * + * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91765 + */ +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wredundant-decls" + +extern const TargetPageBits target_page + __attribute__((alias("init_target_page"))); + +# pragma GCC diagnostic pop +# else +/* + * When aliases are not supported then we force two different declarations, + * by way of suppressing the header declaration with IN_EXEC_VARY. + * We assume that on such an old compiler, LTO cannot be used, and so the + * compiler cannot not detect the mismatched declarations, and all is well. + */ +TargetPageBits target_page; +# define init_target_page target_page +# endif #endif bool set_preferred_target_page_bits(int bits) @@ -36,11 +80,11 @@ bool set_preferred_target_page_bits(int bits) */ #ifdef TARGET_PAGE_BITS_VARY assert(bits >= TARGET_PAGE_BITS_MIN); - if (target_page_bits == 0 || target_page_bits > bits) { - if (target_page_bits_decided) { + if (init_target_page.bits == 0 || init_target_page.bits > bits) { + if (init_target_page.decided) { return false; } - target_page_bits = bits; + init_target_page.bits = bits; } #endif return true; @@ -49,9 +93,15 @@ bool set_preferred_target_page_bits(int bits) void finalize_target_page_bits(void) { #ifdef TARGET_PAGE_BITS_VARY - if (target_page_bits == 0) { - target_page_bits = TARGET_PAGE_BITS_MIN; + if (init_target_page.bits == 0) { + init_target_page.bits = TARGET_PAGE_BITS_MIN; } - target_page_bits_decided = true; + init_target_page.decided = true; + + /* + * For the benefit of an -flto build, prevent the compiler from + * hoisting a read from target_page before we finish initializing. + */ + barrier(); #endif } diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index 255bb186ac..0b449b98ba 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -210,10 +210,16 @@ static inline void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val /* page related stuff */ #ifdef TARGET_PAGE_BITS_VARY -extern bool target_page_bits_decided; -extern int target_page_bits; -#define TARGET_PAGE_BITS ({ assert(target_page_bits_decided); \ - target_page_bits; }) +typedef struct { + bool decided; + int bits; +} TargetPageBits; +#if defined(CONFIG_ATTRIBUTE_ALIAS) || !defined(IN_EXEC_VARY) +extern const TargetPageBits target_page; +#else +extern TargetPageBits target_page; +#endif +#define TARGET_PAGE_BITS ({ assert(target_page.decided); target_page.bits; }) #else #define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS #endif -- cgit v1.2.3-55-g7522 From 639044b5f121a44909eca929d7fd6d070024b9f9 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Fri, 13 Sep 2019 11:41:51 -0400 Subject: exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG This reduces the size of a release build by about 10k. Noticably, within the tlb miss helpers. Reviewed-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Paolo Bonzini Signed-off-by: Richard Henderson --- include/exec/cpu-all.h | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'include/exec') diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index 0b449b98ba..525059970c 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -219,8 +219,12 @@ extern const TargetPageBits target_page; #else extern TargetPageBits target_page; #endif +#ifdef CONFIG_DEBUG_TCG #define TARGET_PAGE_BITS ({ assert(target_page.decided); target_page.bits; }) #else +#define TARGET_PAGE_BITS target_page.bits +#endif +#else #define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS #endif -- cgit v1.2.3-55-g7522 From f048b8a7cefcabb0e55210a76775f9be57c4d3f4 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Tue, 15 Oct 2019 11:10:27 -0700 Subject: exec: Promote TARGET_PAGE_MASK to target_long There are some uint64_t uses that expect TARGET_PAGE_MASK to extend for a 32-bit, so this must continue to be a signed type. Define based on TARGET_PAGE_BITS not TARGET_PAGE_SIZE; this will make a following patch more clear. This should not have a functional effect so far. Reviewed-by: Alex Bennée Reviewed-by: Paolo Bonzini Signed-off-by: Richard Henderson --- include/exec/cpu-all.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/exec') diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index 525059970c..0543359d0f 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -229,7 +229,7 @@ extern TargetPageBits target_page; #endif #define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS) -#define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1) +#define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS) #define TARGET_PAGE_ALIGN(addr) ROUND_UP((addr), TARGET_PAGE_SIZE) /* Using intptr_t ensures that qemu_*_page_mask is sign-extended even -- cgit v1.2.3-55-g7522 From bb8e3ea6fa8f616678133c9e8c8c3bf232c179ec Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Fri, 13 Sep 2019 12:07:40 -0400 Subject: exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY This eliminates a set of runtime shifts. It turns out that we require TARGET_PAGE_MASK more often than TARGET_PAGE_SIZE, so redefine TARGET_PAGE_SIZE based on TARGET_PAGE_MASK instead of the other way around. Reviewed-by: Alex Bennée Reviewed-by: David Hildenbrand Reviewed-by: Paolo Bonzini Signed-off-by: Richard Henderson --- exec-vary.c | 1 + include/exec/cpu-all.h | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'include/exec') diff --git a/exec-vary.c b/exec-vary.c index 8725fd0285..ff905f2a8f 100644 --- a/exec-vary.c +++ b/exec-vary.c @@ -96,6 +96,7 @@ void finalize_target_page_bits(void) if (init_target_page.bits == 0) { init_target_page.bits = TARGET_PAGE_BITS_MIN; } + init_target_page.mask = (target_long)-1 << init_target_page.bits; init_target_page.decided = true; /* diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index 0543359d0f..e96781a455 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -213,6 +213,7 @@ static inline void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val typedef struct { bool decided; int bits; + target_long mask; } TargetPageBits; #if defined(CONFIG_ATTRIBUTE_ALIAS) || !defined(IN_EXEC_VARY) extern const TargetPageBits target_page; @@ -221,15 +222,18 @@ extern TargetPageBits target_page; #endif #ifdef CONFIG_DEBUG_TCG #define TARGET_PAGE_BITS ({ assert(target_page.decided); target_page.bits; }) +#define TARGET_PAGE_MASK ({ assert(target_page.decided); target_page.mask; }) #else #define TARGET_PAGE_BITS target_page.bits +#define TARGET_PAGE_MASK target_page.mask #endif +#define TARGET_PAGE_SIZE (-(int)TARGET_PAGE_MASK) #else #define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS +#define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS) +#define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS) #endif -#define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS) -#define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS) #define TARGET_PAGE_ALIGN(addr) ROUND_UP((addr), TARGET_PAGE_SIZE) /* Using intptr_t ensures that qemu_*_page_mask is sign-extended even -- cgit v1.2.3-55-g7522