[PATCH v22 17/17] i386: move cpu_load_efer into sysemu-only section of cpu.h

Claudio Fontana posted 17 patches 4 years, 11 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Richard Henderson <richard.henderson@linaro.org>
There is a newer version of this series
[PATCH v22 17/17] i386: move cpu_load_efer into sysemu-only section of cpu.h
Posted by Claudio Fontana 4 years, 11 months ago
cpu_load_efer is now used only for sysemu code.

Therefore, make this inline function not visible anymore
in CONFIG_USER_ONLY builds.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 target/i386/cpu.h | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 770c833363..3ccf28b443 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1957,6 +1957,22 @@ static inline AddressSpace *cpu_addressspace(CPUState *cs, MemTxAttrs attrs)
     return cpu_get_address_space(cs, cpu_asidx_from_attrs(cs, attrs));
 }
 
+/*
+ * load efer and update the corresponding hflags. XXX: do consistency
+ * checks with cpuid bits?
+ */
+static inline void cpu_load_efer(CPUX86State *env, uint64_t val)
+{
+    env->efer = val;
+    env->hflags &= ~(HF_LMA_MASK | HF_SVME_MASK);
+    if (env->efer & MSR_EFER_LMA) {
+        env->hflags |= HF_LMA_MASK;
+    }
+    if (env->efer & MSR_EFER_SVME) {
+        env->hflags |= HF_SVME_MASK;
+    }
+}
+
 uint8_t x86_ldub_phys(CPUState *cs, hwaddr addr);
 uint32_t x86_lduw_phys(CPUState *cs, hwaddr addr);
 uint32_t x86_ldl_phys(CPUState *cs, hwaddr addr);
@@ -2053,21 +2069,6 @@ static inline uint32_t cpu_compute_eflags(CPUX86State *env)
     return eflags;
 }
 
-
-/* load efer and update the corresponding hflags. XXX: do consistency
-   checks with cpuid bits? */
-static inline void cpu_load_efer(CPUX86State *env, uint64_t val)
-{
-    env->efer = val;
-    env->hflags &= ~(HF_LMA_MASK | HF_SVME_MASK);
-    if (env->efer & MSR_EFER_LMA) {
-        env->hflags |= HF_LMA_MASK;
-    }
-    if (env->efer & MSR_EFER_SVME) {
-        env->hflags |= HF_SVME_MASK;
-    }
-}
-
 static inline MemTxAttrs cpu_get_mem_attrs(CPUX86State *env)
 {
     return ((MemTxAttrs) { .secure = (env->hflags & HF_SMM_MASK) != 0 });
-- 
2.26.2


Re: [PATCH v22 17/17] i386: move cpu_load_efer into sysemu-only section of cpu.h
Posted by Richard Henderson 4 years, 11 months ago
On 2/24/21 5:34 AM, Claudio Fontana wrote:
> cpu_load_efer is now used only for sysemu code.
> 
> Therefore, make this inline function not visible anymore
> in CONFIG_USER_ONLY builds.
> 
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  target/i386/cpu.h | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)

Perhaps move to cpu-internal.h?  It is not used outside of target/i386/.

Or declared in cpu-internal.h and placed in cpu-sysemu.c?  I don't see that
it's particularly performance sensitive either.

But one way or the other,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v22 17/17] i386: move cpu_load_efer into sysemu-only section of cpu.h
Posted by Claudio Fontana 4 years, 11 months ago
On 2/25/21 5:28 AM, Richard Henderson wrote:
> On 2/24/21 5:34 AM, Claudio Fontana wrote:
>> cpu_load_efer is now used only for sysemu code.
>>
>> Therefore, make this inline function not visible anymore
>> in CONFIG_USER_ONLY builds.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>>  target/i386/cpu.h | 31 ++++++++++++++++---------------
>>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> Perhaps move to cpu-internal.h?  It is not used outside of target/i386/.
> 
> Or declared in cpu-internal.h and placed in cpu-sysemu.c?  I don't see that
> it's particularly performance sensitive either.
> 
> But one way or the other,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
> 

cpu-sysemu.c (and cpu.c) seems now to be about cpu class, model, properties, and relative functions. Maybe worth writing it down in the header..

the file that seems to contain pertinent content now is "helper.c", which maybe should be renamed..

Ciao,

Claudio